folkslinux
/
windows-terminal
mirror of https://github.com/microsoft/terminal.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add support for actions in fragments (#16185) 

Surprisingly easier than I thought this would be. ActionMap already
supports layering (from defaults.json), so this basically re-uses a lot
of that for fun and profit.

The trickiest bits:
* In `SettingsLoader::_parseFragment`, I'm constructing a fake, empty
JSON object, and taking _only_ the actions out from the fragment, and
stuffing them into this temp json. Then, I parse that as a globals
object, and set _that_ as the parent to the user settings file. That
results in _only_ the actions from the fragment being parsed before the
user's actions.
* In that same method, I'm also explicitly preventing the ActionMap (et
al.) from parsing `keys` from these actions. We don't want fragments to
be able to say "ctrl+f is clear buffer" or something like that. This
required a bit of annoying plumbing.


Closes #16063 
Tests added.
Docs need to be updated.
This commit is contained in:
Mike Griese 2024-02-26 12:27:57 -08:00 committed by GitHub
parent a6a0e44088
commit 88def9ddcd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 246 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

190
src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp
View File

@ -74,6 +74,13 @@ namespace SettingsModelLocalTests
TEST_METHOD(TestInheritedCommand);
TEST_METHOD(LoadFragmentsWithMultipleUpdates);
TEST_METHOD(FragmentActionSimple);
TEST_METHOD(FragmentActionNoKeys);
TEST_METHOD(FragmentActionNested);
TEST_METHOD(FragmentActionNestedNoName);
TEST_METHOD(FragmentActionIterable);
TEST_METHOD(FragmentActionRoundtrip);
TEST_METHOD(MigrateReloadEnvVars);
private:
@ -2023,6 +2030,189 @@ namespace SettingsModelLocalTests
VERIFY_ARE_EQUAL(L"NewName", loader.userSettings.profiles[0]->Name());
}
void DeserializationTests::FragmentActionSimple()
{
static constexpr std::wstring_view fragmentSource{ L"fragment" };
static constexpr std::string_view fragmentJson{ R"({
"actions": [
{
"command": { "action": "addMark" },
"name": "Test Action"
},
]
})" };
implementation::SettingsLoader loader{ std::string_view{}, DefaultJson };
loader.MergeInboxIntoUserSettings();
loader.MergeFragmentIntoUserSettings(winrt::hstring{ fragmentSource }, fragmentJson);
loader.FinalizeLayering();
const auto settings = winrt::make_self<implementation::CascadiaSettings>(std::move(loader));
const auto actionMap = winrt::get_self<implementation::ActionMap>(settings->GlobalSettings().ActionMap());
const auto actionsByName = actionMap->NameMap();
VERIFY_IS_NOT_NULL(actionsByName.TryLookup(L"Test Action"));
}
void DeserializationTests::FragmentActionNoKeys()
{
static constexpr std::wstring_view fragmentSource{ L"fragment" };
static constexpr std::string_view fragmentJson{ R"({
"actions": [
{
"command": { "action": "addMark" },
"keys": "ctrl+f",
"name": "Test Action"
},
]
})" };
implementation::SettingsLoader loader{ std::string_view{}, DefaultJson };
loader.MergeInboxIntoUserSettings();
loader.MergeFragmentIntoUserSettings(winrt::hstring{ fragmentSource }, fragmentJson);
loader.FinalizeLayering();
const auto settings = winrt::make_self<implementation::CascadiaSettings>(std::move(loader));
const auto actionMap = winrt::get_self<implementation::ActionMap>(settings->GlobalSettings().ActionMap());
const auto actionsByName = actionMap->NameMap();
VERIFY_IS_NOT_NULL(actionsByName.TryLookup(L"Test Action"));
VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ VirtualKeyModifiers::Control, static_cast<int32_t>('F'), 0 }));
}
void DeserializationTests::FragmentActionNested()
{
static constexpr std::wstring_view fragmentSource{ L"fragment" };
static constexpr std::string_view fragmentJson{ R"({
"actions": [
{
"name": "nested command",
"commands": [
{
"name": "child1",
"command": { "action": "newTab", "commandline": "ssh me@first.com" }
},
{
"name": "child2",
"command": { "action": "newTab", "commandline": "ssh me@second.com" }
}
]
},
]
})" };
implementation::SettingsLoader loader{ std::string_view{}, DefaultJson };
loader.MergeInboxIntoUserSettings();
loader.MergeFragmentIntoUserSettings(winrt::hstring{ fragmentSource }, fragmentJson);
loader.FinalizeLayering();
const auto settings = winrt::make_self<implementation::CascadiaSettings>(std::move(loader));
const auto actionMap = winrt::get_self<implementation::ActionMap>(settings->GlobalSettings().ActionMap());
const auto actionsByName = actionMap->NameMap();
const auto& nested{ actionsByName.TryLookup(L"nested command") };
VERIFY_IS_NOT_NULL(nested);
VERIFY_IS_TRUE(nested.HasNestedCommands());
}
void DeserializationTests::FragmentActionNestedNoName()
{
// Basically the same as TestNestedCommandWithoutName
static constexpr std::wstring_view fragmentSource{ L"fragment" };
static constexpr std::string_view fragmentJson{ R"({
"actions": [
{
"commands": [
{
"name": "child1",
"command": { "action": "newTab", "commandline": "ssh me@first.com" }
},
{
"name": "child2",
"command": { "action": "newTab", "commandline": "ssh me@second.com" }
}
]
},
]
})" };
implementation::SettingsLoader loader{ std::string_view{}, DefaultJson };
loader.MergeInboxIntoUserSettings();
loader.MergeFragmentIntoUserSettings(winrt::hstring{ fragmentSource }, fragmentJson);
loader.FinalizeLayering();
const auto settings = winrt::make_self<implementation::CascadiaSettings>(std::move(loader));
VERIFY_ARE_EQUAL(0u, settings->Warnings().Size());
}
void DeserializationTests::FragmentActionIterable()
{
static constexpr std::wstring_view fragmentSource{ L"fragment" };
static constexpr std::string_view fragmentJson{ R"({
"actions": [
{
"name": "nested",
"commands": [
{
"iterateOn": "schemes",
"name": "${scheme.name}",
"command": { "action": "setColorScheme", "colorScheme": "${scheme.name}" }
}
]
},
]
})" };
implementation::SettingsLoader loader{ std::string_view{}, DefaultJson };
loader.MergeInboxIntoUserSettings();
loader.MergeFragmentIntoUserSettings(winrt::hstring{ fragmentSource }, fragmentJson);
loader.FinalizeLayering();
const auto settings = winrt::make_self<implementation::CascadiaSettings>(std::move(loader));
const auto actionMap = winrt::get_self<implementation::ActionMap>(settings->GlobalSettings().ActionMap());
const auto actionsByName = actionMap->NameMap();
const auto& nested{ actionsByName.TryLookup(L"nested") };
VERIFY_IS_NOT_NULL(nested);
VERIFY_IS_TRUE(nested.HasNestedCommands());
VERIFY_ARE_EQUAL(settings->GlobalSettings().ColorSchemes().Size(), nested.NestedCommands().Size());
}
void DeserializationTests::FragmentActionRoundtrip()
{
static constexpr std::wstring_view fragmentSource{ L"fragment" };
static constexpr std::string_view fragmentJson{ R"({
"actions": [
{
"command": { "action": "addMark" },
"name": "Test Action"
},
]
})" };
implementation::SettingsLoader loader{ std::string_view{}, DefaultJson };
loader.MergeInboxIntoUserSettings();
loader.MergeFragmentIntoUserSettings(winrt::hstring{ fragmentSource }, fragmentJson);
loader.FinalizeLayering();
const auto oldSettings = winrt::make_self<implementation::CascadiaSettings>(std::move(loader));
const auto actionMap = winrt::get_self<implementation::ActionMap>(oldSettings->GlobalSettings().ActionMap());
const auto actionsByName = actionMap->NameMap();
VERIFY_IS_NOT_NULL(actionsByName.TryLookup(L"Test Action"));
const auto oldResult{ oldSettings->ToJson() };
Log::Comment(L"Now, create a _new_ settings object from the re-serialization of the first");
implementation::SettingsLoader newLoader{ toString(oldResult), DefaultJson };
// NOTABLY! Don't load the fragment here.
newLoader.MergeInboxIntoUserSettings();
newLoader.FinalizeLayering();
const auto newSettings = winrt::make_self<implementation::CascadiaSettings>(std::move(newLoader));
const auto& newActionMap = winrt::get_self<implementation::ActionMap>(newSettings->GlobalSettings().ActionMap());
const auto newActionsByName = newActionMap->NameMap();
VERIFY_IS_NULL(newActionsByName.TryLookup(L"Test Action"));
}
void DeserializationTests::MigrateReloadEnvVars()
{
static constexpr std::string_view settings1Json{ R"(

26
src/cascadia/LocalTests_SettingsModel/SerializationTests.cpp
View File

@ -68,6 +68,19 @@ namespace SettingsModelLocalTests
// written alphabetically.
VERIFY_ARE_EQUAL(toString(json), toString(result));
}
// Helper to remove the `$schema` property from a json object. We
// populate that based off the local path to the settings file. Of
// course, that's entirely unpredictable in tests. So cut it out before
// we do any sort of roundtrip testing.
static Json::Value removeSchema(Json::Value json)
{
if (json.isMember("$schema"))
{
json.removeMember("$schema");
}
return json;
}
};
void SerializationTests::GlobalSettings()
@ -262,15 +275,6 @@ namespace SettingsModelLocalTests
{ "command": { "action": "adjustFontSize", "delta": 1.0 }, "keys": "ctrl+c" },
{ "command": { "action": "adjustFontSize", "delta": 1.0 }, "keys": "ctrl+d" }
])" };
// GH#13323 - these can be fragile. In the past, the order these get
// re-serialized as has been not entirely stable. We don't really care
// about the order they get re-serialized in, but the tests aren't
// clever enough to compare the structure, only the literal string
// itself. Feel free to change as needed.
static constexpr std::string_view actionsString4B{ R"([
{ "command": { "action": "findMatch", "direction": "prev" }, "keys": "ctrl+shift+r" },
{ "command": { "action": "adjustFontSize", "delta": 1.0 }, "keys": "ctrl+d" }
])" };
// command with name and icon and multiple key chords
static constexpr std::string_view actionsString5{ R"([
@ -384,7 +388,6 @@ namespace SettingsModelLocalTests
Log::Comment(L"complex commands with key chords");
RoundtripTest<implementation::ActionMap>(actionsString4A);
RoundtripTest<implementation::ActionMap>(actionsString4B);
Log::Comment(L"command with name and icon and multiple key chords");
RoundtripTest<implementation::ActionMap>(actionsString5);
@ -483,7 +486,8 @@ namespace SettingsModelLocalTests
const auto settings{ winrt::make_self<implementation::CascadiaSettings>(settingsString) };
const auto result{ settings->ToJson() };
VERIFY_ARE_EQUAL(toString(VerifyParseSucceeded(settingsString)), toString(result));
VERIFY_ARE_EQUAL(toString(removeSchema(VerifyParseSucceeded(settingsString))),
toString(removeSchema(result)));
}
void SerializationTests::LegacyFontSettings()

