Skip to content

Commit

Permalink
Introduce ActionMap to Terminal Settings Model
Browse files Browse the repository at this point in the history
  • Loading branch information
@carlos-zamora
carlos-zamora committed Mar 25, 2021
1 parent 03b9a06 commit 54f17b8
Show file tree
Hide file tree
Showing 37 changed files with 953 additions and 847 deletions.
68 changes: 31 additions & 37 deletions src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1918,7 +1918,7 @@ namespace SettingsModelLocalTests
const auto settingsObject = VerifyParseSucceeded(badSettings);
auto settings = implementation::CascadiaSettings::FromJson(settingsObject);

VERIFY_ARE_EQUAL(0u, settings->_globals->_keymap->_keyShortcuts.size());
VERIFY_ARE_EQUAL(0u, settings->_globals->ActionMap().KeybindingCount());

VERIFY_ARE_EQUAL(4u, settings->_globals->_keybindingsWarnings.size());
VERIFY_ARE_EQUAL(SettingsLoadWarnings::TooManyKeysForChord, settings->_globals->_keybindingsWarnings.at(0));
Expand Down Expand Up @@ -1962,7 +1962,7 @@ namespace SettingsModelLocalTests

auto settings = implementation::CascadiaSettings::FromJson(settingsObject);

VERIFY_ARE_EQUAL(0u, settings->_globals->_keymap->_keyShortcuts.size());
VERIFY_ARE_EQUAL(0u, settings->_globals->ActionMap().KeybindingCount());

for (const auto& warning : settings->_globals->_keybindingsWarnings)
{
Expand Down Expand Up @@ -2103,18 +2103,17 @@ namespace SettingsModelLocalTests
const auto profile2Guid = settings->_allProfiles.GetAt(2).Guid();
VERIFY_ARE_NOT_EQUAL(winrt::guid{}, profile2Guid);

auto keymap = winrt::get_self<implementation::KeyMapping>(settings->_globals->KeyMap());
VERIFY_ARE_EQUAL(5u, keymap->_keyShortcuts.size());
auto actionMap = winrt::get_self<implementation::ActionMap>(settings->_globals->ActionMap());
VERIFY_ARE_EQUAL(5u, actionMap->KeybindingCount());

// A/D, B, C, E will be in the list of commands, for 4 total.
// * A and D share the same name, so they'll only generate a single action.
// * F's name is set manually to `null`
auto commands = settings->_globals->Commands();
VERIFY_ARE_EQUAL(4u, commands.Size());
VERIFY_ARE_EQUAL(4u, settings->_globals->ActionMap().CommandCount());

{
KeyChord kc{ true, false, false, static_cast<int32_t>('A') };
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc);
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc);
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
Expand All @@ -2131,7 +2130,7 @@ namespace SettingsModelLocalTests

{
KeyChord kc{ true, false, false, static_cast<int32_t>('C') };
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc);
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc);
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
Expand All @@ -2145,7 +2144,7 @@ namespace SettingsModelLocalTests
}
{
KeyChord kc{ true, false, false, static_cast<int32_t>('D') };
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc);
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc);
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
Expand All @@ -2159,7 +2158,7 @@ namespace SettingsModelLocalTests
}
{
KeyChord kc{ true, false, false, static_cast<int32_t>('E') };
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc);
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc);
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
Expand All @@ -2173,7 +2172,7 @@ namespace SettingsModelLocalTests
}
{
KeyChord kc{ true, false, false, static_cast<int32_t>('F') };
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc);
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc);
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
Expand All @@ -2187,9 +2186,9 @@ namespace SettingsModelLocalTests
}

