diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 3bdc2daa049..933b9b9c375 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -1417,18 +1417,44 @@ void DoSrvPrivateAllowCursorBlinking(SCREEN_INFORMATION& screenInfo, const bool // Make sure the cursor doesn't move outside the viewport. screenInfo.GetViewport().Clamp(clampedPos); - // Make sure the cursor stays inside the margins, but only if it started there - if (screenInfo.AreMarginsSet() && screenInfo.IsCursorInMargins(cursor.GetPosition())) + // Make sure the cursor stays inside the margins + if (screenInfo.AreMarginsSet()) { - try + const auto margins = screenInfo.GetAbsoluteScrollMargins().ToInclusive(); + + const auto cursorY = cursor.GetPosition().Y; + + const auto lo = margins.Top; + const auto hi = margins.Bottom; + + // See microsoft/terminal#2929 - If the cursor is _below_ the top + // margin, it should stay below the top margin. If it's _above_ the + // bottom, it should stay above the bottom. Cursor movements that stay + // outside the margins shouldn't necessarily be affected. For example, + // moving up while below the bottom margin shouldn't just jump straight + // to the bottom margin. See + // ScreenBufferTests::CursorUpDownOutsideMargins for a test of that + // behavior. + const bool cursorBelowTop = cursorY >= lo; + const bool cursorAboveBottom = cursorY <= hi; + + if (cursorBelowTop) + { + try + { + clampedPos.Y = std::max(clampedPos.Y, lo); + } + CATCH_RETURN(); + } + + if (cursorAboveBottom) { - const auto margins = screenInfo.GetAbsoluteScrollMargins().ToInclusive(); - const auto v = clampedPos.Y; - const auto lo = margins.Top; - const auto hi = margins.Bottom; - clampedPos.Y = std::clamp(v, lo, hi); + try + { + clampedPos.Y = std::min(clampedPos.Y, hi); + } + CATCH_RETURN(); } - CATCH_RETURN(); } cursor.SetPosition(clampedPos); diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 495933383cc..796be5fa13a 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -184,6 +184,10 @@ class ScreenBufferTests TEST_METHOD(ClearAlternateBuffer); TEST_METHOD(InitializeTabStopsInVTMode); + + TEST_METHOD(CursorUpDownAcrossMargins); + TEST_METHOD(CursorUpDownOutsideMargins); + TEST_METHOD(CursorUpDownExactlyAtMargins); }; void ScreenBufferTests::SingleAlternateBufferCreationTest() @@ -4480,3 +4484,170 @@ void ScreenBufferTests::InitializeTabStopsInVTMode() VERIFY_IS_TRUE(gci.GetActiveOutputBuffer().AreTabsSet()); } + +void ScreenBufferTests::CursorUpDownAcrossMargins() +{ + // Test inspired by: https://github.com/microsoft/terminal/issues/2929 + // echo -e "\e[6;19r\e[24H\e[99AX\e[1H\e[99BY\e[r" + // This does the following: + // * sets the top and bottom DECSTBM margins to 6 and 19 + // * moves to line 24 (i.e. below the bottom margin) + // * executes the CUU sequence with a count of 99, to move up 99 lines + // * writes out X + // * moves to line 1 (i.e. above the top margin) + // * executes the CUD sequence with a count of 99, to move down 99 lines + // * writes out Y + + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + auto& cursor = si.GetTextBuffer().GetCursor(); + + VERIFY_IS_TRUE(si.GetViewport().BottomInclusive() > 24); + + // Set some scrolling margins + stateMachine.ProcessString(L"\x1b[6;19r"); + stateMachine.ProcessString(L"\x1b[24H"); + VERIFY_ARE_EQUAL(23, cursor.GetPosition().Y); + + stateMachine.ProcessString(L"\x1b[99A"); + VERIFY_ARE_EQUAL(5, cursor.GetPosition().Y); + stateMachine.ProcessString(L"X"); + { + auto iter = tbi.GetCellDataAt({ 0, 5 }); + VERIFY_ARE_EQUAL(L"X", iter->Chars()); + } + stateMachine.ProcessString(L"\x1b[1H"); + VERIFY_ARE_EQUAL(0, cursor.GetPosition().Y); + + stateMachine.ProcessString(L"\x1b[99B"); + VERIFY_ARE_EQUAL(18, cursor.GetPosition().Y); + stateMachine.ProcessString(L"Y"); + { + auto iter = tbi.GetCellDataAt({ 0, 18 }); + VERIFY_ARE_EQUAL(L"Y", iter->Chars()); + } + stateMachine.ProcessString(L"\x1b[r"); +} + +void ScreenBufferTests::CursorUpDownOutsideMargins() +{ + // Test inspired by the CursorUpDownAcrossMargins test. + // echo -e "\e[6;19r\e[24H\e[1AX\e[1H\e[1BY\e[r" + // This does the following: + // * sets the top and bottom DECSTBM margins to 6 and 19 + // * moves to line 24 (i.e. below the bottom margin) + // * executes the CUU sequence with a count of 1, to move up 1 lines (still below margins) + // * writes out X + // * moves to line 1 (i.e. above the top margin) + // * executes the CUD sequence with a count of 1, to move down 1 lines (still above margins) + // * writes out Y + + // This test is different becasue the end location of the vertical movement + // should not be within the margins at all. We should not clamp this + // movement to be within the margins. + + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + auto& cursor = si.GetTextBuffer().GetCursor(); + + VERIFY_IS_TRUE(si.GetViewport().BottomInclusive() > 24); + + // Set some scrolling margins + stateMachine.ProcessString(L"\x1b[6;19r"); + stateMachine.ProcessString(L"\x1b[24H"); + VERIFY_ARE_EQUAL(23, cursor.GetPosition().Y); + + stateMachine.ProcessString(L"\x1b[1A"); + VERIFY_ARE_EQUAL(22, cursor.GetPosition().Y); + stateMachine.ProcessString(L"X"); + { + auto iter = tbi.GetCellDataAt({ 0, 22 }); + VERIFY_ARE_EQUAL(L"X", iter->Chars()); + } + stateMachine.ProcessString(L"\x1b[1H"); + VERIFY_ARE_EQUAL(0, cursor.GetPosition().Y); + + stateMachine.ProcessString(L"\x1b[1B"); + VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y); + stateMachine.ProcessString(L"Y"); + { + auto iter = tbi.GetCellDataAt({ 0, 1 }); + VERIFY_ARE_EQUAL(L"Y", iter->Chars()); + } + stateMachine.ProcessString(L"\x1b[r"); +} + +void ScreenBufferTests::CursorUpDownExactlyAtMargins() +{ + // Test inspired by the CursorUpDownAcrossMargins test. + // echo -e "\e[6;19r\e[19H\e[1B1\e[1A2\e[6H\e[1A3\e[1B4\e[r" + // This does the following: + // * sets the top and bottom DECSTBM margins to 6 and 19 + // * moves to line 19 (i.e. on the bottom margin) + // * executes the CUD sequence with a count of 1, to move down 1 lines (still on the margin) + // * writes out 1 + // * executes the CUU sequence with a count of 1, to move up 1 lines (now inside margins) + // * writes out 2 + // * moves to line 6 (i.e. on the top margin) + // * executes the CUU sequence with a count of 1, to move up 1 lines (still on the margin) + // * writes out 3 + // * executes the CUD sequence with a count of 1, to move down 1 lines (still above margins) + // * writes out 4 + + // This test is different becasue the starting location for these scroll + // operations is _exactly_ on the margins + + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + auto& cursor = si.GetTextBuffer().GetCursor(); + + VERIFY_IS_TRUE(si.GetViewport().BottomInclusive() > 24); + + // Set some scrolling margins + stateMachine.ProcessString(L"\x1b[6;19r"); + + stateMachine.ProcessString(L"\x1b[19;1H"); + VERIFY_ARE_EQUAL(18, cursor.GetPosition().Y); + stateMachine.ProcessString(L"\x1b[1B"); + VERIFY_ARE_EQUAL(18, cursor.GetPosition().Y); + stateMachine.ProcessString(L"1"); + { + auto iter = tbi.GetCellDataAt({ 0, 18 }); + VERIFY_ARE_EQUAL(L"1", iter->Chars()); + } + + stateMachine.ProcessString(L"\x1b[1A"); + VERIFY_ARE_EQUAL(17, cursor.GetPosition().Y); + stateMachine.ProcessString(L"2"); + { + auto iter = tbi.GetCellDataAt({ 1, 17 }); + VERIFY_ARE_EQUAL(L"2", iter->Chars()); + } + + stateMachine.ProcessString(L"\x1b[6;1H"); + VERIFY_ARE_EQUAL(5, cursor.GetPosition().Y); + + stateMachine.ProcessString(L"\x1b[1A"); + VERIFY_ARE_EQUAL(5, cursor.GetPosition().Y); + stateMachine.ProcessString(L"3"); + { + auto iter = tbi.GetCellDataAt({ 0, 5 }); + VERIFY_ARE_EQUAL(L"3", iter->Chars()); + } + + stateMachine.ProcessString(L"\x1b[1B"); + VERIFY_ARE_EQUAL(6, cursor.GetPosition().Y); + stateMachine.ProcessString(L"4"); + { + auto iter = tbi.GetCellDataAt({ 1, 6 }); + VERIFY_ARE_EQUAL(L"4", iter->Chars()); + } + + stateMachine.ProcessString(L"\x1b[r"); +}