Miscellaneous bug fixes for Mark Mode

src/cascadia/TerminalControl/ControlCore.cpp

@@ -422,7 +422,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             {
                 auto lock = _terminal->LockForWriting();
                 _terminal->SelectAll();
-                _updateSelectionUI();
+                _updateSelectionUI(true);
                 return true;
             }
 
@@ -431,15 +431,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             {
                 auto lock = _terminal->LockForWriting();
                 _terminal->UpdateSelection(updateSlnParams->first, updateSlnParams->second, modifiers);
-                _updateSelectionUI();
+                _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();
-                _updateSelectionUI();
+                _updateSelectionUI(false);
             }
 
             // When there is a selection active, escape should clear it and NOT flow through
@@ -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.
@@ -1002,10 +1001,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     // - singleLine: collapse all of the text to one line
     // - formats: which formats to copy (defined by action's CopyFormatting arg). nullptr
     //             if we should defer which formats are copied to the global setting
-    // - clearSelection: if true, clear the selection. Used for CopyOnSelect.
     bool ControlCore::CopySelectionToClipboard(bool singleLine,
-                                               const Windows::Foundation::IReference<CopyFormat>& formats,
-                                               bool clearSelection)
+                                               const Windows::Foundation::IReference<CopyFormat>& formats)
     {
         // no selection --> nothing to copy
         if (!_terminal->IsSelectionActive())
@@ -1045,12 +1042,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                                                     bgColor) :
                                  "";
 
-        if (clearSelection)
-        {
-            _terminal->ClearSelection();
-            _updateSelectionUI();
-        }
-
         // send data up for clipboard
         _CopyToClipboardHandlers(*this,
                                  winrt::make<CopyToClipboardEventArgs>(winrt::hstring{ textData },
@@ -1064,7 +1055,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     {
         auto lock = _terminal->LockForWriting();
         _terminal->SelectAll();
-        _updateSelectionUI();
+        _updateSelectionUI(true);
+    }
+
+    void ControlCore::ClearSelection()
+    {
+        auto lock = _terminal->LockForWriting();
+        _terminal->ClearSelection();
+        _updateSelectionUI(false);
     }
 
     bool ControlCore::ToggleBlockSelection()
@@ -1086,7 +1084,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     {
         auto lock = _terminal->LockForWriting();
         _terminal->ToggleMarkMode();
-        _updateSelectionUI();
+        _updateSelectionUI(true);
     }
 
     Control::SelectionInteractionMode ControlCore::SelectionMode() const
@@ -1101,7 +1099,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     {
         _terminal->WritePastedText(hstr);
         _terminal->ClearSelection();
-        _updateSelectionUI();
+        _updateSelectionUI(false);
         _terminal->TrySnapOnInput();
     }
 
@@ -1421,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
@@ -1626,18 +1623,22 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             _terminal->MultiClickSelection(terminalPosition, mode);
             selectionNeedsToBeCopied = true;
         }
-        _renderer->TriggerSelection();
-
         // this is used for mouse selection,
         // so hide the markers
-        _UpdateSelectionMarkersHandlers(*this, winrt::make<implementation::UpdateSelectionMarkersEventArgs>(true));
+        _updateSelectionUI(false);
     }
 
-    void ControlCore::_updateSelectionUI()
+    // 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

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

src/cascadia/TerminalControl/ControlCore.idl

@@ -89,6 +89,7 @@ namespace Microsoft.Terminal.Control
         void SendInput(String text);
         void PasteText(String text);
         void SelectAll();
+        void ClearSelection();
         Boolean ToggleBlockSelection();
         void ToggleMarkMode();
         void ClearBuffer(ClearBufferType clearType);

src/cascadia/TerminalControl/ControlInteractivity.cpp

@@ -146,10 +146,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     // - singleLine: collapse all of the text to one line
     // - formats: which formats to copy (defined by action's CopyFormatting arg). nullptr
     //             if we should defer which formats are copied to the global setting
-    // - clearSelection: if true, clear the selection after copying it. Used for CopyOnSelect.
     bool ControlInteractivity::CopySelectionToClipboard(bool singleLine,
-                                                        const Windows::Foundation::IReference<CopyFormat>& formats,
-                                                        bool clearSelection)
+                                                        const Windows::Foundation::IReference<CopyFormat>& formats)
     {
         if (_core)
         {
@@ -165,7 +163,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             // Mark the current selection as copied
             _selectionNeedsToBeCopied = false;
 
-            return _core->CopySelectionToClipboard(singleLine, formats, clearSelection);
+            return _core->CopySelectionToClipboard(singleLine, formats);
         }
 
         return false;
@@ -258,15 +256,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         }
         else if (WI_IsFlagSet(buttonState, MouseButtonState::IsRightButtonDown))
         {
-            // CopySelectionToClipboard() clears the selection.
-            // So we need to keep track of the state before copying it.
-            const auto initiallyHadSelection = _core->HasSelection();
-            if (initiallyHadSelection)
-            {
-                // copy selected text
-                CopySelectionToClipboard(shiftEnabled, nullptr);
-            }
-            if (_core->CopyOnSelect() || !initiallyHadSelection)
+            // 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
@@ -390,9 +383,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             _selectionNeedsToBeCopied)
         {
             // IMPORTANT!
-            // Set clearSelection to false here!
+            // DO NOT clear the selection here!
             // Otherwise, the selection will be cleared immediately after you make it.
-            CopySelectionToClipboard(false, nullptr, /*clearSelection*/ false);
+            CopySelectionToClipboard(false, nullptr);
         }
 
         _singleClickTouchdownPos = std::nullopt;

src/cascadia/TerminalControl/ControlInteractivity.h

@@ -80,8 +80,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 #pragma endregion
 
         bool CopySelectionToClipboard(bool singleLine,
-                                      const Windows::Foundation::IReference<CopyFormat>& formats,
-                                      bool clearSelection = true);
+                                      const Windows::Foundation::IReference<CopyFormat>& formats);
         void RequestPasteTextFromClipboard();
         void SetEndSelectionPoint(const Core::Point pixelPosition);
         bool ManglePathsForWsl();

src/cascadia/TerminalControl/TermControl.cpp

@@ -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: