maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Use WinRT VirtualKeyModifiers instead of a custom enum (#10603)

Browse source 
Replaces `KeyModifiers` with the pretty much equivalent
`VirtualKeyModifiers` enum in winrt.

After doing this I noticed #10593 which changes the KeyChords a lot, but
it seems these PRs are still compatible

The issue also mentions replacing Vkey with
`Windows::System::VirtualKey`, but I chose not to because that enum only
includes a subset of the keys terminal supports here (no VK_OEM_* keys)

## Validation Steps Performed
Changed key bind in config, and confirmed it still works after
restarting terminal

Closes #877
This commit is contained in:
Mim van den Bos 2021-07-12 23:24:26 +02:00 committed by GitHub
parent 0980a0d50f
commit 17e68a09a8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 62 additions and 97 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp
View file

@ -16,6 +16,7 @@ using namespace WEX::TestExecution;
using namespace WEX::Common;
using namespace winrt::Microsoft::Terminal::Settings::Model;
using namespace winrt::Microsoft::Terminal::Control;
using VirtualKeyModifiers = winrt::Windows::System::VirtualKeyModifiers;
namespace SettingsModelLocalTests
{
@ -1970,9 +1971,9 @@ namespace SettingsModelLocalTests
auto settings = implementation::CascadiaSettings::FromJson(settingsObject);
VERIFY_ARE_EQUAL(3u, settings->_globals->_actionMap->_KeyMap.size());
VERIFY_IS_NULL(settings->_globals->_actionMap->GetActionByKeyChord({ KeyModifiers::Ctrl, static_cast<int32_t>('a') }));
VERIFY_IS_NULL(settings->_globals->_actionMap->GetActionByKeyChord({ KeyModifiers::Ctrl, static_cast<int32_t>('b') }));
VERIFY_IS_NULL(settings->_globals->_actionMap->GetActionByKeyChord({ KeyModifiers::Ctrl, static_cast<int32_t>('c') }));
VERIFY_IS_NULL(settings->_globals->_actionMap->GetActionByKeyChord({ VirtualKeyModifiers::Control, static_cast<int32_t>('a') }));
VERIFY_IS_NULL(settings->_globals->_actionMap->GetActionByKeyChord({ VirtualKeyModifiers::Control, static_cast<int32_t>('b') }));
VERIFY_IS_NULL(settings->_globals->_actionMap->GetActionByKeyChord({ VirtualKeyModifiers::Control, static_cast<int32_t>('c') }));
for (const auto& warning : settings->_globals->_keybindingsWarnings)
{

19
src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp
View file

@ -15,6 +15,7 @@ using namespace winrt::Microsoft::Terminal::Control;
using namespace WEX::Logging;
using namespace WEX::TestExecution;
using namespace WEX::Common;
using VirtualKeyModifiers = winrt::Windows::System::VirtualKeyModifiers;
namespace SettingsModelLocalTests
{
@ -141,7 +142,7 @@ namespace SettingsModelLocalTests
L"Try unbinding a key using `\"unbound\"` to unbind the key"));
actionMap->LayerJson(bindings2Json);
VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size());
VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ KeyModifiers::Ctrl, static_cast<int32_t>('c') }));
VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ VirtualKeyModifiers::Control, static_cast<int32_t>('c') }));
Log::Comment(NoThrowString().Format(
L"Try unbinding a key using `null` to unbind the key"));
@ -151,7 +152,7 @@ namespace SettingsModelLocalTests
// Then try layering in the bad setting
actionMap->LayerJson(bindings3Json);
VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size());
VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ KeyModifiers::Ctrl, static_cast<int32_t>('c') }));
VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ VirtualKeyModifiers::Control, static_cast<int32_t>('c') }));
Log::Comment(NoThrowString().Format(
L"Try unbinding a key using an unrecognized command to unbind the key"));
@ -161,7 +162,7 @@ namespace SettingsModelLocalTests
// Then try layering in the bad setting
actionMap->LayerJson(bindings4Json);
VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size());
VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ KeyModifiers::Ctrl, static_cast<int32_t>('c') }));
VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ VirtualKeyModifiers::Control, static_cast<int32_t>('c') }));
Log::Comment(NoThrowString().Format(
L"Try unbinding a key using a straight up invalid value to unbind the key"));
@ -171,13 +172,13 @@ namespace SettingsModelLocalTests
// Then try layering in the bad setting
actionMap->LayerJson(bindings5Json);
VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size());
VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ KeyModifiers::Ctrl, static_cast<int32_t>('c') }));
VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ VirtualKeyModifiers::Control, static_cast<int32_t>('c') }));
Log::Comment(NoThrowString().Format(
L"Try unbinding a key that wasn't bound at all"));
actionMap->LayerJson(bindings2Json);
VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size());
VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ KeyModifiers::Ctrl, static_cast<int32_t>('c') }));
VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ VirtualKeyModifiers::Control, static_cast<int32_t>('c') }));
}
void KeyBindingsTests::TestArbitraryArgs()
@ -676,7 +677,7 @@ namespace SettingsModelLocalTests
actionMap->LayerJson(bindings0Json);
VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size());
const auto& kbd{ actionMap->GetKeyBindingForAction(ShortcutAction::CloseWindow) };
VerifyKeyChordEquality({ KeyModifiers::Ctrl, static_cast<int32_t>('A') }, kbd);
VerifyKeyChordEquality({ VirtualKeyModifiers::Control, static_cast<int32_t>('A') }, kbd);
}
{
Log::Comment(L"command with args");
@ -687,7 +688,7 @@ namespace SettingsModelLocalTests
args->SingleLine(true);
const auto& kbd{ actionMap->GetKeyBindingForAction(ShortcutAction::CopyText, *args) };
VerifyKeyChordEquality({ KeyModifiers::Ctrl, static_cast<int32_t>('B') }, kbd);
VerifyKeyChordEquality({ VirtualKeyModifiers::Control, static_cast<int32_t>('B') }, kbd);
}
{
Log::Comment(L"command with new terminal args");
@ -699,7 +700,7 @@ namespace SettingsModelLocalTests
auto args{ winrt::make_self<implementation::NewTabArgs>(*newTerminalArgs) };
const auto& kbd{ actionMap->GetKeyBindingForAction(ShortcutAction::NewTab, *args) };
VerifyKeyChordEquality({ KeyModifiers::Ctrl, static_cast<int32_t>('C') }, kbd);
VerifyKeyChordEquality({ VirtualKeyModifiers::Control, static_cast<int32_t>('C') }, kbd);
}
{
Log::Comment(L"command with hidden args");
@ -707,7 +708,7 @@ namespace SettingsModelLocalTests
VERIFY_ARE_EQUAL(4u, actionMap->_KeyMap.size());
const auto& kbd{ actionMap->GetKeyBindingForAction(ShortcutAction::ToggleCommandPalette) };
VerifyKeyChordEquality({ KeyModifiers::Ctrl | KeyModifiers::Shift, static_cast<int32_t>('P') }, kbd);
VerifyKeyChordEquality({ VirtualKeyModifiers::Control | VirtualKeyModifiers::Shift, static_cast<int32_t>('P') }, kbd);
}
}
}

