diff --git a/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp b/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp index a5479f6a7..e5c354d25 100644 --- a/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp @@ -40,7 +40,7 @@ namespace SettingsModelLocalTests TEST_METHOD(ManyKeysSameAction); TEST_METHOD(LayerKeybindings); TEST_METHOD(UnbindKeybindings); - + TEST_METHOD(TestExplicitUnbind); TEST_METHOD(TestArbitraryArgs); TEST_METHOD(TestSplitPaneArgs); @@ -232,6 +232,31 @@ namespace SettingsModelLocalTests VERIFY_IS_NULL(actionMap->GetActionByKeyChord({ VirtualKeyModifiers::Control, static_cast('C'), 0 })); } + void KeyBindingsTests::TestExplicitUnbind() + { + const std::string bindings0String{ R"([ { "command": "copy", "keys": ["ctrl+c"] } ])" }; + const std::string bindings1String{ R"([ { "command": "unbound", "keys": ["ctrl+c"] } ])" }; + const std::string bindings2String{ R"([ { "command": "copy", "keys": ["ctrl+c"] } ])" }; + + const auto bindings0Json = VerifyParseSucceeded(bindings0String); + const auto bindings1Json = VerifyParseSucceeded(bindings1String); + const auto bindings2Json = VerifyParseSucceeded(bindings2String); + + const KeyChord keyChord{ VirtualKeyModifiers::Control, static_cast('C'), 0 }; + + auto actionMap = winrt::make_self(); + VERIFY_IS_FALSE(actionMap->IsKeyChordExplicitlyUnbound(keyChord)); + + actionMap->LayerJson(bindings0Json); + VERIFY_IS_FALSE(actionMap->IsKeyChordExplicitlyUnbound(keyChord)); + + actionMap->LayerJson(bindings1Json); + VERIFY_IS_TRUE(actionMap->IsKeyChordExplicitlyUnbound(keyChord)); + + actionMap->LayerJson(bindings2Json); + VERIFY_IS_FALSE(actionMap->IsKeyChordExplicitlyUnbound(keyChord)); + } + void KeyBindingsTests::TestArbitraryArgs() { const std::string bindings0String{ R"([ diff --git a/src/cascadia/TerminalApp/AppKeyBindings.cpp b/src/cascadia/TerminalApp/AppKeyBindings.cpp index 71a3a03bb..3f921bc55 100644 --- a/src/cascadia/TerminalApp/AppKeyBindings.cpp +++ b/src/cascadia/TerminalApp/AppKeyBindings.cpp @@ -21,6 +21,11 @@ namespace winrt::TerminalApp::implementation return false; } + bool AppKeyBindings::IsKeyChordExplicitlyUnbound(const KeyChord& kc) + { + return _actionMap.IsKeyChordExplicitlyUnbound(kc); + } + void AppKeyBindings::SetDispatch(const winrt::TerminalApp::ShortcutActionDispatch& dispatch) { _dispatch = dispatch; diff --git a/src/cascadia/TerminalApp/AppKeyBindings.h b/src/cascadia/TerminalApp/AppKeyBindings.h index 5004edeca..f4f6c20ba 100644 --- a/src/cascadia/TerminalApp/AppKeyBindings.h +++ b/src/cascadia/TerminalApp/AppKeyBindings.h @@ -20,6 +20,7 @@ namespace winrt::TerminalApp::implementation AppKeyBindings() = default; bool TryKeyChord(winrt::Microsoft::Terminal::Control::KeyChord const& kc); + bool IsKeyChordExplicitlyUnbound(winrt::Microsoft::Terminal::Control::KeyChord const& kc); void SetDispatch(const winrt::TerminalApp::ShortcutActionDispatch& dispatch); void SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap); diff --git a/src/cascadia/TerminalControl/IKeyBindings.idl b/src/cascadia/TerminalControl/IKeyBindings.idl index 2b3901559..83d665faa 100644 --- a/src/cascadia/TerminalControl/IKeyBindings.idl +++ b/src/cascadia/TerminalControl/IKeyBindings.idl @@ -9,5 +9,6 @@ namespace Microsoft.Terminal.Control interface IKeyBindings { Boolean TryKeyChord(KeyChord kc); + Boolean IsKeyChordExplicitlyUnbound(KeyChord kc); } } diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 0d20606d1..238d75c90 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -761,28 +761,39 @@ namespace winrt::Microsoft::Terminal::Control::implementation (void)_TrySendKeyEvent(VK_MENU, scanCode, modifiers, false); handled = true; } - else if (vkey == VK_F7 && down) + else if ((vkey == VK_F7 || vkey == VK_SPACE) && down) { // Manually generate an F7 event into the key bindings or terminal. // This is required as part of GH#638. + // Or do so for alt+space; only send to terminal when explicitly unbound + // That is part of #GH7125 auto bindings{ _settings.KeyBindings() }; + bool isUnbound = false; + const KeyChord kc = { + modifiers.IsCtrlPressed(), + modifiers.IsAltPressed(), + modifiers.IsShiftPressed(), + modifiers.IsWinPressed(), + gsl::narrow_cast(vkey), + 0 + }; if (bindings) { - handled = bindings.TryKeyChord({ - modifiers.IsCtrlPressed(), - modifiers.IsAltPressed(), - modifiers.IsShiftPressed(), - modifiers.IsWinPressed(), - VK_F7, - 0, - }); + handled = bindings.TryKeyChord(kc); + + if (!handled) + { + isUnbound = bindings.IsKeyChordExplicitlyUnbound(kc); + } } - if (!handled) + const bool sendToTerminal = vkey == VK_F7 || (vkey == VK_SPACE && isUnbound); + + if (!handled && sendToTerminal) { // _TrySendKeyEvent pretends it didn't handle F7 for some unknown reason. - (void)_TrySendKeyEvent(VK_F7, scanCode, modifiers, true); + (void)_TrySendKeyEvent(gsl::narrow_cast(vkey), scanCode, modifiers, true); // GH#6438: Note that we're _not_ sending the key up here - that'll // get passed through XAML to our KeyUp handler normally. handled = true; diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 68e8424ed..53ca70878 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -594,14 +594,6 @@ bool Terminal::SendKeyEvent(const WORD vkey, const auto isAltOnlyPressed = states.IsAltPressed() && !states.IsCtrlPressed(); - // DON'T manually handle Alt+Space - the system will use this to bring up - // the system menu for restore, min/maximize, size, move, close. - // (This doesn't apply to Ctrl+Alt+Space.) - if (isAltOnlyPressed && vkey == VK_SPACE) - { - return false; - } - // By default Windows treats Ctrl+Alt as an alias for AltGr. // When the altGrAliasing setting is set to false, this behaviour should be disabled. // @@ -672,13 +664,6 @@ bool Terminal::SendMouseEvent(const COORD viewportPos, const unsigned int uiButt // - false otherwise. bool Terminal::SendCharEvent(const wchar_t ch, const WORD scanCode, const ControlKeyStates states) { - // DON'T manually handle Alt+Space - the system will use this to bring up - // the system menu for restore, min/maximize, size, move, close. - if (ch == L' ' && states.IsAltPressed() && !states.IsCtrlPressed()) - { - return false; - } - auto vkey = _TakeVirtualKeyFromLastKeyEvent(scanCode); if (vkey == 0 && scanCode != 0) { diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.cpp b/src/cascadia/TerminalSettingsModel/ActionMap.cpp index b0740f8d3..75a1213ad 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMap.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionMap.cpp @@ -677,6 +677,32 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } } + // Method Description: + // - Determines whether the given key chord is explicitly unbound + // Arguments: + // - keys: the key chord to check + // Return value: + // - true if the keychord is explicitly unbound + // - false if either the keychord is bound, or not bound at all + bool ActionMap::IsKeyChordExplicitlyUnbound(Control::KeyChord const& keys) const + { + const auto modifiers = keys.Modifiers(); + + // The "keys" given to us can contain both a Vkey, as well as a ScanCode. + // For instance our UI code fills out a KeyChord with all available information. + // But our _KeyMap only contains KeyChords that contain _either_ a Vkey or ScanCode. + // Due to this we'll have to call _GetActionByKeyChordInternal twice. + if (auto vkey = keys.Vkey()) + { + // We use the fact that the ..Internal call returns nullptr for explicitly unbound + // key chords, and nullopt for keychord that are not bound - it allows us to distinguish + // between unbound and lack of binding. + return nullptr == _GetActionByKeyChordInternal({ modifiers, vkey, 0 }); + } + + return nullptr == _GetActionByKeyChordInternal({ modifiers, 0, keys.ScanCode() }); + } + // Method Description: // - Retrieves the assigned command that can be invoked with the given key chord // Arguments: diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.h b/src/cascadia/TerminalSettingsModel/ActionMap.h index c81bd14c3..a79c2c6e8 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMap.h +++ b/src/cascadia/TerminalSettingsModel/ActionMap.h @@ -61,6 +61,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // queries Model::Command GetActionByKeyChord(Control::KeyChord const& keys) const; + bool IsKeyChordExplicitlyUnbound(Control::KeyChord const& keys) const; Control::KeyChord GetKeyBindingForAction(ShortcutAction const& action) const; Control::KeyChord GetKeyBindingForAction(ShortcutAction const& action, IActionArgs const& actionArgs) const; diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.idl b/src/cascadia/TerminalSettingsModel/ActionMap.idl index 0a9c4d07c..806baa17a 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMap.idl +++ b/src/cascadia/TerminalSettingsModel/ActionMap.idl @@ -8,6 +8,8 @@ namespace Microsoft.Terminal.Settings.Model // This interface ensures that no changes are made to ActionMap interface IActionMapView { + Boolean IsKeyChordExplicitlyUnbound(Microsoft.Terminal.Control.KeyChord keys); + Command GetActionByKeyChord(Microsoft.Terminal.Control.KeyChord keys); Microsoft.Terminal.Control.KeyChord GetKeyBindingForAction(ShortcutAction action); diff --git a/src/cascadia/UnitTests_TerminalCore/InputTest.cpp b/src/cascadia/UnitTests_TerminalCore/InputTest.cpp index 131e410d0..34dd9118b 100644 --- a/src/cascadia/UnitTests_TerminalCore/InputTest.cpp +++ b/src/cascadia/UnitTests_TerminalCore/InputTest.cpp @@ -29,7 +29,6 @@ namespace TerminalCoreUnitTests }; TEST_METHOD(AltShiftKey); - TEST_METHOD(AltSpace); TEST_METHOD(InvalidKeyEvent); void _VerifyExpectedInput(std::wstring& actualInput) @@ -57,16 +56,6 @@ namespace TerminalCoreUnitTests VERIFY_IS_TRUE(term.SendCharEvent(L'A', 0, ControlKeyStates::LeftAltPressed | ControlKeyStates::ShiftPressed)); } - void InputTest::AltSpace() - { - // Make sure we don't handle Alt+Space. The system will use this to - // bring up the system menu for restore, min/maximize, size, move, - // close - VERIFY_IS_FALSE(term.SendKeyEvent(L' ', 0, ControlKeyStates::LeftAltPressed, true)); - VERIFY_IS_FALSE(term.SendKeyEvent(L' ', 0, ControlKeyStates::LeftAltPressed, false)); - VERIFY_IS_FALSE(term.SendCharEvent(L' ', 0, ControlKeyStates::LeftAltPressed)); - } - void InputTest::InvalidKeyEvent() { // Certain applications like AutoHotKey and its keyboard remapping feature, diff --git a/src/cascadia/WindowsTerminal/main.cpp b/src/cascadia/WindowsTerminal/main.cpp index 0bf491de7..7fb2523a5 100644 --- a/src/cascadia/WindowsTerminal/main.cpp +++ b/src/cascadia/WindowsTerminal/main.cpp @@ -82,6 +82,10 @@ static bool _messageIsAltKeyup(const MSG& message) { return (message.message == WM_KEYUP || message.message == WM_SYSKEYUP) && message.wParam == VK_MENU; } +static bool _messageIsAltSpaceKeypress(const MSG& message) +{ + return message.message == WM_SYSKEYDOWN && message.wParam == VK_SPACE; +} int __stdcall wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int) { @@ -171,6 +175,18 @@ int __stdcall wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int) } } + // GH#7125 = System XAML will show a system dialog on Alt Space. We might want to + // explicitly prevent that - for example when an action is bound to it. So similar to + // above, we steal the event and hand it off to the host. When the host does not process + // it, we will still dispatch like normal. + if (_messageIsAltSpaceKeypress(message)) + { + if (host.OnDirectKeyEvent(VK_SPACE, LOBYTE(HIWORD(message.lParam)), true)) + { + continue; + } + } + TranslateMessage(&message); DispatchMessage(&message); }