-
Notifications
You must be signed in to change notification settings - Fork 8.6k
Add support for OSC 52 clipboard copy in conhost #18949
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
base: main
Are you sure you want to change the base?
Conversation
// The clipboard can only be updated from the main GUI thread, so we | ||
// need to post a message to trigger the actual copy operation. But if | ||
// the pending clipboard content is already set, a message would have | ||
// already been posted, so there's no need to post another one. | ||
const auto clipboardMessageSent = _pendingClipboardText.has_value(); | ||
_pendingClipboardText = text; | ||
if (!clipboardMessageSent) | ||
{ | ||
PostMessageW(window->GetWindowHandle(), CM_UPDATE_CLIPBOARD, 0, 0); | ||
} |
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 if I've just misunderstood how you're supposed to use the windows clipboard, but I couldn't get it working from a background thread, and this technique worked for me. I don't know if maybe there's a better way to do this.
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.
Yep, reading doesn't require a HWND but writing does, to my knowledge.
void Clipboard::CopyText(const std::wstring& text) | ||
{ | ||
const auto clipboard = _openClipboard(ServiceLocator::LocateConsoleWindow()->GetWindowHandle()); | ||
if (!clipboard) | ||
{ | ||
LOG_LAST_ERROR(); | ||
return; | ||
} | ||
|
||
EmptyClipboard(); | ||
// As per: https://learn.microsoft.com/en-us/windows/win32/dataxchg/standard-clipboard-formats | ||
// CF_UNICODETEXT: [...] A null character signals the end of the data. | ||
// --> We add +1 to the length. This works because .c_str() is null-terminated. | ||
_copyToClipboard(CF_UNICODETEXT, text.c_str(), (text.size() + 1) * sizeof(wchar_t)); | ||
} |
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.
The content of this method is copied directly from StoreSelectionToClipboard
, but there's really not much to it, so I didn't think there was any point in trying to share the code somehow.
src/host/consoleInformation.cpp
Outdated
const auto clipboardText = _pendingClipboardText; | ||
_pendingClipboardText.reset(); | ||
return clipboardText; |
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 can avoid a copy here by doing this:
return std::exchange(_pendingClipboardText, {});
I've tested OSC 52 with Windows Terminal with up to 100MiB and it was relatively snappy. So I think avoiding a copy may be useful to reduce the peak memory usage.
...now that I mention this, I realize that immediately copying the clipboard contents into an HGLOBAL
would have another benefit... Well, that's a minor issue though.
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.
Thanks for the exchange suggestion. I was hoping there was way to avoid that copy. That's definitely made a difference.
Summary of the Pull Request
This adds support for copying to the clipboard in conhost using the OSC
52 escape sequence, extending the original implementation which was for
Windows Terminal only.
References and Relevant Issues
The Windows Terminal implementation was added in PR #5823.
Detailed Description of the Pull Request / Additional comments
Because the clipboard can't be accessed from a background thread, this
works by saving the content in a global variable, and then posting a
custom message to the main GUI thread, which takes care of the actual
copy operation.
Validation Steps Performed
I've manually confirmed that tmux copy mode is now able to copy to the
system clipboard.
PR Checklist