Skip to content
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

ConPTY: transmit DECSET/DECRST state when client application enters/exits ENABLE_VIRTUAL_TERMINAL_INPUT mode #6859

Open
DHowett opened this issue Jul 10, 2020 · 9 comments
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conpty For console issues specifically related to conpty
Milestone

Comments

@DHowett
Copy link
Member

DHowett commented Jul 10, 2020

Certain applications (mainly those using the Cygwin/MSYS runtime) will attempt to change VT input parameters while VT input is technically off. Because we do nothing to communicate the expected VT input parameters to a connected terminal before we turn on VT input passthrough, it's possible for the connected terminal to disagree with the conhost acting as its pty host.

When the application enters ENABLE_VIRTUAL_TERMINAL_INPUT mode, ConPTY should send a string of DECSET/DECRST sequences to bring the connected terminal in line with it to keep up the illusion of passthrough.

Notes from #6737:

From the Terminal's perspective, ENABLE_VIRTUAL_TERMINAL_INPUT is always on. It can't be lost/forgotten, because the terminal only generates VT. In general, it always has conhost sitting between it and the application to do translation from VT input. There are cases when the conhost sitting in the middle enters passthrough mode and dumps input straight out of terminal into the application.
Therefore, Terminal needs to be aware of DECCKM. Whether it receives DECCKM and therefore whether it generates the right cursor keys, however, is based on whether the application has requested ENABLE_VIRTUAL_TERMINAL_INPUT.
(There are applications that ask for DECCKM or other VT input format changes but are not in VT input mode, and sending them that data will freak them out.)

For WT 1.0, here's the matrix of behaviors.

ActionVT Output ModeVT Input ModeNotes
Application prints \e[?1h VT output is off, request ignored
conhost (PTY) (only) enables DECCKM
conhost (PTY) _and Terminal_ enable DECCKM
ActionVT Output ModeVT Input ModePTY in DECCKM modeTerminal in DECCKM modeNotes
User presses arrow key Terminal emits \e[A or \eOA
conhost (PTY) translates it into KEY_EVENT_RECORD
Terminal emits \e[A
conhost (PTY) translates it into KEY_EVENT_RECORD
Terminal emits \eOA
conhost (PTY) translates it into KEY_EVENT_RECORD
passthrough mode Terminal emits \e[A
conhost (PTY) passes it through unmodified
Terminal emits \eOA
conhost (PTY) passes it through unmodified
BEHAVIORS BEFORE TERMINAL 1.0
ActionVT Output ModeVT Input ModeNotes
Application prints \e[?1h VT output is off, request ignored
conhost (PTY) (only) enables DECCKM
ActionVT Output ModeVT Input ModePTY in DECCKM modeNotes
User presses arrow key ignored Terminal emits \e[A
conhost (PTY) translates it into KEY_EVENT_RECORD
Terminal emits \e[A
conhost (PTY) translates it into \e[A
Terminal emits \e[A
conhost (PTY) translates it into \e[OA

@DHowett

Okay, I'm literally seeing this behavior at the PTY:

  • Input Mode = 0x9
  • Input Mode = 0x209
  • Input Mode = 0x9
  • Print out DECCKM
  • Input Mode = 0x209
  • Input Mode = 0x9

This happens with printf, this happens with less's startup (though it prints more than just DECCKM)`, this happens pretty much every time I launch a process.

It's quite literally disabling VT input mode only while it is changing the input configuration. If we'd ever made the console more strict ("VT input mode changes only apply when VT input mode is on"), this would break horribly. I wonder why they'd do this?
@DHowett

Whether it receives DECCKM and therefore whether it generates the right cursor keys, however, is based on whether the application has requested ENABLE_VIRTUAL_TERMINAL_INPUT.

I totally did not expect that. If ENABLE_VIRTUAL_TERMINAL_INPUT is designed to affect which control sequences a pseudoconsole forwards from WriteConsole to the terminal, it is worth documenting.

This regressed with #6485

Why doesn't Windows Terminal Preview 1.1.1812.0 have the same problem, then? 6ff8ba7 was cherry-picked from 5e2c4c6, which is included in that version.
@KalleOlaviNiemitalo

@DHowett DHowett added Product-Conpty For console issues specifically related to conpty Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) labels Jul 10, 2020
@DHowett DHowett added this to the 21H1 milestone Jul 10, 2020
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jul 10, 2020
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jul 10, 2020
@RokyBanana

This comment has been minimized.

@DHowett

This comment has been minimized.

@RokyBanana

This comment has been minimized.

@DHowett
Copy link
Member Author

DHowett commented Jul 16, 2020

Oh, sorry! Windows Terminal Preview 1.1+, which should be released to the stable channel soon, doesn’t exhibit this issue. Note if you switch today that it won’t migrate your settings from the Release branch. It’s still pretty stable; we all use it every day to make sure we’re the most exposed to any weird issues it might have. 😄

@DHowett
Copy link
Member Author

DHowett commented Jul 19, 2020

This is wild!

cygwin/msys2 sets and resets ENABLE_VIRTUAL_TERMINAL_INPUT not on every process launch, but for literally every character that's typed.

When we emit DECSET/DECRST for every state transition (to make sure the connected terminal does the right thing even outside of win32 input mode), each byte input into a msys2 application results in about 200 bytes of control sequences just to maintain terminal state.

(@KalleOlaviNiemitalo might find this interesting, even if just because they've been helping out with some VT_INPUT issues 😄)

@lazka
Copy link

lazka commented Aug 2, 2020

(@tyan0 is working in this area in cygwin, mentioning here in case he wants to chime in)

@dsm1212

This comment has been minimized.

@hlovdal

This comment has been minimized.

@zadjii-msft

This comment has been minimized.

ghost pushed a commit that referenced this issue May 7, 2021
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
When the client application sets the console input mode with the `ENABLE_MOUSE_INPUT` flag, we send along the appropriate VT sequences to the connected terminal (if there is one).

<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> 
## References
#376 
#6859 

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [ ] Closes #xxx
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [x] I work here

<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Far manager works
@zadjii-msft zadjii-msft modified the milestones: Windows vNext, 22H1 Jan 4, 2022
@zadjii-msft zadjii-msft modified the milestones: 22H1, Terminal v1.14 Feb 2, 2022
@zadjii-msft zadjii-msft modified the milestones: Terminal v1.14, 22H2 Mar 10, 2022
@zadjii-msft zadjii-msft modified the milestones: 22H2, Backlog Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conpty For console issues specifically related to conpty
Projects
None yet
Development

No branches or pull requests

6 participants