refactor main.sh

[isidroas]

• fix copy-mode removal when not in copy mode
• earlier conversion from pipes to newlines
• reduced and more clear code

[sainnhe]

Sorry for late reply. I was just too busy recently (maybe you've heard of 996 in China) and I just had a little time to deal with this :(

[sainnhe]

So the code assumes $TMUX_FZF_ORDER is exactly "copy-mode|session|window|pane|command|keybinding|clipboard|process", but in fact users can customize $TMUX_FZF_ORDER, as described in README. That's why there is a detection:

[[ -z "$TMUX_FZF_ORDER" ]] && TMUX_FZF_ORDER="copy-mode|session|window|pane|command|keybinding|clipboard|process"

The if condition [ "$TMUX_FZF_ORDER" == *"copy-mode"* ] is used to detect if there is a substring "copy-mode" in $TMUX_FZF_ORDER, so it should not be removed, although the functionality didn't change.

[isidroas]

I removed it because it makes the code simpler and the functionality didn't change. With this pr, if the substring copy-mode isn't present in $TMUX_FZF_ORDR sed won't remove anything

[sainnhe]

IMO the functionality didn't change here. Doesn't sed -E 's/\|?copy-mode\|?//' work on your machine?

[isidroas]

With the default value of TMUX_FZF_ORDER , it works fine:

$ echo "copy-mode|session|window|pane|command|keybinding|clipboard|process" | sed -E 's/\|?copy-mode\|?//'
session|window|pane|command|keybinding|clipboard|process

but swapping copy-mode and session,

$ echo "session|copy-mode|window|pane|command|keybinding|clipboard|process" | sed -E 's/\|?copy-mode\|?//'
sessionwindow|pane|command|keybinding|clipboard|process

produces that session and window items are merged without the pipe separator.

The change proposed in #66 fixes it:

$ echo "session|copy-mode|window|pane|command|keybinding|clipboard|process" | sed -E 's/\|?copy-mode\|?/|/'
session|window|pane|command|keybinding|clipboard|process

but as you noticed, the first line could become empty since it could add a leading pipe character

$ echo "copy-mode|session|window|pane|command|keybinding|clipboard|process" | sed -E 's/\|?copy-mode\|?/|/'
|session|window|pane|command|keybinding|clipboard|process

[isidroas]

Hi @sainnhe good to see some activity is this project!

I didn't know about 996, it sounds awful 😞

[sainnhe]

LGTM. Thank you so much for this refactor!

[sainnhe]

After some tests I found a bug: the items will not show in copy mode. I did a quick fix: 608356a

The cause is simple: if you enter copy mode, line 10~12 will be skipped, so the variable items_origin will not be defined until line 17, thus the TMUX_FZF_ORDER variable will be completely unused.

[isidroas]

Thank you for letting me know. Next time I will test more before submitting the pull request.