Return to ground when we flush the last char

src/terminal/parser/stateMachine.cpp

@@ -1399,6 +1399,25 @@ void StateMachine::ProcessString(const wchar_t* const rgwch, const size_t cch)
     }
     else if (s_fProcessIndividually)
     {
+        // One of the "weird things" in VT input is the case of something like
+        // <kbd>alt+[</kbd>. In VT, that's encoded as `\x1b[`. However, that's
+        // also the start of a CSI, and could be the start of a longer sequence,
+        // there's no way to know for sure. For an <kbd>alt+[</kbd> keypress,
+        // the parser originally would just sit in the `CsiEntry` state after
+        // processing it, which would pollute the following keypress (e.g.
+        // <kbd>alt+[</kbd>, <kbd>A</kbd> would be processed like `\x1b[A`,
+        // which is _wrong_).
+        //
+        // Fortunately, for VT input, each keystroke comes in as an individual
+        // write operation. So, if at the end of processing a string for the
+        // InputEngine, we find that we're not in the Ground state, that implies
+        // that we've processed some input, but not dispatched it yet. This
+        // block at the end of `ProcessString` will then re-process the
+        // undispatched string, but it will ensure that it dispatches on the
+        // last character of the string. For our previous `\x1b[` scenario, that
+        // means we'll make sure to call `_ActionEscDispatch('[')`., which will
+        // properly decode the string as <kbd>alt+[</kbd>.
+
         if (_pEngine->FlushAtEndOfString())
         {
             // Reset our state, and put all but the last char in again.
@@ -1413,25 +1432,31 @@ void StateMachine::ProcessString(const wchar_t* const rgwch, const size_t cch)
             switch (_state)
             {
             case VTStates::Ground:
-                return _ActionExecute(*pwch);
+                _ActionExecute(*pwch);
+                break;
             case VTStates::Escape:
             case VTStates::EscapeIntermediate:
-                return _ActionEscDispatch(*pwch);
+                _ActionEscDispatch(*pwch);
+                break;
             case VTStates::CsiEntry:
             case VTStates::CsiIntermediate:
             case VTStates::CsiIgnore:
             case VTStates::CsiParam:
-                return _ActionCsiDispatch(*pwch);
+                _ActionCsiDispatch(*pwch);
+                break;
             case VTStates::OscParam:
             case VTStates::OscString:
             case VTStates::OscTermination:
-                return _ActionOscDispatch(*pwch);
+                _ActionOscDispatch(*pwch);
+                break;
             case VTStates::Ss3Entry:
             case VTStates::Ss3Param:
-                return _ActionSs3Dispatch(*pwch);
-            default:
-                return;
+                _ActionSs3Dispatch(*pwch);
+                break;
             }
+            // microsoft/terminal#2746: Make sure to return to the ground state
+            // after dispatching the characters
+            _EnterGround();
         }
     }
 }

src/terminal/parser/ut_parser/InputEngineTest.cpp

@@ -230,6 +230,7 @@ class Microsoft::Console::VirtualTerminal::InputEngineTest
     TEST_METHOD(AltBackspaceTest);
     TEST_METHOD(AltCtrlDTest);
     TEST_METHOD(AltIntermediateTest);
+    TEST_METHOD(AltBackspaceEnterTest);
 
     friend class TestInteractDispatch;
 };
@@ -826,3 +827,54 @@ void InputEngineTest::AltIntermediateTest()
     Log::Comment(NoThrowString().Format(L"Processing \"\\x05\""));
     stateMachine->ProcessString(seq);
 }
