Skip to content

Add initial support for bracketed paste mode#8909

Closed
skyline75489 wants to merge 2 commits intomicrosoft:mainfrom
skyline75489:chesterliu/dev/bracket-paste-2021-edition
Closed

Add initial support for bracketed paste mode#8909
skyline75489 wants to merge 2 commits intomicrosoft:mainfrom
skyline75489:chesterliu/dev/bracket-paste-2021-edition

Conversation

@skyline75489
Copy link
Collaborator

Summary of the Pull Request

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

References

Supersedes #7508

PR Checklist

  • Closes bracketed paste in conhost #395
  • CLA signed. If not, go over here and sign the CLA
  • Tests added/passed
  • Documentation updated. If checked, please file a pull request on our docs repo and link it here: #xxx
  • Schema updated.
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

@ghost 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 Jan 27, 2021
@skyline75489
Copy link
Collaborator Author

Dustin mentioned his effort to bring "bracketed paste mode" into conhost (#7508 (comment)). I don't know enough about conhost or conpty so I limited my implementation to WT. This is an early implementation and will conflict with #8875 , so I marked this as a draft.

@skyline75489
Copy link
Collaborator Author

This is pretty straightforward. I'm wondering if this has any side effects that I don't know about. If the overall direction is correct, I'll polish the PR later the mark it as ready for review.

@oising
Copy link
Collaborator

oising commented Jan 27, 2021

I don't think this is as straightforward as one might think. Are you filtering out an explicit end sequence from the source sequence before pasting? This is a common bypass attempted in malicious scenarios (update: just checked - you're not)

Here are some eye opening dirty tricks to digest if you haven't seen them before: https://thejh.net/misc/website-terminal-copy-paste

@skyline75489
Copy link
Collaborator Author

@oising that’s #8875. I made two separate PRs to minimize the diff

Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

I don't see what's wrong with this. Thanks!

@zadjii-msft
Copy link
Member

@msftbot make sure @DHowett signs off on this

@ghost ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 27, 2021
@ghost
Copy link

ghost commented Jan 27, 2021

Hello @zadjii-msft!

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 @DHowett

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".

@oising
Copy link
Collaborator

oising commented Jan 27, 2021

@oising that’s #8875. I made two separate PRs to minimize the diff

Ah, apologies -- I missed that. Nice work :)

@ghost ghost removed the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 28, 2021
@skyline75489
Copy link
Collaborator Author

Closed in favor of #9034

@skyline75489 skyline75489 deleted the chesterliu/dev/bracket-paste-2021-edition branch February 9, 2021 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bracketed paste in conhost

4 participants