Miscellaneous bug fixes for Mark Mode

doc/cascadia/profiles.schema.json

@@ -350,6 +350,7 @@
         "switchToTab",
         "tabSearch",
         "toggleAlwaysOnTop",
+        "toggleBlockSelection",
         "toggleFocusMode",
         "selectAll",
         "setFocusMode",

src/cascadia/TerminalControl/ControlCore.cpp

@@ -417,11 +417,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             vkey != VK_SNAPSHOT &&
             keyDown)
         {
-            if (_terminal->IsInMarkMode() && modifiers.IsCtrlPressed() && vkey == 'A')
+            const auto isInMarkMode = _terminal->SelectionMode() == ::Microsoft::Terminal::Core::Terminal::SelectionInteractionMode::Mark;
+            if (isInMarkMode && modifiers.IsCtrlPressed() && vkey == 'A')
             {
                 auto lock = _terminal->LockForWriting();
                 _terminal->SelectAll();
-                _updateSelection();
+                _updateSelectionUI(true);
                 return true;
             }
 
@@ -430,15 +431,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             {
                 auto lock = _terminal->LockForWriting();
                 _terminal->UpdateSelection(updateSlnParams->first, updateSlnParams->second, modifiers);
-                _updateSelection();
+                _updateSelectionUI(true);
                 return true;
             }
 
             // GH#8791 - don't dismiss selection if Windows key was also pressed as a key-combination.
             if (!modifiers.IsWinPressed())
             {
                 _terminal->ClearSelection();
-                _updateSelection();
+                _updateSelectionUI(false);
             }
 
             // When there is a selection active, escape should clear it and NOT flow through
@@ -950,8 +951,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         const auto end{ _terminal->SelectionEndForRendering() };
         info.EndPos = { end.X, end.Y };
 
-        info.MovingEnd = _terminal->MovingEnd();
-        info.MovingCursor = _terminal->MovingCursor();
+        info.Endpoint = static_cast<SelectionEndpointTarget>(_terminal->SelectionEndpointTarget());
 
         const auto bufferSize{ _terminal->GetTextBuffer().GetSize() };
         info.StartAtLeftBoundary = _terminal->GetSelectionAnchor().x == bufferSize.Left();
@@ -981,11 +981,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
         // save location (for rendering) + render
         _terminal->SetSelectionEnd(terminalPosition);
-        _renderer->TriggerSelection();
 
         // this is used for mouse dragging,
         // so hide the markers
-        _UpdateSelectionMarkersHandlers(*this, winrt::make<implementation::UpdateSelectionMarkersEventArgs>(true));
+        _updateSelectionUI(false);
     }
 
     // Called when the Terminal wants to set something to the clipboard, i.e.
@@ -998,7 +997,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     // Method Description:
     // - Given a copy-able selection, get the selected text from the buffer and send it to the
     //     Windows Clipboard (CascadiaWin32:main.cpp).
-    // - CopyOnSelect does NOT clear the selection
     // Arguments:
     // - singleLine: collapse all of the text to one line
     // - formats: which formats to copy (defined by action's CopyFormatting arg). nullptr
@@ -1044,12 +1042,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                                                     bgColor) :
                                  "";
 
-        if (!_settings->CopyOnSelect())
-        {
-            _terminal->ClearSelection();
-            _updateSelection();
-        }
-
         // send data up for clipboard
         _CopyToClipboardHandlers(*this,
                                  winrt::make<CopyToClipboardEventArgs>(winrt::hstring{ textData },
@@ -1063,7 +1055,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     {
         auto lock = _terminal->LockForWriting();
         _terminal->SelectAll();
-        _updateSelection();
+        _updateSelectionUI(true);
+    }
+
+    void ControlCore::ClearSelection()
+    {
+        auto lock = _terminal->LockForWriting();
+        _terminal->ClearSelection();
+        _updateSelectionUI(false);
     }
 
     bool ControlCore::ToggleBlockSelection()
@@ -1085,12 +1084,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     {
         auto lock = _terminal->LockForWriting();
         _terminal->ToggleMarkMode();
-        _updateSelection();
+        _updateSelectionUI(true);
     }
 
-    bool ControlCore::IsInMarkMode() const
+    Control::SelectionInteractionMode ControlCore::SelectionMode() const
     {
-        return _terminal->IsInMarkMode();
+        return static_cast<Control::SelectionInteractionMode>(_terminal->SelectionMode());
     }
 
     // Method Description:
@@ -1100,7 +1099,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     {
         _terminal->WritePastedText(hstr);
         _terminal->ClearSelection();
-        _updateSelection();
+        _updateSelectionUI(false);
         _terminal->TrySnapOnInput();
     }
 
@@ -1420,11 +1419,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         {
             _terminal->SetBlockSelection(false);
             search.Select();
-            _renderer->TriggerSelection();
 
             // this is used for search,
             // so hide the markers
-            _UpdateSelectionMarkersHandlers(*this, winrt::make<implementation::UpdateSelectionMarkersEventArgs>(true));
+            _updateSelectionUI(false);
         }
 
         // Raise a FoundMatch event, which the control will use to notify
@@ -1625,14 +1623,22 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             _terminal->MultiClickSelection(terminalPosition, mode);
             selectionNeedsToBeCopied = true;
         }
-        _updateSelection();
+        // this is used for mouse selection,
+        // so hide the markers
+        _updateSelectionUI(false);
     }
 
