Skip to content

Commit

Permalink
Return to ground when we flush the last char (#2823)
Browse files Browse the repository at this point in the history
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.

(cherry picked from commit 53c81a0)
  • Loading branch information
zadjii-msft authored and DHowett committed Oct 17, 2019
1 parent ba62a80 commit 8b31460
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 27 deletions.
39 changes: 32 additions & 7 deletions src/terminal/parser/stateMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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();
}
}
}
Expand Down
52 changes: 52 additions & 0 deletions src/terminal/parser/ut_parser/InputEngineTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ class Microsoft::Console::VirtualTerminal::InputEngineTest
TEST_METHOD(AltBackspaceTest);
TEST_METHOD(AltCtrlDTest);
TEST_METHOD(AltIntermediateTest);
TEST_METHOD(AltBackspaceEnterTest);

friend class TestInteractDispatch;
};
Expand Down Expand Up @@ -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 changes: 0 additions & 19 deletions src/terminal/parser/ut_parser/Parser.UnitTests-common.vcxproj

This file was deleted.

15 changes: 14 additions & 1 deletion src/terminal/parser/ut_parser/Parser.UnitTests.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down

0 comments on commit 8b31460

Please sign in to comment.