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.

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

Jump to bottom

Introduce vk() and sc() key chord specifiers #10666

Merged
15 commits merged into from
Jul 20, 2021
Merged

Introduce vk() and sc() key chord specifiers #10666

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
0bb6e34
Introduce vk() and sc() key chord specifiers
lhecker Jul 15, 2021
561d40a
Improve til::from_wchars
lhecker Jul 15, 2021
7297b41
Address reviewer comments
lhecker Jul 15, 2021
b8d6f68
Fix unit tests
lhecker Jul 15, 2021
9fb176b
Fix formatting
lhecker Jul 15, 2021
877acab
Fix spelling
lhecker Jul 15, 2021
f901604
Fix audit checks
lhecker Jul 15, 2021
098aff1
Update profiles.schema.json
lhecker Jul 15, 2021
bb6e64d
Fix x86 build
lhecker Jul 15, 2021
253072f
Improve documentation and til::from_wchars
lhecker Jul 16, 2021
c3cc1fc
Add some basic KeyChordSerialization unit tests
lhecker Jul 19, 2021
a38e415
Remove unnecessary comment
lhecker Jul 19, 2021
9c54d77
Merge remote-tracking branch 'origin/main' into dev/lhecker/7539-key-…
lhecker Jul 19, 2021
d1bef1e
Fix build error after merging main
lhecker Jul 19, 2021
a8b29ed
Address reviewer comments
lhecker Jul 20, 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
5 changes: 3 additions & 2 deletions .github/actions/spelling/allow/allow.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
Apc
apc
Apc
clickable
copyable
dalet
Dcs
dcs
Dcs
dialytika
dje
downside
Expand Down Expand Up @@ -52,6 +52,7 @@ tonos
tshe
UIs
und
unregister
versioned
We'd
wildcards
Expand Down
1 change: 1 addition & 0 deletions .github/actions/spelling/allow/apis.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ userenv
wcsstr
wcstoui
winmain
wmemcmp
wpc
wsregex
wwinmain
Expand Down
2 changes: 2 additions & 0 deletions .github/actions/spelling/expect/expect.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1502,6 +1502,7 @@ nfe
nlength
Nls
NLSMODE
nnn
NOACTIVATE
NOAPPLYNOW
NOCLIP
Expand Down Expand Up @@ -1582,6 +1583,7 @@ NTVDM
ntverp
NTWIN
nuget
nullability
nullness
nullonfailure
nullopt
Expand Down
19 changes: 11 additions & 8 deletions src/cascadia/TerminalApp/CommandPalette.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,10 +250,12 @@ namespace winrt::TerminalApp::implementation
void CommandPalette::_previewKeyDownHandler(IInspectable const& /*sender*/,
Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e)
{
auto key = e.OriginalKey();
auto const ctrlDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Control), CoreVirtualKeyStates::Down);
auto const altDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Menu), CoreVirtualKeyStates::Down);
auto const shiftDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Shift), CoreVirtualKeyStates::Down);
const auto key = e.OriginalKey();
const auto scanCode = e.KeyStatus().ScanCode;
const auto coreWindow = CoreWindow::GetForCurrentThread();
const auto ctrlDown = WI_IsFlagSet(coreWindow.GetKeyState(winrt::Windows::System::VirtualKey::Control), CoreVirtualKeyStates::Down);
const auto altDown = WI_IsFlagSet(coreWindow.GetKeyState(winrt::Windows::System::VirtualKey::Menu), CoreVirtualKeyStates::Down);
const auto shiftDown = WI_IsFlagSet(coreWindow.GetKeyState(winrt::Windows::System::VirtualKey::Shift), CoreVirtualKeyStates::Down);

