diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json index e0c5888bb2d..5608be20b54 100644 --- a/doc/cascadia/profiles.schema.json +++ b/doc/cascadia/profiles.schema.json @@ -2058,7 +2058,7 @@ ] } }, - "Keybinding": { + "FullCommand": { "additionalProperties": false, "properties": { "command": { @@ -2186,21 +2186,6 @@ } ] }, - "keys": { - "description": "Defines the key combinations used to call the command. It must be composed of...\n -any number of modifiers (ctrl/alt/shift)\n -a non-modifier key", - "oneOf": [ - { - "$ref": "#/$defs/KeyChordSegment" - }, - { - "items": { - "$ref": "#/$defs/KeyChordSegment" - }, - "minItems": 1, - "type": "array" - } - ] - }, "icon": { "$ref": "#/$defs/Icon" }, @@ -2235,10 +2220,10 @@ "type": "object", "properties": { "command": { - "$ref": "#/$defs/Keybinding/properties/command" + "$ref": "#/$defs/FullCommand/properties/command" }, "name": { - "$ref": "#/$defs/Keybinding/properties/name" + "$ref": "#/$defs/FullCommand/properties/name" } } }, @@ -2261,6 +2246,44 @@ ], "type": "object" }, + "Keybinding": { + "additionalProperties": false, + "properties": { + "id": { + "description": "The ID of the command this keybinding should execute.", + "type": "string" + }, + "keys": { + "description": "Defines the key combinations used to call the command. It must be composed of...\n -any number of modifiers (ctrl/alt/shift)\n -a non-modifier key", + "oneOf": [ + { + "$ref": "#/$defs/KeyChordSegment" + }, + { + "items": { + "$ref": "#/$defs/KeyChordSegment" + }, + "minItems": 1, + "type": "array" + } + ] + } + }, + "anyOf": [ + { + "required": [ + "keys", + "id" + ] + }, + { + "required": [ + "keys" + ] + } + ], + "type": "object" + }, "Globals": { "additionalProperties": true, "description": "Properties that affect the entire window, regardless of the profile settings.", @@ -2464,12 +2487,12 @@ "actions": { "description": "Properties are specific to each custom action.", "items": { - "$ref": "#/$defs/Keybinding" + "$ref": "#/$defs/FullCommand" }, "type": "array" }, "keybindings": { - "description": "[deprecated] Use actions instead.", + "description": "A list of keychords bound to action IDs", "deprecated": true, "items": { "$ref": "#/$defs/Keybinding" diff --git a/src/cascadia/TerminalApp/TabBase.cpp b/src/cascadia/TerminalApp/TabBase.cpp index 04b7268ba06..2d5e8da4433 100644 --- a/src/cascadia/TerminalApp/TabBase.cpp +++ b/src/cascadia/TerminalApp/TabBase.cpp @@ -192,7 +192,8 @@ namespace winrt::TerminalApp::implementation // - void TabBase::_UpdateSwitchToTabKeyChord() { - const auto keyChord = _actionMap ? _actionMap.GetKeyBindingForAction(ShortcutAction::SwitchToTab, SwitchToTabArgs{ _TabViewIndex }) : nullptr; + const auto id = fmt::format(FMT_COMPILE(L"Terminal.SwitchToTab{}"), _TabViewIndex); + const auto keyChord{ _actionMap.GetKeyBindingForAction(id) }; const auto keyChordText = keyChord ? KeyChordSerialization::ToString(keyChord) : L""; if (_keyChord == keyChordText) diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 3a82a05aebf..27b12d0ae4c 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -826,7 +826,7 @@ namespace winrt::TerminalApp::implementation newTabFlyout.Items().Append(settingsItem); auto actionMap = _settings.ActionMap(); - const auto settingsKeyChord{ actionMap.GetKeyBindingForAction(ShortcutAction::OpenSettings, OpenSettingsArgs{ SettingsTarget::SettingsUI }) }; + const auto settingsKeyChord{ actionMap.GetKeyBindingForAction(L"Terminal.OpenSettingsUI") }; if (settingsKeyChord) { _SetAcceleratorForMenuItem(settingsItem, settingsKeyChord); @@ -848,7 +848,7 @@ namespace winrt::TerminalApp::implementation commandPaletteFlyout.Click({ this, &TerminalPage::_CommandPaletteButtonOnClick }); newTabFlyout.Items().Append(commandPaletteFlyout); - const auto commandPaletteKeyChord{ actionMap.GetKeyBindingForAction(ShortcutAction::ToggleCommandPalette) }; + const auto commandPaletteKeyChord{ actionMap.GetKeyBindingForAction(L"Terminal.ToggleCommandPalette") }; if (commandPaletteKeyChord) { _SetAcceleratorForMenuItem(commandPaletteFlyout, commandPaletteKeyChord); @@ -1023,7 +1023,8 @@ namespace winrt::TerminalApp::implementation // NewTab(ProfileIndex=N) action NewTerminalArgs newTerminalArgs{ profileIndex }; NewTabArgs newTabArgs{ newTerminalArgs }; - auto profileKeyChord{ _settings.ActionMap().GetKeyBindingForAction(ShortcutAction::NewTab, newTabArgs) }; + const auto id = fmt::format(FMT_COMPILE(L"Terminal.OpenNewTabProfile{}"), profileIndex); + const auto profileKeyChord{ _settings.ActionMap().GetKeyBindingForAction(id) }; // make sure we find one to display if (profileKeyChord) diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.cpp b/src/cascadia/TerminalSettingsModel/ActionMap.cpp index 220cd825565..3458f1e180e 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMap.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionMap.cpp @@ -59,31 +59,87 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } // Method Description: - // - Retrieves the Command in the current layer, if it's valid - // - We internally store invalid commands as full commands. - // This helper function returns nullptr when we get an invalid - // command. This allows us to simply check for null when we - // want a valid command. - // Arguments: - // - actionID: the internal ID associated with a Command - // Return Value: - // - If the command is valid, the command itself. - // - If the command is explicitly unbound, nullptr. - // - If the command cannot be found in this layer, nullopt. - std::optional ActionMap::_GetActionByID(const InternalActionID actionID) const - { - // Check the masking actions - const auto maskingPair{ _MaskingActions.find(actionID) }; - if (maskingPair != _MaskingActions.end()) + // - Detects if any of the user's actions are identical to the inbox actions, + // and if so, deletes them and redirects their keybindings to the inbox actions + // - We have to do this here instead of when loading since we don't actually have + // any parents while loading the user settings, the parents are added after + void ActionMap::_FinalizeInheritance() + { + // first, gather the inbox actions from the relevant parent + std::unordered_map inboxActions; + winrt::com_ptr foundParent{ nullptr }; + for (const auto& parent : _parents) { - // ActionMap should never point to nullptr - FAIL_FAST_IF_NULL(maskingPair->second); + const auto parentMap = parent->_ActionMap; + if (parentMap.begin() != parentMap.end() && parentMap.begin()->second.Origin() == OriginTag::InBox) + { + // only one parent contains all the inbox actions and that parent contains only inbox actions, + // so if we found an inbox action we know this is the parent we are looking for + foundParent = parent; + break; + } + } + + if (foundParent) + { + for (const auto& [_, cmd] : foundParent->_ActionMap) + { + inboxActions.emplace(Hash(cmd.ActionAndArgs()), cmd); + } + } + + std::unordered_map keysToReassign; + + // now, look through our _ActionMap for commands that + // - had an ID generated for them + // - do not have a name/icon path + // - have a hash that matches a command in the inbox actions + std::erase_if(_ActionMap, [&](const auto& pair) { + const auto userCmdImpl{ get_self(pair.second) }; + if (userCmdImpl->IDWasGenerated() && !userCmdImpl->HasName() && userCmdImpl->IconPath().empty()) + { + const auto userActionHash = Hash(userCmdImpl->ActionAndArgs()); + if (const auto inboxCmd = inboxActions.find(userActionHash); inboxCmd != inboxActions.end()) + { + for (const auto& [key, cmdID] : _KeyMap) + { + // for any of our keys that point to the user action, point them to the inbox action instead + if (cmdID == pair.first) + { + keysToReassign.insert_or_assign(key, inboxCmd->second.ID()); + + // register the keys with the inbox action + inboxCmd->second.RegisterKey(key); + } + } + + // remove this pair + return true; + } + } + return false; + }); - // masking actions cannot contain nested or invalid commands, - // so we can just return it directly. - return maskingPair->second; + for (const auto [key, cmdID] : keysToReassign) + { + _KeyMap.insert_or_assign(key, cmdID); } + } + bool ActionMap::FixupsAppliedDuringLoad() const + { + return _fixupsAppliedDuringLoad; + } + + // Method Description: + // - Retrieves the Command referred to be the given ID + // - Will recurse through parents if we don't find it in this layer + // Arguments: + // - actionID: the internal ID associated with a Command + // Return Value: + // - The command if it exists in this layer, otherwise nullptr + Model::Command ActionMap::_GetActionByID(const winrt::hstring& actionID) const + { // Check current layer const auto actionMapPair{ _ActionMap.find(actionID) }; if (actionMapPair != _ActionMap.end()) @@ -93,13 +149,19 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // ActionMap should never point to nullptr FAIL_FAST_IF_NULL(cmd); - return !cmd.HasNestedCommands() && cmd.ActionAndArgs().Action() == ShortcutAction::Invalid ? - nullptr : // explicitly unbound - cmd; + return cmd; + } + + for (const auto& parent : _parents) + { + if (const auto inheritedCmd = parent->_GetActionByID(actionID)) + { + return inheritedCmd; + } } // We don't have an answer - return std::nullopt; + return nullptr; } static void RegisterShortcutAction(ShortcutAction shortcutAction, std::unordered_map& list, std::unordered_set& visited) @@ -142,24 +204,22 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation void ActionMap::_PopulateAvailableActionsWithStandardCommands(std::unordered_map& availableActions, std::unordered_set& visitedActionIDs) const { // Update AvailableActions and visitedActionIDs with our current layer - for (const auto& [actionID, cmd] : _ActionMap) + for (const auto& [_, cmd] : _ActionMap) { - if (cmd.ActionAndArgs().Action() != ShortcutAction::Invalid) + // Only populate AvailableActions with actions that haven't been visited already. + const auto actionID = Hash(cmd.ActionAndArgs()); + if (!visitedActionIDs.contains(actionID)) { - // Only populate AvailableActions with actions that haven't been visited already. - if (visitedActionIDs.find(actionID) == visitedActionIDs.end()) + const auto name{ cmd.Name() }; + if (!name.empty()) { - const auto& name{ cmd.Name() }; - if (!name.empty()) - { - // Update AvailableActions. - const auto actionAndArgsImpl{ get_self(cmd.ActionAndArgs()) }; - availableActions.insert_or_assign(name, *actionAndArgsImpl->Copy()); - } - - // Record that we already handled adding this action to the NameMap. - visitedActionIDs.insert(actionID); + // Update AvailableActions. + const auto actionAndArgsImpl{ get_self(cmd.ActionAndArgs()) }; + availableActions.insert_or_assign(name, *actionAndArgsImpl->Copy()); } + + // Record that we already handled adding this action to the NameMap. + visitedActionIDs.insert(actionID); } } @@ -178,6 +238,10 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { if (!_NameMapCache) { + if (!_CumulativeActionMapCache) + { + _RefreshKeyBindingCaches(); + } // populate _NameMapCache std::unordered_map nameMap{}; _PopulateNameMapWithSpecialCommands(nameMap); @@ -230,59 +294,66 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // - Populates the provided nameMap with all of our actions and our parents actions // while omitting the actions that were already added before // Arguments: - // - nameMap: the nameMap we're populating. This maps the name (hstring) of a command to the command itself. - // There should only ever by one of each command (identified by the actionID) in the nameMap. + // - nameMap: the nameMap we're populating, this maps the name (hstring) of a command to the command itself void ActionMap::_PopulateNameMapWithStandardCommands(std::unordered_map& nameMap) const { - std::unordered_set visitedActionIDs; - for (const auto& cmd : _GetCumulativeActions()) + for (const auto& [_, cmd] : _CumulativeActionMapCache) { - // only populate with valid commands - if (cmd.ActionAndArgs().Action() != ShortcutAction::Invalid) + const auto& name{ cmd.Name() }; + if (!name.empty()) { - // Only populate NameMap with actions that haven't been visited already. - const auto actionID{ Hash(cmd.ActionAndArgs()) }; - if (visitedActionIDs.find(actionID) == visitedActionIDs.end()) + // there might be a collision here, where there could be 2 different commands with the same name + // in this case, prioritize the user's action + // TODO GH #17166: we should no longer use Command.Name to identify commands anywhere + if (!nameMap.contains(name) || cmd.Origin() == OriginTag::User) { - const auto& name{ cmd.Name() }; - if (!name.empty()) - { - // Update NameMap. - nameMap.insert_or_assign(name, cmd); - } - - // Record that we already handled adding this action to the NameMap. - visitedActionIDs.emplace(actionID); + // either a command with this name does not exist, or this is a user-defined command with a name + // in either case, update the name map with the command (if this is a user-defined command with + // the same name as an existing command, the existing one will get overwritten) + nameMap.insert_or_assign(name, cmd); } } } } // Method Description: - // - Provides an accumulated list of actions that are exposed. The accumulated list includes actions added in this layer, followed by actions added by our parents. - std::vector ActionMap::_GetCumulativeActions() const noexcept + // - Recursively populate keyBindingsMap with ours and our parents' key -> id pairs + // - This is a bottom-up approach + // - Keybindings of the parents are overridden by the children + void ActionMap::_PopulateCumulativeKeyMap(std::unordered_map& keyBindingsMap) { - // First, add actions from our current layer - std::vector cumulativeActions; - cumulativeActions.reserve(_MaskingActions.size() + _ActionMap.size()); - - // masking actions have priority. Actions here are constructed from consolidating an inherited action with changes we've found when populating this layer. - std::transform(_MaskingActions.begin(), _MaskingActions.end(), std::back_inserter(cumulativeActions), [](std::pair actionPair) { - return actionPair.second; - }); - std::transform(_ActionMap.begin(), _ActionMap.end(), std::back_inserter(cumulativeActions), [](std::pair actionPair) { - return actionPair.second; - }); + for (const auto& [keys, cmdID] : _KeyMap) + { + if (!keyBindingsMap.contains(keys)) + { + keyBindingsMap.emplace(keys, cmdID); + } + } - // Now, add the accumulated actions from our parents for (const auto& parent : _parents) { - const auto parentActions{ parent->_GetCumulativeActions() }; - cumulativeActions.reserve(cumulativeActions.size() + parentActions.size()); - cumulativeActions.insert(cumulativeActions.end(), parentActions.begin(), parentActions.end()); + parent->_PopulateCumulativeKeyMap(keyBindingsMap); + } + } + + // Method Description: + // - Recursively populate actionMap with ours and our parents' id -> command pairs + // - This is a bottom-up approach + // - Actions of the parents are overridden by the children + void ActionMap::_PopulateCumulativeActionMap(std::unordered_map& actionMap) + { + for (const auto& [cmdID, cmd] : _ActionMap) + { + if (!actionMap.contains(cmdID)) + { + actionMap.emplace(cmdID, cmd); + } } - return cumulativeActions; + for (const auto& parent : _parents) + { + parent->_PopulateCumulativeActionMap(actionMap); + } } IMapView ActionMap::GlobalHotkeys() @@ -296,81 +367,42 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation IMapView ActionMap::KeyBindings() { - if (!_KeyBindingMapCache) + if (!_ResolvedKeyActionMapCache) { _RefreshKeyBindingCaches(); } - return _KeyBindingMapCache.GetView(); + return _ResolvedKeyActionMapCache.GetView(); } void ActionMap::_RefreshKeyBindingCaches() { - std::unordered_map keyBindingsMap; std::unordered_map globalHotkeys; - std::unordered_set unboundKeys; + std::unordered_map accumulatedKeybindingsMap; + std::unordered_map accumulatedActionsMap; + std::unordered_map resolvedKeyActionMap; - _PopulateKeyBindingMapWithStandardCommands(keyBindingsMap, unboundKeys); + _PopulateCumulativeKeyMap(accumulatedKeybindingsMap); + _PopulateCumulativeActionMap(accumulatedActionsMap); - for (const auto& [keys, cmd] : keyBindingsMap) + for (const auto& [keys, cmdID] : accumulatedKeybindingsMap) { - // Only populate GlobalHotkeys with actions whose - // ShortcutAction is GlobalSummon or QuakeMode - if (cmd.ActionAndArgs().Action() == ShortcutAction::GlobalSummon || cmd.ActionAndArgs().Action() == ShortcutAction::QuakeMode) + if (const auto idCmdPair = accumulatedActionsMap.find(cmdID); idCmdPair != accumulatedActionsMap.end()) { - globalHotkeys.emplace(keys, cmd); - } - } - - _KeyBindingMapCache = single_threaded_map(std::move(keyBindingsMap)); - _GlobalHotkeysCache = single_threaded_map(std::move(globalHotkeys)); - } + resolvedKeyActionMap.emplace(keys, idCmdPair->second); - // Method Description: - // - Populates the provided keyBindingsMap with all of our actions and our parents actions - // while omitting the key bindings that were already added before. - // - This needs to be a bottom up approach to ensure that we only add each key chord once. - // Arguments: - // - keyBindingsMap: the keyBindingsMap we're populating. This maps the key chord of a command to the command itself. - // - unboundKeys: a set of keys that are explicitly unbound - void ActionMap::_PopulateKeyBindingMapWithStandardCommands(std::unordered_map& keyBindingsMap, std::unordered_set& unboundKeys) const - { - // Update KeyBindingsMap with our current layer - for (const auto& [keys, actionID] : _KeyMap) - { - // Get the action our KeyMap maps to. - // This _cannot_ be nullopt because KeyMap can only map to - // actions in this layer. - // This _can_ be nullptr because nullptr means it was - // explicitly unbound ( "command": "unbound", "keys": "ctrl+c" ). - const auto cmd{ _GetActionByID(actionID).value() }; - if (cmd) - { - // iterate over all of the action's bound keys - const auto cmdImpl{ get_self(cmd) }; - for (const auto& keys : cmdImpl->KeyMappings()) + // Only populate GlobalHotkeys with actions whose + // ShortcutAction is GlobalSummon or QuakeMode + if (idCmdPair->second.ActionAndArgs().Action() == ShortcutAction::GlobalSummon || idCmdPair->second.ActionAndArgs().Action() == ShortcutAction::QuakeMode) { - // Only populate KeyBindingsMap with actions that... - // (1) haven't been visited already - // (2) aren't explicitly unbound - if (keyBindingsMap.find(keys) == keyBindingsMap.end() && unboundKeys.find(keys) == unboundKeys.end()) - { - keyBindingsMap.emplace(keys, cmd); - } + globalHotkeys.emplace(keys, idCmdPair->second); } } - else - { - // record any keys that are explicitly unbound, - // but don't add them to the list of key bindings - unboundKeys.emplace(keys); - } } - // Update keyBindingsMap and unboundKeys with our parents - for (const auto& parent : _parents) - { - parent->_PopulateKeyBindingMapWithStandardCommands(keyBindingsMap, unboundKeys); - } + _CumulativeKeyMapCache = single_threaded_map(std::move(accumulatedKeybindingsMap)); + _CumulativeActionMapCache = single_threaded_map(std::move(accumulatedActionsMap)); + _ResolvedKeyActionMapCache = single_threaded_map(std::move(resolvedKeyActionMap)); + _GlobalHotkeysCache = single_threaded_map(std::move(globalHotkeys)); } com_ptr ActionMap::Copy() const @@ -387,13 +419,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation actionMap->_ActionMap.emplace(actionID, *winrt::get_self(cmd)->Copy()); } - // ID --> Command - actionMap->_MaskingActions.reserve(_MaskingActions.size()); - for (const auto& [actionID, cmd] : _MaskingActions) - { - actionMap->_MaskingActions.emplace(actionID, *winrt::get_self(cmd)->Copy()); - } - // Name --> Command actionMap->_NestedCommands.reserve(_NestedCommands.size()); for (const auto& [name, cmd] : _NestedCommands) @@ -431,7 +456,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // invalidate caches _NameMapCache = nullptr; _GlobalHotkeysCache = nullptr; - _KeyBindingMapCache = nullptr; + _CumulativeKeyMapCache = nullptr; + _CumulativeActionMapCache = nullptr; + _ResolvedKeyActionMapCache = nullptr; // Handle nested commands const auto cmdImpl{ get_self(cmd) }; @@ -454,156 +481,73 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } // 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); + // Add the new command to the _ActionMap + // Add the new keybinding to the _KeyMap - _TryUpdateName(cmd, oldCmd, maskingCmd); - _TryUpdateKeyChord(cmd, oldCmd, maskingCmd); + _TryUpdateActionMap(cmd); + _TryUpdateKeyChord(cmd); } // 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. + // - Try to add the new command to _ActionMap // 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) + void ActionMap::_TryUpdateActionMap(const Model::Command& cmd) { - // 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()) + // if the shortcut action is invalid, then this is for unbinding and _TryUpdateKeyChord will handle that + if (cmd.ActionAndArgs().Action() != ShortcutAction::Invalid) { - // 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. - 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 && *inheritedCmd) - { - const auto& inheritedCmdImpl{ get_self(*inheritedCmd) }; - 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()) + const auto cmdImpl{ get_self(cmd) }; + if (cmd.Origin() == OriginTag::User && cmd.ID().empty()) { - // The new name differs from the old name, - // update our name. - auto oldCmdImpl{ get_self(oldCmd) }; - oldCmdImpl->Name(newName); + // the user did not define an ID for their non-nested, non-iterable, valid command - generate one for them + cmdImpl->GenerateID(); } - } - // 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()) + // only add to the _ActionMap if there is an ID + if (auto cmdID = cmd.ID(); !cmdID.empty()) { - // The new name differs from the old name, - // update our name. - auto maskingCmdImpl{ get_self(maskingCmd) }; - maskingCmdImpl->Name(newName); + // in the legacy scenario, a user might have several of the same action but only one of them has defined an icon or a name + // eg. { "command": "paste", "name": "myPaste", "keys":"ctrl+a" } + // { "command": "paste", "keys": "ctrl+b" } + // once they port over to the new implementation, we will reduce it to just one Command object with a generated ID + // but several key binding entries, like so + // { "command": "newTab", "id": "User.paste" } -> in the actions map + // { "keys": "ctrl+a", "id": "User.paste" } -> in the keybindings map + // { "keys": "ctrl+b", "id": "User.paste" } -> in the keybindings map + // however, we have to make sure that we preserve the icon/name that might have been there in one of the command objects + // to do that, we check if this command we're adding had an ID that was generated + // if so, we check if there already exists a command with that generated ID, and if there is we port over any name/icon there might be + // (this may cause us to overwrite in scenarios where the user has an existing command that has the same generated ID but + // performs a different action or has different args, but that falls under "play stupid games") + if (cmdImpl->IDWasGenerated()) + { + if (const auto foundCmd{ _GetActionByID(cmdID) }) + { + const auto foundCmdImpl{ get_self(foundCmd) }; + if (foundCmdImpl->HasName() && !cmdImpl->HasName()) + { + cmdImpl->Name(foundCmdImpl->Name()); + } + if (!foundCmdImpl->IconPath().empty() && cmdImpl->IconPath().empty()) + { + cmdImpl->IconPath(foundCmdImpl->IconPath()); + } + } + } + _ActionMap.insert_or_assign(cmdID, cmd); } } - - // Handle a collision with NestedCommands - _NestedCommands.erase(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) + void ActionMap::_TryUpdateKeyChord(const Model::Command& cmd) { - // Example: - // { "command": "copy", "keys": "ctrl+c" } --> we are registering a new key chord, update oldCmd and maskingCmd + // Example (this is a legacy case, where the keys are provided in the same block as the command): + // { "command": "copy", "keys": "ctrl+c" } --> we are registering a new key chord // { "name": "foo", "command": "copy" } --> no change to keys, exit early const auto keys{ cmd.Keys() }; if (!keys) @@ -613,77 +557,24 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } // 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) + if (const auto foundCommand = _GetActionByKeyChordInternal(keys); foundCommand && *foundCommand) { - // Update inner Command with new key chord - auto oldCmdImpl{ get_self(oldCmd) }; - oldCmdImpl->RegisterKey(keys); - } + // collision: the key chord is bound to some command, make sure that command erases + // this key chord as we are about to overwrite it - // 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); + const auto foundCommandImpl{ get_self(*foundCommand) }; + foundCommandImpl->EraseKey(keys); } + + // Assign the new action in the _KeyMap + // However, there's a strange edge case here - since we're parsing a legacy or modern block, + // the user might have { "command": null, "id": "someID", "keys": "ctrl+c" } + // i.e. they provided an ID for a null command (which they really shouldn't, there's no purpose) + // in this case, we do _not_ want to use the id they provided, we want to use an empty id + // (empty id in the _KeyMap indicates the keychord was explicitly unbound) + const auto action = cmd.ActionAndArgs().Action(); + const auto id = action == ShortcutAction::Invalid ? hstring{} : cmd.ID(); + _KeyMap.insert_or_assign(keys, id); } // Method Description: @@ -714,106 +605,82 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } // Method Description: - // - Retrieves the assigned command with the given key chord. + // - Retrieves the assigned command ID 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(const Control::KeyChord& keys) const + // - the command ID with the given key chord + // - an empty string if the key chord is explicitly unbound + // - nullopt if it is not bound + std::optional ActionMap::_GetActionIdByKeyChordInternal(const Control::KeyChord& keys) const { - // Check the current layer - if (const auto actionIDPair = _KeyMap.find(keys); actionIDPair != _KeyMap.end()) + if (const auto keyIDPair = _KeyMap.find(keys); keyIDPair != _KeyMap.end()) { - // the command was explicitly bound, - // return what we found (invalid commands exposed as nullptr) - return _GetActionByID(actionIDPair->second); + // the keychord is defined in this layer, return the ID + return keyIDPair->second; } - // the command was not bound in this layer, - // ask my parents + // search through our parents for (const auto& parent : _parents) { - const auto& inheritedCmd{ parent->_GetActionByKeyChordInternal(keys) }; - if (inheritedCmd) + if (const auto foundCmdID = parent->_GetActionIdByKeyChordInternal(keys)) { - return *inheritedCmd; + return foundCmdID; } } - // This action is not explicitly bound + // we did not find the keychord anywhere, it's not bound and not explicitly unbound either return std::nullopt; } // Method Description: - // - Retrieves the key chord for the provided action + // - Retrieves the assigned command with the given key chord. + // - Can return nullopt to differentiate explicit unbinding vs lack of binding. // Arguments: - // - action: the shortcut action (an action type) we're looking for + // - keys: the key chord of the command to search 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(const ShortcutAction& action) const + // - the command with the given key chord + // - nullptr if the key chord is explicitly unbound + // - nullopt if it is not bound + std::optional ActionMap::_GetActionByKeyChordInternal(const Control::KeyChord& keys) const { - return GetKeyBindingForAction(action, nullptr); + if (const auto actionIDOptional = _GetActionIdByKeyChordInternal(keys)) + { + if (!actionIDOptional->empty()) + { + // there is an ID associated with these keys, find the command + if (const auto foundCmd = _GetActionByID(*actionIDOptional)) + { + return foundCmd; + } + } + // the ID is an empty string, these keys are explicitly unbound + return nullptr; + } + + 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 - // - myArgs: the action args for the action we're looking for + // - cmdID: the ID of the command 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(const ShortcutAction& myAction, const IActionArgs& myArgs) const + Control::KeyChord ActionMap::GetKeyBindingForAction(const winrt::hstring& cmdID) const { - if (myAction == ShortcutAction::Invalid) - { - return nullptr; - } - // Check our internal state. - const auto actionAndArgs = winrt::make(myAction, myArgs); - const auto hash{ Hash(actionAndArgs) }; - if (const auto& cmd{ _GetActionByID(hash) }) + if (const auto cmd{ _GetActionByID(cmdID) }) { - return cmd->Keys(); - } - - // Check our parents - for (const auto& parent : _parents) - { - if (const auto& keys{ parent->GetKeyBindingForAction(myAction, myArgs) }) - { - return keys; - } + return cmd.Keys(); } // This key binding does not exist return nullptr; } - bool ActionMap::GenerateIDsForActions() - { - bool fixedUp{ false }; - for (auto actionPair : _ActionMap) - { - auto cmdImpl{ winrt::get_self(actionPair.second) }; - - // Note: this function should ONLY be called for the action map in the user's settings file - // this debug assert should verify that for debug builds - assert(cmdImpl->Origin() == OriginTag::User); - - if (cmdImpl->ID().empty()) - { - fixedUp = cmdImpl->GenerateID() || fixedUp; - } - } - return fixedUp; - } - // Method Description: // - Rebinds a key binding to a new key chord // Arguments: @@ -823,24 +690,31 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // - true, if successful. False, otherwise. bool ActionMap::RebindKeys(const Control::KeyChord& oldKeys, const Control::KeyChord& newKeys) { - const auto& cmd{ GetActionByKeyChord(oldKeys) }; + const auto cmd{ GetActionByKeyChord(oldKeys) }; if (!cmd) { // oldKeys must be bound. Otherwise, we don't know what action to bind. return false; } - if (newKeys) + if (auto oldKeyPair = _KeyMap.find(oldKeys); oldKeyPair != _KeyMap.end()) { - // Bind newKeys - const auto newCmd{ make_self() }; - newCmd->ActionAndArgs(cmd.ActionAndArgs()); - newCmd->RegisterKey(newKeys); - AddAction(*newCmd); + // oldKeys is bound in our layer, replace it with newKeys + _KeyMap.insert_or_assign(newKeys, cmd.ID()); + _KeyMap.erase(oldKeyPair); } + else + { + // oldKeys is bound in some other layer, set newKeys to cmd in this layer, and oldKeys to unbound in this layer + _KeyMap.insert_or_assign(newKeys, cmd.ID()); + _KeyMap.insert_or_assign(oldKeys, L""); + } + + // make sure to update the Command with these changes + const auto cmdImpl{ get_self(cmd) }; + cmdImpl->EraseKey(oldKeys); + cmdImpl->RegisterKey(newKeys); - // unbind oldKeys - DeleteKeyBinding(oldKeys); return true; } @@ -852,12 +726,19 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // - void ActionMap::DeleteKeyBinding(const KeyChord& keys) { - // create an "unbound" command - // { "command": "unbound", "keys": } - const auto cmd{ make_self() }; - cmd->ActionAndArgs(make()); - cmd->RegisterKey(keys); - AddAction(*cmd); + if (auto keyPair = _KeyMap.find(keys); keyPair != _KeyMap.end()) + { + // this keychord is bound in our layer, delete it + _KeyMap.erase(keyPair); + } + + // either the keychord was never in this layer or we just deleted it above, + // if GetActionByKeyChord still returns a command that means the keychord is bound in another layer + if (GetActionByKeyChord(keys)) + { + // set to unbound in this layer + _KeyMap.emplace(keys, L""); + } } // Method Description: @@ -873,32 +754,10 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation auto cmd{ make_self() }; cmd->RegisterKey(keys); cmd->ActionAndArgs(action); + cmd->GenerateID(); AddAction(*cmd); } - void ActionMap::_recursiveUpdateCommandKeybindingLabels() - { - const auto& commands{ _ExpandedCommandsCache }; - - for (const auto& command : commands) - { - if (command.HasNestedCommands()) - { - _recursiveUpdateCommandKeybindingLabels(); - } - 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{ GetKeyBindingForAction(command.ActionAndArgs().Action(), - command.ActionAndArgs().Args()) }; - command.RegisterKey(keyChord); - } - } - } - // This is a helper to aid in sorting commands by their `Name`s, alphabetically. static bool _compareSchemeNames(const ColorScheme& lhs, const ColorScheme& rhs) { diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.h b/src/cascadia/TerminalSettingsModel/ActionMap.h index 880b9989ff6..87bd5efb585 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMap.h +++ b/src/cascadia/TerminalSettingsModel/ActionMap.h @@ -49,6 +49,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation struct ActionMap : ActionMapT, IInheritable { + void _FinalizeInheritance() override; + // views Windows::Foundation::Collections::IMapView AvailableActions(); Windows::Foundation::Collections::IMapView NameMap(); @@ -59,8 +61,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // queries Model::Command GetActionByKeyChord(const Control::KeyChord& keys) const; bool IsKeyChordExplicitlyUnbound(const Control::KeyChord& keys) const; - Control::KeyChord GetKeyBindingForAction(const ShortcutAction& action) const; - Control::KeyChord GetKeyBindingForAction(const ShortcutAction& action, const IActionArgs& actionArgs) const; + Control::KeyChord GetKeyBindingForAction(const winrt::hstring& cmdID) const; // population void AddAction(const Model::Command& cmd); @@ -69,9 +70,10 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation static com_ptr FromJson(const Json::Value& json, const OriginTag origin = OriginTag::None); std::vector LayerJson(const Json::Value& json, const OriginTag origin, const bool withKeybindings = true); Json::Value ToJson() const; + Json::Value KeyBindingsToJson() const; + bool FixupsAppliedDuringLoad() const; // modification - bool GenerateIDsForActions(); bool RebindKeys(const Control::KeyChord& oldKeys, const Control::KeyChord& newKeys); void DeleteKeyBinding(const Control::KeyChord& keys); void RegisterKeyBinding(Control::KeyChord keys, Model::ActionAndArgs action); @@ -83,46 +85,49 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation winrt::Windows::Foundation::Collections::IVector FilterToSendInput(winrt::hstring currentCommandline); private: - std::optional _GetActionByID(const InternalActionID actionID) const; + Model::Command _GetActionByID(const winrt::hstring& actionID) const; + std::optional _GetActionIdByKeyChordInternal(const Control::KeyChord& keys) const; std::optional _GetActionByKeyChordInternal(const Control::KeyChord& keys) const; void _RefreshKeyBindingCaches(); void _PopulateAvailableActionsWithStandardCommands(std::unordered_map& availableActions, std::unordered_set& visitedActionIDs) const; void _PopulateNameMapWithSpecialCommands(std::unordered_map& nameMap) const; void _PopulateNameMapWithStandardCommands(std::unordered_map& nameMap) const; - void _PopulateKeyBindingMapWithStandardCommands(std::unordered_map& keyBindingsMap, std::unordered_set& unboundKeys) 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); + void _PopulateCumulativeKeyMap(std::unordered_map& keyBindingsMap); + void _PopulateCumulativeActionMap(std::unordered_map& actionMap); - void _recursiveUpdateCommandKeybindingLabels(); + void _TryUpdateActionMap(const Model::Command& cmd); + void _TryUpdateKeyChord(const Model::Command& cmd); Windows::Foundation::Collections::IMap _AvailableActionsCache{ nullptr }; Windows::Foundation::Collections::IMap _NameMapCache{ nullptr }; Windows::Foundation::Collections::IMap _GlobalHotkeysCache{ nullptr }; - Windows::Foundation::Collections::IMap _KeyBindingMapCache{ nullptr }; Windows::Foundation::Collections::IVector _ExpandedCommandsCache{ nullptr }; std::unordered_map _NestedCommands; std::vector _IterableCommands; - 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; + + bool _fixupsAppliedDuringLoad{ false }; + + void _AddKeyBindingHelper(const Json::Value& json, std::vector& warnings); + + // _KeyMap is the map of key chords -> action IDs defined in this layer + // _ActionMap is the map of action IDs -> commands defined in this layer + // These maps are the ones that we deserialize into when parsing the user json and vice-versa + std::unordered_map _KeyMap; + std::unordered_map _ActionMap; + + // _CumulativeKeyMapCache is the map of key chords -> action IDs defined in all layers, with child layers overriding parent layers + Windows::Foundation::Collections::IMap _CumulativeKeyMapCache{ nullptr }; + // _CumulativeActionMapCache is the map of action IDs -> commands defined in all layers, with child layers overriding parent layers + Windows::Foundation::Collections::IMap _CumulativeActionMapCache{ nullptr }; + + // _ResolvedKeyActionMapCache is the map of key chords -> commands defined in all layers, with child layers overriding parent layers + // This is effectively a combination of _CumulativeKeyMapCache and _CumulativeActionMapCache and its purpose is so that + // we can give the SUI a view of the key chords and the commands they map to + Windows::Foundation::Collections::IMap _ResolvedKeyActionMapCache{ nullptr }; friend class SettingsModelUnitTests::KeyBindingsTests; friend class SettingsModelUnitTests::DeserializationTests; diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.idl b/src/cascadia/TerminalSettingsModel/ActionMap.idl index b84305b9d0e..105f00715f5 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMap.idl +++ b/src/cascadia/TerminalSettingsModel/ActionMap.idl @@ -12,8 +12,7 @@ namespace Microsoft.Terminal.Settings.Model 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); + Microsoft.Terminal.Control.KeyChord GetKeyBindingForAction(String cmdID); Windows.Foundation.Collections.IMapView AvailableActions { get; }; diff --git a/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp b/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp index c03fecd82f5..e81c9dbd050 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp @@ -27,12 +27,12 @@ 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. + // - Deserialize an ActionMap from the array `json` + // - The json array either contains an array of serialized `Command` objects, + // or an array of keybindings + // - The actions are added to _ActionMap and the keybindings are added to _KeyMap // Arguments: - // - json: an array of Json::Value's to deserialize into our ActionMap. + // - json: an array of Json::Value's to deserialize into our _ActionMap and _KeyMap // Return value: // - a list of warnings encountered while deserializing the json std::vector ActionMap::LayerJson(const Json::Value& json, const OriginTag origin, const bool withKeybindings) @@ -43,14 +43,49 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // settings phase, so we'll collect them now. std::vector warnings; - for (const auto& cmdJson : json) + for (const auto& jsonBlock : json) { - if (!cmdJson.isObject()) + if (!jsonBlock.isObject()) { continue; } - AddAction(*Command::FromJson(cmdJson, warnings, origin, withKeybindings)); + // the json block may be 1 of 3 things: + // - the legacy style command block, that has the action, args and keys in it + // - the modern style command block, that has the action, args and an ID + // - the modern style keys block, that has the keys and an ID + + // if the block contains a "command" field, it is either a legacy or modern style command block + // and we can call Command::FromJson on it (Command::FromJson can handle parsing both legacy or modern) + + // if there is no "command" field, then it is a modern style keys block + if (jsonBlock.isMember(JsonKey(CommandsKey)) || jsonBlock.isMember(JsonKey(ActionKey))) + { + AddAction(*Command::FromJson(jsonBlock, warnings, origin, withKeybindings)); + + // for non-nested non-iterable commands, + // check if this is a legacy-style command block so we can inform the loader that fixups are needed + if (jsonBlock.isMember(JsonKey(ActionKey)) && !jsonBlock.isMember(JsonKey(IterateOnKey))) + { + if (jsonBlock.isMember(JsonKey(KeysKey))) + { + // there are keys in this command block - it's the legacy style + _fixupsAppliedDuringLoad = true; + } + + if (origin == OriginTag::User && !jsonBlock.isMember(JsonKey(IDKey))) + { + // there's no ID in this command block - we will generate one for the user + // inform the loader that the ID needs to be written into the json + _fixupsAppliedDuringLoad = true; + } + } + } + else + { + // this is not a command block, so it is a keybinding block + _AddKeyBindingHelper(jsonBlock, warnings); + } } return warnings; @@ -60,23 +95,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { Json::Value actionList{ Json::ValueType::arrayValue }; - // Command serializes to an array of JSON objects. - // This is because a Command may have multiple key chords associated with it. - // The name and icon are only serialized in the first object. - // Example: - // { "name": "Custom Copy", "command": "copy", "keys": "ctrl+c" } - // { "command": "copy", "keys": "ctrl+shift+c" } - // { "command": "copy", "keys": "ctrl+ins" } auto toJson = [&actionList](const Model::Command& cmd) { const auto cmdImpl{ winrt::get_self(cmd) }; - const auto& cmdJsonArray{ cmdImpl->ToJson() }; - for (const auto& cmdJson : cmdJsonArray) - { - actionList.append(cmdJson); - } + const auto& cmdJson{ cmdImpl->ToJson() }; + actionList.append(cmdJson); }; - // Serialize all standard Command objects in the current layer for (const auto& [_, cmd] : _ActionMap) { toJson(cmd); @@ -96,4 +120,70 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation return actionList; } + + Json::Value ActionMap::KeyBindingsToJson() const + { + Json::Value keybindingsList{ Json::ValueType::arrayValue }; + + // Serialize all standard keybinding objects in the current layer + for (const auto& [keys, cmdID] : _KeyMap) + { + Json::Value keyIDPair{ Json::ValueType::objectValue }; + JsonUtils::SetValueForKey(keyIDPair, KeysKey, keys); + JsonUtils::SetValueForKey(keyIDPair, IDKey, cmdID); + keybindingsList.append(keyIDPair); + } + + return keybindingsList; + } + + void ActionMap::_AddKeyBindingHelper(const Json::Value& json, std::vector& warnings) + { + // There should always be a "keys" field + // - If there is also an "id" field - we add the pair to our _KeyMap + // - If there is no "id" field - this is an explicit unbinding, still add it to the _KeyMap, + // when this key chord is queried for we will know it is an explicit unbinding + const auto keysJson{ json[JsonKey(KeysKey)] }; + if (keysJson.isArray() && keysJson.size() > 1) + { + warnings.push_back(SettingsLoadWarnings::TooManyKeysForChord); + return; + } + + Control::KeyChord keys{ nullptr }; + winrt::hstring idJson; + if (!JsonUtils::GetValueForKey(json, KeysKey, keys)) + { + return; + } + + // if these keys are already bound to some command, + // we need to update that command to erase these keys as we are about to overwrite them + if (const auto foundCommand = _GetActionByKeyChordInternal(keys); foundCommand && *foundCommand) + { + const auto foundCommandImpl{ get_self(*foundCommand) }; + foundCommandImpl->EraseKey(keys); + } + + // if the "id" field doesn't exist in the json, then idJson will be an empty string which is fine + JsonUtils::GetValueForKey(json, IDKey, idJson); + + // any existing keybinding with the same keychord in this layer will get overwritten + _KeyMap.insert_or_assign(keys, idJson); + + // make sure the command registers these keys + if (!idJson.empty()) + { + // TODO GH#17160 + // if the command with this id is only going to appear later during settings load + // then this will return null, meaning that the command created later on will not register this keybinding + // the keybinding will still work fine within the app, its just that the Command object itself won't know about this key mapping + // we are going to move away from Command needing to know its key mappings in a followup, so this shouldn't matter for very long + if (const auto cmd = _GetActionByID(idJson)) + { + cmd.RegisterKey(keys); + } + } + return; + } } diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp index 1deaa9eb50e..e9acef452c5 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp @@ -461,6 +461,7 @@ bool SettingsLoader::FixupUserSettings() }; auto fixedUp = userSettings.fixupsAppliedDuringLoad; + fixedUp = userSettings.globals->FixupsAppliedDuringLoad() || fixedUp; fixedUp = RemapColorSchemeForProfile(userSettings.baseLayerProfile) || fixedUp; for (const auto& profile : userSettings.profiles) @@ -504,10 +505,6 @@ bool SettingsLoader::FixupUserSettings() fixedUp = true; } - // we need to generate an ID for a command in the user settings if it doesn't already have one - auto actionMap{ winrt::get_self(userSettings.globals->ActionMap()) }; - actionMap->GenerateIDsForActions(); - return fixedUp; } diff --git a/src/cascadia/TerminalSettingsModel/Command.cpp b/src/cascadia/TerminalSettingsModel/Command.cpp index a32ed4efa79..7d786b9008d 100644 --- a/src/cascadia/TerminalSettingsModel/Command.cpp +++ b/src/cascadia/TerminalSettingsModel/Command.cpp @@ -20,14 +20,6 @@ namespace winrt namespace WUX = Windows::UI::Xaml; } -static constexpr std::string_view NameKey{ "name" }; -static constexpr std::string_view IDKey{ "id" }; -static constexpr std::string_view IconKey{ "icon" }; -static constexpr std::string_view ActionKey{ "command" }; -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}" }; static constexpr std::string_view SchemeNameToken{ "${scheme.name}" }; @@ -121,7 +113,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation return hstring{ _ID }; } - bool Command::GenerateID() + void Command::GenerateID() { if (_ActionAndArgs) { @@ -130,10 +122,13 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { _ID = generatedID; _IDWasGenerated = true; - return true; } } - return false; + } + + bool Command::IDWasGenerated() + { + return _IDWasGenerated; } void Command::Name(const hstring& value) @@ -423,14 +418,14 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } // Function Description: - // - Serialize the Command into an array of json actions + // - Serialize the Command into a json value // Arguments: // - // Return Value: - // - an array of serialized actions + // - a serialized command Json::Value Command::ToJson() const { - Json::Value cmdList{ Json::ValueType::arrayValue }; + Json::Value cmdJson{ Json::ValueType::objectValue }; if (_nestedCommand || _IterateOn != ExpandCommandType::None) { @@ -438,15 +433,13 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // For these, we can trust _originalJson to be correct. // In fact, we _need_ to use it here because we don't actually deserialize `iterateOn` // until we expand the command. - cmdList.append(_originalJson); + cmdJson = _originalJson; } - else if (_keyMappings.empty()) + else { - // only write out one command - Json::Value cmdJson{ Json::ValueType::objectValue }; JsonUtils::SetValueForKey(cmdJson, IconKey, _iconPath); JsonUtils::SetValueForKey(cmdJson, NameKey, _name); - if (!_ID.empty() && !_IDWasGenerated) + if (!_ID.empty()) { JsonUtils::SetValueForKey(cmdJson, IDKey, _ID); } @@ -455,38 +448,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { cmdJson[JsonKey(ActionKey)] = ActionAndArgs::ToJson(_ActionAndArgs); } - - cmdList.append(cmdJson); - } - else - { - // we'll write out one command per key mapping - for (auto keys{ _keyMappings.begin() }; keys != _keyMappings.end(); ++keys) - { - Json::Value cmdJson{ Json::ValueType::objectValue }; - - if (keys == _keyMappings.begin()) - { - // First iteration also writes icon and name - JsonUtils::SetValueForKey(cmdJson, IconKey, _iconPath); - JsonUtils::SetValueForKey(cmdJson, NameKey, _name); - if (!_ID.empty()) - { - JsonUtils::SetValueForKey(cmdJson, IDKey, _ID); - } - } - - if (_ActionAndArgs) - { - cmdJson[JsonKey(ActionKey)] = ActionAndArgs::ToJson(_ActionAndArgs); - } - - JsonUtils::SetValueForKey(cmdJson, KeysKey, *keys); - cmdList.append(cmdJson); - } } - return cmdList; + return cmdJson; } // Function Description: diff --git a/src/cascadia/TerminalSettingsModel/Command.h b/src/cascadia/TerminalSettingsModel/Command.h index a6821f55f35..f2d29daa3f9 100644 --- a/src/cascadia/TerminalSettingsModel/Command.h +++ b/src/cascadia/TerminalSettingsModel/Command.h @@ -31,6 +31,14 @@ namespace SettingsModelUnitTests class CommandTests; }; +static constexpr std::string_view NameKey{ "name" }; +static constexpr std::string_view IDKey{ "id" }; +static constexpr std::string_view IconKey{ "icon" }; +static constexpr std::string_view ActionKey{ "command" }; +static constexpr std::string_view IterateOnKey{ "iterateOn" }; +static constexpr std::string_view CommandsKey{ "commands" }; +static constexpr std::string_view KeysKey{ "keys" }; + namespace winrt::Microsoft::Terminal::Settings::Model::implementation { struct Command : CommandT @@ -62,7 +70,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation void Name(const hstring& name); hstring ID() const noexcept; - bool GenerateID(); + void GenerateID(); + bool IDWasGenerated(); Control::KeyChord Keys() const noexcept; hstring KeyChordText() const noexcept; diff --git a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp index ff6dd5eec63..7151444ce7c 100644 --- a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp +++ b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp @@ -17,7 +17,7 @@ using namespace winrt::Windows::UI::Xaml; using namespace ::Microsoft::Console; using namespace winrt::Microsoft::UI::Xaml::Controls; -static constexpr std::string_view LegacyKeybindingsKey{ "keybindings" }; +static constexpr std::string_view KeybindingsKey{ "keybindings" }; static constexpr std::string_view ActionsKey{ "actions" }; static constexpr std::string_view ThemeKey{ "theme" }; static constexpr std::string_view DefaultProfileKey{ "defaultProfile" }; @@ -45,6 +45,7 @@ void GlobalAppSettings::_FinalizeInheritance() } } } + _actionMap->_FinalizeInheritance(); } winrt::com_ptr GlobalAppSettings::Copy() const @@ -155,7 +156,9 @@ void GlobalAppSettings::LayerJson(const Json::Value& json, const OriginTag origi void GlobalAppSettings::LayerActionsFrom(const Json::Value& json, const OriginTag origin, const bool withKeybindings) { - static constexpr std::array bindingsKeys{ LegacyKeybindingsKey, ActionsKey }; + // we want to do the keybindings map after the actions map so that we overwrite any leftover keybindings + // that might have existed in the first pass, in case the user did a partial update from legacy to modern + static constexpr std::array bindingsKeys{ ActionsKey, KeybindingsKey }; for (const auto& jsonKey : bindingsKeys) { if (auto bindings{ json[JsonKey(jsonKey)] }) @@ -259,9 +262,16 @@ Json::Value GlobalAppSettings::ToJson() #undef GLOBAL_SETTINGS_TO_JSON json[JsonKey(ActionsKey)] = _actionMap->ToJson(); + json[JsonKey(KeybindingsKey)] = _actionMap->KeyBindingsToJson(); + return json; } +bool GlobalAppSettings::FixupsAppliedDuringLoad() +{ + return _actionMap->FixupsAppliedDuringLoad(); +} + winrt::Microsoft::Terminal::Settings::Model::Theme GlobalAppSettings::CurrentTheme() noexcept { auto requestedTheme = Model::Theme::IsSystemInDarkTheme() ? diff --git a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h index c977094df53..7b55b7007b5 100644 --- a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h +++ b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h @@ -53,6 +53,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation void LayerActionsFrom(const Json::Value& json, const OriginTag origin, const bool withKeybindings = true); Json::Value ToJson(); + bool FixupsAppliedDuringLoad(); const std::vector& KeybindingsWarnings() const; diff --git a/src/cascadia/TerminalSettingsModel/defaults.json b/src/cascadia/TerminalSettingsModel/defaults.json index 468c2120acf..1a3e7c6c111 100644 --- a/src/cascadia/TerminalSettingsModel/defaults.json +++ b/src/cascadia/TerminalSettingsModel/defaults.json @@ -422,28 +422,27 @@ ], "actions": [ - // Application-level Keys - { "command": "closeWindow", "keys": "alt+f4", "id": "Terminal.CloseWindow" }, - { "command": "toggleFullscreen", "keys": "alt+enter", "id": "Terminal.ToggleFullscreen" }, - { "command": "toggleFullscreen", "keys": "f11", "id": "Terminal.ToggleFullscreen" }, + // Application-level Commands + { "command": "closeWindow", "id": "Terminal.CloseWindow" }, + { "command": "toggleFullscreen", "id": "Terminal.ToggleFullscreen" }, { "command": "toggleFocusMode", "id": "Terminal.ToggleFocusMode" }, { "command": "toggleAlwaysOnTop", "id": "Terminal.ToggleAlwaysOnTop" }, - { "command": "openNewTabDropdown", "keys": "ctrl+shift+space", "id": "Terminal.OpenNewTabDropdown" }, - { "command": { "action": "openSettings", "target": "settingsUI" }, "keys": "ctrl+,", "id": "Terminal.OpenSettingsUI" }, - { "command": { "action": "openSettings", "target": "settingsFile" }, "keys": "ctrl+shift+,", "id": "Terminal.OpenSettingsFile" }, - { "command": { "action": "openSettings", "target": "defaultsFile" }, "keys": "ctrl+alt+,", "id": "Terminal.OpenDefaultSettingsFile" }, - { "command": "find", "keys": "ctrl+shift+f", "id": "Terminal.FindText" }, + { "command": "openNewTabDropdown", "id": "Terminal.OpenNewTabDropdown" }, + { "command": { "action": "openSettings", "target": "settingsUI" }, "id": "Terminal.OpenSettingsUI" }, + { "command": { "action": "openSettings", "target": "settingsFile" }, "id": "Terminal.OpenSettingsFile" }, + { "command": { "action": "openSettings", "target": "defaultsFile" }, "id": "Terminal.OpenDefaultSettingsFile" }, + { "command": "find", "id": "Terminal.FindText" }, { "command": { "action": "findMatch", "direction": "next" }, "id": "Terminal.FindNextMatch" }, { "command": { "action": "findMatch", "direction": "prev" }, "id": "Terminal.FindPrevMatch" }, { "command": "toggleShaderEffects", "id": "Terminal.ToggleShaderEffects" }, { "command": "openTabColorPicker", "id": "Terminal.OpenTabColorPicker" }, { "command": "renameTab", "id": "Terminal.RenameTab" }, { "command": "openTabRenamer", "id": "Terminal.OpenTabRenamer" }, - { "command": "commandPalette", "keys":"ctrl+shift+p", "id": "Terminal.ToggleCommandPalette" }, + { "command": "commandPalette", "id": "Terminal.ToggleCommandPalette" }, { "command": "identifyWindow", "id": "Terminal.IdentifyWindow" }, { "command": "openWindowRenamer", "id": "Terminal.OpenWindowRenamer" }, - { "command": "quakeMode", "keys":"win+sc(41)", "id": "Terminal.QuakeMode" }, - { "command": "openSystemMenu", "keys": "alt+space", "id": "Terminal.OpenSystemMenu" }, + { "command": "quakeMode", "id": "Terminal.QuakeMode" }, + { "command": "openSystemMenu", "id": "Terminal.OpenSystemMenu" }, { "command": "quit", "id": "Terminal.Quit" }, { "command": "restoreLastClosed", "id": "Terminal.RestoreLastClosed" }, { "command": "openAbout", "id": "Terminal.OpenAboutDialog" }, @@ -455,49 +454,50 @@ { "command": "closeTabsAfter", "id": "Terminal.CloseTabsAfter" }, { "command": { "action" : "moveTab", "direction": "forward" }, "id": "Terminal.MoveTabForward" }, { "command": { "action" : "moveTab", "direction": "backward" }, "id": "Terminal.MoveTabBackward" }, - { "command": "newTab", "keys": "ctrl+shift+t", "id": "Terminal.OpenNewTab" }, - { "command": "newWindow", "keys": "ctrl+shift+n", "id": "Terminal.OpenNewWindow" }, - { "command": { "action": "newTab", "index": 0 }, "keys": "ctrl+shift+1", "id": "Terminal.OpenNewTabProfile0" }, - { "command": { "action": "newTab", "index": 1 }, "keys": "ctrl+shift+2", "id": "Terminal.OpenNewTabProfile1" }, - { "command": { "action": "newTab", "index": 2 }, "keys": "ctrl+shift+3", "id": "Terminal.OpenNewTabProfile2" }, - { "command": { "action": "newTab", "index": 3 }, "keys": "ctrl+shift+4", "id": "Terminal.OpenNewTabProfile3" }, - { "command": { "action": "newTab", "index": 4 }, "keys": "ctrl+shift+5", "id": "Terminal.OpenNewTabProfile4" }, - { "command": { "action": "newTab", "index": 5 }, "keys": "ctrl+shift+6", "id": "Terminal.OpenNewTabProfile5" }, - { "command": { "action": "newTab", "index": 6 }, "keys": "ctrl+shift+7", "id": "Terminal.OpenNewTabProfile6" }, - { "command": { "action": "newTab", "index": 7 }, "keys": "ctrl+shift+8", "id": "Terminal.OpenNewTabProfile7" }, - { "command": { "action": "newTab", "index": 8 }, "keys": "ctrl+shift+9", "id": "Terminal.OpenNewTabProfile8" }, - { "command": "duplicateTab", "keys": "ctrl+shift+d", "id": "Terminal.DuplicateTab" }, - { "command": "nextTab", "keys": "ctrl+tab", "id": "Terminal.NextTab" }, - { "command": "prevTab", "keys": "ctrl+shift+tab", "id": "Terminal.PrevTab" }, - { "command": { "action": "switchToTab", "index": 0 }, "keys": "ctrl+alt+1", "id": "Terminal.SwitchToTab0" }, - { "command": { "action": "switchToTab", "index": 1 }, "keys": "ctrl+alt+2", "id": "Terminal.SwitchToTab1" }, - { "command": { "action": "switchToTab", "index": 2 }, "keys": "ctrl+alt+3", "id": "Terminal.SwitchToTab2" }, - { "command": { "action": "switchToTab", "index": 3 }, "keys": "ctrl+alt+4", "id": "Terminal.SwitchToTab3" }, - { "command": { "action": "switchToTab", "index": 4 }, "keys": "ctrl+alt+5", "id": "Terminal.SwitchToTab4" }, - { "command": { "action": "switchToTab", "index": 5 }, "keys": "ctrl+alt+6", "id": "Terminal.SwitchToTab5" }, - { "command": { "action": "switchToTab", "index": 6 }, "keys": "ctrl+alt+7", "id": "Terminal.SwitchToTab6" }, - { "command": { "action": "switchToTab", "index": 7 }, "keys": "ctrl+alt+8", "id": "Terminal.SwitchToTab7" }, - { "command": { "action": "switchToTab", "index": 4294967295 }, "keys": "ctrl+alt+9", "id": "Terminal.SwitchToLastTab" }, + { "command": "newTab", "id": "Terminal.OpenNewTab" }, + { "command": "newWindow", "id": "Terminal.OpenNewWindow" }, + { "command": { "action": "newTab", "index": 0 }, "id": "Terminal.OpenNewTabProfile0" }, + { "command": { "action": "newTab", "index": 1 }, "id": "Terminal.OpenNewTabProfile1" }, + { "command": { "action": "newTab", "index": 2 }, "id": "Terminal.OpenNewTabProfile2" }, + { "command": { "action": "newTab", "index": 3 }, "id": "Terminal.OpenNewTabProfile3" }, + { "command": { "action": "newTab", "index": 4 }, "id": "Terminal.OpenNewTabProfile4" }, + { "command": { "action": "newTab", "index": 5 }, "id": "Terminal.OpenNewTabProfile5" }, + { "command": { "action": "newTab", "index": 6 }, "id": "Terminal.OpenNewTabProfile6" }, + { "command": { "action": "newTab", "index": 7 }, "id": "Terminal.OpenNewTabProfile7" }, + { "command": { "action": "newTab", "index": 8 }, "id": "Terminal.OpenNewTabProfile8" }, + { "command": "duplicateTab", "id": "Terminal.DuplicateTab" }, + { "command": "nextTab", "id": "Terminal.NextTab" }, + { "command": "prevTab", "id": "Terminal.PrevTab" }, + { "command": { "action": "switchToTab", "index": 0 }, "id": "Terminal.SwitchToTab0" }, + { "command": { "action": "switchToTab", "index": 1 }, "id": "Terminal.SwitchToTab1" }, + { "command": { "action": "switchToTab", "index": 2 }, "id": "Terminal.SwitchToTab2" }, + { "command": { "action": "switchToTab", "index": 3 }, "id": "Terminal.SwitchToTab3" }, + { "command": { "action": "switchToTab", "index": 4 }, "id": "Terminal.SwitchToTab4" }, + { "command": { "action": "switchToTab", "index": 5 }, "id": "Terminal.SwitchToTab5" }, + { "command": { "action": "switchToTab", "index": 6 }, "id": "Terminal.SwitchToTab6" }, + { "command": { "action": "switchToTab", "index": 7 }, "id": "Terminal.SwitchToTab7" }, + { "command": { "action": "switchToTab", "index": 4294967295 }, "id": "Terminal.SwitchToLastTab" }, { "command": { "action": "moveTab", "window": "new" }, "id": "Terminal.MoveTabToNewWindow" }, // Pane Management { "command": "closeOtherPanes", "id": "Terminal.CloseOtherPanes" }, - { "command": "closePane", "keys": "ctrl+shift+w", "id": "Terminal.ClosePane" }, + { "command": "closePane", "id": "Terminal.ClosePane" }, { "command": { "action": "splitPane", "split": "up" }, "id": "Terminal.SplitPaneUp" }, { "command": { "action": "splitPane", "split": "down" }, "id": "Terminal.SplitPaneDown" }, { "command": { "action": "splitPane", "split": "left" }, "id": "Terminal.SplitPaneLeft" }, { "command": { "action": "splitPane", "split": "right" }, "id": "Terminal.SplitPaneRight" }, - { "command": { "action": "splitPane", "splitMode": "duplicate", "split": "down" }, "keys": "alt+shift+-", "id": "Terminal.SplitPaneDuplicateDown" }, - { "command": { "action": "splitPane", "splitMode": "duplicate", "split": "right" }, "keys": "alt+shift+plus", "id": "Terminal.SplitPaneDuplicateRight" }, - { "command": { "action": "resizePane", "direction": "down" }, "keys": "alt+shift+down", "id": "Terminal.ResizePaneDown" }, - { "command": { "action": "resizePane", "direction": "left" }, "keys": "alt+shift+left", "id": "Terminal.ResizePaneLeft" }, - { "command": { "action": "resizePane", "direction": "right" }, "keys": "alt+shift+right", "id": "Terminal.ResizePaneRight" }, - { "command": { "action": "resizePane", "direction": "up" }, "keys": "alt+shift+up", "id": "Terminal.ResizePaneUp" }, - { "command": { "action": "moveFocus", "direction": "down" }, "keys": "alt+down", "id": "Terminal.MoveFocusDown" }, - { "command": { "action": "moveFocus", "direction": "left" }, "keys": "alt+left", "id": "Terminal.MoveFocusLeft" }, - { "command": { "action": "moveFocus", "direction": "right" }, "keys": "alt+right", "id": "Terminal.MoveFocusRight" }, - { "command": { "action": "moveFocus", "direction": "up" }, "keys": "alt+up", "id": "Terminal.MoveFocusUp" }, - { "command": { "action": "moveFocus", "direction": "previous" }, "keys": "ctrl+alt+left", "id": "Terminal.MoveFocusPrevious" }, + { "command": { "action": "splitPane", "splitMode": "duplicate", "split": "down" }, "id": "Terminal.DuplicatePaneDown" }, + { "command": { "action": "splitPane", "splitMode": "duplicate", "split": "right" }, "id": "Terminal.DuplicatePaneRight" }, + { "command": { "action": "splitPane", "splitMode": "duplicate", "split": "auto" }, "id": "Terminal.DuplicatePaneAuto" }, + { "command": { "action": "resizePane", "direction": "down" }, "id": "Terminal.ResizePaneDown" }, + { "command": { "action": "resizePane", "direction": "left" }, "id": "Terminal.ResizePaneLeft" }, + { "command": { "action": "resizePane", "direction": "right" }, "id": "Terminal.ResizePaneRight" }, + { "command": { "action": "resizePane", "direction": "up" }, "id": "Terminal.ResizePaneUp" }, + { "command": { "action": "moveFocus", "direction": "down" }, "id": "Terminal.MoveFocusDown" }, + { "command": { "action": "moveFocus", "direction": "left" }, "id": "Terminal.MoveFocusLeft" }, + { "command": { "action": "moveFocus", "direction": "right" }, "id": "Terminal.MoveFocusRight" }, + { "command": { "action": "moveFocus", "direction": "up" }, "id": "Terminal.MoveFocusUp" }, + { "command": { "action": "moveFocus", "direction": "previous" }, "id": "Terminal.MoveFocusPrevious" }, { "command": { "action": "moveFocus", "direction": "previousInOrder" }, "id": "Terminal.MoveFocusPreviousInOrder" }, { "command": { "action": "moveFocus", "direction": "nextInOrder" }, "id": "Terminal.MoveFocusNextInOrder" }, { "command": { "action": "moveFocus", "direction": "first" }, "id": "Terminal.MoveFocusFirst" }, @@ -530,38 +530,35 @@ { "command": "restartConnection", "id": "Terminal.RestartConnection" }, // Clipboard Integration - { "command": { "action": "copy", "singleLine": false }, "keys": "ctrl+shift+c", "id": "Terminal.CopySelectedText" }, - { "command": { "action": "copy", "singleLine": false }, "keys": "ctrl+insert", "id": "Terminal.CopySelectedText" }, - { "command": { "action": "copy", "singleLine": false }, "keys": "enter", "id": "Terminal.CopySelectedText" }, - { "command": "paste", "keys": "ctrl+shift+v", "id": "Terminal.PasteFromClipboard" }, - { "command": "paste", "keys": "shift+insert", "id": "Terminal.PasteFromClipboard" }, - { "command": "selectAll", "keys": "ctrl+shift+a", "id": "Terminal.SelectAll" }, - { "command": "markMode", "keys": "ctrl+shift+m", "id": "Terminal.ToggleMarkMode" }, + { "command": { "action": "copy", "singleLine": false }, "id": "Terminal.CopyToClipboard" }, + { "command": "paste", "id": "Terminal.PasteFromClipboard" }, + { "command": "selectAll", "id": "Terminal.SelectAll" }, + { "command": "markMode", "id": "Terminal.ToggleMarkMode" }, { "command": "toggleBlockSelection", "id": "Terminal.ToggleBlockSelection" }, { "command": "switchSelectionEndpoint", "id": "Terminal.SwitchSelectionEndpoint" }, { "command": "expandSelectionToWord", "id": "Terminal.ExpandSelectionToWord" }, - { "command": "showContextMenu", "keys": "menu", "id": "Terminal.ShowContextMenu" }, + { "command": "showContextMenu", "id": "Terminal.ShowContextMenu" }, // Web Search { "command": { "action": "searchWeb" }, "name": { "key": "SearchWebCommandKey" }, "id": "Terminal.SearchWeb" }, // Scrollback - { "command": "scrollDown", "keys": "ctrl+shift+down", "id": "Terminal.ScrollDown" }, - { "command": "scrollDownPage", "keys": "ctrl+shift+pgdn", "id": "Terminal.ScrollDownPage" }, - { "command": "scrollUp", "keys": "ctrl+shift+up", "id": "Terminal.ScrollUp" }, - { "command": "scrollUpPage", "keys": "ctrl+shift+pgup", "id": "Terminal.ScrollUpPage" }, - { "command": "scrollToTop", "keys": "ctrl+shift+home", "id": "Terminal.ScrollToTop" }, - { "command": "scrollToBottom", "keys": "ctrl+shift+end", "id": "Terminal.ScrollToBottom" }, + { "command": "scrollDown", "id": "Terminal.ScrollDown" }, + { "command": "scrollDownPage", "id": "Terminal.ScrollDownPage" }, + { "command": "scrollUp", "id": "Terminal.ScrollUp" }, + { "command": "scrollUpPage", "id": "Terminal.ScrollUpPage" }, + { "command": "scrollToTop", "id": "Terminal.ScrollToTop" }, + { "command": "scrollToBottom", "id": "Terminal.ScrollToBottom" }, { "command": { "action": "clearBuffer", "clear": "all" }, "id": "Terminal.ClearBuffer" }, { "command": "exportBuffer", "id": "Terminal.ExportBuffer" }, // Visual Adjustments - { "command": { "action": "adjustFontSize", "delta": 1 }, "keys": "ctrl+plus", "id": "Terminal.IncreaseFontSize" }, - { "command": { "action": "adjustFontSize", "delta": -1 }, "keys": "ctrl+minus", "id": "Terminal.DecreaseFontSize" }, - { "command": { "action": "adjustFontSize", "delta": 1 }, "keys": "ctrl+numpad_plus", "id": "Terminal.IncreaseFontSize" }, - { "command": { "action": "adjustFontSize", "delta": -1 }, "keys": "ctrl+numpad_minus", "id": "Terminal.DecreaseFontSize" }, - { "command": "resetFontSize", "keys": "ctrl+0", "id": "Terminal.ResetFontSize" }, - { "command": "resetFontSize", "keys": "ctrl+numpad_0", "id": "Terminal.ResetFontSize" }, + { "command": { "action": "adjustFontSize", "delta": 1 }, "id": "Terminal.IncreaseFontSize" }, + { "command": { "action": "adjustFontSize", "delta": -1 }, "id": "Terminal.DecreaseFontSize" }, + { "command": { "action": "adjustFontSize", "delta": 1 }, "id": "Terminal.IncreaseFontSize" }, + { "command": { "action": "adjustFontSize", "delta": -1 }, "id": "Terminal.DecreaseFontSize" }, + { "command": "resetFontSize", "id": "Terminal.ResetFontSize" }, + { "command": "resetFontSize", "id": "Terminal.ResetFontSize" }, // Other commands { @@ -626,5 +623,86 @@ { "command": { "action": "adjustOpacity", "opacity": 100, "relative": false } } ] } + ], + "keybindings": [ + // Application-level Keys + { "keys": "alt+f4", "id": "Terminal.CloseWindow" }, + { "keys": "alt+enter", "id": "Terminal.ToggleFullscreen" }, + { "keys": "f11", "id": "Terminal.ToggleFullscreen" }, + { "keys": "ctrl+shift+space", "id": "Terminal.OpenNewTabDropdown" }, + { "keys": "ctrl+,", "id": "Terminal.OpenSettingsUI" }, + { "keys": "ctrl+shift+,", "id": "Terminal.OpenSettingsFile" }, + { "keys": "ctrl+alt+,", "id": "Terminal.OpenDefaultSettingsFile" }, + { "keys": "ctrl+shift+f", "id": "Terminal.FindText" }, + { "keys":"ctrl+shift+p", "id": "Terminal.ToggleCommandPalette" }, + { "keys":"win+sc(41)", "id": "Terminal.QuakeMode" }, + { "keys": "alt+space", "id": "Terminal.OpenSystemMenu" }, + + // Tab Management + // "command": "closeTab" is unbound by default. + // The closeTab command closes a tab without confirmation, even if it has multiple panes. + { "keys": "ctrl+shift+t", "id": "Terminal.OpenNewTab" }, + { "keys": "ctrl+shift+n", "id": "Terminal.OpenNewWindow" }, + { "keys": "ctrl+shift+1", "id": "Terminal.OpenNewTabProfile0" }, + { "keys": "ctrl+shift+2", "id": "Terminal.OpenNewTabProfile1" }, + { "keys": "ctrl+shift+3", "id": "Terminal.OpenNewTabProfile2" }, + { "keys": "ctrl+shift+4", "id": "Terminal.OpenNewTabProfile3" }, + { "keys": "ctrl+shift+5", "id": "Terminal.OpenNewTabProfile4" }, + { "keys": "ctrl+shift+6", "id": "Terminal.OpenNewTabProfile5" }, + { "keys": "ctrl+shift+7", "id": "Terminal.OpenNewTabProfile6" }, + { "keys": "ctrl+shift+8", "id": "Terminal.OpenNewTabProfile7" }, + { "keys": "ctrl+shift+9", "id": "Terminal.OpenNewTabProfile8" }, + { "keys": "ctrl+shift+d", "id": "Terminal.DuplicateTab" }, + { "keys": "ctrl+tab", "id": "Terminal.NextTab" }, + { "keys": "ctrl+shift+tab", "id": "Terminal.PrevTab" }, + { "keys": "ctrl+alt+1", "id": "Terminal.SwitchToTab0" }, + { "keys": "ctrl+alt+2", "id": "Terminal.SwitchToTab1" }, + { "keys": "ctrl+alt+3", "id": "Terminal.SwitchToTab2" }, + { "keys": "ctrl+alt+4", "id": "Terminal.SwitchToTab3" }, + { "keys": "ctrl+alt+5", "id": "Terminal.SwitchToTab4" }, + { "keys": "ctrl+alt+6", "id": "Terminal.SwitchToTab5" }, + { "keys": "ctrl+alt+7", "id": "Terminal.SwitchToTab6" }, + { "keys": "ctrl+alt+8", "id": "Terminal.SwitchToTab7" }, + { "keys": "ctrl+alt+9", "id": "Terminal.SwitchToLastTab" }, + + // Pane Management + { "keys": "ctrl+shift+w", "id": "Terminal.ClosePane" }, + { "keys": "alt+shift+-", "id": "Terminal.DuplicatePaneDown" }, + { "keys": "alt+shift+plus", "id": "Terminal.DuplicatePaneRight" }, + { "keys": "alt+shift+down", "id": "Terminal.ResizePaneDown" }, + { "keys": "alt+shift+left", "id": "Terminal.ResizePaneLeft" }, + { "keys": "alt+shift+right", "id": "Terminal.ResizePaneRight" }, + { "keys": "alt+shift+up", "id": "Terminal.ResizePaneUp" }, + { "keys": "alt+down", "id": "Terminal.MoveFocusDown" }, + { "keys": "alt+left", "id": "Terminal.MoveFocusLeft" }, + { "keys": "alt+right", "id": "Terminal.MoveFocusRight" }, + { "keys": "alt+up", "id": "Terminal.MoveFocusUp" }, + { "keys": "ctrl+alt+left", "id": "Terminal.MoveFocusPrevious" }, + + // Clipboard Integration + { "keys": "ctrl+shift+c", "id": "Terminal.CopyToClipboard" }, + { "keys": "ctrl+insert", "id": "Terminal.CopyToClipboard" }, + { "keys": "enter", "id": "Terminal.CopyToClipboard" }, + { "keys": "ctrl+shift+v", "id": "Terminal.PasteFromClipboard" }, + { "keys": "shift+insert", "id": "Terminal.PasteFromClipboard" }, + { "keys": "ctrl+shift+a", "id": "Terminal.SelectAll" }, + { "keys": "ctrl+shift+m", "id": "Terminal.ToggleMarkMode" }, + { "keys": "menu", "id": "Terminal.ShowContextMenu" }, + + // Scrollback + { "keys": "ctrl+shift+down", "id": "Terminal.ScrollDown" }, + { "keys": "ctrl+shift+pgdn", "id": "Terminal.ScrollDownPage" }, + { "keys": "ctrl+shift+up", "id": "Terminal.ScrollUp" }, + { "keys": "ctrl+shift+pgup", "id": "Terminal.ScrollUpPage" }, + { "keys": "ctrl+shift+home", "id": "Terminal.ScrollToTop" }, + { "keys": "ctrl+shift+end", "id": "Terminal.ScrollToBottom" }, + + // Visual Adjustments + { "keys": "ctrl+plus", "id": "Terminal.IncreaseFontSize" }, + { "keys": "ctrl+minus", "id": "Terminal.DecreaseFontSize" }, + { "keys": "ctrl+numpad_plus", "id": "Terminal.IncreaseFontSize" }, + { "keys": "ctrl+numpad_minus", "id": "Terminal.DecreaseFontSize" }, + { "keys": "ctrl+0", "id": "Terminal.ResetFontSize" }, + { "keys": "ctrl+numpad_0", "id": "Terminal.ResetFontSize" }, ] } diff --git a/src/cascadia/TerminalSettingsModel/userDefaults.json b/src/cascadia/TerminalSettingsModel/userDefaults.json index 59fa13a6df0..b6b4582f2a9 100644 --- a/src/cascadia/TerminalSettingsModel/userDefaults.json +++ b/src/cascadia/TerminalSettingsModel/userDefaults.json @@ -21,11 +21,10 @@ } ] }, - "actions": + "keybindings": [ - { "command": {"action": "copy", "singleLine": false }, "keys": "ctrl+c" }, - { "command": "paste", "keys": "ctrl+v" }, - { "command": "find", "keys": "ctrl+shift+f" }, - { "command": { "action": "splitPane", "split": "auto", "splitMode": "duplicate" }, "keys": "alt+shift+d" } + { "id": "Terminal.CopySelectedText", "keys": "ctrl+c" }, + { "id": "Terminal.PasteFromClipboard", "keys": "ctrl+v" }, + { "id": "Terminal.DuplicatePaneAuto", "keys": "alt+shift+d" } ] } diff --git a/src/cascadia/UnitTests_SettingsModel/DeserializationTests.cpp b/src/cascadia/UnitTests_SettingsModel/DeserializationTests.cpp index 50462a72346..aabd6fb06f2 100644 --- a/src/cascadia/UnitTests_SettingsModel/DeserializationTests.cpp +++ b/src/cascadia/UnitTests_SettingsModel/DeserializationTests.cpp @@ -58,6 +58,7 @@ namespace SettingsModelUnitTests TEST_METHOD(TestCloneInheritanceTree); TEST_METHOD(TestValidDefaults); TEST_METHOD(TestInheritedCommand); + TEST_METHOD(TestOverwriteParentCommandAndKeybinding); TEST_METHOD(LoadFragmentsWithMultipleUpdates); TEST_METHOD(FragmentActionSimple); @@ -1235,11 +1236,11 @@ namespace SettingsModelUnitTests const auto settings = createSettings(badSettings); // KeyMap: ctrl+a/b are mapped to "invalid" - // ActionMap: "splitPane" and "invalid" are the only deserialized actions + // ActionMap: "splitPane" is the only deserialized action // NameMap: "splitPane" has no key binding, but it is still added to the name map const auto actionMap = winrt::get_self(settings->GlobalSettings().ActionMap()); VERIFY_ARE_EQUAL(2u, actionMap->_KeyMap.size()); - VERIFY_ARE_EQUAL(2u, actionMap->_ActionMap.size()); + VERIFY_ARE_EQUAL(1u, actionMap->_ActionMap.size()); VERIFY_ARE_EQUAL(1u, actionMap->NameMap().Size()); VERIFY_ARE_EQUAL(5u, settings->Warnings().Size()); @@ -1981,7 +1982,8 @@ namespace SettingsModelUnitTests }, { "name": "bar", - "command": "closePane" + "command": "closePane", + "id": "Test.ClosePane" }, ], })" }; @@ -2005,11 +2007,108 @@ namespace SettingsModelUnitTests } { // Verify ActionMap::GetKeyBindingForAction API - const auto& actualKeyChord{ settings->ActionMap().GetKeyBindingForAction(ShortcutAction::ClosePane) }; + const auto& actualKeyChord{ settings->ActionMap().GetKeyBindingForAction(L"Test.ClosePane") }; VERIFY_IS_NULL(actualKeyChord); } } + void DeserializationTests::TestOverwriteParentCommandAndKeybinding() + { + // Tests: + // - Redefine an action whose ID was originally defined in another layer + // - Redefine a keychord that exists in another layer + // - Define a keychord that points to an action in another layer + + static constexpr std::string_view settings1Json{ R"( + { + "defaultProfile": "{6239a42c-0000-49a3-80bd-e8fdd045185c}", + "profiles": [ + { + "name": "profile0", + "guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}", + "historySize": 1, + "commandline": "cmd.exe" + } + ], + "actions": [ + { + "command": "closePane", + "id": "Parent.ClosePane" + }, + { + "command": "closePane", + "id": "Parent.ClosePane2" + } + ], + "keybindings": [ + { + "keys": "ctrl+shift+w", + "id": "Parent.ClosePane" + }, + { + "keys": "ctrl+shift+x", + "id": "Parent.ClosePane2" + } + ] + })" }; + + // this child actions and keybindings list + // - redefines Parent.ClosePane to perform a newTab action instead of a closePane action + // - redefines ctrl+shift+x to point to Child.ClosePane instead of Parent.ClosePane2 + // - defines ctrl+shift+y to point to Parent.ClosePane2 (an action that does not exist in this child layer) + static constexpr std::string_view settings2Json{ R"( + { + "defaultProfile": "{6239a42c-0000-49a3-80bd-e8fdd045185c}", + "actions": [ + { + "command": "newTab", + "id": "Parent.ClosePane" + }, + { + "command": "closePane", + "id": "Child.ClosePane" + } + ], + "keybindings": [ + { + "id": "Child.ClosePane", + "keys": "ctrl+shift+x" + }, + { + "id": "Parent.ClosePane2", + "keys": "ctrl+shift+y" + } + ] + })" }; + + const auto settings = winrt::make_self(settings2Json, settings1Json); + const KeyChord ctrlShiftW{ true, false, true, false, static_cast('W'), 0 }; + const KeyChord ctrlShiftX{ true, false, true, false, static_cast('X'), 0 }; + const KeyChord ctrlShiftY{ true, false, true, false, static_cast('Y'), 0 }; + + { + // ctrl+shift+w should point to Parent.ClosePane, however Parent.ClosePane should be a newTab action + const auto& cmd{ settings->ActionMap().GetActionByKeyChord(ctrlShiftW) }; + VERIFY_IS_NOT_NULL(cmd); + VERIFY_ARE_EQUAL(cmd.ID(), L"Parent.ClosePane"); + VERIFY_ARE_EQUAL(cmd.ActionAndArgs().Action(), ShortcutAction::NewTab); + } + { + // ctrl+shift+x should point to Child.ClosePane + const auto& cmd{ settings->ActionMap().GetActionByKeyChord(ctrlShiftX) }; + VERIFY_IS_NOT_NULL(cmd); + VERIFY_ARE_EQUAL(cmd.ID(), L"Child.ClosePane"); + VERIFY_ARE_EQUAL(cmd.ActionAndArgs().Action(), ShortcutAction::ClosePane); + } + { + // ctrl+shift+y should point to Parent.ClosePane2 + const auto& cmd{ settings->ActionMap().GetActionByKeyChord(ctrlShiftY) }; + VERIFY_IS_NOT_NULL(cmd); + VERIFY_ARE_EQUAL(cmd.ID(), L"Parent.ClosePane2"); + VERIFY_ARE_EQUAL(cmd.ActionAndArgs().Action(), ShortcutAction::ClosePane); + } + } + // This test ensures GH#11597, GH#12520 don't regress. void DeserializationTests::LoadFragmentsWithMultipleUpdates() { @@ -2049,7 +2148,8 @@ namespace SettingsModelUnitTests "actions": [ { "command": { "action": "addMark" }, - "name": "Test Action" + "name": "Test Action", + "id": "Test.FragmentAction" }, ] })" }; @@ -2074,6 +2174,7 @@ namespace SettingsModelUnitTests { "command": { "action": "addMark" }, "keys": "ctrl+f", + "id": "Test.FragmentAction", "name": "Test Action" }, ] @@ -2195,7 +2296,8 @@ namespace SettingsModelUnitTests "actions": [ { "command": { "action": "addMark" }, - "name": "Test Action" + "name": "Test Action", + "id": "Test.FragmentAction" }, ] })" }; diff --git a/src/cascadia/UnitTests_SettingsModel/KeyBindingsTests.cpp b/src/cascadia/UnitTests_SettingsModel/KeyBindingsTests.cpp index afe22c8930c..771fa87ed8e 100644 --- a/src/cascadia/UnitTests_SettingsModel/KeyBindingsTests.cpp +++ b/src/cascadia/UnitTests_SettingsModel/KeyBindingsTests.cpp @@ -157,17 +157,17 @@ namespace SettingsModelUnitTests void KeyBindingsTests::HashDeduplication() { const auto actionMap = winrt::make_self(); - actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": "splitPane", "keys": ["ctrl+c"] } ])"), OriginTag::None); - actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": "splitPane", "keys": ["ctrl+c"] } ])"), OriginTag::None); + actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": "splitPane", "keys": ["ctrl+c"] } ])"), OriginTag::User); + actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": "splitPane", "keys": ["ctrl+c"] } ])"), OriginTag::User); VERIFY_ARE_EQUAL(1u, actionMap->_ActionMap.size()); } void KeyBindingsTests::HashContentArgs() { - Log::Comment(L"These are two actions with different content args. They should have different hashes for their terminal args."); + Log::Comment(L"These are two actions with different content args. They should have different generated IDs for their terminal args."); const auto actionMap = winrt::make_self(); - actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": { "action": "newTab", } , "keys": ["ctrl+c"] } ])"), OriginTag::None); - actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": { "action": "newTab", "index": 0 } , "keys": ["ctrl+shift+c"] } ])"), OriginTag::None); + actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": { "action": "newTab", } , "keys": ["ctrl+c"] } ])"), OriginTag::User); + actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": { "action": "newTab", "index": 0 } , "keys": ["ctrl+shift+c"] } ])"), OriginTag::User); VERIFY_ARE_EQUAL(2u, actionMap->_ActionMap.size()); KeyChord ctrlC{ VirtualKeyModifiers::Control, static_cast('C'), 0 }; @@ -271,32 +271,32 @@ namespace SettingsModelUnitTests auto actionMap = winrt::make_self(); VERIFY_IS_FALSE(actionMap->IsKeyChordExplicitlyUnbound(keyChord)); - actionMap->LayerJson(bindings0Json, OriginTag::None); + actionMap->LayerJson(bindings0Json, OriginTag::User); VERIFY_IS_FALSE(actionMap->IsKeyChordExplicitlyUnbound(keyChord)); - actionMap->LayerJson(bindings1Json, OriginTag::None); + actionMap->LayerJson(bindings1Json, OriginTag::User); VERIFY_IS_TRUE(actionMap->IsKeyChordExplicitlyUnbound(keyChord)); - actionMap->LayerJson(bindings2Json, OriginTag::None); + actionMap->LayerJson(bindings2Json, OriginTag::User); VERIFY_IS_FALSE(actionMap->IsKeyChordExplicitlyUnbound(keyChord)); } void KeyBindingsTests::TestArbitraryArgs() { const std::string bindings0String{ R"([ - { "command": "copy", "keys": ["ctrl+c"] }, - { "command": { "action": "copy", "singleLine": false }, "keys": ["ctrl+shift+c"] }, - { "command": { "action": "copy", "singleLine": true }, "keys": ["alt+shift+c"] }, + { "command": "copy", "id": "Test.CopyNoArgs", "keys": ["ctrl+c"] }, + { "command": { "action": "copy", "singleLine": false }, "id": "Test.CopyMultiline", "keys": ["ctrl+shift+c"] }, + { "command": { "action": "copy", "singleLine": true }, "id": "Test.CopySingleline", "keys": ["alt+shift+c"] }, - { "command": "newTab", "keys": ["ctrl+t"] }, - { "command": { "action": "newTab", "index": 0 }, "keys": ["ctrl+shift+t"] }, - { "command": { "action": "newTab", "index": 11 }, "keys": ["ctrl+shift+y"] }, + { "command": "newTab", "id": "Test.NewTabNoArgs", "keys": ["ctrl+t"] }, + { "command": { "action": "newTab", "index": 0 }, "id": "Test.NewTab0", "keys": ["ctrl+shift+t"] }, + { "command": { "action": "newTab", "index": 11 }, "id": "Test.NewTab11", "keys": ["ctrl+shift+y"] }, - { "command": { "action": "copy", "madeUpBool": true }, "keys": ["ctrl+b"] }, - { "command": { "action": "copy" }, "keys": ["ctrl+shift+b"] }, + { "command": { "action": "copy", "madeUpBool": true }, "id": "Test.CopyFakeArgs", "keys": ["ctrl+b"] }, + { "command": { "action": "copy" }, "id": "Test.CopyNullArgs", "keys": ["ctrl+shift+b"] }, - { "command": { "action": "adjustFontSize", "delta": 1 }, "keys": ["ctrl+f"] }, - { "command": { "action": "adjustFontSize", "delta": -1 }, "keys": ["ctrl+g"] } + { "command": { "action": "adjustFontSize", "delta": 1 }, "id": "Test.EnlargeFont", "keys": ["ctrl+f"] }, + { "command": { "action": "adjustFontSize", "delta": -1 }, "id": "Test.ReduceFont", "keys": ["ctrl+g"] } ])" }; @@ -428,10 +428,10 @@ namespace SettingsModelUnitTests void KeyBindingsTests::TestSplitPaneArgs() { const std::string bindings0String{ R"([ - { "keys": ["ctrl+d"], "command": { "action": "splitPane", "split": "vertical" } }, - { "keys": ["ctrl+e"], "command": { "action": "splitPane", "split": "horizontal" } }, - { "keys": ["ctrl+g"], "command": { "action": "splitPane" } }, - { "keys": ["ctrl+h"], "command": { "action": "splitPane", "split": "auto" } } + { "keys": ["ctrl+d"], "id": "Test.SplitPaneVertical", "command": { "action": "splitPane", "split": "vertical" } }, + { "keys": ["ctrl+e"], "id": "Test.SplitPaneHorizontal", "command": { "action": "splitPane", "split": "horizontal" } }, + { "keys": ["ctrl+g"], "id": "Test.SplitPane", "command": { "action": "splitPane" } }, + { "keys": ["ctrl+h"], "id": "Test.SplitPaneAuto", "command": { "action": "splitPane", "split": "auto" } } ])" }; const auto bindings0Json = VerifyParseSucceeded(bindings0String); @@ -478,9 +478,9 @@ namespace SettingsModelUnitTests void KeyBindingsTests::TestSetTabColorArgs() { const std::string bindings0String{ R"([ - { "keys": ["ctrl+c"], "command": { "action": "setTabColor", "color": null } }, - { "keys": ["ctrl+d"], "command": { "action": "setTabColor", "color": "#123456" } }, - { "keys": ["ctrl+f"], "command": "setTabColor" }, + { "keys": ["ctrl+c"], "id": "Test.SetTabColorNull", "command": { "action": "setTabColor", "color": null } }, + { "keys": ["ctrl+d"], "id": "Test.SetTabColor", "command": { "action": "setTabColor", "color": "#123456" } }, + { "keys": ["ctrl+f"], "id": "Test.SetTabColorNoArgs", "command": "setTabColor" }, ])" }; const auto bindings0Json = VerifyParseSucceeded(bindings0String); @@ -521,7 +521,7 @@ namespace SettingsModelUnitTests void KeyBindingsTests::TestStringOverload() { const std::string bindings0String{ R"([ - { "command": "copy", "keys": "ctrl+c" } + { "command": "copy", "id": "Test.Copy", "keys": "ctrl+c" } ])" }; const auto bindings0Json = VerifyParseSucceeded(bindings0String); @@ -543,12 +543,12 @@ namespace SettingsModelUnitTests void KeyBindingsTests::TestScrollArgs() { const std::string bindings0String{ R"([ - { "keys": ["up"], "command": "scrollUp" }, - { "keys": ["down"], "command": "scrollDown" }, - { "keys": ["ctrl+up"], "command": { "action": "scrollUp" } }, - { "keys": ["ctrl+down"], "command": { "action": "scrollDown" } }, - { "keys": ["ctrl+shift+up"], "command": { "action": "scrollUp", "rowsToScroll": 10 } }, - { "keys": ["ctrl+shift+down"], "command": { "action": "scrollDown", "rowsToScroll": 10 } } + { "keys": ["up"], "id": "Test.ScrollUp0", "command": "scrollUp" }, + { "keys": ["down"], "id": "Test.ScrollDown0", "command": "scrollDown" }, + { "keys": ["ctrl+up"], "id": "Test.ScrollUp1", "command": { "action": "scrollUp" } }, + { "keys": ["ctrl+down"], "id": "Test.ScrollDown1", "command": { "action": "scrollDown" } }, + { "keys": ["ctrl+shift+up"], "id": "Test.ScrollUp2", "command": { "action": "scrollUp", "rowsToScroll": 10 } }, + { "keys": ["ctrl+shift+down"], "id": "Test.ScrollDown2", "command": { "action": "scrollDown", "rowsToScroll": 10 } } ])" }; const auto bindings0Json = VerifyParseSucceeded(bindings0String); @@ -620,8 +620,8 @@ namespace SettingsModelUnitTests void KeyBindingsTests::TestMoveTabArgs() { const std::string bindings0String{ R"([ - { "keys": ["up"], "command": { "action": "moveTab", "direction": "forward" } }, - { "keys": ["down"], "command": { "action": "moveTab", "direction": "backward" } } + { "keys": ["up"], "id": "Test.MoveTabUp", "command": { "action": "moveTab", "direction": "forward" } }, + { "keys": ["down"], "id": "Test.MoveTabDown", "command": { "action": "moveTab", "direction": "backward" } } ])" }; const auto bindings0Json = VerifyParseSucceeded(bindings0String); @@ -665,9 +665,9 @@ namespace SettingsModelUnitTests void KeyBindingsTests::TestToggleCommandPaletteArgs() { const std::string bindings0String{ R"([ - { "keys": ["up"], "command": "commandPalette" }, - { "keys": ["ctrl+up"], "command": { "action": "commandPalette", "launchMode" : "action" } }, - { "keys": ["ctrl+shift+up"], "command": { "action": "commandPalette", "launchMode" : "commandLine" } } + { "keys": ["up"], "id": "Test.CmdPal", "command": "commandPalette" }, + { "keys": ["ctrl+up"], "id": "Test.CmdPalActionMode", "command": { "action": "commandPalette", "launchMode" : "action" } }, + { "keys": ["ctrl+shift+up"], "id": "Test.CmdPalLineMode", "command": { "action": "commandPalette", "launchMode" : "commandLine" } } ])" }; const auto bindings0Json = VerifyParseSucceeded(bindings0String); @@ -712,10 +712,10 @@ namespace SettingsModelUnitTests 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 std::string bindings3String{ R"([ { "command": "commandPalette", "keys": "ctrl+shift+p" } ])" }; + const std::string bindings0String{ R"([ { "command": "closeWindow", "id": "Test.CloseWindow", "keys": "ctrl+a" } ])" }; + const std::string bindings1String{ R"([ { "command": { "action": "copy", "singleLine": true }, "id": "Test.Copy", "keys": "ctrl+b" } ])" }; + const std::string bindings2String{ R"([ { "command": { "action": "newTab", "index": 0 }, "id": "Test.NewTab", "keys": "ctrl+c" } ])" }; + const std::string bindings3String{ R"([ { "command": "commandPalette", "id": "Test.CmdPal", "keys": "ctrl+shift+p" } ])" }; const auto bindings0Json = VerifyParseSucceeded(bindings0String); const auto bindings1Json = VerifyParseSucceeded(bindings1String); @@ -742,7 +742,7 @@ namespace SettingsModelUnitTests Log::Comment(L"simple command: no args"); actionMap->LayerJson(bindings0Json, OriginTag::None); VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); - const auto& kbd{ actionMap->GetKeyBindingForAction(ShortcutAction::CloseWindow) }; + const auto& kbd{ actionMap->GetKeyBindingForAction(L"Test.CloseWindow") }; VerifyKeyChordEquality({ VirtualKeyModifiers::Control, static_cast('A'), 0 }, kbd); } { @@ -750,10 +750,7 @@ namespace SettingsModelUnitTests actionMap->LayerJson(bindings1Json, OriginTag::None); VERIFY_ARE_EQUAL(2u, actionMap->_KeyMap.size()); - auto args{ winrt::make_self() }; - args->SingleLine(true); - - const auto& kbd{ actionMap->GetKeyBindingForAction(ShortcutAction::CopyText, *args) }; + const auto& kbd{ actionMap->GetKeyBindingForAction(L"Test.Copy") }; VerifyKeyChordEquality({ VirtualKeyModifiers::Control, static_cast('B'), 0 }, kbd); } { @@ -761,11 +758,7 @@ namespace SettingsModelUnitTests actionMap->LayerJson(bindings2Json, OriginTag::None); 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) }; + const auto& kbd{ actionMap->GetKeyBindingForAction(L"Test.NewTab") }; VerifyKeyChordEquality({ VirtualKeyModifiers::Control, static_cast('C'), 0 }, kbd); } { @@ -773,7 +766,7 @@ namespace SettingsModelUnitTests actionMap->LayerJson(bindings3Json, OriginTag::None); VERIFY_ARE_EQUAL(4u, actionMap->_KeyMap.size()); - const auto& kbd{ actionMap->GetKeyBindingForAction(ShortcutAction::ToggleCommandPalette) }; + const auto& kbd{ actionMap->GetKeyBindingForAction(L"Test.CmdPal") }; VerifyKeyChordEquality({ VirtualKeyModifiers::Control | VirtualKeyModifiers::Shift, static_cast('P'), 0 }, kbd); } } @@ -807,7 +800,7 @@ namespace SettingsModelUnitTests void KeyBindingsTests::KeybindingsWithoutVkey() { - const auto json = VerifyParseSucceeded(R"!([{"command": "quakeMode", "keys":"shift+sc(255)"}])!"); + const auto json = VerifyParseSucceeded(R"!([{"command": "quakeMode", "id": "Test.NoVKey", "keys":"shift+sc(255)"}])!"); const auto actionMap = winrt::make_self(); actionMap->LayerJson(json, OriginTag::None); diff --git a/src/cascadia/UnitTests_SettingsModel/SerializationTests.cpp b/src/cascadia/UnitTests_SettingsModel/SerializationTests.cpp index 20357656638..57dc4e1f868 100644 --- a/src/cascadia/UnitTests_SettingsModel/SerializationTests.cpp +++ b/src/cascadia/UnitTests_SettingsModel/SerializationTests.cpp @@ -24,6 +24,12 @@ using namespace winrt::Microsoft::Terminal::Control; #define SEND_INPUT_ARCH_SPECIFIC_ACTION_HASH "A020D2" #endif +#if defined(_M_IX86) +#define SEND_INPUT2_ARCH_SPECIFIC_ACTION_HASH "35488AA6" +#else +#define SEND_INPUT2_ARCH_SPECIFIC_ACTION_HASH "58D1971" +#endif + namespace SettingsModelUnitTests { class SerializationTests : public JsonTestClass @@ -47,6 +53,10 @@ namespace SettingsModelUnitTests TEST_METHOD(RoundtripGenerateActionID); TEST_METHOD(NoGeneratedIDsForIterableAndNestedCommands); TEST_METHOD(GeneratedActionIDsEqualForIdenticalCommands); + TEST_METHOD(RoundtripLegacyToModernActions); + TEST_METHOD(RoundtripUserActionsSameAsInBoxAreRemoved); + TEST_METHOD(RoundtripActionsSameNameDifferentCommandsAreRetained); + TEST_METHOD(MultipleActionsAreCollapsed); private: // Method Description: @@ -120,13 +130,15 @@ namespace SettingsModelUnitTests "experimental.input.forceVT": false, - "actions": [] + "actions": [], + "keybindings": [] })" }; static constexpr std::string_view smallGlobalsString{ R"( { "defaultProfile": "{61c54bbd-c2c6-5271-96e7-009a87ff44bf}", - "actions": [] + "actions": [], + "keybindings": [] })" }; RoundtripTest(globalsString); @@ -275,47 +287,50 @@ namespace SettingsModelUnitTests { // simple command static constexpr std::string_view actionsString1{ R"([ - { "command": "paste" } + { "command": "paste", "id": "Test.Paste" } ])" }; // complex command static constexpr std::string_view actionsString2A{ R"([ - { "command": { "action": "setTabColor" } } + { "command": { "action": "setTabColor" }, "id": "Test.SetTabColor" } ])" }; static constexpr std::string_view actionsString2B{ R"([ - { "command": { "action": "setTabColor", "color": "#112233" } } + { "command": { "action": "setTabColor", "color": "#112233" }, "id": "Test.SetTabColor112233" } ])" }; static constexpr std::string_view actionsString2C{ R"([ - { "command": { "action": "copy" } }, - { "command": { "action": "copy", "singleLine": true, "copyFormatting": "html" } } + { "command": { "action": "copy" }, "id": "Test.Copy" }, + { "command": { "action": "copy", "singleLine": true, "copyFormatting": "html" }, "id": "Test.CopyWithArgs" } ])" }; // simple command with key chords - static constexpr std::string_view actionsString3{ R"([ - { "command": "toggleAlwaysOnTop", "keys": "ctrl+a" }, - { "command": "toggleAlwaysOnTop", "keys": "ctrl+b" } - ])" }; + static constexpr std::string_view actionsString3{ R"({ "actions": [ + { "command": "toggleAlwaysOnTop", "id": "Test.ToggleAlwaysOnTop" } ], + "keybindings": [ + { "keys": "ctrl+a", "id": "Test.ToggleAlwaysOnTop" }, + { "keys": "ctrl+b", "id": "Test.ToggleAlwaysOnTop" } ]})" }; // complex command with key chords - static constexpr std::string_view actionsString4A{ R"([ - { "command": { "action": "adjustFontSize", "delta": 1 }, "keys": "ctrl+c" }, - { "command": { "action": "adjustFontSize", "delta": 1 }, "keys": "ctrl+d" } - ])" }; + static constexpr std::string_view actionsString4A{ R"({ "actions":[ + { "command": { "action": "adjustFontSize", "delta": 1 }, "id": "Test.EnlargeFont" } ], + "keybindings": [ + { "keys": "ctrl+c", "id": "Test.EnlargeFont" }, + { "keys": "ctrl+d", "id": "Test.EnlargeFont" } ]})" }; // command with name and icon and multiple key chords - static constexpr std::string_view actionsString5{ R"([ - { "icon": "image.png", "name": "Scroll To Top Name", "command": "scrollToTop", "keys": "ctrl+e" }, - { "command": "scrollToTop", "keys": "ctrl+f" } - ])" }; + static constexpr std::string_view actionsString5{ R"({ "actions":[ + { "icon": "image.png", "name": "Scroll To Top Name", "command": "scrollToTop", "id": "Test.ScrollToTop" } ], + "keybindings": [ + { "id": "Test.ScrollToTop", "keys": "ctrl+f" }, + { "id": "Test.ScrollToTop", "keys": "ctrl+e" } ]})" }; // complex command with new terminal args static constexpr std::string_view actionsString6{ R"([ - { "command": { "action": "newTab", "index": 0 }, "keys": "ctrl+g" }, + { "command": { "action": "newTab", "index": 0 }, "id": "Test.NewTerminal" }, ])" }; // complex command with meaningful null arg static constexpr std::string_view actionsString7{ R"([ - { "command": { "action": "renameWindow", "name": null }, "keys": "ctrl+h" } + { "command": { "action": "renameWindow", "name": null }, "id": "Test.MeaningfulNull" } ])" }; // nested command @@ -397,9 +412,9 @@ namespace SettingsModelUnitTests ])"" }; // unbound command - static constexpr std::string_view actionsString10{ R"([ - { "command": "unbound", "keys": "ctrl+c" } - ])" }; + static constexpr std::string_view actionsString10{ R"({ "actions": [], + "keybindings": [ + { "id": null, "keys": "ctrl+c" } ]})" }; Log::Comment(L"simple command"); RoundtripTest(actionsString1); @@ -409,14 +424,16 @@ namespace SettingsModelUnitTests RoundtripTest(actionsString2B); RoundtripTest(actionsString2C); + // ActionMap has effectively 2 "to json" calls we need to make, one for the actions and one for the keybindings + // So we cannot use RoundtripTest for actions + keychords, just use RoundTripTest Log::Comment(L"simple command with key chords"); - RoundtripTest(actionsString3); + RoundtripTest(actionsString3); Log::Comment(L"complex commands with key chords"); - RoundtripTest(actionsString4A); + RoundtripTest(actionsString4A); Log::Comment(L"command with name and icon and multiple key chords"); - RoundtripTest(actionsString5); + RoundtripTest(actionsString5); Log::Comment(L"complex command with new terminal args"); RoundtripTest(actionsString6); @@ -434,7 +451,7 @@ namespace SettingsModelUnitTests RoundtripTest(actionsString9D); Log::Comment(L"unbound command"); - RoundtripTest(actionsString10); + RoundtripTest(actionsString10); } void SerializationTests::CascadiaSettings() @@ -503,7 +520,10 @@ namespace SettingsModelUnitTests } ], "actions": [ - { "command": { "action": "sendInput", "input": "VT Griese Mode" }, "id": "User.sendInput.E02B3DF9", "keys": "ctrl+k" } + { "command": { "action": "sendInput", "input": "VT Griese Mode" }, "id": "Test.SendInput" } + ], + "keybindings": [ + { "id": "Test.SendInput", "keys": "ctrl+k" } ], "theme": "system", "themes": [] @@ -995,7 +1015,6 @@ namespace SettingsModelUnitTests { "name": "foo", "command": "closePane", - "keys": "ctrl+shift+w", "id": "thisIsMyClosePane" }, { @@ -1065,4 +1084,217 @@ namespace SettingsModelUnitTests VERIFY_ARE_EQUAL(sendInputCmd1.ID(), sendInputCmd1.ID()); } + + void SerializationTests::RoundtripLegacyToModernActions() + { + static constexpr std::string_view oldSettingsJson{ R"( + { + "actions": [ + { + "name": "foo", + "id": "Test.SendInput", + "command": { "action": "sendInput", "input": "just some input" }, + "keys": "ctrl+shift+w" + }, + { + "command": "unbound", + "keys": "ctrl+shift+x" + } + ] + })" }; + + // modern style: + // - no "unbound" actions, these are just keybindings that have no id + // - no keys in actions, these are keybindings with an id + static constexpr std::string_view newSettingsJson{ R"( + { + "actions": [ + { + "name": "foo", + "command": { "action": "sendInput", "input": "just some input" }, + "id": "Test.SendInput" + } + ], + "keybindings": [ + { + "id": "Test.SendInput", + "keys": "ctrl+shift+w" + }, + { + "id": null, + "keys": "ctrl+shift+x" + } + ] + })" }; + + implementation::SettingsLoader loader{ oldSettingsJson, implementation::LoadStringResource(IDR_DEFAULTS) }; + loader.MergeInboxIntoUserSettings(); + loader.FinalizeLayering(); + VERIFY_IS_TRUE(loader.FixupUserSettings(), L"Validate that this will indicate we need to write them back to disk"); + const auto settings = winrt::make_self(std::move(loader)); + const auto oldResult{ settings->ToJson() }; + + implementation::SettingsLoader newLoader{ newSettingsJson, implementation::LoadStringResource(IDR_DEFAULTS) }; + newLoader.MergeInboxIntoUserSettings(); + newLoader.FinalizeLayering(); + VERIFY_IS_FALSE(newLoader.FixupUserSettings(), L"Validate that there is no need to write back to disk"); + const auto newSettings = winrt::make_self(std::move(newLoader)); + const auto newResult{ newSettings->ToJson() }; + + VERIFY_ARE_EQUAL(toString(newResult), toString(oldResult)); + } + + void SerializationTests::RoundtripUserActionsSameAsInBoxAreRemoved() + { + static constexpr std::string_view oldSettingsJson{ R"( + { + "actions": [ + { + "command": "paste", + "keys": "ctrl+shift+x" + } + ] + })" }; + + // this action is the same as in inbox one, + // so we will delete this action from the user's file but retain the keybinding + static constexpr std::string_view newSettingsJson{ R"( + { + "actions": [ + ], + "keybindings": [ + { + "id": "Terminal.PasteFromClipboard", + "keys": "ctrl+shift+x" + } + ] + })" }; + + implementation::SettingsLoader loader{ oldSettingsJson, implementation::LoadStringResource(IDR_DEFAULTS) }; + loader.MergeInboxIntoUserSettings(); + loader.FinalizeLayering(); + VERIFY_IS_TRUE(loader.FixupUserSettings(), L"Validate that this will indicate we need to write them back to disk"); + const auto settings = winrt::make_self(std::move(loader)); + const auto oldResult{ settings->ToJson() }; + + implementation::SettingsLoader newLoader{ newSettingsJson, implementation::LoadStringResource(IDR_DEFAULTS) }; + newLoader.MergeInboxIntoUserSettings(); + newLoader.FinalizeLayering(); + VERIFY_IS_FALSE(newLoader.FixupUserSettings(), L"Validate that there is no need to write back to disk"); + const auto newSettings = winrt::make_self(std::move(newLoader)); + const auto newResult{ newSettings->ToJson() }; + + VERIFY_ARE_EQUAL(toString(newResult), toString(oldResult)); + } + + void SerializationTests::RoundtripActionsSameNameDifferentCommandsAreRetained() + { + static constexpr std::string_view oldSettingsJson{ R"( + { + "actions": [ + { + "command": { "action": "sendInput", "input": "just some input" }, + "name": "mySendInput" + }, + { + "command": { "action": "sendInput", "input": "just some input 2" }, + "name": "mySendInput" + } + ] + })" }; + + // There are two different actions with the same name, + // ensure that both are kept but have different IDs generated for them + static constexpr std::string_view newSettingsJson{ R"( + { + "actions": [ + { + "name": "mySendInput", + "command": { "action": "sendInput", "input": "just some input" }, + "id": "User.sendInput.)" SEND_INPUT_ARCH_SPECIFIC_ACTION_HASH R"(" + }, + { + "name": "mySendInput", + "command": { "action": "sendInput", "input": "just some input 2" }, + "id": "User.sendInput.)" SEND_INPUT2_ARCH_SPECIFIC_ACTION_HASH R"(" + } + ] + })" }; + + implementation::SettingsLoader loader{ oldSettingsJson, implementation::LoadStringResource(IDR_DEFAULTS) }; + loader.MergeInboxIntoUserSettings(); + loader.FinalizeLayering(); + VERIFY_IS_TRUE(loader.FixupUserSettings(), L"Validate that this will indicate we need to write them back to disk"); + const auto settings = winrt::make_self(std::move(loader)); + const auto oldResult{ settings->ToJson() }; + + implementation::SettingsLoader newLoader{ newSettingsJson, implementation::LoadStringResource(IDR_DEFAULTS) }; + newLoader.MergeInboxIntoUserSettings(); + newLoader.FinalizeLayering(); + VERIFY_IS_FALSE(newLoader.FixupUserSettings(), L"Validate that there is no need to write back to disk"); + const auto newSettings = winrt::make_self(std::move(newLoader)); + const auto newResult{ newSettings->ToJson() }; + + VERIFY_ARE_EQUAL(toString(newResult), toString(oldResult)); + } + + void SerializationTests::MultipleActionsAreCollapsed() + { + static constexpr std::string_view oldSettingsJson{ R"( + { + "actions": [ + { + "name": "foo", + "icon": "myCoolIconPath.png", + "command": { "action": "sendInput", "input": "just some input" }, + "keys": "ctrl+shift+w" + }, + { + "command": { "action": "sendInput", "input": "just some input" }, + "keys": "ctrl+shift+x" + } + ] + })" }; + + // modern style: + // - multiple action blocks whose purpose is simply to define more keybindings for the same action + // get collapsed into one action block, with the name and icon path preserved and have multiple keybindings instead + static constexpr std::string_view newSettingsJson{ R"( + { + "actions": [ + { + "name": "foo", + "icon": "myCoolIconPath.png", + "command": { "action": "sendInput", "input": "just some input" }, + "id": "User.sendInput.)" SEND_INPUT_ARCH_SPECIFIC_ACTION_HASH R"(" + } + ], + "keybindings": [ + { + "keys": "ctrl+shift+w", + "id": "User.sendInput.)" SEND_INPUT_ARCH_SPECIFIC_ACTION_HASH R"(" + }, + { + "keys": "ctrl+shift+x", + "id": "User.sendInput.)" SEND_INPUT_ARCH_SPECIFIC_ACTION_HASH R"(" + } + ] + })" }; + + implementation::SettingsLoader loader{ oldSettingsJson, implementation::LoadStringResource(IDR_DEFAULTS) }; + loader.MergeInboxIntoUserSettings(); + loader.FinalizeLayering(); + VERIFY_IS_TRUE(loader.FixupUserSettings(), L"Validate that this will indicate we need to write them back to disk"); + const auto settings = winrt::make_self(std::move(loader)); + const auto oldResult{ settings->ToJson() }; + + implementation::SettingsLoader newLoader{ newSettingsJson, implementation::LoadStringResource(IDR_DEFAULTS) }; + newLoader.MergeInboxIntoUserSettings(); + newLoader.FinalizeLayering(); + VERIFY_IS_FALSE(newLoader.FixupUserSettings(), L"Validate that there is no need to write back to disk"); + const auto newSettings = winrt::make_self(std::move(newLoader)); + const auto newResult{ newSettings->ToJson() }; + + VERIFY_ARE_EQUAL(toString(newResult), toString(oldResult)); + } }