From 94bcbb92045f9daf16168cd82b1abdf731065bc8 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 14 Jun 2019 14:48:12 -0500 Subject: [PATCH] The InputStateMachine should dispatch Intermediate characters (#1267) The OutputStateMachine needs to collect "Intermediate" characters to be able to call [`Designate G0 Character Set`](https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Controls-beginning-with-ESC) (as well as other sequences we don't yet support). However, the InputStateMachine used by conpty to process input should _not_ collect these characters. The input engine uses `\x1b` as an indicator that a key was pressed with `Alt`. For keys like `/`, we want to dispatch the key immediately, instead of collecting it and leaving us in the Escape state. --- OpenConsole.sln | 3 + src/terminal/parser/IStateMachineEngine.hpp | 1 + .../parser/InputStateMachineEngine.cpp | 13 ++++ .../parser/InputStateMachineEngine.hpp | 1 + .../parser/OutputStateMachineEngine.cpp | 12 ++++ .../parser/OutputStateMachineEngine.hpp | 1 + src/terminal/parser/stateMachine.cpp | 12 +++- .../parser/ut_parser/InputEngineTest.cpp | 61 +++++++++++++++++++ 8 files changed, 102 insertions(+), 2 deletions(-) diff --git a/OpenConsole.sln b/OpenConsole.sln index 892383ddff..a006c00482 100644 --- a/OpenConsole.sln +++ b/OpenConsole.sln @@ -149,6 +149,9 @@ EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "RendererVt", "src\renderer\vt\lib\vt.vcxproj", "{990F2657-8580-4828-943F-5DD657D11842}" EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "VtPipeTerm", "src\tools\vtpipeterm\VtPipeTerm.vcxproj", "{814DBDDE-894E-4327-A6E1-740504850098}" + ProjectSection(ProjectDependencies) = postProject + {9CBD7DFA-1754-4A9D-93D7-857A9D17CB1B} = {9CBD7DFA-1754-4A9D-93D7-857A9D17CB1B} + EndProjectSection EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "ConEchoKey", "src\tools\echokey\ConEchoKey.vcxproj", "{814CBEEE-894E-4327-A6E1-740504850098}" EndProject diff --git a/src/terminal/parser/IStateMachineEngine.hpp b/src/terminal/parser/IStateMachineEngine.hpp index 54a3c29581..97430e359a 100644 --- a/src/terminal/parser/IStateMachineEngine.hpp +++ b/src/terminal/parser/IStateMachineEngine.hpp @@ -52,6 +52,7 @@ namespace Microsoft::Console::VirtualTerminal virtual bool FlushAtEndOfString() const = 0; virtual bool DispatchControlCharsFromEscape() const = 0; + virtual bool DispatchIntermediatesFromEscape() const = 0; }; inline IStateMachineEngine::~IStateMachineEngine() {} diff --git a/src/terminal/parser/InputStateMachineEngine.cpp b/src/terminal/parser/InputStateMachineEngine.cpp index 8ef0049dc9..0720e79ccd 100644 --- a/src/terminal/parser/InputStateMachineEngine.cpp +++ b/src/terminal/parser/InputStateMachineEngine.cpp @@ -883,6 +883,19 @@ bool InputStateMachineEngine::DispatchControlCharsFromEscape() const return true; } +// Routine Description: +// - Returns false if the engine wants to be able to collect intermediate +// characters in the Escape state. We do _not_ want to buffer any characters +// as intermediates, because we use ESC as a prefix to indicate a key was +// pressed while Alt was pressed. +// Return Value: +// - True iff we should dispatch in the Escape state when we encounter a +// Intermediate character. +bool InputStateMachineEngine::DispatchIntermediatesFromEscape() const +{ + return true; +} + // Method Description: // - Retrieves the type of window manipulation operation from the parameter pool // stored during Param actions. diff --git a/src/terminal/parser/InputStateMachineEngine.hpp b/src/terminal/parser/InputStateMachineEngine.hpp index 0e1a334070..cbb8d0acf6 100644 --- a/src/terminal/parser/InputStateMachineEngine.hpp +++ b/src/terminal/parser/InputStateMachineEngine.hpp @@ -66,6 +66,7 @@ namespace Microsoft::Console::VirtualTerminal bool FlushAtEndOfString() const override; bool DispatchControlCharsFromEscape() const override; + bool DispatchIntermediatesFromEscape() const override; private: const std::unique_ptr _pDispatch; diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp index 9b20caa38c..a17699a524 100644 --- a/src/terminal/parser/OutputStateMachineEngine.cpp +++ b/src/terminal/parser/OutputStateMachineEngine.cpp @@ -1317,6 +1317,18 @@ bool OutputStateMachineEngine::DispatchControlCharsFromEscape() const return false; } +// Routine Description: +// - Returns false if the engine wants to be able to collect intermediate +// characters in the Escape state. We do want to buffer characters as +// intermediates. We need them for things like Designate G0 Character Set +// Return Value: +// - True iff we should dispatch in the Escape state when we encounter a +// Intermediate character. +bool OutputStateMachineEngine::DispatchIntermediatesFromEscape() const +{ + return false; +} + // Routine Description: // - Converts a hex character to its equivalent integer value. // Arguments: diff --git a/src/terminal/parser/OutputStateMachineEngine.hpp b/src/terminal/parser/OutputStateMachineEngine.hpp index 5ef8de6bee..c4c5bf334f 100644 --- a/src/terminal/parser/OutputStateMachineEngine.hpp +++ b/src/terminal/parser/OutputStateMachineEngine.hpp @@ -58,6 +58,7 @@ namespace Microsoft::Console::VirtualTerminal bool FlushAtEndOfString() const override; bool DispatchControlCharsFromEscape() const override; + bool DispatchIntermediatesFromEscape() const override; void SetTerminalConnection(Microsoft::Console::ITerminalOutputConnection* const pTtyConnection, std::function pfnFlushToTerminal); diff --git a/src/terminal/parser/stateMachine.cpp b/src/terminal/parser/stateMachine.cpp index ebab72d591..99b49a5865 100644 --- a/src/terminal/parser/stateMachine.cpp +++ b/src/terminal/parser/stateMachine.cpp @@ -853,8 +853,16 @@ void StateMachine::_EventEscape(const wchar_t wch) } else if (s_IsIntermediate(wch)) { - _ActionCollect(wch); - _EnterEscapeIntermediate(); + if (_pEngine->DispatchIntermediatesFromEscape()) + { + _ActionEscDispatch(wch); + _EnterGround(); + } + else + { + _ActionCollect(wch); + _EnterEscapeIntermediate(); + } } else if (s_IsCsiIndicator(wch)) { diff --git a/src/terminal/parser/ut_parser/InputEngineTest.cpp b/src/terminal/parser/ut_parser/InputEngineTest.cpp index bee06f35fa..fcb67d59a1 100644 --- a/src/terminal/parser/ut_parser/InputEngineTest.cpp +++ b/src/terminal/parser/ut_parser/InputEngineTest.cpp @@ -229,6 +229,7 @@ class Microsoft::Console::VirtualTerminal::InputEngineTest TEST_METHOD(CSICursorBackTabTest); TEST_METHOD(AltBackspaceTest); TEST_METHOD(AltCtrlDTest); + TEST_METHOD(AltIntermediateTest); friend class TestInteractDispatch; }; @@ -765,3 +766,63 @@ void InputEngineTest::AltCtrlDTest() Log::Comment(NoThrowString().Format(L"Processing \"\\x1b\\x04\"")); _stateMachine->ProcessString(seq); } + +void InputEngineTest::AltIntermediateTest() +{ + // Tests GH#1209. When we process a alt+key combination where the key just + // so happens to be an intermediate character, we should make sure that an + // immediately subsequent ctrl character is handled correctly. + TestState testState; + + // We'll test this by creating both a TerminalInput and an + // InputStateMachine, and piping the KeyEvents generated by the + // InputStateMachine into the TerminalInput. + std::wstring expectedTranslation{}; + + // First create the callback TerminalInput will call - this will be + // triggered second, after both the state machine and the TerminalInput have + // translated the characters. + auto pfnTerminalInputCallback = [&](std::deque>& inEvents) { + // Get all the characters: + std::wstring wstr = L""; + for (auto& ev : inEvents) + { + if (ev->EventType() == InputEventType::KeyEvent) + { + auto& k = static_cast(*ev); + auto wch = k.GetCharData(); + wstr += wch; + } + } + + VERIFY_ARE_EQUAL(expectedTranslation, wstr); + }; + TerminalInput terminalInput{ pfnTerminalInputCallback }; + + // Create the callback that's fired when the state machine wants to write + // input. We'll take the events and put them straight into the + // TerminalInput. + auto pfnInputStateMachineCallback = [&](std::deque>& inEvents) { + for (auto& ev : inEvents) + { + terminalInput.HandleKey(ev.get()); + } + }; + auto inputEngine = std::make_unique(new TestInteractDispatch(pfnInputStateMachineCallback, &testState)); + auto stateMachine = std::make_unique(inputEngine.release()); + VERIFY_IS_NOT_NULL(stateMachine); + testState._stateMachine = stateMachine.get(); + + // Write a Alt+/, Ctrl+e pair to the input engine, then take it's output and + // run it through the terminalInput translator. We should get ^[/^E back + // out. + std::wstring seq = L"\x1b/"; + expectedTranslation = seq; + Log::Comment(NoThrowString().Format(L"Processing \"\\x1b/\"")); + stateMachine->ProcessString(seq); + + seq = L"\x05"; // 0x05 is ^E + expectedTranslation = seq; + Log::Comment(NoThrowString().Format(L"Processing \"\\x05\"")); + stateMachine->ProcessString(seq); +}