Skip to content

Fix multi line paste detection and filtering#8634

Merged
8 commits merged intomainfrom
dev/pabhoj/paste_fix
Jan 8, 2021
Merged

Fix multi line paste detection and filtering#8634
8 commits merged intomainfrom
dev/pabhoj/paste_fix

Conversation

@PankajBhojwani
Copy link
Contributor

@PankajBhojwani PankajBhojwani commented Dec 21, 2020 •
edited by DHowett
Loading

  • Detect \r when warning about multi line paste
  • Translate \n to \r on paste

PR Checklist

Validation Steps Performed

Manual testing

Don-Vito
Don-Vito reviewed Dec 22, 2020
View reviewed changes
@ghost ghost added Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Jan 7, 2021
DHowett
DHowett reviewed Jan 7, 2021
View reviewed changes
@PankajBhojwani PankajBhojwani changed the title [DRAFT] Fixes for multi line paste detection and filtering Fixes for multi line paste detection and filtering Jan 7, 2021
@PankajBhojwani PankajBhojwani marked this pull request as ready for review January 7, 2021 20:44
DHowett
DHowett reviewed Jan 7, 2021
View reviewed changes
DHowett
DHowett approved these changes Jan 7, 2021
View reviewed changes
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

one shot, one target eliminated. excellent work

@ghost ghost added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Impact-Correctness It be wrong. labels Jan 7, 2021
@DHowett DHowett changed the title Fixes for multi line paste detection and filtering Fix multi line paste detection and filtering Jan 7, 2021
@DHowett DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 7, 2021
@ghost
Copy link

ghost commented Jan 7, 2021

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

DHowett
DHowett requested changes Jan 7, 2021
View reviewed changes
@ghost ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jan 7, 2021
@DHowett DHowett added the Needs-Second It's a PR that needs another sign-off label Jan 8, 2021
@ghost ghost requested review from miniksa and zadjii-msft January 8, 2021 02:26
@ghost ghost requested review from carlos-zamora and leonMSFT January 8, 2021 02:26
@DHowett DHowett mentioned this pull request Jan 8, 2021
Closed
6 tasks
@zadjii-msft zadjii-msft mentioned this pull request Jan 8, 2021
Open
34 tasks
zadjii-msft
zadjii-msft approved these changes Jan 8, 2021
View reviewed changes
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.

Okay that seems fine. I'd feel better with a test for this, since it's a little hard to follow, but there's not a good way of doing TermControl unittests at the moment, so I just stuck this on #7001 for now.

@ghost ghost merged commit 49d0085 into main Jan 8, 2021
@ghost ghost deleted the dev/pabhoj/paste_fix branch January 8, 2021 10:44
DHowett pushed a commit that referenced this pull request Jan 25, 2021
@PankajBhojwani @DHowett
Fix multi line paste detection and filtering (#8634)
a205f45 
- Detect `\r` when warning about multi line paste
- Translate `\n` to `\r` on paste

## PR Checklist
* [x] Closes #8601
* [x] Closes #5821

## Validation Steps Performed
Manual testing

(cherry picked from commit 49d0085)
@ghost
Copy link

ghost commented Jan 28, 2021

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

Handy links:

This was referenced Jan 28, 2021
Closed
Closed
@ghost
Copy link

ghost commented Jan 28, 2021

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

Handy links:

mpela81 pushed a commit to mpela81/terminal that referenced this pull request Jan 28, 2021
@PankajBhojwani @mpela81
Fix multi line paste detection and filtering (microsoft#8634)
4fd557c 
- Detect `\r` when warning about multi line paste
- Translate `\n` to `\r` on paste

## PR Checklist
* [x] Closes microsoft#8601
* [x] Closes microsoft#5821

## Validation Steps Performed
Manual testing
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DHowett DHowett DHowett approved these changes

@zadjii-msft zadjii-msft zadjii-msft approved these changes

@miniksa miniksa Awaiting requested review from miniksa

@carlos-zamora carlos-zamora Awaiting requested review from carlos-zamora

@leonMSFT leonMSFT Awaiting requested review from leonMSFT

+1 more reviewer

@Don-Vito Don-Vito Don-Vito left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Area-UserInterface Issues pertaining to the user interface of the Console or Terminal AutoMerge Marked for automatic merge by the bot when requirements are met Impact-Correctness It be wrong. Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Second It's a PR that needs another sign-off Priority-3 A description (P3) Product-Terminal The new Windows Terminal.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Multi-line paste warning doesn't detect \r, only \n Terminal doesn't translate LF to CR on paste; it only translates CRLF

4 participants

@PankajBhojwani @DHowett @Don-Vito @zadjii-msft