Add support for paste filtering and bracketed paste mode by skyline75489 · Pull Request #9034 · microsoft/terminal

skyline75489 · 2021-02-04T06:59:33Z

This adds "paste filtering" & "bracketed paste mode" to the Windows
Terminal.

I've moved the paste handling code in TerminalControl to
Microsoft::Console::Util to be able to easily test it, and the paste
transformer from TerminalControl to TerminalCore.

Supersedes #7508
References #395 (overall bracketed paste support request)

Tests added. Manually tested.

src/types/ut_types/UtilsTests.cpp

src/types/utils.cpp

src/types/inc/utils.hpp

src/types/utils.cpp

skyline75489 · 2021-02-04T09:40:38Z

Misspellings found, please review:

ALot

This is why we need better AI 😅

src/types/inc/utils.hpp

.github/actions/spelling/expect/expect.txt

src/types/inc/utils.hpp

src/cascadia/TerminalControl/TermControl.cpp

src/types/utils.cpp

src/types/inc/utils.hpp

DHowett · 2021-02-05T01:55:15Z

I love this. Thank you 😀

skyline75489 · 2021-02-05T07:27:42Z

src/cascadia/TerminalControl/TermControl.cpp

-
-        _terminal->ClearSelection();
-        _terminal->TrySnapOnInput();
+        _terminal->WritePastedText(wstr);


This is very satisfying 🎉

I decided to keep the other terminal operations in this method to make WritePastedText more lightweight and decoupling from non-paste-related things. WDYT?

Not sure! I was wondering if the other two operations are something the Core should handle too.

@zadjii-msft if we want Core to be reusable, should it be given more responsibilities for figuring out when to clear the selection and snap on input? I feel like it should -- so maybe we should move the two lines under WritePastedText into WritePastedText. What do you think?

Yea I agree, that should probably move down below too. That makes sense. Doesn't make sense to block this PR over it though, IMO. We could probably just get away with

Suggested change

_terminal->WritePastedText(wstr);

// TODO:projects/5 The calls to ClearSelection and TrySnapOnInput below could probably get moved inside Terminal::WritePastedText.

_terminal->WritePastedText(wstr);

and have me come back to it 😛

oh the bot automerged it. Whoops. I'm sure this whole area's about to be torn up anyways

src/types/utils.cpp

DHowett · 2021-02-08T08:00:44Z

OK this is ready, I think. There's more to do:

