From cbb87b98b75e5cad68dd9e575b6a4e67c1e45841 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Tue, 21 Jan 2020 22:39:15 +0000 Subject: [PATCH] Add support for the HPR and VPR escape sequences (#4297) ## Summary of the Pull Request This PR adds support for the `HPR` and `VPR` escape sequences from the VT510 terminal. `HPR` moves the cursor position forward by a given number of columns, and `VPR` moves the cursor position downward by a given number of rows. They're similar in function to the `CUF` and `CUD` escape sequences, except that they're not constrained by the scrolling margins. ## References #3628 provided the new `_CursorMovePosition` method that made these operations possible ## PR Checklist * [x] Closes #3428 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [x] Tests added/passed * [ ] Requires documentation to be updated * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx ## Detailed Description of the Pull Request / Additional comments Most of the implementation is in the new `_CursorMovePosition` method that was created in PR #3628, so all we're really doing here is hooking up the escape sequences to call that method with the appropriate parameters. ## Validation Steps Performed I've extended the existing state machine tests for CSI cursor movement to confirm that the `HPR` and `VPR` sequences are dispatched correctly, and also added screen buffer tests to make sure the movement is clamped by the screen boundaries and not the scrolling margins (we don't yet support horizontal margins, but the test is at least in place for when we do eventually add that support). I've also checked the `HPR` and `VPR` tests in Vttest (under _Test non-VT100 / ISO-6429 cursor-movement_) and confirmed that they are now working as expected. --- src/host/ut_host/ScreenBufferTests.cpp | 75 +++++++++++++++++++ src/terminal/adapter/ITermDispatch.hpp | 2 + src/terminal/adapter/adaptDispatch.cpp | 24 ++++++ src/terminal/adapter/adaptDispatch.hpp | 2 + src/terminal/adapter/termDispatch.hpp | 2 + .../parser/OutputStateMachineEngine.cpp | 10 +++ .../parser/OutputStateMachineEngine.hpp | 2 + src/terminal/parser/telemetry.cpp | 2 + src/terminal/parser/telemetry.hpp | 2 + .../parser/ut_parser/OutputEngineTest.cpp | 26 +++++++ 10 files changed, 147 insertions(+) diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 1c998d4e9..91de995a6 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -202,6 +202,7 @@ class ScreenBufferTests TEST_METHOD(CursorUpDownExactlyAtMargins); TEST_METHOD(CursorNextPreviousLine); + TEST_METHOD(CursorPositionRelative); TEST_METHOD(CursorSaveRestore); @@ -5450,6 +5451,80 @@ void ScreenBufferTests::CursorNextPreviousLine() VERIFY_ARE_EQUAL(COORD({ 0, 2 }), cursor.GetPosition()); } +void ScreenBufferTests::CursorPositionRelative() +{ + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& stateMachine = si.GetStateMachine(); + auto& cursor = si.GetTextBuffer().GetCursor(); + + Log::Comment(L"Make sure the viewport is at 0,0"); + VERIFY_SUCCEEDED(si.SetViewportOrigin(true, COORD({ 0, 0 }), true)); + + Log::Comment(L"HPR without margins"); + // Starting from column 20 of line 10. + cursor.SetPosition(COORD{ 20, 10 }); + // Move forward 5 columns (HPR). + stateMachine.ProcessString(L"\x1b[5a"); + // We should end up in column 25. + VERIFY_ARE_EQUAL(COORD({ 25, 10 }), cursor.GetPosition()); + + Log::Comment(L"VPR without margins"); + // Starting from column 20 of line 10. + cursor.SetPosition(COORD{ 20, 10 }); + // Move down 5 lines (VPR). + stateMachine.ProcessString(L"\x1b[5e"); + // We should end up on line 15. + VERIFY_ARE_EQUAL(COORD({ 20, 15 }), cursor.GetPosition()); + + // Enable DECLRMM margin mode (future proofing for when we support it) + stateMachine.ProcessString(L"\x1b[?69h"); + // Set horizontal margins to 18:22 (19:23 in VT coordinates). + stateMachine.ProcessString(L"\x1b[19;23s"); + // Set vertical margins to 8:12 (9:13 in VT coordinates). + stateMachine.ProcessString(L"\x1b[9;13r"); + // Make sure we clear the margins on exit so they can't break other tests. + auto clearMargins = wil::scope_exit([&] { + stateMachine.ProcessString(L"\x1b[r"); + stateMachine.ProcessString(L"\x1b[s"); + stateMachine.ProcessString(L"\x1b[?69l"); + }); + + Log::Comment(L"HPR inside margins"); + // Starting from column 20 of line 10. + cursor.SetPosition(COORD{ 20, 10 }); + // Move forward 5 columns (HPR). + stateMachine.ProcessString(L"\x1b[5a"); + // We should end up in column 25 (outside the right margin). + VERIFY_ARE_EQUAL(COORD({ 25, 10 }), cursor.GetPosition()); + + Log::Comment(L"VPR inside margins"); + // Starting from column 20 of line 10. + cursor.SetPosition(COORD{ 20, 10 }); + // Move down 5 lines (VPR). + stateMachine.ProcessString(L"\x1b[5e"); + // We should end up on line 15 (outside the bottom margin). + VERIFY_ARE_EQUAL(COORD({ 20, 15 }), cursor.GetPosition()); + + Log::Comment(L"HPR to end of line"); + // Starting from column 20 of line 10. + cursor.SetPosition(COORD{ 20, 10 }); + // Move forward 9999 columns (HPR). + stateMachine.ProcessString(L"\x1b[9999a"); + // We should end up in the rightmost column. + const auto screenWidth = si.GetBufferSize().Width(); + VERIFY_ARE_EQUAL(COORD({ screenWidth - 1, 10 }), cursor.GetPosition()); + + Log::Comment(L"VPR to bottom of screen"); + // Starting from column 20 of line 10. + cursor.SetPosition(COORD{ 20, 10 }); + // Move down 9999 lines (VPR). + stateMachine.ProcessString(L"\x1b[9999e"); + // We should end up on the last line. + const auto screenHeight = si.GetViewport().Height(); + VERIFY_ARE_EQUAL(COORD({ 20, screenHeight - 1 }), cursor.GetPosition()); +} + void ScreenBufferTests::CursorSaveRestore() { auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); diff --git a/src/terminal/adapter/ITermDispatch.hpp b/src/terminal/adapter/ITermDispatch.hpp index a560f7f06..e60fab485 100644 --- a/src/terminal/adapter/ITermDispatch.hpp +++ b/src/terminal/adapter/ITermDispatch.hpp @@ -38,6 +38,8 @@ public: virtual bool CursorPrevLine(const size_t distance) = 0; // CPL virtual bool CursorHorizontalPositionAbsolute(const size_t column) = 0; // CHA virtual bool VerticalLinePositionAbsolute(const size_t line) = 0; // VPA + virtual bool HorizontalPositionRelative(const size_t distance) = 0; // HPR + virtual bool VerticalPositionRelative(const size_t distance) = 0; // VPR virtual bool CursorPosition(const size_t line, const size_t column) = 0; // CUP virtual bool CursorSaveState() = 0; // DECSC virtual bool CursorRestoreState() = 0; // DECRC diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index fa991e31e..79ac8c086 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -268,6 +268,30 @@ bool AdaptDispatch::VerticalLinePositionAbsolute(const size_t line) return _CursorMovePosition(Offset::Absolute(line), Offset::Unchanged(), false); } +// Routine Description: +// - HPR - Handles cursor forward movement by given distance +// - Unlike CUF, this is not constrained by margin settings. +// Arguments: +// - distance - Distance to move +// Return Value: +// - True if handled successfully. False otherwise. +bool AdaptDispatch::HorizontalPositionRelative(const size_t distance) +{ + return _CursorMovePosition(Offset::Unchanged(), Offset::Forward(distance), false); +} + +// Routine Description: +// - VPR - Handles cursor downward movement by given distance +// - Unlike CUD, this is not constrained by margin settings. +// Arguments: +// - distance - Distance to move +// Return Value: +// - True if handled successfully. False otherwise. +bool AdaptDispatch::VerticalPositionRelative(const size_t distance) +{ + return _CursorMovePosition(Offset::Forward(distance), Offset::Unchanged(), false); +} + // Routine Description: // - CUP - Moves the cursor to an exact X/Column and Y/Row/Line coordinate position. // Arguments: diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp index 2d7c633b1..f42347556 100644 --- a/src/terminal/adapter/adaptDispatch.hpp +++ b/src/terminal/adapter/adaptDispatch.hpp @@ -44,6 +44,8 @@ namespace Microsoft::Console::VirtualTerminal bool CursorPrevLine(const size_t distance) override; // CPL bool CursorHorizontalPositionAbsolute(const size_t column) override; // CHA bool VerticalLinePositionAbsolute(const size_t line) override; // VPA + bool HorizontalPositionRelative(const size_t distance) override; // HPR + bool VerticalPositionRelative(const size_t distance) override; // VPR bool CursorPosition(const size_t line, const size_t column) override; // CUP bool CursorSaveState() override; // DECSC bool CursorRestoreState() override; // DECRC diff --git a/src/terminal/adapter/termDispatch.hpp b/src/terminal/adapter/termDispatch.hpp index 73914b021..45c3d5985 100644 --- a/src/terminal/adapter/termDispatch.hpp +++ b/src/terminal/adapter/termDispatch.hpp @@ -32,6 +32,8 @@ public: bool CursorPrevLine(const size_t /*distance*/) noexcept override { return false; } // CPL bool CursorHorizontalPositionAbsolute(const size_t /*column*/) noexcept override { return false; } // CHA bool VerticalLinePositionAbsolute(const size_t /*line*/) noexcept override { return false; } // VPA + bool HorizontalPositionRelative(const size_t /*distance*/) noexcept override { return false; } // HPR + bool VerticalPositionRelative(const size_t /*distance*/) noexcept override { return false; } // VPR bool CursorPosition(const size_t /*line*/, const size_t /*column*/) noexcept override { return false; } // CUP bool CursorSaveState() noexcept override { return false; } // DECSC bool CursorRestoreState() noexcept override { return false; } // DECRC diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp index 7783e177f..f3b5c2237 100644 --- a/src/terminal/parser/OutputStateMachineEngine.cpp +++ b/src/terminal/parser/OutputStateMachineEngine.cpp @@ -323,6 +323,8 @@ bool OutputStateMachineEngine::ActionCsiDispatch(const wchar_t wch, case VTActionCodes::CHA_CursorHorizontalAbsolute: case VTActionCodes::HPA_HorizontalPositionAbsolute: case VTActionCodes::VPA_VerticalLinePositionAbsolute: + case VTActionCodes::HPR_HorizontalPositionRelative: + case VTActionCodes::VPR_VerticalPositionRelative: case VTActionCodes::ICH_InsertCharacter: case VTActionCodes::DCH_DeleteCharacter: case VTActionCodes::ECH_EraseCharacters: @@ -417,6 +419,14 @@ bool OutputStateMachineEngine::ActionCsiDispatch(const wchar_t wch, success = _dispatch->VerticalLinePositionAbsolute(distance); TermTelemetry::Instance().Log(TermTelemetry::Codes::VPA); break; + case VTActionCodes::HPR_HorizontalPositionRelative: + success = _dispatch->HorizontalPositionRelative(distance); + TermTelemetry::Instance().Log(TermTelemetry::Codes::HPR); + break; + case VTActionCodes::VPR_VerticalPositionRelative: + success = _dispatch->VerticalPositionRelative(distance); + TermTelemetry::Instance().Log(TermTelemetry::Codes::VPR); + break; case VTActionCodes::CUP_CursorPosition: case VTActionCodes::HVP_HorizontalVerticalPosition: success = _dispatch->CursorPosition(line, column); diff --git a/src/terminal/parser/OutputStateMachineEngine.hpp b/src/terminal/parser/OutputStateMachineEngine.hpp index 0c6b5c02f..8739fc1dd 100644 --- a/src/terminal/parser/OutputStateMachineEngine.hpp +++ b/src/terminal/parser/OutputStateMachineEngine.hpp @@ -105,6 +105,8 @@ namespace Microsoft::Console::VirtualTerminal DL_DeleteLine = L'M', // Yes, this is the same as RI, however, RI is not preceeded by a CSI, and DL is. HPA_HorizontalPositionAbsolute = L'`', VPA_VerticalLinePositionAbsolute = L'd', + HPR_HorizontalPositionRelative = L'a', + VPR_VerticalPositionRelative = L'e', DECSTBM_SetScrollingRegion = L'r', NEL_NextLine = L'E', // Not a CSI, so doesn't overlap with CNL IND_Index = L'D', // Not a CSI, so doesn't overlap with CUB diff --git a/src/terminal/parser/telemetry.cpp b/src/terminal/parser/telemetry.cpp index 2343c38c6..068a9b90d 100644 --- a/src/terminal/parser/telemetry.cpp +++ b/src/terminal/parser/telemetry.cpp @@ -226,6 +226,8 @@ void TermTelemetry::WriteFinalTraceLog() const TraceLoggingUInt32(_uiTimesUsed[DSR], "DSR"), TraceLoggingUInt32(_uiTimesUsed[DA], "DA"), TraceLoggingUInt32(_uiTimesUsed[VPA], "VPA"), + TraceLoggingUInt32(_uiTimesUsed[HPR], "HPR"), + TraceLoggingUInt32(_uiTimesUsed[VPR], "VPR"), TraceLoggingUInt32(_uiTimesUsed[ICH], "ICH"), TraceLoggingUInt32(_uiTimesUsed[DCH], "DCH"), TraceLoggingUInt32(_uiTimesUsed[IL], "IL"), diff --git a/src/terminal/parser/telemetry.hpp b/src/terminal/parser/telemetry.hpp index d03441e97..c6d5970f6 100644 --- a/src/terminal/parser/telemetry.hpp +++ b/src/terminal/parser/telemetry.hpp @@ -53,6 +53,8 @@ namespace Microsoft::Console::VirtualTerminal DSR, DA, VPA, + HPR, + VPR, ICH, DCH, SU, diff --git a/src/terminal/parser/ut_parser/OutputEngineTest.cpp b/src/terminal/parser/ut_parser/OutputEngineTest.cpp index 8d570bc01..18a4a1f3e 100644 --- a/src/terminal/parser/ut_parser/OutputEngineTest.cpp +++ b/src/terminal/parser/ut_parser/OutputEngineTest.cpp @@ -660,6 +660,8 @@ public: _cursorPreviousLine{ false }, _cursorHorizontalPositionAbsolute{ false }, _verticalLinePositionAbsolute{ false }, + _horizontalPositionRelative{ false }, + _verticalPositionRelative{ false }, _cursorPosition{ false }, _cursorSave{ false }, _cursorLoad{ false }, @@ -751,6 +753,20 @@ public: return true; } + bool HorizontalPositionRelative(_In_ size_t const uiDistance) noexcept override + { + _horizontalPositionRelative = true; + _cursorDistance = uiDistance; + return true; + } + + bool VerticalPositionRelative(_In_ size_t const uiDistance) noexcept override + { + _verticalPositionRelative = true; + _cursorDistance = uiDistance; + return true; + } + bool CursorPosition(_In_ size_t const uiLine, _In_ size_t const uiColumn) noexcept override { _cursorPosition = true; @@ -965,6 +981,8 @@ public: bool _cursorPreviousLine; bool _cursorHorizontalPositionAbsolute; bool _verticalLinePositionAbsolute; + bool _horizontalPositionRelative; + bool _verticalPositionRelative; bool _cursorPosition; bool _cursorSave; bool _cursorLoad; @@ -1096,6 +1114,10 @@ class StateMachineExternalTest final pDispatch->ClearState(); TestCsiCursorMovement(L'd', uiDistance, true, &pDispatch->_verticalLinePositionAbsolute, mach, *pDispatch); pDispatch->ClearState(); + TestCsiCursorMovement(L'a', uiDistance, true, &pDispatch->_horizontalPositionRelative, mach, *pDispatch); + pDispatch->ClearState(); + TestCsiCursorMovement(L'e', uiDistance, true, &pDispatch->_verticalPositionRelative, mach, *pDispatch); + pDispatch->ClearState(); TestCsiCursorMovement(L'@', uiDistance, true, &pDispatch->_insertCharacter, mach, *pDispatch); pDispatch->ClearState(); TestCsiCursorMovement(L'P', uiDistance, true, &pDispatch->_deleteCharacter, mach, *pDispatch); @@ -1127,6 +1149,10 @@ class StateMachineExternalTest final pDispatch->ClearState(); TestCsiCursorMovement(L'd', uiDistance, false, &pDispatch->_verticalLinePositionAbsolute, mach, *pDispatch); pDispatch->ClearState(); + TestCsiCursorMovement(L'a', uiDistance, false, &pDispatch->_horizontalPositionRelative, mach, *pDispatch); + pDispatch->ClearState(); + TestCsiCursorMovement(L'e', uiDistance, false, &pDispatch->_verticalPositionRelative, mach, *pDispatch); + pDispatch->ClearState(); TestCsiCursorMovement(L'@', uiDistance, false, &pDispatch->_insertCharacter, mach, *pDispatch); pDispatch->ClearState(); TestCsiCursorMovement(L'P', uiDistance, false, &pDispatch->_deleteCharacter, mach, *pDispatch);