Skip to content

Commit

Permalink
Add support for the HPR and VPR escape sequences (#4297)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
j4james authored and msftbot[bot] committed Jan 21, 2020
1 parent 027f122 commit cbb87b9
Show file tree
Hide file tree
Showing 10 changed files with 147 additions and 0 deletions.
75 changes: 75 additions & 0 deletions src/host/ut_host/ScreenBufferTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ class ScreenBufferTests
TEST_METHOD(CursorUpDownExactlyAtMargins);

TEST_METHOD(CursorNextPreviousLine);
TEST_METHOD(CursorPositionRelative);

TEST_METHOD(CursorSaveRestore);

Expand Down Expand Up @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions src/terminal/adapter/ITermDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ class Microsoft::Console::VirtualTerminal::ITermDispatch
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
Expand Down
24 changes: 24 additions & 0 deletions src/terminal/adapter/adaptDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions src/terminal/adapter/adaptDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/terminal/adapter/termDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class Microsoft::Console::VirtualTerminal::TermDispatch : public Microsoft::Cons
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
Expand Down
10 changes: 10 additions & 0 deletions src/terminal/parser/OutputStateMachineEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions src/terminal/parser/OutputStateMachineEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/terminal/parser/telemetry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
2 changes: 2 additions & 0 deletions src/terminal/parser/telemetry.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ namespace Microsoft::Console::VirtualTerminal
DSR,
DA,
VPA,
HPR,
VPR,
ICH,
DCH,
SU,
Expand Down
26 changes: 26 additions & 0 deletions src/terminal/parser/ut_parser/OutputEngineTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,8 @@ class StatefulDispatch final : public TermDispatch
_cursorPreviousLine{ false },
_cursorHorizontalPositionAbsolute{ false },
_verticalLinePositionAbsolute{ false },
_horizontalPositionRelative{ false },
_verticalPositionRelative{ false },
_cursorPosition{ false },
_cursorSave{ false },
_cursorLoad{ false },
Expand Down Expand Up @@ -751,6 +753,20 @@ class StatefulDispatch final : public TermDispatch
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;
Expand Down Expand Up @@ -965,6 +981,8 @@ class StatefulDispatch final : public TermDispatch
bool _cursorPreviousLine;
bool _cursorHorizontalPositionAbsolute;
bool _verticalLinePositionAbsolute;
bool _horizontalPositionRelative;
bool _verticalPositionRelative;
bool _cursorPosition;
bool _cursorSave;
bool _cursorLoad;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit cbb87b9

Please sign in to comment.