-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Pass through DCS responses when VT input mode is disabled #17845
Pass through DCS responses when VT input mode is disabled #17845
Conversation
Note that for this to work correctly you also need PR #17833. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for fixing this!
6e5827a
into
microsoft:main
("I've also hacked Windows Terminal to disable win32-input mode" - FWIW, does |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Marking for 1.22! Thanks so much!
@DHowett That might have been good enough for what I was testing at the time, if I had been aware of it. :) Although having tried it now, it doesn't behave quite the way I expected. For example, I can still enable win32-input mode from within a WSL shell (with And longer term I was hoping we might introduce a concept of "locked" modes and settings that would cover a number of cases like this in a more unified way. I mentioned this before on the |
Hahahaha oh, oh no. Not at all. That's not awesome. I think it stops the generation of all Win32 input events -- or, it should -- even if they've been requested. See
I would love this. It would be perfect, especially now that Terminal is involved in modes, if users could opt in and out and modes could be locked. |
## Summary of the Pull Request When an app makes a VT request that returns a `DCS` response, and it hasn't also enabled VT input mode, the new passthrough implementation loses that response. All the app receives is an `Alt`+`\` key press triggered by the `ST` terminator. This PR fixes that issue. ## References and Relevant Issues This is one of the unresolved issues tracked in #17643. ## Detailed Description of the Pull Request / Additional comments The way `DCS` sequences are handled in the input state machine engine is by returning a nullptr from `ActionDcsDispatch`, which tells the state machine to ignore that content. But the sequence is still buffered, and when the `ST` terminator is eventually received, that buffer is flushed, which passes the whole thing through to the app. Originally this only worked when VT input mode was enabled, otherwise the `ST` sequence is converted into a key press, and the buffered `DCS` content is lost. The way it works now is we set a flag when the `DCS` introducer is received, and if that flag is set when the `ST` arrives, we know to trigger a flush rather a key press. ## Validation Steps Performed I've tested a `DA3` request from the cmd shell (i.e. `echo ^[[=c`), and confirmed that now works as expected. I've also hacked Windows Terminal to disable win32-input mode, so I could check how it works with conpty clients generating standard VT input, and confirmed that an `Alt`+`\` keypress is still translated correctly. (cherry picked from commit 6e5827a) Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgSwKBc Service-Version: 1.22
Summary of the Pull Request
When an app makes a VT request that returns a
DCS
response, and ithasn't also enabled VT input mode, the new passthrough implementation
loses that response. All the app receives is an
Alt
+\
key presstriggered by the
ST
terminator. This PR fixes that issue.References and Relevant Issues
This is one of the unresolved issues tracked in #17643.
Detailed Description of the Pull Request / Additional comments
The way
DCS
sequences are handled in the input state machine engine isby returning a nullptr from
ActionDcsDispatch
, which tells the statemachine to ignore that content. But the sequence is still buffered, and
when the
ST
terminator is eventually received, that buffer is flushed,which passes the whole thing through to the app.
Originally this only worked when VT input mode was enabled, otherwise
the
ST
sequence is converted into a key press, and the bufferedDCS
content is lost. The way it works now is we set a flag when the
DCS
introducer is received, and if that flag is set when the
ST
arrives,we know to trigger a flush rather a key press.
Validation Steps Performed
I've tested a
DA3
request from the cmd shell (i.e.echo ^[[=c
), andconfirmed that now works as expected. I've also hacked Windows Terminal
to disable win32-input mode, so I could check how it works with conpty
clients generating standard VT input, and confirmed that an
Alt
+\
keypress is still translated correctly.