-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Fix system clipboard with mosh + tmux #9834
base: master
Are you sure you want to change the base?
Conversation
d17f96b
to
c25ab35
Compare
c25ab35
to
9f9f6af
Compare
Doesn't this code make helix coupled to tmux specifically and only fix tmux rather than the special issue of any wrapper app not passing clipboard through? |
Hi! This patch makes Helix as coupled to tmux as it has been in the past (helix already checks if it's currently inside tmux to make decisions about how to setup the clipboard). The only difference is that apart from checking if it is inside tmux, it also checks if some particular feature of tmux is enabled to use it.
I'm not sure what you mean here. Helix already has OSC52 support as fallback mechanism is no other was found suitable. From the question it sounds like you were looking to add an option to the config file to force OSC52? I thought about this one too, but I've seen reluctance in adding new options to Helix config and realized that the decision could be automated this way. |
@useche Yes that's what I meant about enabling when wanted, ans thank you for the explanations I didn't know about the existing checks targeting tmux 👍 |
helix-view/src/clipboard.rs
Outdated
} else if tmux_allow_passthrough() { | ||
// If we are in tmux and it allows passthrough, lets use OSC52 and escape tmux. | ||
Box::new(provider::FallbackProvider::new()) | ||
} else if env_var_is_set("TMUX") && binary_exists("tmux") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this a separate branch rather than nesting under the next one that's already checking for tmux?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know why I did that. I combined the the two branches.
if !env_var_is_set("TMUX") { | ||
return false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then this check would be redundant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need this check. The reason is that this function is also used in the OSC52 fallback, which won't necessarily be used with tmux. So, to escape OSC52 string through tmux, we still need to check if we are in tmux and whether the feature is on or not.
190202e
to
6a9b791
Compare
I added documentation for a better explanation of the feature and how to enable it. |
Some times tmux doesn't pass the clipboard information to the terminal correctly. This is the case of mosh + tmux. The current clipboard doesn't work properly in this case. However, when the tmux option `allow-passthrough` is on, we can escape tmux and pass the OSC52 string directly to the terminal. In this patch, if helix detects that tmux `allow-passthrough` is on, it will use the OSC52 clipboard provider and will escape tmux and pass the OSC52 directly to the terminal. Tested with mosh + tmux with the `allow-passthrough` option on.
6a9b791
to
81687f2
Compare
Some times tmux doesn't pass the clipboard information to the terminal correctly. One example of this is mosh + tmux. In this case the current clipboard doesn't work properly. However, when the tmux option
allow-passthrough
is on, we can escape tmux and pass the OSC52 string directly to the terminal.In this patch, if helix detects that tmux
allow-passthrough
is on, it will use the OSC52 clipboard provider and will escape tmux and pass the OSC52 directly to the terminal.Tested with mosh + tmux with the
allow-passthrough
option on.