From 0b126f09cff6c6dbe704cab83f4f2cd8a3d1d306 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 10 Jun 2020 16:36:34 -0500 Subject: [PATCH] lets clean this up so it's not so horrifying. --- src/cascadia/TerminalApp/AppLogic.cpp | 3 +- src/cascadia/TerminalApp/CascadiaSettings.cpp | 18 ++++- src/cascadia/TerminalApp/Command.h | 3 +- .../TerminalApp/CommandSerialization.cpp | 79 +++++++++++++++---- .../Resources/en-US/Resources.resw | 4 + src/cascadia/TerminalApp/TerminalWarnings.h | 1 + 6 files changed, 89 insertions(+), 19 deletions(-) diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index f970b1a862..6ff8f93a6f 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -38,7 +38,8 @@ static const std::array(SettingsLoadWar USES_RESOURCE(L"AtLeastOneKeybindingWarning"), USES_RESOURCE(L"TooManyKeysForChord"), USES_RESOURCE(L"MissingRequiredParameter"), - USES_RESOURCE(L"LegacyGlobalsProperty") + USES_RESOURCE(L"LegacyGlobalsProperty"), + USES_RESOURCE(L"FailedToParseCommandJson") }; static const std::array(SettingsLoadErrors::ERRORS_SIZE)> settingsLoadErrorsLabels { USES_RESOURCE(L"NoProfilesText"), diff --git a/src/cascadia/TerminalApp/CascadiaSettings.cpp b/src/cascadia/TerminalApp/CascadiaSettings.cpp index 9f70a66b2d..a6866a54a5 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.cpp +++ b/src/cascadia/TerminalApp/CascadiaSettings.cpp @@ -721,17 +721,31 @@ std::string CascadiaSettings::_ApplyFirstRunChangesToSettingsTemplate(std::strin return finalSettings; } +// Method Description: +// - Expands any commands with `iterateOn` set. If we successfully generated +// expanded commands for them, then we'll remove the original command, and add +// all the newly generated commands. +// Arguments: +// - +// Return Value: +// - void CascadiaSettings::_ExpandCommands() { std::vector commandsToRemove; std::vector commandsToAdd; + // Any commands that return a non-empty vector from ExpandCommand should be + // replaced with all the commands that ExpandCommand returned. Since we + // can't modify the map while we're iterating over it, we'll do this in a + // couple steps. + + // First, collect up all the commands that need replacing. for (auto nameAndCmd : _globals.GetCommands()) { winrt::com_ptr cmd; cmd.copy_from(winrt::get_self(nameAndCmd.second)); - auto newCommands = winrt::TerminalApp::implementation::Command::ExpandCommand(cmd, _profiles); + auto newCommands = winrt::TerminalApp::implementation::Command::ExpandCommand(cmd, _profiles, _warnings); if (newCommands.size() > 0) { commandsToRemove.push_back(nameAndCmd.first); @@ -739,11 +753,13 @@ void CascadiaSettings::_ExpandCommands() } } + // Second, remove all the commands that need to be removed. for (auto& name : commandsToRemove) { _globals.GetCommands().erase(name); } + // Finally, add all the new commands. for (auto& cmd : commandsToAdd) { _globals.GetCommands().insert_or_assign(cmd.Name(), cmd); diff --git a/src/cascadia/TerminalApp/Command.h b/src/cascadia/TerminalApp/Command.h index dca954a959..cabccfd4e0 100644 --- a/src/cascadia/TerminalApp/Command.h +++ b/src/cascadia/TerminalApp/Command.h @@ -25,7 +25,8 @@ namespace winrt::TerminalApp::implementation const Json::Value& json); static std::vector ExpandCommand(winrt::com_ptr expandable, - const std::vector<::TerminalApp::Profile>& profiles); + const std::vector<::TerminalApp::Profile>& profiles, + std::vector<::TerminalApp::SettingsLoadWarnings>& warnings); WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler); OBSERVABLE_GETSET_PROPERTY(winrt::hstring, Name, _PropertyChangedHandlers); diff --git a/src/cascadia/TerminalApp/CommandSerialization.cpp b/src/cascadia/TerminalApp/CommandSerialization.cpp index 062fc4e704..bcc5897726 100644 --- a/src/cascadia/TerminalApp/CommandSerialization.cpp +++ b/src/cascadia/TerminalApp/CommandSerialization.cpp @@ -18,6 +18,8 @@ static constexpr std::string_view IterateOnKey{ "iterateOn" }; static constexpr std::string_view IterateOnProfilesValue{ "profiles" }; +static constexpr std::wstring_view ProfileName{ L"{$profile.name}" }; + namespace winrt::TerminalApp::implementation { // Function Description: @@ -60,6 +62,16 @@ namespace winrt::TerminalApp::implementation return L""; } + // Function Description: + // - Attempt to get the action of this Command from the provided json + // object. Will return nullptr if there's no "action" property, otherwise + // returns the result of ActionAndArgs::FromJson. + // Arguments: + // - json: The Json::Value representing the command object we should get the action for. + // - warnings: If there were any warnings during parsing, they'll be + // appended to this vector. + // Return Value: + // - the nullptr if we couldn't find an "action" or failed to parse the action, otherwise the parsed action winrt::com_ptr _getActionAndArgsFromJson(const Json::Value& json, std::vector<::TerminalApp::SettingsLoadWarnings>& warnings) { @@ -185,6 +197,8 @@ namespace winrt::TerminalApp::implementation return warnings; } + // Function Description: + // - A helper to replace any occurences of `keyword` with `replaceWith` in `sourceString` winrt::hstring _replaceKeyword(const winrt::hstring& sourceString, const std::wstring_view keyword, const std::wstring_view replaceWith) @@ -204,47 +218,80 @@ namespace winrt::TerminalApp::implementation return winrt::hstring{ result }; } + // Function Description: + // - Attempts to expand the given command into many commands, if the command + // has `"iterateOn": "profiles"` set. + // - If it doesn't, this function will do + // nothing and return an empty vector. + // - If it does, we're going to attempt to build a new set of commands using + // the given command as a prototype. We'll attempt to create a new command + // for each and every profile, to replace the original command. + // * For the new commands, we'll replace any instance of "${profile.name}" + // in the original json used to create this action with the name of the + // given profile. + // - If we encounter any errors while re-parsing the json with the replaced + // name, we'll just return immediately. + // - At the end, we'll return all the new commands we've build for the given command. + // Arguments: + // - expandable: the Command to potentially turn into more commands + // - profiles: A list of all the profiles that this command should be expanded on. + // - warnings: If there were any warnings during parsing, they'll be + // appended to this vector. + // Return Value: + // - and empty vector if the command wasn't expandable, otherwise a list of + // the newly-created commands. std::vector Command::ExpandCommand(winrt::com_ptr expandable, - const std::vector<::TerminalApp::Profile>& profiles) + const std::vector<::TerminalApp::Profile>& profiles, + std::vector<::TerminalApp::SettingsLoadWarnings>& warnings) { - std::vector<::TerminalApp::SettingsLoadWarnings> warnings; std::vector newCommands; + if (expandable->_IterateOn == ExpandCommandType::None) + { + return newCommands; + } + + std::string errs; // This string will receive any error text from failing to parse. + std::unique_ptr reader{ Json::CharReaderBuilder::CharReaderBuilder().newCharReader() }; + if (expandable->_IterateOn == ExpandCommandType::Profiles) { - // commands.erase(expandable->_Name); - for (const auto& p : profiles) { + // For each profile, create a new command. This command will have: + // * the icon path and keychord text of the original command + // * the Name will have any instances of "${profile.name}" + // replaced with the profile's name + // * for the action, we'll take the original json, replace any + // instances of "${profile.name}" with the profile's name, + // then re-attempt to parse the action and args. + // auto newCmd = winrt::make_self(); newCmd->_setIconPath(expandable->_IconPath); newCmd->_setKeyChordText(expandable->_KeyChordText); newCmd->_setName(_replaceKeyword(expandable->_Name, - L"${profile.name}", + ProfileName, p.GetName())); // Replace all the keywords in the original json, and try and parse that auto oldJsonString = winrt::to_hstring(expandable->_originalJson.toStyledString()); + + // TODO: We need to escape the profile name for JSON appropriately auto newJsonString = winrt::to_string(_replaceKeyword(oldJsonString, - L"${profile.name}", + ProfileName, p.GetName())); - Json::Value newJsonValue; // = Json::Value{newJsonString}; - - std::string errs; // This string will receive any error text from failing to parse. - std::unique_ptr reader{ Json::CharReaderBuilder::CharReaderBuilder().newCharReader() }; - auto actualDataStart = newJsonString.data(); + Json::Value newJsonValue; + const auto actualDataStart = newJsonString.data(); const auto actualDataEnd = newJsonString.data() + newJsonString.size(); if (!reader->parse(actualDataStart, actualDataEnd, &newJsonValue, &errs)) { - // This will be caught by App::_TryLoadSettings, who will display - // the text to the user. - throw winrt::hresult_error(WEB_E_INVALID_JSON_STRING, winrt::to_hstring(errs)); + warnings.push_back(::TerminalApp::SettingsLoadWarnings::FailedToParseCommandJson); + // If we encounter a re-parsing error, just stop processing the rest of the commands. + break; } auto actionAndArgs = _getActionAndArgsFromJson(newJsonValue, warnings); - - // newCmd->_setAction(expandable._Action); if (actionAndArgs && !newCmd->_Name.empty()) { newCmd->_setAction(*actionAndArgs); diff --git a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw index 8a7b427160..1af8421836 100644 --- a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw @@ -222,6 +222,10 @@ For more info, see this web page. + + The "globals" property is deprecated - your settings might need updating. + {Locked="\"globals\""} + An optional command, with arguments, to be spawned in the new tab or pane diff --git a/src/cascadia/TerminalApp/TerminalWarnings.h b/src/cascadia/TerminalApp/TerminalWarnings.h index aa0bb1b869..3b0be2b402 100644 --- a/src/cascadia/TerminalApp/TerminalWarnings.h +++ b/src/cascadia/TerminalApp/TerminalWarnings.h @@ -30,6 +30,7 @@ namespace TerminalApp TooManyKeysForChord = 6, MissingRequiredParameter = 7, LegacyGlobalsProperty = 8, + FailedToParseCommandJson = 9, WARNINGS_SIZE // IMPORTANT: This MUST be the last value in this enum. It's an unused placeholder. };