Repository: microsoft/terminal (language: cpp)

Issue:
Add support for passing DCS sequences over conpty
# Description of the new feature/enhancement

We don't currently support passing through `DCS` sequences over conpty, so they just get ignored. This hasn't been a significant issue up to now, because none of the existing `DCS` sequences could be meaningfully interpreted by the connected terminal. However, with the recent addition of the `DECCTR` sequence (for updating the color table), we now have a real use case for `DCS` passthrough.

# Proposed technical implementation details (optional)

My plan was to add a helper function in `AdaptDispatch` that would create a "passthrough" string handler that `DCS` operations could choose to return when in conpty mode. This would start by flushing the renderer to make sure there wasn't any pending frame data, and also flushing the state machine to pass through the sequence introducer and parameters that had already been parsed. From then on, it would be the job of the returned string handler to forward the remaining sequence data via the state machine's `ActionPassThroughString` method.

By default I think we should still be ignoring `DCS` sequences that we don't understand. But if we know of a sequence that can meaningfully be interpreted by the terminal on the other end of the conpty pipe (like `DECCTR`), then we can choose to forward that using this passthrough handler.


Discussion / hints:

Fix patch:
diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
index 69f19e67eda..a497a826597 100644
--- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
+++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
@@ -178,6 +178,8 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final
 
     TEST_METHOD(PassthroughCursorShapeImmediately);
 
+    TEST_METHOD(PassthroughDECCTR);
+
     TEST_METHOD(TestWrappingALongString);
     TEST_METHOD(TestAdvancedWrapping);
     TEST_METHOD(TestExactWrappingWithoutSpaces);
@@ -1206,6 +1208,42 @@ void ConptyRoundtripTests::PassthroughHardReset()
     }
 }
 
