Skip to content

Commit

Permalink
Pass through DCS responses when VT input mode is disabled (#17845)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
j4james authored and DHowett committed Sep 6, 2024
1 parent 03fc325 commit 385a9a6
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
15 changes: 14 additions & 1 deletion src/terminal/parser/InputStateMachineEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,16 @@ bool InputStateMachineEngine::ActionPassThroughString(const std::wstring_view st
// - true iff we successfully dispatched the sequence.
bool InputStateMachineEngine::ActionEscDispatch(const VTID id)
{
// If the _expectingStringTerminator flag is set, that means we've been
// processing a DCS sequence and are waiting for the string terminator.
// Once we receive the ST sequence here, we can return false to force the
// buffered DCS content to be flushed.
if (_expectingStringTerminator && id == VTID("\\"))
{
_expectingStringTerminator = false;
return false;
}

if (_pDispatch->IsVtInputEnabled())
{
return false;
Expand Down Expand Up @@ -507,7 +517,10 @@ bool InputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParameter
// - the data string handler function or nullptr if the sequence is not supported
IStateMachineEngine::StringHandler InputStateMachineEngine::ActionDcsDispatch(const VTID /*id*/, const VTParameters /*parameters*/) noexcept
{
// DCS escape sequences are not used in the input state machine.
// Returning a nullptr here will cause the content of the DCS sequence to be
// ignored, but it'll still be buffered by the state machine, so we can flush
// the whole thing once we receive the string terminator.
_expectingStringTerminator = true;
return nullptr;
}

Expand Down
1 change: 1 addition & 0 deletions src/terminal/parser/InputStateMachineEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ namespace Microsoft::Console::VirtualTerminal
std::atomic<uint64_t> _deviceAttributes{ 0 };
bool _lookingForDSR = false;
bool _encounteredWin32InputModeSequence = false;
bool _expectingStringTerminator = false;
DWORD _mouseButtonState = 0;
std::chrono::milliseconds _doubleClickTime;
std::optional<til::point> _lastMouseClickPos{};
Expand Down

0 comments on commit 385a9a6

Please sign in to comment.