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

Fix viewport moving when we've scrolled up and circled the buffer (#7247)

Browse source 
If you scroll up to view the scrollback, then we want the viewport to
"stay in place", as new output comes in (see #6062). This works fine up
until the buffer circles. In this case, the mutable viewport isn't
actually moving, so we never set `updatedViewport` to true. 

This regressed in #6062
Closes #7222
This commit is contained in:
Mike Griese 2020-08-11 14:57:45 -05:00 committed by GitHub
parent 7ccd1f6f1a
commit bc642bbf2a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 101 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
src/cascadia/TerminalCore/Terminal.cpp
View file

@ -843,8 +843,12 @@ void Terminal::_AdjustCursorPosition(const COORD proposedPosition)
}
}
if (updatedViewport)
// If the viewport moved, or we circled the buffer, we might need to update
// our _scrollOffset
if (updatedViewport || newRows != 0)
{
const auto oldScrollOffset = _scrollOffset;
// scroll if...
// - no selection is active
// - viewport is already at the bottom
@ -852,6 +856,18 @@ void Terminal::_AdjustCursorPosition(const COORD proposedPosition)
_scrollOffset = scrollToOutput ? 0 : _scrollOffset + scrollAmount + newRows;
// Clamp the range to make sure that we don't scroll way off the top of the buffer
_scrollOffset = std::clamp(_scrollOffset,
0,
_buffer->GetSize().Height() - _mutableViewport.Height());
// If the new scroll offset is different, then we'll still want to raise a scroll event
updatedViewport = updatedViewport || (oldScrollOffset != _scrollOffset);
}
// If the viewport moved, then send a scrolling notification.
if (updatedViewport)
{
_NotifyScrollEvent();
}

84
src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp
View file

@ -39,6 +39,8 @@ class TerminalCoreUnitTests::TerminalBufferTests final
TEST_METHOD(TestWrappingCharByChar);
TEST_METHOD(TestWrappingALongString);
TEST_METHOD(DontSnapToOutputTest);
TEST_METHOD_SETUP(MethodSetup)
{
// STEP 1: Set up the Terminal
@ -149,3 +151,85 @@ void TerminalBufferTests::TestWrappingALongString()
TestUtils::VerifyExpectedString(termTb, TestUtils::Test100CharsString, { 0, 0 });
}
void TerminalBufferTests::DontSnapToOutputTest()
{
auto& termTb = *term->_buffer;
auto& termSm = *term->_stateMachine;
const auto initialView = term->GetViewport();
VERIFY_ARE_EQUAL(0, initialView.Top());
VERIFY_ARE_EQUAL(TerminalViewHeight, initialView.BottomExclusive());
VERIFY_ARE_EQUAL(0, term->_scrollOffset);
// -1 so that we don't print the last \n
for (int i = 0; i < TerminalViewHeight + 8 - 1; i++)
{
termSm.ProcessString(L"x\n");
}
const auto secondView = term->GetViewport();
VERIFY_ARE_EQUAL(8, secondView.Top());
VERIFY_ARE_EQUAL(TerminalViewHeight + 8, secondView.BottomExclusive());
VERIFY_ARE_EQUAL(0, term->_scrollOffset);
Log::Comment(L"Scroll up one line");
term->_scrollOffset = 1;
const auto thirdView = term->GetViewport();
VERIFY_ARE_EQUAL(7, thirdView.Top());
VERIFY_ARE_EQUAL(TerminalViewHeight + 7, thirdView.BottomExclusive());
VERIFY_ARE_EQUAL(1, term->_scrollOffset);
Log::Comment(L"Print a few lines, to see that the viewport stays where it was");
for (int i = 0; i < 8; i++)
{
termSm.ProcessString(L"x\n");
}
const auto fourthView = term->GetViewport();
VERIFY_ARE_EQUAL(7, fourthView.Top());
VERIFY_ARE_EQUAL(TerminalViewHeight + 7, fourthView.BottomExclusive());
VERIFY_ARE_EQUAL(1 + 8, term->_scrollOffset);
Log::Comment(L"Print enough lines to get the buffer just about ready to "
L"circle (on the next newline)");
auto viewBottom = term->_mutableViewport.BottomInclusive();
do
{
termSm.ProcessString(L"x\n");
viewBottom = term->_mutableViewport.BottomInclusive();
} while (viewBottom < termTb.GetSize().BottomInclusive());
const auto fifthView = term->GetViewport();
VERIFY_ARE_EQUAL(7, fifthView.Top());
VERIFY_ARE_EQUAL(TerminalViewHeight + 7, fifthView.BottomExclusive());
VERIFY_ARE_EQUAL(TerminalHistoryLength - 7, term->_scrollOffset);
Log::Comment(L"Print 3 more lines, and see that we stick to where the old "
L"rows now are in the buffer (after circling)");
for (int i = 0; i < 3; i++)
{
termSm.ProcessString(L"x\n");
Log::Comment(NoThrowString().Format(
L"_scrollOffset: %d", term->_scrollOffset));
}
const auto sixthView = term->GetViewport();
VERIFY_ARE_EQUAL(4, sixthView.Top());
VERIFY_ARE_EQUAL(TerminalViewHeight + 4, sixthView.BottomExclusive());
VERIFY_ARE_EQUAL(TerminalHistoryLength - 4, term->_scrollOffset);
Log::Comment(L"Print 8 more lines, and see that we're now just stuck at the"
L"top of the buffer");
for (int i = 0; i < 8; i++)
{
termSm.ProcessString(L"x\n");
Log::Comment(NoThrowString().Format(
L"_scrollOffset: %d", term->_scrollOffset));
}
const auto seventhView = term->GetViewport();
VERIFY_ARE_EQUAL(0, seventhView.Top());
VERIFY_ARE_EQUAL(TerminalViewHeight, seventhView.BottomExclusive());
VERIFY_ARE_EQUAL(TerminalHistoryLength, term->_scrollOffset);
}