Fix ConPTY inputs incorrectly being treated as plain text #16352
Conversation
lhecker
changed the title
lhecker
added
Product-Conpty
There was a problem hiding this comment.
I'm mostly comfortable with this, but I wanted to get your opinion @j4james. We're gently skirting around the fact that we don't have a control sequence to input dispatcher that emits a buffered escape character after a timeout (e.g. the one in Vim that makes it possible to type ESC O and have it work "properly")...
I'm hesitant to add a timeout here since (1) we don't have the infrastructure for it and (2) that would compound with (e.g.) Vim's timeout.
This seems like the least-worst of all the worlds, where a Win32 input sequence spends 18 bytes in CsiParam state and if we catch any of them we want to continue buffering until we got the whole thing.
| // Manually execute the last char [pwchCurr] | ||
| _processingLastCharacter = true; | ||
| switch (_state) | ||
| if (run.size() <= 2 && run.front() == L'\x1b') |
There was a problem hiding this comment.
@lhecker make sure this works properly in Terminal, and with wsl vim
|
I really like this. The only edge case I could think of that might be problematic is an Alt key combination where the second key is a surrogate pair. That would assumedly produce a 3 character sequence: |
This is my proposal to avoid aborting ConPTY input parsing because a read accidentally got split up into more than one chunk. This happens a lot with WSL for me, as I often get (for instance) a `\x1b[67;46;99;0;32;` input followed immediately by a `1_` input. The current logic would cause both of these to be flushed out to the client application. This PR fixes the issue by only flushing either a standalone escape character or a escape+character combination. It basically limits the previous code to just `VTStates::Ground` and `VTStates::Escape`. I'm not using the `_state` member, because `VTStates::OscParam` makes no distinction between `\x1b]` and `\x1b]1234` and I only want to flush the former. I felt like checking the contents of `run` directly is easier to understand. Related to #16343 ## Validation Steps Performed * win32-input-mode sequences are now properly buffered ✅ * Standalone alt-key combinations are still being flushed ✅ (cherry picked from commit 5f5ef10) Service-Card-Id: 91270261 Service-Version: 1.19
This is my proposal to avoid aborting ConPTY input parsing because a read accidentally got split up into more than one chunk. This happens a lot with WSL for me, as I often get (for instance) a
\x1b[67;46;99;0;32;input followed immediately by a1_input. The current logic would cause both of these to be flushed out to the client application.This PR fixes the issue by only flushing either a standalone escape character or a escape+character combination. It basically limits the previous code to just
VTStates::GroundandVTStates::Escape.I'm not using the
_statemember, becauseVTStates::OscParammakes no distinction between\x1b]and\x1b]1234and I only want to flush the former. I felt like checking the contents ofrundirectly is easier to understand.Related to #16343
Validation Steps Performed