maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Add logging, test for #10875 (#10907)

Browse source 
## Summary of the Pull Request

This isn't a fix for #10875, but it is logging that help identify the root cause here. The logging may additionally be helpful for some of the other issues we're seeing elsewhere in the repo, namely #10340. 

@lhecker is actually working on the fix for #10875, so hopefully this test will help validate.

## References
* Regressed in #10666.
* logging for #8888

## PR Checklist
* [x] Closes nothing
* [x] I work here
* [x] Tests added, and they absolutely fail, but they're localtests, so ¯\\\_(ツ)_/¯
* [n/a] Requires documentation to be updated

## details

While I was here, I noticed that `KeyBindingsTests::KeyChords` has been broken for some time now. So I fixed that too.
This commit is contained in:
Mike Griese 2021-08-11 10:20:15 -05:00 committed by GitHub
parent 42bf605e1c
commit 9c858cd5b8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 106 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

50
src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp
View file

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

27
src/cascadia/Remoting/Monarch.cpp
View file

@ -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 (...)

12
src/cascadia/WindowsTerminal/AppHost.cpp
View file

@ -692,7 +692,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);
}
}

32
src/cascadia/WindowsTerminal/IslandWindow.cpp
View file

@ -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:
// - <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)
{
@ -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:

2
src/cascadia/WindowsTerminal/IslandWindow.h
View file

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