diff --git a/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp b/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp index e5c354d25..4154cf3d4 100644 --- a/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp @@ -40,7 +40,11 @@ namespace SettingsModelLocalTests TEST_METHOD(ManyKeysSameAction); TEST_METHOD(LayerKeybindings); TEST_METHOD(UnbindKeybindings); + + TEST_METHOD(LayerScancodeKeybindings); + TEST_METHOD(TestExplicitUnbind); + TEST_METHOD(TestArbitraryArgs); TEST_METHOD(TestSplitPaneArgs); @@ -95,23 +99,34 @@ 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)); } } @@ -747,4 +762,31 @@ namespace SettingsModelLocalTests VerifyKeyChordEquality({ VirtualKeyModifiers::Control | VirtualKeyModifiers::Shift, static_cast('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(); + 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()); + } } diff --git a/src/cascadia/Remoting/Monarch.cpp b/src/cascadia/Remoting/Monarch.cpp index f65422d3f..a5444e3d4 100644 --- a/src/cascadia/Remoting/Monarch.cpp +++ b/src/cascadia/Remoting/Monarch.cpp @@ -201,6 +201,12 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation _clearOldMruEntries(id); } + TraceLoggingWrite(g_hRemotingProvider, + "Monarch_lookupPeasantIdForName", + TraceLoggingWideString(std::wstring{ 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 visibility 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 (...) diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index 5f821d190..a89cc3756 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -692,7 +692,17 @@ winrt::fire_and_forget AppHost::_setupGlobalHotkeys() { if (auto summonArgs = cmd.ActionAndArgs().Args().try_as()) { - _window->RegisterHotKey(gsl::narrow_cast(_hotkeys.size()), keyChord); + int index = gsl::narrow_cast(_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); } } diff --git a/src/cascadia/WindowsTerminal/IslandWindow.cpp b/src/cascadia/WindowsTerminal/IslandWindow.cpp index 5cfffa4fc..92dc184c5 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/IslandWindow.cpp @@ -1070,8 +1070,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)); @@ -1088,15 +1089,8 @@ void IslandWindow::UnregisterHotKey(const int index) noexcept // - hotkey: The key-combination to register. // Return Value: // - -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) { @@ -1104,7 +1098,7 @@ void IslandWindow::RegisterHotKey(const int index, const winrt::Microsoft::Termi } if (!vkey) { - return; + return false; } auto hotkeyFlags = MOD_NOREPEAT; @@ -1118,7 +1112,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: diff --git a/src/cascadia/WindowsTerminal/IslandWindow.h b/src/cascadia/WindowsTerminal/IslandWindow.h index b0699864d..bea360862 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.h +++ b/src/cascadia/WindowsTerminal/IslandWindow.h @@ -40,7 +40,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);