Skip to content
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.

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

Introduce ActionMap to Terminal Settings Model #9621

Merged
merged 34 commits into from
May 5, 2021
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
e8dd486
Give Command ownership over KeyChord
carlos-zamora Mar 17, 2021
42c43ff
remove KeyChordText from Command
carlos-zamora Mar 18, 2021
c0c5a49
fix nested key chords
carlos-zamora Mar 19, 2021
68984c8
xaml format
carlos-zamora Mar 29, 2021
9de4f58
add [] notation support to converter
carlos-zamora Mar 31, 2021
460f914
Introduce ActionMap to Terminal Settings Model
carlos-zamora Mar 25, 2021
bc996c0
add action hashing; update command; fix test DllMain error
carlos-zamora Apr 9, 2021
a00b84f
fix most tests. Missing 8 DeserializationTests and 5 SettingsTests
carlos-zamora Apr 9, 2021
8840bc5
fix nested commands
carlos-zamora Apr 13, 2021
9a1af76
code format and minor polish
carlos-zamora Apr 13, 2021
d1b71ed
update dictionary
carlos-zamora Apr 13, 2021
4fde1b0
Apply Griese's PR feedback
carlos-zamora Apr 13, 2021
8507ef0
rename cmd.Action; move HashProperty; fix most tests
carlos-zamora Apr 15, 2021
dc1de34
fix failing tests; We're good to go!!
carlos-zamora Apr 15, 2021
3d3f355
fix all tests and dropdown bug
carlos-zamora Apr 16, 2021
d6f3864
fix old reference to 'cmd.Action'
carlos-zamora Apr 22, 2021
17fc18b
fix build
carlos-zamora Apr 23, 2021
a961fc2
PR feedback; copy parents too
carlos-zamora Apr 23, 2021
0564cae
polish the actions page a bit w/ sorting and removing unnecessary con…
carlos-zamora Apr 23, 2021
0b36562
introduce ActionMap::_GetCumulativeActions(); actually copy ActionAnd…
carlos-zamora Apr 28, 2021
db55f70
introduce IActionMapView
carlos-zamora Apr 28, 2021
1a67c49
bugfix: unbind won't unbind keys for cmd palette
carlos-zamora Apr 29, 2021
867c081
Add ConsolidatedActions; Assert single parent; Break up AddAction
carlos-zamora Apr 29, 2021
a6a6eb1
test this nasty bug Dustin found
carlos-zamora Apr 29, 2021
4ed438a
Merge branch 'main' into dev/cazamor/tsm/action-map
carlos-zamora Apr 29, 2021
d78a581
code format
carlos-zamora Apr 29, 2021
8784189
address Leonard's comments
carlos-zamora Apr 30, 2021
efe5bd6
Apply feedback from Dustin and Leonard
carlos-zamora May 3, 2021
e3b0f48
apply Leonard's feedback
carlos-zamora May 4, 2021
28479dd
address DHowett comments
carlos-zamora May 5, 2021
28ae1d9
Merge branch 'main' into dev/cazamor/tsm/action-map
carlos-zamora May 5, 2021
ad42c2d
moar DHowett comments
carlos-zamora May 5, 2021
900226a
find & replace the wrong one :(
carlos-zamora May 5, 2021
24ada24
Merge branch 'main' into dev/cazamor/tsm/action-map
carlos-zamora May 5, 2021
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
2 changes: 2 additions & 0 deletions .github/actions/spelling/dictionary/dictionary.txt
Original file line number Diff line number Diff line change
Expand Up @@ -345242,6 +345242,8 @@ resequester
resequestration
reserate
reserene
reserialize
reserializes
reserialization
reserpine
reserpinized
Expand Down
126 changes: 63 additions & 63 deletions src/cascadia/LocalTests_SettingsModel/CommandTests.cpp

Large diffs are not rendered by default.

292 changes: 214 additions & 78 deletions src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp

Large diffs are not rendered by default.

308 changes: 187 additions & 121 deletions src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@
</ClCompile>
<Link>
<AdditionalDependencies>onecoreuap.lib;%(AdditionalDependencies)</AdditionalDependencies>
<!--
SettingsModelLib contains a DllMain that we need to force the use of.
If you don't have this, then you'll see an error like
"(init.obj) : error LNK2005: DllMain already defined in MSVCRTD.lib(dll_dllmain_stub.obj)"
-->
<AdditionalOptions Condition="'$(Platform)'=='Win32'">/INCLUDE:_DllMain@12</AdditionalOptions>
<AdditionalOptions Condition="'$(Platform)'!='Win32'">/INCLUDE:DllMain</AdditionalOptions>
</Link>
</ItemDefinitionGroup>

Expand Down
29 changes: 15 additions & 14 deletions src/cascadia/LocalTests_SettingsModel/TerminalSettingsTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,18 @@ namespace SettingsModelLocalTests

CascadiaSettings settings{ til::u8u16(settingsJson) };

auto keymap = settings.GlobalSettings().KeyMap();
auto actionMap = settings.GlobalSettings().ActionMap();
VERIFY_ARE_EQUAL(3u, settings.ActiveProfiles().Size());

const auto profile2Guid = settings.ActiveProfiles().GetAt(2).Guid();
VERIFY_ARE_NOT_EQUAL(winrt::guid{}, profile2Guid);

VERIFY_ARE_EQUAL(12u, keymap.Size());
const auto& actionMapImpl{ winrt::get_self<implementation::ActionMap>(actionMap) };
VERIFY_ARE_EQUAL(12u, actionMapImpl->_KeyMap.size());

{
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 @@ -134,7 +135,7 @@ namespace SettingsModelLocalTests
}
{
KeyChord kc{ true, false, false, static_cast<int32_t>('B') };
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 @@ -156,7 +157,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 @@ -178,7 +179,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 @@ -200,7 +201,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 @@ -222,7 +223,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 @@ -245,7 +246,7 @@ namespace SettingsModelLocalTests
}
{
KeyChord kc{ true, false, false, static_cast<int32_t>('G') };
auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc);
auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc);
VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<NewTabArgs>();
VERIFY_IS_NOT_NULL(realArgs);
Expand All @@ -265,7 +266,7 @@ namespace SettingsModelLocalTests
}
{
KeyChord kc{ true, false, false, static_cast<int32_t>('H') };
auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc);
auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc);
VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<NewTabArgs>();
VERIFY_IS_NOT_NULL(realArgs);
Expand All @@ -287,7 +288,7 @@ namespace SettingsModelLocalTests
}
{
KeyChord kc{ true, false, false, static_cast<int32_t>('I') };
auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc);
auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc);
VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<NewTabArgs>();
VERIFY_IS_NOT_NULL(realArgs);
Expand All @@ -310,7 +311,7 @@ namespace SettingsModelLocalTests
}
{
KeyChord kc{ true, false, false, static_cast<int32_t>('J') };
auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc);
auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc);
VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<NewTabArgs>();
VERIFY_IS_NOT_NULL(realArgs);
Expand All @@ -332,7 +333,7 @@ namespace SettingsModelLocalTests
}
{
KeyChord kc{ true, false, false, static_cast<int32_t>('K') };
auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc);
auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc);
VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<NewTabArgs>();
VERIFY_IS_NOT_NULL(realArgs);
Expand All @@ -355,7 +356,7 @@ namespace SettingsModelLocalTests
}
{
KeyChord kc{ true, false, false, static_cast<int32_t>('L') };
auto actionAndArgs = ::TestUtils::GetActionAndArgs(keymap, kc);
auto actionAndArgs = ::TestUtils::GetActionAndArgs(actionMap, kc);
VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<NewTabArgs>();
VERIFY_IS_NOT_NULL(realArgs);
Expand Down
10 changes: 5 additions & 5 deletions src/cascadia/LocalTests_SettingsModel/TestUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ class TestUtils
// - This is a helper to retrieve the ActionAndArgs from the keybindings
// for a given chord.
// Arguments:
// - keymap: The AppKeyBindings to lookup the ActionAndArgs from.
// - actionMap: The ActionMap to lookup the ActionAndArgs from.
// - kc: The key chord to look up the bound ActionAndArgs for.
// Return Value:
// - The ActionAndArgs bound to the given key, or nullptr if nothing is bound to it.
static const winrt::Microsoft::Terminal::Settings::Model::ActionAndArgs GetActionAndArgs(const winrt::Microsoft::Terminal::Settings::Model::KeyMapping& keymap,
static const winrt::Microsoft::Terminal::Settings::Model::ActionAndArgs GetActionAndArgs(const winrt::Microsoft::Terminal::Settings::Model::ActionMap& actionMap,
const winrt::Microsoft::Terminal::Control::KeyChord& kc)
{
std::wstring buffer{ L"" };
Expand All @@ -42,8 +42,8 @@ class TestUtils
buffer += static_cast<wchar_t>(MapVirtualKeyW(kc.Vkey(), MAPVK_VK_TO_CHAR));
WEX::Logging::Log::Comment(WEX::Common::NoThrowString().Format(L"Looking for key:%s", buffer.c_str()));

const auto action = keymap.TryLookup(kc);
VERIFY_IS_NOT_NULL(action, L"Expected to find an action bound to the given KeyChord");
return action;
const auto cmd = actionMap.GetActionByKeyChord(kc);
VERIFY_IS_NOT_NULL(cmd, L"Expected to find an action bound to the given KeyChord");
return cmd.ActionAndArgs();
};
};
Loading