Skip to content

Commit

Permalink
Rework and simplify selection in TermControl
Browse files Browse the repository at this point in the history 
This commit rewrites selection handling at the TermControl layer.
Previously, we were keeping track of a number of redundant variables
that were easy to get out of sync.

The new selection model is as follows:

* A single left click will always begin a _pending_ selection operation
* A single left click will always clear a selection (#4477)
* A double left click will always begin a word selection
* A triple left click will always begin a line selection
* A selection will only truly start when the cursor moves a quarter of
  the smallest dimension of a cell (usually its width) in any direction
  _This eliminates the selection of a single cell on one click._
  (#4282, #5082)
* We now keep track of whether the selection has been "copied", or
  "updated" since it was last copied. If an endpoint moves, it is
  updated. For copy-on-select, it is only copied if it's updated.
  (#4740)

Because of this, we can stop tracking the position of the focus-raising
click, and whether it was part of click-drag operation. All clicks can
_become_ part of a click-drag operation if the user drags.

We can also eliminate the special handling of single cell selection at
the TerminalCore layer: since TermControl determines when to begin a
selection, TerminalCore no longer needs to know whether copy on select
is enabled _or_ whether the user has started and then backtracked over a
single cell. This is now implicit in TermControl.

Fixes #4082; Fixes #4477
  • Loading branch information
@DHowett
DHowett committed Mar 24, 2020
1 parent a338227 commit 53080d3
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 76 deletions.
60 changes: 31 additions & 29 deletions src/cascadia/TerminalControl/TermControl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
_cursorTimer{},
_lastMouseClickTimestamp{},
_lastMouseClickPos{},
_searchBox{ nullptr },
_focusRaisedClickPos{ std::nullopt },
_clickDrag{ false }
_selectionUpdatedThisCycle{ false },
_searchBox{ nullptr }
{
_EnsureStaticInitialization();
InitializeComponent();
Expand Down Expand Up @@ -911,9 +910,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
if (!_focused)
{
Focus(FocusState::Pointer);
// Save the click position that brought this control into focus
// in case the user wants to perform a click-drag selection.
_focusRaisedClickPos = point.Position();
}

if (ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Mouse || ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Pen)
Expand All @@ -933,13 +929,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation

if (point.Properties().IsLeftButtonPressed())
{
// A single left click from out of focus should only focus.
if (_focusRaisedClickPos)
{
args.Handled(true);
return;
}

const auto cursorPosition = point.Position();
const auto terminalPosition = _GetTerminalPosition(cursorPosition);

Expand All @@ -956,20 +945,26 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
if (multiClickMapper == 3)
{
_terminal->MultiClickSelection(terminalPosition, ::Terminal::SelectionExpansionMode::Line);
_selectionUpdatedThisCycle = true;
}
else if (multiClickMapper == 2)
{
_terminal->MultiClickSelection(terminalPosition, ::Terminal::SelectionExpansionMode::Word);
_selectionUpdatedThisCycle = true;
}
else
{
if (shiftEnabled && _terminal->IsSelectionActive())
{
_terminal->SetSelectionEnd(terminalPosition, ::Terminal::SelectionExpansionMode::Cell);
_selectionUpdatedThisCycle = true;
}
else
{
_terminal->SetSelectionAnchor(terminalPosition);
// A single click down resets the selection and begins a new one.
_terminal->ClearSelection();
_singleClickTouchdownPos = cursorPosition;
_selectionUpdatedThisCycle = false; // there's no selection, so there's nothing to update
}

_lastMouseClickTimestamp = point.Timestamp();
Expand All @@ -980,7 +975,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
else if (point.Properties().IsRightButtonPressed())
{
// CopyOnSelect right click always pastes
if (_terminal->IsCopyOnSelectActive() || !_terminal->IsSelectionActive())
if (!_selectionUpdatedThisCycle || !_terminal->IsSelectionActive())
{
PasteTextFromClipboard();
}
Expand Down Expand Up @@ -1028,16 +1023,22 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation

if (point.Properties().IsLeftButtonPressed())
{
_clickDrag = true;
const auto cursorPosition = point.Position();

// PointerPressedHandler doesn't set the SelectionAnchor when the click was
// from out of focus, so PointerMoved has to set it.
if (_focusRaisedClickPos)
if (_singleClickTouchdownPos)
{
_terminal->SetSelectionAnchor(_GetTerminalPosition(*_focusRaisedClickPos));
// Figure out if the user's moved a quarter of a cell's smaller axis away from the clickdown point
auto& touchdownPoint{ *_singleClickTouchdownPos };
auto distance{ std::sqrtf(std::powf(cursorPosition.X - touchdownPoint.X, 2) + std::powf(cursorPosition.Y - touchdownPoint.Y, 2)) };
const auto fontSize{ _actualFont.GetSize() };
if (distance >= (std::min(fontSize.X, fontSize.Y) / 4.f))
{
_terminal->SetSelectionAnchor(_GetTerminalPosition(touchdownPoint));
// stop tracking the touchdown point
_singleClickTouchdownPos = std::nullopt;
}
}

const auto cursorPosition = point.Position();
_SetEndSelectionPointAtCursor(cursorPosition);

const double cursorBelowBottomDist = cursorPosition.Y - SwapChainPanel().Margin().Top - SwapChainPanel().ActualHeight();
Expand Down Expand Up @@ -1121,17 +1122,14 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation

// Only a left click release when copy on select is active should perform a copy.
// Right clicks and middle clicks should not need to do anything when released.
if (_terminal->IsCopyOnSelectActive() && point.Properties().PointerUpdateKind() == Windows::UI::Input::PointerUpdateKind::LeftButtonReleased)
if (_settings.CopyOnSelect() && point.Properties().PointerUpdateKind() == Windows::UI::Input::PointerUpdateKind::LeftButtonReleased)
{
const auto modifiers = static_cast<uint32_t>(args.KeyModifiers());
// static_cast to a uint32_t because we can't use the WI_IsFlagSet
// macro directly with a VirtualKeyModifiers
const auto shiftEnabled = WI_IsFlagSet(modifiers, static_cast<uint32_t>(VirtualKeyModifiers::Shift));

// In a Copy on Select scenario,
// All left click drags should copy,
// All left clicks on a focused control should copy if a selection is active.
if (_clickDrag || !_focusRaisedClickPos)
if (_selectionUpdatedThisCycle)
{
CopySelectionToClipboard(shiftEnabled);
}
Expand All @@ -1142,8 +1140,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
_touchAnchor = std::nullopt;
}

_focusRaisedClickPos = std::nullopt;
_clickDrag = false;
_singleClickTouchdownPos = std::nullopt;

_TryStopAutoScroll(ptr.PointerId());

Expand Down Expand Up @@ -1649,6 +1646,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
// save location (for rendering) + render
_terminal->SetSelectionEnd(terminalPosition);
_renderer->TriggerSelection();
_selectionUpdatedThisCycle = true;
}

// Method Description:
Expand Down Expand Up @@ -1799,6 +1797,10 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
{
return false;
}

// Mark the current selection as copied
_selectionUpdatedThisCycle = false;

// extract text from buffer
const auto bufferData = _terminal->RetrieveSelectedTextFromBuffer(collapseText);

Expand All @@ -1822,7 +1824,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
_actualFont.GetFaceName(),
_settings.DefaultBackground());

if (!_terminal->IsCopyOnSelectActive())
if (!_settings.CopyOnSelect())
{
_terminal->ClearSelection();
_renderer->TriggerSelection();
Expand Down
7 changes: 3 additions & 4 deletions src/cascadia/TerminalControl/TermControl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,11 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
// imported from WinUser
// Used for PointerPoint.Timestamp Property (https://docs.microsoft.com/en-us/uwp/api/windows.ui.input.pointerpoint.timestamp#Windows_UI_Input_PointerPoint_Timestamp)
Timestamp _multiClickTimer;
Timestamp _lastMouseClickTimestamp;
unsigned int _multiClickCounter;
Timestamp _lastMouseClickTimestamp;
std::optional<winrt::Windows::Foundation::Point> _lastMouseClickPos;

std::optional<winrt::Windows::Foundation::Point> _focusRaisedClickPos;
bool _clickDrag;
std::optional<winrt::Windows::Foundation::Point> _singleClickTouchdownPos;
bool _selectionUpdatedThisCycle;

winrt::Windows::UI::Xaml::Controls::SwapChainPanel::LayoutUpdated_revoker _layoutUpdatedRevoker;

Expand Down
6 changes: 1 addition & 5 deletions src/cascadia/TerminalCore/Terminal.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@ Terminal::Terminal() :
_scrollOffset{ 0 },
_snapOnInput{ true },
_blockSelection{ false },
_selection{ std::nullopt },
_allowSingleCharSelection{ true },
_copyOnSelect{ false }
_selection{ std::nullopt }
{
auto dispatch = std::make_unique<TerminalDispatch>(*this);
auto engine = std::make_unique<OutputStateMachineEngine>(std::move(dispatch));
Expand Down Expand Up @@ -145,8 +143,6 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting

_wordDelimiters = settings.WordDelimiters();

_copyOnSelect = settings.CopyOnSelect();

_suppressApplicationTitle = settings.SuppressApplicationTitle();

_startingTitle = settings.StartingTitle();
Expand Down
4 changes: 0 additions & 4 deletions src/cascadia/TerminalCore/Terminal.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ class Microsoft::Terminal::Core::Terminal final :
Word,
Line
};
const bool IsCopyOnSelectActive() const noexcept;
void MultiClickSelection(const COORD viewportPos, SelectionExpansionMode expansionMode);
void SetSelectionAnchor(const COORD position);
void SetSelectionEnd(const COORD position, std::optional<SelectionExpansionMode> newExpansionMode = std::nullopt);
Expand Down Expand Up @@ -222,8 +221,6 @@ class Microsoft::Terminal::Core::Terminal final :
};
std::optional<SelectionAnchors> _selection;
bool _blockSelection;
bool _allowSingleCharSelection;
bool _copyOnSelect;
std::wstring _wordDelimiters;
SelectionExpansionMode _multiClickSelectionMode;
#pragma endregion
Expand Down Expand Up @@ -275,7 +272,6 @@ class Microsoft::Terminal::Core::Terminal final :
std::pair<COORD, COORD> _PivotSelection(const COORD targetPos) const;
std::pair<COORD, COORD> _ExpandSelectionAnchors(std::pair<COORD, COORD> anchors) const;
COORD _ConvertToBufferCell(const COORD viewportPos) const;
const bool _IsSingleCellSelection() const noexcept;
#pragma endregion

#ifdef UNIT_TESTING
Expand Down
34 changes: 0 additions & 34 deletions src/cascadia/TerminalCore/TerminalSelection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,27 +82,12 @@ const COORD Terminal::GetSelectionEnd() const noexcept
return _selection->end;
}

// Method Description:
// - Checks if selection is on a single cell
// Return Value:
// - bool representing if selection is only a single cell. Used for copyOnSelect
const bool Terminal::_IsSingleCellSelection() const noexcept
{
return (_selection->start == _selection->end);
}

// Method Description:
// - Checks if selection is active
// Return Value:
// - bool representing if selection is active. Used to decide copy/paste on right click
const bool Terminal::IsSelectionActive() const noexcept
{
// A single cell selection is not considered an active selection,
// if it's not allowed
if (!_allowSingleCharSelection && _IsSingleCellSelection())
{
return false;
}
return _selection.has_value();
}

Expand All @@ -111,15 +96,6 @@ const bool Terminal::IsBlockSelection() const noexcept
return _blockSelection;
}

// Method Description:
// - Checks if the CopyOnSelect setting is active
// Return Value:
// - true if feature is active, false otherwise.
const bool Terminal::IsCopyOnSelectActive() const noexcept
{
return _copyOnSelect;
}

// Method Description:
// - Perform a multi-click selection at viewportPos expanding according to the expansionMode
// Arguments:
Expand Down Expand Up @@ -148,8 +124,6 @@ void Terminal::SetSelectionAnchor(const COORD viewportPos)
_selection = SelectionAnchors{};
_selection->pivot = _ConvertToBufferCell(viewportPos);

_allowSingleCharSelection = (_copyOnSelect) ? false : true;

_multiClickSelectionMode = SelectionExpansionMode::Cell;
SetSelectionEnd(viewportPos);

Expand All @@ -172,13 +146,6 @@ void Terminal::SetSelectionEnd(const COORD viewportPos, std::optional<SelectionE

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

// moving the endpoint of what used to be a single cell selection
// allows the user to drag back and select just one cell
if (_copyOnSelect && !_IsSingleCellSelection())
{
_allowSingleCharSelection = true;
}
}

// Method Description:
Expand Down Expand Up @@ -248,7 +215,6 @@ void Terminal::SetBlockSelection(const bool isEnabled) noexcept
#pragma warning(disable : 26440) // changing this to noexcept would require a change to ConHost's selection model
void Terminal::ClearSelection()
{
_allowSingleCharSelection = false;
_selection = std::nullopt;
}

Expand Down

0 comments on commit 53080d3

Please sign in to comment.