Add IDs to Commands (#16904)

As laid out in #16816, adds an `ID` field to `Command`.

**This first PR only adds IDs for built-in commands in defaults, and
generates IDs for user-created commands that don't define an ID.** Also
note that for now we **will not** be allowing IDs for iterable/nested
commands.

The follow-up PR is where we will actually use the IDs by referring to
commands with them.

Refs #16816 

## Validation Steps Performed
User-created commands in the settings file get rewritten with generated
IDs
This commit is contained in:
PankajBhojwani
2024-04-17 17:30:25 -07:00
committed by GitHub
parent 643f7167a6
commit 06ab6f3e1f
10 changed files with 352 additions and 127 deletions

View File

@@ -16,6 +16,14 @@ using namespace WEX::Common;
using namespace winrt::Microsoft::Terminal::Settings::Model;
using namespace winrt::Microsoft::Terminal::Control;
// Different architectures will hash the same SendInput command to a different ID
// Check for the correct ID based on the architecture
#if defined(_M_IX86)
#define SEND_INPUT_ARCH_SPECIFIC_ACTION_HASH "56911147"
#else
#define SEND_INPUT_ARCH_SPECIFIC_ACTION_HASH "A020D2"
#endif
namespace SettingsModelUnitTests
{
class SerializationTests : public JsonTestClass
@@ -36,6 +44,10 @@ namespace SettingsModelUnitTests
TEST_METHOD(RoundtripUserModifiedColorSchemeCollisionUnusedByProfiles);
TEST_METHOD(RoundtripUserDeletedColorSchemeCollision);
TEST_METHOD(RoundtripGenerateActionID);
TEST_METHOD(NoGeneratedIDsForIterableAndNestedCommands);
TEST_METHOD(GeneratedActionIDsEqualForIdenticalCommands);
private:
// Method Description:
// - deserializes and reserializes a json string representing a settings object model of type T
@@ -491,7 +503,7 @@ namespace SettingsModelUnitTests
}
],
"actions": [
{ "command": { "action": "sendInput", "input": "VT Griese Mode" }, "keys": "ctrl+k" }
{ "command": { "action": "sendInput", "input": "VT Griese Mode" }, "id": "User.sendInput.E02B3DF9", "keys": "ctrl+k" }
],
"theme": "system",
"themes": []
@@ -946,4 +958,127 @@ namespace SettingsModelUnitTests
VERIFY_ARE_EQUAL(toString(newResult), toString(oldResult));
}
void SerializationTests::RoundtripGenerateActionID()
{
static constexpr std::string_view oldSettingsJson{ R"(
{
"actions": [
{
"name": "foo",
"command": { "action": "sendInput", "input": "just some input" },
"keys": "ctrl+shift+w"
}
]
})" };
// Key differences: - the sendInput action now has a generated ID
// - this generated ID was created at the time of writing this test,
// and should remain robust (i.e. every time we hash the args we should get the same result)
static constexpr std::string_view newSettingsJson{ R"(
{
"actions": [
{
"name": "foo",
"command": { "action": "sendInput", "input": "just some input" },
"keys": "ctrl+shift+w",
"id" : "User.sendInput.)" SEND_INPUT_ARCH_SPECIFIC_ACTION_HASH R"("
}
]
})" };
implementation::SettingsLoader oldLoader{ oldSettingsJson, implementation::LoadStringResource(IDR_DEFAULTS) };
oldLoader.MergeInboxIntoUserSettings();
oldLoader.FinalizeLayering();
VERIFY_IS_TRUE(oldLoader.FixupUserSettings(), L"Validate that this will indicate we need to write them back to disk");
const auto oldSettings = winrt::make_self<implementation::CascadiaSettings>(std::move(oldLoader));
const auto oldResult{ oldSettings->ToJson() };
implementation::SettingsLoader newLoader{ newSettingsJson, implementation::LoadStringResource(IDR_DEFAULTS) };
newLoader.MergeInboxIntoUserSettings();
newLoader.FinalizeLayering();
newLoader.FixupUserSettings();
const auto newSettings = winrt::make_self<implementation::CascadiaSettings>(std::move(newLoader));
const auto newResult{ newSettings->ToJson() };
VERIFY_ARE_EQUAL(toString(newResult), toString(oldResult));
}
void SerializationTests::NoGeneratedIDsForIterableAndNestedCommands()
{
// for iterable commands, nested commands, and user-defined actions that already have
// an ID, we do not need to generate an ID
static constexpr std::string_view oldSettingsJson{ R"(
{
"actions": [
{
"name": "foo",
"command": "closePane",
"keys": "ctrl+shift+w",
"id": "thisIsMyClosePane"
},
{
"iterateOn": "profiles",
"icon": "${profile.icon}",
"name": "${profile.name}",
"command": { "action": "newTab", "profile": "${profile.name}" }
},
{
"name": "Change font size...",
"commands": [
{ "command": { "action": "adjustFontSize", "delta": 1.0 } },
{ "command": { "action": "adjustFontSize", "delta": -1.0 } },
{ "command": "resetFontSize" },
]
}
]
})" };
implementation::SettingsLoader oldLoader{ oldSettingsJson, implementation::LoadStringResource(IDR_DEFAULTS) };
oldLoader.MergeInboxIntoUserSettings();
oldLoader.FinalizeLayering();
VERIFY_IS_FALSE(oldLoader.FixupUserSettings(), L"Validate that there is no need to write back to disk");
}
void SerializationTests::GeneratedActionIDsEqualForIdenticalCommands()
{
static constexpr std::string_view settingsJson1{ R"(
{
"actions": [
{
"name": "foo",
"command": { "action": "sendInput", "input": "this is some other input string" },
"keys": "ctrl+shift+w"
}
]
})" };
// Both settings files define the same action, so the generated ID should be the same for both
static constexpr std::string_view settingsJson2{ R"(
{
"actions": [
{
"name": "foo",
"command": { "action": "sendInput", "input": "this is some other input string" },
"keys": "ctrl+shift+w"
}
]
})" };
implementation::SettingsLoader loader1{ settingsJson1, implementation::LoadStringResource(IDR_DEFAULTS) };
loader1.MergeInboxIntoUserSettings();
loader1.FinalizeLayering();
VERIFY_IS_TRUE(loader1.FixupUserSettings(), L"Validate that this will indicate we need to write them back to disk");
const auto settings1 = winrt::make_self<implementation::CascadiaSettings>(std::move(loader1));
const auto result1{ settings1->ToJson() };
implementation::SettingsLoader loader2{ settingsJson2, implementation::LoadStringResource(IDR_DEFAULTS) };
loader2.MergeInboxIntoUserSettings();
loader2.FinalizeLayering();
VERIFY_IS_TRUE(loader2.FixupUserSettings(), L"Validate that this will indicate we need to write them back to disk");
const auto settings2 = winrt::make_self<implementation::CascadiaSettings>(std::move(loader2));
const auto result2{ settings2->ToJson() };
VERIFY_ARE_EQUAL(toString(result1), toString(result2));
}
}