Log::Comment(L"Now verify the commands");
_logCommandNames(commands);
_logCommandNames(actionMap->NameMap());
{
auto command = commands.Lookup(L"Split pane, split: vertical");
auto command = actionMap->GetActionByName(L"Split pane, split: vertical");
VERIFY_IS_NOT_NULL(command);
auto actionAndArgs = command.Action();
VERIFY_IS_NOT_NULL(actionAndArgs);
Expand All @@ -2205,7 +2204,7 @@ namespace SettingsModelLocalTests
VERIFY_IS_TRUE(realArgs.TerminalArgs().Profile().empty());
}
{
auto command = commands.Lookup(L"ctrl+b");
auto command = actionMap->GetActionByName(L"ctrl+b");
VERIFY_IS_NOT_NULL(command);
auto actionAndArgs = command.Action();
VERIFY_IS_NOT_NULL(actionAndArgs);
Expand All @@ -2221,7 +2220,7 @@ namespace SettingsModelLocalTests
VERIFY_IS_TRUE(realArgs.TerminalArgs().Profile().empty());
}
{
auto command = commands.Lookup(L"ctrl+c");
auto command = actionMap->GetActionByName(L"ctrl+c");
VERIFY_IS_NOT_NULL(command);
auto actionAndArgs = command.Action();
VERIFY_IS_NOT_NULL(actionAndArgs);
Expand All @@ -2237,7 +2236,7 @@ namespace SettingsModelLocalTests
VERIFY_IS_TRUE(realArgs.TerminalArgs().Profile().empty());
}
{
auto command = commands.Lookup(L"Split pane, split: horizontal");
auto command = actionMap->GetActionByName(L"Split pane, split: horizontal");
VERIFY_IS_NOT_NULL(command);
auto actionAndArgs = command.Action();
VERIFY_IS_NOT_NULL(actionAndArgs);
Expand Down Expand Up @@ -2308,16 +2307,15 @@ namespace SettingsModelLocalTests
VERIFY_ARE_EQUAL(0u, settings->_warnings.Size());
VERIFY_ARE_EQUAL(3u, settings->_allProfiles.Size());

auto commands = settings->_globals->Commands();
settings->_ValidateSettings();
_logCommandNames(commands);
_logCommandNames(settings->ActionMap().NameMap());

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());
VERIFY_ARE_EQUAL(0u, settings->_globals->ActionMap().CommandCount());
}

void DeserializationTests::TestNestedCommandWithBadSubCommands()
Expand Down Expand Up @@ -2358,13 +2356,12 @@ namespace SettingsModelLocalTests
auto settings = winrt::make_self<implementation::CascadiaSettings>();
settings->_ParseJsonString(settingsJson, false);
settings->LayerJson(settings->_userSettings);
auto commands = settings->_globals->Commands();
settings->_ValidateSettings();

VERIFY_ARE_EQUAL(2u, settings->_warnings.Size());
VERIFY_ARE_EQUAL(SettingsLoadWarnings::AtLeastOneKeybindingWarning, settings->_warnings.GetAt(0));
VERIFY_ARE_EQUAL(SettingsLoadWarnings::FailedToParseSubCommands, settings->_warnings.GetAt(1));
VERIFY_ARE_EQUAL(0u, commands.Size());
VERIFY_ARE_EQUAL(0u, settings->_globals->ActionMap().CommandCount());
}

void DeserializationTests::TestUnbindNestedCommand()
Expand Down Expand Up @@ -2432,22 +2429,20 @@ namespace SettingsModelLocalTests
VERIFY_ARE_EQUAL(0u, settings->_warnings.Size());
VERIFY_ARE_EQUAL(3u, settings->_allProfiles.Size());

auto commands = settings->_globals->Commands();
settings->_ValidateSettings();
_logCommandNames(commands);
_logCommandNames(settings->ActionMap().NameMap());

VERIFY_ARE_EQUAL(0u, settings->_warnings.Size());
VERIFY_ARE_EQUAL(1u, commands.Size());
VERIFY_ARE_EQUAL(1u, settings->_globals->ActionMap().CommandCount());

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

settings->_ParseJsonString(settings1Json, false);
settings->LayerJson(settings->_userSettings);
settings->_ValidateSettings();
commands = settings->_globals->Commands();
_logCommandNames(commands);
_logCommandNames(settings->ActionMap().NameMap());
VERIFY_ARE_EQUAL(0u, settings->_warnings.Size());
VERIFY_ARE_EQUAL(0u, commands.Size());
VERIFY_ARE_EQUAL(0u, settings->_globals->ActionMap().CommandCount());
}

