starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-08 13:49:31 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Update DECSC/DECRC to match the newer DEC terminals (#15054)

Browse Source 
When saving and restoring the cursor position with origin mode enabled,
we originally matched the behavior of the early DEC terminals, which
stored the position as an absolute offset. But if the margin boundaries
were changed prior to restoring the position, that could result in the
cursor being outside the margins, potentially with negative coordinates.

Our implementation avoided that bug by clamping the coordinates back
into range, but that's not how DEC ultimately fixed the issue. Starting
with the VT420, they began using relative coordinates (i.e. relative to
the margin origin), so a restored position could never end up negative.
This PR updates our implementation to match that newer behavior.

## Validation Steps Performed

Thank to testing performed by @al20878, we know this was the algorithm
used on the VT420, VT520, and VT525, and I've manually confirmed that
our implementation now matches their behavior.

I've also updated the `CursorSaveRestore` unit test which previously
covered our clamping behavior - it's now being used to confirm that
we're correctly using relative offsets when restoring the cursor.

Closes #15048
This commit is contained in:
James Holderness
2023-03-28 20:03:41 +01:00
committed by GitHub
parent c7816fdb05
commit 34aa6aa0d4
2 changed files with 14 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/host/ut_host/ScreenBufferTests.cpp
View File

@@ -6349,17 +6349,17 @@ void ScreenBufferTests::CursorSaveRestore()
// Verify home position inside margins, i.e. relative origin mode restored.
VERIFY_ARE_EQUAL(til::point(0, 9), cursor.GetPosition());
Log::Comment(L"Clamp inside top margin.");
Log::Comment(L"Restore relative to new origin.");
// Reset margins, with absolute origin, and set cursor position.
stateMachine.ProcessString(L"\x1b[r");
stateMachine.ProcessString(setDECOM);
cursor.SetPosition({ 5, 15 });
cursor.SetPosition({ 5, 5 });
// Save state.
stateMachine.ProcessString(saveCursor);
// Set margins and restore state.
stateMachine.ProcessString(L"\x1b[20;25r");
stateMachine.ProcessString(L"\x1b[15;25r");
stateMachine.ProcessString(restoreCursor);
// Verify Y position is clamped inside the top margin
// Verify Y position is now relative to new top margin
VERIFY_ARE_EQUAL(til::point(5, 19), cursor.GetPosition());
Log::Comment(L"Clamp inside bottom margin.");

28
src/terminal/adapter/adaptDispatch.cpp
View File

@@ -438,6 +438,12 @@ bool AdaptDispatch::CursorSaveState()
auto cursorPosition = textBuffer.GetCursor().GetPosition();
cursorPosition.y -= viewport.top;
// Although if origin mode is set, the cursor is relative to the margin origin.
if (_modes.test(Mode::Origin))
{
cursorPosition.y -= _GetVerticalMargins(viewport, false).first;
}
// VT is also 1 based, not 0 based, so correct by 1.
auto& savedCursorState = _savedCursorState.at(_usingAltBuffer);
savedCursorState.Column = cursorPosition.x + 1;
@@ -464,22 +470,11 @@ bool AdaptDispatch::CursorRestoreState()
{
auto& savedCursorState = _savedCursorState.at(_usingAltBuffer);
auto row = savedCursorState.Row;
const auto col = savedCursorState.Column;
// Restore the origin mode first, since the cursor coordinates may be relative.
_modes.set(Mode::Origin, savedCursorState.IsOriginModeRelative);
// If the origin mode is relative, we need to make sure the restored
// position is clamped within the margins.
if (savedCursorState.IsOriginModeRelative)
{
const auto viewport = _api.GetViewport();
const auto [topMargin, bottomMargin] = _GetVerticalMargins(viewport, false);
// VT origin is at 1,1 so we need to add 1 to these margins.
row = std::clamp(row, topMargin + 1, bottomMargin + 1);
}
// The saved coordinates are always absolute, so we need reset the origin mode temporarily.
_modes.reset(Mode::Origin);
CursorPosition(row, col);
// We can then restore the position with a standard CUP operation.
CursorPosition(savedCursorState.Row, savedCursorState.Column);
// If the delayed wrap flag was set when the cursor was saved, we need to restore that now.
if (savedCursorState.IsDelayedEOLWrap)
@@ -487,9 +482,6 @@ bool AdaptDispatch::CursorRestoreState()
_api.GetTextBuffer().GetCursor().DelayEOLWrap();
}
// Once the cursor position is restored, we can then restore the actual origin mode.
_modes.set(Mode::Origin, savedCursorState.IsOriginModeRelative);
// Restore text attributes.
_api.SetTextAttributes(savedCursorState.Attributes);