+
+void InputEngineTest::AltBackspaceEnterTest()
+{
+    // Created as a test for microsoft/terminal#2746. See that issue for mode
+    // details. We're going to send an Alt+Backspace to conpty, followed by an
+    // enter. The enter should be processed as just a single VK_ENTER, not a
+    // alt+enter.
+
+    TestState testState;
+    auto pfn = std::bind(&TestState::TestInputCallback, &testState, std::placeholders::_1);
+
+    auto inputEngine = std::make_unique<InputStateMachineEngine>(new TestInteractDispatch(pfn, &testState));
+    auto _stateMachine = std::make_unique<StateMachine>(inputEngine.release());
+    VERIFY_IS_NOT_NULL(_stateMachine);
+    testState._stateMachine = _stateMachine.get();
+
+    INPUT_RECORD inputRec;
+
+    inputRec.EventType = KEY_EVENT;
+    inputRec.Event.KeyEvent.bKeyDown = TRUE;
+    inputRec.Event.KeyEvent.dwControlKeyState = LEFT_ALT_PRESSED;
+    inputRec.Event.KeyEvent.wRepeatCount = 1;
+    inputRec.Event.KeyEvent.wVirtualKeyCode = VK_BACK;
+    inputRec.Event.KeyEvent.wVirtualScanCode = static_cast<WORD>(MapVirtualKeyW(VK_BACK, MAPVK_VK_TO_VSC));
+    inputRec.Event.KeyEvent.uChar.UnicodeChar = L'\x08';
+
+    // First, expect a alt+backspace.
+    testState.vExpectedInput.push_back(inputRec);
+
+    std::wstring seq = L"\x1b\x7f";
+    Log::Comment(NoThrowString().Format(L"Processing \"\\x1b\\x7f\""));
+    _stateMachine->ProcessString(seq);
+
+    // Ensure the state machine has correctly returned to the ground state
+    VERIFY_ARE_EQUAL(StateMachine::VTStates::Ground, _stateMachine->_state);
+
+    inputRec.Event.KeyEvent.wVirtualKeyCode = VK_RETURN;
+    inputRec.Event.KeyEvent.dwControlKeyState = 0;
+    inputRec.Event.KeyEvent.wVirtualScanCode = static_cast<WORD>(MapVirtualKeyW(VK_RETURN, MAPVK_VK_TO_VSC));
+    inputRec.Event.KeyEvent.uChar.UnicodeChar = L'\x0d'; //maybe \xa
+
+    // Then, expect a enter
+    testState.vExpectedInput.push_back(inputRec);
+
+    seq = L"\x0d";
+    Log::Comment(NoThrowString().Format(L"Processing \"\\x0d\""));
+    _stateMachine->ProcessString(seq);
+
+    // Ensure the state machine has correctly returned to the ground state
+    VERIFY_ARE_EQUAL(StateMachine::VTStates::Ground, _stateMachine->_state);
+}

src/terminal/parser/ut_parser/Parser.UnitTests.vcxproj

@@ -2,14 +2,27 @@
 <Project DefaultTargets="Build" ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
   <Import Project="$(SolutionDir)src\common.build.pre.props" />
 
-  <Import Project="Parser.UnitTests-common.vcxproj" />
+
+  <ItemGroup>
+    <ClInclude Include="..\precomp.h" />
+  </ItemGroup>
 
   <!-- Only add closed-source files, dependencies to this file.
       Any open-source files can go in Parser.UnitTests-common.vcxproj -->
   <ItemGroup>
     <ClCompile Include="InputEngineTest.cpp" />
+    <ClCompile Include="OutputEngineTest.cpp" />
+    <ClCompile Include="..\precomp.cpp">
+      <PrecompiledHeader>Create</PrecompiledHeader>
+    </ClCompile>
   </ItemGroup>
 
+  <ItemDefinitionGroup>
+    <ClCompile>
+      <AdditionalIncludeDirectories>..;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+    </ClCompile>
+  </ItemDefinitionGroup>
+
   <ItemGroup>
     <ProjectReference Include="..\..\..\types\lib\types.vcxproj">
       <Project>{18d09a24-8240-42d6-8cb6-236eee820263}</Project>

tools/bcz.cmd

@@ -68,7 +68,7 @@ if "%_EXCLUSIVE%" == "1" (
 echo Performing nuget restore...
 nuget.exe restore %OPENCON%\OpenConsole.sln
 
-set _BUILD_CMDLINE="%MSBUILD%" %OPENCON%\OpenConsole.sln /t:%_MSBUILD_TARGET% /m /p:Configuration=%_LAST_BUILD_CONF% /p:Platform=%ARCH% %_APPX_ARGS%
+set _BUILD_CMDLINE="%MSBUILD%" %OPENCON%\OpenConsole.sln /t:"%_MSBUILD_TARGET%" /m /p:Configuration=%_LAST_BUILD_CONF% /p:Platform=%ARCH% %_APPX_ARGS%
 
 echo %_BUILD_CMDLINE%
 echo Starting build...