From e5db9b974e9c6be28df4747c8685aef26606dff4 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 18 Nov 2019 10:39:32 -0600 Subject: [PATCH 1/2] fixes #3603 by searching for ActionAndArgs too --- src/cascadia/TerminalApp/ActionArgs.h | 56 +++++++++++++++++++ src/cascadia/TerminalApp/ActionArgs.idl | 7 ++- src/cascadia/TerminalApp/AppKeyBindings.cpp | 24 +++++++- src/cascadia/TerminalApp/AppKeyBindings.h | 3 +- src/cascadia/TerminalApp/AppKeyBindings.idl | 3 +- .../AppKeyBindingsSerialization.cpp | 2 +- src/cascadia/TerminalApp/TerminalPage.cpp | 19 ++++++- 7 files changed, 105 insertions(+), 9 deletions(-) diff --git a/src/cascadia/TerminalApp/ActionArgs.h b/src/cascadia/TerminalApp/ActionArgs.h index 0a1972d5cc7..cf247bd2ad4 100644 --- a/src/cascadia/TerminalApp/ActionArgs.h +++ b/src/cascadia/TerminalApp/ActionArgs.h @@ -41,6 +41,15 @@ namespace winrt::TerminalApp::implementation static constexpr std::string_view TrimWhitespaceKey{ "trimWhitespace" }; public: + bool Equals(const IActionArgs& other) + { + auto otherAsUs = other.try_as(); + if (otherAsUs) + { + return otherAsUs->_TrimWhitespace == _TrimWhitespace; + } + return false; + }; static winrt::TerminalApp::IActionArgs FromJson(const Json::Value& json) { // LOAD BEARING: Not using make_self here _will_ break you in the future! @@ -61,6 +70,15 @@ namespace winrt::TerminalApp::implementation static constexpr std::string_view ProfileIndexKey{ "index" }; public: + bool Equals(const IActionArgs& other) + { + auto otherAsUs = other.try_as(); + if (otherAsUs) + { + return otherAsUs->_ProfileIndex == _ProfileIndex; + } + return false; + }; static winrt::TerminalApp::IActionArgs FromJson(const Json::Value& json) { // LOAD BEARING: Not using make_self here _will_ break you in the future! @@ -81,6 +99,15 @@ namespace winrt::TerminalApp::implementation static constexpr std::string_view TabIndexKey{ "index" }; public: + bool Equals(const IActionArgs& other) + { + auto otherAsUs = other.try_as(); + if (otherAsUs) + { + return otherAsUs->_TabIndex == _TabIndex; + } + return false; + }; static winrt::TerminalApp::IActionArgs FromJson(const Json::Value& json) { // LOAD BEARING: Not using make_self here _will_ break you in the future! @@ -136,6 +163,15 @@ namespace winrt::TerminalApp::implementation static constexpr std::string_view DirectionKey{ "direction" }; public: + bool Equals(const IActionArgs& other) + { + auto otherAsUs = other.try_as(); + if (otherAsUs) + { + return otherAsUs->_Direction == _Direction; + } + return false; + }; static winrt::TerminalApp::IActionArgs FromJson(const Json::Value& json) { // LOAD BEARING: Not using make_self here _will_ break you in the future! @@ -156,6 +192,15 @@ namespace winrt::TerminalApp::implementation static constexpr std::string_view DirectionKey{ "direction" }; public: + bool Equals(const IActionArgs& other) + { + auto otherAsUs = other.try_as(); + if (otherAsUs) + { + return otherAsUs->_Direction == _Direction; + } + return false; + }; static winrt::TerminalApp::IActionArgs FromJson(const Json::Value& json) { // LOAD BEARING: Not using make_self here _will_ break you in the future! @@ -172,6 +217,17 @@ namespace winrt::TerminalApp::implementation { AdjustFontSizeArgs() = default; GETSET_PROPERTY(int32_t, Delta, 0); + + public: + bool Equals(const IActionArgs& other) + { + auto otherAsUs = other.try_as(); + if (otherAsUs) + { + return otherAsUs->_Delta == _Delta; + } + return false; + }; }; } diff --git a/src/cascadia/TerminalApp/ActionArgs.idl b/src/cascadia/TerminalApp/ActionArgs.idl index 593e7a397d4..cd625931b30 100644 --- a/src/cascadia/TerminalApp/ActionArgs.idl +++ b/src/cascadia/TerminalApp/ActionArgs.idl @@ -3,9 +3,10 @@ namespace TerminalApp { - // An empty interface must specify an explicit [uuid] to ensure uniqueness. - // We also manually have to specify a "version" attribute to make the compiler happy. - [uuid("191C2BDE-1A60-4BAB-9765-D850F0EF2CAC")][version(1)] interface IActionArgs{}; + interface IActionArgs + { + Boolean Equals(IActionArgs other); + }; interface IActionEventArgs { diff --git a/src/cascadia/TerminalApp/AppKeyBindings.cpp b/src/cascadia/TerminalApp/AppKeyBindings.cpp index 4d47681d990..4a54ca58e0c 100644 --- a/src/cascadia/TerminalApp/AppKeyBindings.cpp +++ b/src/cascadia/TerminalApp/AppKeyBindings.cpp @@ -9,6 +9,7 @@ using namespace winrt::Microsoft::Terminal; using namespace winrt::TerminalApp; +using namespace winrt::Microsoft::Terminal::Settings; namespace winrt::TerminalApp::implementation { @@ -29,7 +30,7 @@ namespace winrt::TerminalApp::implementation _keyShortcuts.erase(chord); } - Microsoft::Terminal::Settings::KeyChord AppKeyBindings::GetKeyBinding(TerminalApp::ShortcutAction const& action) + KeyChord AppKeyBindings::GetKeyBindingForAction(TerminalApp::ShortcutAction const& action) { for (auto& kv : _keyShortcuts) { @@ -40,6 +41,27 @@ namespace winrt::TerminalApp::implementation } return { nullptr }; } + // Method Description: + // - Lookup the keychord bound to a particular combination of ShortcutAction + // and IActionArgs. This enables searching no only for the binding of a + // particular ShortcutAction, but also a particular set of values for + // arguments to that action. + // Arguments: + // - actionAndArgs: The ActionAndArgs to lookup the keybinding for. + // Return Value: + // - The bound keychord, if this ActionAndArgs is bound to a key, otherwise nullptr. + KeyChord AppKeyBindings::GetKeyBindingForActionWithArgs(TerminalApp::ActionAndArgs const& actionAndArgs) + { + for (auto& kv : _keyShortcuts) + { + if (kv.second.Action() == actionAndArgs.Action() && + kv.second.Args().Equals(actionAndArgs.Args())) + { + return kv.first; + } + } + return { nullptr }; + } bool AppKeyBindings::TryKeyChord(const Settings::KeyChord& kc) { diff --git a/src/cascadia/TerminalApp/AppKeyBindings.h b/src/cascadia/TerminalApp/AppKeyBindings.h index ebff41e3fdf..fec12c611d2 100644 --- a/src/cascadia/TerminalApp/AppKeyBindings.h +++ b/src/cascadia/TerminalApp/AppKeyBindings.h @@ -45,7 +45,8 @@ namespace winrt::TerminalApp::implementation void SetKeyBinding(TerminalApp::ActionAndArgs const& actionAndArgs, winrt::Microsoft::Terminal::Settings::KeyChord const& chord); void ClearKeyBinding(winrt::Microsoft::Terminal::Settings::KeyChord const& chord); - Microsoft::Terminal::Settings::KeyChord GetKeyBinding(TerminalApp::ShortcutAction const& action); + Microsoft::Terminal::Settings::KeyChord GetKeyBindingForAction(TerminalApp::ShortcutAction const& action); + Microsoft::Terminal::Settings::KeyChord GetKeyBindingForActionWithArgs(TerminalApp::ActionAndArgs const& actionAndArgs); static Windows::System::VirtualKeyModifiers ConvertVKModifiers(winrt::Microsoft::Terminal::Settings::KeyModifiers modifiers); diff --git a/src/cascadia/TerminalApp/AppKeyBindings.idl b/src/cascadia/TerminalApp/AppKeyBindings.idl index 98adffd1d33..ca199fca764 100644 --- a/src/cascadia/TerminalApp/AppKeyBindings.idl +++ b/src/cascadia/TerminalApp/AppKeyBindings.idl @@ -76,7 +76,8 @@ namespace TerminalApp void SetKeyBinding(ActionAndArgs actionAndArgs, Microsoft.Terminal.Settings.KeyChord chord); void ClearKeyBinding(Microsoft.Terminal.Settings.KeyChord chord); - Microsoft.Terminal.Settings.KeyChord GetKeyBinding(ShortcutAction action); + Microsoft.Terminal.Settings.KeyChord GetKeyBindingForAction(ShortcutAction action); + Microsoft.Terminal.Settings.KeyChord GetKeyBindingForActionWithArgs(ActionAndArgs actionAndArgs); event Windows.Foundation.TypedEventHandler CopyText; event Windows.Foundation.TypedEventHandler PasteText; diff --git a/src/cascadia/TerminalApp/AppKeyBindingsSerialization.cpp b/src/cascadia/TerminalApp/AppKeyBindingsSerialization.cpp index c621a9d3bc2..7e60911b808 100644 --- a/src/cascadia/TerminalApp/AppKeyBindingsSerialization.cpp +++ b/src/cascadia/TerminalApp/AppKeyBindingsSerialization.cpp @@ -333,7 +333,7 @@ Json::Value winrt::TerminalApp::implementation::AppKeyBindings::ToJson() const auto searchedForName = actionName.first; const auto searchedForAction = actionName.second; - if (const auto chord{ GetKeyBinding(searchedForAction) }) + if (const auto chord{ GetKeyBindingForAction(searchedForAction) }) { if (const auto serialization{ _ShortcutAsJsonObject(chord, searchedForName) }) { diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 9642ae26d4c..8eb595774ae 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -3,6 +3,7 @@ #include "pch.h" #include "TerminalPage.h" +#include "ActionAndArgs.h" #include "Utils.h" #include @@ -253,7 +254,21 @@ namespace winrt::TerminalApp::implementation { // enum value for ShortcutAction::NewTabProfileX; 0==NewTabProfile0 const auto action = static_cast(profileIndex + static_cast(ShortcutAction::NewTabProfile0)); - auto profileKeyChord = keyBindings.GetKeyBinding(action); + // First, attepmt to search for the keybinding for the simple + // NewTabProfile0-9 ShortcutActions. + auto profileKeyChord = keyBindings.GetKeyBindingForAction(action); + if (!profileKeyChord) + { + // If NewTabProfileN didn't have a binding, look for a + // keychord that is bound to the equivalent + // NewTab(ProfileIndex=N) action + auto actionAndArgs = winrt::make_self(); + actionAndArgs->Action(ShortcutAction::NewTab); + auto newTabArgs = winrt::make_self(); + newTabArgs->ProfileIndex(profileIndex); + actionAndArgs->Args(*newTabArgs); + profileKeyChord = keyBindings.GetKeyBindingForActionWithArgs(*actionAndArgs); + } // make sure we find one to display if (profileKeyChord) @@ -306,7 +321,7 @@ namespace winrt::TerminalApp::implementation settingsItem.Click({ this, &TerminalPage::_SettingsButtonOnClick }); newTabFlyout.Items().Append(settingsItem); - auto settingsKeyChord = keyBindings.GetKeyBinding(ShortcutAction::OpenSettings); + auto settingsKeyChord = keyBindings.GetKeyBindingForAction(ShortcutAction::OpenSettings); if (settingsKeyChord) { _SetAcceleratorForMenuItem(settingsItem, settingsKeyChord); From 1ea072765cf5ea87370a0707be1c679d4efdbe4f Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 18 Nov 2019 15:45:31 -0600 Subject: [PATCH 2/2] Apply suggestions from code review Co-Authored-By: Carlos Zamora --- src/cascadia/TerminalApp/AppKeyBindings.cpp | 1 + src/cascadia/TerminalApp/TerminalPage.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cascadia/TerminalApp/AppKeyBindings.cpp b/src/cascadia/TerminalApp/AppKeyBindings.cpp index 4a54ca58e0c..93cf11ef77e 100644 --- a/src/cascadia/TerminalApp/AppKeyBindings.cpp +++ b/src/cascadia/TerminalApp/AppKeyBindings.cpp @@ -41,6 +41,7 @@ namespace winrt::TerminalApp::implementation } return { nullptr }; } + // Method Description: // - Lookup the keychord bound to a particular combination of ShortcutAction // and IActionArgs. This enables searching no only for the binding of a diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 8eb595774ae..b6721804af0 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -254,7 +254,7 @@ namespace winrt::TerminalApp::implementation { // enum value for ShortcutAction::NewTabProfileX; 0==NewTabProfile0 const auto action = static_cast(profileIndex + static_cast(ShortcutAction::NewTabProfile0)); - // First, attepmt to search for the keybinding for the simple + // First, attempt to search for the keybinding for the simple // NewTabProfile0-9 ShortcutActions. auto profileKeyChord = keyBindings.GetKeyBindingForAction(action); if (!profileKeyChord)