From da24f7d9390a71648a81f17fc1eefc6e2d27bcbf Mon Sep 17 00:00:00 2001 From: Don-Vito Date: Wed, 24 Mar 2021 00:00:07 +0200 Subject: [PATCH] Allow overriding tab switcher mode on command level (#9507) ## Summary of the Pull Request Currently, when the MRU is enabled we lose the keybinding allowing us to go forward/backward (aka right/left in LTR) in the tab view. To fix that, this PR introduces "tabSwitcherMode" optional parameter to the prevTab / nextTab commands. If it is not provided the global setting will be used. So if you want to go to adjacent tabs, even if MRU is enabled on the system level you can use: ``` { "command": { "action": "prevTab", "tabSwitcherMode": "inOrder" }, "keys": "ctrl+f1"} { "command": { "action": "nextTab", "tabSwitcherMode": "inOrder" }, "keys": "ctrl+f2"} ``` or even ``` {"command": { "action": "prevTab", "tabSwitcherMode": "disabled" }, "keys": "ctrl+f1"} { "command": { "action": "nextTab", "tabSwitcherMode": "disabled" }, "keys": "ctrl+f2"} ``` if you don't want tab switcher to show up ## PR Checklist * [x] Closes https://github.com/microsoft/terminal/issues/9330 * [x] CLA signed. * [x] Tests added/passed * [ ] Documentation updated - not yet. Waiting for approval. * [x] Schema updated. * [ ] I've discussed this with core contributors already. --- doc/cascadia/profiles.schema.json | 47 +++++++++++++ .../LocalTests_TerminalApp/TabTests.cpp | 14 ++-- .../TerminalApp/AppActionHandlers.cpp | 16 +++-- src/cascadia/TerminalApp/TerminalPage.cpp | 4 +- src/cascadia/TerminalApp/TerminalPage.h | 2 +- .../TerminalSettingsModel/ActionAndArgs.cpp | 2 + .../TerminalSettingsModel/ActionArgs.cpp | 24 +++++++ .../TerminalSettingsModel/ActionArgs.h | 67 +++++++++++++++++++ .../TerminalSettingsModel/ActionArgs.idl | 17 +++++ .../GlobalAppSettings.idl | 7 -- 10 files changed, 177 insertions(+), 23 deletions(-) diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json index 89a28ad27a..681a0b6beb 100644 --- a/doc/cascadia/profiles.schema.json +++ b/doc/cascadia/profiles.schema.json @@ -231,6 +231,19 @@ }, "type": "object" }, + "SwitchToAdjacentTabArgs" : { + "oneOf": [ + { "type": "null" }, + { + "enum": [ + "mru", + "inOrder", + "disabled" + ], + "type": "string" + } + ] + }, "ShortcutAction": { "properties": { "action": { @@ -602,6 +615,38 @@ } ] }, + "PrevTabAction": { + "description": "Arguments corresponding to a Previous Tab Action", + "allOf": [ + { "$ref": "#/definitions/ShortcutAction" }, + { + "properties": { + "action": { "type":"string", "pattern": "prevTab" }, + "tabSwitcherMode": { + "$ref": "#/definitions/SwitchToAdjacentTabArgs", + "default": null, + "description": "Move to the previous tab using \"tabSwitcherMode\". If no mode is provided, use the one globally defined one." + } + } + } + ] + }, + "NextTabAction": { + "description": "Arguments corresponding to a Next Tab Action", + "allOf": [ + { "$ref": "#/definitions/ShortcutAction" }, + { + "properties": { + "action": { "type":"string", "pattern": "nextTab" }, + "tabSwitcherMode": { + "$ref": "#/definitions/SwitchToAdjacentTabArgs", + "default": null, + "description": "Move to the next tab using \"tabSwitcherMode\". If no mode is provided, use the one globally defined one." + } + } + } + ] + }, "Keybinding": { "additionalProperties": false, "properties": { @@ -628,6 +673,8 @@ { "$ref": "#/definitions/MoveTabAction" }, { "$ref": "#/definitions/FindMatchAction" }, { "$ref": "#/definitions/NewWindowAction" }, + { "$ref": "#/definitions/NextTabAction" }, + { "$ref": "#/definitions/PrevTabAction" }, { "type": "null" } ] }, diff --git a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp index e6f15f44d9..e2b3ab6b73 100644 --- a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp @@ -782,8 +782,7 @@ namespace TerminalAppLocalTests Log::Comment(L"Switch to the next MRU tab, which is the fourth tab"); TestOnUIThread([&page]() { - ActionEventArgs eventArgs{}; - page->_HandleNextTab(nullptr, eventArgs); + page->_SelectNextTab(true, nullptr); }); Log::Comment(L"Sleep to let events propagate"); @@ -804,8 +803,7 @@ namespace TerminalAppLocalTests Log::Comment(L"Switch to the next MRU tab, which is the second tab"); TestOnUIThread([&page]() { - ActionEventArgs eventArgs{}; - page->_HandleNextTab(nullptr, eventArgs); + page->_SelectNextTab(true, nullptr); }); Log::Comment(L"Sleep to let events propagate"); @@ -829,8 +827,7 @@ namespace TerminalAppLocalTests Log::Comment(L"Switch to the next in-order tab, which is the third tab"); TestOnUIThread([&page]() { - ActionEventArgs eventArgs{}; - page->_HandleNextTab(nullptr, eventArgs); + page->_SelectNextTab(true, nullptr); }); TestOnUIThread([&page]() { uint32_t focusedIndex = page->_GetFocusedTabIndex().value_or(-1); @@ -842,8 +839,7 @@ namespace TerminalAppLocalTests Log::Comment(L"Switch to the next in-order tab, which is the fourth tab"); TestOnUIThread([&page]() { - ActionEventArgs eventArgs{}; - page->_HandleNextTab(nullptr, eventArgs); + page->_SelectNextTab(true, nullptr); }); TestOnUIThread([&page]() { uint32_t focusedIndex = page->_GetFocusedTabIndex().value_or(-1); @@ -916,7 +912,7 @@ namespace TerminalAppLocalTests Log::Comment(L"Switch to the next MRU tab, which is the third tab"); RunOnUIThread([&page]() { - page->_SelectNextTab(true); + page->_SelectNextTab(true, nullptr); // In the course of a single tick, the Command Palette will: // * open // * select the proper tab from the mru's list diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp index c45ef8596a..5aebb9d153 100644 --- a/src/cascadia/TerminalApp/AppActionHandlers.cpp +++ b/src/cascadia/TerminalApp/AppActionHandlers.cpp @@ -89,15 +89,23 @@ namespace winrt::TerminalApp::implementation void TerminalPage::_HandleNextTab(const IInspectable& /*sender*/, const ActionEventArgs& args) { - _SelectNextTab(true); - args.Handled(true); + const auto& realArgs = args.ActionArgs().try_as(); + if (realArgs) + { + _SelectNextTab(true, realArgs.SwitcherMode()); + args.Handled(true); + } } void TerminalPage::_HandlePrevTab(const IInspectable& /*sender*/, const ActionEventArgs& args) { - _SelectNextTab(false); - args.Handled(true); + const auto& realArgs = args.ActionArgs().try_as(); + if (realArgs) + { + _SelectNextTab(false, realArgs.SwitcherMode()); + args.Handled(true); + } } void TerminalPage::_HandleSendInput(const IInspectable& /*sender*/, diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 596f917bb1..0eefa036f8 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -1527,10 +1527,10 @@ namespace winrt::TerminalApp::implementation // Method Description: // - Sets focus to the tab to the right or left the currently selected tab. - void TerminalPage::_SelectNextTab(const bool bMoveRight) + void TerminalPage::_SelectNextTab(const bool bMoveRight, const Windows::Foundation::IReference& customTabSwitcherMode) { const auto index{ _GetFocusedTabIndex().value_or(0) }; - const auto tabSwitchMode = _settings.GlobalSettings().TabSwitcherMode(); + const auto tabSwitchMode = customTabSwitcherMode ? customTabSwitcherMode.Value() : _settings.GlobalSettings().TabSwitcherMode(); if (tabSwitchMode == TabSwitcherMode::Disabled) { uint32_t tabCount = _tabs.Size(); diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index d2afb1491c..2f6348a4fd 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -182,7 +182,7 @@ namespace winrt::TerminalApp::implementation void _RegisterTerminalEvents(Microsoft::Terminal::Control::TermControl term, TerminalTab& hostingTab); - void _SelectNextTab(const bool bMoveRight); + void _SelectNextTab(const bool bMoveRight, const Windows::Foundation::IReference& customTabSwitcherMode); bool _SelectTab(const uint32_t tabIndex); void _MoveFocus(const Microsoft::Terminal::Settings::Model::FocusDirection& direction); diff --git a/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp b/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp index 7069d448b4..e1c5fa6de6 100644 --- a/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp @@ -151,6 +151,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { ShortcutAction::ToggleCommandPalette, ToggleCommandPaletteArgs::FromJson }, { ShortcutAction::FindMatch, FindMatchArgs::FromJson }, { ShortcutAction::NewWindow, NewWindowArgs::FromJson }, + { ShortcutAction::PrevTab, PrevTabArgs::FromJson }, + { ShortcutAction::NextTab, NextTabArgs::FromJson }, { ShortcutAction::Invalid, nullptr }, }; diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.cpp b/src/cascadia/TerminalSettingsModel/ActionArgs.cpp index 5c0ce7f52e..5401c050f6 100644 --- a/src/cascadia/TerminalSettingsModel/ActionArgs.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionArgs.cpp @@ -26,6 +26,8 @@ #include "FindMatchArgs.g.cpp" #include "ToggleCommandPaletteArgs.g.cpp" #include "NewWindowArgs.g.cpp" +#include "PrevTabArgs.g.cpp" +#include "NextTabArgs.g.cpp" #include @@ -534,4 +536,26 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation fmt::format(L"{}, {}", RS_(L"NewWindowCommandKey"), newTerminalArgsStr) }; } + + winrt::hstring PrevTabArgs::GenerateName() const + { + if (!_SwitcherMode) + { + return RS_(L"PrevTabCommandKey"); + } + + const auto mode = _SwitcherMode.Value() == TabSwitcherMode::MostRecentlyUsed ? L"most recently used" : L"in order"; + return winrt::hstring(fmt::format(L"{}, {}", RS_(L"PrevTabCommandKey"), mode)); + } + + winrt::hstring NextTabArgs::GenerateName() const + { + if (!_SwitcherMode) + { + return RS_(L"NextTabCommandKey"); + } + + const auto mode = _SwitcherMode.Value() == TabSwitcherMode::MostRecentlyUsed ? L"most recently used" : L"in order"; + return winrt::hstring(fmt::format(L"{}, {}", RS_(L"NextTabCommandKey"), mode)); + } } diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.h b/src/cascadia/TerminalSettingsModel/ActionArgs.h index cea20a7763..c96d08a25e 100644 --- a/src/cascadia/TerminalSettingsModel/ActionArgs.h +++ b/src/cascadia/TerminalSettingsModel/ActionArgs.h @@ -28,6 +28,8 @@ #include "ToggleCommandPaletteArgs.g.h" #include "FindMatchArgs.g.h" #include "NewWindowArgs.g.h" +#include "PrevTabArgs.g.h" +#include "NextTabArgs.g.h" #include "../../cascadia/inc/cppwinrt_utils.h" #include "JsonUtils.h" @@ -924,6 +926,71 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } }; + struct PrevTabArgs : public PrevTabArgsT + { + PrevTabArgs() = default; + WINRT_PROPERTY(Windows::Foundation::IReference, SwitcherMode, nullptr); + static constexpr std::string_view SwitcherModeKey{ "tabSwitcherMode" }; + + public: + hstring GenerateName() const; + + bool Equals(const IActionArgs& other) + { + auto otherAsUs = other.try_as(); + if (otherAsUs) + { + return otherAsUs->_SwitcherMode == _SwitcherMode; + } + 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, SwitcherModeKey, args->_SwitcherMode); + return { *args, {} }; + } + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_SwitcherMode = _SwitcherMode; + return *copy; + } + }; + + struct NextTabArgs : public NextTabArgsT + { + NextTabArgs() = default; + WINRT_PROPERTY(Windows::Foundation::IReference, SwitcherMode, nullptr); + static constexpr std::string_view SwitcherModeKey{ "tabSwitcherMode" }; + + public: + hstring GenerateName() const; + + bool Equals(const IActionArgs& other) + { + auto otherAsUs = other.try_as(); + if (otherAsUs) + { + return otherAsUs->_SwitcherMode == _SwitcherMode; + } + 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, SwitcherModeKey, args->_SwitcherMode); + return { *args, {} }; + } + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_SwitcherMode = _SwitcherMode; + return *copy; + } + }; } namespace winrt::Microsoft::Terminal::Settings::Model::factory_implementation diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.idl b/src/cascadia/TerminalSettingsModel/ActionArgs.idl index 0d2109fa29..67b76be7b6 100644 --- a/src/cascadia/TerminalSettingsModel/ActionArgs.idl +++ b/src/cascadia/TerminalSettingsModel/ActionArgs.idl @@ -77,6 +77,13 @@ namespace Microsoft.Terminal.Settings.Model CommandLine }; + enum TabSwitcherMode + { + MostRecentlyUsed, + InOrder, + Disabled, + }; + [default_interface] runtimeclass NewTerminalArgs { NewTerminalArgs(); NewTerminalArgs(Int32 profileIndex); @@ -226,4 +233,14 @@ namespace Microsoft.Terminal.Settings.Model NewWindowArgs(NewTerminalArgs terminalArgs); NewTerminalArgs TerminalArgs { get; }; }; + + [default_interface] runtimeclass PrevTabArgs : IActionArgs + { + Windows.Foundation.IReference SwitcherMode; + }; + + [default_interface] runtimeclass NextTabArgs : IActionArgs + { + Windows.Foundation.IReference SwitcherMode; + }; } diff --git a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.idl b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.idl index 12a58a9cae..48542a0b0a 100644 --- a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.idl +++ b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.idl @@ -28,13 +28,6 @@ namespace Microsoft.Terminal.Settings.Model MaximizedFocusMode, }; - enum TabSwitcherMode - { - MostRecentlyUsed, - InOrder, - Disabled, - }; - enum WindowingMode { UseNew,