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