Another test case, though this is likely way out of my hands

src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp

@@ -37,10 +37,14 @@ namespace SettingsModelLocalTests
         END_TEST_CLASS()
 
         TEST_METHOD(KeyChords);
+        TEST_METHOD(VkeyAndScanCodeTests);
+
         TEST_METHOD(ManyKeysSameAction);
         TEST_METHOD(LayerKeybindings);
         TEST_METHOD(UnbindKeybindings);
 
+        TEST_METHOD(LayerScancodeKeybindings);
+
         TEST_METHOD(TestArbitraryArgs);
         TEST_METHOD(TestSplitPaneArgs);
 
@@ -95,26 +99,89 @@ namespace SettingsModelLocalTests
                 VirtualKeyModifiers::Control | VirtualKeyModifiers::Menu | VirtualKeyModifiers::Shift | VirtualKeyModifiers::Windows,
                 255,
                 0,
-                L"ctrl+shift+alt+win+vk(255)",
+                L"win+ctrl+alt+shift+vk(255)",
             },
             testCase{
-                VirtualKeyModifiers::Windows,
+                VirtualKeyModifiers::Control | VirtualKeyModifiers::Menu | VirtualKeyModifiers::Shift | VirtualKeyModifiers::Windows,
                 0,
                 123,
-                L"ctrl+shift+alt+win+sc(123)",
+                L"win+ctrl+alt+shift+sc(123)",
             },
         };
 
+        // Use the KeyChordHash and KeyChordEquality to compare if two
+        // KeyChord's are the same. If you try to just directly compare them
+        // with VERIFY_ARE_EQUAL, that will always fail. It'll revert to the
+        // default winrt equality operator, which will compare if they point to
+        // literally the same object (they won't).
+        implementation::KeyChordHash hash;
+        implementation::KeyChordEquality equals;
         for (const auto& tc : testCases)
         {
+            Log::Comment(NoThrowString().Format(L"Testing case:\"%s\"", tc.expected.data()));
+
             KeyChord expectedKeyChord{ tc.modifiers, tc.vkey, tc.scanCode };
             const auto actualString = KeyChordSerialization::ToString(expectedKeyChord);
             VERIFY_ARE_EQUAL(tc.expected, actualString);
+
             const auto actualKeyChord = KeyChordSerialization::FromString(actualString);
-            VERIFY_ARE_EQUAL(expectedKeyChord, actualKeyChord);
+            VERIFY_ARE_EQUAL(hash(expectedKeyChord), hash(actualKeyChord));
+            VERIFY_IS_TRUE(equals(expectedKeyChord, actualKeyChord));
         }
     }
 
