Adding/fixing Alt+Space handling

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<int32_t>('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<int32_t>('C'), 0 };
+
+        auto actionMap = winrt::make_self<implementation::ActionMap>();
+        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"([

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;

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);

src/cascadia/TerminalControl/IKeyBindings.idl

@@ -9,5 +9,6 @@ namespace Microsoft.Terminal.Control
     interface IKeyBindings
     {
         Boolean TryKeyChord(KeyChord kc);
+        Boolean IsKeyChordExplicitlyUnbound(KeyChord kc);
     }
 }

src/cascadia/TerminalControl/TermControl.cpp

@@ -786,28 +786,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<WORD>(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<WORD>(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;

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)
     {

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:

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;
 

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);

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,

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);
     }