-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Allow copying with ANSI escape code control sequences #17059
Open
FuegoFro
wants to merge
1
commit into
microsoft:main
Choose a base branch
from
FuegoFro:copy_with_control_sequences
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 believe we can add a new
CopyFormat
,CopyFormat::VT
, that would avoid the need for plumbingWithControlSequences
down toControlCore
+ introducing a new setting. It will just be carried forward with other copy formats.If the user has "vt" in the
copyFormatting
list:we copy the text in the new VT format (including in any other formats set by the user).
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.
You can add the new format
VT = 0x04
here:terminal/src/cascadia/TerminalControl/EventArgs.idl
Lines 7 to 12 in 5f3a857
In
ControlCore::CopySelectionToClipboard
, you can check if we need to copy in VT format like so: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.
Ah, I had the same thought when implementing this but wasn't sure if that's actually what we want here. In particular, currently the formats represent a wholly different clipboard format. That being said
singleLine
does affect all the formats so it's a bit odd to pass this boolean through to just affect the "plain text" (CF_UNICODETEXT
) format, so I'm certainly open to having a "pseudo format" here that modifies the plain text rather than opting into another one! Not totally sure how to make this call haha.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.
Surely this isn't plain text, so it should actually be a separate clipboard format? Personally I would expect it to paste as ANSI only if the application can handle that, and the plain text format should still be available for applications that just accept text. Typically it should be the app's decision as to what format they want.
That said, I don't know if there's a generally agreed upon clipboard registration for ANSI text that will work automatically, but worst case I think apps should be able to offer users a choice of the available formats, and let them decide want they want at the time of the paste.
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.
As far as I'm aware there's no such format. It likely wouldn't work with a custom format either. My understanding is that the goal is that you can paste the ANSI text into markdown documents. For instance Discord supports this:
This will only work if the ANSI text is in the
CF_UNICODETEXT
format.We've briefly talked about this as a team today and we felt like this new format should not be exposed in the settings UI here:
Precisely because of the interop issue that James mentioned. Instead it could be triggered with an action via the command palette. That is, we could for instance introduce a new action type for 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.
In an ideal world, the apps that wanted functionality like this would have got together and decided on a custom clipboard format name, say "ANSIX3.64", which they'd all agree to support. Then when someone like Discord saw that on the clipboard, they could have automatically pasted it surrounded by their ansi markdown wrapper. Having to do that manually is sad. But I guess we have to live with what we've got.
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.
Hahaha indeed, I'm the person who hacked in the
ansi
"syntax highlighting" in Discord 😅It's interesting (and, to be clear, I'm not sure what the "right" answer is here or if there is one), but in the context of using this in markdown (aka in Discord or in some other markdown as mentioned in #15703), what you're authoring is plaintext. Like, if I copied code from a website and it populated both the unicode and the HTML formats in the clipboard, when I pasted it into my markdown editor I'd personally expect it to paste the plaintext and not try to... paste formatted/wysiwyg contents, or to auto-insert a code block for me. And (again, in my opinion/use cases) similarly here. I would expect this to act less like a seamless parallel copy and more like a different behavior.
Especially given the above that makes sense! Would that mean something like what's done here, or not piggy backing on the
copy
action at all and introducing a new one with it's own arguments (if any)?I really appreciate the discussion here, as well as you taking the time to talk this over with your team! 🙏
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.
This is what the "Paste Special" menu option is for. Have a look at WordPad if you want to see an example of how it's supposed to work. Just type in some text with some formatting and copy it to the clipboard, then press Ctrl+Alt+V. You should see a popup dialog offering you a bunch of choices for how you want the content to paste. If you paste as "Rich Text" it'll keep the formatting. If you paste as "Unformatted Text", you just get the plain text.
If you're responsible for the Discord clipboard code, you should be able to implement the same sort of thing. So by default you can paste as plain text, but still give users the option to paste as ANSI via a "Paste Special" context menu, or shortcut key.
Edit: I should add that I don't feel strongly about this if everyone else disagrees with me, or it's too much effort. It's just that this is the way I would have expected it to work in a perfect world.