microsoft/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2025-12-11 22:48:41 -06:00
Code Issues Packages Projects Releases 138 Wiki Activity

Make sure to update the selection pivot while circling (#14636)

Browse Source 
This builds upon #10749. When we added a separate pivot to track the "active" anchor, we forgot to update the pivot while circling. What does that mean? Moving the mouse would trigger us to update the selection using new endpoint and the old _pivot_, which hadn't been updated. 

There's probably a more elegant way of doing this, but it's good enough. 

Updated the test to cover this as well. 

Closes #14462
This commit is contained in:
Mike Griese 2023-01-16 10:13:20 -06:00 committed by GitHub
parent 020746d93d
commit dd2736f334
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 93 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/cascadia/TerminalCore/Terminal.cpp
View File

@ -1177,6 +1177,8 @@ void Terminal::_AdjustCursorPosition(const til::point proposedPosition)
// Update our selection too, so it doesn't move as the buffer is cycled
if (_selection)
{
// Stash this, so we can make sure to update the pivot to match later
const auto pivotWasStart = _selection->start == _selection->pivot;
// If the start of the selection is above 0, we can reduce both the start and end by 1
if (_selection->start.y > 0)
{
@ -1197,6 +1199,15 @@ void Terminal::_AdjustCursorPosition(const til::point proposedPosition)
_selection.reset();
}
}
// If we still have a selection, make sure to sync the pivot
// with whichever value is the right one.
//
// Failure to do this might lead to GH #14462
if (_selection.has_value())
{
_selection->pivot = pivotWasStart ? _selection->start : _selection->end;
}
}
}

84
src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp
View File

@ -757,12 +757,14 @@ namespace ControlUnitTests
_standardInit(core, interactivity);
Log::Comment(L"Fill up the history buffer");
const auto scrollbackLength = settings->HistorySize();
// Output lines equal to history size + viewport height to make sure we're
// at the point where outputting more lines causes circular incrementing
for (auto i = 0; i < settings->HistorySize() + core->ViewHeight(); ++i)
{
conn->WriteInput(L"Foo\r\n");
}
VERIFY_ARE_EQUAL(scrollbackLength, core->_terminal->GetScrollOffset());
// For this test, don't use any modifiers
const auto modifiers = ControlKeyStates();
@ -811,8 +813,86 @@ namespace ControlUnitTests
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());
{
const auto anchor{ core->_terminal->GetSelectionAnchor() };
const auto end{ core->_terminal->GetSelectionEnd() };
Log::Comment(fmt::format(L"expectedAnchor:({},{})", expectedAnchor.x, expectedAnchor.y).c_str());
Log::Comment(fmt::format(L"anchor:({},{})", anchor.x, anchor.y).c_str());
Log::Comment(fmt::format(L"end:({},{})", end.x, end.y).c_str());
VERIFY_ARE_EQUAL(expectedAnchor, anchor);
VERIFY_ARE_EQUAL(expectedAnchor, end);
}
VERIFY_ARE_EQUAL(scrollbackLength - 1, core->_terminal->GetScrollOffset());
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;
{
const auto anchor{ core->_terminal->GetSelectionAnchor() };
const auto end{ core->_terminal->GetSelectionEnd() };
Log::Comment(fmt::format(L"expectedAnchor:({},{})", expectedAnchor.x, expectedAnchor.y).c_str());
Log::Comment(fmt::format(L"anchor:({},{})", anchor.x, anchor.y).c_str());
Log::Comment(fmt::format(L"end:({},{})", end.x, end.y).c_str());
VERIFY_ARE_EQUAL(expectedAnchor, anchor);
VERIFY_ARE_EQUAL(expectedAnchor, end);
}
VERIFY_ARE_EQUAL(scrollbackLength - 2, core->_terminal->GetScrollOffset());
Log::Comment(L"Move the mouse a little, to update the selection");
// At this point, there should only be one selection region! The
// viewport moved up to keep the selection at the same relative spot. So
// wiggling the cursor should continue to select only the same
// character in the buffer (if, albeit in a new location).
//
// This helps test GH #14462, a regression from #10749.
interactivity->PointerMoved(leftMouseDown,
WM_LBUTTONDOWN, //pointerUpdateKind
modifiers,
true, // focused,
cursorPosition0.to_core_point(),
true);
VERIFY_IS_TRUE(core->HasSelection());
VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size());
{
const auto anchor{ core->_terminal->GetSelectionAnchor() };
const auto end{ core->_terminal->GetSelectionEnd() };
Log::Comment(fmt::format(L"expectedAnchor:({},{})", expectedAnchor.x, expectedAnchor.y).c_str());
Log::Comment(fmt::format(L"anchor:({},{})", anchor.x, anchor.y).c_str());
Log::Comment(fmt::format(L"end:({},{})", end.x, end.y).c_str());
VERIFY_ARE_EQUAL(expectedAnchor, anchor);
VERIFY_ARE_EQUAL(expectedAnchor, end);
}
Log::Comment(L"Output a line ant move the mouse a little to update the selection, all at once");
// Same as above. The viewport has moved, so the mouse is still over the
// same character, even though it's at a new offset.
conn->WriteInput(L"Foo\r\n");
expectedAnchor.y -= 1;
VERIFY_ARE_EQUAL(scrollbackLength - 3, core->_terminal->GetScrollOffset());
interactivity->PointerMoved(leftMouseDown,
WM_LBUTTONDOWN, //pointerUpdateKind
modifiers,
true, // focused,
cursorPosition1.to_core_point(),
true);
VERIFY_IS_TRUE(core->HasSelection());
VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size());
{
const auto anchor{ core->_terminal->GetSelectionAnchor() };
const auto end{ core->_terminal->GetSelectionEnd() };
Log::Comment(fmt::format(L"expectedAnchor:({},{})", expectedAnchor.x, expectedAnchor.y).c_str());
Log::Comment(fmt::format(L"anchor:({},{})", anchor.x, anchor.y).c_str());
Log::Comment(fmt::format(L"end:({},{})", end.x, end.y).c_str());
VERIFY_ARE_EQUAL(expectedAnchor, anchor);
VERIFY_ARE_EQUAL(expectedAnchor, end);
}
// Output enough text for the selection to get pushed off the buffer
for (auto i = 0; i < settings->HistorySize() + core->ViewHeight(); ++i)