From ad28b1cec0f15ad21e65359132d34bc2f9518dea Mon Sep 17 00:00:00 2001 From: khvitaly Date: Sun, 20 Dec 2020 20:16:22 +0200 Subject: [PATCH 1/3] Remove action dispatching from Command Palette --- src/cascadia/TerminalApp/CommandPalette.cpp | 56 ++++++++------------- src/cascadia/TerminalApp/CommandPalette.h | 5 +- src/cascadia/TerminalApp/CommandPalette.idl | 2 +- src/cascadia/TerminalApp/TerminalPage.cpp | 2 +- 4 files changed, 24 insertions(+), 41 deletions(-) diff --git a/src/cascadia/TerminalApp/CommandPalette.cpp b/src/cascadia/TerminalApp/CommandPalette.cpp index add9138eafd..46ceee98c90 100644 --- a/src/cascadia/TerminalApp/CommandPalette.cpp +++ b/src/cascadia/TerminalApp/CommandPalette.cpp @@ -245,18 +245,26 @@ namespace winrt::TerminalApp::implementation // Only give anchored tab switcher the ability to cycle through tabs with the tab button. // For unanchored mode, accessibility becomes an issue when we try to hijack tab since it's // a really widely used keyboard navigation key. - if (_currentMode == CommandPaletteMode::TabSwitchMode && key == VirtualKey::Tab) + if (_currentMode == CommandPaletteMode::TabSwitchMode && _keymap) { - auto const state = CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Shift); - if (WI_IsFlagSet(state, CoreVirtualKeyStates::Down)) - { - SelectNextItem(false); - e.Handled(true); - } - else + auto const ctrlDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Control), CoreVirtualKeyStates::Down); + auto const altDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Menu), CoreVirtualKeyStates::Down); + auto const shiftDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Shift), CoreVirtualKeyStates::Down); + + winrt::Microsoft::Terminal::TerminalControl::KeyChord kc{ ctrlDown, altDown, shiftDown, static_cast(key) }; + const auto action = _keymap.TryLookup(kc); + if (action) { - SelectNextItem(true); - e.Handled(true); + if (action.Action() == ShortcutAction::PrevTab) + { + SelectNextItem(false); + e.Handled(true); + } + else if (action.Action() == ShortcutAction::NextTab) + { + SelectNextItem(true); + e.Handled(true); + } } } else if (key == VirtualKey::Home) @@ -350,30 +358,6 @@ namespace winrt::TerminalApp::implementation e.Handled(true); } - else - { - const auto vkey = ::gsl::narrow_cast(e.OriginalKey()); - - // In the interest of not telling all modes to check for keybindings, limit to TabSwitch mode for now. - if (_currentMode == CommandPaletteMode::TabSwitchMode) - { - auto const ctrlDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Control), CoreVirtualKeyStates::Down); - auto const altDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Menu), CoreVirtualKeyStates::Down); - auto const shiftDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Shift), CoreVirtualKeyStates::Down); - - auto success = _bindings.TryKeyChord({ - ctrlDown, - altDown, - shiftDown, - vkey, - }); - - if (success) - { - e.Handled(true); - } - } - } } // Method Description: @@ -852,9 +836,9 @@ namespace winrt::TerminalApp::implementation return _filteredActions; } - void CommandPalette::SetKeyBindings(Microsoft::Terminal::TerminalControl::IKeyBindings bindings) + void CommandPalette::SetKeyMap(const Microsoft::Terminal::Settings::Model::KeyMapping& keymap) { - _bindings = bindings; + _keymap = keymap; } void CommandPalette::SetCommands(Collections::IVector const& actions) diff --git a/src/cascadia/TerminalApp/CommandPalette.h b/src/cascadia/TerminalApp/CommandPalette.h index 07a20f56bb4..39f655a6465 100644 --- a/src/cascadia/TerminalApp/CommandPalette.h +++ b/src/cascadia/TerminalApp/CommandPalette.h @@ -32,7 +32,7 @@ namespace winrt::TerminalApp::implementation void SetCommands(Windows::Foundation::Collections::IVector const& actions); void SetTabs(Windows::Foundation::Collections::IVector const& tabs, const bool clearList); - void SetKeyBindings(Microsoft::Terminal::TerminalControl::IKeyBindings bindings); + void SetKeyMap(const Microsoft::Terminal::Settings::Model::KeyMapping& keymap); void EnableCommandPaletteMode(Microsoft::Terminal::Settings::Model::CommandPaletteLaunchMode const launchMode); @@ -47,7 +47,6 @@ namespace winrt::TerminalApp::implementation // Tab Switcher void EnableTabSwitcherMode(const bool searchMode, const uint32_t startIdx); - void SetTabSwitchOrder(const Microsoft::Terminal::Settings::Model::TabSwitcherMode order); WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler); OBSERVABLE_GETSET_PROPERTY(winrt::hstring, NoMatchesText, _PropertyChangedHandlers); @@ -109,7 +108,7 @@ namespace winrt::TerminalApp::implementation std::wstring _getTrimmedInput(); void _evaluatePrefix(); - Microsoft::Terminal::TerminalControl::IKeyBindings _bindings; + Microsoft::Terminal::Settings::Model::KeyMapping _keymap{ nullptr }; // Tab Switcher Windows::Foundation::Collections::IVector _tabActions{ nullptr }; diff --git a/src/cascadia/TerminalApp/CommandPalette.idl b/src/cascadia/TerminalApp/CommandPalette.idl index 57347ecc6b5..28f0f89c0e7 100644 --- a/src/cascadia/TerminalApp/CommandPalette.idl +++ b/src/cascadia/TerminalApp/CommandPalette.idl @@ -23,7 +23,7 @@ namespace TerminalApp void SetCommands(Windows.Foundation.Collections.IVector actions); void SetTabs(Windows.Foundation.Collections.IVector tabs, Boolean clearList); - void SetKeyBindings(Microsoft.Terminal.TerminalControl.IKeyBindings bindings); + void SetKeyMap(Microsoft.Terminal.Settings.Model.KeyMapping keymap); void EnableCommandPaletteMode(Microsoft.Terminal.Settings.Model.CommandPaletteLaunchMode launchMode); void SelectNextItem(Boolean moveDown); diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 3f9b5ee83ff..ba54d08b735 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -108,7 +108,7 @@ namespace winrt::TerminalApp::implementation if (auto page{ weakThis.get() }) { _UpdateCommandsForPalette(); - CommandPalette().SetKeyBindings(*_bindings); + CommandPalette().SetKeyMap(_settings.KeyMap()); } } From 9cd1df717f0a58d94c1af57c327d87e1f50992a9 Mon Sep 17 00:00:00 2001 From: khvitaly Date: Sun, 20 Dec 2020 20:53:59 +0200 Subject: [PATCH 2/3] Cleanup redundant palette visibility checks --- src/cascadia/TerminalApp/TerminalPage.cpp | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index ba54d08b735..b301ee50766 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -1319,14 +1319,6 @@ namespace winrt::TerminalApp::implementation // - Sets focus to the tab to the right or left the currently selected tab. void TerminalPage::_SelectNextTab(const bool bMoveRight) { - if (CommandPalette().Visibility() == Visibility::Visible) - { - // If the tab switcher is currently open, don't change its mode. - // Just select the new tab. - CommandPalette().SelectNextItem(bMoveRight); - return; - } - const auto tabSwitchMode = _settings.GlobalSettings().TabSwitcherMode(); const bool useInOrderTabIndex = tabSwitchMode != TabSwitcherMode::MostRecentlyUsed; @@ -2234,7 +2226,7 @@ namespace winrt::TerminalApp::implementation // here, then the user won't be able to navigate the ATS any // longer. // - // When the tab swither is eventually dismissed, the focus will + // When the tab switcher is eventually dismissed, the focus will // get tossed back to the focused terminal control, so we don't // need to worry about focus getting lost. if (CommandPalette().Visibility() != Visibility::Visible) From 312028f389bfe5db6186351fe2d37e575501a36a Mon Sep 17 00:00:00 2001 From: khvitaly Date: Mon, 21 Dec 2020 04:20:10 +0200 Subject: [PATCH 3/3] Decouple TerminalPage from AppLogic --- src/cascadia/TerminalApp/TerminalPage.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index b301ee50766..a820306042e 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -4,7 +4,6 @@ #include "pch.h" #include "TerminalPage.h" #include "Utils.h" -#include "AppLogic.h" #include "../../types/inc/utils.hpp" #include @@ -943,9 +942,7 @@ namespace winrt::TerminalApp::implementation auto const shiftDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Shift), CoreVirtualKeyStates::Down); winrt::Microsoft::Terminal::TerminalControl::KeyChord kc{ ctrlDown, altDown, shiftDown, static_cast(key) }; - auto setting = AppLogic::CurrentAppSettings(); - auto keymap = setting.GlobalSettings().KeyMap(); - const auto actionAndArgs = keymap.TryLookup(kc); + const auto actionAndArgs = _settings.KeyMap().TryLookup(kc); if (actionAndArgs) { if (CommandPalette().Visibility() == Visibility::Visible && actionAndArgs.Action() != ShortcutAction::ToggleCommandPalette)