8
src/cascadia/LocalTests_SettingsModel/TestUtils.h
View file

@ -26,16 +26,18 @@ public:
static const winrt::Microsoft::Terminal::Settings::Model::ActionAndArgs GetActionAndArgs(const winrt::Microsoft::Terminal::Settings::Model::ActionMap& actionMap,
const winrt::Microsoft::Terminal::Control::KeyChord& kc)
{
using VirtualKeyModifiers = winrt::Windows::System::VirtualKeyModifiers;
std::wstring buffer{ L"" };
if (WI_IsFlagSet(kc.Modifiers(), winrt::Microsoft::Terminal::Control::KeyModifiers::Ctrl))
if (WI_IsFlagSet(kc.Modifiers(), VirtualKeyModifiers::Control))
{
buffer += L"Ctrl+";
}
if (WI_IsFlagSet(kc.Modifiers(), winrt::Microsoft::Terminal::Control::KeyModifiers::Shift))
if (WI_IsFlagSet(kc.Modifiers(), VirtualKeyModifiers::Shift))
{
buffer += L"Shift+";
}
if (WI_IsFlagSet(kc.Modifiers(), winrt::Microsoft::Terminal::Control::KeyModifiers::Alt))
if (WI_IsFlagSet(kc.Modifiers(), VirtualKeyModifiers::Menu))
{
buffer += L"Alt+";
}

16
src/cascadia/TerminalApp/TerminalPage.cpp
View file

@ -41,6 +41,7 @@ namespace winrt
namespace MUX = Microsoft::UI::Xaml;
namespace WUX = Windows::UI::Xaml;
using IInspectable = Windows::Foundation::IInspectable;
using VirtualKeyModifiers = Windows::System::VirtualKeyModifiers;
}
namespace winrt::TerminalApp::implementation
@ -1348,26 +1349,26 @@ namespace winrt::TerminalApp::implementation
// Return Value:
// - a string representation of the key modifiers for the shortcut
//NOTE: This needs to be localized with https://github.com/microsoft/terminal/issues/794 if XAML framework issue not resolved before then
static std::wstring _FormatOverrideShortcutText(KeyModifiers modifiers)
static std::wstring _FormatOverrideShortcutText(VirtualKeyModifiers modifiers)
{
std::wstring buffer{ L"" };
if (WI_IsFlagSet(modifiers, KeyModifiers::Ctrl))
if (WI_IsFlagSet(modifiers, VirtualKeyModifiers::Control))
{
buffer += L"Ctrl+";
}
if (WI_IsFlagSet(modifiers, KeyModifiers::Shift))
if (WI_IsFlagSet(modifiers, VirtualKeyModifiers::Shift))
{
buffer += L"Shift+";
}
if (WI_IsFlagSet(modifiers, KeyModifiers::Alt))
if (WI_IsFlagSet(modifiers, VirtualKeyModifiers::Menu))
{
buffer += L"Alt+";
}
if (WI_IsFlagSet(modifiers, KeyModifiers::Windows))
if (WI_IsFlagSet(modifiers, VirtualKeyModifiers::Windows))
{
buffer += L"Win+";
}
@ -1393,11 +1394,8 @@ namespace winrt::TerminalApp::implementation
// TODO: Modify this when https://github.com/microsoft/terminal/issues/877 is resolved
menuShortcut.Key(static_cast<Windows::System::VirtualKey>(keyChord.Vkey()));
// inspect the modifiers from the KeyChord and set the flags int he XAML value
auto modifiers = ActionMap::ConvertVKModifiers(keyChord.Modifiers());
// add the modifiers to the shortcut
menuShortcut.Modifiers(modifiers);
menuShortcut.Modifiers(keyChord.Modifiers());
// add to the menu
menuItem.KeyboardAccelerators().Append(menuShortcut);

22
src/cascadia/TerminalControl/KeyChord.cpp
View file

@ -6,6 +6,8 @@
#include "KeyChord.g.cpp"
using VirtualKeyModifiers = winrt::Windows::System::VirtualKeyModifiers;
namespace winrt::Microsoft::Terminal::Control::implementation
{
KeyChord::KeyChord() noexcept :
@ -15,34 +17,34 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}
KeyChord::KeyChord(bool ctrl, bool alt, bool shift, int32_t vkey) noexcept :
_modifiers{ (ctrl ? Control::KeyModifiers::Ctrl : Control::KeyModifiers::None) |
(alt ? Control::KeyModifiers::Alt : Control::KeyModifiers::None) |
(shift ? Control::KeyModifiers::Shift : Control::KeyModifiers::None) },
_modifiers{ (ctrl ? VirtualKeyModifiers::Control : VirtualKeyModifiers::None) |
(alt ? VirtualKeyModifiers::Menu : VirtualKeyModifiers::None) |
(shift ? VirtualKeyModifiers::Shift : VirtualKeyModifiers::None) },
_vkey{ vkey }
{
}
KeyChord::KeyChord(bool ctrl, bool alt, bool shift, bool win, int32_t vkey) noexcept :
_modifiers{ (ctrl ? Control::KeyModifiers::Ctrl : Control::KeyModifiers::None) |
(alt ? Control::KeyModifiers::Alt : Control::KeyModifiers::None) |
(shift ? Control::KeyModifiers::Shift : Control::KeyModifiers::None) |
(win ? Control::KeyModifiers::Windows : Control::KeyModifiers::None) },
_modifiers{ (ctrl ? VirtualKeyModifiers::Control : VirtualKeyModifiers::None) |
(alt ? VirtualKeyModifiers::Menu : VirtualKeyModifiers::None) |
(shift ? VirtualKeyModifiers::Shift : VirtualKeyModifiers::None) |
(win ? VirtualKeyModifiers::Windows : VirtualKeyModifiers::None) },
_vkey{ vkey }
{
}
KeyChord::KeyChord(Control::KeyModifiers const& modifiers, int32_t vkey) noexcept :
KeyChord::KeyChord(VirtualKeyModifiers const& modifiers, int32_t vkey) noexcept :
_modifiers{ modifiers },
_vkey{ vkey }
{
}
Control::KeyModifiers KeyChord::Modifiers() noexcept
VirtualKeyModifiers KeyChord::Modifiers() noexcept
{
return _modifiers;
}
void KeyChord::Modifiers(Control::KeyModifiers const& value) noexcept
void KeyChord::Modifiers(VirtualKeyModifiers const& value) noexcept
{
_modifiers = value;
}

8
src/cascadia/TerminalControl/KeyChord.h
View file

@ -10,17 +10,17 @@ namespace winrt::Microsoft::Terminal::Control::implementation
struct KeyChord : KeyChordT<KeyChord>
{
KeyChord() noexcept;
KeyChord(Control::KeyModifiers const& modifiers, int32_t vkey) noexcept;
KeyChord(winrt::Windows::System::VirtualKeyModifiers const& modifiers, int32_t vkey) noexcept;
KeyChord(bool ctrl, bool alt, bool shift, int32_t vkey) noexcept;
KeyChord(bool ctrl, bool alt, bool shift, bool win, int32_t vkey) noexcept;
Control::KeyModifiers Modifiers() noexcept;
void Modifiers(Control::KeyModifiers const& value) noexcept;
winrt::Windows::System::VirtualKeyModifiers Modifiers() noexcept;
void Modifiers(winrt::Windows::System::VirtualKeyModifiers const& value) noexcept;
int32_t Vkey() noexcept;
void Vkey(int32_t value) noexcept;
private:
Control::KeyModifiers _modifiers;
winrt::Windows::System::VirtualKeyModifiers _modifiers;
int32_t _vkey;
};
}

