Add findNext, findPrev actions (#8917)

This PR is a resurrection of #8522. @Hegunumo has apparently deleted their account, but the contribution was still valuable. I'm just here to get it across the finish line. This PR adds new action for navigating to the next & previous search results. These actions are unbound by default. These actions can be used from directly within the search dialog also, to immediately navigate the results. Furthermore, if you have a search started, and close the search box, then press this keybinding, _it will still perform the search_. So you can just hit <kbd>F3</kbd> repeatedly with the dialog closed to keep searching new results. Neat! If you dispatch the action on the key down, then dismiss a selection on a key up, we'll end up immediately destroying the selection when you release the bound key. That's annoying. It also bothers @carlos-zamora in #3758. However, I _think_ we can just only dismiss the selection on a key up. I _think_ that's fine. It _seems_ fine so far. We've got an entire release cycle to futz with it. ## Validation Steps Performed I've played with it all day and it seems _crisp_. Closes #7695 Co-authored-by: Kiminori Kaburagi <[email protected]>
microsoft · Feb 18, 2021 · 491cb21 · 491cb21
1 parent 90699da
commit 491cb21
Show file tree

Hide file tree

Showing 18 changed files with 168 additions and 4 deletions.
diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json
@@ -76,6 +76,7 @@
         "copy",
         "duplicateTab",
         "find",
+        "findMatch",
         "moveFocus",
         "moveTab",
         "newTab",
@@ -138,6 +139,13 @@
       ],
       "type": "string"
     },
+    "FindMatchDirection": {
+      "enum": [
+        "next",
+        "prev"
+      ],
+      "type": "string"
+    },
     "SplitState": {
       "enum": [
         "vertical",
@@ -559,6 +567,23 @@
         }
       ]
     },
+    "FindMatchAction": {
+      "description": "Arguments corresponding to a Find Match Action",
+      "allOf": [
+        { "$ref": "#/definitions/ShortcutAction" },
+        {
+          "properties": {
+            "action": { "type": "string", "pattern": "findMatch" },
+            "direction": {
+              "$ref": "#/definitions/FindMatchDirection",
+              "default": "prev",
+              "description": "The direction to search in. \"prev\" will search upwards in the buffer, and \"next\" will search downwards."
+            }
+          }
+        }
+      ],
+      "required": [ "direction" ]
+    },
     "Keybinding": {
       "additionalProperties": false,
       "properties": {
@@ -583,6 +608,7 @@
               { "$ref": "#/definitions/ScrollUpAction" },
               { "$ref": "#/definitions/ScrollDownAction" },
               { "$ref": "#/definitions/MoveTabAction" },
+              { "$ref": "#/definitions/FindMatchAction" },
               { "type": "null" }
             ]
         },

diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp
@@ -193,6 +193,18 @@ namespace winrt::TerminalApp::implementation
         args.Handled(true);
     }
 
+    void TerminalPage::_HandleFindMatch(const IInspectable& /*sender*/,
+                                        const ActionEventArgs& args)
+    {
+        if (const auto& realArgs = args.ActionArgs().try_as<FindMatchArgs>())
+        {
+            if (const auto& control{ _GetActiveControl() })
+            {
+                control.SearchMatch(realArgs.Direction() == FindMatchDirection::Next);
+                args.Handled(true);
+            }
+        }
+    }
     void TerminalPage::_HandleOpenSettings(const IInspectable& /*sender*/,
                                            const ActionEventArgs& args)
     {

diff --git a/src/cascadia/TerminalApp/ShortcutActionDispatch.cpp b/src/cascadia/TerminalApp/ShortcutActionDispatch.cpp
@@ -256,6 +256,11 @@ namespace winrt::TerminalApp::implementation
             _BreakIntoDebuggerHandlers(*this, eventArgs);
             break;
         }
