Skip to content

Commit

Permalink
Implement Shift+MultiClick Selection Expansion (#6322)
Browse files Browse the repository at this point in the history 
This pull request implements shift+double/triple click. Proper behavior
(as described in #4557) is to only expand one selection point, not both.

Adding the `bool targetStart` was a bit weird. I decided on this being
the cleanest approach though because I still want `PivotSelection` to be
its own helper function. Otherwise, the concept of "pivoting" gets kinda
messy.

## Validation Steps Performed
Manual testing as described on attached issue.
Tests were added for Shift+Click and pivoting the selection too.

Closes #4557
  • Loading branch information
@carlos-zamora
carlos-zamora authored Jun 25, 2020
1 parent cffd4eb commit 9215b52
Show file tree
Hide file tree
Showing 4 changed files with 219 additions and 24 deletions.
46 changes: 27 additions & 19 deletions src/cascadia/TerminalControl/TermControl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1078,35 +1078,43 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
const unsigned int MAX_CLICK_COUNT = 3;
const auto multiClickMapper = clickCount > MAX_CLICK_COUNT ? ((clickCount + MAX_CLICK_COUNT - 1) % MAX_CLICK_COUNT) + 1 : clickCount;

if (multiClickMapper == 3)
::Terminal::SelectionExpansionMode mode = ::Terminal::SelectionExpansionMode::Cell;
if (multiClickMapper == 1)
{
_terminal->MultiClickSelection(terminalPosition, ::Terminal::SelectionExpansionMode::Line);
_selectionNeedsToBeCopied = true;
mode = ::Terminal::SelectionExpansionMode::Cell;
}
else if (multiClickMapper == 2)
{
_terminal->MultiClickSelection(terminalPosition, ::Terminal::SelectionExpansionMode::Word);
mode = ::Terminal::SelectionExpansionMode::Word;
}
else if (multiClickMapper == 3)
{
mode = ::Terminal::SelectionExpansionMode::Line;
}

// Update the selection appropriately
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)
{
// 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
{
if (shiftEnabled && _terminal->IsSelectionActive())
{
_terminal->SetSelectionEnd(terminalPosition, ::Terminal::SelectionExpansionMode::Cell);
_selectionNeedsToBeCopied = true;
}
else
{
// A single click down resets the selection and begins a new one.
_terminal->ClearSelection();
_singleClickTouchdownPos = cursorPosition;
_selectionNeedsToBeCopied = false; // there's no selection, so there's nothing to update
}

_lastMouseClickTimestamp = point.Timestamp();
_lastMouseClickPos = cursorPosition;
// Multi-Click Selection: 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())
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalCore/Terminal.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ class Microsoft::Terminal::Core::Terminal final :
#pragma region TextSelection
// These methods are defined in TerminalSelection.cpp
std::vector<SMALL_RECT> _GetSelectionRects() const noexcept;
std::pair<COORD, COORD> _PivotSelection(const COORD targetPos) const;
std::pair<COORD, COORD> _PivotSelection(const COORD targetPos, bool& targetStart) const;
std::pair<COORD, COORD> _ExpandSelectionAnchors(std::pair<COORD, COORD> anchors) const;
COORD _ConvertToBufferCell(const COORD viewportPos) const;
#pragma endregion
Expand Down
26 changes: 22 additions & 4 deletions src/cascadia/TerminalCore/TerminalSelection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,20 +151,38 @@ void Terminal::SetSelectionEnd(const COORD viewportPos, std::optional<SelectionE
// Otherwise, we may accidentally expand during other selection-based actions
_multiClickSelectionMode = newExpansionMode.has_value() ? *newExpansionMode : _multiClickSelectionMode;

const auto anchors = _PivotSelection(textBufferPos);
std::tie(_selection->start, _selection->end) = _ExpandSelectionAnchors(anchors);
bool targetStart = false;
const auto anchors = _PivotSelection(textBufferPos, targetStart);
const auto expandedAnchors = _ExpandSelectionAnchors(anchors);

if (newExpansionMode.has_value())
{
// shift-click operations only expand the target side
auto& anchorToExpand = targetStart ? _selection->start : _selection->end;
anchorToExpand = targetStart ? expandedAnchors.first : expandedAnchors.second;

// the other anchor should then become the pivot (we don't expand it)
auto& anchorToPivot = targetStart ? _selection->end : _selection->start;
anchorToPivot = _selection->pivot;
}
else
{
// expand both anchors
std::tie(_selection->start, _selection->end) = expandedAnchors;
}
}

// Method Description:
// - returns a new pair of selection anchors for selecting around the pivot
// - This ensures start < end when compared
// Arguments:
// - targetPos: the (x,y) coordinate we are moving to on the text buffer
// - targetStart: if true, target will be the new start. Otherwise, target will be the new end.
// Return Value:
// - the new start/end for a selection
std::pair<COORD, COORD> Terminal::_PivotSelection(const COORD targetPos) const
std::pair<COORD, COORD> Terminal::_PivotSelection(const COORD targetPos, bool& targetStart) const
{
if (_buffer->GetSize().CompareInBounds(targetPos, _selection->pivot) <= 0)
if (targetStart = _buffer->GetSize().CompareInBounds(targetPos, _selection->pivot) <= 0)
{
// target is before pivot
// treat target as start
Expand Down
169 changes: 169 additions & 0 deletions src/cascadia/UnitTests_TerminalCore/SelectionTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -672,5 +672,174 @@ namespace TerminalCoreUnitTests
selection = term.GetViewport().ConvertToOrigin(selectionRects.at(1)).ToInclusive();
VERIFY_ARE_EQUAL(selection, SMALL_RECT({ 0, 11, 99, 11 }));
}

TEST_METHOD(ShiftClick)
{
Terminal term;
DummyRenderTarget emptyRT;
term.Create({ 100, 100 }, 0, emptyRT);

// set word delimiters for terminal
auto settings = winrt::make<MockTermSettings>(0, 100, 100);
term.UpdateSettings(settings);

// Insert text at position (4,10)
const std::wstring_view text = L"doubleClickMe dragThroughHere";
term.SetCursorPosition(4, 10);
term.Write(text);

// Step 1: Create a selection on "doubleClickMe"
{
// Simulate double click at (x,y) = (5,10)
term.MultiClickSelection({ 5, 10 }, Terminal::SelectionExpansionMode::Word);

// Validate selection area: "doubleClickMe" selected
ValidateSingleRowSelection(term, SMALL_RECT({ 4, 10, 16, 10 }));
}

// Step 2: Shift+Click to "dragThroughHere"
{
// Simulate Shift+Click at (x,y) = (21,10)
//
// buffer: doubleClickMe dragThroughHere
// ^ ^
// start finish
term.SetSelectionEnd({ 21, 10 }, ::Terminal::SelectionExpansionMode::Cell);

// Validate selection area: "doubleClickMe drag" selected
ValidateSingleRowSelection(term, SMALL_RECT({ 4, 10, 21, 10 }));
}

// Step 3: Shift+Double-Click at "dragThroughHere"
{
// Simulate Shift+DoubleClick at (x,y) = (21,10)
//
// buffer: doubleClickMe dragThroughHere
// ^ ^ ^
// start click finish
term.SetSelectionEnd({ 21, 10 }, ::Terminal::SelectionExpansionMode::Word);

// Validate selection area: "doubleClickMe dragThroughHere" selected
ValidateSingleRowSelection(term, SMALL_RECT({ 4, 10, 32, 10 }));
}

// Step 4: Shift+Triple-Click at "dragThroughHere"
{
// Simulate Shift+TripleClick at (x,y) = (21,10)
//
// buffer: doubleClickMe dragThroughHere |
// ^ ^ ^
// start click finish (boundary)
term.SetSelectionEnd({ 21, 10 }, ::Terminal::SelectionExpansionMode::Line);

// Validate selection area: "doubleClickMe dragThroughHere..." selected
ValidateSingleRowSelection(term, SMALL_RECT({ 4, 10, 99, 10 }));
}

// Step 5: Shift+Double-Click at "dragThroughHere"
{
// Simulate Shift+DoubleClick at (x,y) = (21,10)
//
// buffer: doubleClickMe dragThroughHere
// ^ ^ ^
// start click finish
term.SetSelectionEnd({ 21, 10 }, ::Terminal::SelectionExpansionMode::Word);

// Validate selection area: "doubleClickMe dragThroughHere" selected
ValidateSingleRowSelection(term, SMALL_RECT({ 4, 10, 32, 10 }));
}

// Step 6: Drag past "dragThroughHere"
{
// Simulate drag to (x,y) = (35,10)
// Since we were preceded by a double-click, we're in "word" expansion mode
//
// buffer: doubleClickMe dragThroughHere |
// ^ ^
// start finish (boundary)
term.SetSelectionEnd({ 35, 10 });

// Validate selection area: "doubleClickMe dragThroughHere..." selected
ValidateSingleRowSelection(term, SMALL_RECT({ 4, 10, 99, 10 }));
}

// Step 6: Drag back to "dragThroughHere"
{
// Simulate drag to (x,y) = (21,10)
//
// buffer: doubleClickMe dragThroughHere
// ^ ^ ^
// start drag finish
term.SetSelectionEnd({ 21, 10 });

// Validate selection area: "doubleClickMe dragThroughHere" selected
ValidateSingleRowSelection(term, SMALL_RECT({ 4, 10, 32, 10 }));
}

// Step 7: Drag within "dragThroughHere"
{
// Simulate drag to (x,y) = (25,10)
//
// buffer: doubleClickMe dragThroughHere
// ^ ^ ^
// start drag finish
term.SetSelectionEnd({ 25, 10 });

// Validate selection area: "doubleClickMe dragThroughHere" still selected
ValidateSingleRowSelection(term, SMALL_RECT({ 4, 10, 32, 10 }));
}
}

TEST_METHOD(Pivot)
{
Terminal term;
DummyRenderTarget emptyRT;
term.Create({ 100, 100 }, 0, emptyRT);

// Step 1: Create a selection
{
// (10,10) to (20, 10)
term.SelectNewRegion({ 10, 10 }, { 20, 10 });

// Validate selection area
ValidateSingleRowSelection(term, SMALL_RECT({ 10, 10, 20, 10 }));
}

// Step 2: Drag to (5,10)
{
term.SetSelectionEnd({ 5, 10 });

// Validate selection area
// NOTE: Pivot should be (10, 10)
ValidateSingleRowSelection(term, SMALL_RECT({ 5, 10, 10, 10 }));
}

// Step 3: Drag back to (20,10)
{
term.SetSelectionEnd({ 20, 10 });

// Validate selection area
// NOTE: Pivot should still be (10, 10)
ValidateSingleRowSelection(term, SMALL_RECT({ 10, 10, 20, 10 }));
}

// Step 4: Shift+Click at (5,10)
{
term.SetSelectionEnd({ 5, 10 }, ::Terminal::SelectionExpansionMode::Cell);

// Validate selection area
// NOTE: Pivot should still be (10, 10)
ValidateSingleRowSelection(term, SMALL_RECT({ 5, 10, 10, 10 }));
}

// Step 5: Shift+Click back at (20,10)
{
term.SetSelectionEnd({ 20, 10 }, ::Terminal::SelectionExpansionMode::Cell);

// Validate selection area
// NOTE: Pivot should still be (10, 10)
ValidateSingleRowSelection(term, SMALL_RECT({ 10, 10, 20, 10 }));
}
}
};
}

0 comments on commit 9215b52

Please sign in to comment.