maxmustermann
/
terminal
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Return to ground when we flush the last char (#2823) 

## Summary of the Pull Request
The InputStateMachineEngine was incorrectly not returning to the ground state after flushing the last sequence. That means that something like alt+backspace would leave us in the Escape state, not the ground state. This makes sure we return to ground.

Additionally removes the "Parser.UnitTests-common.vcxproj" file, which was originally used for a theoretical time when we only open-sourced the parser. It's unnecessary now, and we can get rid of it.

Also includes a small patch to bcz.cmd, to make sure bx works with projects with a space in their name.

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes #2746
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated


<hr>

* Return to ground when we flush the last char

  The InputStateMachineEngine was incorrectly not returning to the ground state
  after flushing the last sequence. That means that something like alt+backspace
  would leave us in the Escape state, not the ground state. This makes sure we
  return to ground.

  Fixes #2746.

  Additionally removes the "Parser.UnitTests-common.vcxproj" file, which was
  originally used for a theoretical time when we only open-sourced the parser.
  It's unnecessary now, and we can get rid of it.

  Also includes a small patch to bcz.cmd, to make sure bx works with projects
  with a space in their name.

* Update src/terminal/parser/stateMachine.cpp

Co-Authored-By: Dustin L. Howett (MSFT) <duhowett@microsoft.com>

* add the comment @miniksa wanted
This commit is contained in:
Mike Griese 2019-10-04 10:47:39 -05:00 committed by GitHub
parent 505ceaccf6
commit 53c81a08f8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 99 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
src/terminal/parser/stateMachine.cpp
View File

@ -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();
}
}
}

52
src/terminal/parser/ut_parser/InputEngineTest.cpp
View File

@ -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);
}

19
src/terminal/parser/ut_parser/Parser.UnitTests-common.vcxproj
View File

@ -1,19 +0,0 @@
<?xml version="1.0" encoding="utf-8"?>
<Project DefaultTargets="Build" ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<ItemGroup>
<ClCompile Include="OutputEngineTest.cpp" />
<ClCompile Include="..\precomp.cpp">
<PrecompiledHeader>Create</PrecompiledHeader>
</ClCompile>
</ItemGroup>
<ItemGroup>
<ClInclude Include="..\precomp.h" />
</ItemGroup>
<ItemDefinitionGroup>
<ClCompile>
<AdditionalIncludeDirectories>..;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
</ClCompile>
</ItemDefinitionGroup>
</Project>

15
src/terminal/parser/ut_parser/Parser.UnitTests.vcxproj
View File

@ -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>

2
tools/bcz.cmd
View File

@ -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...