From 22fd06e19b3e564d448d60ebf34ffd63764f8080 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 4 May 2021 21:50:13 -0700 Subject: [PATCH] Introduce ActionMap to Terminal Settings Model (#9621) This entirely removes `KeyMapping` from the settings model, and builds on the work done in #9543 to consolidate all actions (key bindings and commands) into a unified data structure (`ActionMap`). ## References #9428 - Spec #6900 - Actions page Closes #7441 ## Detailed Description of the Pull Request / Additional comments The important thing here is to remember that we're shifting our philosophy of how to interact/represent actions. Prior to this, the actions arrays in the JSON would be deserialized twice: once for key bindings, and again for commands. By thinking of every entry in the relevant JSON as a `Command`, we can remove a lot of the context switching between working with a key binding vs a command palette item. #9543 allows us to make that shift. Given the work in that PR, we can now deserialize all of the relevant information from each JSON action item. This allows us to simplify `ActionMap::FromJson` to simply iterate over each JSON action item, deserialize it, and add it to our `ActionMap`. Internally, our `ActionMap` operates as discussed in #9428 by maintaining a `_KeyMap` that points to an action ID, and using that action ID to retrieve the `Command` from the `_ActionMap`. Adding actions to the `ActionMap` automatically accounts for name/key-chord collisions. A `NameMap` can be constructed when requested; this is for the Command Palette. Querying the `ActionMap` is fairly straightforward. Helper functions were needed to be able to distinguish an explicit unbinding vs the command not being found in the current layer. Internally, we store explicitly unbound names/key-chords as `ShortcutAction::Invalid` commands. However, we return `nullptr` when a query points to an unbound command. This is done to hide this complexity away from any caller. The command palette still needs special handling for nested and iterable commands. Thankfully, the expansion of iterable commands is performed on an `IMapView`, so we can just expose `NameMap` as a consolidation of `ActionMap`'s `NameMap` with its parents. The same can be said for exposing key chords in nested commands. ## Validation Steps Performed All local tests pass. --- .../spelling/dictionary/dictionary.txt | 2 + .../LocalTests_SettingsModel/CommandTests.cpp | 126 ++-- .../DeserializationTests.cpp | 292 +++++--- .../KeyBindingsTests.cpp | 308 +++++---- .../SettingsModel.LocalTests.vcxproj | 7 + .../TerminalSettingsTests.cpp | 29 +- .../LocalTests_SettingsModel/TestUtils.h | 10 +- .../LocalTests_TerminalApp/SettingsTests.cpp | 136 ++-- .../TerminalApp/ActionPreviewHandlers.cpp | 10 +- src/cascadia/TerminalApp/AppKeyBindings.cpp | 9 +- src/cascadia/TerminalApp/AppKeyBindings.h | 4 +- src/cascadia/TerminalApp/AppKeyBindings.idl | 2 +- src/cascadia/TerminalApp/AppLogic.cpp | 4 +- src/cascadia/TerminalApp/AppLogic.h | 2 +- src/cascadia/TerminalApp/AppLogic.idl | 2 +- src/cascadia/TerminalApp/CommandPalette.cpp | 13 +- src/cascadia/TerminalApp/CommandPalette.h | 4 +- src/cascadia/TerminalApp/CommandPalette.idl | 2 +- src/cascadia/TerminalApp/TabBase.cpp | 8 +- src/cascadia/TerminalApp/TabBase.h | 4 +- src/cascadia/TerminalApp/TabManagement.cpp | 2 +- src/cascadia/TerminalApp/TerminalPage.cpp | 69 +- src/cascadia/TerminalApp/TerminalPage.h | 2 +- .../TerminalSettingsEditor/Actions.cpp | 11 +- src/cascadia/TerminalSettingsEditor/Actions.h | 8 + .../TerminalSettingsEditor/Actions.xaml | 6 +- .../TerminalSettingsEditor/Converters.idl | 1 - .../TerminalSettingsModel/ActionAndArgs.cpp | 20 +- .../TerminalSettingsModel/ActionAndArgs.h | 1 + .../TerminalSettingsModel/ActionArgs.h | 117 ++++ .../TerminalSettingsModel/ActionArgs.idl | 2 + .../TerminalSettingsModel/ActionMap.cpp | 631 ++++++++++++++++++ .../TerminalSettingsModel/ActionMap.h | 106 +++ .../TerminalSettingsModel/ActionMap.idl | 23 + .../ActionMapSerialization.cpp | 80 +++ .../CascadiaSettings.cpp | 8 +- .../TerminalSettingsModel/CascadiaSettings.h | 2 +- .../CascadiaSettings.idl | 2 +- .../CascadiaSettingsSerialization.cpp | 51 -- .../TerminalSettingsModel/Command.cpp | 259 ++++--- src/cascadia/TerminalSettingsModel/Command.h | 25 +- .../TerminalSettingsModel/Command.idl | 30 +- .../GlobalAppSettings.cpp | 42 +- .../TerminalSettingsModel/GlobalAppSettings.h | 9 +- .../GlobalAppSettings.idl | 7 +- .../TerminalSettingsModel/HashUtils.h | 70 ++ .../KeyChordSerialization.cpp | 69 +- .../KeyChordSerialization.h | 10 + .../TerminalSettingsModel/KeyMapping.cpp | 172 ----- .../TerminalSettingsModel/KeyMapping.h | 82 --- .../TerminalSettingsModel/KeyMapping.idl | 41 -- .../KeyMappingSerialization.cpp | 162 ----- ...crosoft.Terminal.Settings.ModelLib.vcxproj | 21 +- ...Terminal.Settings.ModelLib.vcxproj.filters | 2 +- .../Microsoft.Terminal.Settings.Model.vcxproj | 1 + src/cascadia/WindowsTerminal/AppHost.cpp | 6 +- src/cascadia/WindowsTerminal/AppHost.h | 2 +- .../ut_app/TerminalApp.UnitTests.vcxproj | 7 + 58 files changed, 2021 insertions(+), 1112 deletions(-) create mode 100644 src/cascadia/TerminalSettingsModel/ActionMap.cpp create mode 100644 src/cascadia/TerminalSettingsModel/ActionMap.h create mode 100644 src/cascadia/TerminalSettingsModel/ActionMap.idl create mode 100644 src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp create mode 100644 src/cascadia/TerminalSettingsModel/HashUtils.h delete mode 100644 src/cascadia/TerminalSettingsModel/KeyMapping.cpp delete mode 100644 src/cascadia/TerminalSettingsModel/KeyMapping.h delete mode 100644 src/cascadia/TerminalSettingsModel/KeyMapping.idl delete mode 100644 src/cascadia/TerminalSettingsModel/KeyMappingSerialization.cpp diff --git a/.github/actions/spelling/dictionary/dictionary.txt b/.github/actions/spelling/dictionary/dictionary.txt index 2b3236d31dc..acc9f04bf17 100644 --- a/.github/actions/spelling/dictionary/dictionary.txt +++ b/.github/actions/spelling/dictionary/dictionary.txt @@ -345242,6 +345242,8 @@ resequester resequestration reserate reserene +reserialize +reserializes reserialization reserpine reserpinized diff --git a/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp b/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp index 44526b36382..ce5f06014c6 100644 --- a/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp @@ -106,9 +106,9 @@ namespace SettingsModelLocalTests VERIFY_ARE_EQUAL(1u, commands.Size()); auto command = commands.Lookup(L"action0"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::CopyText, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::CopyText, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); } { @@ -117,9 +117,9 @@ namespace SettingsModelLocalTests VERIFY_ARE_EQUAL(1u, commands.Size()); auto command = commands.Lookup(L"action0"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::PasteText, command.Action().Action()); - VERIFY_IS_NULL(command.Action().Args()); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::PasteText, command.ActionAndArgs().Action()); + VERIFY_IS_NULL(command.ActionAndArgs().Args()); } { auto warnings = implementation::Command::LayerJson(commands, commands2Json); @@ -127,9 +127,9 @@ namespace SettingsModelLocalTests VERIFY_ARE_EQUAL(1u, commands.Size()); auto command = commands.Lookup(L"action0"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::NewTab, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::NewTab, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); } { @@ -165,9 +165,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"command1"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); // Verify the args have the expected value VERIFY_ARE_EQUAL(SplitState::Vertical, realArgs.SplitStyle()); @@ -176,9 +176,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"command2"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); // Verify the args have the expected value VERIFY_ARE_EQUAL(SplitState::Horizontal, realArgs.SplitStyle()); @@ -187,9 +187,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"command4"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); // Verify the args have the expected value VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle()); @@ -198,9 +198,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"command5"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); // Verify the args have the expected value VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle()); @@ -209,9 +209,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"command6"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); // Verify the args have the expected value VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle()); @@ -239,9 +239,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"command1"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); // Verify the args have the expected value VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle()); @@ -268,9 +268,9 @@ namespace SettingsModelLocalTests // this test will break. auto command = commands.Lookup(L"Duplicate tab"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::CopyText, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::CopyText, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); } } @@ -309,9 +309,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"Split pane"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); // Verify the args have the expected value VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle()); @@ -319,9 +319,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"Split pane, split: vertical"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); // Verify the args have the expected value VERIFY_ARE_EQUAL(SplitState::Vertical, realArgs.SplitStyle()); @@ -329,9 +329,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"Split pane, split: horizontal"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); // Verify the args have the expected value VERIFY_ARE_EQUAL(SplitState::Horizontal, realArgs.SplitStyle()); @@ -355,9 +355,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"Split pane"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); // Verify the args have the expected value VERIFY_ARE_EQUAL(SplitState::Vertical, realArgs.SplitStyle()); @@ -410,9 +410,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"action0"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::NewWindow, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::NewWindow, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); const auto& terminalArgs = realArgs.TerminalArgs(); VERIFY_IS_NOT_NULL(terminalArgs); @@ -423,9 +423,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"action1"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::NewTab, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::NewTab, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); const auto& terminalArgs = realArgs.TerminalArgs(); VERIFY_IS_NOT_NULL(terminalArgs); @@ -436,9 +436,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"action2"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::NewWindow, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::NewWindow, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); const auto& terminalArgs = realArgs.TerminalArgs(); VERIFY_IS_NOT_NULL(terminalArgs); @@ -449,9 +449,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"action3"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::NewWindow, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::NewWindow, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); const auto& terminalArgs = realArgs.TerminalArgs(); VERIFY_IS_NOT_NULL(terminalArgs); @@ -462,9 +462,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"action4"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::NewWindow, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::NewWindow, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); const auto& terminalArgs = realArgs.TerminalArgs(); VERIFY_IS_NOT_NULL(terminalArgs); @@ -477,9 +477,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"action5"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::NewWindow, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::NewWindow, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); const auto& terminalArgs = realArgs.TerminalArgs(); VERIFY_IS_NOT_NULL(terminalArgs); @@ -492,9 +492,9 @@ namespace SettingsModelLocalTests { auto command = commands.Lookup(L"action6"); VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::NewWindow, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); + VERIFY_IS_NOT_NULL(command.ActionAndArgs()); + VERIFY_ARE_EQUAL(ShortcutAction::NewWindow, command.ActionAndArgs().Action()); + const auto& realArgs = command.ActionAndArgs().Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); const auto& terminalArgs = realArgs.TerminalArgs(); VERIFY_IS_NOT_NULL(terminalArgs); diff --git a/src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp b/src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp index d0a075a7de6..7e5efc04ddb 100644 --- a/src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp @@ -88,6 +88,8 @@ namespace SettingsModelLocalTests TEST_METHOD(TestValidDefaults); + TEST_METHOD(TestInheritedCommand); + TEST_CLASS_SETUP(ClassSetup) { InitializeJsonReader(); @@ -1918,7 +1920,12 @@ namespace SettingsModelLocalTests const auto settingsObject = VerifyParseSucceeded(badSettings); auto settings = implementation::CascadiaSettings::FromJson(settingsObject); - VERIFY_ARE_EQUAL(0u, settings->_globals->_keymap->_keyShortcuts.size()); + // KeyMap: ctrl+a/b are mapped to "invalid" + // ActionMap: "splitPane" and "invalid" are the only deserialized actions + // NameMap: "splitPane" has no key binding, but it is still added to the name map + VERIFY_ARE_EQUAL(2u, settings->_globals->_actionMap->_KeyMap.size()); + VERIFY_ARE_EQUAL(2u, settings->_globals->_actionMap->_ActionMap.size()); + VERIFY_ARE_EQUAL(1u, settings->_globals->_actionMap->NameMap().Size()); VERIFY_ARE_EQUAL(4u, settings->_globals->_keybindingsWarnings.size()); VERIFY_ARE_EQUAL(SettingsLoadWarnings::TooManyKeysForChord, settings->_globals->_keybindingsWarnings.at(0)); @@ -1962,7 +1969,10 @@ namespace SettingsModelLocalTests auto settings = implementation::CascadiaSettings::FromJson(settingsObject); - VERIFY_ARE_EQUAL(0u, settings->_globals->_keymap->_keyShortcuts.size()); + VERIFY_ARE_EQUAL(3u, settings->_globals->_actionMap->_KeyMap.size()); + VERIFY_IS_NULL(settings->_globals->_actionMap->GetActionByKeyChord({ KeyModifiers::Ctrl, static_cast('a') })); + VERIFY_IS_NULL(settings->_globals->_actionMap->GetActionByKeyChord({ KeyModifiers::Ctrl, static_cast('b') })); + VERIFY_IS_NULL(settings->_globals->_actionMap->GetActionByKeyChord({ KeyModifiers::Ctrl, static_cast('c') })); for (const auto& warning : settings->_globals->_keybindingsWarnings) { @@ -2103,18 +2113,18 @@ namespace SettingsModelLocalTests const auto profile2Guid = settings->_allProfiles.GetAt(2).Guid(); VERIFY_ARE_NOT_EQUAL(winrt::guid{}, profile2Guid); - auto keymap = winrt::get_self(settings->_globals->KeyMap()); - VERIFY_ARE_EQUAL(5u, keymap->_keyShortcuts.size()); + auto actionMap = winrt::get_self(settings->_globals->ActionMap()); + VERIFY_ARE_EQUAL(5u, actionMap->_KeyMap.size()); // A/D, B, C, E will be in the list of commands, for 4 total. // * A and D share the same name, so they'll only generate a single action. // * F's name is set manually to `null` - auto commands = settings->_globals->Commands(); - VERIFY_ARE_EQUAL(4u, commands.Size()); + const auto& nameMap{ actionMap->NameMap() }; + VERIFY_ARE_EQUAL(1u, nameMap.Size()); { KeyChord kc{ true, false, false, static_cast('A') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -2131,7 +2141,7 @@ namespace SettingsModelLocalTests { KeyChord kc{ true, false, false, static_cast('C') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -2145,7 +2155,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('D') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -2159,7 +2169,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('E') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -2173,7 +2183,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('F') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -2187,43 +2197,21 @@ namespace SettingsModelLocalTests } Log::Comment(L"Now verify the commands"); - _logCommandNames(commands); + _logCommandNames(nameMap); { - auto command = commands.Lookup(L"Split pane, split: vertical"); - VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); - VERIFY_IS_NOT_NULL(actionAndArgs); - VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); - const auto& realArgs = actionAndArgs.Args().try_as(); - VERIFY_IS_NOT_NULL(realArgs); - // Verify the args have the expected value - VERIFY_ARE_EQUAL(SplitState::Vertical, realArgs.SplitStyle()); - VERIFY_IS_NOT_NULL(realArgs.TerminalArgs()); - VERIFY_IS_TRUE(realArgs.TerminalArgs().Commandline().empty()); - VERIFY_IS_TRUE(realArgs.TerminalArgs().StartingDirectory().empty()); - VERIFY_IS_TRUE(realArgs.TerminalArgs().TabTitle().empty()); - VERIFY_IS_TRUE(realArgs.TerminalArgs().Profile().empty()); + // This was renamed to "ctrl+c" in C. So this does not exist. + auto command = nameMap.TryLookup(L"Split pane, split: vertical"); + VERIFY_IS_NULL(command); } { - auto command = commands.Lookup(L"ctrl+b"); - VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); - VERIFY_IS_NOT_NULL(actionAndArgs); - VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); - const auto& realArgs = actionAndArgs.Args().try_as(); - VERIFY_IS_NOT_NULL(realArgs); - // Verify the args have the expected value - VERIFY_ARE_EQUAL(SplitState::Vertical, realArgs.SplitStyle()); - VERIFY_IS_NOT_NULL(realArgs.TerminalArgs()); - VERIFY_IS_TRUE(realArgs.TerminalArgs().Commandline().empty()); - VERIFY_IS_TRUE(realArgs.TerminalArgs().StartingDirectory().empty()); - VERIFY_IS_TRUE(realArgs.TerminalArgs().TabTitle().empty()); - VERIFY_IS_TRUE(realArgs.TerminalArgs().Profile().empty()); + // This was renamed to "ctrl+c" in C. So this does not exist. + auto command = nameMap.TryLookup(L"ctrl+b"); + VERIFY_IS_NULL(command); } { - auto command = commands.Lookup(L"ctrl+c"); + auto command = nameMap.TryLookup(L"ctrl+c"); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -2237,20 +2225,9 @@ namespace SettingsModelLocalTests VERIFY_IS_TRUE(realArgs.TerminalArgs().Profile().empty()); } { - auto command = commands.Lookup(L"Split pane, split: horizontal"); - VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); - VERIFY_IS_NOT_NULL(actionAndArgs); - VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); - const auto& realArgs = actionAndArgs.Args().try_as(); - VERIFY_IS_NOT_NULL(realArgs); - // Verify the args have the expected value - VERIFY_ARE_EQUAL(SplitState::Horizontal, realArgs.SplitStyle()); - VERIFY_IS_NOT_NULL(realArgs.TerminalArgs()); - VERIFY_IS_TRUE(realArgs.TerminalArgs().Commandline().empty()); - VERIFY_IS_TRUE(realArgs.TerminalArgs().StartingDirectory().empty()); - VERIFY_IS_TRUE(realArgs.TerminalArgs().TabTitle().empty()); - VERIFY_IS_TRUE(realArgs.TerminalArgs().Profile().empty()); + // This was renamed to null (aka removed from the name map) in F. So this does not exist. + auto command = nameMap.TryLookup(L"Split pane, split: horizontal"); + VERIFY_IS_NULL(command); } } @@ -2308,16 +2285,16 @@ namespace SettingsModelLocalTests VERIFY_ARE_EQUAL(0u, settings->_warnings.Size()); VERIFY_ARE_EQUAL(3u, settings->_allProfiles.Size()); - auto commands = settings->_globals->Commands(); settings->_ValidateSettings(); - _logCommandNames(commands); + const auto& nameMap{ settings->ActionMap().NameMap() }; + _logCommandNames(nameMap); VERIFY_ARE_EQUAL(0u, settings->_warnings.Size()); // Because the "parent" command didn't have a name, it couldn't be // placed into the list of commands. It and it's children are just // ignored. - VERIFY_ARE_EQUAL(0u, commands.Size()); + VERIFY_ARE_EQUAL(0u, nameMap.Size()); } void DeserializationTests::TestNestedCommandWithBadSubCommands() @@ -2358,13 +2335,13 @@ namespace SettingsModelLocalTests auto settings = winrt::make_self(); settings->_ParseJsonString(settingsJson, false); settings->LayerJson(settings->_userSettings); - auto commands = settings->_globals->Commands(); settings->_ValidateSettings(); VERIFY_ARE_EQUAL(2u, settings->_warnings.Size()); VERIFY_ARE_EQUAL(SettingsLoadWarnings::AtLeastOneKeybindingWarning, settings->_warnings.GetAt(0)); VERIFY_ARE_EQUAL(SettingsLoadWarnings::FailedToParseSubCommands, settings->_warnings.GetAt(1)); - VERIFY_ARE_EQUAL(0u, commands.Size()); + const auto& nameMap{ settings->ActionMap().NameMap() }; + VERIFY_ARE_EQUAL(0u, nameMap.Size()); } void DeserializationTests::TestUnbindNestedCommand() @@ -2432,22 +2409,23 @@ namespace SettingsModelLocalTests VERIFY_ARE_EQUAL(0u, settings->_warnings.Size()); VERIFY_ARE_EQUAL(3u, settings->_allProfiles.Size()); - auto commands = settings->_globals->Commands(); settings->_ValidateSettings(); - _logCommandNames(commands); + auto nameMap{ settings->ActionMap().NameMap() }; + _logCommandNames(nameMap); VERIFY_ARE_EQUAL(0u, settings->_warnings.Size()); - VERIFY_ARE_EQUAL(1u, commands.Size()); + VERIFY_ARE_EQUAL(1u, nameMap.Size()); Log::Comment(L"Layer second bit of json, to unbind the original command."); settings->_ParseJsonString(settings1Json, false); settings->LayerJson(settings->_userSettings); settings->_ValidateSettings(); - commands = settings->_globals->Commands(); - _logCommandNames(commands); + + nameMap = settings->ActionMap().NameMap(); + _logCommandNames(nameMap); VERIFY_ARE_EQUAL(0u, settings->_warnings.Size()); - VERIFY_ARE_EQUAL(0u, commands.Size()); + VERIFY_ARE_EQUAL(0u, nameMap.Size()); } void DeserializationTests::TestRebindNestedCommand() @@ -2516,16 +2494,17 @@ namespace SettingsModelLocalTests VERIFY_ARE_EQUAL(0u, settings->_warnings.Size()); VERIFY_ARE_EQUAL(3u, settings->_allProfiles.Size()); - auto commands = settings->_globals->Commands(); + const auto& actionMap{ settings->ActionMap() }; settings->_ValidateSettings(); - _logCommandNames(commands); + auto nameMap{ actionMap.NameMap() }; + _logCommandNames(nameMap); VERIFY_ARE_EQUAL(0u, settings->_warnings.Size()); - VERIFY_ARE_EQUAL(1u, commands.Size()); + VERIFY_ARE_EQUAL(1u, nameMap.Size()); { winrt::hstring commandName{ L"parent" }; - auto commandProj = commands.Lookup(commandName); + auto commandProj = nameMap.TryLookup(commandName); VERIFY_IS_NOT_NULL(commandProj); winrt::com_ptr commandImpl; @@ -2539,17 +2518,18 @@ namespace SettingsModelLocalTests settings->_ParseJsonString(settings1Json, false); settings->LayerJson(settings->_userSettings); settings->_ValidateSettings(); - commands = settings->_globals->Commands(); - _logCommandNames(commands); + + nameMap = settings->ActionMap().NameMap(); + _logCommandNames(nameMap); VERIFY_ARE_EQUAL(0u, settings->_warnings.Size()); - VERIFY_ARE_EQUAL(1u, commands.Size()); + VERIFY_ARE_EQUAL(1u, nameMap.Size()); { winrt::hstring commandName{ L"parent" }; - auto commandProj = commands.Lookup(commandName); + auto commandProj = nameMap.TryLookup(commandName); VERIFY_IS_NOT_NULL(commandProj); - auto actionAndArgs = commandProj.Action(); + auto actionAndArgs = commandProj.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -2642,8 +2622,10 @@ namespace SettingsModelLocalTests VERIFY_ARE_EQUAL(settings->_globals->_colorSchemes.HasKey(schemeName), copyImpl->_globals->_colorSchemes.HasKey(schemeName)); // test actions - VERIFY_ARE_EQUAL(settings->_globals->_keymap->_keyShortcuts.size(), copyImpl->_globals->_keymap->_keyShortcuts.size()); - VERIFY_ARE_EQUAL(settings->_globals->_commands.Size(), copyImpl->_globals->_commands.Size()); + VERIFY_ARE_EQUAL(settings->_globals->_actionMap->_KeyMap.size(), copyImpl->_globals->_actionMap->_KeyMap.size()); + const auto& nameMapOriginal{ settings->_globals->_actionMap->NameMap() }; + const auto& nameMapCopy{ copyImpl->_globals->_actionMap->NameMap() }; + VERIFY_ARE_EQUAL(nameMapOriginal.Size(), nameMapCopy.Size()); // Test that changing the copy should not change the original VERIFY_ARE_EQUAL(settings->_globals->WordDelimiters(), copyImpl->_globals->WordDelimiters()); @@ -2781,4 +2763,158 @@ namespace SettingsModelLocalTests VERIFY_ARE_EQUAL(settings.ActiveProfiles().Size(), settings.AllProfiles().Size()); VERIFY_ARE_EQUAL(settings.AllProfiles().Size(), 2u); } + + void DeserializationTests::TestInheritedCommand() + { + // Test unbinding a command's key chord or name that originated in another layer. + + const std::string settings1Json{ R"( + { + "defaultProfile": "{6239a42c-0000-49a3-80bd-e8fdd045185c}", + "profiles": [ + { + "name": "profile0", + "guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}", + "historySize": 1, + "commandline": "cmd.exe" + }, + { + "name": "profile1", + "guid": "{6239a42c-1111-49a3-80bd-e8fdd045185c}", + "historySize": 2, + "commandline": "pwsh.exe" + }, + { + "name": "profile2", + "historySize": 3, + "commandline": "wsl.exe" + } + ], + "actions": [ + { + "name": "foo", + "command": "closePane", + "keys": "ctrl+shift+w" + } + ], + "schemes": [ { "name": "Campbell" } ] // This is included here to prevent settings validation errors. + })" }; + + const std::string settings2Json{ R"( + { + "defaultProfile": "{6239a42c-0000-49a3-80bd-e8fdd045185c}", + "actions": [ + { + "command": null, + "keys": "ctrl+shift+w" + }, + ], + })" }; + + const std::string settings3Json{ R"( + { + "defaultProfile": "{6239a42c-0000-49a3-80bd-e8fdd045185c}", + "actions": [ + { + "name": "bar", + "command": "closePane" + }, + ], + })" }; + + VerifyParseSucceeded(settings1Json); + VerifyParseSucceeded(settings2Json); + VerifyParseSucceeded(settings3Json); + + auto settings = winrt::make_self(); + settings->_ParseJsonString(settings1Json, false); + settings->LayerJson(settings->_userSettings); + + VERIFY_ARE_EQUAL(0u, settings->_warnings.Size()); + VERIFY_ARE_EQUAL(3u, settings->_allProfiles.Size()); + + settings->_ValidateSettings(); + auto nameMap{ settings->ActionMap().NameMap() }; + _logCommandNames(nameMap); + + VERIFY_ARE_EQUAL(0u, settings->_warnings.Size()); + VERIFY_ARE_EQUAL(1u, nameMap.Size()); + + const KeyChord expectedKeyChord{ true, false, true, static_cast('W') }; + { + // Verify NameMap returns correct value + const auto& cmd{ nameMap.TryLookup(L"foo") }; + VERIFY_IS_NOT_NULL(cmd); + VERIFY_IS_NOT_NULL(cmd.Keys()); + VERIFY_IS_TRUE(cmd.Keys().Modifiers() == expectedKeyChord.Modifiers() && cmd.Keys().Vkey() == expectedKeyChord.Vkey()); + } + { + // Verify ActionMap::GetActionByKeyChord API + const auto& cmd{ settings->ActionMap().GetActionByKeyChord(expectedKeyChord) }; + VERIFY_IS_NOT_NULL(cmd); + VERIFY_IS_NOT_NULL(cmd.Keys()); + VERIFY_IS_TRUE(cmd.Keys().Modifiers() == expectedKeyChord.Modifiers() && cmd.Keys().Vkey() == expectedKeyChord.Vkey()); + } + { + // Verify ActionMap::GetKeyBindingForAction API + const auto& actualKeyChord{ settings->ActionMap().GetKeyBindingForAction(ShortcutAction::ClosePane) }; + VERIFY_IS_NOT_NULL(actualKeyChord); + VERIFY_IS_TRUE(actualKeyChord.Modifiers() == expectedKeyChord.Modifiers() && actualKeyChord.Vkey() == expectedKeyChord.Vkey()); + } + + Log::Comment(L"Layer second bit of json, to unbind the key chord of the original command."); + + settings->_ParseJsonString(settings2Json, false); + settings->LayerJson(settings->_userSettings); + settings->_ValidateSettings(); + + nameMap = settings->ActionMap().NameMap(); + _logCommandNames(nameMap); + VERIFY_ARE_EQUAL(0u, settings->_warnings.Size()); + VERIFY_ARE_EQUAL(1u, nameMap.Size()); + { + // Verify NameMap returns correct value + const auto& cmd{ nameMap.TryLookup(L"foo") }; + VERIFY_IS_NOT_NULL(cmd); + VERIFY_IS_NULL(cmd.Keys()); + } + { + // Verify ActionMap::GetActionByKeyChord API + const auto& cmd{ settings->ActionMap().GetActionByKeyChord(expectedKeyChord) }; + VERIFY_IS_NULL(cmd); + } + { + // Verify ActionMap::GetKeyBindingForAction API + const auto& actualKeyChord{ settings->ActionMap().GetKeyBindingForAction(ShortcutAction::ClosePane) }; + VERIFY_IS_NULL(actualKeyChord); + } + + Log::Comment(L"Layer third bit of json, to unbind name of the original command."); + + settings->_ParseJsonString(settings3Json, false); + settings->LayerJson(settings->_userSettings); + settings->_ValidateSettings(); + + nameMap = settings->ActionMap().NameMap(); + _logCommandNames(nameMap); + VERIFY_ARE_EQUAL(0u, settings->_warnings.Size()); + VERIFY_ARE_EQUAL(1u, nameMap.Size()); + { + // Verify NameMap returns correct value + const auto& cmd{ nameMap.TryLookup(L"bar") }; + VERIFY_IS_NOT_NULL(cmd); + VERIFY_IS_NULL(cmd.Keys()); + VERIFY_ARE_EQUAL(L"bar", cmd.Name()); + } + { + // Verify ActionMap::GetActionByKeyChord API + const auto& cmd{ settings->ActionMap().GetActionByKeyChord(expectedKeyChord) }; + VERIFY_IS_NULL(cmd); + } + { + // Verify ActionMap::GetKeyBindingForAction API + const auto& actualKeyChord{ settings->ActionMap().GetKeyBindingForAction(ShortcutAction::ClosePane) }; + VERIFY_IS_NULL(actualKeyChord); + } + } } diff --git a/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp b/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp index 506c6fde610..35fde481bea 100644 --- a/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp @@ -5,7 +5,7 @@ #include "../TerminalSettingsModel/ColorScheme.h" #include "../TerminalSettingsModel/CascadiaSettings.h" -#include "../TerminalSettingsModel/KeyMapping.h" +#include "../TerminalSettingsModel/ActionMap.h" #include "JsonTestClass.h" #include "TestUtils.h" @@ -51,6 +51,8 @@ namespace SettingsModelLocalTests TEST_METHOD(TestToggleCommandPaletteArgs); TEST_METHOD(TestMoveTabArgs); + TEST_METHOD(TestGetKeyBindingForAction); + TEST_CLASS_SETUP(ClassSetup) { InitializeJsonReader(); @@ -71,18 +73,18 @@ namespace SettingsModelLocalTests const auto bindings1Json = VerifyParseSucceeded(bindings1String); const auto bindings2Json = VerifyParseSucceeded(bindings2String); - auto keymap = winrt::make_self(); - VERIFY_IS_NOT_NULL(keymap); - VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size()); + auto actionMap = winrt::make_self(); + VERIFY_IS_NOT_NULL(actionMap); + VERIFY_ARE_EQUAL(0u, actionMap->_KeyMap.size()); - keymap->LayerJson(bindings0Json); - VERIFY_ARE_EQUAL(1u, keymap->_keyShortcuts.size()); + actionMap->LayerJson(bindings0Json); + VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); - keymap->LayerJson(bindings1Json); - VERIFY_ARE_EQUAL(2u, keymap->_keyShortcuts.size()); + actionMap->LayerJson(bindings1Json); + VERIFY_ARE_EQUAL(2u, actionMap->_KeyMap.size()); - keymap->LayerJson(bindings2Json); - VERIFY_ARE_EQUAL(4u, keymap->_keyShortcuts.size()); + actionMap->LayerJson(bindings2Json); + VERIFY_ARE_EQUAL(4u, actionMap->_KeyMap.size()); } void KeyBindingsTests::LayerKeybindings() @@ -95,18 +97,18 @@ namespace SettingsModelLocalTests const auto bindings1Json = VerifyParseSucceeded(bindings1String); const auto bindings2Json = VerifyParseSucceeded(bindings2String); - auto keymap = winrt::make_self(); - VERIFY_IS_NOT_NULL(keymap); - VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size()); + auto actionMap = winrt::make_self(); + VERIFY_IS_NOT_NULL(actionMap); + VERIFY_ARE_EQUAL(0u, actionMap->_KeyMap.size()); - keymap->LayerJson(bindings0Json); - VERIFY_ARE_EQUAL(1u, keymap->_keyShortcuts.size()); + actionMap->LayerJson(bindings0Json); + VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); - keymap->LayerJson(bindings1Json); - VERIFY_ARE_EQUAL(1u, keymap->_keyShortcuts.size()); + actionMap->LayerJson(bindings1Json); + VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); - keymap->LayerJson(bindings2Json); - VERIFY_ARE_EQUAL(2u, keymap->_keyShortcuts.size()); + actionMap->LayerJson(bindings2Json); + VERIFY_ARE_EQUAL(2u, actionMap->_KeyMap.size()); } void KeyBindingsTests::UnbindKeybindings() @@ -125,52 +127,57 @@ namespace SettingsModelLocalTests const auto bindings4Json = VerifyParseSucceeded(bindings4String); const auto bindings5Json = VerifyParseSucceeded(bindings5String); - auto keymap = winrt::make_self(); - VERIFY_IS_NOT_NULL(keymap); - VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size()); + auto actionMap = winrt::make_self(); + VERIFY_IS_NOT_NULL(actionMap); + VERIFY_ARE_EQUAL(0u, actionMap->_KeyMap.size()); - keymap->LayerJson(bindings0Json); - VERIFY_ARE_EQUAL(1u, keymap->_keyShortcuts.size()); + actionMap->LayerJson(bindings0Json); + VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); - keymap->LayerJson(bindings1Json); - VERIFY_ARE_EQUAL(1u, keymap->_keyShortcuts.size()); + actionMap->LayerJson(bindings1Json); + VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); Log::Comment(NoThrowString().Format( L"Try unbinding a key using `\"unbound\"` to unbind the key")); - keymap->LayerJson(bindings2Json); - VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size()); + actionMap->LayerJson(bindings2Json); + VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); + VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ KeyModifiers::Ctrl, static_cast('c') })); Log::Comment(NoThrowString().Format( L"Try unbinding a key using `null` to unbind the key")); // First add back a good binding - keymap->LayerJson(bindings0Json); - VERIFY_ARE_EQUAL(1u, keymap->_keyShortcuts.size()); + actionMap->LayerJson(bindings0Json); + VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); // Then try layering in the bad setting - keymap->LayerJson(bindings3Json); - VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size()); + actionMap->LayerJson(bindings3Json); + VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); + VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ KeyModifiers::Ctrl, static_cast('c') })); Log::Comment(NoThrowString().Format( L"Try unbinding a key using an unrecognized command to unbind the key")); // First add back a good binding - keymap->LayerJson(bindings0Json); - VERIFY_ARE_EQUAL(1u, keymap->_keyShortcuts.size()); + actionMap->LayerJson(bindings0Json); + VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); // Then try layering in the bad setting - keymap->LayerJson(bindings4Json); - VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size()); + actionMap->LayerJson(bindings4Json); + VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); + VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ KeyModifiers::Ctrl, static_cast('c') })); Log::Comment(NoThrowString().Format( L"Try unbinding a key using a straight up invalid value to unbind the key")); // First add back a good binding - keymap->LayerJson(bindings0Json); - VERIFY_ARE_EQUAL(1u, keymap->_keyShortcuts.size()); + actionMap->LayerJson(bindings0Json); + VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); // Then try layering in the bad setting - keymap->LayerJson(bindings5Json); - VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size()); + actionMap->LayerJson(bindings5Json); + VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); + VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ KeyModifiers::Ctrl, static_cast('c') })); Log::Comment(NoThrowString().Format( L"Try unbinding a key that wasn't bound at all")); - keymap->LayerJson(bindings2Json); - VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size()); + actionMap->LayerJson(bindings2Json); + VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); + VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ KeyModifiers::Ctrl, static_cast('c') })); } void KeyBindingsTests::TestArbitraryArgs() @@ -194,17 +201,17 @@ namespace SettingsModelLocalTests const auto bindings0Json = VerifyParseSucceeded(bindings0String); - auto keymap = winrt::make_self(); - VERIFY_IS_NOT_NULL(keymap); - VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size()); - keymap->LayerJson(bindings0Json); - VERIFY_ARE_EQUAL(10u, keymap->_keyShortcuts.size()); + auto actionMap = winrt::make_self(); + VERIFY_IS_NOT_NULL(actionMap); + VERIFY_ARE_EQUAL(0u, actionMap->_KeyMap.size()); + actionMap->LayerJson(bindings0Json); + VERIFY_ARE_EQUAL(10u, actionMap->_KeyMap.size()); { Log::Comment(NoThrowString().Format( L"Verify that `copy` without args parses as Copy(SingleLine=false)")); KeyChord kc{ true, false, false, static_cast('C') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); // Verify the args have the expected value @@ -215,7 +222,7 @@ namespace SettingsModelLocalTests Log::Comment(NoThrowString().Format( L"Verify that `copy` with args parses them correctly")); KeyChord kc{ true, false, true, static_cast('C') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); // Verify the args have the expected value @@ -226,7 +233,7 @@ namespace SettingsModelLocalTests Log::Comment(NoThrowString().Format( L"Verify that `copy` with args parses them correctly")); KeyChord kc{ false, true, true, static_cast('C') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); // Verify the args have the expected value @@ -237,7 +244,7 @@ namespace SettingsModelLocalTests Log::Comment(NoThrowString().Format( L"Verify that `newTab` without args parses as NewTab(Index=null)")); KeyChord kc{ true, false, false, static_cast('T') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -249,7 +256,7 @@ namespace SettingsModelLocalTests Log::Comment(NoThrowString().Format( L"Verify that `newTab` parses args correctly")); KeyChord kc{ true, false, true, static_cast('T') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -263,7 +270,7 @@ namespace SettingsModelLocalTests L"Verify that `newTab` with an index greater than the legacy " L"args afforded parses correctly")); KeyChord kc{ true, false, true, static_cast('Y') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -277,7 +284,7 @@ namespace SettingsModelLocalTests Log::Comment(NoThrowString().Format( L"Verify that `copy` ignores args it doesn't understand")); KeyChord kc{ true, false, true, static_cast('B') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::CopyText, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -289,7 +296,7 @@ namespace SettingsModelLocalTests Log::Comment(NoThrowString().Format( L"Verify that `copy` null as it's `args` parses as the default option")); KeyChord kc{ true, false, true, static_cast('B') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::CopyText, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -301,7 +308,7 @@ namespace SettingsModelLocalTests Log::Comment(NoThrowString().Format( L"Verify that `adjustFontSize` with a positive delta parses args correctly")); KeyChord kc{ true, false, false, static_cast('F') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::AdjustFontSize, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -313,7 +320,7 @@ namespace SettingsModelLocalTests Log::Comment(NoThrowString().Format( L"Verify that `adjustFontSize` with a negative delta parses args correctly")); KeyChord kc{ true, false, false, static_cast('G') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::AdjustFontSize, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -333,15 +340,15 @@ namespace SettingsModelLocalTests const auto bindings0Json = VerifyParseSucceeded(bindings0String); - auto keymap = winrt::make_self(); - VERIFY_IS_NOT_NULL(keymap); - VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size()); - keymap->LayerJson(bindings0Json); - VERIFY_ARE_EQUAL(4u, keymap->_keyShortcuts.size()); + auto actionMap = winrt::make_self(); + VERIFY_IS_NOT_NULL(actionMap); + VERIFY_ARE_EQUAL(0u, actionMap->_KeyMap.size()); + actionMap->LayerJson(bindings0Json); + VERIFY_ARE_EQUAL(4u, actionMap->_KeyMap.size()); { KeyChord kc{ true, false, false, static_cast('D') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -350,7 +357,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('E') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -359,7 +366,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('G') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -368,7 +375,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('H') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -387,15 +394,15 @@ namespace SettingsModelLocalTests const auto bindings0Json = VerifyParseSucceeded(bindings0String); - auto keymap = winrt::make_self(); - VERIFY_IS_NOT_NULL(keymap); - VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size()); - keymap->LayerJson(bindings0Json); - VERIFY_ARE_EQUAL(3u, keymap->_keyShortcuts.size()); + auto actionMap = winrt::make_self(); + VERIFY_IS_NOT_NULL(actionMap); + VERIFY_ARE_EQUAL(0u, actionMap->_KeyMap.size()); + actionMap->LayerJson(bindings0Json); + VERIFY_ARE_EQUAL(3u, actionMap->_KeyMap.size()); { KeyChord kc{ true, false, false, static_cast('C') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SetTabColor, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -404,7 +411,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('D') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SetTabColor, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -415,7 +422,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('F') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SetTabColor, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -432,15 +439,15 @@ namespace SettingsModelLocalTests const auto bindings0Json = VerifyParseSucceeded(bindings0String); - auto keymap = winrt::make_self(); - VERIFY_IS_NOT_NULL(keymap); - VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size()); - keymap->LayerJson(bindings0Json); - VERIFY_ARE_EQUAL(1u, keymap->_keyShortcuts.size()); + auto actionMap = winrt::make_self(); + VERIFY_IS_NOT_NULL(actionMap); + VERIFY_ARE_EQUAL(0u, actionMap->_KeyMap.size()); + actionMap->LayerJson(bindings0Json); + VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); { KeyChord kc{ true, false, false, static_cast('C') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); // Verify the args have the expected value @@ -461,15 +468,15 @@ namespace SettingsModelLocalTests const auto bindings0Json = VerifyParseSucceeded(bindings0String); - auto keymap = winrt::make_self(); - VERIFY_IS_NOT_NULL(keymap); - VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size()); - keymap->LayerJson(bindings0Json); - VERIFY_ARE_EQUAL(6u, keymap->_keyShortcuts.size()); + auto actionMap = winrt::make_self(); + VERIFY_IS_NOT_NULL(actionMap); + VERIFY_ARE_EQUAL(0u, actionMap->_KeyMap.size()); + actionMap->LayerJson(bindings0Json); + VERIFY_ARE_EQUAL(6u, actionMap->_KeyMap.size()); { KeyChord kc{ false, false, false, static_cast(VK_UP) }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::ScrollUp, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -478,7 +485,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ false, false, false, static_cast(VK_DOWN) }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::ScrollDown, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -487,7 +494,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast(VK_UP) }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::ScrollUp, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -496,7 +503,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast(VK_DOWN) }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::ScrollDown, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -505,7 +512,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, true, static_cast(VK_UP) }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::ScrollUp, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -515,7 +522,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, true, static_cast(VK_DOWN) }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::ScrollDown, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -526,10 +533,10 @@ namespace SettingsModelLocalTests { const std::string bindingsInvalidString{ R"([{ "keys": ["up"], "command": { "action": "scrollDown", "rowsToScroll": -1 } }])" }; const auto bindingsInvalidJson = VerifyParseSucceeded(bindingsInvalidString); - auto invalidKeyMap = winrt::make_self(); - VERIFY_IS_NOT_NULL(invalidKeyMap); - VERIFY_ARE_EQUAL(0u, invalidKeyMap->_keyShortcuts.size()); - VERIFY_THROWS(invalidKeyMap->LayerJson(bindingsInvalidJson);, std::exception); + auto invalidActionMap = winrt::make_self(); + VERIFY_IS_NOT_NULL(invalidActionMap); + VERIFY_ARE_EQUAL(0u, invalidActionMap->_KeyMap.size()); + VERIFY_THROWS(invalidActionMap->LayerJson(bindingsInvalidJson);, std::exception); } } @@ -542,15 +549,15 @@ namespace SettingsModelLocalTests const auto bindings0Json = VerifyParseSucceeded(bindings0String); - auto keymap = winrt::make_self(); - VERIFY_IS_NOT_NULL(keymap); - VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size()); - keymap->LayerJson(bindings0Json); - VERIFY_ARE_EQUAL(2u, keymap->_keyShortcuts.size()); + auto actionMap = winrt::make_self(); + VERIFY_IS_NOT_NULL(actionMap); + VERIFY_ARE_EQUAL(0u, actionMap->_KeyMap.size()); + actionMap->LayerJson(bindings0Json); + VERIFY_ARE_EQUAL(2u, actionMap->_KeyMap.size()); { KeyChord kc{ false, false, false, static_cast(VK_UP) }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::MoveTab, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -559,7 +566,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ false, false, false, static_cast(VK_DOWN) }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::MoveTab, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -568,17 +575,17 @@ namespace SettingsModelLocalTests } { const std::string bindingsInvalidString{ R"([{ "keys": ["up"], "command": "moveTab" }])" }; - auto keyMapNoArgs = winrt::make_self(); - keyMapNoArgs->LayerJson(bindingsInvalidString); - VERIFY_ARE_EQUAL(0u, keyMapNoArgs->_keyShortcuts.size()); + auto actionMapNoArgs = winrt::make_self(); + actionMapNoArgs->LayerJson(bindingsInvalidString); + VERIFY_ARE_EQUAL(0u, actionMapNoArgs->_KeyMap.size()); } { const std::string bindingsInvalidString{ R"([{ "keys": ["up"], "command": { "action": "moveTab", "direction": "bad" } }])" }; const auto bindingsInvalidJson = VerifyParseSucceeded(bindingsInvalidString); - auto invalidKeyMap = winrt::make_self(); - VERIFY_IS_NOT_NULL(invalidKeyMap); - VERIFY_ARE_EQUAL(0u, invalidKeyMap->_keyShortcuts.size()); - VERIFY_THROWS(invalidKeyMap->LayerJson(bindingsInvalidJson);, std::exception); + auto invalidActionMap = winrt::make_self(); + VERIFY_IS_NOT_NULL(invalidActionMap); + VERIFY_ARE_EQUAL(0u, invalidActionMap->_KeyMap.size()); + VERIFY_THROWS(invalidActionMap->LayerJson(bindingsInvalidJson);, std::exception); } } @@ -592,15 +599,15 @@ namespace SettingsModelLocalTests const auto bindings0Json = VerifyParseSucceeded(bindings0String); - auto keymap = winrt::make_self(); - VERIFY_IS_NOT_NULL(keymap); - VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size()); - keymap->LayerJson(bindings0Json); - VERIFY_ARE_EQUAL(3u, keymap->_keyShortcuts.size()); + auto actionMap = winrt::make_self(); + VERIFY_IS_NOT_NULL(actionMap); + VERIFY_ARE_EQUAL(0u, actionMap->_KeyMap.size()); + actionMap->LayerJson(bindings0Json); + VERIFY_ARE_EQUAL(3u, actionMap->_KeyMap.size()); { KeyChord kc{ false, false, false, static_cast(VK_UP) }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::ToggleCommandPalette, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -609,7 +616,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast(VK_UP) }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::ToggleCommandPalette, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -618,7 +625,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, true, static_cast(VK_UP) }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::ToggleCommandPalette, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -628,10 +635,69 @@ namespace SettingsModelLocalTests { const std::string bindingsInvalidString{ R"([{ "keys": ["up"], "command": { "action": "commandPalette", "launchMode": "bad" } }])" }; const auto bindingsInvalidJson = VerifyParseSucceeded(bindingsInvalidString); - auto invalidKeyMap = winrt::make_self(); - VERIFY_IS_NOT_NULL(invalidKeyMap); - VERIFY_ARE_EQUAL(0u, invalidKeyMap->_keyShortcuts.size()); - VERIFY_THROWS(invalidKeyMap->LayerJson(bindingsInvalidJson);, std::exception); + auto invalidActionMap = winrt::make_self(); + VERIFY_IS_NOT_NULL(invalidActionMap); + VERIFY_ARE_EQUAL(0u, invalidActionMap->_KeyMap.size()); + VERIFY_THROWS(invalidActionMap->LayerJson(bindingsInvalidJson);, std::exception); + } + } + + void KeyBindingsTests::TestGetKeyBindingForAction() + { + const std::string bindings0String{ R"([ { "command": "closeWindow", "keys": "ctrl+a" } ])" }; + const std::string bindings1String{ R"([ { "command": { "action": "copy", "singleLine": true }, "keys": "ctrl+b" } ])" }; + const std::string bindings2String{ R"([ { "command": { "action": "newTab", "index": 0 }, "keys": "ctrl+c" } ])" }; + + const auto bindings0Json = VerifyParseSucceeded(bindings0String); + const auto bindings1Json = VerifyParseSucceeded(bindings1String); + const auto bindings2Json = VerifyParseSucceeded(bindings2String); + + auto VerifyKeyChordEquality = [](const KeyChord& expected, const KeyChord& actual) { + if (expected) + { + VERIFY_IS_NOT_NULL(actual); + VERIFY_ARE_EQUAL(expected.Modifiers(), actual.Modifiers()); + VERIFY_ARE_EQUAL(expected.Vkey(), actual.Vkey()); + } + else + { + VERIFY_IS_NULL(actual); + } + }; + + auto actionMap = winrt::make_self(); + VERIFY_IS_NOT_NULL(actionMap); + VERIFY_ARE_EQUAL(0u, actionMap->_KeyMap.size()); + + { + Log::Comment(L"simple command: no args"); + actionMap->LayerJson(bindings0Json); + VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); + const auto& kbd{ actionMap->GetKeyBindingForAction(ShortcutAction::CloseWindow) }; + VerifyKeyChordEquality({ KeyModifiers::Ctrl, static_cast('A') }, kbd); + } + { + Log::Comment(L"command with args"); + actionMap->LayerJson(bindings1Json); + VERIFY_ARE_EQUAL(2u, actionMap->_KeyMap.size()); + + auto args{ winrt::make_self() }; + args->SingleLine(true); + + const auto& kbd{ actionMap->GetKeyBindingForAction(ShortcutAction::CopyText, *args) }; + VerifyKeyChordEquality({ KeyModifiers::Ctrl, static_cast('B') }, kbd); + } + { + Log::Comment(L"command with new terminal args"); + actionMap->LayerJson(bindings2Json); + VERIFY_ARE_EQUAL(3u, actionMap->_KeyMap.size()); + + auto newTerminalArgs{ winrt::make_self() }; + newTerminalArgs->ProfileIndex(0); + auto args{ winrt::make_self(*newTerminalArgs) }; + + const auto& kbd{ actionMap->GetKeyBindingForAction(ShortcutAction::NewTab, *args) }; + VerifyKeyChordEquality({ KeyModifiers::Ctrl, static_cast('C') }, kbd); } } } diff --git a/src/cascadia/LocalTests_SettingsModel/SettingsModel.LocalTests.vcxproj b/src/cascadia/LocalTests_SettingsModel/SettingsModel.LocalTests.vcxproj index 7de78c0418b..72ac1a87e27 100644 --- a/src/cascadia/LocalTests_SettingsModel/SettingsModel.LocalTests.vcxproj +++ b/src/cascadia/LocalTests_SettingsModel/SettingsModel.LocalTests.vcxproj @@ -77,6 +77,13 @@ onecoreuap.lib;%(AdditionalDependencies) + + /INCLUDE:_DllMain@12 + /INCLUDE:DllMain diff --git a/src/cascadia/LocalTests_SettingsModel/TerminalSettingsTests.cpp b/src/cascadia/LocalTests_SettingsModel/TerminalSettingsTests.cpp index e46d4eb78d3..459fadc18fe 100644 --- a/src/cascadia/LocalTests_SettingsModel/TerminalSettingsTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/TerminalSettingsTests.cpp @@ -103,17 +103,18 @@ namespace SettingsModelLocalTests CascadiaSettings settings{ til::u8u16(settingsJson) }; - auto keymap = settings.GlobalSettings().KeyMap(); + auto actionMap = settings.GlobalSettings().ActionMap(); VERIFY_ARE_EQUAL(3u, settings.ActiveProfiles().Size()); const auto profile2Guid = settings.ActiveProfiles().GetAt(2).Guid(); VERIFY_ARE_NOT_EQUAL(winrt::guid{}, profile2Guid); - VERIFY_ARE_EQUAL(12u, keymap.Size()); + const auto& actionMapImpl{ winrt::get_self(actionMap) }; + VERIFY_ARE_EQUAL(12u, actionMapImpl->_KeyMap.size()); { KeyChord kc{ true, false, false, static_cast('A') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -134,7 +135,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('B') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -156,7 +157,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('C') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -178,7 +179,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('D') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -200,7 +201,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('E') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -222,7 +223,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('F') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -245,7 +246,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('G') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -265,7 +266,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('H') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -287,7 +288,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('I') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -310,7 +311,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('J') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -332,7 +333,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('K') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); @@ -355,7 +356,7 @@ namespace SettingsModelLocalTests } { KeyChord kc{ true, false, false, static_cast('L') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc); + auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc); VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); VERIFY_IS_NOT_NULL(realArgs); diff --git a/src/cascadia/LocalTests_SettingsModel/TestUtils.h b/src/cascadia/LocalTests_SettingsModel/TestUtils.h index 12c8dd4f9d3..e16e55441a7 100644 --- a/src/cascadia/LocalTests_SettingsModel/TestUtils.h +++ b/src/cascadia/LocalTests_SettingsModel/TestUtils.h @@ -19,11 +19,11 @@ class TestUtils // - This is a helper to retrieve the ActionAndArgs from the keybindings // for a given chord. // Arguments: - // - keymap: The AppKeyBindings to lookup the ActionAndArgs from. + // - actionMap: The ActionMap to lookup the ActionAndArgs from. // - kc: The key chord to look up the bound ActionAndArgs for. // Return Value: // - The ActionAndArgs bound to the given key, or nullptr if nothing is bound to it. - static const winrt::Microsoft::Terminal::Settings::Model::ActionAndArgs GetActionAndArgs(const winrt::Microsoft::Terminal::Settings::Model::KeyMapping& keymap, + static const winrt::Microsoft::Terminal::Settings::Model::ActionAndArgs GetActionAndArgs(const winrt::Microsoft::Terminal::Settings::Model::ActionMap& actionMap, const winrt::Microsoft::Terminal::Control::KeyChord& kc) { std::wstring buffer{ L"" }; @@ -42,8 +42,8 @@ class TestUtils buffer += static_cast(MapVirtualKeyW(kc.Vkey(), MAPVK_VK_TO_CHAR)); WEX::Logging::Log::Comment(WEX::Common::NoThrowString().Format(L"Looking for key:%s", buffer.c_str())); - const auto action = keymap.TryLookup(kc); - VERIFY_IS_NOT_NULL(action, L"Expected to find an action bound to the given KeyChord"); - return action; + const auto cmd = actionMap.GetActionByKeyChord(kc); + VERIFY_IS_NOT_NULL(cmd, L"Expected to find an action bound to the given KeyChord"); + return cmd.ActionAndArgs(); }; }; diff --git a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp index 2d6273ad894..a7cfa529765 100644 --- a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp @@ -120,13 +120,13 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(3u, settings.ActiveProfiles().Size()); - auto commands = settings.GlobalSettings().Commands(); - VERIFY_ARE_EQUAL(1u, commands.Size()); + auto nameMap{ settings.ActionMap().NameMap() }; + VERIFY_ARE_EQUAL(1u, nameMap.Size()); { - auto command = commands.Lookup(L"iterable command ${profile.name}"); + auto command = nameMap.TryLookup(L"iterable command ${profile.name}"); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -141,7 +141,7 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(L"${profile.name}", realArgs.TerminalArgs().Profile()); } - auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(commands, settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); + auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(nameMap, settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); _logCommandNames(expandedCommands.GetView()); VERIFY_ARE_EQUAL(0u, settings.Warnings().Size()); @@ -150,7 +150,7 @@ namespace TerminalAppLocalTests { auto command = expandedCommands.Lookup(L"iterable command profile0"); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -168,7 +168,7 @@ namespace TerminalAppLocalTests { auto command = expandedCommands.Lookup(L"iterable command profile1"); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -186,7 +186,7 @@ namespace TerminalAppLocalTests { auto command = expandedCommands.Lookup(L"iterable command profile2"); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -247,13 +247,13 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(3u, settings.ActiveProfiles().Size()); - auto commands = settings.GlobalSettings().Commands(); - VERIFY_ARE_EQUAL(1u, commands.Size()); + auto nameMap{ settings.ActionMap().NameMap() }; + VERIFY_ARE_EQUAL(1u, nameMap.Size()); { - auto command = commands.Lookup(L"Split pane, profile: ${profile.name}"); + auto command = nameMap.TryLookup(L"Split pane, profile: ${profile.name}"); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -268,7 +268,7 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(L"${profile.name}", realArgs.TerminalArgs().Profile()); } - auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(commands, settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); + auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(nameMap, settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); _logCommandNames(expandedCommands.GetView()); VERIFY_ARE_EQUAL(0u, settings.Warnings().Size()); @@ -277,7 +277,7 @@ namespace TerminalAppLocalTests { auto command = expandedCommands.Lookup(L"Split pane, profile: profile0"); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -295,7 +295,7 @@ namespace TerminalAppLocalTests { auto command = expandedCommands.Lookup(L"Split pane, profile: profile1"); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -313,7 +313,7 @@ namespace TerminalAppLocalTests { auto command = expandedCommands.Lookup(L"Split pane, profile: profile2"); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -376,13 +376,13 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(3u, settings.ActiveProfiles().Size()); - auto commands = settings.GlobalSettings().Commands(); - VERIFY_ARE_EQUAL(1u, commands.Size()); + auto nameMap{ settings.ActionMap().NameMap() }; + VERIFY_ARE_EQUAL(1u, nameMap.Size()); { - auto command = commands.Lookup(L"iterable command ${profile.name}"); + auto command = nameMap.TryLookup(L"iterable command ${profile.name}"); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -397,7 +397,7 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(L"${profile.name}", realArgs.TerminalArgs().Profile()); } - auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(commands, settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); + auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(nameMap, settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); _logCommandNames(expandedCommands.GetView()); VERIFY_ARE_EQUAL(0u, settings.Warnings().Size()); @@ -406,7 +406,7 @@ namespace TerminalAppLocalTests { auto command = expandedCommands.Lookup(L"iterable command profile0"); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -424,7 +424,7 @@ namespace TerminalAppLocalTests { auto command = expandedCommands.Lookup(L"iterable command profile1\""); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -442,7 +442,7 @@ namespace TerminalAppLocalTests { auto command = expandedCommands.Lookup(L"iterable command profile2"); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -513,8 +513,7 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(0u, settings.Warnings().Size()); VERIFY_ARE_EQUAL(3u, settings.ActiveProfiles().Size()); - auto commands = settings.GlobalSettings().Commands(); - auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(commands, settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); + auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(settings.ActionMap().NameMap(), settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); _logCommandNames(expandedCommands.GetView()); VERIFY_ARE_EQUAL(0u, settings.Warnings().Size()); @@ -522,8 +521,9 @@ namespace TerminalAppLocalTests auto rootCommand = expandedCommands.Lookup(L"Connect to ssh..."); VERIFY_IS_NOT_NULL(rootCommand); - auto rootActionAndArgs = rootCommand.Action(); - VERIFY_IS_NULL(rootActionAndArgs); + auto rootActionAndArgs = rootCommand.ActionAndArgs(); + VERIFY_IS_NOT_NULL(rootActionAndArgs); + VERIFY_ARE_EQUAL(ShortcutAction::Invalid, rootActionAndArgs.Action()); VERIFY_ARE_EQUAL(2u, rootCommand.NestedCommands().Size()); @@ -531,7 +531,7 @@ namespace TerminalAppLocalTests winrt::hstring commandName{ L"first.com" }; auto command = rootCommand.NestedCommands().Lookup(commandName); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_IS_FALSE(command.HasNestedCommands()); @@ -540,7 +540,7 @@ namespace TerminalAppLocalTests winrt::hstring commandName{ L"second.com" }; auto command = rootCommand.NestedCommands().Lookup(commandName); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_IS_FALSE(command.HasNestedCommands()); @@ -608,8 +608,7 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(0u, settings.Warnings().Size()); VERIFY_ARE_EQUAL(3u, settings.ActiveProfiles().Size()); - auto commands = settings.GlobalSettings().Commands(); - auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(commands, settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); + auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(settings.ActionMap().NameMap(), settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); _logCommandNames(expandedCommands.GetView()); VERIFY_ARE_EQUAL(0u, settings.Warnings().Size()); @@ -617,23 +616,25 @@ namespace TerminalAppLocalTests auto grandparentCommand = expandedCommands.Lookup(L"grandparent"); VERIFY_IS_NOT_NULL(grandparentCommand); - auto grandparentActionAndArgs = grandparentCommand.Action(); - VERIFY_IS_NULL(grandparentActionAndArgs); + auto grandparentActionAndArgs = grandparentCommand.ActionAndArgs(); + VERIFY_IS_NOT_NULL(grandparentActionAndArgs); + VERIFY_ARE_EQUAL(ShortcutAction::Invalid, grandparentActionAndArgs.Action()); VERIFY_ARE_EQUAL(1u, grandparentCommand.NestedCommands().Size()); winrt::hstring parentName{ L"parent" }; auto parent = grandparentCommand.NestedCommands().Lookup(parentName); VERIFY_IS_NOT_NULL(parent); - auto parentActionAndArgs = parent.Action(); - VERIFY_IS_NULL(parentActionAndArgs); + auto parentActionAndArgs = parent.ActionAndArgs(); + VERIFY_IS_NOT_NULL(parentActionAndArgs); + VERIFY_ARE_EQUAL(ShortcutAction::Invalid, parentActionAndArgs.Action()); VERIFY_ARE_EQUAL(2u, parent.NestedCommands().Size()); { winrt::hstring childName{ L"child1" }; auto child = parent.NestedCommands().Lookup(childName); VERIFY_IS_NOT_NULL(child); - auto childActionAndArgs = child.Action(); + auto childActionAndArgs = child.ActionAndArgs(); VERIFY_IS_NOT_NULL(childActionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::NewTab, childActionAndArgs.Action()); @@ -653,7 +654,7 @@ namespace TerminalAppLocalTests winrt::hstring childName{ L"child2" }; auto child = parent.NestedCommands().Lookup(childName); VERIFY_IS_NOT_NULL(child); - auto childActionAndArgs = child.Action(); + auto childActionAndArgs = child.ActionAndArgs(); VERIFY_IS_NOT_NULL(childActionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::NewTab, childActionAndArgs.Action()); @@ -731,8 +732,7 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(0u, settings.Warnings().Size()); VERIFY_ARE_EQUAL(3u, settings.ActiveProfiles().Size()); - auto commands = settings.GlobalSettings().Commands(); - auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(commands, settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); + auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(settings.ActionMap().NameMap(), settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); _logCommandNames(expandedCommands.GetView()); VERIFY_ARE_EQUAL(0u, settings.Warnings().Size()); @@ -744,8 +744,9 @@ namespace TerminalAppLocalTests winrt::hstring commandName{ name + L"..." }; auto command = expandedCommands.Lookup(commandName); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); - VERIFY_IS_NULL(actionAndArgs); + auto actionAndArgs = command.ActionAndArgs(); + VERIFY_IS_NOT_NULL(actionAndArgs); + VERIFY_ARE_EQUAL(ShortcutAction::Invalid, actionAndArgs.Action()); VERIFY_IS_TRUE(command.HasNestedCommands()); VERIFY_ARE_EQUAL(3u, command.NestedCommands().Size()); @@ -754,7 +755,7 @@ namespace TerminalAppLocalTests winrt::hstring childCommandName{ fmt::format(L"Split pane, profile: {}", name) }; auto childCommand = command.NestedCommands().Lookup(childCommandName); VERIFY_IS_NOT_NULL(childCommand); - auto childActionAndArgs = childCommand.Action(); + auto childActionAndArgs = childCommand.ActionAndArgs(); VERIFY_IS_NOT_NULL(childActionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, childActionAndArgs.Action()); @@ -775,7 +776,7 @@ namespace TerminalAppLocalTests winrt::hstring childCommandName{ fmt::format(L"Split pane, split: horizontal, profile: {}", name) }; auto childCommand = command.NestedCommands().Lookup(childCommandName); VERIFY_IS_NOT_NULL(childCommand); - auto childActionAndArgs = childCommand.Action(); + auto childActionAndArgs = childCommand.ActionAndArgs(); VERIFY_IS_NOT_NULL(childActionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, childActionAndArgs.Action()); @@ -796,7 +797,7 @@ namespace TerminalAppLocalTests winrt::hstring childCommandName{ fmt::format(L"Split pane, split: vertical, profile: {}", name) }; auto childCommand = command.NestedCommands().Lookup(childCommandName); VERIFY_IS_NOT_NULL(childCommand); - auto childActionAndArgs = childCommand.Action(); + auto childActionAndArgs = childCommand.ActionAndArgs(); VERIFY_IS_NOT_NULL(childActionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, childActionAndArgs.Action()); @@ -868,8 +869,7 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(0u, settings.Warnings().Size()); VERIFY_ARE_EQUAL(3u, settings.ActiveProfiles().Size()); - auto commands = settings.GlobalSettings().Commands(); - auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(commands, settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); + auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(settings.ActionMap().NameMap(), settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); _logCommandNames(expandedCommands.GetView()); VERIFY_ARE_EQUAL(0u, settings.Warnings().Size()); @@ -877,8 +877,9 @@ namespace TerminalAppLocalTests auto rootCommand = expandedCommands.Lookup(L"New Tab With Profile..."); VERIFY_IS_NOT_NULL(rootCommand); - auto rootActionAndArgs = rootCommand.Action(); - VERIFY_IS_NULL(rootActionAndArgs); + auto rootActionAndArgs = rootCommand.ActionAndArgs(); + VERIFY_IS_NOT_NULL(rootActionAndArgs); + VERIFY_ARE_EQUAL(ShortcutAction::Invalid, rootActionAndArgs.Action()); VERIFY_ARE_EQUAL(3u, rootCommand.NestedCommands().Size()); @@ -887,7 +888,7 @@ namespace TerminalAppLocalTests winrt::hstring commandName{ fmt::format(L"New tab, profile: {}", name) }; auto command = rootCommand.NestedCommands().Lookup(commandName); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action()); @@ -971,8 +972,7 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(0u, settings.Warnings().Size()); VERIFY_ARE_EQUAL(3u, settings.ActiveProfiles().Size()); - auto commands = settings.GlobalSettings().Commands(); - auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(commands, settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); + auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(settings.ActionMap().NameMap(), settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); _logCommandNames(expandedCommands.GetView()); VERIFY_ARE_EQUAL(0u, settings.Warnings().Size()); @@ -980,8 +980,9 @@ namespace TerminalAppLocalTests auto rootCommand = expandedCommands.Lookup(L"New Pane..."); VERIFY_IS_NOT_NULL(rootCommand); - auto rootActionAndArgs = rootCommand.Action(); - VERIFY_IS_NULL(rootActionAndArgs); + auto rootActionAndArgs = rootCommand.ActionAndArgs(); + VERIFY_IS_NOT_NULL(rootActionAndArgs); + VERIFY_ARE_EQUAL(ShortcutAction::Invalid, rootActionAndArgs.Action()); VERIFY_ARE_EQUAL(3u, rootCommand.NestedCommands().Size()); @@ -990,8 +991,9 @@ namespace TerminalAppLocalTests winrt::hstring commandName{ name + L"..." }; auto command = rootCommand.NestedCommands().Lookup(commandName); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); - VERIFY_IS_NULL(actionAndArgs); + auto actionAndArgs = command.ActionAndArgs(); + VERIFY_IS_NOT_NULL(actionAndArgs); + VERIFY_ARE_EQUAL(ShortcutAction::Invalid, actionAndArgs.Action()); VERIFY_IS_TRUE(command.HasNestedCommands()); VERIFY_ARE_EQUAL(3u, command.NestedCommands().Size()); @@ -1001,7 +1003,7 @@ namespace TerminalAppLocalTests winrt::hstring childCommandName{ fmt::format(L"Split pane, profile: {}", name) }; auto childCommand = command.NestedCommands().Lookup(childCommandName); VERIFY_IS_NOT_NULL(childCommand); - auto childActionAndArgs = childCommand.Action(); + auto childActionAndArgs = childCommand.ActionAndArgs(); VERIFY_IS_NOT_NULL(childActionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, childActionAndArgs.Action()); @@ -1022,7 +1024,7 @@ namespace TerminalAppLocalTests winrt::hstring childCommandName{ fmt::format(L"Split pane, split: horizontal, profile: {}", name) }; auto childCommand = command.NestedCommands().Lookup(childCommandName); VERIFY_IS_NOT_NULL(childCommand); - auto childActionAndArgs = childCommand.Action(); + auto childActionAndArgs = childCommand.ActionAndArgs(); VERIFY_IS_NOT_NULL(childActionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, childActionAndArgs.Action()); @@ -1043,7 +1045,7 @@ namespace TerminalAppLocalTests winrt::hstring childCommandName{ fmt::format(L"Split pane, split: vertical, profile: {}", name) }; auto childCommand = command.NestedCommands().Lookup(childCommandName); VERIFY_IS_NOT_NULL(childCommand); - auto childActionAndArgs = childCommand.Action(); + auto childActionAndArgs = childCommand.ActionAndArgs(); VERIFY_IS_NOT_NULL(childActionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, childActionAndArgs.Action()); @@ -1113,13 +1115,13 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(3u, settings.ActiveProfiles().Size()); - auto commands = settings.GlobalSettings().Commands(); - VERIFY_ARE_EQUAL(1u, commands.Size()); + auto nameMap{ settings.ActionMap().NameMap() }; + VERIFY_ARE_EQUAL(1u, nameMap.Size()); { - auto command = commands.Lookup(L"iterable command ${scheme.name}"); + auto command = nameMap.TryLookup(L"iterable command ${scheme.name}"); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -1134,7 +1136,7 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(L"${scheme.name}", realArgs.TerminalArgs().Profile()); } - auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(commands, settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); + auto expandedCommands = winrt::TerminalApp::implementation::TerminalPage::_ExpandCommands(nameMap, settings.ActiveProfiles().GetView(), settings.GlobalSettings().ColorSchemes()); _logCommandNames(expandedCommands.GetView()); // This is the same warning as above @@ -1148,7 +1150,7 @@ namespace TerminalAppLocalTests { auto command = expandedCommands.Lookup(L"iterable command scheme_0"); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -1166,7 +1168,7 @@ namespace TerminalAppLocalTests { auto command = expandedCommands.Lookup(L"iterable command scheme_1"); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); @@ -1184,7 +1186,7 @@ namespace TerminalAppLocalTests { auto command = expandedCommands.Lookup(L"iterable command scheme_2"); VERIFY_IS_NOT_NULL(command); - auto actionAndArgs = command.Action(); + auto actionAndArgs = command.ActionAndArgs(); VERIFY_IS_NOT_NULL(actionAndArgs); VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); const auto& realArgs = actionAndArgs.Args().try_as(); diff --git a/src/cascadia/TerminalApp/ActionPreviewHandlers.cpp b/src/cascadia/TerminalApp/ActionPreviewHandlers.cpp index 8062d107608..f09c9755322 100644 --- a/src/cascadia/TerminalApp/ActionPreviewHandlers.cpp +++ b/src/cascadia/TerminalApp/ActionPreviewHandlers.cpp @@ -42,11 +42,11 @@ namespace winrt::TerminalApp::implementation // - void TerminalPage::_EndPreview() { - if (_lastPreviewedCommand == nullptr || _lastPreviewedCommand.Action() == nullptr) + if (_lastPreviewedCommand == nullptr || _lastPreviewedCommand.ActionAndArgs() == nullptr) { return; } - switch (_lastPreviewedCommand.Action().Action()) + switch (_lastPreviewedCommand.ActionAndArgs().Action()) { case ShortcutAction::SetColorScheme: { @@ -160,17 +160,17 @@ namespace winrt::TerminalApp::implementation void TerminalPage::_PreviewActionHandler(const IInspectable& /*sender*/, const Microsoft::Terminal::Settings::Model::Command& args) { - if (args == nullptr || args.Action() == nullptr) + if (args == nullptr || args.ActionAndArgs() == nullptr) { _EndPreview(); } else { - switch (args.Action().Action()) + switch (args.ActionAndArgs().Action()) { case ShortcutAction::SetColorScheme: { - _PreviewColorScheme(args.Action().Args().try_as()); + _PreviewColorScheme(args.ActionAndArgs().Args().try_as()); break; } } diff --git a/src/cascadia/TerminalApp/AppKeyBindings.cpp b/src/cascadia/TerminalApp/AppKeyBindings.cpp index 4559e7624cf..71a3a03bbb4 100644 --- a/src/cascadia/TerminalApp/AppKeyBindings.cpp +++ b/src/cascadia/TerminalApp/AppKeyBindings.cpp @@ -14,10 +14,9 @@ namespace winrt::TerminalApp::implementation { bool AppKeyBindings::TryKeyChord(const KeyChord& kc) { - const auto actionAndArgs = _keymap.TryLookup(kc); - if (actionAndArgs) + if (const auto cmd{ _actionMap.GetActionByKeyChord(kc) }) { - return _dispatch.DoAction(actionAndArgs); + return _dispatch.DoAction(cmd.ActionAndArgs()); } return false; } @@ -27,8 +26,8 @@ namespace winrt::TerminalApp::implementation _dispatch = dispatch; } - void AppKeyBindings::SetKeyMapping(const winrt::Microsoft::Terminal::Settings::Model::KeyMapping& keymap) + void AppKeyBindings::SetActionMap(const winrt::Microsoft::Terminal::Settings::Model::IActionMapView& actionMap) { - _keymap = keymap; + _actionMap = actionMap; } } diff --git a/src/cascadia/TerminalApp/AppKeyBindings.h b/src/cascadia/TerminalApp/AppKeyBindings.h index 301367eb58b..5004edeca4d 100644 --- a/src/cascadia/TerminalApp/AppKeyBindings.h +++ b/src/cascadia/TerminalApp/AppKeyBindings.h @@ -22,10 +22,10 @@ namespace winrt::TerminalApp::implementation bool TryKeyChord(winrt::Microsoft::Terminal::Control::KeyChord const& kc); void SetDispatch(const winrt::TerminalApp::ShortcutActionDispatch& dispatch); - void SetKeyMapping(const Microsoft::Terminal::Settings::Model::KeyMapping& keymap); + void SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap); private: - winrt::Microsoft::Terminal::Settings::Model::KeyMapping _keymap{ nullptr }; + winrt::Microsoft::Terminal::Settings::Model::IActionMapView _actionMap{ nullptr }; winrt::TerminalApp::ShortcutActionDispatch _dispatch{ nullptr }; diff --git a/src/cascadia/TerminalApp/AppKeyBindings.idl b/src/cascadia/TerminalApp/AppKeyBindings.idl index 1bd93de0d9d..24b2dc1b7b4 100644 --- a/src/cascadia/TerminalApp/AppKeyBindings.idl +++ b/src/cascadia/TerminalApp/AppKeyBindings.idl @@ -9,6 +9,6 @@ namespace TerminalApp AppKeyBindings(); void SetDispatch(ShortcutActionDispatch dispatch); - void SetKeyMapping(Microsoft.Terminal.Settings.Model.KeyMapping keymap); + void SetActionMap(Microsoft.Terminal.Settings.Model.IActionMapView actionMap); } } diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index e8229157b7d..4c9e7612bb2 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -1411,9 +1411,9 @@ namespace winrt::TerminalApp::implementation return _root ? _root->AlwaysOnTop() : false; } - Windows::Foundation::Collections::IMap AppLogic::GlobalHotkeys() + Windows::Foundation::Collections::IMapView AppLogic::GlobalHotkeys() { - return _settings.GlobalSettings().KeyMap().GlobalHotkeys(); + return _settings.GlobalSettings().ActionMap().GlobalHotkeys(); } void AppLogic::IdentifyWindow() diff --git a/src/cascadia/TerminalApp/AppLogic.h b/src/cascadia/TerminalApp/AppLogic.h index 951185f473d..c86b543005b 100644 --- a/src/cascadia/TerminalApp/AppLogic.h +++ b/src/cascadia/TerminalApp/AppLogic.h @@ -90,7 +90,7 @@ namespace winrt::TerminalApp::implementation winrt::Windows::Foundation::IAsyncOperation ShowDialog(winrt::Windows::UI::Xaml::Controls::ContentDialog dialog); - Windows::Foundation::Collections::IMap GlobalHotkeys(); + Windows::Foundation::Collections::IMapView GlobalHotkeys(); // -------------------------------- WinRT Events --------------------------------- TYPED_EVENT(RequestedThemeChanged, winrt::Windows::Foundation::IInspectable, winrt::Windows::UI::Xaml::ElementTheme); diff --git a/src/cascadia/TerminalApp/AppLogic.idl b/src/cascadia/TerminalApp/AppLogic.idl index bbad80254d8..fec67c970c3 100644 --- a/src/cascadia/TerminalApp/AppLogic.idl +++ b/src/cascadia/TerminalApp/AppLogic.idl @@ -71,7 +71,7 @@ namespace TerminalApp FindTargetWindowResult FindTargetWindow(String[] args); - Windows.Foundation.Collections.IMap GlobalHotkeys(); + Windows.Foundation.Collections.IMapView GlobalHotkeys(); // See IDialogPresenter and TerminalPage's DialogPresenter for more // information. diff --git a/src/cascadia/TerminalApp/CommandPalette.cpp b/src/cascadia/TerminalApp/CommandPalette.cpp index 1c71a69d282..322cbc6262c 100644 --- a/src/cascadia/TerminalApp/CommandPalette.cpp +++ b/src/cascadia/TerminalApp/CommandPalette.cpp @@ -261,19 +261,18 @@ 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 && _keymap) + if (_currentMode == CommandPaletteMode::TabSwitchMode && _actionMap) { winrt::Microsoft::Terminal::Control::KeyChord kc{ ctrlDown, altDown, shiftDown, static_cast(key) }; - const auto action = _keymap.TryLookup(kc); - if (action) + if (const auto cmd{ _actionMap.GetActionByKeyChord(kc) }) { - if (action.Action() == ShortcutAction::PrevTab) + if (cmd.ActionAndArgs().Action() == ShortcutAction::PrevTab) { SelectNextItem(false); e.Handled(true); return; } - else if (action.Action() == ShortcutAction::NextTab) + else if (cmd.ActionAndArgs().Action() == ShortcutAction::NextTab) { SelectNextItem(true); e.Handled(true); @@ -864,9 +863,9 @@ namespace winrt::TerminalApp::implementation return _filteredActions; } - void CommandPalette::SetKeyMap(const Microsoft::Terminal::Settings::Model::KeyMapping& keymap) + void CommandPalette::SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap) { - _keymap = keymap; + _actionMap = actionMap; } void CommandPalette::SetCommands(Collections::IVector const& actions) diff --git a/src/cascadia/TerminalApp/CommandPalette.h b/src/cascadia/TerminalApp/CommandPalette.h index fab6ec91492..af065d67d6c 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::IObservableVector const& tabs, Windows::Foundation::Collections::IObservableVector const& mruTabs); - void SetKeyMap(const Microsoft::Terminal::Settings::Model::KeyMapping& keymap); + void SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap); bool OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, const bool down); @@ -107,7 +107,7 @@ namespace winrt::TerminalApp::implementation std::wstring _getTrimmedInput(); void _evaluatePrefix(); - Microsoft::Terminal::Settings::Model::KeyMapping _keymap{ nullptr }; + Microsoft::Terminal::Settings::Model::IActionMapView _actionMap{ nullptr }; // Tab Switcher Windows::Foundation::Collections::IVector _tabActions{ nullptr }; diff --git a/src/cascadia/TerminalApp/CommandPalette.idl b/src/cascadia/TerminalApp/CommandPalette.idl index c38da8acbf8..4bb72f46dda 100644 --- a/src/cascadia/TerminalApp/CommandPalette.idl +++ b/src/cascadia/TerminalApp/CommandPalette.idl @@ -25,7 +25,7 @@ namespace TerminalApp void SetTabs(Windows.Foundation.Collections.IObservableVector tabs, Windows.Foundation.Collections.IObservableVector mruTabs); - void SetKeyMap(Microsoft.Terminal.Settings.Model.KeyMapping keymap); + void SetActionMap(Microsoft.Terminal.Settings.Model.IActionMapView actionMap); void SelectNextItem(Boolean moveDown); diff --git a/src/cascadia/TerminalApp/TabBase.cpp b/src/cascadia/TerminalApp/TabBase.cpp index ff51c2dfb0b..a7a973801d2 100644 --- a/src/cascadia/TerminalApp/TabBase.cpp +++ b/src/cascadia/TerminalApp/TabBase.cpp @@ -148,9 +148,9 @@ namespace winrt::TerminalApp::implementation _dispatch = dispatch; } - void TabBase::SetKeyMap(const Microsoft::Terminal::Settings::Model::KeyMapping& keymap) + void TabBase::SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap) { - _keymap = keymap; + _actionMap = actionMap; _UpdateSwitchToTabKeyChord(); } @@ -163,9 +163,7 @@ namespace winrt::TerminalApp::implementation // - winrt::fire_and_forget TabBase::_UpdateSwitchToTabKeyChord() { - SwitchToTabArgs args{ _TabViewIndex }; - ActionAndArgs switchToTab{ ShortcutAction::SwitchToTab, args }; - const auto keyChord = _keymap ? _keymap.GetKeyBindingForActionWithArgs(switchToTab) : nullptr; + const auto keyChord = _actionMap ? _actionMap.GetKeyBindingForAction(ShortcutAction::SwitchToTab, SwitchToTabArgs{ _TabViewIndex }) : nullptr; const auto keyChordText = keyChord ? KeyChordSerialization::ToString(keyChord) : L""; if (_keyChord == keyChordText) diff --git a/src/cascadia/TerminalApp/TabBase.h b/src/cascadia/TerminalApp/TabBase.h index 6ae3093d346..51295133334 100644 --- a/src/cascadia/TerminalApp/TabBase.h +++ b/src/cascadia/TerminalApp/TabBase.h @@ -23,7 +23,7 @@ namespace winrt::TerminalApp::implementation void SetDispatch(const winrt::TerminalApp::ShortcutActionDispatch& dispatch); void UpdateTabViewIndex(const uint32_t idx, const uint32_t numTabs); - void SetKeyMap(const Microsoft::Terminal::Settings::Model::KeyMapping& keymap); + void SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap); WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler); WINRT_CALLBACK(CloseRequested, winrt::Windows::Foundation::EventHandler); @@ -46,7 +46,7 @@ namespace winrt::TerminalApp::implementation winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _closeOtherTabsMenuItem{}; winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _closeTabsAfterMenuItem{}; winrt::TerminalApp::ShortcutActionDispatch _dispatch; - Microsoft::Terminal::Settings::Model::KeyMapping _keymap{ nullptr }; + Microsoft::Terminal::Settings::Model::IActionMapView _actionMap{ nullptr }; winrt::hstring _keyChord{}; virtual void _CreateContextMenu(); diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp index adf6d7984b6..e774935ab72 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/src/cascadia/TerminalApp/TabManagement.cpp @@ -130,7 +130,7 @@ namespace winrt::TerminalApp::implementation _mruTabs.Append(*newTabImpl); newTabImpl->SetDispatch(*_actionDispatch); - newTabImpl->SetKeyMap(_settings.KeyMap()); + newTabImpl->SetActionMap(_settings.ActionMap()); // Give the tab its index in the _tabs vector so it can manage its own SwitchToTab command. _UpdateTabIndices(); diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 7d661f3b3ae..7f728e5ad94 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -76,21 +76,19 @@ namespace winrt::TerminalApp::implementation for (const auto& nameAndCmd : commands) { const auto& command = nameAndCmd.Value(); - // If there's a keybinding that's bound to exactly this command, - // then get the string for that keychord and display it as a - // part of the command in the UI. Each Command's KeyChordText is - // unset by default, so we don't need to worry about clearing it - // if there isn't a key associated with it. - auto keyChord{ settings.KeyMap().GetKeyBindingForActionWithArgs(command.Action()) }; - - if (keyChord) - { - command.KeyChordText(KeyChordSerialization::ToString(keyChord)); - } if (command.HasNestedCommands()) { _recursiveUpdateCommandKeybindingLabels(settings, command.NestedCommands()); } + else + { + // If there's a keybinding that's bound to exactly this command, + // then get the keychord and display it as a + // part of the command in the UI. + // We specifically need to do this for nested commands. + const auto keyChord{ settings.ActionMap().GetKeyBindingForAction(command.ActionAndArgs().Action(), command.ActionAndArgs().Args()) }; + command.RegisterKey(keyChord); + } } } @@ -108,7 +106,7 @@ namespace winrt::TerminalApp::implementation // happen before the Settings UI is reloaded and tries to re-read // those values. _UpdateCommandsForPalette(); - CommandPalette().SetKeyMap(_settings.KeyMap()); + CommandPalette().SetActionMap(_settings.ActionMap()); if (needRefreshUI) { @@ -124,7 +122,7 @@ namespace winrt::TerminalApp::implementation void TerminalPage::Create() { // Hookup the key bindings - _HookupKeyBindings(_settings.KeyMap()); + _HookupKeyBindings(_settings.ActionMap()); _tabContent = this->TabContent(); _tabRow = this->TabRow(); @@ -277,7 +275,7 @@ namespace winrt::TerminalApp::implementation // - void TerminalPage::_OnDispatchCommandRequested(const IInspectable& /*sender*/, const Microsoft::Terminal::Settings::Model::Command& command) { - const auto& actionAndArgs = command.Action(); + const auto& actionAndArgs = command.ActionAndArgs(); _actionDispatch->DoAction(actionAndArgs); } @@ -526,8 +524,7 @@ namespace winrt::TerminalApp::implementation auto newTabFlyout = WUX::Controls::MenuFlyout{}; newTabFlyout.Placement(WUX::Controls::Primitives::FlyoutPlacementMode::BottomEdgeAlignedLeft); - auto keyBindings = _settings.KeyMap(); - + auto actionMap = _settings.ActionMap(); const auto defaultProfileGuid = _settings.GlobalSettings().DefaultProfile(); // the number of profiles should not change in the loop for this to work auto const profileCount = gsl::narrow_cast(_settings.ActiveProfiles().Size()); @@ -541,8 +538,7 @@ namespace winrt::TerminalApp::implementation // NewTab(ProfileIndex=N) action NewTerminalArgs newTerminalArgs{ profileIndex }; NewTabArgs newTabArgs{ newTerminalArgs }; - ActionAndArgs actionAndArgs{ ShortcutAction::NewTab, newTabArgs }; - auto profileKeyChord{ keyBindings.GetKeyBindingForActionWithArgs(actionAndArgs) }; + auto profileKeyChord{ actionMap.GetKeyBindingForAction(ShortcutAction::NewTab, newTabArgs) }; // make sure we find one to display if (profileKeyChord) @@ -666,9 +662,7 @@ namespace winrt::TerminalApp::implementation settingsItem.Click({ this, &TerminalPage::_SettingsButtonOnClick }); newTabFlyout.Items().Append(settingsItem); - Microsoft::Terminal::Settings::Model::OpenSettingsArgs args{ SettingsTarget::SettingsUI }; - Microsoft::Terminal::Settings::Model::ActionAndArgs settingsAction{ ShortcutAction::OpenSettings, args }; - const auto settingsKeyChord{ keyBindings.GetKeyBindingForActionWithArgs(settingsAction) }; + const auto settingsKeyChord{ actionMap.GetKeyBindingForAction(ShortcutAction::OpenSettings, OpenSettingsArgs{ SettingsTarget::SettingsUI }) }; if (settingsKeyChord) { _SetAcceleratorForMenuItem(settingsItem, settingsKeyChord); @@ -892,14 +886,13 @@ namespace winrt::TerminalApp::implementation auto const shiftDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Shift), CoreVirtualKeyStates::Down); winrt::Microsoft::Terminal::Control::KeyChord kc{ ctrlDown, altDown, shiftDown, static_cast(key) }; - const auto actionAndArgs = _settings.KeyMap().TryLookup(kc); - if (actionAndArgs) + if (const auto cmd{ _settings.ActionMap().GetActionByKeyChord(kc) }) { - if (CommandPalette().Visibility() == Visibility::Visible && actionAndArgs.Action() != ShortcutAction::ToggleCommandPalette) + if (CommandPalette().Visibility() == Visibility::Visible && cmd.ActionAndArgs().Action() != ShortcutAction::ToggleCommandPalette) { CommandPalette().Visibility(Visibility::Collapsed); } - _actionDispatch->DoAction(actionAndArgs); + _actionDispatch->DoAction(cmd.ActionAndArgs()); e.Handled(true); } } @@ -920,23 +913,23 @@ namespace winrt::TerminalApp::implementation auto const shiftDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Shift), CoreVirtualKeyStates::Down); winrt::Microsoft::Terminal::Control::KeyChord kc{ ctrlDown, altDown, shiftDown, static_cast(key) }; - const auto actionAndArgs = _settings.KeyMap().TryLookup(kc); - if (actionAndArgs && (actionAndArgs.Action() == ShortcutAction::CloseTab || actionAndArgs.Action() == ShortcutAction::NextTab || actionAndArgs.Action() == ShortcutAction::PrevTab || actionAndArgs.Action() == ShortcutAction::ClosePane)) + const auto cmd{ _settings.ActionMap().GetActionByKeyChord(kc) }; + if (cmd && (cmd.ActionAndArgs().Action() == ShortcutAction::CloseTab || cmd.ActionAndArgs().Action() == ShortcutAction::NextTab || cmd.ActionAndArgs().Action() == ShortcutAction::PrevTab || cmd.ActionAndArgs().Action() == ShortcutAction::ClosePane)) { - _actionDispatch->DoAction(actionAndArgs); + _actionDispatch->DoAction(cmd.ActionAndArgs()); e.Handled(true); } } // Method Description: - // - Configure the AppKeyBindings to use our ShortcutActionDispatch and the updated KeyMapping - // as the object to handle dispatching ShortcutAction events. + // - Configure the AppKeyBindings to use our ShortcutActionDispatch and the updated ActionMap + // as the object to handle dispatching ShortcutAction events. // Arguments: - // - bindings: A AppKeyBindings object to wire up with our event handlers - void TerminalPage::_HookupKeyBindings(const KeyMapping& keymap) noexcept + // - bindings: An IActionMapView object to wire up with our event handlers + void TerminalPage::_HookupKeyBindings(const IActionMapView& actionMap) noexcept { _bindings->SetDispatch(*_actionDispatch); - _bindings->SetKeyMapping(keymap); + _bindings->SetActionMap(actionMap); } // Method Description: @@ -1390,7 +1383,7 @@ namespace winrt::TerminalApp::implementation menuShortcut.Key(static_cast(keyChord.Vkey())); // inspect the modifiers from the KeyChord and set the flags int he XAML value - auto modifiers = AppKeyBindings::ConvertVKModifiers(keyChord.Modifiers()); + auto modifiers = ActionMap::ConvertVKModifiers(keyChord.Modifiers()); // add the modifiers to the shortcut menuShortcut.Modifiers(modifiers); @@ -1823,7 +1816,7 @@ namespace winrt::TerminalApp::implementation { // Re-wire the keybindings to their handlers, as we'll have created a // new AppKeyBindings object. - _HookupKeyBindings(_settings.KeyMap()); + _HookupKeyBindings(_settings.ActionMap()); // Refresh UI elements auto profiles = _settings.ActiveProfiles(); @@ -1871,7 +1864,7 @@ namespace winrt::TerminalApp::implementation } auto tabImpl{ winrt::get_self(tab) }; - tabImpl->SetKeyMap(_settings.KeyMap()); + tabImpl->SetActionMap(_settings.ActionMap()); } auto weakThis{ get_weak() }; @@ -1952,7 +1945,7 @@ namespace winrt::TerminalApp::implementation // - void TerminalPage::_UpdateCommandsForPalette() { - IMap copyOfCommands = _ExpandCommands(_settings.GlobalSettings().Commands(), + IMap copyOfCommands = _ExpandCommands(_settings.GlobalSettings().ActionMap().NameMap(), _settings.ActiveProfiles().GetView(), _settings.GlobalSettings().ColorSchemes()); @@ -2357,7 +2350,7 @@ namespace winrt::TerminalApp::implementation _mruTabs.Append(*newTabImpl); newTabImpl->SetDispatch(*_actionDispatch); - newTabImpl->SetKeyMap(_settings.KeyMap()); + newTabImpl->SetActionMap(_settings.ActionMap()); // Give the tab its index in the _tabs vector so it can manage its own SwitchToTab command. _UpdateTabIndices(); diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 8ff980e24ea..794fd0e1568 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -200,7 +200,7 @@ namespace winrt::TerminalApp::implementation void _KeyDownHandler(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e); void _SUIPreviewKeyDownHandler(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e); - void _HookupKeyBindings(const Microsoft::Terminal::Settings::Model::KeyMapping& keymap) noexcept; + void _HookupKeyBindings(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap) noexcept; void _RegisterActionCallbacks(); void _UpdateTitle(const TerminalTab& tab); diff --git a/src/cascadia/TerminalSettingsEditor/Actions.cpp b/src/cascadia/TerminalSettingsEditor/Actions.cpp index 4d082997745..bd9403cece6 100644 --- a/src/cascadia/TerminalSettingsEditor/Actions.cpp +++ b/src/cascadia/TerminalSettingsEditor/Actions.cpp @@ -19,24 +19,27 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { InitializeComponent(); - _filteredActions = winrt::single_threaded_observable_vector(); + _filteredActions = winrt::single_threaded_observable_vector(); } void Actions::OnNavigatedTo(const NavigationEventArgs& e) { _State = e.Parameter().as(); - for (const auto& [k, command] : _State.Settings().GlobalSettings().Commands()) + std::vector keyBindingList; + for (const auto& [_, command] : _State.Settings().GlobalSettings().ActionMap().NameMap()) { // Filter out nested commands, and commands that aren't bound to a // key. This page is currently just for displaying the actions that // _are_ bound to keys. - if (command.HasNestedCommands() || command.KeyChordText().empty()) + if (command.HasNestedCommands() || !command.Keys()) { continue; } - _filteredActions.Append(command); + keyBindingList.push_back(command); } + std::sort(begin(keyBindingList), end(keyBindingList), CommandComparator{}); + _filteredActions = single_threaded_observable_vector(std::move(keyBindingList)); } Collections::IObservableVector Actions::FilteredActions() diff --git a/src/cascadia/TerminalSettingsEditor/Actions.h b/src/cascadia/TerminalSettingsEditor/Actions.h index 536b7251999..e49c1aebc3f 100644 --- a/src/cascadia/TerminalSettingsEditor/Actions.h +++ b/src/cascadia/TerminalSettingsEditor/Actions.h @@ -9,6 +9,14 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { + struct CommandComparator + { + bool operator()(const Model::Command& lhs, const Model::Command& rhs) const + { + return lhs.Name() < rhs.Name(); + } + }; + struct ActionsPageNavigationState : ActionsPageNavigationStateT { public: diff --git a/src/cascadia/TerminalSettingsEditor/Actions.xaml b/src/cascadia/TerminalSettingsEditor/Actions.xaml index 9911c415503..3167229a987 100644 --- a/src/cascadia/TerminalSettingsEditor/Actions.xaml +++ b/src/cascadia/TerminalSettingsEditor/Actions.xaml @@ -62,9 +62,6 @@ Text="{x:Bind Name, Mode=OneWay}" /> ID) + actionMap->_KeyMap = _KeyMap; + + // copy _ActionMap (ID --> Command) + for (const auto& [actionID, cmd] : _ActionMap) + { + actionMap->_ActionMap.emplace(actionID, *(get_self(cmd)->Copy())); + } + + // copy _MaskingActions (ID --> Command) + for (const auto& [actionID, cmd] : _MaskingActions) + { + actionMap->_MaskingActions.emplace(actionID, *(get_self(cmd)->Copy())); + } + + // copy _NestedCommands (Name --> Command) + for (const auto& [name, cmd] : _NestedCommands) + { + actionMap->_NestedCommands.Insert(name, *(get_self(cmd)->Copy())); + } + + // repeat this for each of our parents + FAIL_FAST_IF(_parents.size() > 1); + for (const auto& parent : _parents) + { + actionMap->_parents.emplace_back(parent->Copy()); + } + + return actionMap; + } + + // Method Description: + // - Adds a command to the ActionMap + // Arguments: + // - cmd: the command we're adding + void ActionMap::AddAction(const Model::Command& cmd) + { + // _Never_ add null to the ActionMap + if (!cmd) + { + return; + } + + // invalidate caches + _NameMapCache = nullptr; + _GlobalHotkeysCache = nullptr; + + // Handle nested commands + const auto cmdImpl{ get_self(cmd) }; + if (cmdImpl->IsNestedCommand()) + { + // But check if it actually has a name to bind to first + const auto name{ cmd.Name() }; + if (!name.empty()) + { + _NestedCommands.Insert(name, cmd); + } + return; + } + + // General Case: + // Add the new command to the KeyMap. + // This map directs you to an entry in the ActionMap. + + // Removing Actions from the Command Palette: + // cmd.Name and cmd.Action have a one-to-one relationship. + // If cmd.Name is empty, we must retrieve the old name and remove it. + + // Removing Key Bindings: + // cmd.Keys and cmd.Action have a many-to-one relationship. + // If cmd.Keys is empty, we don't care. + // If action is "unbound"/"invalid", you're explicitly unbinding the provided cmd.keys. + // NOTE: If we're unbinding a command from a different layer, we must use maskingActions + // to keep track of what key mappings are still valid. + + // _TryUpdateActionMap may update oldCmd and maskingCmd + Model::Command oldCmd{ nullptr }; + Model::Command maskingCmd{ nullptr }; + _TryUpdateActionMap(cmd, oldCmd, maskingCmd); + + _TryUpdateName(cmd, oldCmd, maskingCmd); + _TryUpdateKeyChord(cmd, oldCmd, maskingCmd); + } + + // Method Description: + // - Try to add the new command to _ActionMap. + // - If the command was added previously in this layer, populate oldCmd. + // - If the command was added previously in another layer, populate maskingCmd. + // Arguments: + // - cmd: the action we're trying to register + // - oldCmd: the action found in _ActionMap, if one already exists + // - maskingAction: the action found in a parent layer, if one already exists + void ActionMap::_TryUpdateActionMap(const Model::Command& cmd, Model::Command& oldCmd, Model::Command& maskingCmd) + { + // Example: + // { "command": "copy", "keys": "ctrl+c" } --> add the action in for the first time + // { "command": "copy", "keys": "ctrl+shift+c" } --> update oldCmd + const auto actionID{ Hash(cmd.ActionAndArgs()) }; + const auto& actionPair{ _ActionMap.find(actionID) }; + if (actionPair == _ActionMap.end()) + { + // add this action in for the first time + _ActionMap.emplace(actionID, cmd); + } + else + { + // We're adding an action that already exists in our layer. + // Record it so that we update it with any new information. + oldCmd = actionPair->second; + } + + // Masking Actions + // + // Example: + // parent: { "command": "copy", "keys": "ctrl+c" } --> add the action to parent._ActionMap + // current: { "command": "copy", "keys": "ctrl+shift+c" } --> look through parents for the "ctrl+c" binding, add it to _MaskingActions + // { "command": "copy", "keys": "ctrl+ins" } --> this should already be in _MaskingActions + + // Now check if this action was introduced in another layer. + const auto& maskingActionPair{ _MaskingActions.find(actionID) }; + if (maskingActionPair == _MaskingActions.end()) + { + // Check if we need to add this to our list of masking commands. + FAIL_FAST_IF(_parents.size() > 1); + for (const auto& parent : _parents) + { + // NOTE: This only checks the layer above us, but that's ok. + // If we had to find one from a layer above that, parent->_MaskingActions + // would have found it, so we inherit it for free! + const auto& inheritedCmd{ parent->_GetActionByID(actionID) }; + if (inheritedCmd.has_value() && inheritedCmd.value()) + { + const auto& inheritedCmdImpl{ get_self(inheritedCmd.value()) }; + maskingCmd = *inheritedCmdImpl->Copy(); + _MaskingActions.emplace(actionID, maskingCmd); + } + } + } + else + { + // This is an action that we already have a mutable "masking" record for. + // Record it so that we update it with any new information. + maskingCmd = maskingActionPair->second; + } + } + + // Method Description: + // - Update our internal state with the name of the newly registered action + // Arguments: + // - cmd: the action we're trying to register + // - oldCmd: the action that already exists in our internal state. May be null. + // - maskingCmd: the masking action that already exists in our internal state. May be null. + void ActionMap::_TryUpdateName(const Model::Command& cmd, const Model::Command& oldCmd, const Model::Command& maskingCmd) + { + // Example: + // { "name": "foo", "command": "copy" } --> we are setting a name, update oldCmd and maskingCmd + // { "command": "copy" } --> no change to name, exit early + const auto cmdImpl{ get_self(cmd) }; + if (!cmdImpl->HasName()) + { + // the user is not trying to update the name. + return; + } + + // Update oldCmd: + // If we have a Command in our _ActionMap that we're trying to update, + // update it. + const auto newName{ cmd.Name() }; + if (oldCmd) + { + // This command has a name, check if it's new. + if (newName != oldCmd.Name()) + { + // The new name differs from the old name, + // update our name. + auto oldCmdImpl{ get_self(oldCmd) }; + oldCmdImpl->Name(newName); + } + } + + // Update maskingCmd: + // We have a Command that is masking one from a parent layer. + // We need to ensure that this has the correct name. That way, + // we can return an accumulated view of a Command at this layer. + // This differs from oldCmd which is mainly used for serialization + // by recording the delta of the Command in this layer. + if (maskingCmd) + { + // This command has a name, check if it's new. + if (newName != maskingCmd.Name()) + { + // The new name differs from the old name, + // update our name. + auto maskingCmdImpl{ get_self(maskingCmd) }; + maskingCmdImpl->Name(newName); + } + } + + // Handle a collision with NestedCommands + _NestedCommands.TryRemove(newName); + } + + // Method Description: + // - Update our internal state with the key chord of the newly registered action + // Arguments: + // - cmd: the action we're trying to register + // - oldCmd: the action that already exists in our internal state. May be null. + // - maskingCmd: the masking action that already exists in our internal state. May be null. + void ActionMap::_TryUpdateKeyChord(const Model::Command& cmd, const Model::Command& oldCmd, const Model::Command& maskingCmd) + { + // Example: + // { "command": "copy", "keys": "ctrl+c" } --> we are registering a new key chord, update oldCmd and maskingCmd + // { "name": "foo", "command": "copy" } --> no change to keys, exit early + const auto keys{ cmd.Keys() }; + if (!keys) + { + // the user is not trying to update the keys. + return; + } + + // Handle collisions + const auto oldKeyPair{ _KeyMap.find(keys) }; + if (oldKeyPair != _KeyMap.end()) + { + // Collision: The key chord was already in use. + // + // Example: + // { "command": "copy", "keys": "ctrl+c" } --> register "ctrl+c" (different branch) + // { "command": "paste", "keys": "ctrl+c" } --> Collision! (this branch) + // + // Remove the old one. (unbind "copy" in the example above) + const auto actionPair{ _ActionMap.find(oldKeyPair->second) }; + const auto conflictingCmd{ actionPair->second }; + const auto conflictingCmdImpl{ get_self(conflictingCmd) }; + conflictingCmdImpl->EraseKey(keys); + } + else if (const auto& conflictingCmd{ GetActionByKeyChord(keys) }) + { + // Collision with ancestor: The key chord was already in use, but by an action in another layer + // + // Example: + // parent: { "command": "copy", "keys": "ctrl+c" } --> register "ctrl+c" (different branch) + // current: { "command": "paste", "keys": "ctrl+c" } --> Collision with ancestor! (this branch, sub-branch 1) + // { "command": "unbound", "keys": "ctrl+c" } --> Collision with masking action! (this branch, sub-branch 2) + const auto conflictingActionID{ Hash(conflictingCmd.ActionAndArgs()) }; + const auto maskingCmdPair{ _MaskingActions.find(conflictingActionID) }; + if (maskingCmdPair == _MaskingActions.end()) + { + // This is the first time we're colliding with an action from a different layer, + // so let's add this action to _MaskingActions and update it appropriately. + // Create a copy of the conflicting action, + // and erase the conflicting key chord from the copy. + const auto conflictingCmdImpl{ get_self(conflictingCmd) }; + const auto conflictingCmdCopy{ conflictingCmdImpl->Copy() }; + conflictingCmdCopy->EraseKey(keys); + _MaskingActions.emplace(conflictingActionID, *conflictingCmdCopy); + } + else + { + // We've collided with this action before. Let's resolve a collision with a masking action. + const auto maskingCmdImpl{ get_self(maskingCmdPair->second) }; + maskingCmdImpl->EraseKey(keys); + } + } + + // Assign the new action in the _KeyMap. + const auto actionID{ Hash(cmd.ActionAndArgs()) }; + _KeyMap.insert_or_assign(keys, actionID); + + // Additive operation: + // Register the new key chord with oldCmd (an existing _ActionMap entry) + // Example: + // { "command": "copy", "keys": "ctrl+c" } --> register "ctrl+c" (section above) + // { "command": "copy", "keys": "ctrl+shift+c" } --> also register "ctrl+shift+c" to the same Command (oldCmd) + if (oldCmd) + { + // Update inner Command with new key chord + auto oldCmdImpl{ get_self(oldCmd) }; + oldCmdImpl->RegisterKey(keys); + } + + // Additive operation: + // Register the new key chord with maskingCmd (an existing _maskingAction entry) + // Example: + // parent: { "command": "copy", "keys": "ctrl+c" } --> register "ctrl+c" to parent._ActionMap (different branch in a different layer) + // current: { "command": "copy", "keys": "ctrl+shift+c" } --> also register "ctrl+shift+c" to the same Command (maskingCmd) + if (maskingCmd) + { + // Update inner Command with new key chord + auto maskingCmdImpl{ get_self(maskingCmd) }; + maskingCmdImpl->RegisterKey(keys); + } + } + + // Method Description: + // - Retrieves the assigned command that can be invoked with the given key chord + // Arguments: + // - keys: the key chord of the command to search for + // Return Value: + // - the command with the given key chord + // - nullptr if the key chord is explicitly unbound + Model::Command ActionMap::GetActionByKeyChord(Control::KeyChord const& keys) const + { + // Check the current layer + const auto cmd{ _GetActionByKeyChordInternal(keys) }; + if (cmd.has_value()) + { + return *cmd; + } + + // This key chord is not explicitly bound + return nullptr; + } + + // Method Description: + // - Retrieves the assigned command with the given key chord. + // - Can return nullopt to differentiate explicit unbinding vs lack of binding. + // Arguments: + // - keys: the key chord of the command to search for + // Return Value: + // - the command with the given key chord + // - nullptr if the key chord is explicitly unbound + // - nullopt if it was not bound in this layer + std::optional ActionMap::_GetActionByKeyChordInternal(Control::KeyChord const& keys) const + { + // Check the current layer + const auto actionIDPair{ _KeyMap.find(keys) }; + if (actionIDPair != _KeyMap.end()) + { + // the command was explicitly bound, + // return what we found (invalid commands exposed as nullptr) + return _GetActionByID(actionIDPair->second); + } + + // the command was not bound in this layer, + // ask my parents + FAIL_FAST_IF(_parents.size() > 1); + for (const auto& parent : _parents) + { + const auto& inheritedCmd{ parent->_GetActionByKeyChordInternal(keys) }; + if (inheritedCmd.has_value()) + { + return *inheritedCmd; + } + } + + // This action is not explicitly bound + return std::nullopt; + } + + // Method Description: + // - Retrieves the key chord for the provided action + // Arguments: + // - action: the shortcut action (an action type) we're looking for + // Return Value: + // - the key chord that executes the given action + // - nullptr if the action is not bound to a key chord + Control::KeyChord ActionMap::GetKeyBindingForAction(ShortcutAction const& action) const + { + return GetKeyBindingForAction(action, nullptr); + } + + // Method Description: + // - Retrieves the key chord for the provided action + // Arguments: + // - action: the shortcut action (an action type) we're looking for + // - myArgs: the action args for the action we're looking for + // Return Value: + // - the key chord that executes the given action + // - nullptr if the action is not bound to a key chord + Control::KeyChord ActionMap::GetKeyBindingForAction(ShortcutAction const& myAction, IActionArgs const& myArgs) const + { + if (myAction == ShortcutAction::Invalid) + { + return nullptr; + } + + // Check our internal state. + const ActionAndArgs& actionAndArgs{ myAction, myArgs }; + const auto hash{ Hash(actionAndArgs) }; + if (const auto& cmd{ _GetActionByID(hash) }) + { + return cmd.value().Keys(); + } + + // Check our parents + FAIL_FAST_IF(_parents.size() > 1); + for (const auto& parent : _parents) + { + if (const auto& keys{ parent->GetKeyBindingForAction(myAction, myArgs) }) + { + return keys; + } + } + + // This key binding does not exist + return nullptr; + } +} diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.h b/src/cascadia/TerminalSettingsModel/ActionMap.h new file mode 100644 index 00000000000..b968712ee42 --- /dev/null +++ b/src/cascadia/TerminalSettingsModel/ActionMap.h @@ -0,0 +1,106 @@ +/*++ +Copyright (c) Microsoft Corporation +Licensed under the MIT license. + +Module Name: +- ActionMap.h + +Abstract: +- A mapping of key chords to actions. Includes (de)serialization logic. + +Author(s): +- Carlos Zamora - September 2020 + +--*/ + +#pragma once + +#include "ActionMap.g.h" +#include "IInheritable.h" +#include "Command.h" +#include "../inc/cppwinrt_utils.h" + +// fwdecl unittest classes +namespace SettingsModelLocalTests +{ + class KeyBindingsTests; + class DeserializationTests; + class TerminalSettingsTests; +} + +namespace winrt::Microsoft::Terminal::Settings::Model::implementation +{ + using InternalActionID = size_t; + + struct KeyChordHash + { + std::size_t operator()(const Control::KeyChord& key) const + { + return ::Microsoft::Terminal::Settings::Model::HashUtils::HashProperty(key.Modifiers(), key.Vkey()); + } + }; + + struct KeyChordEquality + { + bool operator()(const Control::KeyChord& lhs, const Control::KeyChord& rhs) const + { + return lhs.Modifiers() == rhs.Modifiers() && lhs.Vkey() == rhs.Vkey(); + } + }; + + struct ActionMap : ActionMapT, IInheritable + { + ActionMap(); + + // views + Windows::Foundation::Collections::IMapView NameMap(); + Windows::Foundation::Collections::IMapView GlobalHotkeys(); + com_ptr Copy() const; + + // queries + Model::Command GetActionByKeyChord(Control::KeyChord const& keys) const; + Control::KeyChord GetKeyBindingForAction(ShortcutAction const& action) const; + Control::KeyChord GetKeyBindingForAction(ShortcutAction const& action, IActionArgs const& actionArgs) const; + + // population + void AddAction(const Model::Command& cmd); + std::vector LayerJson(const Json::Value& json); + + static Windows::System::VirtualKeyModifiers ConvertVKModifiers(Control::KeyModifiers modifiers); + + private: + std::optional _GetActionByID(const InternalActionID actionID) const; + std::optional _GetActionByKeyChordInternal(Control::KeyChord const& keys) const; + + void _PopulateNameMapWithNestedCommands(std::unordered_map& nameMap) const; + void _PopulateNameMapWithStandardCommands(std::unordered_map& nameMap) const; + std::vector _GetCumulativeActions() const noexcept; + + void _TryUpdateActionMap(const Model::Command& cmd, Model::Command& oldCmd, Model::Command& consolidatedCmd); + void _TryUpdateName(const Model::Command& cmd, const Model::Command& oldCmd, const Model::Command& consolidatedCmd); + void _TryUpdateKeyChord(const Model::Command& cmd, const Model::Command& oldCmd, const Model::Command& consolidatedCmd); + + Windows::Foundation::Collections::IMap _NameMapCache{ nullptr }; + Windows::Foundation::Collections::IMap _GlobalHotkeysCache{ nullptr }; + Windows::Foundation::Collections::IMap _NestedCommands{ nullptr }; + std::unordered_map _KeyMap; + std::unordered_map _ActionMap; + + // Masking Actions: + // These are actions that were introduced in an ancestor, + // but were edited (or unbound) in the current layer. + // _ActionMap shows a Command with keys that were added in this layer, + // whereas _MaskingActions provides a view that encompasses all of + // the valid associated key chords. + // Maintaining this map allows us to return a valid Command + // in GetKeyBindingForAction. + // Additionally, these commands to not need to be serialized, + // whereas those in _ActionMap do. These actions provide more data + // than is necessary to be serialized. + std::unordered_map _MaskingActions; + + friend class SettingsModelLocalTests::KeyBindingsTests; + friend class SettingsModelLocalTests::DeserializationTests; + friend class SettingsModelLocalTests::TerminalSettingsTests; + }; +} diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.idl b/src/cascadia/TerminalSettingsModel/ActionMap.idl new file mode 100644 index 00000000000..11022ecbc9b --- /dev/null +++ b/src/cascadia/TerminalSettingsModel/ActionMap.idl @@ -0,0 +1,23 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import "Command.idl"; + +namespace Microsoft.Terminal.Settings.Model +{ + // This interface ensures that no changes are made to ActionMap + interface IActionMapView + { + Command GetActionByKeyChord(Microsoft.Terminal.Control.KeyChord keys); + + Microsoft.Terminal.Control.KeyChord GetKeyBindingForAction(ShortcutAction action); + [method_name("GetKeyBindingForActionWithArgs")] Microsoft.Terminal.Control.KeyChord GetKeyBindingForAction(ShortcutAction action, IActionArgs actionArgs); + + Windows.Foundation.Collections.IMapView NameMap { get; }; + Windows.Foundation.Collections.IMapView GlobalHotkeys(); + }; + + [default_interface] runtimeclass ActionMap : IActionMapView + { + } +} diff --git a/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp b/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp new file mode 100644 index 00000000000..cda5f929689 --- /dev/null +++ b/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp @@ -0,0 +1,80 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// - A couple helper functions for serializing/deserializing a KeyMapping +// to/from json. +// +// Author(s): +// - Mike Griese - May 2019 + +#include "pch.h" +#include "ActionMap.h" +#include "ActionAndArgs.h" +#include "KeyChordSerialization.h" +#include "JsonUtils.h" + +#include "Command.h" + +using namespace winrt::Microsoft::Terminal::Control; +using namespace winrt::Microsoft::Terminal::Settings::Model; + +namespace winrt::Microsoft::Terminal::Settings::Model::implementation +{ + // Method Description: + // - Deserialize an ActionMap from the array `json`. The json array should contain + // an array of serialized `Command` objects. + // - These actions are added to the `ActionMap`, where we automatically handle + // overwriting and unbinding actions. + // Arguments: + // - json: an array of Json::Value's to deserialize into our ActionMap. + // Return value: + // - a list of warnings encountered while deserializing the json + std::vector ActionMap::LayerJson(const Json::Value& json) + { + // It's possible that the user provided keybindings have some warnings in + // them - problems that we should alert the user to, but we can recover + // from. Most of these warnings cannot be detected later in the Validate + // settings phase, so we'll collect them now. + std::vector warnings; + + for (const auto& cmdJson : json) + { + if (!cmdJson.isObject()) + { + continue; + } + + AddAction(*Command::FromJson(cmdJson, warnings)); + } + + return warnings; + } + + // Method Description: + // - Takes the KeyModifier flags from Terminal and maps them to the Windows WinRT types + // Return Value: + // - a Windows::System::VirtualKeyModifiers object with the flags of which modifiers used. + Windows::System::VirtualKeyModifiers ActionMap::ConvertVKModifiers(KeyModifiers modifiers) + { + Windows::System::VirtualKeyModifiers keyModifiers = Windows::System::VirtualKeyModifiers::None; + + if (WI_IsFlagSet(modifiers, KeyModifiers::Ctrl)) + { + keyModifiers |= Windows::System::VirtualKeyModifiers::Control; + } + if (WI_IsFlagSet(modifiers, KeyModifiers::Shift)) + { + keyModifiers |= Windows::System::VirtualKeyModifiers::Shift; + } + if (WI_IsFlagSet(modifiers, KeyModifiers::Alt)) + { + // note: Menu is the Alt VK_MENU + keyModifiers |= Windows::System::VirtualKeyModifiers::Menu; + } + if (WI_IsFlagSet(modifiers, KeyModifiers::Windows)) + { + keyModifiers |= Windows::System::VirtualKeyModifiers::Windows; + } + + return keyModifiers; + } +} diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp index 0913a37f66c..42eac23fe77 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp @@ -191,9 +191,9 @@ IObservableVector Cascadia // - // Return Value: // - the globally configured keybindings -winrt::Microsoft::Terminal::Settings::Model::KeyMapping CascadiaSettings::KeyMap() const noexcept +winrt::Microsoft::Terminal::Settings::Model::ActionMap CascadiaSettings::ActionMap() const noexcept { - return _globals->KeyMap(); + return _globals->ActionMap(); } // Method Description: @@ -916,7 +916,7 @@ void CascadiaSettings::_ValidateKeybindings() void CascadiaSettings::_ValidateColorSchemesInCommands() { bool foundInvalidScheme{ false }; - for (const auto& nameAndCmd : _globals->Commands()) + for (const auto& nameAndCmd : _globals->ActionMap().NameMap()) { if (_HasInvalidColorScheme(nameAndCmd.Value())) { @@ -945,7 +945,7 @@ bool CascadiaSettings::_HasInvalidColorScheme(const Model::Command& command) } } } - else if (const auto& actionAndArgs = command.Action()) + else if (const auto& actionAndArgs = command.ActionAndArgs()) { if (const auto& realArgs = actionAndArgs.Args().try_as()) { diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.h b/src/cascadia/TerminalSettingsModel/CascadiaSettings.h index d63d39d458c..04b5e6650a3 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.h +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.h @@ -70,7 +70,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation Model::GlobalAppSettings GlobalSettings() const; Windows::Foundation::Collections::IObservableVector AllProfiles() const noexcept; Windows::Foundation::Collections::IObservableVector ActiveProfiles() const noexcept; - Model::KeyMapping KeyMap() const noexcept; + Model::ActionMap ActionMap() const noexcept; static com_ptr FromJson(const Json::Value& json); void LayerJson(const Json::Value& json); diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.idl b/src/cascadia/TerminalSettingsModel/CascadiaSettings.idl index 0428a98be4b..85882ec08c0 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.idl +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.idl @@ -33,7 +33,7 @@ namespace Microsoft.Terminal.Settings.Model Profile DuplicateProfile(Profile sourceProfile); - KeyMapping KeyMap { get; }; + ActionMap ActionMap { get; }; Windows.Foundation.Collections.IVectorView Warnings { get; }; Windows.Foundation.IReference GetLoadingError { get; }; diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp index 016b6bb3b37..bd83fd44970 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp @@ -248,57 +248,6 @@ winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings CascadiaSettings:: // If this throws, the app will catch it and use the default settings resultPtr->_ValidateSettings(); - // GH 3855 - Gathering Data on custom profiles to inform better defaults - // Do it after everything else so it won't happen unless validation passed. - // Also, avoid processing unless someone's listening for measures. The keybindings work, at least, - // is a lot of computation we can skip if no one cares. - if (TraceLoggingProviderEnabled(g_hSettingsModelProvider, 0, MICROSOFT_KEYWORD_MEASURES)) - { - const auto guid = resultPtr->GlobalSettings().DefaultProfile(); - - // Compare to the defaults.json one that we set on install. - // If it's different, log what the user chose. - if (hardcodedDefaultGuid != guid) - { - TraceLoggingWrite( - g_hSettingsModelProvider, // handle to TerminalApp tracelogging provider - "CustomDefaultProfile", - TraceLoggingDescription("Event emitted when user has chosen a different default profile than hardcoded one on load/reload"), - TraceLoggingGuid(guid, "DefaultProfile", "ID of user-chosen default profile"), - TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); - } - - // If the user had keybinding settings preferences, we want to learn from them to make better defaults - auto userKeybindings = resultPtr->_userSettings[JsonKey(LegacyKeybindingsKey)]; - if (!userKeybindings.empty()) - { - // If there are custom key bindings, let's understand what they are because maybe the defaults aren't good enough - - // Run it through the object so we can parse it apart and then only serialize the fields we're interested in - // and avoid extraneous data. - auto km = winrt::make_self(); - km->LayerJson(userKeybindings); - auto value = km->ToJson(); - - // Reserialize the keybindings - Json::StreamWriterBuilder wbuilder; - // Use 4 spaces to indent instead of \t - wbuilder.settings_["indentation"] = " "; - wbuilder.settings_["enableYAMLCompatibility"] = true; // suppress spaces around colons - - const auto keybindingsString = Json::writeString(wbuilder, value); - - TraceLoggingWrite( - g_hSettingsModelProvider, // handle to TerminalApp tracelogging provider - "CustomKeybindings", - TraceLoggingDescription("Event emitted when custom keybindings are identified on load/reload"), - TraceLoggingUtf8String(keybindingsString.c_str(), "Keybindings", "Keybindings as JSON"), - TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); - } - } - return *resultPtr; } catch (const SettingsException& ex) diff --git a/src/cascadia/TerminalSettingsModel/Command.cpp b/src/cascadia/TerminalSettingsModel/Command.cpp index 5d42d04743d..0f954225977 100644 --- a/src/cascadia/TerminalSettingsModel/Command.cpp +++ b/src/cascadia/TerminalSettingsModel/Command.cpp @@ -6,7 +6,7 @@ #include "Command.g.cpp" #include "ActionAndArgs.h" -#include "JsonUtils.h" +#include "KeyChordSerialization.h" #include #include "TerminalSettingsSerializationHelpers.h" @@ -26,6 +26,7 @@ static constexpr std::string_view ActionKey{ "command" }; static constexpr std::string_view ArgsKey{ "args" }; static constexpr std::string_view IterateOnKey{ "iterateOn" }; static constexpr std::string_view CommandsKey{ "commands" }; +static constexpr std::string_view KeysKey{ "keys" }; static constexpr std::string_view ProfileNameToken{ "${profile.name}" }; static constexpr std::string_view ProfileIconToken{ "${profile.icon}" }; @@ -35,16 +36,15 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { Command::Command() { - _setAction(nullptr); } com_ptr Command::Copy() const { auto command{ winrt::make_self() }; - command->_Name = _Name; - command->_Action = _Action; - command->_KeyChordText = _KeyChordText; - command->_IconPath = _IconPath; + command->_name = _name; + command->_ActionAndArgs = *get_self(_ActionAndArgs)->Copy(); + command->_keyMappings = _keyMappings; + command->_iconPath = _iconPath; command->_IterateOn = _IterateOn; command->_originalJson = _originalJson; @@ -65,10 +65,135 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation return _subcommands ? _subcommands.GetView() : nullptr; } + // Function Description: + // - reports if the current command has nested commands + // - This CANNOT detect { "name": "foo", "commands": null } bool Command::HasNestedCommands() const { return _subcommands ? _subcommands.Size() > 0 : false; } + + // Function Description: + // - reports if the current command IS a nested command + // - This CAN be used to detect cases like { "name": "foo", "commands": null } + bool Command::IsNestedCommand() const noexcept + { + return _nestedCommand; + } + + bool Command::HasName() const noexcept + { + return _name.has_value(); + } + + hstring Command::Name() const noexcept + { + if (_name.has_value()) + { + // name was explicitly set, return that value. + return hstring{ _name.value() }; + } + else if (_ActionAndArgs) + { + // generate a name from our action + return get_self(_ActionAndArgs)->GenerateName(); + } + else + { + // we have no name + return {}; + } + } + + void Command::Name(const hstring& value) + { + if (!_name.has_value() || _name.value() != value) + { + _name = value; + } + } + + std::vector Command::KeyMappings() const noexcept + { + return _keyMappings; + } + + // Function Description: + // - Add the key chord to the command's list of key mappings. + // - If the key chord was already registered, move it to the back + // of the line, and dispatch a notification that Command::Keys changed. + // Arguments: + // - keys: the new key chord that we are registering this command to + // Return Value: + // - + void Command::RegisterKey(const Control::KeyChord& keys) + { + if (!keys) + { + return; + } + + // Remove the KeyChord and add it to the back of the line. + // This makes it so that the main key chord associated with this + // command is updated. + EraseKey(keys); + _keyMappings.push_back(keys); + } + + // Function Description: + // - Remove the key chord from the command's list of key mappings. + // Arguments: + // - keys: the key chord that we are unregistering + // Return Value: + // - + void Command::EraseKey(const Control::KeyChord& keys) + { + _keyMappings.erase(std::remove_if(_keyMappings.begin(), _keyMappings.end(), [&keys](const Control::KeyChord& iterKey) { + return keys.Modifiers() == iterKey.Modifiers() && keys.Vkey() == iterKey.Vkey(); + }), + _keyMappings.end()); + } + + // Function Description: + // - Keys is the Command's identifying KeyChord. The command may have multiple keys associated + // with it, but we'll only ever display the most recently added one externally. To do this, + // _keyMappings stores all of the associated key chords, but ensures that the last entry + // is the most recently added one. + // Arguments: + // - + // Return Value: + // - the primary key chord associated with this Command + Control::KeyChord Command::Keys() const noexcept + { + if (_keyMappings.empty()) + { + return nullptr; + } + return _keyMappings.back(); + } + + hstring Command::KeyChordText() const noexcept + { + return KeyChordSerialization::ToString(Keys()); + } + + hstring Command::IconPath() const noexcept + { + if (_iconPath.has_value()) + { + return hstring{ *_iconPath }; + } + return {}; + } + + void Command::IconPath(const hstring& val) + { + if (!_iconPath.has_value() || _iconPath.value() != val) + { + _iconPath = val; + } + } + // Function Description: // - attempt to get the name of this command from the provided json object. // * If the "name" property is a string, return that value. @@ -79,7 +204,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // - json: The Json::Value representing the command object we should get the name for. // Return Value: // - the empty string if we couldn't find a name, otherwise the command's name. - static winrt::hstring _nameFromJson(const Json::Value& json) + static std::optional _nameFromJson(const Json::Value& json) { if (const auto name{ json[JsonKey(NameKey)] }) { @@ -89,43 +214,23 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { if (HasLibraryResourceWithName(*resourceKey)) { - return GetLibraryResourceString(*resourceKey); + return std::wstring{ GetLibraryResourceString(*resourceKey) }; } } } else if (name.isString()) { - return JsonUtils::GetValue(name); + return JsonUtils::GetValue(name); } } - - return L""; - } - - // Method Description: - // - Get the name for the command specified in `json`. If there is no "name" - // property in the provided json object, then instead generate a name for - // the provided ActionAndArgs. - // Arguments: - // - json: json for the command to generate a name for. - // - actionAndArgs: An ActionAndArgs object to use to generate a name for, - // if the json object doesn't contain a "name". - // Return Value: - // - The "name" from the json, or the generated name from ActionAndArgs::GenerateName - static winrt::hstring _nameFromJsonOrAction(const Json::Value& json, - winrt::com_ptr actionAndArgs) - { - auto manualName = _nameFromJson(json); - if (!manualName.empty()) - { - return manualName; - } - if (!actionAndArgs) + else if (json.isMember(JsonKey(NameKey))) { - return L""; + // { "name": null, "command": "copy" } will land in this case, which + // should also be used for unbinding. + return std::wstring{}; } - return actionAndArgs->GenerateName(); + return std::nullopt; } // Method Description: @@ -158,6 +263,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { // Initialize our list of subcommands. result->_subcommands = winrt::single_threaded_map(); + result->_nestedCommand = true; auto nestedWarnings = Command::LayerJson(result->_subcommands, nestedCommandsJson); // It's possible that the nested commands have some warnings warnings.insert(warnings.end(), nestedWarnings.begin(), nestedWarnings.end()); @@ -165,7 +271,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation if (result->_subcommands.Size() == 0) { warnings.push_back(SettingsLoadWarnings::FailedToParseSubCommands); - return nullptr; + result->_ActionAndArgs = make(); } nested = true; @@ -174,59 +280,60 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { // { "name": "foo", "commands": null } will land in this case, which // should also be used for unbinding. - return nullptr; + + // create an "invalid" ActionAndArgs + result->_ActionAndArgs = make(); + result->_nestedCommand = true; } - JsonUtils::GetValueForKey(json, IconKey, result->_IconPath); + JsonUtils::GetValueForKey(json, IconKey, result->_iconPath); // If we're a nested command, we can ignore the current action. if (!nested) { if (const auto actionJson{ json[JsonKey(ActionKey)] }) { - auto actionAndArgs = ActionAndArgs::FromJson(actionJson, warnings); - - if (actionAndArgs) - { - result->_setAction(*actionAndArgs); - } - else - { - // Something like - // { name: "foo", action: "unbound" } - // will _remove_ the "foo" command, by returning null here. - return nullptr; - } - - // If an iterable command doesn't have a name set, we'll still just - // try and generate a fake name for the command give the string we - // currently have. It'll probably generate something like "New tab, - // profile: ${profile.name}". This string will only be temporarily - // used internally, so there's no problem. - result->_setName(_nameFromJsonOrAction(json, actionAndArgs)); + result->_ActionAndArgs = *ActionAndArgs::FromJson(actionJson, warnings); } else { // { name: "foo", action: null } will land in this case, which // should also be used for unbinding. - return nullptr; + + // create an "invalid" ActionAndArgs + result->_ActionAndArgs = make(); + } + + // GH#4239 - If the user provided more than one key + // chord to a "keys" array, warn the user here. + // TODO: GH#1334 - remove this check. + const auto keysJson{ json[JsonKey(KeysKey)] }; + if (keysJson.isArray() && keysJson.size() > 1) + { + warnings.push_back(SettingsLoadWarnings::TooManyKeysForChord); + } + else + { + Control::KeyChord keys{ nullptr }; + if (JsonUtils::GetValueForKey(json, KeysKey, keys)) + { + result->RegisterKey(keys); + } } } - else - { - result->_setName(_nameFromJson(json)); - } + + // If an iterable command doesn't have a name set, we'll still just + // try and generate a fake name for the command give the string we + // currently have. It'll probably generate something like "New tab, + // profile: ${profile.name}". This string will only be temporarily + // used internally, so there's no problem. + result->_name = _nameFromJson(json); // Stash the original json value in this object. If the command is // iterable, we'll need to re-parse it later, once we know what all the // values we can iterate on are. result->_originalJson = json; - if (result->_Name.empty()) - { - return nullptr; - } - return result; } @@ -252,23 +359,23 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { try { - auto result = Command::FromJson(value, warnings); - if (result) - { - // Override commands with the same name - commands.Insert(result->Name(), *result); - } - else + const auto result = Command::FromJson(value, warnings); + if (result->ActionAndArgs().Action() == ShortcutAction::Invalid && !result->HasNestedCommands()) { // If there wasn't a parsed command, then try to get the // name from the json blob. If that name currently // exists in our list of commands, we should remove it. const auto name = _nameFromJson(value); - if (!name.empty()) + if (name.has_value() && !name->empty()) { - commands.Remove(name); + commands.Remove(*name); } } + else + { + // Override commands with the same name + commands.Insert(result->Name(), *result); + } } CATCH_LOG(); } diff --git a/src/cascadia/TerminalSettingsModel/Command.h b/src/cascadia/TerminalSettingsModel/Command.h index e8f1ea6b849..41125b73a20 100644 --- a/src/cascadia/TerminalSettingsModel/Command.h +++ b/src/cascadia/TerminalSettingsModel/Command.h @@ -21,6 +21,7 @@ Author(s): #include "Command.g.h" #include "TerminalWarnings.h" #include "Profile.h" +#include "ActionAndArgs.h" #include "../inc/cppwinrt_utils.h" #include "SettingsTypes.h" @@ -49,21 +50,35 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation static std::vector LayerJson(Windows::Foundation::Collections::IMap& commands, const Json::Value& json); bool HasNestedCommands() const; + bool IsNestedCommand() const noexcept; Windows::Foundation::Collections::IMapView NestedCommands() const; + bool HasName() const noexcept; + hstring Name() const noexcept; + void Name(const hstring& name); + + Control::KeyChord Keys() const noexcept; + hstring KeyChordText() const noexcept; + std::vector KeyMappings() const noexcept; + void RegisterKey(const Control::KeyChord& keys); + void EraseKey(const Control::KeyChord& keys); + + hstring IconPath() const noexcept; + void IconPath(const hstring& val); + winrt::Windows::UI::Xaml::Data::INotifyPropertyChanged::PropertyChanged_revoker propertyChangedRevoker; WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler); - WINRT_OBSERVABLE_PROPERTY(winrt::hstring, Name, _PropertyChangedHandlers); - WINRT_OBSERVABLE_PROPERTY(Model::ActionAndArgs, Action, _PropertyChangedHandlers); - WINRT_OBSERVABLE_PROPERTY(winrt::hstring, KeyChordText, _PropertyChangedHandlers); - WINRT_OBSERVABLE_PROPERTY(winrt::hstring, IconPath, _PropertyChangedHandlers); - WINRT_PROPERTY(ExpandCommandType, IterateOn, ExpandCommandType::None); + WINRT_PROPERTY(Model::ActionAndArgs, ActionAndArgs); private: Json::Value _originalJson; Windows::Foundation::Collections::IMap _subcommands{ nullptr }; + std::vector _keyMappings; + std::optional _name; + std::optional _iconPath; + bool _nestedCommand{ false }; static std::vector _expandCommand(Command* const expandable, Windows::Foundation::Collections::IVectorView profiles, diff --git a/src/cascadia/TerminalSettingsModel/Command.idl b/src/cascadia/TerminalSettingsModel/Command.idl index 0a2b8e3715e..b749a79fb0a 100644 --- a/src/cascadia/TerminalSettingsModel/Command.idl +++ b/src/cascadia/TerminalSettingsModel/Command.idl @@ -1,20 +1,42 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import "KeyMapping.idl"; +#include "AllShortcutActions.h" + +import "ActionArgs.idl"; import "Profile.idl"; import "ColorScheme.idl"; import "TerminalWarnings.idl"; namespace Microsoft.Terminal.Settings.Model { + enum ShortcutAction + { + Invalid = 0, // treat Invalid as unbound actions + + // When adding a new action, add them to AllShortcutActions.h! + #define ON_ALL_ACTIONS(action) action, + ALL_SHORTCUT_ACTIONS + #undef ON_ALL_ACTIONS + }; + + [default_interface] runtimeclass ActionAndArgs { + ActionAndArgs(); + ActionAndArgs(ShortcutAction action, IActionArgs args); + + IActionArgs Args; + ShortcutAction Action; + }; + [default_interface] runtimeclass Command : Windows.UI.Xaml.Data.INotifyPropertyChanged { Command(); - String Name; - ActionAndArgs Action; - String KeyChordText; + String Name { get; }; + ActionAndArgs ActionAndArgs { get; }; + Microsoft.Terminal.Control.KeyChord Keys { get; }; + void RegisterKey(Microsoft.Terminal.Control.KeyChord keys); + String KeyChordText { get; }; String IconPath; diff --git a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp index cc98e3d4a14..ee28bb8f51e 100644 --- a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp +++ b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp @@ -66,12 +66,11 @@ bool GlobalAppSettings::_getDefaultDebugFeaturesValue() } GlobalAppSettings::GlobalAppSettings() : - _keymap{ winrt::make_self() }, + _actionMap{ winrt::make_self() }, _keybindingsWarnings{}, _validDefaultProfile{ false }, _defaultProfile{} { - _commands = winrt::single_threaded_map(); _colorSchemes = winrt::single_threaded_map(); } @@ -87,10 +86,9 @@ void GlobalAppSettings::_FinalizeInheritance() FAIL_FAST_IF(_parents.size() > 1); for (auto parent : _parents) { - _keymap = std::move(parent->_keymap); + _actionMap->InsertParent(parent->_actionMap); _keybindingsWarnings = std::move(parent->_keybindingsWarnings); _colorSchemes = std::move(parent->_colorSchemes); - _commands = std::move(parent->_commands); } } @@ -131,10 +129,7 @@ winrt::com_ptr GlobalAppSettings::Copy() const globals->_UnparsedDefaultProfile = _UnparsedDefaultProfile; globals->_validDefaultProfile = _validDefaultProfile; globals->_defaultProfile = _defaultProfile; - if (_keymap) - { - globals->_keymap = _keymap->Copy(); - } + globals->_actionMap = _actionMap->Copy(); std::copy(_keybindingsWarnings.begin(), _keybindingsWarnings.end(), std::back_inserter(globals->_keybindingsWarnings)); if (_colorSchemes) @@ -146,15 +141,6 @@ winrt::com_ptr GlobalAppSettings::Copy() const } } - if (_commands) - { - for (auto kv : _commands) - { - const auto commandImpl{ winrt::get_self(kv.Value()) }; - globals->_commands.Insert(kv.Key(), *commandImpl->Copy()); - } - } - // Globals only ever has 1 parent FAIL_FAST_IF(_parents.size() > 1); for (auto parent : _parents) @@ -235,9 +221,9 @@ std::optional GlobalAppSettings::_getUnparsedDefaultProfileImpl( } #pragma endregion -winrt::Microsoft::Terminal::Settings::Model::KeyMapping GlobalAppSettings::KeyMap() const noexcept +winrt::Microsoft::Terminal::Settings::Model::ActionMap GlobalAppSettings::ActionMap() const noexcept { - return *_keymap; + return *_actionMap; } // Method Description: @@ -331,7 +317,8 @@ void GlobalAppSettings::LayerJson(const Json::Value& json) auto parseBindings = [this, &json](auto jsonKey) { if (auto bindings{ json[JsonKey(jsonKey)] }) { - auto warnings = _keymap->LayerJson(bindings); + auto warnings = _actionMap->LayerJson(bindings); + // It's possible that the user provided keybindings have some warnings // in them - problems that we should alert the user to, but we can // recover from. Most of these warnings cannot be detected later in the @@ -339,16 +326,6 @@ void GlobalAppSettings::LayerJson(const Json::Value& json) // warnings generated from parsing these keybindings, add them to our // list of warnings. _keybindingsWarnings.insert(_keybindingsWarnings.end(), warnings.begin(), warnings.end()); - - // Now parse the array again, but this time as a list of commands. - warnings = implementation::Command::LayerJson(_commands, bindings); - - // We cannot add all warnings, as some of them were already populated while parsing key mapping. - // Hence let's cherry-pick the ones relevant for command parsing. - if (std::count(warnings.begin(), warnings.end(), SettingsLoadWarnings::FailedToParseSubCommands)) - { - _keybindingsWarnings.push_back(SettingsLoadWarnings::FailedToParseSubCommands); - } } }; parseBindings(LegacyKeybindingsKey); @@ -385,11 +362,6 @@ std::vector G return _keybindingsWarnings; } -winrt::Windows::Foundation::Collections::IMapView GlobalAppSettings::Commands() noexcept -{ - return _commands.GetView(); -} - // Method Description: // - Create a new serialized JsonObject from an instance of this class // Arguments: diff --git a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h index 56026e6982b..016c7e11cc6 100644 --- a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h +++ b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h @@ -18,7 +18,7 @@ Author(s): #include "GlobalAppSettings.g.h" #include "IInheritable.h" -#include "KeyMapping.h" +#include "ActionMap.h" #include "Command.h" #include "ColorScheme.h" @@ -42,7 +42,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation void AddColorScheme(const Model::ColorScheme& scheme); void RemoveColorScheme(hstring schemeName); - Model::KeyMapping KeyMap() const noexcept; + Model::ActionMap ActionMap() const noexcept; static com_ptr FromJson(const Json::Value& json); void LayerJson(const Json::Value& json); @@ -51,8 +51,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation std::vector KeybindingsWarnings() const; - Windows::Foundation::Collections::IMapView Commands() noexcept; - // These are implemented manually to handle the string/GUID exchange // by higher layers in the app. void DefaultProfile(const guid& defaultProfile) noexcept; @@ -98,11 +96,10 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation std::optional _UnparsedDefaultProfile{ std::nullopt }; bool _validDefaultProfile; - com_ptr _keymap; + com_ptr _actionMap; std::vector _keybindingsWarnings; Windows::Foundation::Collections::IMap _colorSchemes; - Windows::Foundation::Collections::IMap _commands; std::optional _getUnparsedDefaultProfileImpl() const; static bool _getDefaultDebugFeaturesValue(); diff --git a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.idl b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.idl index a4df3eb111c..10992d54d3f 100644 --- a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.idl +++ b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.idl @@ -4,8 +4,7 @@ #include "IInheritable.idl.h" import "ColorScheme.idl"; -import "KeyMapping.idl"; -import "Command.idl"; +import "ActionMap.idl"; namespace Microsoft.Terminal.Settings.Model { @@ -74,8 +73,6 @@ namespace Microsoft.Terminal.Settings.Model void AddColorScheme(ColorScheme scheme); void RemoveColorScheme(String schemeName); - KeyMapping KeyMap(); - - Windows.Foundation.Collections.IMapView Commands(); + ActionMap ActionMap { get; }; } } diff --git a/src/cascadia/TerminalSettingsModel/HashUtils.h b/src/cascadia/TerminalSettingsModel/HashUtils.h new file mode 100644 index 00000000000..f0b81499507 --- /dev/null +++ b/src/cascadia/TerminalSettingsModel/HashUtils.h @@ -0,0 +1,70 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/*++ +Module Name: +- HashUtils.h + +Abstract: +- This module is used for hashing data consistently + +Author(s): +- Carlos Zamora (CaZamor) 15-Apr-2021 + +Revision History: +- N/A +--*/ + +#pragma once + +namespace Microsoft::Terminal::Settings::Model::HashUtils +{ + // This is a helper template function for hashing multiple variables in conjunction to each other. + template + constexpr size_t HashProperty(const T& val) + { + std::hash hashFunc; + return hashFunc(val); + } + + template + constexpr size_t HashProperty(const T& val, Args&&... more) + { + // Inspired by boost::hash_combine, which causes this effect... + // seed ^= hash_value(v) + 0x9e3779b9 + (seed << 6) + (seed >> 2); + // Source: https://www.boost.org/doc/libs/1_35_0/doc/html/boost/hash_combine_id241013.html + const auto seed{ HashProperty(val) }; + return seed ^ (0x9e3779b9 + (seed << 6) + (seed >> 2) + HashProperty(std::forward(more)...)); + } + + template<> + constexpr size_t HashProperty(const til::color& val) + { + return HashProperty(val.a, val.r, val.g, val.b); + } + +#ifdef WINRT_Windows_Foundation_H + template + constexpr size_t HashProperty(const winrt::Windows::Foundation::IReference& val) + { + return val ? HashProperty(val.Value()) : 0; + } +#endif + +#ifdef WINRT_Windows_UI_H + template<> + constexpr size_t HashProperty(const winrt::Windows::UI::Color& val) + { + return HashProperty(til::color{ val }); + } +#endif + +#ifdef WINRT_Microsoft_Terminal_Core_H + template<> + constexpr size_t HashProperty(const winrt::Microsoft::Terminal::Core::Color& val) + { + return HashProperty(til::color{ val }); + } +#endif + +}; diff --git a/src/cascadia/TerminalSettingsModel/KeyChordSerialization.cpp b/src/cascadia/TerminalSettingsModel/KeyChordSerialization.cpp index b240b8d2802..25dec03d0d4 100644 --- a/src/cascadia/TerminalSettingsModel/KeyChordSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/KeyChordSerialization.cpp @@ -7,6 +7,7 @@ using namespace winrt::Microsoft::Terminal::Control; using namespace winrt::Microsoft::Terminal::Settings::Model::implementation; +using namespace Microsoft::Terminal::Settings::Model::JsonUtils; static constexpr std::wstring_view CTRL_KEY{ L"ctrl" }; static constexpr std::wstring_view SHIFT_KEY{ L"shift" }; @@ -104,14 +105,13 @@ static const std::unordered_map vkeyNamePairs { // - hstr: the string to parse into a keychord. // Return Value: // - a newly constructed KeyChord -KeyChord KeyChordSerialization::FromString(const winrt::hstring& hstr) +static KeyChord _fromString(const std::wstring_view& wstr) { - std::wstring wstr{ hstr }; - // Split the string on '+' std::wstring temp; std::vector parts; - std::wstringstream wss(wstr); + std::wstringstream wss; + wss << wstr; while (std::getline(wss, temp, L'+')) { @@ -220,8 +220,13 @@ KeyChord KeyChordSerialization::FromString(const winrt::hstring& hstr) // names listed in the vkeyNamePairs vector above, or is one of 0-9a-z. // Return Value: // - a string which is an equivalent serialization of this object. -winrt::hstring KeyChordSerialization::ToString(const KeyChord& chord) +static std::wstring _toString(const KeyChord& chord) { + if (!chord) + { + return {}; + } + bool serializedSuccessfully = false; const auto modifiers = chord.Modifiers(); const auto vkey = chord.Vkey(); @@ -292,5 +297,57 @@ winrt::hstring KeyChordSerialization::ToString(const KeyChord& chord) buffer = L""; } - return winrt::hstring{ buffer }; + return buffer; +} + +KeyChord KeyChordSerialization::FromString(const winrt::hstring& hstr) +{ + return _fromString(hstr); +} + +winrt::hstring KeyChordSerialization::ToString(const KeyChord& chord) +{ + return hstring{ _toString(chord) }; +} + +KeyChord ConversionTrait::FromJson(const Json::Value& json) +{ + try + { + std::string keyChordText; + if (json.isString()) + { + // "keys": "ctrl+c" + keyChordText = json.asString(); + } + else if (json.isArray() && json.size() == 1 && json[0].isString()) + { + // "keys": [ "ctrl+c" ] + keyChordText = json[0].asString(); + } + else + { + throw winrt::hresult_invalid_argument{}; + } + return _fromString(til::u8u16(keyChordText)); + } + catch (...) + { + return nullptr; + } +} + +bool ConversionTrait::CanConvert(const Json::Value& json) +{ + return json.isString() || (json.isArray() && json.size() == 1 && json[0].isString()); +} + +Json::Value ConversionTrait::ToJson(const KeyChord& val) +{ + return til::u16u8(_toString(val)); +} + +std::string ConversionTrait::TypeDescription() const +{ + return "key chord"; } diff --git a/src/cascadia/TerminalSettingsModel/KeyChordSerialization.h b/src/cascadia/TerminalSettingsModel/KeyChordSerialization.h index 5b795f89040..3f80c920496 100644 --- a/src/cascadia/TerminalSettingsModel/KeyChordSerialization.h +++ b/src/cascadia/TerminalSettingsModel/KeyChordSerialization.h @@ -4,6 +4,7 @@ #pragma once #include "KeyChordSerialization.g.h" +#include "JsonUtils.h" #include "../inc/cppwinrt_utils.h" namespace winrt::Microsoft::Terminal::Settings::Model::implementation @@ -17,6 +18,15 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation }; } +template<> +struct Microsoft::Terminal::Settings::Model::JsonUtils::ConversionTrait +{ + winrt::Microsoft::Terminal::Control::KeyChord FromJson(const Json::Value& json); + bool CanConvert(const Json::Value& json); + Json::Value ToJson(const winrt::Microsoft::Terminal::Control::KeyChord& val); + std::string TypeDescription() const; +}; + namespace winrt::Microsoft::Terminal::Settings::Model::factory_implementation { // C++/WinRT generates a constructor even though one is not specified in the IDL diff --git a/src/cascadia/TerminalSettingsModel/KeyMapping.cpp b/src/cascadia/TerminalSettingsModel/KeyMapping.cpp deleted file mode 100644 index 3bd93cfe59f..00000000000 --- a/src/cascadia/TerminalSettingsModel/KeyMapping.cpp +++ /dev/null @@ -1,172 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#include "pch.h" -#include "KeyMapping.h" -#include "KeyChordSerialization.h" -#include "ActionAndArgs.h" - -#include "KeyMapping.g.cpp" - -using namespace winrt::Microsoft::Terminal::Settings::Model; -using namespace winrt::Microsoft::Terminal::Control; - -namespace winrt::Microsoft::Terminal::Settings::Model::implementation -{ - winrt::com_ptr KeyMapping::Copy() const - { - auto keymap{ winrt::make_self() }; - std::for_each(_keyShortcuts.begin(), _keyShortcuts.end(), [keymap](auto& kv) { - const auto actionAndArgsImpl{ winrt::get_self(kv.second) }; - keymap->_keyShortcuts[kv.first] = *actionAndArgsImpl->Copy(); - }); - return keymap; - } - - Microsoft::Terminal::Settings::Model::ActionAndArgs KeyMapping::TryLookup(KeyChord const& chord) const - { - const auto result = _keyShortcuts.find(chord); - if (result != _keyShortcuts.end()) - { - return result->second; - } - return nullptr; - } - - uint64_t KeyMapping::Size() const - { - return _keyShortcuts.size(); - } - - void KeyMapping::SetKeyBinding(const Microsoft::Terminal::Settings::Model::ActionAndArgs& actionAndArgs, - const KeyChord& chord) - { - // if the chord is already mapped - clear the mapping - if (_keyShortcuts.find(chord) != _keyShortcuts.end()) - { - ClearKeyBinding(chord); - } - - _keyShortcuts[chord] = actionAndArgs; - _keyShortcutsByInsertionOrder.push_back(std::make_pair(chord, actionAndArgs)); - } - - // Method Description: - // - Remove the action that's bound to a particular KeyChord. - // Arguments: - // - chord: the keystroke to remove the action for. - // Return Value: - // - - void KeyMapping::ClearKeyBinding(const KeyChord& chord) - { - _keyShortcuts.erase(chord); - - KeyChordEquality keyChordEquality; - _keyShortcutsByInsertionOrder.erase(std::remove_if(_keyShortcutsByInsertionOrder.begin(), _keyShortcutsByInsertionOrder.end(), [keyChordEquality, chord](const auto& keyBinding) { - return keyChordEquality(keyBinding.first, chord); - }), - _keyShortcutsByInsertionOrder.end()); - } - - KeyChord KeyMapping::GetKeyBindingForAction(Microsoft::Terminal::Settings::Model::ShortcutAction const& action) - { - for (auto it = _keyShortcutsByInsertionOrder.rbegin(); it != _keyShortcutsByInsertionOrder.rend(); ++it) - { - const auto& kv = *it; - if (kv.second.Action() == action) - { - return kv.first; - } - } - 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. - // If several bindings might match the lookup, prefers the one that was added last. - // 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 KeyMapping::GetKeyBindingForActionWithArgs(Microsoft::Terminal::Settings::Model::ActionAndArgs const& actionAndArgs) - { - if (actionAndArgs == nullptr) - { - return { nullptr }; - } - - for (auto it = _keyShortcutsByInsertionOrder.rbegin(); it != _keyShortcutsByInsertionOrder.rend(); ++it) - { - const auto& kv = *it; - const auto action = kv.second.Action(); - const auto args = kv.second.Args(); - const auto actionMatched = action == actionAndArgs.Action(); - const auto argsMatched = args ? args.Equals(actionAndArgs.Args()) : args == actionAndArgs.Args(); - if (actionMatched && argsMatched) - { - return kv.first; - } - } - return { nullptr }; - } - - // Method Description: - // - Takes the KeyModifier flags from Terminal and maps them to the WinRT types which are used by XAML - // Return Value: - // - a Windows::System::VirtualKeyModifiers object with the flags of which modifiers used. - Windows::System::VirtualKeyModifiers KeyMapping::ConvertVKModifiers(KeyModifiers modifiers) - { - Windows::System::VirtualKeyModifiers keyModifiers = Windows::System::VirtualKeyModifiers::None; - - if (WI_IsFlagSet(modifiers, KeyModifiers::Ctrl)) - { - keyModifiers |= Windows::System::VirtualKeyModifiers::Control; - } - if (WI_IsFlagSet(modifiers, KeyModifiers::Shift)) - { - keyModifiers |= Windows::System::VirtualKeyModifiers::Shift; - } - if (WI_IsFlagSet(modifiers, KeyModifiers::Alt)) - { - // note: Menu is the Alt VK_MENU - keyModifiers |= Windows::System::VirtualKeyModifiers::Menu; - } - if (WI_IsFlagSet(modifiers, KeyModifiers::Windows)) - { - keyModifiers |= Windows::System::VirtualKeyModifiers::Windows; - } - - return keyModifiers; - } - - // Method Description: - // - Build a map of all the globalSummon actions. - // - quakeMode actions are included in this, but expanded to the equivalent - // set of GlobalSummonArgs - // - This is only ever called in two scenarios: - // - on becoming the monarch (which only happens once per window) - // - when the settings reload (and the cache would inevitably be dirty) - // So it's perfectly reasonable to not cache these results. - // Arguments: - // - - // Return Value: - // - a map of KeyChord -> ActionAndArgs containing all globally bindable actions. - Windows::Foundation::Collections::IMap KeyMapping::GlobalHotkeys() - { - std::unordered_map justGlobals; - - for (const auto& [k, v] : _keyShortcuts) - { - if (v.Action() == ShortcutAction::GlobalSummon || v.Action() == ShortcutAction::QuakeMode) - { - justGlobals[k] = v; - } - } - - return winrt::single_threaded_map(std::move(justGlobals)); - } - -} diff --git a/src/cascadia/TerminalSettingsModel/KeyMapping.h b/src/cascadia/TerminalSettingsModel/KeyMapping.h deleted file mode 100644 index eea978b59a6..00000000000 --- a/src/cascadia/TerminalSettingsModel/KeyMapping.h +++ /dev/null @@ -1,82 +0,0 @@ -/*++ -Copyright (c) Microsoft Corporation -Licensed under the MIT license. - -Module Name: -- KeyMapping.h - -Abstract: -- A mapping of key chords to actions. Includes (de)serialization logic. - -Author(s): -- Carlos Zamora - September 2020 - ---*/ - -#pragma once - -#include "KeyMapping.g.h" -#include "ActionArgs.h" -#include "../inc/cppwinrt_utils.h" - -// fwdecl unittest classes -namespace SettingsModelLocalTests -{ - class DeserializationTests; - class KeyBindingsTests; - class TestUtils; -} - -namespace winrt::Microsoft::Terminal::Settings::Model::implementation -{ - struct KeyChordHash - { - std::size_t operator()(const Control::KeyChord& key) const - { - std::hash keyHash; - std::hash modifiersHash; - std::size_t hashedKey = keyHash(key.Vkey()); - std::size_t hashedMods = modifiersHash(key.Modifiers()); - return hashedKey ^ hashedMods; - } - }; - - struct KeyChordEquality - { - bool operator()(const Control::KeyChord& lhs, const Control::KeyChord& rhs) const - { - return lhs.Modifiers() == rhs.Modifiers() && lhs.Vkey() == rhs.Vkey(); - } - }; - - struct KeyMapping : KeyMappingT - { - KeyMapping() = default; - com_ptr Copy() const; - - Model::ActionAndArgs TryLookup(Control::KeyChord const& chord) const; - uint64_t Size() const; - - void SetKeyBinding(Model::ActionAndArgs const& actionAndArgs, - Control::KeyChord const& chord); - void ClearKeyBinding(Control::KeyChord const& chord); - Control::KeyChord GetKeyBindingForAction(Model::ShortcutAction const& action); - Control::KeyChord GetKeyBindingForActionWithArgs(Model::ActionAndArgs const& actionAndArgs); - - static Windows::System::VirtualKeyModifiers ConvertVKModifiers(Control::KeyModifiers modifiers); - - // Defined in KeyMappingSerialization.cpp - std::vector LayerJson(const Json::Value& json); - Json::Value ToJson(); - - Windows::Foundation::Collections::IMap GlobalHotkeys(); - - private: - std::unordered_map _keyShortcuts; - std::vector> _keyShortcutsByInsertionOrder; - - friend class SettingsModelLocalTests::DeserializationTests; - friend class SettingsModelLocalTests::KeyBindingsTests; - friend class SettingsModelLocalTests::TestUtils; - }; -} diff --git a/src/cascadia/TerminalSettingsModel/KeyMapping.idl b/src/cascadia/TerminalSettingsModel/KeyMapping.idl deleted file mode 100644 index 1b6886acded..00000000000 --- a/src/cascadia/TerminalSettingsModel/KeyMapping.idl +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#include "AllShortcutActions.h" - -import "ActionArgs.idl"; - -namespace Microsoft.Terminal.Settings.Model -{ - enum ShortcutAction - { - Invalid = 0, - - // When adding a new action, add them to AllShortcutActions.h! - #define ON_ALL_ACTIONS(action) action, - ALL_SHORTCUT_ACTIONS - #undef ON_ALL_ACTIONS - }; - - [default_interface] runtimeclass ActionAndArgs { - ActionAndArgs(); - ActionAndArgs(ShortcutAction action, IActionArgs args); - - IActionArgs Args; - ShortcutAction Action; - }; - - [default_interface] runtimeclass KeyMapping - { - ActionAndArgs TryLookup(Microsoft.Terminal.Control.KeyChord chord); - UInt64 Size(); - - void SetKeyBinding(ActionAndArgs actionAndArgs, Microsoft.Terminal.Control.KeyChord chord); - void ClearKeyBinding(Microsoft.Terminal.Control.KeyChord chord); - - Microsoft.Terminal.Control.KeyChord GetKeyBindingForAction(ShortcutAction action); - Microsoft.Terminal.Control.KeyChord GetKeyBindingForActionWithArgs(ActionAndArgs actionAndArgs); - - Windows.Foundation.Collections.IMap GlobalHotkeys(); - } -} diff --git a/src/cascadia/TerminalSettingsModel/KeyMappingSerialization.cpp b/src/cascadia/TerminalSettingsModel/KeyMappingSerialization.cpp deleted file mode 100644 index 24d6a4bee16..00000000000 --- a/src/cascadia/TerminalSettingsModel/KeyMappingSerialization.cpp +++ /dev/null @@ -1,162 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. -// - A couple helper functions for serializing/deserializing a KeyMapping -// to/from json. -// -// Author(s): -// - Mike Griese - May 2019 - -#include "pch.h" -#include "KeyMapping.h" -#include "ActionAndArgs.h" -#include "KeyChordSerialization.h" -#include "JsonUtils.h" - -using namespace winrt::Microsoft::Terminal::Control; -using namespace winrt::Microsoft::Terminal::Settings::Model; - -static constexpr std::string_view KeysKey{ "keys" }; -static constexpr std::string_view CommandKey{ "command" }; -static constexpr std::string_view ActionKey{ "action" }; - -// Function Description: -// - Small helper to create a json value serialization of a single -// KeyBinding->Action mapping. -// { -// keys:[String], -// command:String -// } -// Arguments: -// - chord: The KeyChord to serialize -// - actionName: the name of the ShortcutAction to use with this KeyChord -// Return Value: -// - a Json::Value which is an equivalent serialization of this object. -static Json::Value _ShortcutAsJsonObject(const KeyChord& chord, - const std::string_view actionName) -{ - const auto keyString = KeyChordSerialization::ToString(chord); - if (keyString == L"") - { - return nullptr; - } - - Json::Value jsonObject; - Json::Value keysArray; - keysArray.append(winrt::to_string(keyString)); - - jsonObject[JsonKey(KeysKey)] = keysArray; - jsonObject[JsonKey(CommandKey)] = actionName.data(); - - return jsonObject; -} - -// Method Description: -// - Serialize this KeyMapping to a json array of objects. Each object in -// the array represents a single keybinding, mapping a KeyChord to a -// ShortcutAction. -// Return Value: -// - a Json::Value which is an equivalent serialization of this object. -Json::Value winrt::Microsoft::Terminal::Settings::Model::implementation::KeyMapping::ToJson() -{ - Json::Value bindingsArray; - - // Iterate over all the possible actions in the names list, and see if - // it has a binding. - for (auto& actionName : ActionAndArgs::ActionKeyNamesMap) - { - const auto searchedForName = actionName.first; - const auto searchedForAction = actionName.second; - - if (const auto chord{ GetKeyBindingForAction(searchedForAction) }) - { - if (const auto serialization{ _ShortcutAsJsonObject(chord, searchedForName) }) - { - bindingsArray.append(serialization); - } - } - } - - return bindingsArray; -} - -// Method Description: -// - Deserialize a KeyMapping from the key mappings that are in the array -// `json`. The json array should contain an array of objects with both a -// `command` string and a `keys` array, where `command` is one of the names -// listed in `ActionAndArgs::ActionKeyNamesMap`, and `keys` is an array of -// keypresses. Currently, the array should contain a single string, which can -// be deserialized into a KeyChord. -// - Applies the deserialized keybindings to the provided `bindings` object. If -// a key chord in `json` is already bound to an action, that chord will be -// overwritten with the new action. If a chord is bound to `null` or -// `"unbound"`, then we'll clear the keybinding from the existing keybindings. -// Arguments: -// - json: an array of Json::Value's to deserialize into our _keyShortcuts mapping. -std::vector winrt::Microsoft::Terminal::Settings::Model::implementation::KeyMapping::LayerJson(const Json::Value& json) -{ - // It's possible that the user provided keybindings have some warnings in - // them - problems that we should alert the user to, but we can recover - // from. Most of these warnings cannot be detected later in the Validate - // settings phase, so we'll collect them now. - std::vector warnings; - - for (const auto& value : json) - { - if (!value.isObject()) - { - continue; - } - - const auto commandVal = value[JsonKey(CommandKey)]; - const auto keys = value[JsonKey(KeysKey)]; - - if (keys) - { - const auto validString = keys.isString(); - const auto validArray = keys.isArray() && keys.size() == 1; - - // GH#4239 - If the user provided more than one key - // chord to a "keys" array, warn the user here. - // TODO: GH#1334 - remove this check. - if (keys.isArray() && keys.size() > 1) - { - warnings.push_back(SettingsLoadWarnings::TooManyKeysForChord); - } - - if (!validString && !validArray) - { - continue; - } - const auto keyChordString = keys.isString() ? winrt::to_hstring(keys.asString()) : winrt::to_hstring(keys[0].asString()); - - // If the action was null, "unbound", or something we didn't - // understand, this will return nullptr. - auto actionAndArgs = ActionAndArgs::FromJson(commandVal, warnings); - - // Try parsing the chord - try - { - const auto chord = KeyChordSerialization::FromString(keyChordString); - - // If we couldn't find the action they want to set the chord to, - // or the action was `null` or `"unbound"`, just clear out the - // keybinding. Otherwise, set the keybinding to the action we - // found. - if (actionAndArgs) - { - SetKeyBinding(*actionAndArgs, chord); - } - else - { - ClearKeyBinding(chord); - } - } - catch (...) - { - continue; - } - } - } - - return warnings; -} diff --git a/src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj b/src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj index 09463cbea0c..b9351f9a634 100644 --- a/src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj +++ b/src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj @@ -29,6 +29,9 @@ ActionArgs.idl + + ActionMap.idl + CascadiaSettings.idl @@ -46,12 +49,10 @@ + KeyChordSerialization.idl - - KeyMapping.idl - Profile.idl @@ -90,6 +91,12 @@ ActionArgs.idl + + ActionMap.idl + + + ActionMap.idl + CascadiaSettings.idl @@ -110,12 +117,6 @@ KeyChordSerialization.idl - - KeyMapping.idl - - - KeyMapping.idl - Profile.idl @@ -140,13 +141,13 @@ + - diff --git a/src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj.filters b/src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj.filters index 62b725e6667..d844a34c86d 100644 --- a/src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj.filters +++ b/src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj.filters @@ -69,10 +69,10 @@ + - diff --git a/src/cascadia/TerminalSettingsModel/dll/Microsoft.Terminal.Settings.Model.vcxproj b/src/cascadia/TerminalSettingsModel/dll/Microsoft.Terminal.Settings.Model.vcxproj index 730a8518251..f83d4a8ce65 100644 --- a/src/cascadia/TerminalSettingsModel/dll/Microsoft.Terminal.Settings.Model.vcxproj +++ b/src/cascadia/TerminalSettingsModel/dll/Microsoft.Terminal.Settings.Model.vcxproj @@ -22,6 +22,7 @@ + diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index aa5f79369df..348a0a9a7f4 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -680,10 +680,10 @@ void AppHost::_GlobalHotkeyPressed(const long hotkeyIndex) } // Lookup the matching keychord Control::KeyChord kc = _hotkeys.at(hotkeyIndex); - // Get the stored ActionAndArgs for that chord - if (const auto& actionAndArgs{ _hotkeyActions.Lookup(kc) }) + // Get the stored Command for that chord + if (const auto& cmd{ _hotkeyActions.Lookup(kc) }) { - if (const auto& summonArgs{ actionAndArgs.Args().try_as() }) + if (const auto& summonArgs{ cmd.ActionAndArgs().Args().try_as() }) { Remoting::SummonWindowSelectionArgs args{ summonArgs.Name() }; diff --git a/src/cascadia/WindowsTerminal/AppHost.h b/src/cascadia/WindowsTerminal/AppHost.h index 6abfb511b30..712b757ef52 100644 --- a/src/cascadia/WindowsTerminal/AppHost.h +++ b/src/cascadia/WindowsTerminal/AppHost.h @@ -29,7 +29,7 @@ class AppHost winrt::Microsoft::Terminal::Remoting::WindowManager _windowManager{ nullptr }; std::vector _hotkeys{}; - winrt::Windows::Foundation::Collections::IMap _hotkeyActions{ nullptr }; + winrt::Windows::Foundation::Collections::IMapView _hotkeyActions{ nullptr }; winrt::com_ptr _desktopManager{ nullptr }; diff --git a/src/cascadia/ut_app/TerminalApp.UnitTests.vcxproj b/src/cascadia/ut_app/TerminalApp.UnitTests.vcxproj index 822db5a74e0..276daed1725 100644 --- a/src/cascadia/ut_app/TerminalApp.UnitTests.vcxproj +++ b/src/cascadia/ut_app/TerminalApp.UnitTests.vcxproj @@ -72,6 +72,13 @@ WindowsApp.lib;%(AdditionalDependencies) + + /INCLUDE:_DllMain@12 + /INCLUDE:DllMain