+void ConptyRoundtripTests::PassthroughDECCTR()
+{
+    Log::Comment(L"Update the color table with DECCTR. This should immediately be flushed to the Terminal.");
+
+    auto& g = ServiceLocator::LocateGlobals();
+    auto& gci = g.getConsoleInformation();
+    auto& si = gci.GetActiveOutputBuffer();
+    auto& hostSm = si.GetStateMachine();
+    auto& termRenderSettings = term->GetRenderSettings();
+
+    _flushFirstFrame();
+
+    _logConpty = true;
+
+    // We're going to update color table entries 101 through 103, so we start by
+    // initializing those entries to black in the Terminal render settings.
+    termRenderSettings.SetColorTableEntry(101, RGB(0, 0, 0));
+    termRenderSettings.SetColorTableEntry(102, RGB(0, 0, 0));
+    termRenderSettings.SetColorTableEntry(103, RGB(0, 0, 0));
+
+    // DECCTR is expected to arrive in two parts. The control sequence comes
+    // first, and the string content follows in the second packet.
+    expectedOutput.push_back("\x1bP2$p");
+    expectedOutput.push_back("101;2;100;0;0/102;2;0;100;0/103;2;0;0;100\x1b\\");
+
+    // Send the control sequence to the host.
+    hostSm.ProcessString(L"\x1bP2$p101;2;100;0;0/102;2;0;100;0/103;2;0;0;100\x1b\\");
+
+    // Verify that the color table entries have been updated in the Terminal.
+    VERIFY_ARE_EQUAL(RGB(255, 0, 0), termRenderSettings.GetColorTableEntry(101));
+    VERIFY_ARE_EQUAL(RGB(0, 255, 0), termRenderSettings.GetColorTableEntry(102));
+    VERIFY_ARE_EQUAL(RGB(0, 0, 255), termRenderSettings.GetColorTableEntry(103));
+
+    termRenderSettings.ResetColorTable();
+}
+
 void ConptyRoundtripTests::OutputWrappedLinesAtTopOfBuffer()
 {
     Log::Comment(
diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp
index 0531b4aef66..a61b5deffc7 100644
--- a/src/terminal/adapter/adaptDispatch.cpp
+++ b/src/terminal/adapter/adaptDispatch.cpp
@@ -2578,6 +2578,13 @@ ITermDispatch::StringHandler AdaptDispatch::RestoreTerminalState(const DispatchT
 // - a function to parse the report data.
 ITermDispatch::StringHandler AdaptDispatch::_RestoreColorTable()
 {
+    // If we're a conpty, we create a passthrough string handler to forward the
+    // color report to the connected terminal.
+    if (_api.IsConsolePty())
+    {
+        return _CreatePassthroughHandler();
+    }
+
     return [this, parameter = VTInt{}, parameters = std::vector<VTParameter>{}](const auto ch) mutable {
         if (ch >= L'0' && ch <= L'9')
         {
@@ -2790,3 +2797,47 @@ bool AdaptDispatch::PlaySounds(const VTParameters parameters)
         return true;
     });
 }
+
+// Routine Description:
+// - Helper method to create a string handler that can be used to pass through
+//   DCS sequences when in conpty mode.
+// Arguments:
+// - <none>
+// Return value:
+// - a function to receive the data or nullptr if the initial flush fails
+ITermDispatch::StringHandler AdaptDispatch::_CreatePassthroughHandler()
+{
+    // Before we pass through any more data, we need to flush the current frame
+    // first, otherwise it can end up arriving out of sync.
+    _renderer.TriggerFlush(false);
+    // Then we need to flush the sequence introducer and parameters that have
+    // already been parsed by the state machine.
+    auto& stateMachine = _api.GetStateMachine();
+    if (stateMachine.FlushToTerminal())
+    {
+        // And finally we create a StringHandler to receive the rest of the
+        // sequence data, and pass it through to the connected terminal.
+        auto& engine = stateMachine.Engine();
+        return [&, buffer = std::wstring{}](const auto ch) mutable {
+            // To make things more efficient, we buffer the string data before
+            // passing it through, only flushing if the buffer gets too large,
+            // or we're dealing with the last character in the current output
+            // fragment, or we've reached the end of the string.
+            const auto endOfString = ch == AsciiChars::ESC;
+            buffer += ch;
+            if (buffer.length() >= 4096 || stateMachine.IsProcessingLastCharacter() || endOfString)
+            {
+                // The end of the string is signaled with an escape, but for it
+                // to be a valid string terminator we need to add a backslash.
+                if (endOfString)
+                {
+                    buffer += L'\\';
+                }
+                engine.ActionPassThroughString(buffer);
+                buffer.clear();
+            }
+            return !endOfString;
+        };
+    }
+    return nullptr;
+}
diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp
index 41e91885e25..44b4f87f279 100644
--- a/src/terminal/adapter/adaptDispatch.hpp
+++ b/src/terminal/adapter/adaptDispatch.hpp
@@ -205,6 +205,8 @@ namespace Microsoft::Console::VirtualTerminal
         void _ReportSGRSetting() const;
         void _ReportDECSTBMSetting();
 
+        StringHandler _CreatePassthroughHandler();
+
         std::vector<bool> _tabStopColumns;
         bool _initDefaultTabStops = true;
 
diff --git a/src/terminal/parser/stateMachine.cpp b/src/terminal/parser/stateMachine.cpp
index 4fe3b58cbf3..34fab677423 100644
--- a/src/terminal/parser/stateMachine.cpp
+++ b/src/terminal/parser/stateMachine.cpp
@@ -919,6 +919,7 @@ void StateMachine::_EnterDcsParam() noexcept
 void StateMachine::_EnterDcsIgnore() noexcept
 {
     _state = VTStates::DcsIgnore;
+    _cachedSequence.reset();
     _trace.TraceStateChange(L"DcsIgnore");
 }
 
@@ -950,6 +951,7 @@ void StateMachine::_EnterDcsIntermediate() noexcept
 void StateMachine::_EnterDcsPassThrough() noexcept
 {
     _state = VTStates::DcsPassThrough;
+    _cachedSequence.reset();
     _trace.TraceStateChange(L"DcsPassThrough");
 }
 
@@ -966,6 +968,7 @@ void StateMachine::_EnterDcsPassThrough() noexcept
 void StateMachine::_EnterSosPmApcString() noexcept
 {
     _state = VTStates::SosPmApcString;
+    _cachedSequence.reset();
     _trace.TraceStateChange(L"SosPmApcString");
 }
 
@@ -1843,6 +1846,8 @@ void StateMachine::ProcessString(const std::wstring_view string)
 
         if (_processingIndividually)
         {
+            // Note whether we're dealing with the last character in the buffer.
+            _processingLastCharacter = (current + 1 >= string.size());
             // If we're processing characters individually, send it to the state machine.
             ProcessCharacter(til::at(string, current));
             ++current;
@@ -1921,6 +1926,7 @@ void StateMachine::ProcessString(const std::wstring_view string)
         {
             // Reset our state, and put all but the last char in again.
             ResetState();
+            _processingLastCharacter = false;
             // Chars to flush are [pwchSequenceStart, pwchCurr)
             auto wchIter = run.cbegin();
             while (wchIter < run.cend() - 1)
@@ -1929,6 +1935,7 @@ void StateMachine::ProcessString(const std::wstring_view string)
                 wchIter++;
             }
             // Manually execute the last char [pwchCurr]
+            _processingLastCharacter = true;
             switch (_state)
             {
             case VTStates::Ground:
@@ -1958,11 +1965,13 @@ void StateMachine::ProcessString(const std::wstring_view string)
             // after dispatching the characters
             _EnterGround();
         }
-        else
+        else if (_state != VTStates::SosPmApcString && _state != VTStates::DcsPassThrough && _state != VTStates::DcsIgnore)
         {
             // If the engine doesn't require flushing at the end of the string, we
             // want to cache the partial sequence in case we have to flush the whole
-            // thing to the terminal later.
+            // thing to the terminal later. There is no need to do this if we've
+            // reached one of the string processing states, though, since that data
+            // will be dealt with as soon as it is received.
             if (!_cachedSequence)
             {
                 _cachedSequence.emplace(std::wstring{});
@@ -1974,6 +1983,19 @@ void StateMachine::ProcessString(const std::wstring_view string)
     }
 }
 
+// Routine Description:
+// - Determines whether the character being processed is the last in the
+//   current output fragment, or there are more still to come. Other parts
+//   of the framework can use this information to work more efficiently.
+// Arguments:
+// - <none>
+// Return Value:
+// - True if we're processing the last character. False if not.
+bool StateMachine::IsProcessingLastCharacter() const noexcept
+{
+    return _processingLastCharacter;
+}
+
 // Routine Description:
 // - Wherever the state machine is, whatever it's going, go back to ground.
 //     This is used by conhost to "jiggle the handle" - when VT support is
diff --git a/src/terminal/parser/stateMachine.hpp b/src/terminal/parser/stateMachine.hpp
index 193734e27e2..2e3b741d6e6 100644
--- a/src/terminal/parser/stateMachine.hpp
+++ b/src/terminal/parser/stateMachine.hpp
@@ -58,6 +58,7 @@ namespace Microsoft::Console::VirtualTerminal
 
         void ProcessCharacter(const wchar_t wch);
         void ProcessString(const std::wstring_view string);
+        bool IsProcessingLastCharacter() const noexcept;
 
         void ResetState() noexcept;
 
@@ -199,5 +200,6 @@ namespace Microsoft::Console::VirtualTerminal
         // This is tracked per state machine instance so that separate calls to Process*
         //   can start and finish a sequence.
         bool _processingIndividually;
+        bool _processingLastCharacter;
     };
 }


Test patch: