diff --git a/src/cascadia/UnitTests_SettingsModel/KeyBindingsTests.cpp b/src/cascadia/UnitTests_SettingsModel/KeyBindingsTests.cpp index 0e38d164f0b..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", "id": "Test.SplitPane", "keys": ["ctrl+c"] } ])"), OriginTag::None); - actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": "splitPane", "id": "Test.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", } , "id": "Test.NewTabNoArgs", "keys": ["ctrl+c"] } ])"), OriginTag::None); - actionMap->LayerJson(VerifyParseSucceeded(R"([ { "command": { "action": "newTab", "index": 0 } , "id": "Test.NewTab0", "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 }; diff --git a/src/cascadia/UnitTests_SettingsModel/SerializationTests.cpp b/src/cascadia/UnitTests_SettingsModel/SerializationTests.cpp index 0e1cab1ddc9..2afa1baa6c6 100644 --- a/src/cascadia/UnitTests_SettingsModel/SerializationTests.cpp +++ b/src/cascadia/UnitTests_SettingsModel/SerializationTests.cpp @@ -47,6 +47,8 @@ namespace SettingsModelUnitTests TEST_METHOD(RoundtripGenerateActionID); TEST_METHOD(NoGeneratedIDsForIterableAndNestedCommands); TEST_METHOD(GeneratedActionIDsEqualForIdenticalCommands); + TEST_METHOD(RoundtripLegacyToModernActions); + TEST_METHOD(MultipleActionsAreCollapsed); private: // Method Description: @@ -1074,4 +1076,123 @@ 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::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 iconpath 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)); + } }