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

Copy text using OSC52 #2548

Merged
merged 3 commits into from
Mar 15, 2025
Merged

Conversation

naseschwarz
Copy link
Contributor

@naseschwarz naseschwarz commented Mar 5, 2025

This Pull Request closes #2366.

It changes the following:

  • If copying using wl-clip or xclip and xsel fails, copy using OSC52 (Search for "Ps = 5 2").

This does not change the default behavior. On both wayland and X, wl-copy and xclip/xsel will be preferred. Only if one path fails, an OSC52 escape sequence will be issued.

I have tested this by running tmux, which has the option to turn support for this off and on (using :set -g set-clipboard off or :set -g set-clipboard on) and SSH. When tmux's clipboard support was off, no text was copied. Once I enabled it, copying worked as expected. From this I conclude that the whole setup works as expected.

N.B.: There is no easy path for gitui itself to check whether copying worked. Also, this essentially disables error reporting on wl-clip, xclip and xsel, as it acts as a fallback for these options.

I followed the checklist:

  • I added unittests
  • I ran make check without errors
  • I tested the overall application
  • I added an appropriate item to the changelog

@naseschwarz
Copy link
Contributor Author

I'm also trying to get this into crossterm. If that were merged, we could consider using that instead.

@cruessler
Copy link
Collaborator

I’m going to add myself as a reviewer!

@cruessler cruessler self-requested a review March 12, 2025 08:36
Copy link
Collaborator

@cruessler cruessler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this PR on my Linux machine, using Alacritty, after having commented out the wl-copy block, forcing the usage of escape codes.

From my side, everything looks good!

@extrawurst
Copy link
Collaborator

extrawurst commented Mar 13, 2025

Awesome once the CI is green (minus the lints) I will merge. Thank you both!

Naseschwarz added 2 commits March 13, 2025 17:24
Copying logic seems too nested to comprehend with the introcution of two
paths towards OSC52 otherwise.
@naseschwarz naseschwarz force-pushed the feature/osc52-yank branch 2 times, most recently from 0db0630 to ac7e8da Compare March 13, 2025 16:39
@naseschwarz
Copy link
Contributor Author

naseschwarz commented Mar 13, 2025

CI failed as my split up copy function was only called on Linux and thus were marked as dead code on Mac OS and Windows. Should be fixed now.

make check fails due to #2554 now, though.

@naseschwarz naseschwarz force-pushed the feature/osc52-yank branch 2 times, most recently from 50315d0 to 85d7018 Compare March 14, 2025 09:58
@naseschwarz
Copy link
Contributor Author

I'm really sorry for bothering you because of build failures in CI, @extrawurst. My bad. I have now set up a CI run in my own namespace and, apart from cargo deny because of paste, it succeeds.

@extrawurst extrawurst merged commit 3c1e35e into gitui-org:master Mar 15, 2025
21 checks passed
@extrawurst
Copy link
Collaborator

@naseschwarz Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add OSC52 Support
3 participants