HAX: try to make selection exclusive

still has issues around block, word pivot

src/buffer/out/textBuffer.cpp

@@ -1149,25 +1149,21 @@ const COORD TextBuffer::_GetWordEndForSelection(const COORD target, const std::w
 {
     const auto bufferSize = GetSize();
 
-    // can't expand right
-    if (target.X == bufferSize.RightInclusive())
-    {
-        return target;
-    }
-
     COORD result = target;
+    if (!bufferSize.DecrementInBounds(result, true))
+    {
+        // We can't walk backwards; return as-is
+        return result;
+    }
+    // End was exclusive; walk it back one to get the text
     const auto initialDelimiter = _GetDelimiterClassAt(result, wordDelimiters);
 
-    // expand right until we hit the right boundary or a different delimiter class
-    while (result.X < bufferSize.RightInclusive() && (_GetDelimiterClassAt(result, wordDelimiters) == initialDelimiter))
+    while (_GetDelimiterClassAt(result, wordDelimiters) == initialDelimiter)
     {
-        bufferSize.IncrementInBounds(result);
-    }
-
-    if (_GetDelimiterClassAt(result, wordDelimiters) != initialDelimiter)
-    {
-        // move off of delimiter
-        bufferSize.DecrementInBounds(result);
+        if (!bufferSize.IncrementInBounds(result, true))
+        {
+            break;
+        }
     }
 
     return result;

src/cascadia/TerminalControl/TermControl.cpp

@@ -69,9 +69,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         _cursorTimer{},
         _lastMouseClickTimestamp{},
         _lastMouseClickPos{},
-        _searchBox{ nullptr },
-        _focusRaisedClickPos{ std::nullopt },
-        _clickDrag{ false }
+        _selectionNeedsToBeCopied{ false },
+        _searchBox{ nullptr }
     {
         _EnsureStaticInitialization();
         InitializeComponent();
@@ -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)
@@ -933,15 +929,8 @@ 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);
+                const auto terminalPosition = _GetTerminalPosition(cursorPosition, true);
 
                 // handle ALT key
                 _terminal->SetBlockSelection(altEnabled);
@@ -956,20 +945,26 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
                 if (multiClickMapper == 3)
                 {
                     _terminal->MultiClickSelection(terminalPosition, ::Terminal::SelectionExpansionMode::Line);
+                    _selectionNeedsToBeCopied = true;
                 }
                 else if (multiClickMapper == 2)
                 {
                     _terminal->MultiClickSelection(terminalPosition, ::Terminal::SelectionExpansionMode::Word);
+                    _selectionNeedsToBeCopied = true;
                 }
                 else
                 {
                     if (shiftEnabled && _terminal->IsSelectionActive())
                     {
                         _terminal->SetSelectionEnd(terminalPosition, ::Terminal::SelectionExpansionMode::Cell);
+                        _selectionNeedsToBeCopied = true;
                     }
                     else
                     {
-                        _terminal->SetSelectionAnchor(terminalPosition);
+                        // 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();
@@ -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 (_settings.CopyOnSelect() || !_terminal->IsSelectionActive())
                 {
                     PasteTextFromClipboard();
                 }
@@ -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, true));
+                        // 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();
@@ -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 (_selectionNeedsToBeCopied)
                 {
                     CopySelectionToClipboard(shiftEnabled);
                 }
@@ -1142,8 +1140,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
             _touchAnchor = std::nullopt;
         }
 
-        _focusRaisedClickPos = std::nullopt;
-        _clickDrag = false;
+        _singleClickTouchdownPos = std::nullopt;
 
         _TryStopAutoScroll(ptr.PointerId());
 
@@ -1638,7 +1635,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
     // - cursorPosition: in pixels, relative to the origin of the control
     void TermControl::_SetEndSelectionPointAtCursor(Windows::Foundation::Point const& cursorPosition)
     {
-        auto terminalPosition = _GetTerminalPosition(cursorPosition);
+        auto terminalPosition = _GetTerminalPosition(cursorPosition, true);
 
         const short lastVisibleRow = std::max<short>(_terminal->GetViewport().Height() - 1, 0);
         const short lastVisibleCol = std::max<short>(_terminal->GetViewport().Width() - 1, 0);
@@ -1649,6 +1646,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         // save location (for rendering) + render
         _terminal->SetSelectionEnd(terminalPosition);
         _renderer->TriggerSelection();
+        _selectionNeedsToBeCopied = true;
     }
 
     // Method Description:
@@ -1799,6 +1797,10 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         {
             return false;
         }
+
+        // Mark the current selection as copied
+        _selectionNeedsToBeCopied = false;
+
         // extract text from buffer
         const auto bufferData = _terminal->RetrieveSelectedTextFromBuffer(collapseText);
 
@@ -1822,7 +1824,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
                                                 _actualFont.GetFaceName(),
                                                 _settings.DefaultBackground());
 
-        if (!_terminal->IsCopyOnSelectActive())
+        if (!_settings.CopyOnSelect())
         {
             _terminal->ClearSelection();
             _renderer->TriggerSelection();
@@ -2165,12 +2167,12 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
     //    NOTE: origin (0,0) is top-left.
     // Return Value:
     // - the corresponding viewport terminal position for the given Point parameter
-    const COORD TermControl::_GetTerminalPosition(winrt::Windows::Foundation::Point cursorPosition)
+    const COORD TermControl::_GetTerminalPosition(winrt::Windows::Foundation::Point cursorPosition, bool roundedForSelection)
     {
         // Exclude padding from cursor position calculation
-        COORD terminalPosition = {
-            static_cast<SHORT>(cursorPosition.X - SwapChainPanel().Margin().Left),
-            static_cast<SHORT>(cursorPosition.Y - SwapChainPanel().Margin().Top)
+        winrt::Windows::Foundation::Point terminalPosition = {
+            cursorPosition.X - gsl::narrow_cast<float>(SwapChainPanel().Margin().Left),
+            cursorPosition.Y - gsl::narrow_cast<float>(SwapChainPanel().Margin().Top)
         };
 
         const auto fontSize = _actualFont.GetSize();
@@ -2178,10 +2180,18 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         FAIL_FAST_IF(fontSize.Y == 0);
 
         // Normalize to terminal coordinates by using font size
-        terminalPosition.X /= fontSize.X;
-        terminalPosition.Y /= fontSize.Y;
+        if (roundedForSelection)
+        {
+            return COORD{
+                ::base::ClampedNumeric<SHORT>(std::roundf(terminalPosition.X / fontSize.X)),
+                ::base::ClampedNumeric<SHORT>(terminalPosition.Y / fontSize.Y)
+            };
+        }
 
-        return terminalPosition;
+        return COORD{
+            ::base::ClampedNumeric<SHORT>(terminalPosition.X / fontSize.X),
+            ::base::ClampedNumeric<SHORT>(terminalPosition.Y / fontSize.Y)
+        };
     }
 
     // Method Description:

src/cascadia/TerminalControl/TermControl.h

@@ -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 _selectionNeedsToBeCopied;
 
         winrt::Windows::UI::Xaml::Controls::SwapChainPanel::LayoutUpdated_revoker _layoutUpdatedRevoker;
 
@@ -214,7 +213,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         bool _TrySendMouseEvent(Windows::UI::Input::PointerPoint const& point);
         bool _CanSendVTMouseInput();
 
-        const COORD _GetTerminalPosition(winrt::Windows::Foundation::Point cursorPosition);
+        const COORD _GetTerminalPosition(winrt::Windows::Foundation::Point cursorPosition, bool roundedForSelection = false);
         const unsigned int _NumberOfClicks(winrt::Windows::Foundation::Point clickPos, Timestamp clickTime);
         double _GetAutoScrollSpeed(double cursorDistanceFromBorder) const;
 

src/cascadia/TerminalCore/Terminal.cpp

@@ -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));
@@ -145,8 +143,6 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting
 
     _wordDelimiters = settings.WordDelimiters();
 
-    _copyOnSelect = settings.CopyOnSelect();
-
     _suppressApplicationTitle = settings.SuppressApplicationTitle();
 
     _startingTitle = settings.StartingTitle();

src/cascadia/TerminalCore/Terminal.hpp

@@ -181,7 +181,6 @@ public:
         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);
@@ -222,8 +221,6 @@ private:
     };
     std::optional<SelectionAnchors> _selection;
     bool _blockSelection;
-    bool _allowSingleCharSelection;
-    bool _copyOnSelect;
     std::wstring _wordDelimiters;
     SelectionExpansionMode _multiClickSelectionMode;
 #pragma endregion
@@ -275,7 +272,6 @@ private:
     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

src/cascadia/TerminalCore/TerminalSelection.cpp

@@ -54,7 +54,9 @@ std::vector<SMALL_RECT> Terminal::_GetSelectionRects() const noexcept
 
     try
     {
-        return _buffer->GetTextRects(_selection->start, _selection->end, _blockSelection);
+        auto inclusiveEnd{ _selection->end };
+        _buffer->GetSize().DecrementInBounds(inclusiveEnd, true);
+        return _buffer->GetTextRects(_selection->start, inclusiveEnd, _blockSelection);
     }
     CATCH_LOG();
     return result;
@@ -82,28 +84,13 @@ 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();
+    return _selection.has_value() && _selection->start != _selection->end;
 }
 
 const bool Terminal::IsBlockSelection() const noexcept
@@ -111,15 +98,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:
@@ -130,6 +108,7 @@ void Terminal::MultiClickSelection(const COORD viewportPos, SelectionExpansionMo
     // set the selection pivot to expand the selection using SetSelectionEnd()
     _selection = SelectionAnchors{};
     _selection->pivot = _ConvertToBufferCell(viewportPos);
+    _buffer->GetSize().DecrementInBounds(_selection->pivot, true);
 
     _multiClickSelectionMode = expansionMode;
     SetSelectionEnd(viewportPos);
@@ -148,8 +127,6 @@ void Terminal::SetSelectionAnchor(const COORD viewportPos)
     _selection = SelectionAnchors{};
     _selection->pivot = _ConvertToBufferCell(viewportPos);
 
-    _allowSingleCharSelection = (_copyOnSelect) ? false : true;
-
     _multiClickSelectionMode = SelectionExpansionMode::Cell;
     SetSelectionEnd(viewportPos);
 
@@ -172,13 +149,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:
@@ -190,18 +160,30 @@ void Terminal::SetSelectionEnd(const COORD viewportPos, std::optional<SelectionE
 // - the new start/end for a selection
 std::pair<COORD, COORD> Terminal::_PivotSelection(const COORD targetPos) const
 {
-    if (_buffer->GetSize().CompareInBounds(targetPos, _selection->pivot) <= 0)
+    const auto bufferSize = _buffer->GetSize();
+    const auto compareResult = bufferSize.CompareInBounds(targetPos, _selection->pivot, true);
+    if (compareResult < 0)
     {
-        // target is before pivot
-        // treat target as start
+// target is before pivot
+// treat target as start
+// the targetPos was the end, but now we're making it the start: swap their exclusivities
+#if 0
+        auto newInclusiveStart{ targetPos };
+        auto newExclusiveEnd{ _selection->pivot };
+        _buffer->GetSize().DecrementInBounds(newInclusiveStart, true);
+        _buffer->GetSize().IncrementInBounds(newExclusiveEnd, true);
+#endif
         return std::make_pair(targetPos, _selection->pivot);
     }
-    else
+    else if (compareResult > 0)
     {
         // target is after pivot
         // treat pivot as start
         return std::make_pair(_selection->pivot, targetPos);
     }
+
+    // If they were the same point, doesn't matter what we return
+    return std::make_pair(targetPos, targetPos);
 }
 
 // Method Description:
@@ -220,7 +202,7 @@ std::pair<COORD, COORD> Terminal::_ExpandSelectionAnchors(std::pair<COORD, COORD
     {
     case SelectionExpansionMode::Line:
         start = { bufferSize.Left(), start.Y };
-        end = { bufferSize.RightInclusive(), end.Y };
+        end = { 0, end.Y + 1 };
         break;
     case SelectionExpansionMode::Word:
         start = _buffer->GetWordStart(start, _wordDelimiters);
@@ -248,7 +230,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;
 }
 

src/cascadia/UnitTests_TerminalCore/SelectionTest.cpp

@@ -672,49 +672,5 @@ namespace TerminalCoreUnitTests
             selection = term.GetViewport().ConvertToOrigin(selectionRects.at(1)).ToInclusive();
             VERIFY_ARE_EQUAL(selection, SMALL_RECT({ 0, 11, 99, 11 }));
         }
-
-        TEST_METHOD(CopyOnSelect)
-        {
-            Terminal term;
-            DummyRenderTarget emptyRT;
-            term.Create({ 100, 100 }, 0, emptyRT);
-
-            // set copyOnSelect for terminal
-            auto settings = winrt::make<MockTermSettings>(0, 100, 100);
-            settings.CopyOnSelect(true);
-            term.UpdateSettings(settings);
-
-            // Simulate click at (x,y) = (5,10)
-            term.SetSelectionAnchor({ 5, 10 });
-
-            // Simulate move to (x,y) = (5,10)
-            // (So, no movement)
-            term.SetSelectionEnd({ 5, 10 });
-
-            // Case 1: single cell selection not allowed
-            {
-                // Simulate renderer calling TriggerSelection and acquiring selection area
-                auto selectionRects = term.GetSelectionRects();
-
-                // Validate selection area
-                VERIFY_ARE_EQUAL(selectionRects.size(), static_cast<size_t>(0));
-
-                // single cell selection should not be allowed
-                // thus, selection is NOT active
-                VERIFY_IS_FALSE(term.IsSelectionActive());
-            }
-
-            // Case 2: move off of single cell
-            term.SetSelectionEnd({ 6, 10 });
-            ValidateSingleRowSelection(term, { 5, 10, 6, 10 });
-            VERIFY_IS_TRUE(term.IsSelectionActive());
-
-            // Case 3: move back onto single cell (now allowed)
-            term.SetSelectionEnd({ 5, 10 });
-            ValidateSingleRowSelection(term, { 5, 10, 5, 10 });
-
-            // single cell selection should now be allowed
-            VERIFY_IS_TRUE(term.IsSelectionActive());
-        }
     };
 }