+    void KeyBindingsTests::VkeyAndScanCodeTests()
+    {
+        Log::Comment(L"In this test, we're going to construct two KeyChords, "
+                     L"one with a vkey, and one with a scancode. We'll use "
+                     L"MapVirtualKeyW to make sure these are the same key. They"
+                     L" should compare as the same key, but they should "
+                     L"roundtrip as they were initially serialized.");
+
+        struct testCase
+        {
+            VirtualKeyModifiers modifiers;
+            int32_t vkey;
+            int32_t scanCode;
+            std::wstring expected;
+        };
+
+        int32_t vsc = 123;
+        int32_t vk = MapVirtualKeyW(vsc, MAPVK_VSC_TO_VK);
+        static const std::array testCases{
+            testCase{
+                VirtualKeyModifiers::Control,
+                vk,
+                0,
+                fmt::format(L"ctrl+vk({})", vk) },
+            testCase{
+                VirtualKeyModifiers::Control,
+                0,
+                vsc,
+                fmt::format(L"ctrl+sc({})", vsc) }
+        };
+        implementation::KeyChordHash hash;
+        implementation::KeyChordEquality equals;
+        for (const auto& tc : testCases)
+        {
+            Log::Comment(fmt::format(L"Testing case:\"{}\"", tc.expected).c_str());
+
+            KeyChord expectedKeyChord{ tc.modifiers, tc.vkey, tc.scanCode };
+            const auto actualString = KeyChordSerialization::ToString(expectedKeyChord);
+            VERIFY_ARE_EQUAL(tc.expected, actualString);
+
+            const auto actualKeyChord = KeyChordSerialization::FromString(actualString);
+            VERIFY_ARE_EQUAL(hash(expectedKeyChord), hash(actualKeyChord));
+            VERIFY_IS_TRUE(equals(expectedKeyChord, actualKeyChord));
+        }
+
+        KeyChord vkChord{ testCases[0].modifiers, testCases[0].vkey, testCases[0].scanCode };
+        KeyChord scChord{ testCases[1].modifiers, testCases[1].vkey, testCases[1].scanCode };
+
+        VERIFY_ARE_EQUAL(hash(vkChord), hash(scChord));
+        VERIFY_IS_TRUE(equals(vkChord, scChord));
+    }
+
     void KeyBindingsTests::ManyKeysSameAction()
     {
         const std::string bindings0String{ R"([ { "command": "copy", "keys": ["ctrl+c"] } ])" };
@@ -722,4 +789,31 @@ namespace SettingsModelLocalTests
             VerifyKeyChordEquality({ VirtualKeyModifiers::Control | VirtualKeyModifiers::Shift, static_cast<int32_t>('P'), 0 }, kbd);
         }
     }
+
+    void KeyBindingsTests::LayerScancodeKeybindings()
+    {
+        Log::Comment(L"Layering a keybinding with a character literal on top of"
+                     L" an equivalent sc() key should replace it.");
+
+        // Wrap the first one in `R"!(...)!"` because it has `()` internally.
+        const std::string bindings0String{ R"!([ { "command": "quakeMode", "keys":"win+sc(41)" } ])!" };
+        const std::string bindings1String{ R"([ { "keys": "win+`", "command": { "action": "globalSummon", "monitor": "any" } } ])" };
+        const std::string bindings2String{ R"([ { "keys": "ctrl+shift+`", "command": { "action": "quakeMode" } } ])" };
+
+        const auto bindings0Json = VerifyParseSucceeded(bindings0String);
+        const auto bindings1Json = VerifyParseSucceeded(bindings1String);
+        const auto bindings2Json = VerifyParseSucceeded(bindings2String);
+
+        auto actionMap = winrt::make_self<implementation::ActionMap>();
+        VERIFY_ARE_EQUAL(0u, actionMap->_KeyMap.size());
+
+        actionMap->LayerJson(bindings0Json);
+        VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size());
+
+        actionMap->LayerJson(bindings1Json);
+        VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size(), L"Layering the second action should replace the first one.");
+
+        actionMap->LayerJson(bindings2Json);
+        VERIFY_ARE_EQUAL(2u, actionMap->_KeyMap.size());
+    }
 }

src/cascadia/Remoting/Monarch.cpp

@@ -201,6 +201,12 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
             _clearOldMruEntries(id);
         }
 
+        TraceLoggingWrite(g_hRemotingProvider,
+                          "Monarch_lookupPeasantIdForName",
+                          TraceLoggingWideString(winrt::hstring{ name }.c_str(), "name", "the name we're looking for"),
+                          TraceLoggingUInt64(result, "peasantID", "the ID of the peasant with that name"),
+                          TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
+                          TraceLoggingKeyword(TIL_KEYWORD_TRACE));
         return result;
     }
 