Make ConPTY passthrough bracketed paste related sequences (See #395 (comment)).

DECSET 2004 request: If conhost's dispatcher returns false for EnableXtermBracketedPasteMode, it will pass through the request automatically. #8840 did this correctly.

200~ and 201~ in the input stream: In ENABLE_VIRTUAL_TERMINAL_INPUT mode, the bracketed paste sequences will automatically be passed through from the terminal to the client (we skip a lot of ConPTY input processing when VT input is on)

Make a setting for filtering control codes

We should only do this if more than one person asks for it. Jantari is the first; will there be a second?

Maybe also a setting entry for bracketed paste? (Who wouldn't want the feature though)

Nope. Not unless somebody complains.

Backport brackted paste mode to conhost.exe

I'd like to handle this myself as part of the input stack change. Is that okay? 😄

All of the above are not really my comfort zone. I don't know conpty or settings model that well. I guess we can track them in the bracketed paste main issue for future work.

Not a problem! I feel like you don't have too much work to do, given the above notes.

Thanks again for doing this!

DHowett

This looks excellent to me. Thank you.

DHowett · 2021-02-08T08:02:45Z

src/cascadia/TerminalControl/TermControl.cpp

-
-        _terminal->ClearSelection();
-        _terminal->TrySnapOnInput();
+        _terminal->WritePastedText(wstr);


Not sure! I was wondering if the other two operations are something the Core should handle too.

@zadjii-msft if we want Core to be reusable, should it be given more responsibilities for figuring out when to clear the selection and snap on input? I feel like it should -- so maybe we should move the two lines under WritePastedText into WritePastedText. What do you think?

DHowett · 2021-02-08T08:03:20Z

src/cascadia/TerminalCore/Terminal.cpp

+        filtered.append(L"\x1b[201~");
+    }
+
+    if (_pfnWriteInput)


nit: we don't need to do any of this work if there is no _pfnWriteInput ! 😁

DHowett · 2021-02-08T08:06:23Z

src/cascadia/TerminalCore/Terminal.cpp

+                  ::Microsoft::Console::Utils::FilterOption::ControlCodes;
+
+    std::wstring filtered = ::Microsoft::Console::Utils::FilterStringForPaste(stringView, option);
+    if (IsXtermBracketedPasteModeEnabled())


here's an idea. Maybe to handle Jantari's case (since they want to paste literal control sequences), we can strip control codes only when bracketed paste mode is enabled.

@j4james, do you think that's a reasonable compromise? It solves the case where a clipboard payload can break out of bracketed paste and doesn't make unguarded paste any worse.

At the same time, I still can't see the value in pasting raw control codes here today in Common Era 2021, so...

The only time I ever did it was when I was actively trying to blow up bracketed paste in xterm/rxvt.

Otherwise, that's what ^V is for.

Personally, my vote is let's strip them always.

I'd like to just strip them always, too. Also I don't like the idea of coupling "control codes filtering" and "bracketed paste mode". This is what @j4james told me specifically not to do in #7508 (comment)

For the record, I'm in agreement with you guys. I think it's preferable that they're always filtered.

DHowett · 2021-02-08T08:12:18Z

@msftbot make sure @zadjii-msft signs off

ghost · 2021-02-08T08:12:24Z

Hello @DHowett!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

I'll only merge this pull request if it's approved by @zadjii-msft

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

zadjii-msft

This all seems good to me. Not sure how many times we've reviewed it now haha. I'm not gonna block over a nit comment

zadjii-msft · 2021-02-08T13:09:16Z

src/cascadia/TerminalControl/TermControl.cpp

-
-        _terminal->ClearSelection();
-        _terminal->TrySnapOnInput();
+        _terminal->WritePastedText(wstr);


Yea I agree, that should probably move down below too. That makes sense. Doesn't make sense to block this PR over it though, IMO. We could probably just get away with

Suggested change

_terminal->WritePastedText(wstr);

// TODO:projects/5 The calls to ClearSelection and TrySnapOnInput below could probably get moved inside Terminal::WritePastedText.

_terminal->WritePastedText(wstr);

and have me come back to it 😛

skyline75489 · 2021-02-08T13:16:58Z

@DHowett Reading your comments regarding my list of future work just make me realize I really do not know ConPTY & conhost.exe related infrastructure that well 😅 . But I can sleep at night knowing it's in good hands of you & @zadjii-msft .

ghost · 2021-03-01T18:02:18Z

🎉Windows Terminal Preview v1.7.572.0 has been released which incorporates this pull request.:tada:

Handy links:

mnpenner · 2021-03-02T04:18:20Z

I was going to ask how we can disable the warning now that we have bracketed paste mode; good thing I searched first 😆 Answer is here for anyone else wondering. Documented here.

skyline75489 · 2021-04-27T08:43:23Z

@mnpenner If you want the terminal to be more "smart" about this, check out #9586 which is available now in 1.8 Preview.

This adds support for XTerm's "bracketed paste" mode in ConHost. When enabled, any pasted text is bracketed with a pair of escape sequences, which lets the receiving application know that the content was pasted rather than typed. ## References and Relevant Issues Bracketed paste mode was added to Windows Terminal in PR #9034. Adding it to ConHost ticks one more item off the list in #13408. ## Detailed Description of the Pull Request / Additional comments This only applies when VT input mode is enabled, since that is the way Windows Terminal currently works. When it comes to filtering, though, the only change I've made is to filter out the escape character, and only when bracketed mode is enabled. That's necessary to prevent any attempts to bypass the bracketing, but I didn't want to mess with the expected behavior for legacy apps if bracketed mode is disabled. ## Validation Steps Performed Manually tested in bash with `bind 'set enable-bracketed-paste on'` and confirmed that pasted content is now buffered, instead of being executed immediately. Also tested in VIM, and confirmed that you can now paste preformatted code without the autoindent breaking the formatting. Closes #395

Add support for paste filtering and bracketed paste mode"

0eec78e

ghost added Area-VT Virtual Terminal sequence support Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Conhost For issues in the Console codebase labels Feb 4, 2021

This was referenced Feb 4, 2021

Add initial support for bracketed paste mode #8909

Closed

Sanitize paste content by filtering out control chars #8875

Closed

skyline75489 commented Feb 4, 2021

View reviewed changes

src/types/ut_types/UtilsTests.cpp Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

skyline75489 commented Feb 4, 2021

View reviewed changes

src/types/utils.cpp Show resolved Hide resolved

skyline75489 commented Feb 4, 2021

View reviewed changes

src/types/utils.cpp Outdated Show resolved Hide resolved

skyline75489 added 2 commits February 4, 2021 15:08

Spelling

d168f76

this is lol

5b94939

This comment has been minimized.

Sign in to view

skyline75489 changed the title ~~Add support for paste filtering and bracketed paste mode"~~ Add support for paste filtering and bracketed paste mode Feb 4, 2021

This comment has been minimized.

Sign in to view

you happy now?

9e2d997

skyline75489 commented Feb 4, 2021

View reviewed changes

src/types/inc/utils.hpp Outdated Show resolved Hide resolved

skyline75489 commented Feb 4, 2021

View reviewed changes

src/types/utils.cpp Outdated Show resolved Hide resolved

skyline75489 commented Feb 4, 2021

View reviewed changes

src/types/inc/utils.hpp Outdated Show resolved Hide resolved

skyline75489 commented Feb 4, 2021

View reviewed changes

.github/actions/spelling/expect/expect.txt Outdated Show resolved Hide resolved

zadjii-msft self-assigned this Feb 4, 2021

DHowett requested changes Feb 5, 2021

View reviewed changes

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Feb 5, 2021

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Feb 5, 2021

Feedback

df187f6

skyline75489 commented Feb 5, 2021

View reviewed changes

src/types/utils.cpp Outdated Show resolved Hide resolved

j4james reviewed Feb 5, 2021

View reviewed changes

src/types/utils.cpp Outdated Show resolved Hide resolved

DHowett approved these changes Feb 8, 2021

View reviewed changes

ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Feb 8, 2021

zadjii-msft approved these changes Feb 8, 2021

View reviewed changes

ghost merged commit 2c603ef into microsoft:main Feb 8, 2021

zadjii-msft mentioned this pull request Feb 8, 2021

Introduce read-only panes #8867

Merged

6 tasks

skyline75489 deleted the chesterliu/dev/paste-converter-2021-edition branch February 9, 2021 06:09

This was referenced Feb 14, 2021

bracketed paste in conhost #395

Closed

Support "Paste Bracketing" PowerShell/PSReadLine#1471

Open

Add initial support for paste filtering and bracketed paste mode #7508

Closed

jnikolich mentioned this pull request Mar 3, 2021

Bracketed Paste appears either non-functional, or disabled #9364

Closed

alvaro-gh mentioned this pull request Mar 4, 2021

When Copying, Line Wrapped Text Is Inconsistently Broken Into Multiple Lines #6901

Closed

bhgomes mentioned this pull request Mar 11, 2021

Bracketed Paste Events crossterm-rs/crossterm#545

Closed

skyline75489 mentioned this pull request Mar 17, 2021

pasting with 1.7.572.0 preview and fish causes strange behavior #9517

Closed

zadjii-msft mentioned this pull request Apr 23, 2021

How do I paste text from the clipboard into the console editors without breaking the formatting? #9922

Closed

DHowett mentioned this pull request May 20, 2021

Unable to paste ASCII escape character (ASCII #001B) #10141

Closed

daxian-dbw mentioned this pull request Oct 7, 2021

[API Proposal]: Allow Console.ReadKey to parse VT sequences when ENABLE_VIRTUAL_TERMINAL_INPUT is set on Windows dotnet/runtime#60107

Open

gnodet mentioned this pull request Oct 18, 2021

Paste functionality different on windows/Linux as paste to buffer does not fire paste widget jline/jline3#685

Closed

zadjii-msft mentioned this pull request Feb 18, 2022

Copy-Paste in some editors #12411

Closed

ShrykeWindgrace mentioned this pull request Feb 20, 2023

Evaluate feature parity between Windows Terminal sequences and existing backends jtdaugherty/vty#251

Closed

j4james mentioned this pull request Apr 10, 2023

Add support for bracketed paste mode in ConHost #15155

Merged

nathanpovo mentioned this pull request May 10, 2024

After Paste, Undo removes one character at a time #7647

Closed

RokeJulianLockhart mentioned this pull request Jul 11, 2024

Pasted UTF-8 characters into PowerShell shell are interpreted incorrectly. PowerShell/PowerShell#24037

Open

5 tasks

This pull request was closed.

	_terminal->WritePastedText(wstr);
	// TODO:projects/5 The calls to ClearSelection and TrySnapOnInput below could probably get moved inside Terminal::WritePastedText.
	_terminal->WritePastedText(wstr);

Conversation

skyline75489 commented Feb 4, 2021 • edited by DHowett Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

skyline75489 commented Feb 4, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DHowett commented Feb 5, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DHowett commented Feb 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DHowett left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DHowett commented Feb 8, 2021

Uh oh!

ghost commented Feb 8, 2021

Uh oh!

zadjii-msft left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skyline75489 commented Feb 8, 2021

Uh oh!

ghost commented Mar 1, 2021

Uh oh!

mnpenner commented Mar 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skyline75489 commented Apr 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

skyline75489 commented Feb 4, 2021 •

edited by DHowett

Loading

DHowett commented Feb 8, 2021 •

edited

Loading

mnpenner commented Mar 2, 2021 •

edited

Loading