Fix text selection while new lines are being printed when history buffer is full (#10749)

When our text buffer is full, newlines cause the buffer to scroll underneath the viewport (rather than the viewport moving down). This was causing selections made during text output to scroll down. To solve this, when we increment the circular buffer, we decrement the y-coordinates of the current selections by 1. We also invalidate the previous selection rects.

Closes #10319
This commit is contained in:
PankajBhojwani 2021-08-20 15:36:25 -07:00 committed by GitHub
parent 49874d1b9e
commit cb2f347c2f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 107 additions and 0 deletions

View file

@ -991,6 +991,31 @@ void Terminal::_AdjustCursorPosition(const COORD proposedPosition)
_buffer->IncrementCircularBuffer();
proposedCursorPosition.Y--;
rowsPushedOffTopOfBuffer++;
// Update our selection too, so it doesn't move as the buffer is cycled
if (_selection)
{
// If the start of the selection is above 0, we can reduce both the start and end by 1
if (_selection->start.Y > 0)
{
_selection->start.Y -= 1;
_selection->end.Y -= 1;
}
else
{
// The start of the selection is at 0, if the end is greater than 0, then only reduce the end
if (_selection->end.Y > 0)
{
_selection->start.X = 0;
_selection->end.Y -= 1;
}
else
{
// Both the start and end of the selection are at 0, clear the selection
_selection.reset();
}
}
}
}
// manually erase our pattern intervals since the locations have changed now

View file

@ -36,6 +36,7 @@ namespace ControlUnitTests
TEST_METHOD(TestDragSelectOutsideBounds);
TEST_METHOD(PointerClickOutsideActiveRegion);
TEST_METHOD(IncrementCircularBufferWithSelection);
TEST_CLASS_SETUP(ClassSetup)
{
@ -719,4 +720,81 @@ namespace ControlUnitTests
Log::Comment(L"Verify that there's still no selection");
VERIFY_IS_FALSE(core->HasSelection());
}
void ControlInteractivityTests::IncrementCircularBufferWithSelection()
{
// This is a test for GH#10749
WEX::TestExecution::DisableVerifyExceptions disableVerifyExceptions{};
auto [settings, conn] = _createSettingsAndConnection();
auto [core, interactivity] = _createCoreAndInteractivity(*settings, *conn);
_standardInit(core, interactivity);
Log::Comment(L"Fill up the history buffer");
// Output lines equal to history size + viewport height to make sure we're
// at the point where outputting more lines causes circular incrementing
for (int i = 0; i < settings->HistorySize() + core->ViewHeight(); ++i)
{
conn->WriteInput(L"Foo\r\n");
}
// For this test, don't use any modifiers
const auto modifiers = ControlKeyStates();
const Control::MouseButtonState leftMouseDown{ Control::MouseButtonState::IsLeftButtonDown };
const Control::MouseButtonState noMouseDown{};
const til::size fontSize{ 9, 21 };
Log::Comment(L"Click on the terminal");
const til::point terminalPosition0{ 5, 5 };
const til::point cursorPosition0{ terminalPosition0 * fontSize };
interactivity->PointerPressed(leftMouseDown,
WM_LBUTTONDOWN, //pointerUpdateKind
0, // timestamp
modifiers,
cursorPosition0);
Log::Comment(L"Verify that there's not yet a selection");
VERIFY_IS_FALSE(core->HasSelection());
VERIFY_IS_TRUE(interactivity->_singleClickTouchdownPos.has_value());
VERIFY_ARE_EQUAL(cursorPosition0, interactivity->_singleClickTouchdownPos.value());
Log::Comment(L"Drag the mouse just a little");
// move not quite a whole cell, but enough to start a selection
const til::point cursorPosition1{ cursorPosition0 + til::point{ 6, 0 } };
interactivity->PointerMoved(leftMouseDown,
WM_LBUTTONDOWN, //pointerUpdateKind
modifiers,
true, // focused,
cursorPosition1,
true);
Log::Comment(L"Verify that there's one selection");
VERIFY_IS_TRUE(core->HasSelection());
VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size());
Log::Comment(L"Verify the location of the selection");
// The viewport is on row (historySize + 5), so the selection will be on:
// {(5, (historySize+5))+(0, 21)} to {(5, (historySize+5))+(0, 21)}
COORD expectedAnchor{ 5, gsl::narrow_cast<SHORT>(settings->HistorySize()) + 5 };
VERIFY_ARE_EQUAL(expectedAnchor, core->_terminal->GetSelectionAnchor());
VERIFY_ARE_EQUAL(expectedAnchor, core->_terminal->GetSelectionEnd());
Log::Comment(L"Output a line of text");
conn->WriteInput(L"Foo\r\n");
Log::Comment(L"Verify the location of the selection");
// The selection should now be 1 row lower
expectedAnchor.Y -= 1;
VERIFY_ARE_EQUAL(expectedAnchor, core->_terminal->GetSelectionAnchor());
VERIFY_ARE_EQUAL(expectedAnchor, core->_terminal->GetSelectionEnd());
// Output enough text for the selection to get pushed off the buffer
for (int i = 0; i < settings->HistorySize() + core->ViewHeight(); ++i)
{
conn->WriteInput(L"Foo\r\n");
}
// Verify that the selection got reset
VERIFY_IS_FALSE(core->HasSelection());
}
}

View file

@ -466,12 +466,16 @@ void Renderer::TriggerScroll(const COORD* const pcoordDelta)
// - <none>
void Renderer::TriggerCircling()
{
const auto rects = _GetSelectionRects();
for (IRenderEngine* const pEngine : _rgpEngines)
{
bool fEngineRequestsRepaint = false;
HRESULT hr = pEngine->InvalidateCircling(&fEngineRequestsRepaint);
LOG_IF_FAILED(hr);
LOG_IF_FAILED(pEngine->InvalidateSelection(rects));
if (SUCCEEDED(hr) && fEngineRequestsRepaint)
{
LOG_IF_FAILED(_PaintFrameForEngine(pEngine));