Skip to content

Add support for commands iterable on color schemes #7329

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
6 commits merged into from
Aug 19, 2020
Merged

Add support for commands iterable on color schemes #7329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3b34451
This is the implementation, now I need tests
zadjii-msft Aug 14, 2020
ab9c4cd
fix the tests
zadjii-msft Aug 14, 2020
e3a7727
Add a test for this too
zadjii-msft Aug 14, 2020
f2fd2c4
good bot
zadjii-msft Aug 18, 2020
765c6b2
Dustin's got nits and I've got fixes
zadjii-msft Aug 18, 2020
020d8f5
polish for Dustin
zadjii-msft Aug 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 147 additions & 8 deletions src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ namespace TerminalAppLocalTests
TEST_METHOD(TestUnbindNestedCommand);
TEST_METHOD(TestRebindNestedCommand);

TEST_METHOD(TestIterableColorSchemeCommands);

TEST_CLASS_SETUP(ClassSetup)
{
InitializeJsonReader();
Expand Down Expand Up @@ -2685,7 +2687,7 @@ namespace TerminalAppLocalTests
VERIFY_ARE_EQUAL(L"${profile.name}", realArgs.TerminalArgs().Profile());
}

auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles());
auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles(), settings._globals.GetColorSchemes());
_logCommandNames(expandedCommands);

VERIFY_ARE_EQUAL(0u, settings._warnings.size());
Expand Down Expand Up @@ -2815,7 +2817,7 @@ namespace TerminalAppLocalTests
VERIFY_ARE_EQUAL(L"${profile.name}", realArgs.TerminalArgs().Profile());
}

auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles());
auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles(), settings._globals.GetColorSchemes());
_logCommandNames(expandedCommands);

VERIFY_ARE_EQUAL(0u, settings._warnings.size());
Expand Down Expand Up @@ -2948,7 +2950,7 @@ namespace TerminalAppLocalTests
}

settings._ValidateSettings();
auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles());
auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles(), settings._globals.GetColorSchemes());
_logCommandNames(expandedCommands);

VERIFY_ARE_EQUAL(0u, settings._warnings.size());
Expand Down Expand Up @@ -3069,7 +3071,7 @@ namespace TerminalAppLocalTests

auto& commands = settings._globals.GetCommands();
settings._ValidateSettings();
auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles());
auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles(), settings._globals.GetColorSchemes());
_logCommandNames(expandedCommands);

VERIFY_ARE_EQUAL(0u, settings._warnings.size());
Expand Down Expand Up @@ -3177,7 +3179,7 @@ namespace TerminalAppLocalTests

auto& commands = settings._globals.GetCommands();
settings._ValidateSettings();
auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles());
auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles(), settings._globals.GetColorSchemes());
_logCommandNames(expandedCommands);

VERIFY_ARE_EQUAL(0u, settings._warnings.size());
Expand Down Expand Up @@ -3316,7 +3318,7 @@ namespace TerminalAppLocalTests

auto& commands = settings._globals.GetCommands();
settings._ValidateSettings();
auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles());
auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles(), settings._globals.GetColorSchemes());
_logCommandNames(expandedCommands);

VERIFY_ARE_EQUAL(0u, settings._warnings.size());
Expand Down Expand Up @@ -3469,7 +3471,7 @@ namespace TerminalAppLocalTests

auto& commands = settings._globals.GetCommands();
settings._ValidateSettings();
auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles());
auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles(), settings._globals.GetColorSchemes());
_logCommandNames(expandedCommands);

VERIFY_ARE_EQUAL(0u, settings._warnings.size());
Expand Down Expand Up @@ -3582,7 +3584,7 @@ namespace TerminalAppLocalTests

auto& commands = settings._globals.GetCommands();
settings._ValidateSettings();
auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles());
auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles(), settings._globals.GetColorSchemes());
_logCommandNames(expandedCommands);