@@ -750,6 +756,27 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
             {
                 targetPeasant.Summon(args.SummonBehavior());
                 args.FoundMatch(true);
+
+                TraceLoggingWrite(g_hRemotingProvider,
+                                  "Monarch_SummonWindow_Success",
+                                  TraceLoggingWideString(searchedForName.c_str(), "searchedForName", "The name of the window we tried to summon"),
+                                  TraceLoggingUInt64(windowId, "peasantID", "The id of the window we tried to summon"),
+                                  TraceLoggingBoolean(args.OnCurrentDesktop(), "OnCurrentDesktop", "true iff the window needs to be on the current virtual desktop"),
+                                  TraceLoggingBoolean(args.SummonBehavior().MoveToCurrentDesktop(), "MoveToCurrentDesktop", "if true, move the window to the current virtual desktop"),
+                                  TraceLoggingBoolean(args.SummonBehavior().ToggleVisibility(), "ToggleVisibility", "true if we should toggle the visibity of the window"),
+                                  TraceLoggingUInt32(args.SummonBehavior().DropdownDuration(), "DropdownDuration", "the duration to dropdown the window"),
+                                  TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
+                                  TraceLoggingKeyword(TIL_KEYWORD_TRACE));
+            }
+            else
+            {
+                TraceLoggingWrite(g_hRemotingProvider,
+                                  "Monarch_SummonWindow_NoPeasant",
+                                  TraceLoggingWideString(searchedForName.c_str(), "searchedForName", "The name of the window we tried to summon"),
+                                  TraceLoggingUInt64(windowId, "peasantID", "The id of the window we tried to summon"),
+                                  TraceLoggingBoolean(args.OnCurrentDesktop(), "OnCurrentDesktop", "true iff the window needs to be on the current virtual desktop"),
+                                  TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
+                                  TraceLoggingKeyword(TIL_KEYWORD_TRACE));
             }
         }
         catch (...)

src/cascadia/TerminalSettingsModel/ActionMap.h

@@ -36,7 +36,8 @@ 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(), key.ScanCode());
+            return ::Microsoft::Terminal::Settings::Model::HashUtils::HashProperty(key.Modifiers(),
+                                                                                   key.Vkey() == 0 ? MapVirtualKeyW(key.ScanCode(), MAPVK_VSC_TO_VK) : key.Vkey());
         }
     };
 
@@ -44,7 +45,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     {
         bool operator()(const Control::KeyChord& lhs, const Control::KeyChord& rhs) const
         {
-            return lhs.Modifiers() == rhs.Modifiers() && lhs.Vkey() == rhs.Vkey() && lhs.ScanCode() == rhs.ScanCode();
+            const auto leftVkey = lhs.Vkey() == 0 ? MapVirtualKeyW(lhs.ScanCode(), MAPVK_VSC_TO_VK) : lhs.Vkey();
+            const auto rightVkey = rhs.Vkey() == 0 ? MapVirtualKeyW(rhs.ScanCode(), MAPVK_VSC_TO_VK) : rhs.Vkey();
+            return lhs.Modifiers() == rhs.Modifiers() && leftVkey == rightVkey;
         }
     };
 

src/cascadia/WindowsTerminal/AppHost.cpp

@@ -689,7 +689,17 @@ winrt::fire_and_forget AppHost::_setupGlobalHotkeys()
     {
         if (auto summonArgs = cmd.ActionAndArgs().Args().try_as<Settings::Model::GlobalSummonArgs>())
         {
-            _window->RegisterHotKey(gsl::narrow_cast<int>(_hotkeys.size()), keyChord);
+            int index = gsl::narrow_cast<int>(_hotkeys.size());
+            const bool succeeded = _window->RegisterHotKey(index, keyChord);
+
+            TraceLoggingWrite(g_hWindowsTerminalProvider,
+                              "AppHost_setupGlobalHotkey",
+                              TraceLoggingDescription("Emitted when setting a single hotkey"),
+                              TraceLoggingInt64(index, "index", "the index of the hotkey to add"),
+                              TraceLoggingWideString(cmd.Name().c_str(), "name", "the name of the command"),
+                              TraceLoggingBoolean(succeeded, "succeeded", "true if we succeeded"),
+                              TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
+                              TraceLoggingKeyword(TIL_KEYWORD_TRACE));
             _hotkeys.emplace_back(summonArgs);
         }
     }

src/cascadia/WindowsTerminal/IslandWindow.cpp

@@ -1046,8 +1046,9 @@ void IslandWindow::UnregisterHotKey(const int index) noexcept
 {
     TraceLoggingWrite(
         g_hWindowsTerminalProvider,
-        "UnregisterAllHotKeys",
+        "UnregisterHotKey",
         TraceLoggingDescription("Emitted when clearing previously set hotkeys"),
+        TraceLoggingInt64(index, "index", "the index of the hotkey to remove"),
         TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
         TraceLoggingKeyword(TIL_KEYWORD_TRACE));
 
@@ -1064,15 +1065,8 @@ void IslandWindow::UnregisterHotKey(const int index) noexcept
 // - hotkey: The key-combination to register.
 // Return Value:
 // - <none>
-void IslandWindow::RegisterHotKey(const int index, const winrt::Microsoft::Terminal::Control::KeyChord& hotkey) noexcept
+bool IslandWindow::RegisterHotKey(const int index, const winrt::Microsoft::Terminal::Control::KeyChord& hotkey) noexcept
 {
-    TraceLoggingWrite(
-        g_hWindowsTerminalProvider,
-        "RegisterHotKey",
-        TraceLoggingDescription("Emitted when setting hotkeys"),
-        TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
-        TraceLoggingKeyword(TIL_KEYWORD_TRACE));
-
     auto vkey = hotkey.Vkey();
     if (!vkey)
     {
@@ -1080,7 +1074,7 @@ void IslandWindow::RegisterHotKey(const int index, const winrt::Microsoft::Termi
     }
     if (!vkey)
     {
-        return;
+        return false;
     }
 
     auto hotkeyFlags = MOD_NOREPEAT;
@@ -1094,7 +1088,23 @@ void IslandWindow::RegisterHotKey(const int index, const winrt::Microsoft::Termi
 
     // TODO GH#8888: We should display a warning of some kind if this fails.
     // This can fail if something else already bound this hotkey.
-    LOG_IF_WIN32_BOOL_FALSE(::RegisterHotKey(_window.get(), index, hotkeyFlags, vkey));
+    const auto result = ::RegisterHotKey(_window.get(), index, hotkeyFlags, vkey);
+    LOG_IF_WIN32_BOOL_FALSE(result);
+
+    TraceLoggingWrite(g_hWindowsTerminalProvider,
+                      "RegisterHotKey",
+                      TraceLoggingDescription("Emitted when setting hotkeys"),
+                      TraceLoggingInt64(index, "index", "the index of the hotkey to add"),
+                      TraceLoggingUInt64(vkey, "vkey", "the key"),
+                      TraceLoggingUInt64(WI_IsFlagSet(hotkeyFlags, MOD_WIN), "win", "is WIN in the modifiers"),
+                      TraceLoggingUInt64(WI_IsFlagSet(hotkeyFlags, MOD_ALT), "alt", "is ALT in the modifiers"),
+                      TraceLoggingUInt64(WI_IsFlagSet(hotkeyFlags, MOD_CONTROL), "control", "is CONTROL in the modifiers"),
+                      TraceLoggingUInt64(WI_IsFlagSet(hotkeyFlags, MOD_SHIFT), "shift", "is SHIFT in the modifiers"),
+                      TraceLoggingBool(result, "succeeded", "true if we succeeded"),
+                      TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
+                      TraceLoggingKeyword(TIL_KEYWORD_TRACE));
+
+    return result;
 }
 
 // Method Description:

src/cascadia/WindowsTerminal/IslandWindow.h

@@ -39,7 +39,7 @@ public:
     void SetTaskbarProgress(const size_t state, const size_t progress);
 
     void UnregisterHotKey(const int index) noexcept;
-    void RegisterHotKey(const int index, const winrt::Microsoft::Terminal::Control::KeyChord& hotkey) noexcept;
+    bool RegisterHotKey(const int index, const winrt::Microsoft::Terminal::Control::KeyChord& hotkey) noexcept;
 
     winrt::fire_and_forget SummonWindow(winrt::Microsoft::Terminal::Remoting::SummonWindowBehavior args);