Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fzf-tmux disables remain-on-exit for all other panes in window #3397

Closed
8 of 13 tasks
ss-raicangu opened this issue Aug 13, 2023 · 8 comments · Fixed by #3410
Closed
8 of 13 tasks

fzf-tmux disables remain-on-exit for all other panes in window #3397

ss-raicangu opened this issue Aug 13, 2023 · 8 comments · Fixed by #3410
Labels

Comments

@ss-raicangu
Copy link
Contributor

ss-raicangu commented Aug 13, 2023

  • I have read through the manual page (man fzf)
    • I have read through the fzf-tmux script
  • I have the latest version of fzf
  • I have searched through the existing issues
    • Search: is:issue remain-on-exit

Info

  • OS
    • Linux
    • Mac OS X Ventura 13.4.1
    • Windows
    • Etc.
  • Shell
    • bash
    • zsh 5.9 (x86_64-apple-darwin22.0)
    • fish
  • App
    • fzf-tmux (with fzf 0.42.0 (brew))
    • tmux 3.3a

Problem / Steps to reproduce

Using fzf-tmux overwrites remain-on-exit for all panes in a window, if it is only set globally or at a higher scope than window.

set-option -wg remain-on-exit on
set-option -s remain-on-exit on

This makes other panes in that window close immediately on exit after using fzf-tmux, even though I expect them to remain open.

Potential solution

Since TMux 3.0 [1][2], remain-on-exit and synchronize-panes are pane options that can be set via set-option -p. The following patch [3] solves this issue on my environment and could be an acceptable solution, depending on how backwards-compatible this script is meant to be.

diff --git a/bin/fzf-tmux b/bin/fzf-tmux
index e891b78..e86b44b 100755
--- a/bin/fzf-tmux
+++ b/bin/fzf-tmux
@@ -151,12 +151,15 @@ argsf="${TMPDIR:-/tmp}/fzf-args-$id"
 fifo1="${TMPDIR:-/tmp}/fzf-fifo1-$id"
 fifo2="${TMPDIR:-/tmp}/fzf-fifo2-$id"
 fifo3="${TMPDIR:-/tmp}/fzf-fifo3-$id"