VERIFY_ARE_EQUAL(0u, settings._warnings.size());
Expand Down Expand Up @@ -3946,4 +3948,141 @@ namespace TerminalAppLocalTests
}
}

void SettingsTests::TestIterableColorSchemeCommands()
{
// For this test, put an iterable command with a given `name`,
// containing a ${profile.name} to replace. When we expand it, it should
// have created one command for each profile.

const std::string settingsJson{ R"(
{
"defaultProfile": "{6239a42c-0000-49a3-80bd-e8fdd045185c}",
"profiles": [
{
"name": "profile0",
"guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}",
"historySize": 1,
"commandline": "cmd.exe"
},
{
"name": "profile1",
"guid": "{6239a42c-1111-49a3-80bd-e8fdd045185c}",
"historySize": 2,
"commandline": "pwsh.exe"
},
{
"name": "profile2",
"historySize": 3,
"commandline": "wsl.exe"
}
],
"schemes": [
{ "name": "scheme_0" },
{ "name": "scheme_1" },
{ "name": "scheme_2" },
],
"bindings": [
{
"name": "iterable command ${scheme.name}",
"iterateOn": "schemes",
"command": { "action": "splitPane", "profile": "${scheme.name}" }

@DHowett Dustin L. Howett (DHowett) Aug 18, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL, well, i mean.. it's effective for sure

},
]
})" };

VerifyParseSucceeded(settingsJson);
CascadiaSettings settings{};
settings._ParseJsonString(settingsJson, false);
settings.LayerJson(settings._userSettings);

VERIFY_ARE_EQUAL(0u, settings._warnings.size());

VERIFY_ARE_EQUAL(3u, settings.GetProfiles().size());

auto& commands = settings._globals.GetCommands();
VERIFY_ARE_EQUAL(1u, commands.Size());

{
auto command = commands.Lookup(L"iterable command ${scheme.name}");
VERIFY_IS_NOT_NULL(command);
auto actionAndArgs = command.Action();
VERIFY_IS_NOT_NULL(actionAndArgs);
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(winrt::TerminalApp::SplitState::Automatic, realArgs.SplitStyle());
VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
VERIFY_IS_TRUE(realArgs.TerminalArgs().Commandline().empty());
VERIFY_IS_TRUE(realArgs.TerminalArgs().StartingDirectory().empty());
VERIFY_IS_TRUE(realArgs.TerminalArgs().TabTitle().empty());
VERIFY_IS_FALSE(realArgs.TerminalArgs().Profile().empty());
VERIFY_ARE_EQUAL(L"${scheme.name}", realArgs.TerminalArgs().Profile());
}

auto expandedCommands = implementation::TerminalPage::_ExpandCommands(commands.GetView(), settings.GetProfiles(), settings._globals.GetColorSchemes());
_logCommandNames(expandedCommands);

VERIFY_ARE_EQUAL(0u, settings._warnings.size());
VERIFY_ARE_EQUAL(3u, expandedCommands.Size());

// Yes, this test is testing splitPane with profiles named after each
// color scheme. These would obviously not work in real life, they're
// just easy tests to write.

{
auto command = expandedCommands.Lookup(L"iterable command scheme_0");
VERIFY_IS_NOT_NULL(command);
auto actionAndArgs = command.Action();
VERIFY_IS_NOT_NULL(actionAndArgs);
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(winrt::TerminalApp::SplitState::Automatic, realArgs.SplitStyle());
VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
VERIFY_IS_TRUE(realArgs.TerminalArgs().Commandline().empty());
VERIFY_IS_TRUE(realArgs.TerminalArgs().StartingDirectory().empty());
VERIFY_IS_TRUE(realArgs.TerminalArgs().TabTitle().empty());
VERIFY_IS_FALSE(realArgs.TerminalArgs().Profile().empty());
VERIFY_ARE_EQUAL(L"scheme_0", realArgs.TerminalArgs().Profile());
}

{
auto command = expandedCommands.Lookup(L"iterable command scheme_1");
VERIFY_IS_NOT_NULL(command);
auto actionAndArgs = command.Action();
VERIFY_IS_NOT_NULL(actionAndArgs);
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(winrt::TerminalApp::SplitState::Automatic, realArgs.SplitStyle());
VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
VERIFY_IS_TRUE(realArgs.TerminalArgs().Commandline().empty());
VERIFY_IS_TRUE(realArgs.TerminalArgs().StartingDirectory().empty());
VERIFY_IS_TRUE(realArgs.TerminalArgs().TabTitle().empty());
VERIFY_IS_FALSE(realArgs.TerminalArgs().Profile().empty());
VERIFY_ARE_EQUAL(L"scheme_1", realArgs.TerminalArgs().Profile());
}

{
auto command = expandedCommands.Lookup(L"iterable command scheme_2");
VERIFY_IS_NOT_NULL(command);
auto actionAndArgs = command.Action();
VERIFY_IS_NOT_NULL(actionAndArgs);
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(winrt::TerminalApp::SplitState::Automatic, realArgs.SplitStyle());
VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
VERIFY_IS_TRUE(realArgs.TerminalArgs().Commandline().empty());
VERIFY_IS_TRUE(realArgs.TerminalArgs().StartingDirectory().empty());
VERIFY_IS_TRUE(realArgs.TerminalArgs().TabTitle().empty());
VERIFY_IS_FALSE(realArgs.TerminalArgs().Profile().empty());
VERIFY_ARE_EQUAL(L"scheme_2", realArgs.TerminalArgs().Profile());
}
}

}
74 changes: 50 additions & 24 deletions src/cascadia/TerminalApp/Command.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "ActionAndArgs.h"
#include "JsonUtils.h"
#include <LibraryResources.h>
#include "TerminalSettingsSerializationHelpers.h"

using namespace winrt::TerminalApp;
using namespace winrt::Windows::Foundation;
Expand All @@ -21,9 +22,8 @@ static constexpr std::string_view ArgsKey{ "args" };
static constexpr std::string_view IterateOnKey{ "iterateOn" };
static constexpr std::string_view CommandsKey{ "commands" };

static constexpr std::string_view IterateOnProfilesValue{ "profiles" };

static constexpr std::string_view ProfileName{ "${profile.name}" };
static constexpr std::string_view ProfileNameToken{ "${profile.name}" };
static constexpr std::string_view SchemeNameToken{ "${scheme.name}" };

namespace winrt::TerminalApp::implementation
{
Expand Down Expand Up @@ -121,14 +121,7 @@ namespace winrt::TerminalApp::implementation
auto result = winrt::make_self<Command>();

bool nested = false;
if (const auto iterateOnJson{ json[JsonKey(IterateOnKey)] })
{
auto s = iterateOnJson.asString();
if (s == IterateOnProfilesValue)
{
result->_IterateOn = ExpandCommandType::Profiles;
}
}
JsonUtils::GetValueForKey(json, IterateOnKey, result->_IterateOn);

// For iterable commands, we'll make another pass at parsing them once
// the json is patched. So ignore parsing sub-commands for now. Commands
Expand Down Expand Up @@ -290,6 +283,7 @@ namespace winrt::TerminalApp::implementation
// - <none>
void Command::ExpandCommands(Windows::Foundation::Collections::IMap<winrt::hstring, winrt::TerminalApp::Command>& commands,
gsl::span<const ::TerminalApp::Profile> profiles,
gsl::span<winrt::TerminalApp::ColorScheme> schemes,
std::vector<::TerminalApp::SettingsLoadWarnings>& warnings)
{
std::vector<winrt::hstring> commandsToRemove;
Expand All @@ -300,7 +294,7 @@ namespace winrt::TerminalApp::implementation
{
auto cmd{ get_self<implementation::Command>(nameAndCmd.Value()) };

auto newCommands = _expandCommand(cmd, profiles, warnings);
auto newCommands = _expandCommand(cmd, profiles, schemes, warnings);
if (newCommands.size() > 0)
{
commandsToRemove.push_back(nameAndCmd.Key());
Expand Down Expand Up @@ -345,13 +339,14 @@ namespace winrt::TerminalApp::implementation
// the newly-created commands.
std::vector<winrt::TerminalApp::Command> Command::_expandCommand(Command* const expandable,
gsl::span<const ::TerminalApp::Profile> profiles,
gsl::span<winrt::TerminalApp::ColorScheme> schemes,
std::vector<::TerminalApp::SettingsLoadWarnings>& warnings)
{
std::vector<winrt::TerminalApp::Command> newCommands;

if (expandable->HasNestedCommands())
{
ExpandCommands(expandable->_subcommands, profiles, warnings);
ExpandCommands(expandable->_subcommands, profiles, schemes, warnings);
}

if (expandable->_IterateOn == ExpandCommandType::None)
Expand All @@ -365,6 +360,26 @@ namespace winrt::TerminalApp::implementation
// First, get a string for the original Json::Value
auto oldJsonString = expandable->_originalJson.toStyledString();

auto reParseJson = [&](const auto& newJsonString) -> bool {
// - Now, re-parse the modified value.
Json::Value newJsonValue;
const auto actualDataStart = newJsonString.data();
const auto actualDataEnd = newJsonString.data() + newJsonString.size();
if (!reader->parse(actualDataStart, actualDataEnd, &newJsonValue, &errs))
{
warnings.push_back(::TerminalApp::SettingsLoadWarnings::FailedToParseCommandJson);
// If we encounter a re-parsing error, just stop processing the rest of the commands.
return false;
}

// Pass the new json back though FromJson, to get the new expanded value.
if (auto newCmd{ Command::FromJson(newJsonValue, warnings) })
{
newCommands.push_back(*newCmd);
}
return true;
};

if (expandable->_IterateOn == ExpandCommandType::Profiles)
{
for (const auto& p : profiles)
Expand All @@ -382,24 +397,35 @@ namespace winrt::TerminalApp::implementation
// - Escape the profile name for JSON appropriately
auto escapedProfileName = _escapeForJson(til::u16u8(p.GetName()));
auto newJsonString = til::replace_needle_in_haystack(oldJsonString,
ProfileName,
ProfileNameToken,
escapedProfileName);

// - Now, re-parse the modified value.
Json::Value newJsonValue;
const auto actualDataStart = newJsonString.data();
const auto actualDataEnd = newJsonString.data() + newJsonString.size();
if (!reader->parse(actualDataStart, actualDataEnd, &newJsonValue, &errs))
// If we encounter a re-parsing error, just stop processing the rest of the commands.
if (!reParseJson(newJsonString))
{
warnings.push_back(::TerminalApp::SettingsLoadWarnings::FailedToParseCommandJson);
// If we encounter a re-parsing error, just stop processing the rest of the commands.
break;
}
}
}
else if (expandable->_IterateOn == ExpandCommandType::ColorSchemes)
{
for (const auto& s : schemes)
{
// For each scheme, create a new command. We'll take the
// original json, replace any instances of "${scheme.name}" with
// the scheme's name, then re-attempt to parse the action and
// args.

// Pass the new json back though FromJson, to get the new expanded value.
if (auto newCmd{ Command::FromJson(newJsonValue, warnings) })
// - Escape the profile name for JSON appropriately
auto escapedSchemeName = _escapeForJson(til::u16u8(s.Name()));
auto newJsonString = til::replace_needle_in_haystack(oldJsonString,
SchemeNameToken,
escapedSchemeName);

// If we encounter a re-parsing error, just stop processing the rest of the commands.
if (!reParseJson(newJsonString))
{
newCommands.push_back(*newCmd);
break;
}
}
}
Expand Down
Loading