14
src/cascadia/TerminalControl/KeyChord.idl
View file

@ -3,25 +3,15 @@
namespace Microsoft.Terminal.Control
{
[flags]
enum KeyModifiers
{
None = 0x0000,
Alt = 0x0001,
Ctrl = 0x0002,
Shift = 0x0004,
Windows = 0x0008
};
[default_interface]
runtimeclass KeyChord
{
KeyChord();
KeyChord(KeyModifiers modifiers, Int32 vkey);
KeyChord(Windows.System.VirtualKeyModifiers modifiers, Int32 vkey);
KeyChord(Boolean ctrl, Boolean alt, Boolean shift, Int32 vkey);
KeyChord(Boolean ctrl, Boolean alt, Boolean shift, Boolean win, Int32 vkey);
KeyModifiers Modifiers;
Windows.System.VirtualKeyModifiers Modifiers;
Int32 Vkey;
}
}

2
src/cascadia/TerminalSettingsModel/ActionMap.h
View file

@ -77,8 +77,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
void DeleteKeyBinding(Control::KeyChord const& keys);
void RegisterKeyBinding(Control::KeyChord keys, Model::ActionAndArgs action);
static Windows::System::VirtualKeyModifiers ConvertVKModifiers(Control::KeyModifiers modifiers);
private:
std::optional<Model::Command> _GetActionByID(const InternalActionID actionID) const;
std::optional<Model::Command> _GetActionByKeyChordInternal(Control::KeyChord const& keys) const;

