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

Fix selection logic with shift on multi-click (#9403)

Browse source 
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/issues/9382
* [x] CLA signed. 
* [ ] Tests added/passed
* [ ] Documentation updated. 
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. 

## Detailed Description of the Pull Request / Additional comments
The selection with shift is quite broken in 1.6.

It started with #8611 that introduces cell selection on `shift+click`.
This change resulted in the following defect:
`shift+double-click`, `shift+triple-click` select only parts  of the word.
The reason for this is that the first `shift+click` establishes the selection,
while the consequent clicks simply extend it to the relevant boundary
(aka word / line boundary)

However, the logic was broken even before #8611.
For instance, `shift+triple-click` had exactly the same handicap:
`shift+double-click` was establishing the selection and the
third click was simply extending it to the line boundary.

This PR addresses the both defects in the following manner:
upon multi-click that starts new selection we establish
a new selection on every consequent click using appropriate mode
(cell/word/line) rather than trying to extend one.
For this purpose we remember the position that started the selection.
This commit is contained in:
Don-Vito 2021-03-08 21:15:46 +02:00 committed by GitHub
parent 87fa526fb7
commit 83f2a3bb3d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 43 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

63
src/cascadia/TerminalControl/TermControl.cpp
View file

@ -79,6 +79,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
_blinkTimer{},
_lastMouseClickTimestamp{},
_lastMouseClickPos{},
_lastMouseClickPosNoSelection{},
_selectionNeedsToBeCopied{ false },
_searchBox{ nullptr }
{
@ -1345,35 +1346,55 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
mode = ::Terminal::SelectionExpansionMode::Line;
}
// Update the selection appropriately
if (ctrlEnabled && multiClickMapper == 1 &&
!(_terminal->GetHyperlinkAtPosition(terminalPosition).empty()))
{
_HyperlinkHandler(_terminal->GetHyperlinkAtPosition(terminalPosition));
}
else if (shiftEnabled && _terminal->IsSelectionActive())
{
// Shift+Click: only set expand on the "end" selection point
_terminal->SetSelectionEnd(terminalPosition, mode);
_selectionNeedsToBeCopied = true;
}
else if (mode == ::Terminal::SelectionExpansionMode::Cell && !shiftEnabled)
{
// Single Click: reset the selection and begin a new one
_terminal->ClearSelection();
_singleClickTouchdownPos = cursorPosition;
_selectionNeedsToBeCopied = false; // there's no selection, so there's nothing to update
}
else
{
// Multi-Click Selection: expand both "start" and "end" selection points
_terminal->MultiClickSelection(terminalPosition, mode);
_selectionNeedsToBeCopied = true;
}
// Update the selection appropriately
_lastMouseClickTimestamp = point.Timestamp();
_lastMouseClickPos = cursorPosition;
_renderer->TriggerSelection();
// Capture the position of the first click when no selection is active
if (mode == ::Terminal::SelectionExpansionMode::Cell && !_terminal->IsSelectionActive())
{
_singleClickTouchdownPos = cursorPosition;
_lastMouseClickPosNoSelection = cursorPosition;
}
// We reset the active selection if one of the conditions apply:
// - shift is not held
// - GH#9384: the position is the same as of the first click starting the selection
// (we need to reset selection on double-click or triple-click, so it captures the word or the line,
// rather than extending the selection)
if (_terminal->IsSelectionActive() && (!shiftEnabled || _lastMouseClickPosNoSelection == cursorPosition))
{
// Reset the selection
_terminal->ClearSelection();
_selectionNeedsToBeCopied = false; // there's no selection, so there's nothing to update
}
if (shiftEnabled)
{
if (_terminal->IsSelectionActive())
{
// If there is a selection we extend it using the selection mode
// (expand the "end"selection point)
_terminal->SetSelectionEnd(terminalPosition, mode);
}
else
{
// If there is no selection we establish it using the selected mode
// (expand both "start" and "end" selection points)
_terminal->MultiClickSelection(terminalPosition, mode);
}
_selectionNeedsToBeCopied = true;
}
_lastMouseClickTimestamp = point.Timestamp();
_lastMouseClickPos = cursorPosition;
_renderer->TriggerSelection();
}
}
else if (point.Properties().IsRightButtonPressed())
{

1
src/cascadia/TerminalControl/TermControl.h
View file

@ -261,6 +261,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
unsigned int _multiClickCounter;
Timestamp _lastMouseClickTimestamp;
std::optional<winrt::Windows::Foundation::Point> _lastMouseClickPos;
std::optional<winrt::Windows::Foundation::Point> _lastMouseClickPosNoSelection;
std::optional<winrt::Windows::Foundation::Point> _singleClickTouchdownPos;
// This field tracks whether the selection has changed meaningfully
// since it was last copied. It's generally used to prevent copyOnSelect