Skip to content

Add copying to clipboard using OSC52#974

Merged
TimonPost merged 1 commit intocrossterm-rs:masterfrom
naseschwarz:feature/copy-text
Apr 5, 2025
Merged

Add copying to clipboard using OSC52#974
TimonPost merged 1 commit intocrossterm-rs:masterfrom
naseschwarz:feature/copy-text

Conversation

@naseschwarz
Copy link
Contributor

Many terminal emulators support copying text to clipboard using ANSI OSC Ps; PT ST with Ps = 5 2, see
https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h3-Operating-System-Commands

This enables copying even through SSH and terminal multiplexers.

I'm not too sure whether you want this feature or whether src/terminal.rs is the right place for it. I had just implemented it for gitui, which uses crossterm, and thought, it would be nice to have it upstream too.

It does not come for free, as it adds a new dependency to base64 which is not yet required by any of crossterm's dependencies.

@extrawurst
Copy link
Contributor

@joshka would love your input on this

Copy link
Collaborator

@joshka joshka left a comment

Choose a reason for hiding this comment

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

General support for the idea with a few questions / thoughts. terminal seems like the right module for this unless you were to add a clipboard module, but there's only one command likely to end up there, so that feels like probably a bit unnecessary.

@naseschwarz naseschwarz marked this pull request as draft March 14, 2025 12:39
@naseschwarz naseschwarz force-pushed the feature/copy-text branch 2 times, most recently from ab8c54f to 16bc099 Compare March 14, 2025 14:41
@naseschwarz naseschwarz marked this pull request as ready for review March 14, 2025 20:50
@naseschwarz naseschwarz force-pushed the feature/copy-text branch 2 times, most recently from 9131b92 to 626768a Compare March 15, 2025 13:01
Copy link
Collaborator

@joshka joshka left a comment

Choose a reason for hiding this comment

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

Lots of extra comments after reading the docs more and giving this a deeper review. Apologies that my first review was fairly surface level in comparison.

At a high level, I support the addition of this. To summarize my thoughts across the various comments raised here, I have some concern around what level of abstraction Crossterm should provide for this. I don't have any immediately concrete suggestions though, so looking for some back and forth of ideas before settling on the right approach. Feel free to shoot down any of the perspectives I've thrown here with your viewpoints - you're likely just as much or more of an expert on these than I am.


Regarding the paste side of this (passing ? as the data). It seems like it could be worth adding this usage of OSC 52 at the same time as adding this feature. WDYT?

      If the second parameter is a ? , xterm replies to the host
      with the selection data encoded using the same protocol.  It
      uses the first selection found by asking successively for each
      item from the list of selection parameters.

Also there's a clear clipboard function - worth worrying about?

      If the second parameter is neither a base64 string nor ? ,
      then the selection is cleared.

@naseschwarz
Copy link
Contributor Author

naseschwarz commented Mar 16, 2025

Hi @joshka,

Lots of extra comments after reading the docs more and giving this a deeper review. Apologies that my first review was fairly surface level in comparison.

I appreciate this very much. Thank you for taking the time. (Also, I know the feeling of having to double down on a review very well. No worries. :) )

At a high level, I support the addition of this. To summarize my thoughts across the various comments raised here, I have some concern around what level of abstraction Crossterm should provide for this.

I've had these, too. This whole change feels odd as clipboard support breaks a barrier in so far as that it is very different from the usual presentation and input mechanisms supplied by terminals and in this case crossterm. It is only really relevant to applications as a means for copying from remote hosts, where the only line is a TTY. Thus, while odd, signalling the copy operation in-band is the go-to solution.

Regarding the paste side of this (passing ? as the data). It seems like it could be worth adding this usage of OSC 52 at the same time as adding this feature. WDYT?

I don't have much to add to my previously stated opinion. There is essentially no need for paste (as pasting can be done through the terminal emulator and normal input) and normalizing its use is a huge security concern. You can't even use it to cross-check OSC52, as some terminal emulators disable OSC52 paste while having OSC52 copy enabled. (And after this has been merged I'll probably try to get alacritty's behavior into more emulators. I was quite surprised by how many emulators actually allow paste.)

