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

Multiple tmux clients causes buggy inputs #2050

Closed
3 tasks done
musjj opened this issue Dec 15, 2024 · 14 comments · Fixed by #2052 or #2058
Closed
3 tasks done

Multiple tmux clients causes buggy inputs #2050

musjj opened this issue Dec 15, 2024 · 14 comments · Fixed by #2052 or #2058
Labels
bug Something isn't working

Comments

@musjj
Copy link
Contributor

musjj commented Dec 15, 2024

What system are you running Yazi on?

Linux Wayland

What terminal are you running Yazi in?

kitty 0.37.0

yazi --debug output

Yazi
    Version: 0.4.0 (0b340b6 2024-12-09)
    Debug  : false
    Triple : x86_64-unknown-linux-gnu (linux-x86_64)
    Rustc  : 1.82.0 (f6e511ee 2024-10-15)

Ya
    Version: 0.4.0 (0b340b6 2024-12-09)

Emulator
    Brand.from_env      : Some(Kitty)
    Emulator.detect     : Emulator { kind: Left(Kitty), light: false, cell_size: Some((8, 18)) }
    Emulator.detect_full: Ok(Emulator { kind: Left(Kitty), light: false, cell_size: Some((8, 18)) })

Adapter
    Adapter.matches: Kgp

Desktop
    XDG_SESSION_TYPE           : Some("wayland")
    WAYLAND_DISPLAY            : Some("wayland-0")
    DISPLAY                    : Some(":0")
    SWAYSOCK                   : None
    HYPRLAND_INSTANCE_SIGNATURE: None
    WAYFIRE_SOCKET             : None

SSH
    shared.in_ssh_connection: false

WSL
    WSL: false

NVIM
    NVIM          : false
    Neovim version: 0.10.2

Variables
    SHELL           : Some("/run/current-system/sw/bin/zsh")
    EDITOR          : Some("nvim")
    VISUAL          : None
    YAZI_FILE_ONE   : None
    YAZI_CONFIG_HOME: None
    YAZI_ZOXIDE_OPTS: None

Text Opener
    default     : Some(Opener { run: "$EDITOR \"$@\"", block: true, orphan: false, desc: "$EDITOR", for_: None, spread: true })
    block-create: Some(Opener { run: "$EDITOR \"$@\"", block: true, orphan: false, desc: "$EDITOR", for_: None, spread: true })
    block-rename: Some(Opener { run: "$EDITOR \"$@\"", block: true, orphan: false, desc: "$EDITOR", for_: None, spread: true })

Multiplexers
    TMUX               : 2
    tmux version       : tmux 3.5a
    tmux build flags   : enable-sixel=Unknown
    ZELLIJ_SESSION_NAME: None
    Zellij version     : No such file or directory (os error 2)

Dependencies
    file          : 5.45
    ueberzugpp    : No such file or directory (os error 2)
    ffmpeg/ffprobe: 7.1 / 7.1
    pdftoppm      : 24.02.0
    magick        : 7.1.1-39
    fzf           : 0.55.0
    fd/fdfind     : 10.2.0 / No such file or directory (os error 2)
    rg            : 14.1.1
    chafa         : 1.14.4
    zoxide        : 0.9.6
    7z/7zz        : 17.05 / 24.08
    jq            : 1.7.1

Clipboard
    wl-copy/paste: 2.2.1 / 2.2.1
    xclip        : No such file or directory (os error 2)
    xsel         : No such file or directory (os error 2)

Describe the bug

When you have multiple active tmux clients sharing the same group, opening yazi in one of them causes other windows in other clients to receive weird key inputs.

Minimal reproducer

  1. First we set up the sessions:
    # Create a base session
    tmux new-session -s base -d
    
    # Create two new sessions in the same group as the base session.
    # They will share the same windows, but can have different views.
    tmux new-session -t base -s child_1 -d
    tmux new-session -t base -s child_2 -d
  2. Let's open a new terminal window. Here, we attach to the first child.
    tmux attach-session -t child_1
  3. Now we open another terminal window and attach to the second child.
    tmux attach-session -t child_2
    Additionally, create a new tmux window by pressing Ctrl+b, then c.
    Finally, we open yazi in this window.
    You should now see strange characters getting inputted to the first terminal window (child_1). I think this varies depending on certain factors, but in my case I'm seeing: :1$r1 q^[\^[[?12;0$y^[[?62;c.

Anything else?

My tmux version:

$ tmux display-message -p "#{version}"
3.5a

I've also made sure that there's no side-effects from custom config:

mv ~/.config/tmux ~/.config/tmux-backup
mv ~/.config/yazi ~/.config/yazi-backup
mv ~/.bashrc ~/.bashrc-backup

Checklist

  • I tried the latest nightly build, and the issue is still reproducible
  • I updated the debug information (yazi --debug) input box to the nightly that I tried
  • I can reproduce it after disabling all custom configs/plugins (mv ~/.config/yazi ~/.config/yazi-backup)
@musjj musjj added the bug Something isn't working label Dec 15, 2024
@sxyazi
Copy link
Owner

sxyazi commented Dec 15, 2024

I can reproduce it, but I'm not sure how to fix it, or what Yazi can do, because it looks like an issue with tmux itself.

Specifically, Yazi needs to detect the terminal's image support when it starts up, so it sends a Kitty graphics protocol probe sequence to the actual user terminal (like kitty, not tmux, because tmux itself doesn't support the Kitty graphics protocol):

execute!(
LineWriter::new(stderr()),
SavePosition,
Print(Mux::csi("\x1b_Gi=31,s=1,v=1,a=q,t=d,f=24;AAAA\x1b\\")), // Detect KGP
Print(Mux::csi("\x1b[>q")), // Request terminal version
Print("\x1b[16t"), // Request cell size
Print("\x1b]11;?\x07"), // Request background color
Print(Mux::csi("\x1b[0c")), // Request device attributes
RestorePosition
)?;

So, it needs to have allow-passthrough all set to pass those sequences through to Kitty.

Somehow, tmux forwards the response from the actual user terminal (kitty) to all sessions, not just the active one (the one Yazi is in), this caused the issue.

The only related issue I found is tmux/tmux#3489, but it looks like it has already been fixed.

I tried changing the value of allow-passthrough from all to on, but found that it doesn't work at all. When testing with your steps, Yazi never receives a response from kitty. And here, all refers to whether it's forwarded to the "invisible" pane, not the "inactive" pane.

 allow-passthrough [on | off | all]
        Allow programs in the pane to bypass tmux using a terminal escape sequence (\ePtmux;...\e\\).  If set to on, passthrough sequences will be allowed only if the pane is visible.  If set to
        all, they will be allowed even if the pane is invisible.

I'm not familiar with tmux — I don't use it myself, I just installed it for testing purposes, so any help would be greatly appreciated!


Note that in your case, Yazi sends DECRQM because Yazi first detects your terminal is Kitty through the environment variable, so it skips the Kitty protocol detection process:

Print(Mux::csi("\x1bP$q q\x1b\\")), // Request cursor shape (DECRQM)

But the issue is the same — somehow tmux forwarded the response to all sessions, not just the one where Yazi is running.

You can test it with yazi --debug so that it won't be affected by the environment variable.

@sxyazi sxyazi added the help wanted Extra attention is needed label Dec 15, 2024
@sxyazi
Copy link
Owner

sxyazi commented Dec 16, 2024

I tried changing the value of allow-passthrough from all to on, but found that it doesn't work at all. When testing with your steps, Yazi never receives a response from kitty.

After further testing, I found that moving EnterAlternateScreen after the query makes it work, though I'm not sure why, but it works!

execute!(
	BufWriter::new(stderr()),
-	EnterAlternateScreen,
	Print(Mux::csi("\x1bP$q q\x1b\\")), // Request cursor shape (DECRQM)
	Print(Mux::csi("\x1b[?12$p")),      // Request cursor blink status (DECSET)
	Print("\x1b[?u"),                   // Request keyboard enhancement flags (CSI u)
	Print(Mux::csi("\x1b[0c")),         // Request device attributes
+	EnterAlternateScreen,
	EnableBracketedPaste,
	mouse::SetMouse(true),
)?;

I'll create a PR to fix it shortly.

@sxyazi
Copy link
Owner

sxyazi commented Dec 16, 2024

Please try #2052

@musjj
Copy link
Contributor Author

musjj commented Dec 16, 2024

Thank you for the quick response! It works! 🎉

@musjj
Copy link
Contributor Author

musjj commented Dec 16, 2024

Actually, there's one more bug related to this. The reproduction method is the same, except that in child_2, don't open a new tmux window and instead open yazi directly. When you have two clients viewing the same window, opening yazi seems to trigger the bug again.

@sxyazi
Copy link
Owner

sxyazi commented Dec 16, 2024

viewing the same window