void DeserializationTests::TestRebindNestedCommand()
Expand Down Expand Up @@ -2516,16 +2511,16 @@ namespace SettingsModelLocalTests
VERIFY_ARE_EQUAL(0u, settings->_warnings.Size());
VERIFY_ARE_EQUAL(3u, settings->_allProfiles.Size());

auto commands = settings->_globals->Commands();
const auto& actionMap{ settings->ActionMap() };
settings->_ValidateSettings();
_logCommandNames(commands);
_logCommandNames(actionMap.NameMap());

VERIFY_ARE_EQUAL(0u, settings->_warnings.Size());
VERIFY_ARE_EQUAL(1u, commands.Size());
VERIFY_ARE_EQUAL(1u, settings->_globals->ActionMap().CommandCount());

{
winrt::hstring commandName{ L"parent" };
auto commandProj = commands.Lookup(commandName);
auto commandProj = actionMap.GetActionByName(commandName);
VERIFY_IS_NOT_NULL(commandProj);

winrt::com_ptr<implementation::Command> commandImpl;
Expand All @@ -2539,14 +2534,13 @@ namespace SettingsModelLocalTests
settings->_ParseJsonString(settings1Json, false);
settings->LayerJson(settings->_userSettings);
settings->_ValidateSettings();
commands = settings->_globals->Commands();
_logCommandNames(commands);
_logCommandNames(settings->ActionMap().NameMap());
VERIFY_ARE_EQUAL(0u, settings->_warnings.Size());
VERIFY_ARE_EQUAL(1u, commands.Size());
VERIFY_ARE_EQUAL(1u, settings->ActionMap().CommandCount());

{
winrt::hstring commandName{ L"parent" };
auto commandProj = commands.Lookup(commandName);
auto commandProj = actionMap.GetActionByName(commandName);

VERIFY_IS_NOT_NULL(commandProj);
auto actionAndArgs = commandProj.Action();
Expand Down Expand Up @@ -2642,8 +2636,8 @@ namespace SettingsModelLocalTests
VERIFY_ARE_EQUAL(settings->_globals->_colorSchemes.HasKey(schemeName), copyImpl->_globals->_colorSchemes.HasKey(schemeName));

// test actions
VERIFY_ARE_EQUAL(settings->_globals->_keymap->_keyShortcuts.size(), copyImpl->_globals->_keymap->_keyShortcuts.size());
VERIFY_ARE_EQUAL(settings->_globals->_commands.Size(), copyImpl->_globals->_commands.Size());
VERIFY_ARE_EQUAL(settings->ActionMap().KeybindingCount(), copyImpl->ActionMap().KeybindingCount());
VERIFY_ARE_EQUAL(settings->ActionMap().CommandCount(), copyImpl->ActionMap().CommandCount());

// Test that changing the copy should not change the original
VERIFY_ARE_EQUAL(settings->_globals->WordDelimiters(), copyImpl->_globals->WordDelimiters());
Expand Down
Loading

1 comment on commit 54f17b8

@github-actions
Copy link

Choose a reason for hiding this comment

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

Misspellings found, please review:

  • reserializes
To accept these changes, run the following commands from this repository on this branch 
pushd $(git rev-parse --show-toplevel)
perl -e '
my @expect_files=qw('".github/actions/spelling/expect/alphabet.txt
.github/actions/spelling/expect/expect.txt
.github/actions/spelling/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"aspnet boostorg Cac COINIT dahall fde fea fmtlib isocpp mintty NVDA pinam QOL remoting reserialize unte vcrt what3words xamarin "');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
  if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
  next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spelling/expect/54f17b8ebb626f4ac2d090a214f93bdbfc502b90.txt";
use File::Path qw(make_path);
make_path ".github/actions/spelling/expect";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"cac coinit Remoting reserializes "');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a) cmp lc($b)} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;'
popd
✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spelling/dictionary/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spelling/dictionary/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

🗜️ If you see a bunch of garbage and it relates to a binary-ish string, please add a file path to the .github/actions/spelling/excludes.txt file instead of just accepting the garbage.

File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

Please sign in to comment.