maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

do not allow CUU and CUD to move "across" margins. (#2996)

Browse source 
If we're moving the cursor up, its vertical movement should be stopped
at the top margin. It should not magically jump up to the bottom margin.
Similarly, this applies to moving down and the bottom margin.
Furthermore, this constraint should always apply, not just when the
start position is within BOTH margins

Fixes #2929.
This commit is contained in:
Mike Griese 2019-10-01 12:42:33 -05:00 committed by Dustin L. Howett (MSFT)
parent 33361698f7
commit 0ce08aff32
2 changed files with 206 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
src/host/getset.cpp
View file

@ -1430,18 +1430,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)
{
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::max(clampedPos.Y, lo);
}
CATCH_RETURN();
}
if (cursorAboveBottom)
{
try
{
clampedPos.Y = std::min(clampedPos.Y, hi);
}
CATCH_RETURN();
}
CATCH_RETURN();
}
cursor.SetPosition(clampedPos);

171
src/host/ut_host/ScreenBufferTests.cpp
View file

@ -184,6 +184,10 @@ class ScreenBufferTests
TEST_METHOD(ClearAlternateBuffer);
TEST_METHOD(InitializeTabStopsInVTMode);
TEST_METHOD(CursorUpDownAcrossMargins);
TEST_METHOD(CursorUpDownOutsideMargins);
TEST_METHOD(CursorUpDownExactlyAtMargins);
};
void ScreenBufferTests::SingleAlternateBufferCreationTest()
@ -4500,3 +4504,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");
}