Can you record a video to show me your steps? I'm a bit confused about the concepts of session, window, and pane in tmux, not quite sure what you mean by "same window" here

@musjj
Copy link
Contributor Author

musjj commented Dec 16, 2024

I meant viewing the same tmux window from two different sessions. Here's a video:

Screencast.From.2024-12-16.18-01-54.webm

The reproduction steps is exactly the same as above, but you skip this particular step:

Additionally, create a new tmux window by pressing Ctrl+b, then c.

@sxyazi
Copy link
Owner

sxyazi commented Dec 16, 2024

This is how allow-passthrough on works. If you just run (instead of yazi):

tmux set -p allow-passthrough on
echo -e -n "\x1bPtmux;\x1b\x1b[0c\x1b\\"

you'll get the same result.

In other words, unless tmux provides a more specific session-specific value, other than on and all, like allow-passthrough session, there's no way to actually fix it, because that's how allow-passthrough is designed:

     allow-passthrough [on | off | all]
             Allow programs in the pane to bypass tmux using a terminal escape sequence (\ePtmux;...\e\\).  If set to on, passthrough sequences will be allowed only if the
             pane is visible.  If set to all, they will be allowed even if the pane is invisible.

In your case, even though you're in different sessions, they are still both visible panes.

@musjj
Copy link
Contributor Author

musjj commented Dec 16, 2024

I see, but what I'm confused about is why is it preventing yazi from launching? Is there a way to work around this?

@sxyazi
Copy link
Owner

sxyazi commented Dec 16, 2024

Because Tmux forwards the terminal's response to Yazi twice (once for each "visible pane"), but Yazi assumes and only needs the first one.

So, it stops reading when it reaches the end of the first response, and the second one is treated as a keypress event, which might include the q key (i.e., to exit Yazi).

I don't think there's a solution for this right now. It could only be fixed if tmux adds a new allow-passthrough option that only sends the sequence to the "visible pane of the active session", not just the "visible pane".

@sxyazi
Copy link
Owner

sxyazi commented Dec 16, 2024

I created a new PR #2058, where I replaced DA1 with DSR since its response is a bit safer. Could you try it out?

Note that this is just a workaround - tmux will still forward the terminal response to Yazi twice, but now it's forwarding the DSR response ^[[0n, and this doesn't trigger anything to Yazi at the moment, it seems that crossterm silently ignored it instead of interpreting it as a key press.

@musjj
Copy link
Contributor Author

musjj commented Dec 16, 2024

Thank you for the PR! I think it now works some of the time. But it still fails occasionally, with the following printed to shell:

12;2$y62;4;9;22cnq

Not sure why.

@sxyazi
Copy link
Owner

sxyazi commented Dec 17, 2024

This might be caused by tmux reordering the responses from the terminal. I've encountered similar issues in the past (See #1557 and #1621), and this is also the biggest problem with the current workaround.

Specifically, Yazi now sends DA1 first and then sends DSR, while Yazi expects the terminal responses in the order of DA1 - DSR.

However, since Yazi reads and processes the terminal responses very quickly, it moves on to request DSR right after reading DA1. At this point, tmux may not have forwarded the second part (from another "visible pane") of DA1 to Yazi yet, meaning there's a slight delay in tmux forwarding the response to the two visible panes.

As far as I know, WezTerm solved this by introducing a delay to give tmux enough time to forward all terminal responses:

        if is_tmux || cfg!(windows) {
            self.write.flush()?;
            // I really wanted to avoid a delay here, but tmux and conpty will
            // both re-order the response to dev_attributes before sending the
            // response for the passthru of query_pixels if we don't delay.
            // The delay is potentially imperfect for things like a laggy ssh
            // connection. The consequence of the timing being wrong is that
            // we won't be able to reason about the pixel dimensions, which is
            // "OK", but that was kinda the whole point of probing this way
            // vs. termios.

            std::thread::sleep(std::time::Duration::from_millis(100));
        }

However, this is still quite a hacky solution because it's time-based and slows down the startup for every tmux user, especially since it's only meant to fix this rare issue. So, I'd like to avoid this approach if possible.

As I mentioned earlier, to fully solve this, it would require tmux support anyway, specifically adding a new value to allow-passthrough that forwards content only to the "active session's visible pane", rather than "all visible panes".

@sxyazi sxyazi mentioned this issue Dec 27, 2024
3 tasks
Copy link

I'm going to lock this issue because it has been closed for 30 days. ⏳
This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please file a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
2 participants