-tmux_win_opts=( $(tmux show-window-options remain-on-exit \; show-window-options synchronize-panes | sed '/ off/d; s/^/set-window-option /; s/$/ \\;/') )
+tmux_version=$(tmux -V | sed 's/[^0-9.]//g')
+if [[ "$tmux_version" != 3* ]]; then
+  tmux_win_opts=( $(tmux show-window-options remain-on-exit \; show-window-options synchronize-panes | sed '/ off/d; s/^/set-window-option /; s/$/ \\;/') )
+fi
 cleanup() {
   \rm -f $argsf $fifo1 $fifo2 $fifo3
 
-  # Restore tmux window options
-  if [[ "${#tmux_win_opts[@]}" -gt 0 ]]; then
+  # Restore tmux window options, if set
+  if [[ "$tmux_version" != 3* && "${#tmux_win_opts[@]}" -gt 0 ]]; then
     eval "tmux ${tmux_win_opts[*]}"
   fi
 
@@ -179,7 +182,6 @@ trap 'cleanup' EXIT
 
 envs="export TERM=$TERM "
 if [[ "$opt" =~ "-E" ]]; then
-  tmux_version=$(tmux -V | sed 's/[^0-9.]//g')
   if [[ $(awk '{print ($1 > 3.2)}' <<< "$tmux_version" 2> /dev/null || bc -l <<< "$tmux_version > 3.2") = 1 ]]; then
     FZF_DEFAULT_OPTS="--border $FZF_DEFAULT_OPTS"
     opt="-B $opt"
@@ -226,9 +228,18 @@ else
   cat <<< "\"$fzf\" $opts < $fifo1 > $fifo2; echo \$? > $fifo3 $close" >> $argsf
   cat <&0 > $fifo1 &
 fi
-tmux set-window-option synchronize-panes off \;\
-  set-window-option remain-on-exit off \;\
-  split-window -c "$PWD" $opt "bash -c 'exec -a fzf bash $argsf'" $swap \
+
+tmux \
+  split-window -c "$PWD" $opt "bash -c 'exec -a fzf bash $argsf'" $swap \
   > /dev/null 2>&1 || { "$fzf" "${args[@]}"; exit $?; }
+# Pane options (`set-option -p`) were added in version 3
+# Link: https://github.com/tmux/tmux/blob/11e69f6025f5783fe17d43247de1c3f659a19b69/CHANGES#L753-L760
+if [[ "$tmux_version" != 3* ]]; then
+  tmux set-window-option synchronize-panes off \;\
+    set-window-option remain-on-exit off > /dev/null 2>&1
+else
+  tmux set-option -p synchronize-panes off \;\
+    set-option -p remain-on-exit off > /dev/null 2>&1
+fi
+
 cat $fifo2
 exit "$(cat $fifo3)"

[1]: TMux changelog detailing pane options being added in version 3.0.
[2]: TMux 3.0 was released on 2019-05-29.
[3]: Filepaths in the patch header were manually edited, since I tested these changes by copying the fzf-tmux script directly out of /usr/local/bin rather than cloning the junegunn/fzf repo.
Related: 2a2c0a0 ([fzf-tmux] Turn off remain-on-exit option, 2016-01-07)

@ss-raicangu
Copy link
Contributor Author

Aware this is a niche issue, so thanks in advance for even considering it 🙂 .

@junegunn junegunn added the bug label Aug 14, 2023
@junegunn
Copy link
Owner

junegunn commented Aug 21, 2023

Can you test this patch? It will first try using the new method, but if it fails, it will try again using the old method.

diff --git a/bin/fzf-tmux b/bin/fzf-tmux
index ed255fc..ca8ae32 100755
--- a/bin/fzf-tmux
+++ b/bin/fzf-tmux
@@ -151,7 +151,13 @@ argsf="${TMPDIR:-/tmp}/fzf-args-$id"
 fifo1="${TMPDIR:-/tmp}/fzf-fifo1-$id"
 fifo2="${TMPDIR:-/tmp}/fzf-fifo2-$id"
 fifo3="${TMPDIR:-/tmp}/fzf-fifo3-$id"
-tmux_win_opts=( $(tmux show-window-options remain-on-exit \; show-window-options synchronize-panes | sed '/ off/d; s/^/set-window-option /; s/$/ \\;/') )
+if tmux_win_opts=$(tmux show-options -p remain-on-exit \; show-options -w synchronize-panes 2> /dev/null); then
+  tmux_win_opts=( $(sed '/ off/d; s/synchronize-panes/set-option -w synchronize-panes/; s/remain-on-exit/set-option -p remain-on-exit/; s/$/ \\;/' <<< "$tmux_win_opts") )
+  tmux_off_opts='set-option -w synchronize-panes off ; set-option -p remain-on-exit off ;'
+else
+  tmux_win_opts=( $(tmux show-window-options remain-on-exit \; show-window-options synchronize-panes | sed '/ off/d; s/^/set-window-option /; s/$/ \\;/') )
+  tmux_off_opts='set-window-option synchronize-panes off ; set-window-option remain-on-exit off ;'
+fi
 cleanup() {
   \rm -f $argsf $fifo1 $fifo2 $fifo3
 
@@ -227,8 +233,7 @@ else
   cat <<< "\"$fzf\" $opts < $fifo1 > $fifo2; echo \$? > $fifo3 $close" >> $argsf
   cat <&0 > $fifo1 &
 fi
-tmux set-window-option synchronize-panes off \;\
-  set-window-option remain-on-exit off \;\
+tmux $tmux_off_opts \
   split-window -c "$PWD" $opt "bash -c 'exec -a fzf bash $argsf'" $swap \
   > /dev/null 2>&1 || { "$fzf" "${args[@]}"; exit $?; }
 cat $fifo2

@ss-raicangu
Copy link
Contributor Author

@junegunn, this patch does the opposite of what is desired 😅 . Slightly tweaked patch suggested below.

pane-layout
Fig 1. Pane layouts above. Top is original, bottom is fzf-tmux.

remain-on-exit-off-on-original-pane
Fig 2. show-options -p remain-on-exit shows it being off on original pane but does nothing for fzf-tmux pane.

diff --git a/bin/fzf-tmux b/bin/fzf-tmux
index e891b78..88ddf88 100755
--- a/bin/fzf-tmux
+++ b/bin/fzf-tmux
@@ -151,7 +151,15 @@ argsf="${TMPDIR:-/tmp}/fzf-args-$id"
 fifo1="${TMPDIR:-/tmp}/fzf-fifo1-$id"
 fifo2="${TMPDIR:-/tmp}/fzf-fifo2-$id"
 fifo3="${TMPDIR:-/tmp}/fzf-fifo3-$id"
-tmux_win_opts=( $(tmux show-window-options remain-on-exit \; show-window-options synchronize-panes | sed '/ off/d; s/^/set-window-option /; s/$/ \\;/') )
+if tmux_win_opts=$(tmux show-options -p remain-on-exit \; show-options -w synchronize-panes 2> /dev/null); then
+  tmux_win_opts=( $(sed '/ off/d; s/synchronize-panes/set-option -w synchronize-panes/; s/remain-on-exit/set-option -p remain-on-exit/; s/$/ \\;/' <<< "$tmux_win_opts") )
+  tmux_off_opts='; set-option -w synchronize-panes off ; set-option -p remain-on-exit off'
+else
+  tmux_win_opts=( $(tmux show-window-options remain-on-exit \; show-window-options synchronize-panes | sed '/ off/d; s/^/set-window-option /; s/$/ \\;/') )
+  tmux_off_opts='; set-window-option synchronize-panes off ; set-window-option remain-on-exit off'
+fi
 cleanup() {
   \rm -f $argsf $fifo1 $fifo2 $fifo3
 
@@ -226,9 +234,9 @@ else
   cat <<< "\"$fzf\" $opts < $fifo1 > $fifo2; echo \$? > $fifo3 $close" >> $argsf
   cat <&0 > $fifo1 &
 fi
-tmux set-window-option synchronize-panes off \;\
-  set-window-option remain-on-exit off \;\
+tmux \
   split-window -c "$PWD" $opt "bash -c 'exec -a fzf bash $argsf'" $swap \
+  $tmux_off_opts \
   > /dev/null 2>&1 || { "$fzf" "${args[@]}"; exit $?; }
 cat $fifo2
 exit "$(cat $fifo3)"

Ref 1. Original patch didn't work because TMux executed the $tmux_off_opts and split-window in the wrong order. This suggested patch flips the order they are executed. It also changes the trailing semi-colon to be a leading one in $tmux_off_opts.

@ss-raicangu
Copy link
Contributor Author

Thanks for the quick fix, btw 🙂 .

@ss-raicangu
Copy link
Contributor Author

Also, out of curiosity, why is synchronize-panes enabled or disabled at the window level?

I don't use it, so unsure of the edge-case this is preventing. Also means it doesn't affect me, so any answer appreciated but not required 🙂 🙏 .

synchronize-panes-not-syncing-when-off.mov

Fig 1. When I tried it out, it didn't seem to duplicate input from other synchronized panes in same window if I disabled it for a specific pane (set-option -p synchronize-panes off).

@junegunn
Copy link
Owner

why is synchronize-panes enabled or disabled at the window level?

If it works as expected with the pane-level option, there's no need to use the window-level option.

Slightly tweaked patch suggested below.

Thanks. Can you open a pull request with your patch?

@ss-raicangu
Copy link
Contributor Author

Will do.

@ss-raicangu
Copy link
Contributor Author

ss-raicangu commented Aug 23, 2023

Nevermind 😅 . synchronize-panes does need to be disabled for full window, unless multiple panes are desired.

Edit: Note only, no action required 🙂 . Sorry, thought it was a reason but seems to happen with window-option disabled as well. Will investigate properly before commenting in pull request.

synchronize-panes-creating-multiple-fzf-panes.mov

Fig 2. fzf-tmux creates multiple panes if synchronize-panes is enabled for window and new pane is created afterwards.

Resolved: Not a bug. If panes are synchronized when starting fzf-tmux, then have to accept as desired behaviour to have multiple fzf-tmux panes. What we don't want instead is for input to fzf-tmux pane to be mirrored in original panes. That is why sychronize-panes is turned off.

ss-raicangu added a commit to ss-raicangu/fzf-fork that referenced this issue Aug 23, 2023
Using `fzf-tmux` overwrites `remain-on-exit` for all panes in a window,
if it is only set globally or at a higher scope than window.

	set-option -wg remain-on-exit on
	set-option -s remain-on-exit on

This makes other panes in that window close immediately on exit after
using `fzf-tmux`, even though I expect them to remain open.

Since TMux 3.0, `remain-on-exit` is a pane option that can be set via
`set-option -p`. This will limit the option's scope to just the `fzf`
pane, thus allowing us to close it immediately without overriding
`remain-on-exit` on other panes.

Co-authored-by: Junegunn Choi <[email protected]>
Link: https://github.com/tmux/tmux/blob/11e69f6025f5783fe17d43247de1c3f659a19b69/CHANGES#L753-L760
Link: https://github.com/tmux/tmux/releases/tag/3.0
Related: junegunn#3397
ss-raicangu added a commit to ss-raicangu/fzf-fork that referenced this issue Aug 23, 2023
Similar reason to bb96d1d (fix(fzf-tmux): turn off `remain-on-exit` only
on `fzf` pane, 2023-08-24). Limit scope that option is set to bare
minimum.

Have confirmed this will not feed input back to other panes which are
set to be synchronized. However, note that this will not stop `fzf-tmux`
from being launched by two synchronized panes in parallel.

Link: junegunn#3397 (comment)
ss-raicangu added a commit to ss-raicangu/fzf-fork that referenced this issue Aug 23, 2023
Using `fzf-tmux` overwrites `remain-on-exit` for all panes in a window,
if it is only set globally or at a higher scope than window.

	set-option -wg remain-on-exit on
	set-option -s remain-on-exit on

This makes other panes in that window close immediately on exit after
using `fzf-tmux`, even though I expect them to remain open.

Since TMux 3.0, `remain-on-exit` is a pane option that can be set via
`set-option -p`. This will limit the option's scope to just the
`fzf-tmux` pane, thus allowing us to close it immediately without
overriding `remain-on-exit` on other panes in the window.

Co-authored-by: Junegunn Choi <[email protected]>
Link: https://github.com/tmux/tmux/blob/11e69f6025f5783fe17d43247de1c3f659a19b69/CHANGES#L753-L760
Link: https://github.com/tmux/tmux/releases/tag/3.0
Related: junegunn#3397
ss-raicangu added a commit to ss-raicangu/fzf-fork that referenced this issue Aug 23, 2023
Similar reason to 482fd2b (fix: turn off remain-on-exit only on fzf-tmux
pane, 2023-08-24).

	Limit scope on which option is set to bare minimum.

Have confirmed this will not feed input back to other panes which are
set to be synchronized. However, note that this will not stop `fzf-tmux`
from being launched by two synchronized panes in parallel.

Link: junegunn#3397 (comment)
junegunn added a commit that referenced this issue Aug 24, 2023
* fix: turn off remain-on-exit only on fzf-tmux pane

Using `fzf-tmux` overwrites `remain-on-exit` for all panes in a window,
if it is only set globally or at a higher scope than window.

	set-option -wg remain-on-exit on
	set-option -s remain-on-exit on

This makes other panes in that window close immediately on exit after
using `fzf-tmux`, even though I expect them to remain open.

Since TMux 3.0, `remain-on-exit` is a pane option that can be set via
`set-option -p`. This will limit the option's scope to just the
`fzf-tmux` pane, thus allowing us to close it immediately without
overriding `remain-on-exit` on other panes in the window.

Co-authored-by: Junegunn Choi <[email protected]>
Link: https://github.com/tmux/tmux/blob/11e69f6025f5783fe17d43247de1c3f659a19b69/CHANGES#L753-L760
Link: https://github.com/tmux/tmux/releases/tag/3.0
Related: #3397

* fix: turn off synchronize-panes only on fzf-tmux pane

Similar reason to 482fd2b (fix: turn off remain-on-exit only on fzf-tmux
pane, 2023-08-24).

	Limit scope on which option is set to bare minimum.

Have confirmed this will not feed input back to other panes which are
set to be synchronized. However, note that this will not stop `fzf-tmux`
from being launched by two synchronized panes in parallel.

Link: #3397 (comment)

---------

Co-authored-by: Junegunn Choi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants