Skip to content

Commit

Permalink
more tests, code cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@zadjii-msft
zadjii-msft committed Jul 9, 2020
1 parent 92167a5 commit 7968b2a
Show file tree
Hide file tree
Showing 3 changed files with 275 additions and 38 deletions.
245 changes: 234 additions & 11 deletions src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ namespace TerminalAppLocalTests
TEST_METHOD(TestIterableInNestedCommand);
TEST_METHOD(TestMixedNestedAndIterableCommand);
TEST_METHOD(TestNestedCommandWithoutName);
TEST_METHOD(TestUnbindNestedCommand);
TEST_METHOD(TestRebindNestedCommand);

TEST_CLASS_SETUP(ClassSetup)
{
Expand All @@ -101,7 +103,10 @@ namespace TerminalAppLocalTests
private:
void _logCommandNames(std::unordered_map<winrt::hstring, winrt::TerminalApp::Command>& commands, const int indentation = 1)
{
Log::Comment(L"Commands:");
if (indentation == 1)
{
Log::Comment(commands.empty() ? L"Commands:\n <none>" : L"Commands:");
}
for (auto& nameAndCommand : commands)
{
Log::Comment(fmt::format(L"{0:>{1}}* {2}->{3}",
Expand Down Expand Up @@ -2957,9 +2962,7 @@ namespace TerminalAppLocalTests

void SettingsTests::TestNestedCommands()
{
// This test TODODODOD

// This test checks a iterable command that includes a nested command.
// This test checks a nested command.
// The commands should look like:
//
// <Command Palette>
Expand Down Expand Up @@ -3060,15 +3063,14 @@ namespace TerminalAppLocalTests

void SettingsTests::TestNestedInNestedCommand()
{
// This test TODODODOD

// This test checks a iterable command that includes a nested command.
// This test checks a nested command that includes nested commands.
// The commands should look like:
//
// <Command Palette>
// └─ Connect to ssh...
// ├─ first.com
// └─ second.com
// └─ grandparent
// └─ parent
// ├─ child1
// └─ child2

const std::string settingsJson{ R"(
{
Expand Down Expand Up @@ -3629,12 +3631,233 @@ namespace TerminalAppLocalTests
}
}
}

void SettingsTests::TestNestedCommandWithoutName()
{
// This test tests a nested command without a name specified. This type
// of command should just be ignored, since we can't auto-generate names
// for nested commands, they _must_ have names specified.
Log::Result(WEX::Logging::TestResults::Skipped);

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"
}
],
"bindings": [
{
"commands": [
{
"name": "child1",
"command": { "action": "newTab", "commandline": "ssh me@first.com" }
},
{
"name": "child2",
"command": { "action": "newTab", "commandline": "ssh me@second.com" }
}
]
},
],
"schemes": [ { "name": "Campbell" } ] // This is included here to prevent settings validation errors.
})" };

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();
settings._ValidateSettings();
_logCommandNames(commands);

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

// Because the "parent" command didn't have a name, it couldn't be
// placed into the list of commands. It and it's children are just
// ignored.
VERIFY_ARE_EQUAL(0u, commands.size());
}

void SettingsTests::TestUnbindNestedCommand()
{
// TODO: description

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"
}
],
"bindings": [
{
"name": "parent",
"commands": [
{
"name": "child1",
"command": { "action": "newTab", "commandline": "ssh me@first.com" }
},
{
"name": "child2",
"command": { "action": "newTab", "commandline": "ssh me@second.com" }
}
]
},
],
"schemes": [ { "name": "Campbell" } ] // This is included here to prevent settings validation errors.
})" };

const std::string settings1Json{ R"(
{
"defaultProfile": "{6239a42c-0000-49a3-80bd-e8fdd045185c}",
"bindings": [
{
"name": "parent",
"commands": null
},
],
})" };

VerifyParseSucceeded(settingsJson);
VerifyParseSucceeded(settings1Json);

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();
settings._ValidateSettings();
_logCommandNames(commands);

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

Log::Comment(L"Layer second bit of json, to unbind the original command.");

settings._ParseJsonString(settings1Json, false);
settings.LayerJson(settings._userSettings);
settings._ValidateSettings();
_logCommandNames(commands);
VERIFY_ARE_EQUAL(0u, settings._warnings.size());
VERIFY_ARE_EQUAL(0u, commands.size());
}

void SettingsTests::TestRebindNestedCommand()
{
// TODO: description

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"
}
],
"bindings": [
{
"name": "parent",
"commands": [
{
"name": "child1",
"command": { "action": "newTab", "commandline": "ssh me@first.com" }
},
{
"name": "child2",
"command": { "action": "newTab", "commandline": "ssh me@second.com" }
}
]
},
],
"schemes": [ { "name": "Campbell" } ] // This is included here to prevent settings validation errors.
})" };

const std::string settings1Json{ R"(
{
"defaultProfile": "{6239a42c-0000-49a3-80bd-e8fdd045185c}",
"bindings": [
{
"name": "parent",
"command": "newTab"
},
],
})" };

VerifyParseSucceeded(settingsJson);
VerifyParseSucceeded(settings1Json);

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();
settings._ValidateSettings();
_logCommandNames(commands);

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

Log::Comment(L"Layer second bit of json, to unbind the original command.");
settings._ParseJsonString(settings1Json, false);
settings.LayerJson(settings._userSettings);
settings._ValidateSettings();
_logCommandNames(commands);
VERIFY_ARE_EQUAL(0u, settings._warnings.size());
VERIFY_ARE_EQUAL(1u, commands.size());
}

}
17 changes: 6 additions & 11 deletions src/cascadia/TerminalApp/Command.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,12 @@ namespace winrt::TerminalApp::implementation

nested = true;
}

// TODO: else if (hasKey(CommandKey) )
// {
// // { name: "foo", commands: null } will land in this case, which
// // should also be used for unbinding.
// return nullptr;
// }
else if (json.isMember(JsonKey(CommandsKey)))
{
// { "name": "foo", "commands": null } will land in this case, which
// should also be used for unbinding.
return nullptr;
}

// TODO GH#6644: iconPath not implemented quite yet. Can't seem to get
// the binding quite right. Additionally, do we want it to be an image,
Expand Down Expand Up @@ -443,10 +442,6 @@ namespace winrt::TerminalApp::implementation
{
warnings.push_back(::TerminalApp::SettingsLoadWarnings::FailedToParseCommandJson);
// If we encounter a re-parsing error, just stop processing the rest of the commands.

// TODO: if we fail to expand the json, we should return NO
// commands, and also remove the current command from the
// list of commands.
break;
}

Expand Down
51 changes: 35 additions & 16 deletions src/cascadia/TerminalApp/TerminalPage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,37 @@ namespace winrt::TerminalApp::implementation
InitializeComponent();
}

// Function Description:
// - Recursively check our commands to see if there's a keybinding for
// exactly their action. If there is, label that command with the text
// corresponding to that key chord.
// - Will recurse into nested commands as well.
// Arguments:
// - settings: The settings who's keybindings we should use to look up the key chords from
// - commands: The list fo commands to label.
// Return Value:
// - <none>
static void _recursiveUpdateCommandKeybindingLabels(std::shared_ptr<::TerminalApp::CascadiaSettings> settings,
Windows::Foundation::Collections::IVector<TerminalApp::Command> const& commands)
{
for (auto command : commands)
{
// If there's a keybinding that's bound to exactly this command,
// then get the string for that keychord and display it as a
// part of the command in the UI. Each Command's KeyChordText is
// unset by default, so we don't need to worry about clearing it
// if there isn't a key associated with it.
auto keyChord{ settings->GetKeybindings().GetKeyBindingForActionWithArgs(command.Action()) };

if (keyChord)
{
command.KeyChordText(KeyChordSerialization::ToString(keyChord));
}

_recursiveUpdateCommandKeybindingLabels(settings, command.NestedCommands());
}
}

winrt::fire_and_forget TerminalPage::SetSettings(std::shared_ptr<::TerminalApp::CascadiaSettings> settings,
bool needRefreshUI)
{
Expand All @@ -97,23 +128,11 @@ namespace winrt::TerminalApp::implementation
auto commandsCollection = winrt::single_threaded_vector<winrt::TerminalApp::Command>();
for (auto& nameAndCommand : _settings->GlobalSettings().GetCommands())
{
auto command = nameAndCommand.second;

// If there's a keybinding that's bound to exactly this command,
// then get the string for that keychord and display it as a
// part of the command in the UI. Each Command's KeyChordText is
// unset by default, so we don't need to worry about clearing it
// if there isn't a key associated with it.
auto keyChord{ _settings->GetKeybindings().GetKeyBindingForActionWithArgs(command.Action()) };

// TODO: Make this recursive, because commands might have
// subcommands that also have keybindings
if (keyChord)
{
command.KeyChordText(KeyChordSerialization::ToString(keyChord));
}
commandsCollection.Append(command);
commandsCollection.Append(nameAndCommand.second);
}

_recursiveUpdateCommandKeybindingLabels(_settings, commandsCollection);

CommandPalette().SetActions(commandsCollection);
}
}
Expand Down

1 comment on commit 7968b2a

@github-actions

This comment was marked as resolved.

Sign in to view

Please sign in to comment.