Skip to content

Fix another VT input double-encoding issue #19083

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.

Sign up for GitHub

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

Jump to bottom
Merged
DHowett merged 3 commits into from
Jul 2, 2025
Merged

Fix another VT input double-encoding issue #19083

DHowett merged 3 commits into from
Jul 2, 2025

Conversation

@lhecker
Copy link
Member

@lhecker lhecker commented Jun 30, 2025

Closes #17264
Closes microsoft/edit#182

Long shot, but probably also...
Closes #18579
Closes #19082

@lhecker lhecker force-pushed the dev/lhecker/17264-double-encoding-again branch from 26e4e4c to 5c06ff4 Compare June 30, 2025 15:22
@lhecker
Copy link
Member Author

lhecker commented Jun 30, 2025

@j4james You wrote:

But I think there is a flaw in this approach. The WriteString path [...] does some CONSOLE_SUSPENDED handling [...]

I believe we don't need to care about this, because the VT parser already detects control characters and F-key input sequences, etc., and calls Write directly in that case. I tested this with Ctrl+S and it worked just fine with this PR.

@j4james j4james mentioned this pull request Jul 1, 2025
Closed
DHowett
DHowett approved these changes Jul 2, 2025
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.

Will this have a negative impact on hosting terminals which do not use mode 9001, talking to applications which use ENABLE_VIRTUAL_TERMINAL_INPUT?

@DHowett DHowett merged commit 97f0a06 into main Jul 2, 2025
19 checks passed
@DHowett DHowett deleted the dev/lhecker/17264-double-encoding-again branch July 2, 2025 23:49
@mengq627
Copy link

mengq627 commented Jul 3, 2025 •
edited
Loading

How do I get a daily build version contains this commit?

@lhecker
Copy link
Member Author

lhecker commented Jul 3, 2025

Check out Windows Terminal Canary. The next version with this should be out soon (there are currently some CI issues).

@mengq627
Copy link

mengq627 commented Jul 3, 2025

OK, so thanks

DHowett pushed a commit that referenced this pull request Jul 29, 2025
@lhecker @DHowett
Fix another VT input double-encoding issue (#19083)
0512af5 
Closes #17264
Closes microsoft/edit#182

Long shot, but probably also...
Closes #18579
Closes #19082

(cherry picked from commit 97f0a06)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgWZBNU
Service-Version: 1.22
@DHowett DHowett mentioned this pull request Aug 1, 2025
Closed
@DHowett
Copy link
Member

DHowett commented Aug 1, 2025

ugh, i forgot to put this into 1.23. It's in 1.22 though!

DHowett pushed a commit that referenced this pull request Aug 1, 2025
@lhecker @DHowett
Fix another VT input double-encoding issue (#19083)
ed022e8 
Closes #17264
Closes microsoft/edit#182

Long shot, but probably also...
Closes #18579
Closes #19082

(cherry picked from commit 97f0a06)
Service-Card-Id: PVTI_lADOAF3p4s4AxadtzgdOWF0
Service-Version: 1.23
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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

4 participants

@lhecker @mengq627 @DHowett