From 9703815f59731f96b82c5ffd154b59b45ce116d4 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 31 May 2024 16:32:21 -0700 Subject: [PATCH] nits n fixes --- .../TerminalSettingsModel/ActionMap.cpp | 51 +++--- .../TerminalSettingsModel/ActionMap.h | 8 +- .../ActionMapSerialization.cpp | 6 +- .../CascadiaSettingsSerialization.cpp | 2 +- .../TerminalSettingsModel/Command.cpp | 6 +- src/cascadia/TerminalSettingsModel/Command.h | 4 +- .../GlobalAppSettings.cpp | 10 +- .../TerminalSettingsModel/GlobalAppSettings.h | 2 +- .../TerminalSettingsModel/defaults.json | 150 +++++++++--------- .../SerializationTests.cpp | 44 +++++ 10 files changed, 169 insertions(+), 114 deletions(-) diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.cpp b/src/cascadia/TerminalSettingsModel/ActionMap.cpp index 1834a202307..e8605b41ac2 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMap.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionMap.cpp @@ -76,9 +76,13 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { // only one parent contains all the inbox actions and that parent contains only inbox actions, // so if we found a non-inbox action we can just skip to the next parent - continue; + break; } foundParent = parent; + } + + if (foundParent) + { break; } } @@ -91,13 +95,15 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } } + 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()) + if (userCmdImpl->IDWasGenerated() && !userCmdImpl->HasName() && userCmdImpl->IconPath().empty()) { const auto userActionHash = Hash(userCmdImpl->ActionAndArgs()); if (const auto inboxCmd = inboxActions.find(userActionHash); inboxCmd != inboxActions.end()) @@ -107,7 +113,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // for any of our keys that point to the user action, point them to the inbox action instead if (cmdID == pair.first) { - _KeyMap.insert_or_assign(key, inboxCmd->second.ID()); + keysToReassign.insert_or_assign(key, inboxCmd->second.ID()); // register the keys with the inbox action inboxCmd->second.RegisterKey(key); @@ -120,11 +126,16 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } return false; }); + + for (const auto [key, cmdID] : keysToReassign) + { + _KeyMap.insert_or_assign(key, cmdID); + } } - bool ActionMap::FixUpsAppliedDuringLoad() const + bool ActionMap::FixupsAppliedDuringLoad() const { - return _fixUpsAppliedDuringLoad; + return _fixupsAppliedDuringLoad; } // Method Description: @@ -134,7 +145,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // - 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 + Model::Command ActionMap::_GetActionByID(const winrt::hstring& actionID) const { // Check current layer const auto actionMapPair{ _ActionMap.find(actionID) }; @@ -204,9 +215,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { // Only populate AvailableActions with actions that haven't been visited already. const auto actionID = Hash(cmd.ActionAndArgs()); - if (visitedActionIDs.find(actionID) == visitedActionIDs.end()) + if (!visitedActionIDs.contains(actionID)) { - const auto& name{ cmd.Name() }; + const auto name{ cmd.Name() }; if (!name.empty()) { // Update AvailableActions. @@ -314,7 +325,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // Method Description: // - Recursively populate keyBindingsMap with ours and our parents' key -> id pairs - // - This is a bottom-up approach, ensuring that the keybindings of the parents are overridden by the children + // - This is a bottom-up approach + // - Keybindings of the parents are overridden by the children void ActionMap::_PopulateCumulativeKeyMap(std::unordered_map& keyBindingsMap) { for (const auto& [keys, cmdID] : _KeyMap) @@ -333,7 +345,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // Method Description: // - Recursively populate actionMap with ours and our parents' id -> command pairs - // - This is a bottom-up approach, ensuring that the actions of the parents are overridden by the children + // - 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) @@ -499,7 +512,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } // only add to the _ActionMap if there is an ID - if (auto cmdID = cmd.ID(); !cmdID.empty() && cmd.ActionAndArgs().Action() != ShortcutAction::Invalid) + if (auto cmdID = cmd.ID(); !cmdID.empty()) { // 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" } @@ -514,7 +527,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // 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 (cmdImpl->IDWasGenerated()) { if (const auto foundCmd{ _GetActionByID(cmdID) }) { @@ -611,7 +624,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { if (const auto keyIDPair = _KeyMap.find(keys); keyIDPair != _KeyMap.end()) { - // the keychord is defined in this layer, return the ID (ID be empty, in which case this key is explicitly unbound) + // the keychord is defined in this layer, return the ID return keyIDPair->second; } @@ -624,7 +637,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } } - // we did not find the keychord anywhere, its not bound and not explicity unbound either + // we did not find the keychord anywhere, it's not bound and not explicitly unbound either return std::nullopt; } @@ -641,7 +654,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { if (const auto actionIDOptional = _GetActionIdByKeyChordInternal(keys)) { - if (!(*actionIDOptional).empty()) + if (!actionIDOptional->empty()) { // there is an ID associated with these keys, find the command if (const auto foundCmd = _GetActionByID(*actionIDOptional)) @@ -649,10 +662,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation return foundCmd; } } - { - // the ID is an empty string, these keys are explicitly unbound - return nullptr; - } + // the ID is an empty string, these keys are explicitly unbound + return nullptr; } return std::nullopt; @@ -665,7 +676,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // 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(winrt::hstring cmdID) const + Control::KeyChord ActionMap::GetKeyBindingForAction(const winrt::hstring& cmdID) const { // Check our internal state. if (const auto cmd{ _GetActionByID(cmdID) }) diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.h b/src/cascadia/TerminalSettingsModel/ActionMap.h index 8d4b078a340..87bd5efb585 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMap.h +++ b/src/cascadia/TerminalSettingsModel/ActionMap.h @@ -61,7 +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(winrt::hstring cmdID) const; + Control::KeyChord GetKeyBindingForAction(const winrt::hstring& cmdID) const; // population void AddAction(const Model::Command& cmd); @@ -71,7 +71,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation 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; + bool FixupsAppliedDuringLoad() const; // modification bool RebindKeys(const Control::KeyChord& oldKeys, const Control::KeyChord& newKeys); @@ -85,7 +85,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation winrt::Windows::Foundation::Collections::IVector FilterToSendInput(winrt::hstring currentCommandline); private: - Model::Command _GetActionByID(const winrt::hstring 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; @@ -109,7 +109,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation std::unordered_map _NestedCommands; std::vector _IterableCommands; - bool _fixUpsAppliedDuringLoad; + bool _fixupsAppliedDuringLoad{ false }; void _AddKeyBindingHelper(const Json::Value& json, std::vector& warnings); diff --git a/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp b/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp index bbb229bdf9a..e81c9dbd050 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp @@ -69,15 +69,15 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { if (jsonBlock.isMember(JsonKey(KeysKey))) { - // there are keys in this command block - its the legacy style - _fixUpsAppliedDuringLoad = true; + // 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; + _fixupsAppliedDuringLoad = true; } } } diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp index 009154eb199..e9acef452c5 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp @@ -461,7 +461,7 @@ bool SettingsLoader::FixupUserSettings() }; auto fixedUp = userSettings.fixupsAppliedDuringLoad; - fixedUp = userSettings.globals->FixUpsAppliedDuringLoad() || fixedUp; + fixedUp = userSettings.globals->FixupsAppliedDuringLoad() || fixedUp; fixedUp = RemapColorSchemeForProfile(userSettings.baseLayerProfile) || fixedUp; for (const auto& profile : userSettings.profiles) diff --git a/src/cascadia/TerminalSettingsModel/Command.cpp b/src/cascadia/TerminalSettingsModel/Command.cpp index 97ff954a77d..7d786b9008d 100644 --- a/src/cascadia/TerminalSettingsModel/Command.cpp +++ b/src/cascadia/TerminalSettingsModel/Command.cpp @@ -121,14 +121,14 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation if (const auto generatedID = actionAndArgsImpl->GenerateID(); !generatedID.empty()) { _ID = generatedID; - _IdWasGenerated = true; + _IDWasGenerated = true; } } } - bool Command::IdWasGenerated() + bool Command::IDWasGenerated() { - return _IdWasGenerated; + return _IDWasGenerated; } void Command::Name(const hstring& value) diff --git a/src/cascadia/TerminalSettingsModel/Command.h b/src/cascadia/TerminalSettingsModel/Command.h index 9a3bdd77e31..f2d29daa3f9 100644 --- a/src/cascadia/TerminalSettingsModel/Command.h +++ b/src/cascadia/TerminalSettingsModel/Command.h @@ -71,7 +71,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation hstring ID() const noexcept; void GenerateID(); - bool IdWasGenerated(); + bool IDWasGenerated(); Control::KeyChord Keys() const noexcept; hstring KeyChordText() const noexcept; @@ -97,7 +97,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation std::vector _keyMappings; std::optional _name; std::wstring _ID; - bool _IdWasGenerated{ false }; + bool _IDWasGenerated{ false }; std::optional _iconPath; bool _nestedCommand{ false }; diff --git a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp index 834513489c1..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" }; @@ -158,7 +158,7 @@ void GlobalAppSettings::LayerActionsFrom(const Json::Value& json, const OriginTa { // 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, LegacyKeybindingsKey }; + static constexpr std::array bindingsKeys{ ActionsKey, KeybindingsKey }; for (const auto& jsonKey : bindingsKeys) { if (auto bindings{ json[JsonKey(jsonKey)] }) @@ -262,14 +262,14 @@ Json::Value GlobalAppSettings::ToJson() #undef GLOBAL_SETTINGS_TO_JSON json[JsonKey(ActionsKey)] = _actionMap->ToJson(); - json[JsonKey(LegacyKeybindingsKey)] = _actionMap->KeyBindingsToJson(); + json[JsonKey(KeybindingsKey)] = _actionMap->KeyBindingsToJson(); return json; } -bool GlobalAppSettings::FixUpsAppliedDuringLoad() +bool GlobalAppSettings::FixupsAppliedDuringLoad() { - return _actionMap->FixUpsAppliedDuringLoad(); + return _actionMap->FixupsAppliedDuringLoad(); } winrt::Microsoft::Terminal::Settings::Model::Theme GlobalAppSettings::CurrentTheme() noexcept diff --git a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h index 7b3e1ff9a82..7b55b7007b5 100644 --- a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h +++ b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h @@ -53,7 +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(); + bool FixupsAppliedDuringLoad(); const std::vector& KeybindingsWarnings() const; diff --git a/src/cascadia/TerminalSettingsModel/defaults.json b/src/cascadia/TerminalSettingsModel/defaults.json index 6424311c518..1a3e7c6c111 100644 --- a/src/cascadia/TerminalSettingsModel/defaults.json +++ b/src/cascadia/TerminalSettingsModel/defaults.json @@ -530,7 +530,7 @@ { "command": "restartConnection", "id": "Terminal.RestartConnection" }, // Clipboard Integration - { "command": { "action": "copy", "singleLine": false }, "id": "Terminal.CopySelectedText" }, + { "command": { "action": "copy", "singleLine": false }, "id": "Terminal.CopyToClipboard" }, { "command": "paste", "id": "Terminal.PasteFromClipboard" }, { "command": "selectAll", "id": "Terminal.SelectAll" }, { "command": "markMode", "id": "Terminal.ToggleMarkMode" }, @@ -625,84 +625,84 @@ } ], "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" }, + // 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" }, + // 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" }, + // 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.CopySelectedText" }, - { "keys": "ctrl+insert", "id": "Terminal.CopySelectedText" }, - { "keys": "enter", "id": "Terminal.CopySelectedText" }, - { "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" }, + // 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" }, + // 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" }, + // 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/UnitTests_SettingsModel/SerializationTests.cpp b/src/cascadia/UnitTests_SettingsModel/SerializationTests.cpp index b372182974f..433c98ba3f9 100644 --- a/src/cascadia/UnitTests_SettingsModel/SerializationTests.cpp +++ b/src/cascadia/UnitTests_SettingsModel/SerializationTests.cpp @@ -48,6 +48,7 @@ namespace SettingsModelUnitTests TEST_METHOD(NoGeneratedIDsForIterableAndNestedCommands); TEST_METHOD(GeneratedActionIDsEqualForIdenticalCommands); TEST_METHOD(RoundtripLegacyToModernActions); + TEST_METHOD(RoundtripUserActionsSameAsInBoxAreRemoved); TEST_METHOD(MultipleActionsAreCollapsed); private: @@ -1136,6 +1137,49 @@ namespace SettingsModelUnitTests 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::MultipleActionsAreCollapsed() { static constexpr std::string_view oldSettingsJson{ R"(