29
src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp
View file

@ -96,33 +96,4 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
return actionList;
}
// Method Description:
// - Takes the KeyModifier flags from Terminal and maps them to the Windows WinRT types
// Return Value:
// - a Windows::System::VirtualKeyModifiers object with the flags of which modifiers used.
Windows::System::VirtualKeyModifiers ActionMap::ConvertVKModifiers(KeyModifiers modifiers)
{
Windows::System::VirtualKeyModifiers keyModifiers = Windows::System::VirtualKeyModifiers::None;
if (WI_IsFlagSet(modifiers, KeyModifiers::Ctrl))
{
keyModifiers |= Windows::System::VirtualKeyModifiers::Control;
}
if (WI_IsFlagSet(modifiers, KeyModifiers::Shift))
{
keyModifiers |= Windows::System::VirtualKeyModifiers::Shift;
}
if (WI_IsFlagSet(modifiers, KeyModifiers::Alt))
{
// note: Menu is the Alt VK_MENU
keyModifiers |= Windows::System::VirtualKeyModifiers::Menu;
}
if (WI_IsFlagSet(modifiers, KeyModifiers::Windows))
{
keyModifiers |= Windows::System::VirtualKeyModifiers::Windows;
}
return keyModifiers;
}
}

25
src/cascadia/TerminalSettingsModel/KeyChordSerialization.cpp
View file