(I agree that there is a small impedance mismatch. If I run my favorite editor and if that were to use crossterm on a remote host, I can't just type "+p in order to paste from my host's clipboard. I'd argue that inconvenience is small, given the price.)

My suggestion to consolidate this would be to actually move the clipboard code into a separate clipboard.rs and make rename ClipboardDestination to ClipboardSelection such that the clipboard features stand out a bit more and it'll be easy to add PasteFromClipboard later, if one wants to do so. Would that be alright?

Also there's a clear clipboard function - worth worrying about?

      If the second parameter is neither a base64 string nor ? ,
      then the selection is cleared.

I guess you're asking because a CopyToClipboard("", _) will base64-encode to an empty second parameter, i.e. clear the clipboard instead of copying an empty string? I would have guessed that from a user's standpoint an empty clipboard and a clipboard containing an empty string would be equivalent.

However, it is true that for clipboard protocols, there generally is a difference between being cleared and containing an empty string.

That being said, I'm not sure how to address this given OSC52's specification. A wider API would only be relevant if there is a plan to support other clipboard protocols (like wayland, X.org, winapi, whatever macos is doing and whatever we want to implement ourselves) in the future. My view as a new contributor is that this is not in crossterm's scope.

If you disagree, we should discuss this, though. Frankly, I do see a potential use case for copying a text/uri-list that contains sftp-URIs for pasting into your favorite file browser for file download through SSH. The only limit here is that associating a MIME type is not supported by OSC52. That does not really keep us from widening the interface to something resembling CopyToClipboard(T, MimeType, ClipboardSelection) in order to support that in the future.

I'll respond to the other items in line or implement them.

Cheers and thanks again,

Johannes

@joshka
Copy link
Collaborator

joshka commented Mar 16, 2025

My suggestion to consolidate this would be to actually move the clipboard code into a separate clipboard.rs and make rename ClipboardDestination to ClipboardSelection such that the clipboard features stand out a bit more and it'll be easy to add PasteFromClipboard later, if one wants to do so. Would that be alright?

Sounds good - this also simplifies the feature flag checks to just include the entire module.

That being said, I'm not sure how to address this given OSC52's specification. A wider API would only be relevant if there is a plan to support other clipboard protocols (like wayland, X.org, winapi, whatever macos is doing and whatever we want to implement ourselves) in the future. My view as a new contributor is that this is not in crossterm's scope.

Your logic there seems sound to me.

@naseschwarz naseschwarz force-pushed the feature/copy-text branch 4 times, most recently from 40ed098 to 90f1898 Compare March 19, 2025 21:19
@naseschwarz
Copy link
Contributor Author

So, here's a new API that is a bit wider but offers and prioritizes simple constructors. I guess there will be criticism, I'm still open to any changes!

Commands are now primarily constructed using CopyToClipboard::into_clipboard_from(T) or CopyToClipboard::into_primary_from(T). Thus, the interface is straightforward. Users can also build custom CopyToClipboards and setting destination to any sequence of clipboards, even the empty sequence.

Now this covers the whole OSC52 copy feature set, I guess, without too many visible complications.

@joshka: Might I ask you to have another look at this if you can spare the time? Thank you, also for your excellent feedback in the previous two rounds!

@naseschwarz naseschwarz force-pushed the feature/copy-text branch 3 times, most recently from 044c616 to ab0fd66 Compare March 19, 2025 22:07
@kovidgoyal
Copy link

Just FYI, pasting is best supported with permissions, just as it is on the web. And copy/paste for different mimetypes is implemented using the OSC 5522 protocol, documented at:
https://sw.kovidgoyal.net/kitty/clipboard/

Copy link
Collaborator

@joshka joshka left a comment

Choose a reason for hiding this comment

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

nits and bikeshedding comments mainly. I'm fine with the approach here in general. Happy to take it as is or let you fix up which parts you think are relevant here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

As a non-blocking idea, what about making this more interactive rather than command driven to make it clear that the behavior is part of a standard part of an application flow. E.g.

  1. print instructions to press c/p/esc
  2. loop, accept keys
  3. print a message that says what happened (copied "foo" to the primary / clipboard0

Absolutely this is mostly gold plating the demo. But I think it makes it a little bit more obvious. Also, what about naming this clipboard for making it easier to discover (when users are browsing the repo the name is often the first thing they'll see.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe this would be a good addition when paste get's implemented to show a fully interactive clipboard roundtrip. I've renamed it to copy-to-clipboard.rs for clarity, though.

I find this tool actually quite helpful as is. It allows me to quickly check crossterm's implementation against different terminal emulators without needing further interaction.

src/clipboard.rs Outdated
/// use crossterm::clipboard::CopyToClipboard;
/// execute!(std::io::stdout(), CopyToClipboard::into_clipboard_from("foo"));
/// ```
pub fn into_clipboard_from(content: T) -> CopyToClipboard<T> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

bikeshed: I think maybe to_clipboard / to_primary might be obvious enough names here. into_ gives greater connotations of conversion of the type into another type, which is not happening here. with_ would be the obvious builder style name, but that feels wrong here to me too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is taking off some mental load, thanks. Naming has been a major pain in this patch and I'm still dissatisfied. Everything is quite clunky. I'd love to name CopyToClipboard just Copy, but has an obvious readability issue in Rust due to std::marker::Copy. I've converted it, without having any better suggestion, CopyToClipboard::to_clipboard... also is an odd one.

@naseschwarz
Copy link
Contributor Author

Just FYI, pasting is best supported with permissions, just as it is on the web. And copy/paste for different mimetypes is implemented using the OSC 5522 protocol, documented at: https://sw.kovidgoyal.net/kitty/clipboard/

Hi @kovidgoyal. Thank you for your input on paste, I appreciate it! It seems that, at the moment, this is only implemented in kitty, right?

@kovidgoyal
Copy link

kovidgoyal commented Mar 20, 2025 via email

@naseschwarz naseschwarz force-pushed the feature/copy-text branch 2 times, most recently from 01e65ca to 5751c03 Compare March 20, 2025 09:03
@naseschwarz
Copy link
Contributor Author

Huge thanks again, @joshka. I've implemented all suggestions except for the more involved clipboard.rs example.

I also noticed that the CI isn't running clippy on all features and replaced my ToString implementation with ClipboardSelection::to_osc52_pc().

@archseer
Copy link
Contributor

archseer commented Mar 24, 2025

Worth noting that OSC52 support was previously rejected by the author due to new dependencies (the exact same ones this PR is using)

Copy link
Member

@TimonPost TimonPost left a comment

Choose a reason for hiding this comment

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

Yes indeed, previously dismissed the addition in the other PR, and similar concerns are raised here. In this PR we went for OSC52 support only without windows support, and compared the other PR this PR has done more work in documenting/testing the edge cases. On a second thought, with clear documentation and the feature behind a feature flag, I think its fine to have those features in the library given they are terminal protocols which crossterm aims to help simplifying access to.

LGTM, just one comment regarding the feature flag.

Cargo.toml Outdated
derive-more = ["dep:derive_more"]

## Enables interacting with a host clipboard via [`clipboard`](clipboard/index.html)
clipboard = ["dep:base64"]
Copy link
Member

Choose a reason for hiding this comment

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

Since its only osc52 perhaps we can name the feature flag to reflect that? Otherwise folks may think its a general clipboard feature which can have many kinds of implementations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your time and the approval, @TimonPost. I'll gladly yield to renaming the flag to osc52, see my latest change.

Apart from this, the latest change to the MR is rebasing onto master and adding an entry to CHANGELOG.md. I hope that's fine.

Many terminal emulators support copying text to clipboard using
ANSI OSC Ps; PT ST with Ps = 5 2, see
https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h3-Operating-System-Commands

This enables copying even through SSH and terminal multiplexers.
@naseschwarz naseschwarz force-pushed the feature/copy-text branch 2 times, most recently from 1fe99be to feaeb56 Compare April 5, 2025 13:34
@TimonPost TimonPost merged commit 7da7e31 into crossterm-rs:master Apr 5, 2025
6 checks passed
@TimonPost
Copy link
Member

Thanks for your patience and work!

takumi-earth pushed a commit to earthlings-dev/crossterm that referenced this pull request Jan 27, 2026
Many terminal emulators support copying text to clipboard using
ANSI OSC Ps; PT ST with Ps = 5 2, see
https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h3-Operating-System-Commands

This enables copying even through SSH and terminal multiplexers.

Co-authored-by: Naseschwarz <naseschwarz@0x53a.de>
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.

6 participants