Enable Win+key combos as toggle hotkeys and intercept in dialog#4434
Open
daniele-liprandi wants to merge 4 commits intoFlow-Launcher:devfrom
Open
Enable Win+key combos as toggle hotkeys and intercept in dialog#4434daniele-liprandi wants to merge 4 commits intoFlow-Launcher:devfrom
daniele-liprandi wants to merge 4 commits intoFlow-Launcher:devfrom
Conversation
…sterGlobalKeyboardCallback as fallback when RegisterHotKey fails for system-reserved combinations (Win+A, Win+D, etc.). Also intercepts Win+key in the hotkey dialog so the user can type them without triggering system actions.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enables using Windows-reserved Win+<key> combinations as the main “Open Flow Launcher” toggle hotkey by falling back to the existing low-level keyboard hook when RegisterHotKey registration fails, and temporarily intercepts Win+<key> while the hotkey capture dialog is open to avoid triggering Windows system actions.
Changes:
- Add a low-level-hook fallback path for registering
Win+<key>toggle hotkeys when normal hotkey registration fails. - Register a temporary global keyboard interceptor while the hotkey capture dialog is open (for the main toggle hotkey).
- Adjust hotkey availability checks to allow
Win+<key>for the main toggle hotkey, and make the pre-build taskkill step non-fatal.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| Flow.Launcher/HotkeyControlDialog.xaml.cs | Adds temporary global keyboard interception during hotkey capture; loosens availability checks for main toggle Win+<key>. |
| Flow.Launcher/HotkeyControl.xaml.cs | Bypasses availability checks for main toggle Win+<key> combos. |
| Flow.Launcher/Helper/HotKeyMapper.cs | Falls back to a global keyboard hook callback for Win+<key> when normal registration throws; removes callback on hotkey removal. |
| Flow.Launcher/Flow.Launcher.csproj | Makes the PreBuild taskkill step ignore exit codes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+113
to
+126
| _winComboCallback = (keyEvent, vkCode, state) => | ||
| { | ||
| if (keyEvent == (int)KeyEvent.WM_KEYDOWN | ||
| && vkCode == expectedVkCode | ||
| && state.WinPressed | ||
| && state.CtrlPressed == needCtrl | ||
| && state.AltPressed == needAlt | ||
| && state.ShiftPressed == needShift) | ||
| { | ||
| OnToggleHotkeyWithChefKeys(); | ||
| return false; | ||
| } | ||
| return true; | ||
| }; |
Comment on lines
+106
to
+130
| private static void SetWithGlobalCallback(HotkeyModel hotkey) | ||
| { | ||
| int expectedVkCode = KeyInterop.VirtualKeyFromKey(hotkey.CharKey); | ||
| bool needCtrl = hotkey.Ctrl; | ||
| bool needAlt = hotkey.Alt; | ||
| bool needShift = hotkey.Shift; | ||
|
|
||
| _winComboCallback = (keyEvent, vkCode, state) => | ||
| { | ||
| if (keyEvent == (int)KeyEvent.WM_KEYDOWN | ||
| && vkCode == expectedVkCode | ||
| && state.WinPressed | ||
| && state.CtrlPressed == needCtrl | ||
| && state.AltPressed == needAlt | ||
| && state.ShiftPressed == needShift) | ||
| { | ||
| OnToggleHotkeyWithChefKeys(); | ||
| return false; | ||
| } | ||
| return true; | ||
| }; | ||
|
|
||
| _winComboHotkeyStr = hotkey.ToString(); | ||
| App.API.RegisterGlobalKeyboardCallback(_winComboCallback); | ||
| } |
| catch (Exception e) | ||
| { | ||
| if (hotkey.Win && hotkey.CharKey != Key.None) | ||
| { |
Comment on lines
+62
to
+84
| if (isOpenFlowHotkey) | ||
| { | ||
| _winComboInterceptor = (keyEvent, vkCode, state) => | ||
| { | ||
| const int VK_LWIN = 0x5B; | ||
| const int VK_RWIN = 0x5C; | ||
| if (keyEvent == (int)KeyEvent.WM_KEYDOWN | ||
| && state.WinPressed | ||
| && vkCode != VK_LWIN && vkCode != VK_RWIN) | ||
| { | ||
| var key = KeyInterop.KeyFromVirtualKey(vkCode); | ||
| _ = App.Current.Dispatcher.InvokeAsync(() => | ||
| { | ||
| var hotkeyModel = new HotkeyModel(state.AltPressed, state.ShiftPressed, true, state.CtrlPressed, key); | ||
| CurrentHotkey = hotkeyModel; | ||
| SetKeysToDisplay(CurrentHotkey); | ||
| }); | ||
| return false; | ||
| } | ||
| return true; | ||
| }; | ||
| App.API.RegisterGlobalKeyboardCallback(_winComboInterceptor); | ||
| } |
Comment on lines
+223
to
226
| if (isOpenFlowHotkey && (hotkey.ToString() == "LWin" || hotkey.ToString() == "RWin" | ||
| || (hotkey.Win && hotkey.CharKey != Key.None))) | ||
| return true; | ||
|
|
Comment on lines
280
to
285
| // TODO: This is a temporary way to enforce changing only the open flow hotkey to Win, and will be removed by PR #3157 | ||
| if (keyModel.ToString() == "LWin" || keyModel.ToString() == "RWin") | ||
| if (keyModel.ToString() == "LWin" || keyModel.ToString() == "RWin" | ||
| || (Type == HotkeyType.Hotkey && keyModel.Win && keyModel.CharKey != Key.None)) | ||
| { | ||
| hotkeyAvailable = true; | ||
| } |
Contributor
📝 WalkthroughWalkthroughAdds a fallback global keyboard callback for Win+char hotkeys when native registration fails, broadens Win-modified hotkey validation in UI components, makes global keyboard callback registration/removal thread-safe, and makes PreBuild Changes
Sequence DiagramsequenceDiagram
participant User
participant Dialog as HotkeyControlDialog
participant Control as HotkeyControl
participant Mapper as HotKeyMapper
participant HKMgr as HotkeyManager
participant Kbd as GlobalKeyboard
User->>Dialog: Open dialog / press Win+Key
Dialog->>Kbd: Install dialog interceptor (global hook)
Note over User,Kbd: User presses Win+X
Kbd->>Dialog: Interceptor detects Win+X (WM_KEYDOWN)
Dialog->>Dialog: Validate and set CurrentHotkey
User->>Dialog: Click Save
Dialog->>Control: SetHotkey(Win+X)
Control->>Mapper: AddOrReplace(Win+X)
Mapper->>HKMgr: Try register hotkey
alt Registration succeeds
HKMgr-->>Mapper: Success
else Registration throws
Mapper->>Kbd: Register fallback global callback for Win+X
Kbd-->>Mapper: Fallback callback registered
end
Mapper-->>Control: Return registration result
Dialog->>Kbd: Unregister dialog interceptor
Dialog-->>User: Close
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Flow.Launcher/Helper/HotKeyMapper.cs (1)
87-130:⚠️ Potential issue | 🟠 MajorFallback ignores
actionand uses single-slot static state — wrong action for non-toggle hotkeys and leaks on subsequent Win+key registrations.Two related defects in this fallback path:
actionparameter is dropped.SetWithGlobalCallbackunconditionally invokesOnToggleHotkeyWithChefKeys()(line 122). ButSetHotkey(HotkeyModel, EventHandler<HotkeyEventArgs>)is called from multiple sites with different actions:
SetCustomQueryHotkey(lines 184‑193) for plugin custom hotkeysInitializeforDialogJump.OnToggleHotkey(line 32)- The main toggle via
OnToggleHotkeyIf any non-main hotkey is configured as
Win + <key>andAddOrReplacethrows, the catch block at lines 89‑93 falls back, and pressing the hotkey will incorrectly toggle the main launcher instead of running the intended action (plugin query / dialog jump). The PR description scopes this feature to the main toggle, but the code path is not gated and HotkeyControl's bypass only affects UI validation, not the runtime SetHotkey path.Single static slot leaks.
_winComboCallback/_winComboHotkeyStronly hold one registration. If a second Win+key hotkey hits this catch (e.g., user has main hotkeyWin+Aand a custom plugin hotkeyWin+B),SetWithGlobalCallbackreassigns_winComboCallback(line 113) without first callingApp.API.RemoveGlobalKeyboardCallbackfor the old one. The old callback remains registered with the low-level hook (still firing, still toggling launcher), andRemoveHotkeycan never reach it because_winComboHotkeyStrnow points to the newer hotkey.🔧 Suggested fixes
Minimal patch — gate the fallback to the main toggle action and remove any prior callback before re-registering. For full multi-hotkey support, switch to a
Dictionary<string, Func<...>>keyed byhotkeyStr, store theaction, and dispatch through it on a match.catch (Exception e) { - if (hotkey.Win && hotkey.CharKey != Key.None) - { - SetWithGlobalCallback(hotkey); - return; - } + // Fallback only supports the main toggle; non-toggle actions would otherwise + // be silently replaced by OnToggleHotkeyWithChefKeys. + if (hotkey.Win && hotkey.CharKey != Key.None && action == OnToggleHotkey) + { + SetWithGlobalCallback(hotkey); + return; + } @@ private static void SetWithGlobalCallback(HotkeyModel hotkey) { + // Avoid leaking a previously-registered hook callback. + if (_winComboCallback != null) + { + App.API.RemoveGlobalKeyboardCallback(_winComboCallback); + _winComboCallback = null; + _winComboHotkeyStr = null; + } + int expectedVkCode = KeyInterop.VirtualKeyFromKey(hotkey.CharKey);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher/Helper/HotKeyMapper.cs` around lines 87 - 130, The fallback SetWithGlobalCallback currently drops the passed-in action and overwrites a single static slot (_winComboCallback/_winComboHotkeyStr), causing non-toggle hotkeys to invoke OnToggleHotkeyWithChefKeys and leaking previous callbacks; fix by (1) making SetHotkey/SetWithGlobalCallback preserve and invoke the provided action delegate instead of unconditionally calling OnToggleHotkeyWithChefKeys (use the EventHandler<HotkeyEventArgs> passed into SetHotkey or store it on the HotkeyModel), (2) before assigning a new _winComboCallback/_winComboHotkeyStr call App.API.RemoveGlobalKeyboardCallback for the existing _winComboCallback to unregister the old hook, and (3) optionally convert the single-slot storage into a dictionary keyed by hotkeyStr mapping to the specific action to support multiple concurrent Win+key fallbacks.
🧹 Nitpick comments (4)
Flow.Launcher/HotkeyControlDialog.xaml.cs (3)
38-38:isOpenFlowHotkeyshould be an instance field.This is a pre-existing
staticfield, but the PR now relies on it from the constructor to decide whether to install a global hook. If twoHotkeyControlDialoginstances ever exist concurrently (or one is constructed while another is hiding/closing), they will clobber each other's value, and the constructor's hook decision can race withCheckHotkeyAvailability. Promoting it to an instance field makes the new logic self-consistent.- private static bool isOpenFlowHotkey; + private bool isOpenFlowHotkey;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher/HotkeyControlDialog.xaml.cs` at line 38, The field isOpenFlowHotkey is currently static and can be clobbered between concurrent HotkeyControlDialog instances; change it to an instance field (remove static) so each HotkeyControlDialog has its own isOpenFlowHotkey state, update any references in the constructor and methods like CheckHotkeyAvailability and the global hook install/uninstall logic to use the instance field instead of the class-level static, and ensure any static accessors/usages are adjusted to reference the instance (this.isOpenFlowHotkey) so hook installation decisions are consistent per-instance.
223-225: Bypass mirrorsHotkeyControl.SetHotkey— sameValidate()skip applies here.Same observation as in
HotkeyControl.xaml.cs(lines 281‑285): when this branch fires we skip bothValidate(...)andHotKeyMapper.CheckAvailability(...). For the open-flow case we additionally accept literalLWin/RWin(already correct) plus anyWin + CharKey, but a malformed combo likeWin + LeftCtrlwill be reported as available even though the downstream registration (KeyInterop.VirtualKeyFromKeyinSetWithGlobalCallback) won't produce a meaningful trigger. Consider keeping thehotkey.Validate(validateKeyGesture)gate alongside the Win-bypass.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher/HotkeyControlDialog.xaml.cs` around lines 223 - 225, The open-flow bypass in HotkeyControlDialog.xaml.cs currently returns true when isOpenFlowHotkey and a Win key pattern is detected, which skips hotkey.Validate(validateKeyGesture) and HotKeyMapper.CheckAvailability; update the branch so it still calls hotkey.Validate(validateKeyGesture) (and only bypasses availability checks for the intended Win+CharKey / LWin / RWin cases) before returning true, mirroring the fix applied to HotkeyControl.SetHotkey; ensure SetWithGlobalCallback and KeyInterop.VirtualKeyFromKey will still receive only validated combos to avoid malformed Win+modifier combos being treated as available.
64-82: Edge cases for non-character vkCodes and stale-dialog dispatch.A few smaller concerns in the same block:
KeyInterop.KeyFromVirtualKey(vkCode)returnsKey.Nonefor some vkCodes (e.g., media/browser keys, IME keys). The resultingHotkeyModelwould haveCharKey == Key.None, which the dialog's existing validation will reject anyway, but the displayed key sequence inKeysToDisplaywill momentarily look broken.- When a modifier-only press reaches here (e.g., user presses Win+Ctrl with no character),
vkCodeisVK_CONTROLandKeyInteropreturnsKey.LeftCtrl/Key.RightCtrl. The constructedHotkeyModel(..., true, state.CtrlPressed, Key.LeftCtrl)will display as something likeWin + Ctrl + Ctrl._ = App.Current.Dispatcher.InvokeAsync(...)is fire-and-forget; if the user closes the dialog right as a Win+key event is in flight, the dispatched lambda will still run against the closed dialog and mutateKeysToDisplay. Harmless but worth checking with aif (!IsLoaded) return;or similar.Winis hardcoded totruewhile the other modifier flags read fromstate— minor inconsistency; usingstate.WinPressedkeeps the source of truth uniform.- if (keyEvent == (int)KeyEvent.WM_KEYDOWN - && state.WinPressed - && vkCode != VK_LWIN && vkCode != VK_RWIN) - { - var key = KeyInterop.KeyFromVirtualKey(vkCode); + if (keyEvent == (int)KeyEvent.WM_KEYDOWN + && state.WinPressed + && vkCode != VK_LWIN && vkCode != VK_RWIN) + { + var key = KeyInterop.KeyFromVirtualKey(vkCode); + if (key is Key.None + or Key.LeftCtrl or Key.RightCtrl + or Key.LeftAlt or Key.RightAlt + or Key.LeftShift or Key.RightShift) + { + return false; // suppress, but don't update display with a non-character key + } _ = App.Current.Dispatcher.InvokeAsync(() => { - var hotkeyModel = new HotkeyModel(state.AltPressed, state.ShiftPressed, true, state.CtrlPressed, key); + var hotkeyModel = new HotkeyModel(state.AltPressed, state.ShiftPressed, state.WinPressed, state.CtrlPressed, key); CurrentHotkey = hotkeyModel; SetKeysToDisplay(CurrentHotkey); }); return false; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher/HotkeyControlDialog.xaml.cs` around lines 64 - 82, In the _winComboInterceptor lambda fix three issues: (1) after creating key via KeyInterop.KeyFromVirtualKey(vkCode) ignore non-character results (Key.None or other non-printable/media/IME keys) so you don't momentarily display a broken sequence; (2) detect modifier-only vkCodes (e.g., VK_CONTROL) and avoid treating the vkCode as the character key—if the vkCode maps to a modifier key, set the CharKey to None instead of duplicating the modifier in HotkeyModel; (3) replace the hardcoded true with state.WinPressed when constructing HotkeyModel and make the Dispatcher invocation safe by checking dialog state before mutating (e.g., skip updating if !IsLoaded or verify the window is still open inside the dispatched lambda) so closing the dialog won't leave stale UI updates; use the existing symbols KeyInterop.KeyFromVirtualKey, HotkeyModel, CurrentHotkey, SetKeysToDisplay, App.Current.Dispatcher.InvokeAsync and IsLoaded to locate and implement these changes.Flow.Launcher/Helper/HotKeyMapper.cs (1)
87-103: OriginalAddOrReplaceexception is silently swallowed on the fallback branch.When
hotkey.Win && hotkey.CharKey != Key.None, the catch returns immediately without logging the original exception fromHotkeyManager.Current.AddOrReplace. If the fallback registration ever produces unexpected behavior, there will be no breadcrumb explaining why the standard path failed. Consider a debug/info log before the early return.if (hotkey.Win && hotkey.CharKey != Key.None) { + App.API.LogDebug(ClassName, + $"|HotkeyMapper.SetHotkey|AddOrReplace failed for {hotkeyStr} ({e.Message}); falling back to global keyboard callback."); SetWithGlobalCallback(hotkey); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher/Helper/HotKeyMapper.cs` around lines 87 - 103, The catch block for failures from HotkeyManager.Current.AddOrReplace swallows the original exception when taking the fallback path (hotkey.Win && hotkey.CharKey != Key.None) by returning immediately; add a log entry (e.g., App.API.LogDebug or LogInfo) that includes the caught Exception (e.Message and/or e.StackTrace) and context (hotkeyStr and that AddOrReplace failed) immediately before calling SetWithGlobalCallback(hotkey) so the original error is recorded while preserving the existing fallback behavior in SetWithGlobalCallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Flow.Launcher/Helper/HotKeyMapper.cs`:
- Around line 142-148: _globalKeyboardHandlers is accessed concurrently
(iterated in KListener_hookedKeyboardCallback on the hook thread and modified in
RegisterGlobalKeyboardCallback/RemoveGlobalKeyboardCallback) causing a race; fix
by making access thread-safe: either wrap the foreach in
KListener_hookedKeyboardCallback and the Add/Remove in
RegisterGlobalKeyboardCallback/RemoveGlobalKeyboardCallback with a common lock
object, or replace the List<T> _globalKeyboardHandlers with a thread-safe
collection (e.g., ConcurrentBag<T>) and update usages accordingly so iteration
and modification are safe.
In `@Flow.Launcher/HotkeyControl.xaml.cs`:
- Around line 281-285: The current branch sets hotkeyAvailable = true for Win or
Win+modifier combos without invoking HotkeyModel.Validate or CheckAvailability,
which allows invalid combos like Win+LeftCtrl; change the branch to call the
existing validation path instead of bypassing it: when detecting LWin/RWin or
(Type == HotkeyType.Hotkey && keyModel.Win && keyModel.CharKey != Key.None)
invoke keyModel.Validate(...) (or the same CheckAvailability helper used
elsewhere) and set hotkeyAvailable only if that validation returns true (or
CheckAvailability returns true), ensuring the same rejection rules for CharKey
and modifier combinations are enforced.
In `@Flow.Launcher/HotkeyControlDialog.xaml.cs`:
- Around line 64-82: The interceptor _winComboInterceptor currently returns
false for any WM_KEYDOWN where state.WinPressed and vkCode not in
{VK_LWIN,VK_RWIN}, which suppresses all OS Win+key shortcuts; modify the lambda
so it only suppresses (returns false) when the dialog is actively capturing a
hotkey (introduce/consult an isCapturing flag) or implement a small allow-list
for system shortcuts (e.g., VK_L + VK_TAB, VK_L + VK_D, VK_L + VK_L for lock)
before returning false; make the change inside the _winComboInterceptor lambda
(around KeyEvent.WM_KEYDOWN handling) and keep the existing behavior of setting
CurrentHotkey and calling SetKeysToDisplay only when you actually capture the
hotkey.
- Around line 62-84: The global keyboard hook callback _winComboInterceptor is
registered via App.API.RegisterGlobalKeyboardCallback in the ContentDialog
constructor but only unregistered from the Save and Cancel handlers
(UnregisterWinComboInterceptor), which leaks the interceptor when the dialog
closes other ways; fix by tying cleanup to the dialog lifecycle: override the
dialog's OnClosed (or attach to Unloaded/Closed) and call
UnregisterWinComboInterceptor there (make UnregisterWinComboInterceptor
idempotent/safe to call multiple times so Save/Cancel can remain as defensive
duplicates), and ensure any pending Dispatcher.InvokeAsync callbacks on the
closed dialog are guarded or canceled if necessary.
---
Outside diff comments:
In `@Flow.Launcher/Helper/HotKeyMapper.cs`:
- Around line 87-130: The fallback SetWithGlobalCallback currently drops the
passed-in action and overwrites a single static slot
(_winComboCallback/_winComboHotkeyStr), causing non-toggle hotkeys to invoke
OnToggleHotkeyWithChefKeys and leaking previous callbacks; fix by (1) making
SetHotkey/SetWithGlobalCallback preserve and invoke the provided action delegate
instead of unconditionally calling OnToggleHotkeyWithChefKeys (use the
EventHandler<HotkeyEventArgs> passed into SetHotkey or store it on the
HotkeyModel), (2) before assigning a new _winComboCallback/_winComboHotkeyStr
call App.API.RemoveGlobalKeyboardCallback for the existing _winComboCallback to
unregister the old hook, and (3) optionally convert the single-slot storage into
a dictionary keyed by hotkeyStr mapping to the specific action to support
multiple concurrent Win+key fallbacks.
---
Nitpick comments:
In `@Flow.Launcher/Helper/HotKeyMapper.cs`:
- Around line 87-103: The catch block for failures from
HotkeyManager.Current.AddOrReplace swallows the original exception when taking
the fallback path (hotkey.Win && hotkey.CharKey != Key.None) by returning
immediately; add a log entry (e.g., App.API.LogDebug or LogInfo) that includes
the caught Exception (e.Message and/or e.StackTrace) and context (hotkeyStr and
that AddOrReplace failed) immediately before calling
SetWithGlobalCallback(hotkey) so the original error is recorded while preserving
the existing fallback behavior in SetWithGlobalCallback.
In `@Flow.Launcher/HotkeyControlDialog.xaml.cs`:
- Line 38: The field isOpenFlowHotkey is currently static and can be clobbered
between concurrent HotkeyControlDialog instances; change it to an instance field
(remove static) so each HotkeyControlDialog has its own isOpenFlowHotkey state,
update any references in the constructor and methods like
CheckHotkeyAvailability and the global hook install/uninstall logic to use the
instance field instead of the class-level static, and ensure any static
accessors/usages are adjusted to reference the instance (this.isOpenFlowHotkey)
so hook installation decisions are consistent per-instance.
- Around line 223-225: The open-flow bypass in HotkeyControlDialog.xaml.cs
currently returns true when isOpenFlowHotkey and a Win key pattern is detected,
which skips hotkey.Validate(validateKeyGesture) and
HotKeyMapper.CheckAvailability; update the branch so it still calls
hotkey.Validate(validateKeyGesture) (and only bypasses availability checks for
the intended Win+CharKey / LWin / RWin cases) before returning true, mirroring
the fix applied to HotkeyControl.SetHotkey; ensure SetWithGlobalCallback and
KeyInterop.VirtualKeyFromKey will still receive only validated combos to avoid
malformed Win+modifier combos being treated as available.
- Around line 64-82: In the _winComboInterceptor lambda fix three issues: (1)
after creating key via KeyInterop.KeyFromVirtualKey(vkCode) ignore non-character
results (Key.None or other non-printable/media/IME keys) so you don't
momentarily display a broken sequence; (2) detect modifier-only vkCodes (e.g.,
VK_CONTROL) and avoid treating the vkCode as the character key—if the vkCode
maps to a modifier key, set the CharKey to None instead of duplicating the
modifier in HotkeyModel; (3) replace the hardcoded true with state.WinPressed
when constructing HotkeyModel and make the Dispatcher invocation safe by
checking dialog state before mutating (e.g., skip updating if !IsLoaded or
verify the window is still open inside the dispatched lambda) so closing the
dialog won't leave stale UI updates; use the existing symbols
KeyInterop.KeyFromVirtualKey, HotkeyModel, CurrentHotkey, SetKeysToDisplay,
App.Current.Dispatcher.InvokeAsync and IsLoaded to locate and implement these
changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0602b0e4-32d3-412c-9fd7-fe48e5c1dda1
📒 Files selected for processing (4)
Flow.Launcher/Flow.Launcher.csprojFlow.Launcher/Helper/HotKeyMapper.csFlow.Launcher/HotkeyControl.xaml.csFlow.Launcher/HotkeyControlDialog.xaml.cs
PublicAPIInstance.cs Added _globalKeyboardHandlersLock object RegisterGlobalKeyboardCallback and RemoveGlobalKeyboardCallback now lock before mutating the list KListener_hookedKeyboardCallback snapshots the list under lock before iterating — prevents InvalidOperationException when the dialog registers/unregisters mid-callback HotKeyMapper.cs Logs the original RegisterHotKey exception (debug level) before falling back SetWithGlobalCallback now unregisters any previous callback before registering a new one — fixes the stacking leak on hotkey changes Accepts and invokes the action delegate instead of always calling OnToggleHotkeyWithChefKeys — fixes wrong action for non-toggle Win+key hotkeys Tracks keyCurrentlyDown via WM_KEYDOWN/WM_KEYUP — fires the action only once per physical press, preventing the open/close flicker from key auto-repeat HotkeyControlDialog.xaml.cs isOpenFlowHotkey changed from static to instance field; CheckHotkeyAvailability changed to non-static accordingly — no concurrent-instance clobbering this.Closed += UnregisterWinComboInterceptor added — cleanup now happens regardless of how the dialog closes (Esc, system button, etc.) Interceptor lambda guards against modifier-only vkCode values (Ctrl, Alt, Shift, Win) — won't show broken combos like Win+Ctrl Uses state.WinPressed instead of hardcoded true; guards dispatcher lambda with !IsLoaded CheckHotkeyAvailability Win+key fast-path now calls hotkey.Validate() — rejects Win+LeftShift and similar invalid combos HotkeyControl.xaml.cs Win+key bypass in SetHotkey now calls keyModel.Validate(ValidateKeyGesture) instead of unconditionally setting hotkeyAvailable = true — same invalid-combo rejection
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
Flow.Launcher/HotkeyControlDialog.xaml.cs (1)
68-90:⚠️ Potential issue | 🟠 MajorDialog interceptor still suppresses all Win+key OS shortcuts while open.
This still blocks system Win shortcuts globally during capture (e.g., Win+L/Win+D/Win+E), not just assignment behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher/HotkeyControlDialog.xaml.cs` around lines 68 - 90, The handler is suppressing OS Win+key shortcuts by returning false after capturing a hotkey; change the behavior so the OS still receives system Win+key combos: inside the KeyEvent.WM_KEYDOWN branch (where state.WinPressed and vkCode != VK_LWIN && vkCode != VK_RWIN) keep creating the HotkeyModel, setting CurrentHotkey and calling SetKeysToDisplay, but replace the final return false in that branch with return true (or otherwise allow the event to propagate) so system shortcuts (Win+L/Win+D/Win+E, etc.) are not blocked; this affects the block that references KeyEvent.WM_KEYDOWN, state.WinPressed, vkCode, CurrentHotkey, SetKeysToDisplay and HotkeyModel.
🧹 Nitpick comments (1)
Flow.Launcher/Helper/HotKeyMapper.cs (1)
89-95: Scope Win-fallback explicitly to the main toggle hotkey.This branch currently enables fallback for any Win+char hotkey. Since
_winComboCallbackis singleton state, a later fallback registration can replace the main toggle callback unexpectedly.♻️ Narrow fallback scope
- if (hotkey.Win && hotkey.CharKey != Key.None) + if (action == OnToggleHotkey && hotkey.Win && hotkey.CharKey != Key.None) { App.API.LogDebug(ClassName, $"|HotkeyMapper.SetHotkey|RegisterHotKey failed for {hotkeyStr} ({e.Message}); falling back to global keyboard callback."); SetWithGlobalCallback(hotkey, action); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher/Helper/HotKeyMapper.cs` around lines 89 - 95, The current Win+char fallback in HotKeyMapper.SetHotkey unconditionally calls SetWithGlobalCallback and can overwrite the singleton _winComboCallback; restrict this fallback so it only runs for the main toggle hotkey. Modify the branch in SetHotkey to check if the hotkey being registered is the app's toggle hotkey (e.g., compare hotkey to Hotkeys.ToggleHotkey or call a helper IsMainToggleHotkey(hotkey)) and only call SetWithGlobalCallback(hotkey, action) and return when that check is true; otherwise log the failure and surface/throw the registration error without switching the global _winComboCallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Flow.Launcher/Helper/HotKeyMapper.cs`:
- Around line 131-140: The matcher only checks KeyEvent.WM_KEYDOWN/WM_KEYUP and
latches keyCurrentlyDown only when isMatch is true, causing missed WM_SYS*
events and a stuck latch if the modifier (Win) is released first; update the
conditional logic in HotKeyMapper.cs to treat WM_SYSKEYDOWN and WM_SYSKEYUP the
same as WM_KEYDOWN/WM_KEYUP (i.e., consider keyEvent == KeyEvent.WM_KEYDOWN ||
keyEvent == KeyEvent.WM_SYSKEYDOWN for down and similarly for up), invoke action
when isMatch and a down event occurs, and also always clear keyCurrentlyDown on
any up event (WM_KEYUP or WM_SYSKEYUP) regardless of isMatch so the latch resets
even when the modifier was released earlier.
In `@Flow.Launcher/HotkeyControlDialog.xaml.cs`:
- Around line 68-70: The interceptor's event-type filter only checks
KeyEvent.WM_KEYDOWN, so Alt-modified keys (WM_SYSKEYDOWN) are ignored; update
the conditional that checks keyEvent in the capture path (the branch using
KeyEvent.WM_KEYDOWN, state.WinPressed, vkCode, VK_LWIN, VK_RWIN and
state.AltPressed) to also accept KeyEvent.WM_SYSKEYDOWN (i.e., treat
WM_SYSKEYDOWN the same as WM_KEYDOWN) so Win+Alt combinations are captured when
state.WinPressed and state.AltPressed are set.
In `@Flow.Launcher/PublicAPIInstance.cs`:
- Around line 672-673: The loop that invokes each delegate in snapshot (foreach
(var x in snapshot) continueHook &= x((int)keyevent, vkcode, state);) must
isolate exceptions so one handler can't abort the entire hook dispatch; change
the body to call each delegate inside a try/catch, catch and log the exception
(including which delegate or relevant context), and still continue iterating,
applying the returned bool to continueHook only when the invocation succeeds;
keep using snapshot, continueHook, keyevent, vkcode and state to locate the
change.
---
Duplicate comments:
In `@Flow.Launcher/HotkeyControlDialog.xaml.cs`:
- Around line 68-90: The handler is suppressing OS Win+key shortcuts by
returning false after capturing a hotkey; change the behavior so the OS still
receives system Win+key combos: inside the KeyEvent.WM_KEYDOWN branch (where
state.WinPressed and vkCode != VK_LWIN && vkCode != VK_RWIN) keep creating the
HotkeyModel, setting CurrentHotkey and calling SetKeysToDisplay, but replace the
final return false in that branch with return true (or otherwise allow the event
to propagate) so system shortcuts (Win+L/Win+D/Win+E, etc.) are not blocked;
this affects the block that references KeyEvent.WM_KEYDOWN, state.WinPressed,
vkCode, CurrentHotkey, SetKeysToDisplay and HotkeyModel.
---
Nitpick comments:
In `@Flow.Launcher/Helper/HotKeyMapper.cs`:
- Around line 89-95: The current Win+char fallback in HotKeyMapper.SetHotkey
unconditionally calls SetWithGlobalCallback and can overwrite the singleton
_winComboCallback; restrict this fallback so it only runs for the main toggle
hotkey. Modify the branch in SetHotkey to check if the hotkey being registered
is the app's toggle hotkey (e.g., compare hotkey to Hotkeys.ToggleHotkey or call
a helper IsMainToggleHotkey(hotkey)) and only call SetWithGlobalCallback(hotkey,
action) and return when that check is true; otherwise log the failure and
surface/throw the registration error without switching the global
_winComboCallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e23a0373-6a53-4eb5-bf8f-10be2c3225af
📒 Files selected for processing (4)
Flow.Launcher/Helper/HotKeyMapper.csFlow.Launcher/HotkeyControl.xaml.csFlow.Launcher/HotkeyControlDialog.xaml.csFlow.Launcher/PublicAPIInstance.cs
Comment on lines
+131
to
+140
| if (isMatch && keyEvent == (int)KeyEvent.WM_KEYDOWN && !keyCurrentlyDown) | ||
| { | ||
| keyCurrentlyDown = true; | ||
| action?.Invoke(null, null); | ||
| return false; | ||
| } | ||
| if (isMatch && keyEvent == (int)KeyEvent.WM_KEYUP) | ||
| { | ||
| keyCurrentlyDown = false; | ||
| return false; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the hook emits WM_SYS* events while matcher currently filters on WM_KEY* only.
rg -nP --type=cs -C3 'WM_KEYDOWN|WM_KEYUP|WM_SYSKEYDOWN|WM_SYSKEYUP' \
Flow.Launcher.Infrastructure/Hotkey/GlobalHotkey.cs \
Flow.Launcher/Helper/HotKeyMapper.cs \
Flow.Launcher/HotkeyControlDialog.xaml.csRepository: Flow-Launcher/Flow.Launcher
Length of output: 2731
🏁 Script executed:
# Get the complete HotKeyMapper method to understand the stuck key scenario
rg -n 'class\s|public\s+(bool|void).*\s+Hook|private.*keyCurrentlyDown' \
Flow.Launcher/Helper/HotKeyMapper.cs -A 20 | head -100Repository: Flow-Launcher/Flow.Launcher
Length of output: 858
🏁 Script executed:
# Check the full method context around lines 131-140
sed -n '100,150p' Flow.Launcher/Helper/HotKeyMapper.csRepository: Flow-Launcher/Flow.Launcher
Length of output: 1826
🏁 Script executed:
# Look for any other places where keyCurrentlyDown is accessed/reset
rg -n 'keyCurrentlyDown' Flow.Launcher/Helper/HotKeyMapper.csRepository: Flow-Launcher/Flow.Launcher
Length of output: 295
🏁 Script executed:
# Check if there are tests or other references that show expected behavior
rg -n 'WinComboCallback|SetWithGlobalCallback|keyCurrentlyDown' \
Flow.Launcher/ -t cs --max-count=20Repository: Flow-Launcher/Flow.Launcher
Length of output: 683
🏁 Script executed:
# Look for how KeyEvent enum is defined to understand WM_SYS variants
rg -n 'enum KeyEvent|WM_KEYDOWN|WM_SYSKEYDOWN' Flow.Launcher.Infrastructure/ -t cs -B 2 -A 10Repository: Flow-Launcher/Flow.Launcher
Length of output: 1549
🏁 Script executed:
# Find and display the KeyEvent enum definition
rg -n 'enum KeyEvent' Flow.Launcher.Infrastructure/ -A 20Repository: Flow-Launcher/Flow.Launcher
Length of output: 53
🏁 Script executed:
# Also check if there are any unit tests for the hotkey functionality
fd -e '.*Test.*\.cs$' | xargs rg -l 'HotKeyMapper|SetWithGlobalCallback' 2>/dev/null || echo "No hotkey tests found"Repository: Flow-Launcher/Flow.Launcher
Length of output: 402
🏁 Script executed:
# Search more broadly for KeyEvent enum definition
rg -n 'enum KeyEvent' --type cs -iRepository: Flow-Launcher/Flow.Launcher
Length of output: 132
🏁 Script executed:
# Check in common locations for enums
find . -name '*.cs' -type f | xargs grep -l 'enum KeyEvent' 2>/dev/null | head -5Repository: Flow-Launcher/Flow.Launcher
Length of output: 106
🏁 Script executed:
# Look at the GlobalHotkey file more carefully to understand KeyEvent usage
sed -n '1,50p' Flow.Launcher.Infrastructure/Hotkey/GlobalHotkey.csRepository: Flow-Launcher/Flow.Launcher
Length of output: 1976
🏁 Script executed:
# Get the KeyEvent enum definition
cat Flow.Launcher.Plugin/KeyEvent.csRepository: Flow-Launcher/Flow.Launcher
Length of output: 928
🏁 Script executed:
# Verify the complete flow: check HotkeyControlDialog to see how it uses the matcher
cat -n Flow.Launcher/HotkeyControlDialog.xaml.cs | sed -n '50,80p'Repository: Flow-Launcher/Flow.Launcher
Length of output: 1590
Include WM_SYS key events and fix stuck key latch in fallback matcher.*
The hook emits WM_SYSKEYDOWN/UP events (GlobalHotkey.cs) but the matcher only checks WM_KEYDOWN/UP. Additionally, if Win is released before the character key, keyCurrentlyDown cannot reset because isMatch becomes false (Win is no longer pressed), causing subsequent hotkey triggers to fail.
🔧 Suggested matcher fix
- if (isMatch && keyEvent == (int)KeyEvent.WM_KEYDOWN && !keyCurrentlyDown)
+ if (isMatch
+ && (keyEvent == (int)KeyEvent.WM_KEYDOWN || keyEvent == (int)KeyEvent.WM_SYSKEYDOWN)
+ && !keyCurrentlyDown)
{
keyCurrentlyDown = true;
action?.Invoke(null, null);
return false;
}
- if (isMatch && keyEvent == (int)KeyEvent.WM_KEYUP)
+ if (vkCode == expectedVkCode
+ && (keyEvent == (int)KeyEvent.WM_KEYUP || keyEvent == (int)KeyEvent.WM_SYSKEYUP))
{
keyCurrentlyDown = false;
- return false;
+ return isMatch ? false : true;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (isMatch && keyEvent == (int)KeyEvent.WM_KEYDOWN && !keyCurrentlyDown) | |
| { | |
| keyCurrentlyDown = true; | |
| action?.Invoke(null, null); | |
| return false; | |
| } | |
| if (isMatch && keyEvent == (int)KeyEvent.WM_KEYUP) | |
| { | |
| keyCurrentlyDown = false; | |
| return false; | |
| if (isMatch | |
| && (keyEvent == (int)KeyEvent.WM_KEYDOWN || keyEvent == (int)KeyEvent.WM_SYSKEYDOWN) | |
| && !keyCurrentlyDown) | |
| { | |
| keyCurrentlyDown = true; | |
| action?.Invoke(null, null); | |
| return false; | |
| } | |
| if (vkCode == expectedVkCode | |
| && (keyEvent == (int)KeyEvent.WM_KEYUP || keyEvent == (int)KeyEvent.WM_SYSKEYUP)) | |
| { | |
| keyCurrentlyDown = false; | |
| return isMatch ? false : true; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Flow.Launcher/Helper/HotKeyMapper.cs` around lines 131 - 140, The matcher
only checks KeyEvent.WM_KEYDOWN/WM_KEYUP and latches keyCurrentlyDown only when
isMatch is true, causing missed WM_SYS* events and a stuck latch if the modifier
(Win) is released first; update the conditional logic in HotKeyMapper.cs to
treat WM_SYSKEYDOWN and WM_SYSKEYUP the same as WM_KEYDOWN/WM_KEYUP (i.e.,
consider keyEvent == KeyEvent.WM_KEYDOWN || keyEvent == KeyEvent.WM_SYSKEYDOWN
for down and similarly for up), invoke action when isMatch and a down event
occurs, and also always clear keyCurrentlyDown on any up event (WM_KEYUP or
WM_SYSKEYUP) regardless of isMatch so the latch resets even when the modifier
was released earlier.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Flow.Launcher/Helper/HotKeyMapper.cs (1)
137-140:⚠️ Potential issue | 🟠 MajorClear the debounce latch even when Win is released first.
keyCurrentlyDownis only reset whenisMatchis still true on key-up. If the user releases Win before the character key,state.WinPressedis already false on that key-up event, so the latch never clears and the fallback hotkey stops firing until it is re-registered.🔧 Suggested fix
- if (isMatch && (keyEvent == (int)KeyEvent.WM_KEYUP || keyEvent == (int)KeyEvent.WM_SYSKEYUP)) + if (vkCode == expectedVkCode + && (keyEvent == (int)KeyEvent.WM_KEYUP || keyEvent == (int)KeyEvent.WM_SYSKEYUP)) { keyCurrentlyDown = false; - return false; + return isMatch ? false : true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher/Helper/HotKeyMapper.cs` around lines 137 - 140, The latch (keyCurrentlyDown) isn't cleared when Win is released before the character key because the current key-up branch only clears it when isMatch is true; change the logic in HotKeyMapper.cs so that on any matching key-up event (keyEvent == WM_KEYUP or WM_SYSKEYUP for the same virtual key/code) you set keyCurrentlyDown = false and return appropriately regardless of state.WinPressed/isMatch; preserve any existing return value behavior but ensure keyCurrentlyDown is always reset when the character key is released so the fallback hotkey can re-register.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Flow.Launcher/Helper/HotKeyMapper.cs`:
- Around line 21-22: The current singleton fallback fields _winComboCallback and
_winComboHotkeyStr cause later calls to SetHotkey/SetWithGlobalCallback (used
for _settings.Hotkey, _settings.DialogJumpHotkey, and CustomPluginHotkey) to
overwrite and disable earlier Win+char fallbacks; fix this by either gating the
fallback path so it only runs for the main toggle caller (e.g., check the caller
identity or a boolean flag before creating the global low-level hook) or replace
the single static slots with a per-hotkey registry (e.g., a Dictionary keyed by
hotkey string that stores each hotkey's Func<int,int,SpecialKeyState,bool>
callback and its hotkey string) and update the lifecycle code in the
SetWithGlobalCallback and related teardown paths (see the other fallback sites
around the commented ranges) to add/remove entries from that registry instead of
overwriting the single static fields.
---
Duplicate comments:
In `@Flow.Launcher/Helper/HotKeyMapper.cs`:
- Around line 137-140: The latch (keyCurrentlyDown) isn't cleared when Win is
released before the character key because the current key-up branch only clears
it when isMatch is true; change the logic in HotKeyMapper.cs so that on any
matching key-up event (keyEvent == WM_KEYUP or WM_SYSKEYUP for the same virtual
key/code) you set keyCurrentlyDown = false and return appropriately regardless
of state.WinPressed/isMatch; preserve any existing return value behavior but
ensure keyCurrentlyDown is always reset when the character key is released so
the fallback hotkey can re-register.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f99abba8-d047-459a-b201-2162884035ba
📒 Files selected for processing (3)
Flow.Launcher/Helper/HotKeyMapper.csFlow.Launcher/HotkeyControlDialog.xaml.csFlow.Launcher/PublicAPIInstance.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- Flow.Launcher/HotkeyControlDialog.xaml.cs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Flow.Launcher/Helper/HotKeyMapper.cs (1)
131-141:⚠️ Potential issue | 🟠 MajorReset
keyCurrentlyDownon key-up regardless of current modifier state.On Line 137, key-up handling is gated by
isMatch. If Win is released before the character key,isMatchbecomes false and the latch may stay set, causing the next press to be missed.Suggested fix
- if (isMatch && (keyEvent == (int)KeyEvent.WM_KEYUP || keyEvent == (int)KeyEvent.WM_SYSKEYUP)) + if (vkCode == expectedVkCode + && (keyEvent == (int)KeyEvent.WM_KEYUP || keyEvent == (int)KeyEvent.WM_SYSKEYUP)) { keyCurrentlyDown = false; - return false; + return isMatch ? false : true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher/Helper/HotKeyMapper.cs` around lines 131 - 141, The key-up branch currently requires isMatch, which can leave keyCurrentlyDown true if the modifier (e.g., Win) is released before the character key; update the key-up handling in HotKeyMapper.cs so that keyCurrentlyDown is cleared whenever a key-up event (KeyEvent.WM_KEYUP or KeyEvent.WM_SYSKEYUP) occurs, regardless of isMatch. Locate the block checking keyEvent against KeyEvent.WM_KEYUP/WM_SYSKEYUP and remove the isMatch gate so keyCurrentlyDown = false always runs on those key-up events (keep the existing return false behavior and continue invoking action only in the existing key-down branch).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Flow.Launcher/Helper/HotKeyMapper.cs`:
- Around line 89-95: The fallback call to SetWithGlobalCallback in
HotKeyMapper.SetHotkey can throw and currently escapes the outer catch; wrap the
fallback invocation in its own try/catch so any exception is caught, logged via
App.API.LogError/LogDebug with the hotkeyStr and exception details, and then
invoke the existing hotkey error handling/UI path (same dialog or handler used
in the outer catch) instead of letting the exception propagate; apply the same
guard around the other fallback site referenced (lines calling
SetWithGlobalCallback around 146-147) so all fallback registrations are
protected.
---
Duplicate comments:
In `@Flow.Launcher/Helper/HotKeyMapper.cs`:
- Around line 131-141: The key-up branch currently requires isMatch, which can
leave keyCurrentlyDown true if the modifier (e.g., Win) is released before the
character key; update the key-up handling in HotKeyMapper.cs so that
keyCurrentlyDown is cleared whenever a key-up event (KeyEvent.WM_KEYUP or
KeyEvent.WM_SYSKEYUP) occurs, regardless of isMatch. Locate the block checking
keyEvent against KeyEvent.WM_KEYUP/WM_SYSKEYUP and remove the isMatch gate so
keyCurrentlyDown = false always runs on those key-up events (keep the existing
return false behavior and continue invoking action only in the existing key-down
branch).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 670af18b-2edf-4bc3-9a6e-81693b56d42c
📒 Files selected for processing (1)
Flow.Launcher/Helper/HotKeyMapper.cs
Comment on lines
+89
to
+95
| if (hotkey.Win && hotkey.CharKey != Key.None) | ||
| { | ||
| App.API.LogDebug(ClassName, | ||
| $"|HotkeyMapper.SetHotkey|RegisterHotKey failed for {hotkeyStr} ({e.Message}); falling back to global keyboard callback."); | ||
| SetWithGlobalCallback(hotkey, action); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
Guard fallback registration failures to avoid uncaught exceptions.
If SetWithGlobalCallback fails (e.g., Line 146), that exception escapes from the catch block path and bypasses the normal error dialog flow for hotkey setup.
Suggested fix
if (hotkey.Win && hotkey.CharKey != Key.None)
{
- App.API.LogDebug(ClassName,
- $"|HotkeyMapper.SetHotkey|RegisterHotKey failed for {hotkeyStr} ({e.Message}); falling back to global keyboard callback.");
- SetWithGlobalCallback(hotkey, action);
- return;
+ try
+ {
+ App.API.LogDebug(ClassName,
+ $"|HotkeyMapper.SetHotkey|RegisterHotKey failed for {hotkeyStr} ({e.Message}); falling back to global keyboard callback.");
+ SetWithGlobalCallback(hotkey, action);
+ return;
+ }
+ catch (Exception fallbackEx)
+ {
+ App.API.LogError(ClassName,
+ $"|HotkeyMapper.SetHotkey|Fallback registration failed for {hotkeyStr}: {fallbackEx.Message} \nStackTrace:{fallbackEx.StackTrace}");
+ string errorMsg = Localize.registerHotkeyFailed(hotkeyStr);
+ string errorMsgTitle = Localize.MessageBoxTitle();
+ App.API.ShowMsgBox(errorMsg, errorMsgTitle);
+ return;
+ }
}Also applies to: 146-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Flow.Launcher/Helper/HotKeyMapper.cs` around lines 89 - 95, The fallback call
to SetWithGlobalCallback in HotKeyMapper.SetHotkey can throw and currently
escapes the outer catch; wrap the fallback invocation in its own try/catch so
any exception is caught, logged via App.API.LogError/LogDebug with the hotkeyStr
and exception details, and then invoke the existing hotkey error handling/UI
path (same dialog or handler used in the outer catch) instead of letting the
exception propagate; apply the same guard around the other fallback site
referenced (lines calling SetWithGlobalCallback around 146-147) so all fallback
registrations are protected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allow Windows-reserved Win+key combos as the main toggle hotkey
Problem
#1358
Win+key combinations are reserved by Windows and cannot be registered via RegisterHotKey. This made them unavailable in Flow Launcher.
Changes
When HotkeyManager.AddOrReplace fails for a hotkey with the Win modifier, the registration now falls back to RegisterGlobalKeyboardCallback, which uses the existing WH_KEYBOARD_LL low-level keyboard hook that I think was implemented a couple of years back. This hook fires before Windows routes the key to any system handler, so returning false suppresses the system action (Action Center, etc.) and triggers the launcher instead.
The same mechanism is used to intercept Win+key combos while the hotkey capture dialog is open, so the user could potentially press the hotkey again without triggering the default windows combo.
I only added it to "Open Flow Launcher"
Changes
input
Summary by cubic
Enable Windows-reserved Win+key combos as the main toggle hotkey by falling back to a low-level keyboard hook when
RegisterHotKeyfails. Intercepts Win+key in the hotkey dialog, prevents auto-repeat flicker, and hardens global keyboard callback handling.HotKeyMapperfalls back to a global keyboard callback for Win+key; manages callbacks in a dictionary keyed by hotkey; unregisters any previous callback before registering a new one; invokes the provided action; debounces WM_KEYDOWN/UP to fire once per press; logs the originalRegisterHotKeyfailure; relaxes Win+key availability for the main toggle but validates combos. Hotkey dialog/control now validate Win+key viaValidateKeyGesture;isOpenFlowHotkeyis instance-scoped; dialog cleanup runs on any close; interceptor ignores modifier-only keys, usesstate.WinPressed, and guards against!IsLoaded.PublicAPIInstancelocks on mutate, snapshots before iterate, and logs exceptions from callbacks.Flow.Launcher.csprojPreBuild now ignorestaskkillexit codes.Written for commit dc427fe. Summary will update on new commits. Review in cubic