-    void ControlCore::_updateSelection()
+    // Method Description:
+    // - Updates the renderer's representation of the selection as well as the selection marker overlay in TermControl
+    // Arguments:
+    // - tryShowMarkers: if true, show the selection marker overlay. Otherwise hide it.
+    void ControlCore::_updateSelectionUI(bool tryShowMarkers)
     {
         _renderer->TriggerSelection();
-        const bool clearMarkers{ !_terminal->IsSelectionActive() };
-        _UpdateSelectionMarkersHandlers(*this, winrt::make<implementation::UpdateSelectionMarkersEventArgs>(clearMarkers));
+        // if show markers and there's a selection, show the markers.
+        // otherwise, hide the markers (i.e. mouse selection, search, etc...)
+        const bool showMarkers{ _terminal->IsSelectionActive() && tryShowMarkers };
+        _UpdateSelectionMarkersHandlers(*this, winrt::make<implementation::UpdateSelectionMarkersEventArgs>(!showMarkers));
     }
 
     void ControlCore::AttachUiaEngine(::Microsoft::Console::Render::IRenderEngine* const pEngine)

src/cascadia/TerminalControl/ControlCore.h

@@ -82,9 +82,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         void PasteText(const winrt::hstring& hstr);
         bool CopySelectionToClipboard(bool singleLine, const Windows::Foundation::IReference<CopyFormat>& formats);
         void SelectAll();
+        void ClearSelection();
         bool ToggleBlockSelection();
         void ToggleMarkMode();
-        bool IsInMarkMode() const;
+        Control::SelectionInteractionMode SelectionMode() const;
 
         void GotFocus();
         void LostFocus();
@@ -271,7 +272,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         bool _setFontSizeUnderLock(int fontSize);
         void _updateFont(const bool initialUpdate = false);
         void _refreshSizeUnderLock();
-        void _updateSelection();
+        void _updateSelectionUI(bool tryShowMarkers);
 
         void _sendInputToConnection(std::wstring_view wstr);
 

src/cascadia/TerminalControl/ControlCore.idl

@@ -29,12 +29,26 @@ namespace Microsoft.Terminal.Control
         All
     };
 
+    enum SelectionInteractionMode
+    {
+        None,
+        Mouse,
+        Keyboard,
+        Mark
+    };
+
+    [flags]
+    enum SelectionEndpointTarget
+    {
+        Start = 0x1,
+        End = 0x2
+    };
+
     struct SelectionData
     {
         Microsoft.Terminal.Core.Point StartPos;
         Microsoft.Terminal.Core.Point EndPos;
-        Boolean MovingEnd;
-        Boolean MovingCursor;
+        SelectionEndpointTarget Endpoint;
         Boolean StartAtLeftBoundary;
         Boolean EndAtRightBoundary;
     };
@@ -75,10 +89,10 @@ namespace Microsoft.Terminal.Control
         void SendInput(String text);
         void PasteText(String text);
         void SelectAll();
+        void ClearSelection();
         Boolean ToggleBlockSelection();
         void ToggleMarkMode();
         void ClearBuffer(ClearBufferType clearType);
-        Boolean IsInMarkMode();
 
         void SetHoveredCell(Microsoft.Terminal.Core.Point terminalPosition);
         void ClearHoveredCell();
@@ -101,6 +115,7 @@ namespace Microsoft.Terminal.Control
         Boolean HasSelection { get; };
         IVector<String> SelectedText(Boolean trimTrailingWhitespace);
         SelectionData SelectionInfo { get; };
+        SelectionInteractionMode SelectionMode();
 
         String HoveredUriText { get; };
         Windows.Foundation.IReference<Microsoft.Terminal.Core.Point> HoveredCell { get; };

src/cascadia/TerminalControl/ControlInteractivity.cpp

@@ -142,7 +142,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     // Method Description:
     // - Given a copy-able selection, get the selected text from the buffer and send it to the
     //     Windows Clipboard (CascadiaWin32:main.cpp).
-    // - CopyOnSelect does NOT clear the selection
     // Arguments:
     // - singleLine: collapse all of the text to one line
     // - formats: which formats to copy (defined by action's CopyFormatting arg). nullptr