+        case ShortcutAction::FindMatch:
+        {
+            _FindMatchHandlers(*this, eventArgs);
+            break;
+        }
         case ShortcutAction::TogglePaneReadOnly:
         {
             _TogglePaneReadOnlyHandlers(*this, eventArgs);

diff --git a/src/cascadia/TerminalApp/ShortcutActionDispatch.h b/src/cascadia/TerminalApp/ShortcutActionDispatch.h
@@ -65,6 +65,7 @@ namespace winrt::TerminalApp::implementation
         TYPED_EVENT(TabSearch,            TerminalApp::ShortcutActionDispatch, Microsoft::Terminal::Settings::Model::ActionEventArgs);
         TYPED_EVENT(MoveTab,              TerminalApp::ShortcutActionDispatch, Microsoft::Terminal::Settings::Model::ActionEventArgs);
         TYPED_EVENT(BreakIntoDebugger,    TerminalApp::ShortcutActionDispatch, Microsoft::Terminal::Settings::Model::ActionEventArgs);
+        TYPED_EVENT(FindMatch,            TerminalApp::ShortcutActionDispatch, Microsoft::Terminal::Settings::Model::ActionEventArgs);
         TYPED_EVENT(TogglePaneReadOnly,   TerminalApp::ShortcutActionDispatch, Microsoft::Terminal::Settings::Model::ActionEventArgs);
         // clang-format on
 

diff --git a/src/cascadia/TerminalApp/ShortcutActionDispatch.idl b/src/cascadia/TerminalApp/ShortcutActionDispatch.idl
@@ -51,6 +51,7 @@ namespace TerminalApp
         event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, Microsoft.Terminal.Settings.Model.ActionEventArgs> TabSearch;
         event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, Microsoft.Terminal.Settings.Model.ActionEventArgs> MoveTab;
         event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, Microsoft.Terminal.Settings.Model.ActionEventArgs> BreakIntoDebugger;
+        event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, Microsoft.Terminal.Settings.Model.ActionEventArgs> FindMatch;
         event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, Microsoft.Terminal.Settings.Model.ActionEventArgs> TogglePaneReadOnly;
     }
 }
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -1128,6 +1128,7 @@ namespace winrt::TerminalApp::implementation
         _actionDispatch->TabSearch({ this, &TerminalPage::_HandleOpenTabSearch });
         _actionDispatch->MoveTab({ this, &TerminalPage::_HandleMoveTab });
         _actionDispatch->BreakIntoDebugger({ this, &TerminalPage::_HandleBreakIntoDebugger });
+        _actionDispatch->FindMatch({ this, &TerminalPage::_HandleFindMatch });
         _actionDispatch->TogglePaneReadOnly({ this, &TerminalPage::_HandleTogglePaneReadOnly });
     }
 

diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h
@@ -315,6 +315,7 @@ namespace winrt::TerminalApp::implementation
         void _HandleOpenTabSearch(const IInspectable& sender, const Microsoft::Terminal::Settings::Model::ActionEventArgs& args);
         void _HandleMoveTab(const IInspectable& sender, const Microsoft::Terminal::Settings::Model::ActionEventArgs& args);
         void _HandleBreakIntoDebugger(const IInspectable& sender, const Microsoft::Terminal::Settings::Model::ActionEventArgs& args);
+        void _HandleFindMatch(const IInspectable& sender, const Microsoft::Terminal::Settings::Model::ActionEventArgs& args);
         void _HandleTogglePaneReadOnly(const IInspectable& sender, const Microsoft::Terminal::Settings::Model::ActionEventArgs& args);
 
         // Make sure to hook new actions up in _RegisterActionCallbacks!

diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp
@@ -225,6 +225,18 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         }
     }
 
+    void TermControl::SearchMatch(const bool goForward)
+    {
+        if (!_searchBox)
+        {
+            CreateSearchBoxControl();
+        }
+        else
+        {
+            _Search(_searchBox->TextBox().Text(), goForward, false);
+        }
+    }
+
     // Method Description:
     // - Search text in text buffer. This is triggered if the user click
     //   search button or press enter.
@@ -234,7 +246,9 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
     // - caseSensitive: boolean that represents if the current search is case sensitive
     // Return Value:
     // - <none>
-    void TermControl::_Search(const winrt::hstring& text, const bool goForward, const bool caseSensitive)
+    void TermControl::_Search(const winrt::hstring& text,
+                              const bool goForward,
+                              const bool caseSensitive)
     {
         if (text.size() == 0 || _closing)
         {
@@ -267,7 +281,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
     // - RoutedEventArgs: not used
     // Return Value:
     // - <none>
-    void TermControl::_CloseSearchBoxControl(const winrt::Windows::Foundation::IInspectable& /*sender*/, RoutedEventArgs const& /*args*/)
+    void TermControl::_CloseSearchBoxControl(const winrt::Windows::Foundation::IInspectable& /*sender*/,
+                                             RoutedEventArgs const& /*args*/)
     {
         _searchBox->Visibility(Visibility::Collapsed);
 
@@ -1098,7 +1113,13 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         // modifier key. We'll wait for a real keystroke to dismiss the
         // GH #7395 - don't dismiss selection when taking PrintScreen
         // selection.
-        if (_terminal->IsSelectionActive() && !KeyEvent::IsModifierKey(vkey) && vkey != VK_SNAPSHOT)
+        // GH#8522, GH#3758 - Only dismiss the selection on key _down_. If we
+        // dismiss on key up, then there's chance that we'll immediately dismiss
+        // a selection created by an action bound to a keydown.
+        if (_terminal->IsSelectionActive() &&
+            !KeyEvent::IsModifierKey(vkey) &&
+            vkey != VK_SNAPSHOT &&
+            keyDown)
         {
             const CoreWindow window = CoreWindow::GetForCurrentThread();
             const auto leftWinKeyState = window.GetKeyState(VirtualKey::LeftWindows);

diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h
@@ -133,6 +133,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
 
         void CreateSearchBoxControl();
 
+        void SearchMatch(const bool goForward);
+
         bool OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, const bool down);
 
         bool OnMouseWheel(const Windows::Foundation::Point location, const int32_t delta, const bool leftButtonDown, const bool midButtonDown, const bool rightButtonDown);
@@ -343,7 +345,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         void _CompositionCompleted(winrt::hstring text);
         void _CurrentCursorPositionHandler(const IInspectable& sender, const CursorPositionEventArgs& eventArgs);
         void _FontInfoHandler(const IInspectable& sender, const FontInfoEventArgs& eventArgs);
-
         winrt::fire_and_forget _AsyncCloseConnection();
 
         winrt::fire_and_forget _RaiseReadOnlyWarning();

diff --git a/src/cascadia/TerminalControl/TermControl.idl b/src/cascadia/TerminalControl/TermControl.idl
@@ -102,6 +102,8 @@ namespace Microsoft.Terminal.TerminalControl
 
         void CreateSearchBoxControl();
 
+        void SearchMatch(Boolean goForward);
+
         void AdjustFontSize(Int32 fontSizeDelta);
         void ResetFontSize();
 

diff --git a/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp b/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp
@@ -51,6 +51,7 @@ static constexpr std::string_view LegacyToggleRetroEffectKey{ "toggleRetroEffect
 static constexpr std::string_view ToggleShaderEffectsKey{ "toggleShaderEffects" };
 static constexpr std::string_view MoveTabKey{ "moveTab" };
 static constexpr std::string_view BreakIntoDebuggerKey{ "breakIntoDebugger" };
+static constexpr std::string_view FindMatchKey{ "findMatch" };
 static constexpr std::string_view TogglePaneReadOnlyKey{ "toggleReadOnlyMode" };
 
 static constexpr std::string_view ActionKey{ "action" };
@@ -116,6 +117,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         { MoveTabKey, ShortcutAction::MoveTab },
         { BreakIntoDebuggerKey, ShortcutAction::BreakIntoDebugger },
         { UnboundKey, ShortcutAction::Invalid },
+        { FindMatchKey, ShortcutAction::FindMatch },
         { TogglePaneReadOnlyKey, ShortcutAction::TogglePaneReadOnly },
     };
 
@@ -147,6 +149,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         { ShortcutAction::ScrollDown, ScrollDownArgs::FromJson },
         { ShortcutAction::MoveTab, MoveTabArgs::FromJson },
         { ShortcutAction::ToggleCommandPalette, ToggleCommandPaletteArgs::FromJson },
+        { ShortcutAction::FindMatch, FindMatchArgs::FromJson },
 
         { ShortcutAction::Invalid, nullptr },
     };
@@ -316,6 +319,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
                 { ShortcutAction::ToggleShaderEffects, RS_(L"ToggleShaderEffectsCommandKey") },
                 { ShortcutAction::MoveTab, L"" }, // Intentionally omitted, must be generated by GenerateName
                 { ShortcutAction::BreakIntoDebugger, RS_(L"BreakIntoDebuggerCommandKey") },
+                { ShortcutAction::FindMatch, L"" }, // Intentionally omitted, must be generated by GenerateName
                 { ShortcutAction::TogglePaneReadOnly, RS_(L"TogglePaneReadOnlyCommandKey") },
             };
         }();

diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.cpp b/src/cascadia/TerminalSettingsModel/ActionArgs.cpp
@@ -23,6 +23,7 @@
 #include "CloseOtherTabsArgs.g.cpp"
 #include "CloseTabsAfterArgs.g.cpp"
 #include "MoveTabArgs.g.cpp"
+#include "FindMatchArgs.g.cpp"
 #include "ToggleCommandPaletteArgs.g.cpp"
 
 #include <LibraryResources.h>
@@ -431,4 +432,16 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         }
         return RS_(L"ToggleCommandPaletteCommandKey");
     }
+
+    winrt::hstring FindMatchArgs::GenerateName() const
+    {
+        switch (_Direction)
+        {
+        case FindMatchDirection::Next:
+            return winrt::hstring{ RS_(L"FindNextCommandKey") };
+        case FindMatchDirection::Previous:
+            return winrt::hstring{ RS_(L"FindPrevCommandKey") };
+        }
+        return L"";
+    }
 }
diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.h b/src/cascadia/TerminalSettingsModel/ActionArgs.h
@@ -26,6 +26,7 @@
 #include "ScrollDownArgs.g.h"
 #include "MoveTabArgs.g.h"
 #include "ToggleCommandPaletteArgs.g.h"
+#include "FindMatchArgs.g.h"
 
 #include "../../cascadia/inc/cppwinrt_utils.h"
 #include "JsonUtils.h"
@@ -838,6 +839,50 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
             return *copy;
         }
     };
+
+    struct FindMatchArgs : public FindMatchArgsT<FindMatchArgs>
+    {
+        FindMatchArgs() = default;
+        FindMatchArgs(FindMatchDirection direction) :
+            _Direction{ direction } {};
+        GETSET_PROPERTY(FindMatchDirection, Direction, FindMatchDirection::None);
+
+        static constexpr std::string_view DirectionKey{ "direction" };
+
+    public:
+        hstring GenerateName() const;
+
+        bool Equals(const IActionArgs& other)
+        {
+            auto otherAsUs = other.try_as<FindMatchArgs>();
+            if (otherAsUs)
+            {
+                return otherAsUs->_Direction == _Direction;
+            }
+            return false;
+        };
+        static FromJsonResult FromJson(const Json::Value& json)
+        {
+            // LOAD BEARING: Not using make_self here _will_ break you in the future!
+            auto args = winrt::make_self<FindMatchArgs>();
+            JsonUtils::GetValueForKey(json, DirectionKey, args->_Direction);
+            if (args->_Direction == FindMatchDirection::None)
+            {
+                return { nullptr, { SettingsLoadWarnings::MissingRequiredParameter } };
+            }
+            else
+            {
+                return { *args, {} };
+            }
+        }
+        IActionArgs Copy() const
+        {
+            auto copy{ winrt::make_self<FindMatchArgs>() };
+            copy->_Direction = _Direction;
+            return *copy;
+        }
+    };
+
 }
 
 namespace winrt::Microsoft::Terminal::Settings::Model::factory_implementation
@@ -853,4 +898,5 @@ namespace winrt::Microsoft::Terminal::Settings::Model::factory_implementation
     BASIC_FACTORY(CloseTabsAfterArgs);
     BASIC_FACTORY(MoveTabArgs);
     BASIC_FACTORY(OpenSettingsArgs);
+    BASIC_FACTORY(FindMatchArgs);
 }
diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.idl b/src/cascadia/TerminalSettingsModel/ActionArgs.idl
@@ -64,6 +64,13 @@ namespace Microsoft.Terminal.Settings.Model
         Backward
     };
 
+    enum FindMatchDirection
+    {
+        None = 0,
+        Next,
+        Previous
+    };
+
     enum CommandPaletteLaunchMode
     {
         Action = 0,
@@ -204,4 +211,10 @@ namespace Microsoft.Terminal.Settings.Model
     {
         CommandPaletteLaunchMode LaunchMode { get; };
     };
+
+    [default_interface] runtimeclass FindMatchArgs : IActionArgs
+    {
+        FindMatchArgs(FindMatchDirection direction);
+        FindMatchDirection Direction { get; };
+    };
 }
diff --git a/src/cascadia/TerminalSettingsModel/KeyMapping.idl b/src/cascadia/TerminalSettingsModel/KeyMapping.idl
@@ -53,6 +53,7 @@ namespace Microsoft.Terminal.Settings.Model
         TabSearch,
         MoveTab,
         BreakIntoDebugger,
+        FindMatch,
         TogglePaneReadOnly
     };
 

diff --git a/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw b/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw
@@ -215,6 +215,12 @@
   <data name="FindCommandKey" xml:space="preserve">
     <value>Find</value>
   </data>
+  <data name="FindNextCommandKey" xml:space="preserve">
+    <value>Find next search match</value>
+  </data>
+  <data name="FindPrevCommandKey" xml:space="preserve">
+    <value>Find previous search match</value>
+  </data>
   <data name="IncreaseFontSizeCommandKey" xml:space="preserve">
     <value>Increase font size</value>
   </data>

diff --git a/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h b/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h
@@ -431,3 +431,11 @@ JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Settings::Model::CommandPaletteLa
         pair_type{ "commandLine", ValueType::CommandLine },
     };
 };
+
+JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Settings::Model::FindMatchDirection)
+{
+    JSON_MAPPINGS(2) = {
+        pair_type{ "next", ValueType::Next },
+        pair_type{ "prev", ValueType::Previous },
+    };
+};
diff --git a/src/cascadia/TerminalSettingsModel/defaults.json b/src/cascadia/TerminalSettingsModel/defaults.json
@@ -287,6 +287,8 @@
         { "command": "openSettings", "keys": "ctrl+," },
         { "command": { "action": "openSettings", "target": "defaultsFile" }, "keys": "ctrl+alt+," },
         { "command": "find", "keys": "ctrl+shift+f" },
+        { "command": { "action": "findMatch", "direction": "next" } },
+        { "command": { "action": "findMatch", "direction": "prev" } },
         { "command": "toggleShaderEffects" },
         { "command": "openTabColorPicker" },
         { "command": "renameTab" },