Make Mark Mode keybindings precede custom keybindings (#13659)

This PR moves the key handling for mark mode into a helper method that is then called before an action/key binding is attempted.

Epic: #4993
Closes #13533

- Add custom keybinding to "down" arrow key
- in mark mode --> selection updates appropriately
- out of mark mode --> keybinding executed

Amended by DHowett to remove URL targeting features.

(cherry picked from commit 70313db)
Service-Card-Id: 84711521
Service-Version: 1.15

src/cascadia/TerminalControl/ControlCore.cpp

@@ -391,6 +391,55 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         return _terminal->SendCharEvent(ch, scanCode, modifiers);
     }
 
+    bool ControlCore::_shouldTryUpdateSelection(const WORD vkey)
+    {
+        // GH#6423 - don't update selection if the key that was pressed was a
+        // modifier key. We'll wait for a real keystroke to dismiss the
+        // GH #7395 - don't update selection when taking PrintScreen
+        // selection.
+        return HasSelection() && !KeyEvent::IsModifierKey(vkey) && vkey != VK_SNAPSHOT;
+    }
+
+    bool ControlCore::TryMarkModeKeybinding(const WORD vkey,
+                                            const ::Microsoft::Terminal::Core::ControlKeyStates mods)
+    {
+        if (_shouldTryUpdateSelection(vkey) && _terminal->SelectionMode() == ::Terminal::SelectionInteractionMode::Mark)
+        {
+            if (vkey == 'A' && !mods.IsAltPressed() && !mods.IsShiftPressed() && mods.IsCtrlPressed())
+            {
+                // Ctrl + A --> Select all
+                auto lock = _terminal->LockForWriting();
+                _terminal->SelectAll();
+                _updateSelectionUI();
+                return true;
+            }
+            else if (vkey == VK_RETURN && !mods.IsCtrlPressed() && !mods.IsAltPressed())
+            {
+                // [Shift +] Enter --> copy text
+                // Don't lock here! CopySelectionToClipboard already locks for you!
+                CopySelectionToClipboard(mods.IsShiftPressed(), nullptr);
+                _terminal->ClearSelection();
+                _updateSelectionUI();
+                return true;
+            }
+            else if (vkey == VK_ESCAPE)
+            {
+                _terminal->ClearSelection();
+                _updateSelectionUI();
+                return true;
+            }
+            else if (const auto updateSlnParams{ _terminal->ConvertKeyEventToUpdateSelectionParams(mods, vkey) })
+            {
+                // try to update the selection
+                auto lock = _terminal->LockForWriting();
+                _terminal->UpdateSelection(updateSlnParams->first, updateSlnParams->second, mods);
+                _updateSelectionUI();
+                return true;
+            }
+        }
+        return false;
+    }
+
     // Method Description:
     // - Send this particular key event to the terminal.
     //   See Terminal::SendKeyEvent for more information.
@@ -407,27 +456,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                                       const bool keyDown)
     {
         // Update the selection, if it's present
-        // GH#6423 - don't dismiss selection if the key that was pressed was a
-        // modifier key. We'll wait for a real keystroke to dismiss the
-        // GH #7395 - don't dismiss selection when taking PrintScreen
-        // selection.
         // GH#8522, GH#3758 - Only modify the selection on key _down_. If we
         // modify on key up, then there's chance that we'll immediately dismiss
         // a selection created by an action bound to a keydown.
-        if (HasSelection() &&
-            !KeyEvent::IsModifierKey(vkey) &&
-            vkey != VK_SNAPSHOT &&
-            keyDown)
+        if (_shouldTryUpdateSelection(vkey) && keyDown)
         {
-            const auto isInMarkMode = _terminal->SelectionMode() == ::Microsoft::Terminal::Core::Terminal::SelectionInteractionMode::Mark;
-            if (isInMarkMode && modifiers.IsCtrlPressed() && vkey == 'A')
-            {
-                auto lock = _terminal->LockForWriting();
-                _terminal->SelectAll();
-                _updateSelectionUI();
-                return true;
-            }
-
             // try to update the selection
             if (const auto updateSlnParams{ _terminal->ConvertKeyEventToUpdateSelectionParams(modifiers, vkey) })
             {

src/cascadia/TerminalControl/ControlCore.h

@@ -87,6 +87,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         void ToggleMarkMode();
         Control::SelectionInteractionMode SelectionMode() const;
         bool SwitchSelectionEndpoint();
+        bool TryMarkModeKeybinding(const WORD vkey,
+                                   const ::Microsoft::Terminal::Core::ControlKeyStates modifiers);
 
         void GotFocus();
         void LostFocus();
@@ -273,6 +275,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         void _updateFont(const bool initialUpdate = false);
         void _refreshSizeUnderLock();
         void _updateSelectionUI();
+        bool _shouldTryUpdateSelection(const WORD vkey);
 
         void _sendInputToConnection(std::wstring_view wstr);
 

src/cascadia/TerminalControl/ControlCore.idl

@@ -14,9 +14,7 @@ namespace Microsoft.Terminal.Control
     // !! LOAD BEARING !! If you make this a struct with Booleans (like they
     // make the most sense as), then the app will crash trying to toss one of
     // these across the process boundary. I haven't the damndest idea why.
-    [flags]
-    enum MouseButtonState
-    {
+    [flags] enum MouseButtonState {
         IsLeftButtonDown = 0x1,
         IsMiddleButtonDown = 0x2,
         IsRightButtonDown = 0x4
@@ -37,9 +35,7 @@ namespace Microsoft.Terminal.Control
         Mark
     };
 
-    [flags]
-    enum SelectionEndpointTarget
-    {
+    [flags] enum SelectionEndpointTarget {
         Start = 0x1,
         End = 0x2
     };
@@ -79,13 +75,15 @@ namespace Microsoft.Terminal.Control
         Double Opacity { get; };
         Boolean UseAcrylic { get; };
 
+        Boolean TryMarkModeKeybinding(Int16 vkey,
+                                      Microsoft.Terminal.Core.ControlKeyStates modifiers);
         Boolean TrySendKeyEvent(Int16 vkey,
-                             Int16 scanCode,
-                             Microsoft.Terminal.Core.ControlKeyStates modifiers,
-                             Boolean keyDown);
+                                Int16 scanCode,
+                                Microsoft.Terminal.Core.ControlKeyStates modifiers,
+                                Boolean keyDown);
         Boolean SendCharEvent(Char ch,
-                           Int16 scanCode,
-                           Microsoft.Terminal.Core.ControlKeyStates modifiers);
+                              Int16 scanCode,
+                              Microsoft.Terminal.Core.ControlKeyStates modifiers);
         void SendInput(String text);
         void PasteText(String text);
         void SelectAll();

src/cascadia/TerminalControl/TermControl.cpp

@@ -1081,7 +1081,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         // keybindings on the keyUp, then we'll still send the keydown to the
         // connected terminal application, and something like ctrl+shift+T will
         // emit a ^T to the pipe.
-        if (!modifiers.IsAltGrPressed() && keyDown && _TryHandleKeyBinding(vkey, scanCode, modifiers))
+        if (!modifiers.IsAltGrPressed() &&
+            keyDown &&
+            _TryHandleKeyBinding(vkey, scanCode, modifiers))
         {
             e.Handled(true);
             return;
@@ -1107,6 +1109,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     // - modifiers: The ControlKeyStates representing the modifier key states.
     bool TermControl::_TryHandleKeyBinding(const WORD vkey, const WORD scanCode, ::Microsoft::Terminal::Core::ControlKeyStates modifiers) const
     {
+        // Mark mode has a specific set of pre-defined key bindings.
+        // If we're in mark mode, we should be prioritizing those over
+        // the custom defined key bindings.
+        if (_core.TryMarkModeKeybinding(vkey, modifiers))
+        {
+            return true;
+        }
+
         // TODO: GH#5000
         // The Core owning the keybindings is weird. That's for sure. In the
         // future, we may want to pass the keybindings into the control