@@ -257,15 +256,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         }
         else if (WI_IsFlagSet(buttonState, MouseButtonState::IsRightButtonDown))
         {
-            // CopyOnSelect right click always pastes
-            if (_core->CopyOnSelect() || !_core->HasSelection())
+            // Try to copy the text and clear the selection
+            const auto successfulCopy = CopySelectionToClipboard(shiftEnabled, nullptr);
+            _core->ClearSelection();
+            if (_core->CopyOnSelect() || !successfulCopy)
             {
+                // CopyOnSelect: right click always pastes!
+                // Otherwise: no selection --> paste
                 RequestPasteTextFromClipboard();
             }
-            else
-            {
-                CopySelectionToClipboard(shiftEnabled, nullptr);
-            }
         }
     }
 
@@ -383,6 +382,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             isLeftMouseRelease &&
             _selectionNeedsToBeCopied)
         {
+            // IMPORTANT!
+            // DO NOT clear the selection here!
+            // Otherwise, the selection will be cleared immediately after you make it.
             CopySelectionToClipboard(false, nullptr);
         }
 

src/cascadia/TerminalControl/TermControl.cpp

@@ -1188,7 +1188,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         {
             // Manually show the cursor when a key is pressed. Restarting
             // the timer prevents flickering.
-            _core.CursorOn(!_core.IsInMarkMode());
+            _core.CursorOn(_core.SelectionMode() != SelectionInteractionMode::Mark);
             _cursorTimer->Start();
         }
 
@@ -1659,7 +1659,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         if (_cursorTimer)
         {
             // When the terminal focuses, show the cursor immediately
-            _core.CursorOn(!_core.IsInMarkMode());
+            _core.CursorOn(_core.SelectionMode() != SelectionInteractionMode::Mark);
             _cursorTimer->Start();
         }
 
@@ -1902,7 +1902,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             return false;
         }
 
-        return _interactivity.CopySelectionToClipboard(singleLine, formats);
+        const auto successfulCopy = _interactivity.CopySelectionToClipboard(singleLine, formats);
+        _core.ClearSelection();
+        return successfulCopy;
     }
 
     // Method Description:
@@ -2830,9 +2832,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
                 // show/update selection markers
                 // figure out which endpoint to move, get it and the relevant icon (hide the other icon)
-                const auto selectionAnchor{ markerData.MovingEnd ? markerData.EndPos : markerData.StartPos };
-                const auto& marker{ markerData.MovingEnd ? SelectionEndMarker() : SelectionStartMarker() };
-                const auto& otherMarker{ markerData.MovingEnd ? SelectionStartMarker() : SelectionEndMarker() };
+                const auto movingEnd{ WI_IsFlagSet(markerData.Endpoint, SelectionEndpointTarget::End) };
+                const auto selectionAnchor{ movingEnd ? markerData.EndPos : markerData.StartPos };
+                const auto& marker{ movingEnd ? SelectionEndMarker() : SelectionStartMarker() };
+                const auto& otherMarker{ movingEnd ? SelectionStartMarker() : SelectionEndMarker() };
                 if (selectionAnchor.Y < 0 || selectionAnchor.Y >= _core.ViewHeight())
                 {
                     // if the endpoint is outside of the viewport,
@@ -2841,7 +2844,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                     otherMarker.Visibility(Visibility::Collapsed);
                     co_return;
                 }
-                else if (markerData.MovingCursor)
+                else if (WI_AreAllFlagsSet(markerData.Endpoint, SelectionEndpointTarget::Start | SelectionEndpointTarget::End))
                 {
                     // display both markers
                     displayMarker(true);
@@ -2851,7 +2854,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                 {
                     // display one marker,
                     // but hide the other
-                    displayMarker(markerData.MovingEnd);
+                    displayMarker(movingEnd);
                     otherMarker.Visibility(Visibility::Collapsed);
                 }
             }

src/cascadia/TerminalCore/Terminal.cpp

@@ -45,8 +45,9 @@ Terminal::Terminal() :
     _snapOnInput{ true },
     _altGrAliasing{ true },
     _blockSelection{ false },
-    _markMode{ false },
+    _selectionMode{ SelectionInteractionMode::None },
     _selection{ std::nullopt },
+    _selectionEndpoint{ static_cast<SelectionEndpoint>(0) },
     _taskbarState{ 0 },
     _taskbarProgress{ 0 },
     _trimBlockSelection{ false },
@@ -1369,7 +1370,7 @@ void Terminal::SetCursorOn(const bool isOn)
 bool Terminal::IsCursorBlinkingAllowed() const noexcept
 {
     const auto& cursor = _activeBuffer().GetCursor();
-    return !_markMode && cursor.IsBlinkingAllowed();
+    return _selectionMode != SelectionInteractionMode::Mark && cursor.IsBlinkingAllowed();
 }
 
 // Method Description: