From 491cb217222a35f69dd027f46184d5e26adb8f15 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 18 Feb 2021 13:21:35 -0600 Subject: [PATCH] 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 F3 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 --- doc/cascadia/profiles.schema.json | 26 +++++++++++ .../TerminalApp/AppActionHandlers.cpp | 12 +++++ .../TerminalApp/ShortcutActionDispatch.cpp | 5 ++ .../TerminalApp/ShortcutActionDispatch.h | 1 + .../TerminalApp/ShortcutActionDispatch.idl | 1 + src/cascadia/TerminalApp/TerminalPage.cpp | 1 + src/cascadia/TerminalApp/TerminalPage.h | 1 + src/cascadia/TerminalControl/TermControl.cpp | 27 +++++++++-- src/cascadia/TerminalControl/TermControl.h | 3 +- src/cascadia/TerminalControl/TermControl.idl | 2 + .../TerminalSettingsModel/ActionAndArgs.cpp | 4 ++ .../TerminalSettingsModel/ActionArgs.cpp | 13 ++++++ .../TerminalSettingsModel/ActionArgs.h | 46 +++++++++++++++++++ .../TerminalSettingsModel/ActionArgs.idl | 13 ++++++ .../TerminalSettingsModel/KeyMapping.idl | 1 + .../Resources/en-US/Resources.resw | 6 +++ .../TerminalSettingsSerializationHelpers.h | 8 ++++ .../TerminalSettingsModel/defaults.json | 2 + 18 files changed, 168 insertions(+), 4 deletions(-) diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json index 9a43234411a..13b8a1b31f1 100644 --- 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 index ea5ea18a71f..227f7285f90 100644 --- 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()) + { + 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 index 3fee3e6a084..033f1a37813 100644 --- 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 index b42b276db2f..0f2488e40ce 100644 --- 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 index 9b376c5d244..906de95498d 100644 --- a/src/cascadia/TerminalApp/ShortcutActionDispatch.idl +++ b/src/cascadia/TerminalApp/ShortcutActionDispatch.idl @@ -51,6 +51,7 @@ namespace TerminalApp event Windows.Foundation.TypedEventHandler TabSearch; event Windows.Foundation.TypedEventHandler MoveTab; event Windows.Foundation.TypedEventHandler BreakIntoDebugger; + event Windows.Foundation.TypedEventHandler FindMatch; event Windows.Foundation.TypedEventHandler TogglePaneReadOnly; } } diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index debb96e6c03..595a99876fb 100644 --- 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 index f8897f6d691..67ca1841d60 100644 --- 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 index 0fd5273ce1b..d929597a6c1 100644 --- 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: // - - 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: // - - 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 index 171c28de8a5..52b76422ec8 100644 --- 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 index 0bad381493b..4470c0555d6 100644 --- 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 index 6130475bee4..501c43db175 100644 --- 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 index 22744cacfad..218368125d5 100644 --- 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 @@ -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 index 7a249330cf0..31898031cdc 100644 --- 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() = 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(); + 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(); + 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() }; + 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 index e4413fbd452..498cc475db3 100644 --- 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 index 31795b2f081..7df3dbb489f 100644 --- 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 index 12a9a7599e9..f8a588553f5 100644 --- a/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw @@ -215,6 +215,12 @@ Find + + Find next search match + + + Find previous search match + Increase font size diff --git a/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h b/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h index 82b16cdefb0..c71e858d06b 100644 --- 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 index 7c7e8329d7b..b3b4888ec06 100644 --- 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" },