// Some keypresses such as Tab, Return, Esc, and Arrow Keys are ignored by controls because
// they're not considered input key presses. While they don't raise KeyDown events,
Expand All @@ -264,7 +266,7 @@ namespace winrt::TerminalApp::implementation
// a really widely used keyboard navigation key.
if (_currentMode == CommandPaletteMode::TabSwitchMode && _actionMap)
{
winrt::Microsoft::Terminal::Control::KeyChord kc{ ctrlDown, altDown, shiftDown, static_cast<int32_t>(key) };
winrt::Microsoft::Terminal::Control::KeyChord kc{ ctrlDown, altDown, shiftDown, false, static_cast<int32_t>(key), static_cast<int32_t>(scanCode) };
if (const auto cmd{ _actionMap.GetActionByKeyChord(kc) })
{
if (cmd.ActionAndArgs().Action() == ShortcutAction::PrevTab)
Expand Down Expand Up @@ -402,9 +404,10 @@ namespace winrt::TerminalApp::implementation
// - <none>
void CommandPalette::_anchorKeyUpHandler()
{
auto const ctrlDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Control), CoreVirtualKeyStates::Down);
auto const altDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Menu), CoreVirtualKeyStates::Down);
auto const shiftDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Shift), CoreVirtualKeyStates::Down);
const auto coreWindow = CoreWindow::GetForCurrentThread();
const auto ctrlDown = WI_IsFlagSet(coreWindow.GetKeyState(winrt::Windows::System::VirtualKey::Control), CoreVirtualKeyStates::Down);
const auto altDown = WI_IsFlagSet(coreWindow.GetKeyState(winrt::Windows::System::VirtualKey::Menu), CoreVirtualKeyStates::Down);
const auto shiftDown = WI_IsFlagSet(coreWindow.GetKeyState(winrt::Windows::System::VirtualKey::Shift), CoreVirtualKeyStates::Down);

if (!ctrlDown && !altDown && !shiftDown)
{
Expand Down
14 changes: 8 additions & 6 deletions src/cascadia/TerminalApp/TerminalPage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -914,12 +914,14 @@ namespace winrt::TerminalApp::implementation
// - <none>
void TerminalPage::_KeyDownHandler(Windows::Foundation::IInspectable const& /*sender*/, Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e)
{
auto key = e.OriginalKey();
auto const ctrlDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Control), CoreVirtualKeyStates::Down);
auto const altDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Menu), CoreVirtualKeyStates::Down);
auto const shiftDown = WI_IsFlagSet(CoreWindow::GetForCurrentThread().GetKeyState(winrt::Windows::System::VirtualKey::Shift), CoreVirtualKeyStates::Down);

winrt::Microsoft::Terminal::Control::KeyChord kc{ ctrlDown, altDown, shiftDown, static_cast<int32_t>(key) };
const auto key = e.OriginalKey();
const auto scanCode = e.KeyStatus().ScanCode;
const auto coreWindow = CoreWindow::GetForCurrentThread();
const auto ctrlDown = WI_IsFlagSet(coreWindow.GetKeyState(winrt::Windows::System::VirtualKey::Control), CoreVirtualKeyStates::Down);
const auto altDown = WI_IsFlagSet(coreWindow.GetKeyState(winrt::Windows::System::VirtualKey::Menu), CoreVirtualKeyStates::Down);
const auto shiftDown = WI_IsFlagSet(coreWindow.GetKeyState(winrt::Windows::System::VirtualKey::Shift), CoreVirtualKeyStates::Down);

winrt::Microsoft::Terminal::Control::KeyChord kc{ ctrlDown, altDown, shiftDown, false, static_cast<int32_t>(key), static_cast<int32_t>(scanCode) };
if (const auto cmd{ _settings.ActionMap().GetActionByKeyChord(kc) })
{
if (CommandPalette().Visibility() == Visibility::Visible && cmd.ActionAndArgs().Action() != ShortcutAction::ToggleCommandPalette)
Expand Down
41 changes: 22 additions & 19 deletions src/cascadia/TerminalControl/KeyChord.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,32 +10,27 @@ using VirtualKeyModifiers = winrt::Windows::System::VirtualKeyModifiers;

namespace winrt::Microsoft::Terminal::Control::implementation
{
KeyChord::KeyChord() noexcept :
_modifiers{ 0 },
_vkey{ 0 }
static VirtualKeyModifiers modifiersFromBooleans(bool ctrl, bool alt, bool shift, bool win)
{
VirtualKeyModifiers modifiers = VirtualKeyModifiers::None;
WI_SetFlagIf(modifiers, VirtualKeyModifiers::Control, ctrl);
WI_SetFlagIf(modifiers, VirtualKeyModifiers::Menu, alt);
WI_SetFlagIf(modifiers, VirtualKeyModifiers::Shift, shift);
WI_SetFlagIf(modifiers, VirtualKeyModifiers::Windows, win);
return modifiers;
}

KeyChord::KeyChord(bool ctrl, bool alt, bool shift, int32_t vkey) noexcept :
_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, int32_t scanCode) noexcept :
_modifiers{ modifiersFromBooleans(ctrl, alt, shift, win) },
_vkey{ vkey },
_scanCode{ scanCode }
{
}

KeyChord::KeyChord(bool ctrl, bool alt, bool shift, bool win, int32_t vkey) noexcept :
_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(VirtualKeyModifiers const& modifiers, int32_t vkey) noexcept :
KeyChord::KeyChord(const VirtualKeyModifiers modifiers, int32_t vkey, int32_t scanCode) noexcept :
_modifiers{ modifiers },
_vkey{ vkey }
_vkey{ vkey },
_scanCode{ scanCode }
{
}

Expand All @@ -58,4 +53,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
_vkey = value;
}

