Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return to ground when we flush the last char #2823

Merged
merged 4 commits into from
Oct 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
2 changes: 1 addition & 1 deletion tools/bcz.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -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%
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved

echo %_BUILD_CMDLINE%
echo Starting build...
Expand Down