From 2559ed6efa05cb8f999fbf0b8d890ec81161220a Mon Sep 17 00:00:00 2001 From: James Holderness Date: Fri, 30 Apr 2021 20:17:30 +0100 Subject: [PATCH] Introduce a mechanism for passing through DCS data strings (#9307) This PR introduces a mechanism via which DCS data strings can be passed through directly to the dispatch method that will be handling them, so the data can be processed as it is received, rather than being buffered in the state machine. This also simplifies the way string termination is handled, so it now more closely matches the behaviour of the original DEC terminals. * Initial support for DCS sequences was introduced in PR #6328. * Handling of DCS (and other) C1 controls was added in PR #7340. * This is a prerequisite for Sixel (#448) and Soft Font (#9164) support. The way this now works, a `DCS` sequence is dispatched as soon as the final character of the `VTID` is received. Based on that ID, the `OutputStateMachineEngine` should forward the call to the corresponding dispatch method, and its the responsibility of that method to return an appropriate handler function for the sequence. From then on, the `StateMachine` will pass on all of the remaining bytes in the data string to the handler function. When a data string is terminated (with `CAN`, `SUB`, or `ESC`), the `StateMachine` will pass on one final `ESC` character to let the handler know that the sequence is finished. The handler can also end a sequence prematurely by returning false, and then all remaining data bytes will be ignored. Note that once a `DCS` sequence has been dispatched, it's not possible to abort the data string. Both `CAN` and `SUB` are considered valid forms of termination, and an `ESC` doesn't necessarily have to be followed by a `\` for the string terminator. This is because the data string is typically processed as it's received. For example, when outputting a Sixel image, you wouldn't erase the parts that had already been displayed if the data string is terminated early. With this new way of handling the string termination, I was also able to simplify some of the `StateMachine` processing, and get rid of a few states that are no longer necessary. These changes don't apply to the `OSC` sequences, though, since we're more likely to want to match the XTerm behavior for those cases (which requires a valid `ST` control for the sequence to be accepted). ## Validation Steps Performed For the unit tests, I've had to make a few changes to some of the `OutputEngineTests` to account for the updated `StateMachine` processing. I've also added a new `StateMachineTest` to confirm that the data strings are correctly passed through to the string handler under all forms of termination. To test whether the framework is actually usable, I've been working on DRCS Soft Font support branched off of this PR, and haven't encountered any problems. To test the throughput speed, I also hacked together a basic Sixel parser, and that seemed to perform reasonably well. Closes #7316 --- src/terminal/parser/IStateMachineEngine.hpp | 3 + .../parser/InputStateMachineEngine.cpp | 15 ++ .../parser/InputStateMachineEngine.hpp | 2 + .../parser/OutputStateMachineEngine.cpp | 18 ++ .../parser/OutputStateMachineEngine.hpp | 2 + src/terminal/parser/stateMachine.cpp | 228 +++++++----------- src/terminal/parser/stateMachine.hpp | 14 +- .../parser/ut_parser/OutputEngineTest.cpp | 51 ++-- .../parser/ut_parser/StateMachineTest.cpp | 102 +++++++- 9 files changed, 249 insertions(+), 186 deletions(-) diff --git a/src/terminal/parser/IStateMachineEngine.hpp b/src/terminal/parser/IStateMachineEngine.hpp index 46b88f3c946..48decb8f6a2 100644 --- a/src/terminal/parser/IStateMachineEngine.hpp +++ b/src/terminal/parser/IStateMachineEngine.hpp @@ -20,6 +20,8 @@ namespace Microsoft::Console::VirtualTerminal class IStateMachineEngine { public: + using StringHandler = std::function; + virtual ~IStateMachineEngine() = 0; IStateMachineEngine(const IStateMachineEngine&) = default; IStateMachineEngine(IStateMachineEngine&&) = default; @@ -36,6 +38,7 @@ namespace Microsoft::Console::VirtualTerminal virtual bool ActionEscDispatch(const VTID id) = 0; virtual bool ActionVt52EscDispatch(const VTID id, const VTParameters parameters) = 0; virtual bool ActionCsiDispatch(const VTID id, const VTParameters parameters) = 0; + virtual StringHandler ActionDcsDispatch(const VTID id, const VTParameters parameters) = 0; virtual bool ActionClear() = 0; diff --git a/src/terminal/parser/InputStateMachineEngine.cpp b/src/terminal/parser/InputStateMachineEngine.cpp index fa0de145ccb..53a7cab66f2 100644 --- a/src/terminal/parser/InputStateMachineEngine.cpp +++ b/src/terminal/parser/InputStateMachineEngine.cpp @@ -446,6 +446,21 @@ bool InputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParameter return success; } +// Routine Description: +// - Triggers the DcsDispatch action to indicate that the listener should handle +// a control sequence. Returns the handler function that is to be used to +// process the subsequent data string characters in the sequence. +// Arguments: +// - id - Identifier of the control sequence to dispatch. +// - parameters - set of numeric parameters collected while parsing the sequence. +// Return Value: +// - 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. + return nullptr; +} + // Routine Description: // - Triggers the Ss3Dispatch action to indicate that the listener should handle // a control sequence. These sequences perform various API-type commands diff --git a/src/terminal/parser/InputStateMachineEngine.hpp b/src/terminal/parser/InputStateMachineEngine.hpp index 9d36ee86732..7336511e4e1 100644 --- a/src/terminal/parser/InputStateMachineEngine.hpp +++ b/src/terminal/parser/InputStateMachineEngine.hpp @@ -146,6 +146,8 @@ namespace Microsoft::Console::VirtualTerminal bool ActionCsiDispatch(const VTID id, const VTParameters parameters) override; + StringHandler ActionDcsDispatch(const VTID id, const VTParameters parameters) noexcept override; + bool ActionClear() noexcept override; bool ActionIgnore() noexcept override; diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp index cdc753a7c23..0ecd5ea4da1 100644 --- a/src/terminal/parser/OutputStateMachineEngine.cpp +++ b/src/terminal/parser/OutputStateMachineEngine.cpp @@ -636,6 +636,24 @@ bool OutputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParamete return success; } +// Routine Description: +// - Triggers the DcsDispatch action to indicate that the listener should handle +// a control sequence. Returns the handler function that is to be used to +// process the subsequent data string characters in the sequence. +// Arguments: +// - id - Identifier of the control sequence to dispatch. +// - parameters - set of numeric parameters collected while parsing the sequence. +// Return Value: +// - the data string handler function or nullptr if the sequence is not supported +IStateMachineEngine::StringHandler OutputStateMachineEngine::ActionDcsDispatch(const VTID /*id*/, const VTParameters /*parameters*/) noexcept +{ + StringHandler handler = nullptr; + + _ClearLastChar(); + + return handler; +} + // Routine Description: // - Triggers the Clear action to indicate that the state machine should erase // all internal state. diff --git a/src/terminal/parser/OutputStateMachineEngine.hpp b/src/terminal/parser/OutputStateMachineEngine.hpp index ccfb49b915a..1f79795a91b 100644 --- a/src/terminal/parser/OutputStateMachineEngine.hpp +++ b/src/terminal/parser/OutputStateMachineEngine.hpp @@ -39,6 +39,8 @@ namespace Microsoft::Console::VirtualTerminal bool ActionCsiDispatch(const VTID id, const VTParameters parameters) override; + StringHandler ActionDcsDispatch(const VTID id, const VTParameters parameters) noexcept override; + bool ActionClear() noexcept override; bool ActionIgnore() noexcept override; diff --git a/src/terminal/parser/stateMachine.cpp b/src/terminal/parser/stateMachine.cpp index 9ea8ff47c09..de34fdaf84f 100644 --- a/src/terminal/parser/stateMachine.cpp +++ b/src/terminal/parser/stateMachine.cpp @@ -576,6 +576,8 @@ void StateMachine::_ActionClear() _oscString.clear(); _oscParameter = 0; + _dcsStringHandler = nullptr; + _engine->ActionClear(); } @@ -591,6 +593,24 @@ void StateMachine::_ActionIgnore() noexcept _trace.TraceOnAction(L"Ignore"); } +// Routine Description: +// - Triggers the end of a data string when a CAN, SUB, or ESC is seen. +// Arguments: +// - +// Return Value: +// - +void StateMachine::_ActionInterrupt() +{ + // This is only applicable for DCS strings. OSC strings require a full + // ST sequence to be received before they can be dispatched. + if (_state == VTStates::DcsPassThrough) + { + // The ESC signals the end of the data string. + _dcsStringHandler(AsciiChars::ESC); + _dcsStringHandler = nullptr; + } +} + // Routine Description: // - Stores this character as part of the param indicating which OSC action to take. // Arguments: @@ -664,16 +684,36 @@ void StateMachine::_ActionSs3Dispatch(const wchar_t wch) } // Routine Description: -// - Triggers the DcsPassThrough action to indicate that the listener should handle a DCS data string character. +// - Triggers the DcsDispatch action to indicate that the listener should handle a control sequence. +// The returned handler function will be used to process the subsequent data string characters. // Arguments: // - wch - Character to dispatch. // Return Value: // - -void StateMachine::_ActionDcsPassThrough(const wchar_t wch) +void StateMachine::_ActionDcsDispatch(const wchar_t wch) { - _trace.TraceOnAction(L"DcsPassThrough"); - _trace.TraceOnExecute(wch); - // TODO:GH#7316: Send the DCS passthrough sequence to the engine + _trace.TraceOnAction(L"DcsDispatch"); + + _dcsStringHandler = _engine->ActionDcsDispatch(_identifier.Finalize(wch), + { _parameters.data(), _parameters.size() }); + + // If the returned handler is null, the sequence is not supported. + const bool success = _dcsStringHandler != nullptr; + + // Trace the result. + _trace.DispatchSequenceTrace(success); + + if (success) + { + // If successful, enter the pass through state. + _EnterDcsPassThrough(); + } + else + { + // Otherwise ignore remaining chars and log telemetry on failed cases + _EnterDcsIgnore(); + TermTelemetry::Instance().LogFailed(wch); + } } // Routine Description: @@ -942,21 +982,6 @@ void StateMachine::_EnterDcsPassThrough() noexcept _trace.TraceStateChange(L"DcsPassThrough"); } -// Routine Description: -// - Moves the state machine into the DcsTermination state. -// This state is entered: -// 1. When an ESC is seen in a DCS string. This escape will be followed by a -// '\', as to encode a 0x9C as a 7-bit ASCII char stream. -// Arguments: -// - -// Return Value: -// - -void StateMachine::_EnterDcsTermination() noexcept -{ - _state = VTStates::DcsTermination; - _trace.TraceStateChange(L"DcsTermination"); -} - // Routine Description: // - Moves the state machine into the SosPmApcString state. // This state is entered: @@ -973,21 +998,6 @@ void StateMachine::_EnterSosPmApcString() noexcept _trace.TraceStateChange(L"SosPmApcString"); } -// Routine Description: -// - Moves the state machine into the SosPmApcStringTermination state. -// This state is entered: -// 1. When an ESC is seen in a SOS/PM/APC string. This escape will be followed by a -// '\', as to encode a 0x9C as a 7-bit ASCII char stream. -// Arguments: -// - -// Return Value: -// - -void StateMachine::_EnterSosPmApcTermination() noexcept -{ - _state = VTStates::SosPmApcTermination; - _trace.TraceStateChange(L"SosPmApcStringTermination"); -} - // Routine Description: // - Processes a character event into an Action that occurs while in the Ground state. // Events in this state will: @@ -1375,10 +1385,25 @@ void StateMachine::_EventOscString(const wchar_t wch) // - Handle the two-character termination of a OSC sequence. // Events in this state will: // 1. Trigger the OSC action associated with the param on an OscTerminator +// 2. Otherwise treat this as a normal escape character event. // Arguments: // - wch - Character that triggered the event // Return Value: // - +void StateMachine::_EventOscTermination(const wchar_t wch) +{ + _trace.TraceOnEvent(L"OscTermination"); + if (_isStringTerminatorIndicator(wch)) + { + _ActionOscDispatch(wch); + _EnterGround(); + } + else + { + _EnterEscape(); + _EventEscape(wch); + } +} // Routine Description: // - Processes a character event into an Action that occurs while in the Ss3Entry state. @@ -1505,7 +1530,7 @@ void StateMachine::_EventVt52Param(const wchar_t wch) // 3. Begin to ignore all remaining characters when an invalid character is detected (DcsIgnore) // 4. Store parameter data // 5. Collect Intermediate characters -// 6. Pass through everything else +// 6. Dispatch the Final character in preparation for parsing the data string // DCS sequences are structurally almost the same as CSI sequences, just with an // extra data string. It's safe to reuse CSI functions for // determining if a character is a parameter, delimiter, or invalid. @@ -1540,8 +1565,7 @@ void StateMachine::_EventDcsEntry(const wchar_t wch) } else { - _ActionDcsPassThrough(wch); - _EnterDcsPassThrough(); + _ActionDcsDispatch(wch); } } @@ -1566,8 +1590,7 @@ void StateMachine::_EventDcsIgnore() noexcept // 2. Ignore Delete characters // 3. Collect intermediate data. // 4. Begin to ignore all remaining intermediates when an invalid character is detected (DcsIgnore) -// 5. Enter DcsPassThrough if we see DCS pass through indicator -// 6. Pass through everything else. +// 5. Dispatch the Final character in preparation for parsing the data string // Arguments: // - wch - Character that triggered the event // Return Value: @@ -1593,8 +1616,7 @@ void StateMachine::_EventDcsIntermediate(const wchar_t wch) } else { - _ActionDcsPassThrough(wch); - _EnterDcsPassThrough(); + _ActionDcsDispatch(wch); } } @@ -1606,7 +1628,7 @@ void StateMachine::_EventDcsIntermediate(const wchar_t wch) // 3. Collect DCS parameter data // 4. Enter DcsIntermediate if we see an intermediate // 5. Begin to ignore all remaining parameters when an invalid character is detected (DcsIgnore) -// 6. Pass through everything else. +// 6. Dispatch the Final character in preparation for parsing the data string // Arguments: // - wch - Character that triggered the event // Return Value: @@ -1637,8 +1659,7 @@ void StateMachine::_EventDcsParam(const wchar_t wch) } else { - _ActionDcsPassThrough(wch); - _EnterDcsPassThrough(); + _ActionDcsDispatch(wch); } } @@ -1646,8 +1667,8 @@ void StateMachine::_EventDcsParam(const wchar_t wch) // - Processes a character event into an Action that occurs while in the DcsPassThrough state. // Events in this state will: // 1. Pass through if character is valid. -// 2. If we see a ESC, enter the DcsTermination state. -// 3. Ignore everything else. +// 2. Ignore everything else. +// The termination state is handled outside when an ESC is seen. // Arguments: // - wch - Character that triggered the event // Return Value: @@ -1657,11 +1678,10 @@ void StateMachine::_EventDcsPassThrough(const wchar_t wch) _trace.TraceOnEvent(L"DcsPassThrough"); if (_isC0Code(wch) || _isDcsPassThroughValid(wch)) { - _ActionDcsPassThrough(wch); - } - else if (_isEscape(wch)) - { - _EnterDcsTermination(); + if (!_dcsStringHandler(wch)) + { + _EnterDcsIgnore(); + } } else { @@ -1671,58 +1691,16 @@ void StateMachine::_EventDcsPassThrough(const wchar_t wch) // Routine Description: // - Handle SOS/PM/APC string. -// Events in this state will: -// 1. If we see a ESC, enter the SosPmApcTermination state. -// 2. Ignore everything else. +// In this state the entire string is ignored. +// The termination state is handled outside when an ESC is seen. // Arguments: // - wch - Character that triggered the event // Return Value: // - -void StateMachine::_EventSosPmApcString(const wchar_t wch) noexcept +void StateMachine::_EventSosPmApcString(const wchar_t /*wch*/) noexcept { _trace.TraceOnEvent(L"SosPmApcString"); - if (_isEscape(wch)) - { - _EnterSosPmApcTermination(); - } - else - { - _ActionIgnore(); - } -} - -// Routine Description: -// - Handle "Variable Length String" termination. -// Events in this state will: -// 1. Trigger the corresponding action and enter ground if we see a string terminator, -// 2. Otherwise treat this as a normal escape character event. -// Arguments: -// - wch - Character that triggered the event -// Return Value: -// - -void StateMachine::_EventVariableLengthStringTermination(const wchar_t wch) -{ - if (_isStringTerminatorIndicator(wch)) - { - if (_state == VTStates::OscTermination) - { - _ActionOscDispatch(wch); - } - else if (_state == VTStates::DcsTermination) - { - // TODO:GH#7316: The Dcs sequence has successfully terminated. This is where we'd be dispatching the DCS command. - } - else if (_state == VTStates::SosPmApcTermination) - { - // We don't support any SOS/PM/APC control string yet. - } - _EnterGround(); - } - else - { - _EnterEscape(); - _EventEscape(wch); - } + _ActionIgnore(); } // Routine Description: @@ -1744,43 +1722,20 @@ void StateMachine::ProcessCharacter(const wchar_t wch) // these from any state. if (isFromAnywhereChar && !(_state == VTStates::Escape && _engine->DispatchControlCharsFromEscape())) { + _ActionInterrupt(); _ActionExecute(wch); _EnterGround(); } // Preprocess C1 control characters and treat them as ESC + their 7-bit equivalent. else if (_isC1ControlCharacter(wch)) { - // When we are in "Variable Length String" state, a C1 control character - // should effectively acts as an ESC and move us into the corresponding - // termination state. - if (_IsVariableLengthStringState()) - { - if (_state == VTStates::OscString) - { - _EnterOscTermination(); - } - else if (_state == VTStates::DcsPassThrough) - { - _EnterDcsTermination(); - } - else if (_state == VTStates::SosPmApcString) - { - _EnterSosPmApcTermination(); - } - - _EventVariableLengthStringTermination(_c1To7Bit(wch)); - } - // Enter Escape state and pass the converted 7-bit character. - else - { - _EnterEscape(); - _EventEscape(_c1To7Bit(wch)); - } + ProcessCharacter(AsciiChars::ESC); + ProcessCharacter(_c1To7Bit(wch)); } - // Don't go to escape from the "Variable Length String" state - ESC (and C1 String Terminator) - // can be used to terminate variable length control string. - else if (_isEscape(wch) && !_IsVariableLengthStringState()) + // Don't go to escape from the OSC string state - ESC can be used to terminate OSC strings. + else if (_isEscape(wch) && _state != VTStates::OscString) { + _ActionInterrupt(); _EnterEscape(); } else @@ -1807,7 +1762,7 @@ void StateMachine::ProcessCharacter(const wchar_t wch) case VTStates::OscString: return _EventOscString(wch); case VTStates::OscTermination: - return _EventVariableLengthStringTermination(wch); + return _EventOscTermination(wch); case VTStates::Ss3Entry: return _EventSs3Entry(wch); case VTStates::Ss3Param: @@ -1824,12 +1779,8 @@ void StateMachine::ProcessCharacter(const wchar_t wch) return _EventDcsParam(wch); case VTStates::DcsPassThrough: return _EventDcsPassThrough(wch); - case VTStates::DcsTermination: - return _EventVariableLengthStringTermination(wch); case VTStates::SosPmApcString: return _EventSosPmApcString(wch); - case VTStates::SosPmApcTermination: - return _EventVariableLengthStringTermination(wch); default: return; } @@ -2051,16 +2002,3 @@ void StateMachine::_AccumulateTo(const wchar_t wch, size_t& value) noexcept value = MAX_PARAMETER_VALUE; } } - -// Routine Description: -// - Determines if the engine is in "Variable Length String" state, which is a combination -// of all states that are expecting a string that has a undetermined length. -// -// Arguments: -// - -// Return Value: -// - True if it is. False if it isn't. -const bool StateMachine::_IsVariableLengthStringState() const noexcept -{ - return _state == VTStates::OscString || _state == VTStates::DcsPassThrough || _state == VTStates::SosPmApcString; -} diff --git a/src/terminal/parser/stateMachine.hpp b/src/terminal/parser/stateMachine.hpp index f2f70fef112..87d0f88c8d3 100644 --- a/src/terminal/parser/stateMachine.hpp +++ b/src/terminal/parser/stateMachine.hpp @@ -67,10 +67,11 @@ namespace Microsoft::Console::VirtualTerminal void _ActionOscPut(const wchar_t wch); void _ActionOscDispatch(const wchar_t wch); void _ActionSs3Dispatch(const wchar_t wch); - void _ActionDcsPassThrough(const wchar_t wch); + void _ActionDcsDispatch(const wchar_t wch); void _ActionClear(); void _ActionIgnore() noexcept; + void _ActionInterrupt(); void _EnterGround() noexcept; void _EnterEscape(); @@ -90,9 +91,7 @@ namespace Microsoft::Console::VirtualTerminal void _EnterDcsIgnore() noexcept; void _EnterDcsIntermediate() noexcept; void _EnterDcsPassThrough() noexcept; - void _EnterDcsTermination() noexcept; void _EnterSosPmApcString() noexcept; - void _EnterSosPmApcTermination() noexcept; void _EventGround(const wchar_t wch); void _EventEscape(const wchar_t wch); @@ -103,6 +102,7 @@ namespace Microsoft::Console::VirtualTerminal void _EventCsiParam(const wchar_t wch); void _EventOscParam(const wchar_t wch) noexcept; void _EventOscString(const wchar_t wch); + void _EventOscTermination(const wchar_t wch); void _EventSs3Entry(const wchar_t wch); void _EventSs3Param(const wchar_t wch); void _EventVt52Param(const wchar_t wch); @@ -112,10 +112,8 @@ namespace Microsoft::Console::VirtualTerminal void _EventDcsParam(const wchar_t wch); void _EventDcsPassThrough(const wchar_t wch); void _EventSosPmApcString(const wchar_t wch) noexcept; - void _EventVariableLengthStringTermination(const wchar_t wch); void _AccumulateTo(const wchar_t wch, size_t& value) noexcept; - const bool _IsVariableLengthStringState() const noexcept; enum class VTStates { @@ -137,9 +135,7 @@ namespace Microsoft::Console::VirtualTerminal DcsIntermediate, DcsParam, DcsPassThrough, - DcsTermination, - SosPmApcString, - SosPmApcTermination + SosPmApcString }; Microsoft::Console::VirtualTerminal::ParserTracing _trace; @@ -159,6 +155,8 @@ namespace Microsoft::Console::VirtualTerminal std::wstring _oscString; size_t _oscParameter; + IStateMachineEngine::StringHandler _dcsStringHandler; + std::optional _cachedSequence; // This is tracked per state machine instance so that separate calls to Process* diff --git a/src/terminal/parser/ut_parser/OutputEngineTest.cpp b/src/terminal/parser/ut_parser/OutputEngineTest.cpp index b13adaf8afb..c5b9647312d 100644 --- a/src/terminal/parser/ut_parser/OutputEngineTest.cpp +++ b/src/terminal/parser/ut_parser/OutputEngineTest.cpp @@ -56,7 +56,7 @@ class Microsoft::Console::VirtualTerminal::OutputEngineTest final TEST_METHOD(TestEscapePath) { BEGIN_TEST_METHOD_PROPERTIES() - TEST_METHOD_PROPERTY(L"Data:uiTest", L"{0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19}") // one value for each type of state test below. + TEST_METHOD_PROPERTY(L"Data:uiTest", L"{0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17}") // one value for each type of state test below. END_TEST_METHOD_PROPERTIES() size_t uiTest; @@ -173,27 +173,16 @@ class Microsoft::Console::VirtualTerminal::OutputEngineTest final Log::Comment(L"Escape from DcsPassThrough"); shouldEscapeOut = false; mach._state = StateMachine::VTStates::DcsPassThrough; + mach._dcsStringHandler = [](const auto) { return true; }; break; } case 17: - { - Log::Comment(L"Escape from DcsTermination"); - mach._state = StateMachine::VTStates::DcsTermination; - break; - } - case 18: { Log::Comment(L"Escape from SosPmApcString"); shouldEscapeOut = false; mach._state = StateMachine::VTStates::SosPmApcString; break; } - case 19: - { - Log::Comment(L"Escape from SosPmApcTermination"); - mach._state = StateMachine::VTStates::SosPmApcTermination; - break; - } } mach.ProcessCharacter(AsciiChars::ESC); @@ -806,9 +795,10 @@ class Microsoft::Console::VirtualTerminal::OutputEngineTest final mach.ProcessCharacter(L' '); VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsIntermediate); mach.ProcessCharacter(L'x'); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsPassThrough); + // Note that without a dispatcher the pass through data is instead ignored. + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsIgnore); mach.ProcessCharacter(AsciiChars::ESC); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsTermination); + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Escape); mach.ProcessCharacter(L'\\'); VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Ground); } @@ -825,19 +815,20 @@ class Microsoft::Console::VirtualTerminal::OutputEngineTest final mach.ProcessCharacter(L'P'); VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsEntry); mach.ProcessCharacter(L'q'); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsPassThrough); + // Note that without a dispatcher the pass through state is instead ignored. + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsIgnore); mach.ProcessCharacter(L'#'); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsPassThrough); + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsIgnore); mach.ProcessCharacter(L'1'); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsPassThrough); + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsIgnore); mach.ProcessCharacter(L'N'); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsPassThrough); + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsIgnore); mach.ProcessCharacter(L'N'); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsPassThrough); + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsIgnore); mach.ProcessCharacter(L'N'); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsPassThrough); + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsIgnore); mach.ProcessCharacter(AsciiChars::ESC); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsTermination); + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Escape); mach.ProcessCharacter(L'\\'); VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Ground); } @@ -854,11 +845,11 @@ class Microsoft::Console::VirtualTerminal::OutputEngineTest final mach.ProcessCharacter(L'P'); VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsEntry); mach.ProcessCharacter(L'q'); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsPassThrough); + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsIgnore); mach.ProcessCharacter(L'#'); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsPassThrough); + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsIgnore); mach.ProcessCharacter(AsciiChars::ESC); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsTermination); + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Escape); mach.ProcessCharacter(L'['); // This is not a string terminator. VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiEntry); mach.ProcessCharacter(L'4'); @@ -885,7 +876,7 @@ class Microsoft::Console::VirtualTerminal::OutputEngineTest final mach.ProcessCharacter(L'2'); VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::SosPmApcString); mach.ProcessCharacter(AsciiChars::ESC); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::SosPmApcTermination); + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Escape); mach.ProcessCharacter(L'\\'); VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Ground); @@ -898,7 +889,7 @@ class Microsoft::Console::VirtualTerminal::OutputEngineTest final mach.ProcessCharacter(L'4'); VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::SosPmApcString); mach.ProcessCharacter(AsciiChars::ESC); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::SosPmApcTermination); + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Escape); mach.ProcessCharacter(L'\\'); VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Ground); @@ -911,7 +902,7 @@ class Microsoft::Console::VirtualTerminal::OutputEngineTest final mach.ProcessCharacter(L'6'); VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::SosPmApcString); mach.ProcessCharacter(AsciiChars::ESC); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::SosPmApcTermination); + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Escape); mach.ProcessCharacter(L'\\'); VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Ground); } @@ -943,9 +934,9 @@ class Microsoft::Console::VirtualTerminal::OutputEngineTest final mach.ProcessCharacter(L'P'); VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsEntry); mach.ProcessCharacter(L'q'); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsPassThrough); + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsIgnore); mach.ProcessCharacter(L'#'); - VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsPassThrough); + VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::DcsIgnore); mach.ProcessCharacter(L'1'); mach.ProcessCharacter(L'\x9c'); VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Ground); diff --git a/src/terminal/parser/ut_parser/StateMachineTest.cpp b/src/terminal/parser/ut_parser/StateMachineTest.cpp index 521f3e7a8bf..d2d79d99e53 100644 --- a/src/terminal/parser/ut_parser/StateMachineTest.cpp +++ b/src/terminal/parser/ut_parser/StateMachineTest.cpp @@ -32,10 +32,20 @@ class Microsoft::Console::VirtualTerminal::TestStateMachineEngine : public IStat { printed.clear(); passedThrough.clear(); + executed.clear(); + csiId = 0; csiParams.clear(); + dcsId = 0; + dcsParams.clear(); + dcsDataString.clear(); } - bool ActionExecute(const wchar_t /* wch */) override { return true; }; + bool ActionExecute(const wchar_t wch) override + { + executed += wch; + return true; + }; + bool ActionExecuteFromEscape(const wchar_t /* wch */) override { return true; }; bool ActionPrint(const wchar_t /* wch */) override { return true; }; bool ActionPrintString(const std::wstring_view string) override @@ -78,7 +88,7 @@ class Microsoft::Console::VirtualTerminal::TestStateMachineEngine : public IStat bool DispatchIntermediatesFromEscape() const override { return false; }; // ActionCsiDispatch is the only method that's actually implemented. - bool ActionCsiDispatch(const VTID /*id*/, const VTParameters parameters) override + bool ActionCsiDispatch(const VTID id, const VTParameters parameters) override { // If flush to terminal is registered for a test, then use it. if (pfnFlushToTerminal) @@ -88,6 +98,7 @@ class Microsoft::Console::VirtualTerminal::TestStateMachineEngine : public IStat } else { + csiId = id; for (size_t i = 0; i < parameters.size(); i++) { csiParams.push_back(parameters.at(i).value_or(0)); @@ -96,7 +107,19 @@ class Microsoft::Console::VirtualTerminal::TestStateMachineEngine : public IStat } } - // This will only be populated if ActionCsiDispatch is called. + IStateMachineEngine::StringHandler ActionDcsDispatch(const VTID id, const VTParameters parameters) override + { + dcsId = id; + for (size_t i = 0; i < parameters.size(); i++) + { + dcsParams.push_back(parameters.at(i).value_or(0)); + } + dcsDataString.clear(); + return [=](const auto ch) { dcsDataString += ch; return true; }; + } + + // These will only be populated if ActionCsiDispatch is called. + uint64_t csiId = 0; std::vector csiParams; // Flush function for pass-through test. @@ -107,6 +130,14 @@ class Microsoft::Console::VirtualTerminal::TestStateMachineEngine : public IStat // Printed string. std::wstring printed; + + // Executed string. + std::wstring executed; + + // These will only be populated if ActionDcsDispatch is called. + uint64_t dcsId = 0; + std::vector dcsParams; + std::wstring dcsDataString; }; class Microsoft::Console::VirtualTerminal::StateMachineTest @@ -129,6 +160,8 @@ class Microsoft::Console::VirtualTerminal::StateMachineTest TEST_METHOD(RunStorageBeforeEscape); TEST_METHOD(BulkTextPrint); TEST_METHOD(PassThroughUnhandledSplitAcrossWrites); + + TEST_METHOD(DcsDataStringsReceivedByHandler); }; void StateMachineTest::TwoStateMachinesDoNotInterfereWithEachother() @@ -246,3 +279,66 @@ void StateMachineTest::PassThroughUnhandledSplitAcrossWrites() VERIFY_ARE_EQUAL(L"\x1b]99;foo\x1b\\", engine.passedThrough); VERIFY_ARE_EQUAL(L"", engine.printed); } + +void StateMachineTest::DcsDataStringsReceivedByHandler() +{ + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"Data:terminatorType", L"{ 0, 1, 2, 3 }") + END_TEST_METHOD_PROPERTIES() + + size_t terminatorType; + VERIFY_SUCCEEDED(TestData::TryGetValue(L"terminatorType", terminatorType)); + + auto enginePtr{ std::make_unique() }; + // this dance is required because StateMachine presumes to take ownership of its engine. + auto& engine{ *enginePtr.get() }; + StateMachine machine{ std::move(enginePtr) }; + + uint64_t expectedCsiId = 0; + std::wstring expectedExecuted = L""; + + std::wstring terminatorString; + switch (terminatorType) + { + case 0: + Log::Comment(L"Data string terminated with ST"); + terminatorString = L"\033\\"; + break; + case 1: + Log::Comment(L"Data string terminated with CSI sequence"); + terminatorString = L"\033[m"; + expectedCsiId = VTID(L'm'); + break; + case 2: + Log::Comment(L"Data string terminated with CAN"); + terminatorString = L"\030"; + expectedExecuted = L"\030"; + break; + case 3: + Log::Comment(L"Data string terminated with SUB"); + terminatorString = L"\032"; + expectedExecuted = L"\032"; + break; + } + + // Output a DCS sequence terminated with the current test string + machine.ProcessString(L"\033P1;2;3|data string"); + machine.ProcessString(terminatorString); + machine.ProcessString(L"printed text"); + + // Verify the sequence ID and parameters are received. + VERIFY_ARE_EQUAL(VTID("|"), engine.dcsId); + VERIFY_ARE_EQUAL(std::vector({ 1, 2, 3 }), engine.dcsParams); + + // Verify that the data string is received (ESC terminated). + VERIFY_ARE_EQUAL(L"data string\033", engine.dcsDataString); + + // Verify the characters following the sequence are printed. + VERIFY_ARE_EQUAL(L"printed text", engine.printed); + + // Verify the CSI sequence was received (if expected). + VERIFY_ARE_EQUAL(expectedCsiId, engine.csiId); + + // Verify the control characters were executed (if expected). + VERIFY_ARE_EQUAL(expectedExecuted, engine.executed); +}