2
src/cascadia/TerminalSettingsModel/ActionMap.h
View File

@ -67,7 +67,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// JSON
static com_ptr<ActionMap> FromJson(const Json::Value& json);
std::vector<SettingsLoadWarnings> LayerJson(const Json::Value& json);
std::vector<SettingsLoadWarnings> LayerJson(const Json::Value& json, const bool withKeybindings = true);
Json::Value ToJson() const;
// modification

4
src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp
View File

@ -35,7 +35,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// - json: an array of Json::Value's to deserialize into our ActionMap.
// Return value:
// - a list of warnings encountered while deserializing the json
std::vector<SettingsLoadWarnings> ActionMap::LayerJson(const Json::Value& json)
std::vector<SettingsLoadWarnings> ActionMap::LayerJson(const Json::Value& json, const bool withKeybindings)
{
// It's possible that the user provided keybindings have some warnings in
// them - problems that we should alert the user to, but we can recover
@ -50,7 +50,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
continue;
}
AddAction(*Command::FromJson(cmdJson, warnings));
AddAction(*Command::FromJson(cmdJson, warnings, withKeybindings));
}
return warnings;

15
src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp
View File

@ -629,7 +629,7 @@ void SettingsLoader::_parse(const OriginTag origin, const winrt::hstring& source
// schemes and profiles. Additionally this function supports profiles which specify an "updates" key.
void SettingsLoader::_parseFragment(const winrt::hstring& source, const std::string_view& content, ParsedSettings& settings)
{
const auto json = _parseJson(content);
auto json = _parseJson(content);
settings.clear();
@ -647,6 +647,11 @@ void SettingsLoader::_parseFragment(const winrt::hstring& source, const std::str
}
CATCH_LOG()
}
// Parse out actions from the fragment. Manually opt-out of keybinding
// parsing - fragments shouldn't be allowed to bind actions to keys
// directly. We may want to revisit circa GH#2205
settings.globals->LayerActionsFrom(json.root, false);
}
{
@ -688,10 +693,10 @@ void SettingsLoader::_parseFragment(const winrt::hstring& source, const std::str
}
}
for (const auto& kv : settings.globals->ColorSchemes())
{
userSettings.globals->AddColorScheme(kv.Value());
}
// Add the parsed fragment globals as a parent of the user's settings.
// Later, in FinalizeInheritance, this will result in the action map from
// the fragments being applied before the user's own settings.
userSettings.globals->AddLeastImportantParent(settings.globals);
}
SettingsLoader::JsonSettings SettingsLoader::_parseJson(const std::string_view& content)

30
src/cascadia/TerminalSettingsModel/Command.cpp
View File

@ -258,7 +258,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// Return Value:
// - the newly constructed Command object.
winrt::com_ptr<Command> Command::FromJson(const Json::Value& json,
std::vector<SettingsLoadWarnings>& warnings)
std::vector<SettingsLoadWarnings>& warnings,
const bool parseKeys)
{
auto result = winrt::make_self<Command>();
@ -313,20 +314,23 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
result->_ActionAndArgs = make<implementation::ActionAndArgs>();
}
// GH#4239 - If the user provided more than one key
// chord to a "keys" array, warn the user here.
// TODO: GH#1334 - remove this check.
const auto keysJson{ json[JsonKey(KeysKey)] };
if (keysJson.isArray() && keysJson.size() > 1)
if (parseKeys)
{
warnings.push_back(SettingsLoadWarnings::TooManyKeysForChord);
}
else
{
Control::KeyChord keys{ nullptr };
if (JsonUtils::GetValueForKey(json, KeysKey, keys))
// GH#4239 - If the user provided more than one key
// chord to a "keys" array, warn the user here.
// TODO: GH#1334 - remove this check.
const auto keysJson{ json[JsonKey(KeysKey)] };
if (keysJson.isArray() && keysJson.size() > 1)
{
result->RegisterKey(keys);
warnings.push_back(SettingsLoadWarnings::TooManyKeysForChord);
}
else
{
Control::KeyChord keys{ nullptr };
if (JsonUtils::GetValueForKey(json, KeysKey, keys))
{
result->RegisterKey(keys);
}
}
}
}

3
src/cascadia/TerminalSettingsModel/Command.h
View File

@ -39,7 +39,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
com_ptr<Command> Copy() const;
static winrt::com_ptr<Command> FromJson(const Json::Value& json,
std::vector<SettingsLoadWarnings>& warnings);
std::vector<SettingsLoadWarnings>& warnings,
const bool parseKeys = true);
static void ExpandCommands(Windows::Foundation::Collections::IMap<winrt::hstring, Model::Command>& commands,
Windows::Foundation::Collections::IVectorView<Model::Profile> profiles,

11
src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp
View File

@ -142,12 +142,19 @@ void GlobalAppSettings::LayerJson(const Json::Value& json)
MTSM_GLOBAL_SETTINGS(GLOBAL_SETTINGS_LAYER_JSON)
#undef GLOBAL_SETTINGS_LAYER_JSON
LayerActionsFrom(json, true);
JsonUtils::GetValueForKey(json, LegacyReloadEnvironmentVariablesKey, _legacyReloadEnvironmentVariables);
}
void GlobalAppSettings::LayerActionsFrom(const Json::Value& json, const bool withKeybindings)
{
static constexpr std::array bindingsKeys{ LegacyKeybindingsKey, ActionsKey };
for (const auto& jsonKey : bindingsKeys)
{
if (auto bindings{ json[JsonKey(jsonKey)] })
{
auto warnings = _actionMap->LayerJson(bindings);
auto warnings = _actionMap->LayerJson(bindings, withKeybindings);
// It's possible that the user provided keybindings have some warnings
// in them - problems that we should alert the user to, but we can
@ -158,8 +165,6 @@ void GlobalAppSettings::LayerJson(const Json::Value& json)
_keybindingsWarnings.insert(_keybindingsWarnings.end(), warnings.begin(), warnings.end());
}
}
JsonUtils::GetValueForKey(json, LegacyReloadEnvironmentVariablesKey, _legacyReloadEnvironmentVariables);
}
// Method Description:

1
src/cascadia/TerminalSettingsModel/GlobalAppSettings.h
View File

@ -49,6 +49,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
static com_ptr<GlobalAppSettings> FromJson(const Json::Value& json);
void LayerJson(const Json::Value& json);
void LayerActionsFrom(const Json::Value& json, const bool withKeybindings = true);
Json::Value ToJson() const;