int32_t KeyChord::ScanCode() noexcept {
return _scanCode;
}

void KeyChord::ScanCode(int32_t value) noexcept {
_scanCode = value;
}
}
14 changes: 8 additions & 6 deletions src/cascadia/TerminalControl/KeyChord.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,21 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
struct KeyChord : KeyChordT<KeyChord>
{
KeyChord() 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;
KeyChord() noexcept = default;
KeyChord(const winrt::Windows::System::VirtualKeyModifiers modifiers, int32_t vkey, int32_t scanCode) noexcept;
KeyChord(bool ctrl, bool alt, bool shift, bool win, int32_t vkey, int32_t scanCode) 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;
int32_t ScanCode() noexcept;
void ScanCode(int32_t value) noexcept;

private:
winrt::Windows::System::VirtualKeyModifiers _modifiers;
int32_t _vkey;
winrt::Windows::System::VirtualKeyModifiers _modifiers{};
int32_t _vkey{};
int32_t _scanCode{};
};
}

Expand Down
6 changes: 3 additions & 3 deletions src/cascadia/TerminalControl/KeyChord.idl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ namespace Microsoft.Terminal.Control
runtimeclass KeyChord
{
KeyChord();
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);
KeyChord(Windows.System.VirtualKeyModifiers modifiers, Int32 vkey, Int32 scanCode);
KeyChord(Boolean ctrl, Boolean alt, Boolean shift, Boolean win, Int32 vkey, Int32 scanCode);
Comment on lines +10 to +11
Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't want to add a 5th constructor, so I removed 1 instead.


Windows.System.VirtualKeyModifiers Modifiers;
Int32 Vkey;
Int32 ScanCode;
}
}
3 changes: 3 additions & 0 deletions src/cascadia/TerminalControl/TermControl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -802,7 +802,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
modifiers.IsCtrlPressed(),
modifiers.IsAltPressed(),
modifiers.IsShiftPressed(),
modifiers.IsWinPressed(),
VK_F7,
0,
});
}

Expand Down Expand Up @@ -931,6 +933,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
modifiers.IsShiftPressed(),
modifiers.IsWinPressed(),
vkey,
scanCode,
});
if (!success)
{
Expand Down
27 changes: 14 additions & 13 deletions src/cascadia/TerminalSettingsModel/ActionMap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,9 +513,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// If we had to find one from a layer above that, parent->_MaskingActions
// would have found it, so we inherit it for free!
const auto& inheritedCmd{ parent->_GetActionByID(actionID) };
if (inheritedCmd.has_value() && inheritedCmd.value())
if (inheritedCmd && *inheritedCmd)
{
const auto& inheritedCmdImpl{ get_self<Command>(inheritedCmd.value()) };
const auto& inheritedCmdImpl{ get_self<Command>(*inheritedCmd) };
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
maskingCmd = *inheritedCmdImpl->Copy();
_MaskingActions.emplace(actionID, maskingCmd);
}
Expand Down Expand Up @@ -683,18 +683,20 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// - keys: the key chord of the command to search for
// Return Value:
// - the command with the given key chord
// - nullptr if the key chord is explicitly unbound
// - nullptr if the key chord doesn't exist
Model::Command ActionMap::GetActionByKeyChord(Control::KeyChord const& keys) const
{
// Check the current layer
const auto cmd{ _GetActionByKeyChordInternal(keys) };
if (cmd.has_value())
const auto modifiers = keys.Modifiers();

if (auto vkey = keys.Vkey())
Copy link
Member

Choose a reason for hiding this comment

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

would it be tremendously dumb to oneline this as

        return _GetActionByKeyChordInternal({ modifiers, keys.Vkey(), 0 }).value_or( _GetActionByKeyChordInternal({ modifiers, 0, keys.ScanCode() }).value_or(nullptr));

Copy link
Member Author

Choose a reason for hiding this comment

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

It would certainly be nice! But it would also eagerly check both variants, before returning either, and I'm not sure if that's a good idea...

{
return *cmd;
if (auto command = _GetActionByKeyChordInternal({ modifiers, vkey, 0 }))
{
return *command;
}
}

// This key chord is not explicitly bound
return nullptr;
return _GetActionByKeyChordInternal({ modifiers, 0, keys.ScanCode() }).value_or(nullptr);
Copy link
Member Author

Choose a reason for hiding this comment

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

We'll now have to run _GetActionByKeyChordInternal twice, since the given keys might have both a vkey as well as a scan code set, but the internal hashmaps have only key bindings loaded that contain either of the two.

I used to have a comment explaining this here, but I must've lost it at some point. I'll re-add one.

}

// Method Description:
Expand All @@ -709,8 +711,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
std::optional<Model::Command> ActionMap::_GetActionByKeyChordInternal(Control::KeyChord const& keys) const
{
// Check the current layer
const auto actionIDPair{ _KeyMap.find(keys) };
if (actionIDPair != _KeyMap.end())
if (const auto actionIDPair = _KeyMap.find(keys); actionIDPair != _KeyMap.end())
{
// the command was explicitly bound,
// return what we found (invalid commands exposed as nullptr)
Expand All @@ -723,7 +724,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
for (const auto& parent : _parents)
{
const auto& inheritedCmd{ parent->_GetActionByKeyChordInternal(keys) };
if (inheritedCmd.has_value())
if (inheritedCmd)
{
return *inheritedCmd;
}
Expand Down Expand Up @@ -765,7 +766,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
const auto hash{ Hash(actionAndArgs) };
if (const auto& cmd{ _GetActionByID(hash) })
{
return cmd.value().Keys();
return cmd->Keys();
}

// Check our parents
Expand Down
6 changes: 3 additions & 3 deletions src/cascadia/TerminalSettingsModel/ActionMap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{
std::size_t operator()(const Control::KeyChord& key) const
{
return ::Microsoft::Terminal::Settings::Model::HashUtils::HashProperty(key.Modifiers(), key.Vkey());
return ::Microsoft::Terminal::Settings::Model::HashUtils::HashProperty(key.Modifiers(), key.Vkey(), key.ScanCode());
}
};

struct KeyChordEquality
{
bool operator()(const Control::KeyChord& lhs, const Control::KeyChord& rhs) const
{
return lhs.Modifiers() == rhs.Modifiers() && lhs.Vkey() == rhs.Vkey();
return lhs.Modifiers() == rhs.Modifiers() && lhs.Vkey() == rhs.Vkey() && lhs.ScanCode() == rhs.ScanCode();
}
};

Expand Down Expand Up @@ -79,7 +79,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation

private:
std::optional<Model::Command> _GetActionByID(const InternalActionID actionID) const;
std::optional<Model::Command> _GetActionByKeyChordInternal(Control::KeyChord const& keys) const;
std::optional<Model::Command> _GetActionByKeyChordInternal(const Control::KeyChord& keys) const;

void _PopulateAvailableActionsWithStandardCommands(std::unordered_map<hstring, Model::ActionAndArgs>& availableActions, std::unordered_set<InternalActionID>& visitedActionIDs) const;
void _PopulateNameMapWithSpecialCommands(std::unordered_map<hstring, Model::Command>& nameMap) const;
Expand Down
Loading