Correct passthrough sequences after refactor

src/terminal/parser/stateMachine.cpp

@@ -1249,7 +1249,10 @@ void StateMachine::ProcessString(const std::wstring_view string)
 
     while (current < string.size())
     {
-        _run = string.substr(start, current - start);
+        // The run will be everything from the start INCLUDING the current one
+        // in case we process the current character and it turns into a passthrough
+        // fallback that picks up this _run inside `FlushToTerminal` above.
+        _run = string.substr(start, current - start + 1);
 
         if (_processingIndividually)
         {
@@ -1266,8 +1269,13 @@ void StateMachine::ProcessString(const std::wstring_view string)
         {
             if (_isActionableFromGround(string.at(current))) // If the current char is the start of an escape sequence, or should be executed in ground state...
             {
-                _engine->ActionPrintString(_run); // ... print all the chars leading up to it as part of the run...
-                _trace.DispatchPrintRunTrace(_run);
+                // Because the _run above is composed INCLUDING current, we must
+                // trim it off here since we just determined it's actionable
+                // and only pass through everything before it.
+                const auto allLeadingUpTo = _run.substr(0, _run.size() - 1);
+
+                _engine->ActionPrintString(allLeadingUpTo); // ... print all the chars leading up to it as part of the run...
+                _trace.DispatchPrintRunTrace(allLeadingUpTo);
                 _processingIndividually = true; // begin processing future characters individually...
                 start = current;
                 continue;
@@ -1279,6 +1287,11 @@ void StateMachine::ProcessString(const std::wstring_view string)
         }
     }
 
+    // When we leave the loop, current has been advanced to the length of the string itself
+    // (or one past the array index to the final char) so this `substr` operation doesn't +1
+    // to include the final character (unlike the one inside the top of the loop above.)
+    // NOTE: std::basic_string_view will auto-trim excessively large sizes down to the valid length
+    //       so passing something too large in the second parameter WILL NOT FAIL.
     _run = string.substr(start, current - start);
 
     // If we're at the end of the string and have remaining un-printed characters,

src/terminal/parser/ut_parser/StateMachineTest.cpp

@@ -31,9 +31,17 @@ class Microsoft::Console::VirtualTerminal::TestStateMachineEngine : public IStat
     bool ActionExecute(const wchar_t /* wch */) override { 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 { return true; };
+    bool ActionPrintString(const std::wstring_view string) override
+    {
+        printed = string;
+        return true;
+    };
 
-    bool ActionPassThroughString(const std::wstring_view /* string */) override { return true; };
+    bool ActionPassThroughString(const std::wstring_view string) override
+    {
+        passedThrough = string;
+        return true;
+    };
 
     bool ActionEscDispatch(const wchar_t /* wch */,
                            const std::basic_string_view<wchar_t> /* intermediates */) override { return true; };
@@ -58,12 +66,30 @@ class Microsoft::Console::VirtualTerminal::TestStateMachineEngine : public IStat
                            const std::basic_string_view<wchar_t> /*intermediates*/,
                            const std::basic_string_view<size_t> parameters) override
     {
-        csiParams.emplace(parameters.cbegin(), parameters.cend());
-        return true;
+        // If flush to terminal is registered for a test, then use it.
+        if (pfnFlushToTerminal)
+        {
+            pfnFlushToTerminal();
+            return true;
+        }
+        else
+        {
+            csiParams.emplace(parameters.cbegin(), parameters.cend());
+            return true;
+        }
     }
 
     // This will only be populated if ActionCsiDispatch is called.
     std::optional<std::vector<size_t>> csiParams;
+
+    // Flush function for pass-through test.
+    std::function<bool()> pfnFlushToTerminal;
+
+    // Passed through string.
+    std::wstring passedThrough;
+
+    // Printed string.
+    std::wstring printed;
 };
 
 class Microsoft::Console::VirtualTerminal::StateMachineTest
@@ -81,6 +107,10 @@ class Microsoft::Console::VirtualTerminal::StateMachineTest
     }
 
     TEST_METHOD(TwoStateMachinesDoNotInterfereWithEachother);
+
+    TEST_METHOD(PassThroughUnhandled);
+    TEST_METHOD(RunStorageBeforeEscape);
+    TEST_METHOD(BulkTextPrint);
 };
 
 void StateMachineTest::TwoStateMachinesDoNotInterfereWithEachother()
@@ -104,3 +134,51 @@ void StateMachineTest::TwoStateMachinesDoNotInterfereWithEachother()
     VERIFY_ARE_EQUAL(expectedFirstCsi, firstEngine.csiParams);
     VERIFY_ARE_EQUAL(expectedSecondCsi, secondEngine.csiParams);
 }
+
+void StateMachineTest::PassThroughUnhandled()
+{
+    auto enginePtr{ std::make_unique<TestStateMachineEngine>() };
+    // this dance is required because StateMachine presumes to take ownership of its engine.
+    auto& engine{ *enginePtr.get() };
+    StateMachine machine{ std::move(enginePtr) };
+
+    // Hook up the passthrough function.
+    engine.pfnFlushToTerminal = std::bind(&StateMachine::FlushToTerminal, &machine);
+
+    machine.ProcessString(L"\x1b[?999h 12345 Hello World");
+
+    VERIFY_ARE_EQUAL(String(L"\x1b[?999h"), String(engine.passedThrough.c_str()));
+    VERIFY_ARE_EQUAL(String(L" 12345 Hello World"), String(engine.printed.c_str()));
+}
+
+void StateMachineTest::RunStorageBeforeEscape()
+{
+    auto enginePtr{ std::make_unique<TestStateMachineEngine>() };
+    // this dance is required because StateMachine presumes to take ownership of its engine.
+    auto& engine{ *enginePtr.get() };
+    StateMachine machine{ std::move(enginePtr) };
+
+    // Hook up the passthrough function.
+    engine.pfnFlushToTerminal = std::bind(&StateMachine::FlushToTerminal, &machine);
+
+    // Print a bunch of regular text to build up the run buffer before transitioning state.
+    machine.ProcessString(L"12345 Hello World\x1b[?999h");
+
+    // Then ensure the entire buffered run was printed all at once back to us.
+    VERIFY_ARE_EQUAL(String(L"12345 Hello World"), String(engine.printed.c_str()));
+    VERIFY_ARE_EQUAL(String(L"\x1b[?999h"), String(engine.passedThrough.c_str()));
+}
+
+void StateMachineTest::BulkTextPrint()
+{
+    auto enginePtr{ std::make_unique<TestStateMachineEngine>() };
+    // this dance is required because StateMachine presumes to take ownership of its engine.
+    auto& engine{ *enginePtr.get() };
+    StateMachine machine{ std::move(enginePtr) };
+
+    // Print a bunch of regular text to build up the run buffer before transitioning state.
+    machine.ProcessString(L"12345 Hello World");
+
+    // Then ensure the entire buffered run was printed all at once back to us.
+    VERIFY_ARE_EQUAL(String(L"12345 Hello World"), String(engine.printed.c_str()));
+}