@ -8,6 +8,7 @@
using namespace winrt::Microsoft::Terminal::Control;
using namespace winrt::Microsoft::Terminal::Settings::Model::implementation;
using namespace Microsoft::Terminal::Settings::Model::JsonUtils;
using VirtualKeyModifiers = winrt::Windows::System::VirtualKeyModifiers;
static constexpr std::wstring_view CTRL_KEY{ L"ctrl" };
static constexpr std::wstring_view SHIFT_KEY{ L"shift" };
@ -124,7 +125,7 @@ static KeyChord _fromString(const std::wstring_view& wstr)
}
}
KeyModifiers modifiers = KeyModifiers::None;
VirtualKeyModifiers modifiers = VirtualKeyModifiers::None;
int32_t vkey = 0;
// Look for ctrl, shift, alt. Anything else might be a key
@ -134,19 +135,19 @@ static KeyChord _fromString(const std::wstring_view& wstr)
std::transform(lowercase.begin(), lowercase.end(), lowercase.begin(), std::towlower);
if (lowercase == CTRL_KEY)
{
modifiers |= KeyModifiers::Ctrl;
modifiers |= VirtualKeyModifiers::Control;
}
else if (lowercase == ALT_KEY)
{
modifiers |= KeyModifiers::Alt;
modifiers |= VirtualKeyModifiers::Menu;
}
else if (lowercase == SHIFT_KEY)
{
modifiers |= KeyModifiers::Shift;
modifiers |= VirtualKeyModifiers::Shift;
}
else if (lowercase == WIN_KEY)
{
modifiers |= KeyModifiers::Windows;
modifiers |= VirtualKeyModifiers::Windows;
}
else
{
@ -195,9 +196,9 @@ static KeyChord _fromString(const std::wstring_view& wstr)
// to the user's specified modifiers. ctrl+| should be the same as ctrl+shift+\,
// but if we used WI_UpdateFlag, ctrl+shift+\ would turn _off_ Shift because \ doesn't
// require it.
WI_SetFlagIf(modifiers, KeyModifiers::Shift, WI_IsFlagSet(oemModifiers, 1U));
WI_SetFlagIf(modifiers, KeyModifiers::Ctrl, WI_IsFlagSet(oemModifiers, 2U));
WI_SetFlagIf(modifiers, KeyModifiers::Alt, WI_IsFlagSet(oemModifiers, 4U));
WI_SetFlagIf(modifiers, VirtualKeyModifiers::Shift, WI_IsFlagSet(oemModifiers, 1U));
WI_SetFlagIf(modifiers, VirtualKeyModifiers::Control, WI_IsFlagSet(oemModifiers, 2U));
WI_SetFlagIf(modifiers, VirtualKeyModifiers::Menu, WI_IsFlagSet(oemModifiers, 4U));
foundKey = true;
}
}
@ -234,22 +235,22 @@ static std::wstring _toString(const KeyChord& chord)
std::wstring buffer{ L"" };
// Add modifiers
if (WI_IsFlagSet(modifiers, KeyModifiers::Windows))
if (WI_IsFlagSet(modifiers, VirtualKeyModifiers::Windows))
{
buffer += WIN_KEY;
buffer += L"+";
}
if (WI_IsFlagSet(modifiers, KeyModifiers::Ctrl))
if (WI_IsFlagSet(modifiers, VirtualKeyModifiers::Control))
{
buffer += CTRL_KEY;
buffer += L"+";
}
if (WI_IsFlagSet(modifiers, KeyModifiers::Alt))
if (WI_IsFlagSet(modifiers, VirtualKeyModifiers::Menu))
{
buffer += ALT_KEY;
buffer += L"+";
}
if (WI_IsFlagSet(modifiers, KeyModifiers::Shift))
if (WI_IsFlagSet(modifiers, VirtualKeyModifiers::Shift))
{
buffer += SHIFT_KEY;
buffer += L"+";

9
src/cascadia/WindowsTerminal/IslandWindow.cpp
View file

@ -18,6 +18,7 @@ using namespace winrt::Microsoft::Terminal::Settings::Model;
using namespace winrt::Microsoft::Terminal::Control;
using namespace winrt::Microsoft::Terminal;
using namespace ::Microsoft::Console::Types;
using VirtualKeyModifiers = winrt::Windows::System::VirtualKeyModifiers;
#define XAML_HOSTING_WINDOW_CLASS_NAME L"CASCADIA_HOSTING_WINDOW_CLASS"
@ -1009,10 +1010,10 @@ void IslandWindow::SetGlobalHotkeys(const std::vector<winrt::Microsoft::Terminal
{
const auto modifiers = hotkey.Modifiers();
const auto hotkeyFlags = MOD_NOREPEAT |
(WI_IsFlagSet(modifiers, KeyModifiers::Windows) ? MOD_WIN : 0) |
(WI_IsFlagSet(modifiers, KeyModifiers::Alt) ? MOD_ALT : 0) |
(WI_IsFlagSet(modifiers, KeyModifiers::Ctrl) ? MOD_CONTROL : 0) |
(WI_IsFlagSet(modifiers, KeyModifiers::Shift) ? MOD_SHIFT : 0);
(WI_IsFlagSet(modifiers, VirtualKeyModifiers::Windows) ? MOD_WIN : 0) |
(WI_IsFlagSet(modifiers, VirtualKeyModifiers::Menu) ? MOD_ALT : 0) |
(WI_IsFlagSet(modifiers, VirtualKeyModifiers::Control) ? MOD_CONTROL : 0) |
(WI_IsFlagSet(modifiers, VirtualKeyModifiers::Shift) ? MOD_SHIFT : 0);
// TODO GH#8888: We should display a warning of some kind if this fails.
// This can fail if something else already bound this hotkey.