-
Notifications
You must be signed in to change notification settings - Fork 0
Keyboard shortcuts clean #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Add support for using the fn/Globe key as a shortcut trigger on macOS. The fn key is a modifier-only key that cannot be captured by standard shortcut libraries like tauri-plugin-global-shortcut. Implementation: - Add fn_monitor.rs using NSEvent.addGlobalMonitorForEventsMatchingMask to detect fn key press/release via FlagsChanged events - Add routing layer (register_binding/unregister_binding) to direct fn bindings to fn_monitor and regular bindings to global-shortcut - Add dispatch_binding_event to share action dispatch logic between regular shortcuts and fn key - Allow "fn" as valid binding in validate_shortcut_string (macOS only) - Remove unused rdev dependency, add objc2 dependencies for macOS The fn key requires Accessibility permission, which is already needed for the enigo pasting functionality. Limitations: - fn key events stop when Secure Input is active (password fields) - fn+key combinations conflict with system shortcuts; fn alone is safe
Cancel recording mid-transcription with Escape key. - Dynamic bindings: only registered while recording is active - Disabled on Linux due to global-shortcut plugin instability - Idempotent registration avoids callback deadlocks
Add UI elements for the fn key shortcut on macOS: - "Use fn" button to quickly set fn/Globe key as shortcut - "fn (Globe)" display when fn key is active - Tooltips explaining fn key usage
The fn key shortcut still works on macOS, but users must configure it manually by editing settings.json and setting current_binding to "fn". This commit can be reverted to restore the "Use fn" button in the UI.
WalkthroughThis change introduces dynamic shortcut binding registration for macOS, enabling bindings (particularly the cancel shortcut) to be registered and unregistered at runtime rather than at startup. A new macOS-specific fn-key monitor module provides accessibility permission checks and fn-key event handling via Objective-C bindings. The binding lifecycle is refactored to support this dynamic registration flow. Changes
Sequence DiagramsequenceDiagram
actor User
participant TranscribeAction as TranscribeAction<br/>(start)
participant MainThread as Main Thread<br/>Dispatcher
participant FnMonitor as Fn-Key Monitor
participant macOS as macOS/AX API
participant ActionDispatch as Action<br/>Dispatcher
User->>TranscribeAction: Start recording
TranscribeAction->>MainThread: Schedule dynamic binding<br/>registration (cancel)
MainThread->>FnMonitor: register_fn_binding(cancel)
FnMonitor->>macOS: check_accessibility_permission()
macOS-->>FnMonitor: Permission result
FnMonitor->>macOS: addGlobalMonitorForEventsMatchingMask<br/>(FlagsChanged)
FnMonitor-->>MainThread: Binding registered ✓
MainThread-->>TranscribeAction: Dynamic binding active
rect rgba(100, 200, 100, 0.2)
Note over User,ActionDispatch: Recording in progress
User->>macOS: Press Fn key
macOS->>FnMonitor: FlagsChanged event
FnMonitor->>FnMonitor: Detect Fn press/release
FnMonitor->>ActionDispatch: dispatch_binding_event(cancel)
ActionDispatch-->>User: Cancel signal sent
end
User->>TranscribeAction: Stop recording
TranscribeAction->>MainThread: Schedule dynamic binding<br/>unregistration (cancel)
MainThread->>FnMonitor: unregister_fn_binding(cancel)
FnMonitor->>macOS: Remove event monitor
FnMonitor-->>MainThread: Binding unregistered ✓
MainThread-->>TranscribeAction: Dynamic binding inactive
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes Areas requiring particular attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src-tauri/src/actions.rs (1)
263-270: Dynamic cancel binding registration on main thread is appropriate.Using
run_on_main_threadfor shortcut registration is correct. However, there's a brief window betweenrecording_startedbeing true and the cancel binding being registered where pressing Escape won't work.Consider whether the registration should happen synchronously before
recording_startedis set, or document this expected behaviour. For most use cases, this window is negligible.src-tauri/src/shortcut/mod.rs (1)
108-113: Redundant clone in cancel binding return.
bis already a cloned value from line 104. Theb.clone()on line 110 is unnecessary.return Ok(BindingResponse { success: true, - binding: Some(b.clone()), + binding: Some(b), error: None, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
src-tauri/Cargo.toml(1 hunks)src-tauri/src/actions.rs(15 hunks)src-tauri/src/settings.rs(5 hunks)src-tauri/src/shortcut/fn_monitor.rs(1 hunks)src-tauri/src/shortcut/mod.rs(22 hunks)src-tauri/src/utils.rs(1 hunks)src/bindings.ts(1 hunks)src/components/settings/HandyShortcut.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src-tauri/src/settings.rs (2)
src/bindings.ts (1)
ShortcutBinding(621-627)src-tauri/src/commands/mod.rs (1)
get_default_settings(36-38)
src-tauri/src/shortcut/fn_monitor.rs (2)
src/bindings.ts (1)
ShortcutBinding(621-627)src-tauri/src/shortcut/mod.rs (4)
app(881-881)binding(915-915)binding(949-949)dispatch_binding_event(854-908)
src-tauri/src/utils.rs (5)
src-tauri/src/actions.rs (5)
app(210-210)app(217-217)app(292-292)app(293-293)app(294-294)src-tauri/src/shortcut/mod.rs (1)
app(881-881)src-tauri/src/tray.rs (3)
app(59-59)app(137-137)change_tray_icon(58-75)src-tauri/src/commands/audio.rs (2)
app(51-51)app(102-102)src-tauri/src/overlay.rs (1)
hide_recording_overlay(269-282)
src-tauri/src/actions.rs (2)
src-tauri/src/shortcut/mod.rs (3)
app(881-881)register_dynamic_binding(789-824)unregister_dynamic_binding(830-850)src-tauri/src/utils.rs (3)
app(25-25)app(34-34)cancel_current_operation(18-48)
src-tauri/src/shortcut/mod.rs (4)
src-tauri/src/settings.rs (5)
get_settings(568-584)get_default_settings(427-496)load_or_create_app_settings(521-566)serde_json(529-529)serde_json(574-574)src-tauri/src/actions.rs (5)
app(210-210)app(217-217)app(292-292)app(293-293)app(294-294)src-tauri/src/utils.rs (2)
app(25-25)app(34-34)src-tauri/src/shortcut/fn_monitor.rs (2)
register_fn_binding(105-131)unregister_fn_binding(134-155)
🔇 Additional comments (23)
src/components/settings/HandyShortcut.tsx (1)
305-305: Good UX improvement.Adding a tooltip helps users understand that the shortcut is clickable for recording a new binding.
src-tauri/src/utils.rs (1)
13-48: Well-structured cancellation flow with clear documentation.The ordering (cancel recording → remove mute → reset toggles → UI updates) is correct and the comments clearly explain why
action.stop()is intentionally avoided to prevent triggering transcription. The sequencing ensures safe state transitions.src/bindings.ts (1)
621-627: Type definition correctly mirrors the Rust struct.The optional
dynamic?: booleanfield aligns with the Rust#[serde(default)] pub dynamic: boolinsettings.rs. Since this file is auto-generated by tauri-specta, ensure the Rust source is the authoritative definition.src-tauri/src/settings.rs (2)
83-88: Dynamic binding field correctly implemented with backwards compatibility.The
#[serde(default)]attribute ensures existing settings without this field will deserialize correctly, defaulting tofalse. The documentation clearly explains the purpose.
449-458: Cancel binding defaults look correct.The cancel binding is properly configured with
dynamic: trueso it won't be registered at startup, and "Escape" is a sensible default key for cancellation.src-tauri/src/actions.rs (3)
283-289: Unregistration on stop aligns with lifecycle.The cancel binding is correctly unregistered when transcription completes. The error is logged at debug level which is appropriate since failure to unregister is non-critical (the binding becomes stale but will be cleaned up on next registration).
75-77: Log formatting improvement.The braced variable formatting is cleaner and consistent with Rust 2021 edition style.
426-443: Deadlock avoidance pattern in CancelAction is correct.The implementation properly avoids self-unregistration deadlock by relying on
register_dynamic_binding's idempotent behavior. Whenregister_dynamic_bindingis called (during the next transcription start), it internally callsunregister_bindingwith errors explicitly ignored (line 820:let _ = unregister_binding(app, binding.clone());), which safely cleans up any stale cancel binding registration. This pragmatic approach eliminates the deadlock risk from within-callback unregistration while ensuring proper cleanup on the next binding registration.src-tauri/Cargo.toml (1)
94-97: macOS Objective-C bindings are correctly configured and use current versions.The dependencies are properly scoped to the macOS target and the
NSEventfeature is correctly enabled forobjc2-app-kit. The version constraints (0.6for objc2 and block2,0.3for objc2-app-kit and objc2-foundation) use semantic versioning and will automatically include current patch releases (objc2 0.6.1, objc2-app-kit 0.3.2, objc2-foundation 0.3.2, and block2 0.6.2), ensuring compatibility with the fn-key monitoring functionality.src-tauri/src/shortcut/fn_monitor.rs (7)
1-14: Well-documented module with clear limitations.The architecture section and known limitations (Secure Input behaviour, fn+key conflicts) are valuable for future maintainers.
45-59: FFI accessibility check implementation looks correct.The use of
NSDictionary::from_slicesfor building the options dictionary and casting viaRetained::as_ptrfollows the objc2 idioms correctly.
86-92: Handler field retention is correct.The
handlerfield with#[allow(dead_code)]is necessary to keep theRcBlockalive for the duration of the monitor. Without it, the callback would be deallocated. Good use of the annotation to document intent.
103-131: Registration flow is correct.The function properly ensures the monitor is started before adding the binding. The
HashMap::insertsemantics provide idempotent behaviour if the same binding is registered twice.
133-155: Idempotent unregistration with proper state cleanup.Resetting
fn_pressedwhen no bindings remain (line 148) prevents stale state from affecting future bindings. The unused_appparameter maintains API symmetry withregister_fn_binding.
157-243: Main thread synchronisation is correctly implemented.The potential race between checking
MONITOR_STARTED.load()and the main thread installation is safely handled: the main thread closure performs a secondary check viahandle.monitor_token.is_some()(line 187), ensuring only one monitor is installed. The channel-based result passing is the correct pattern for cross-thread communication with Tauri'srun_on_main_thread.
245-293: Event processing with proper lock discipline.The pattern of cloning bindings while holding the lock (line 269), then releasing it before dispatching (line 270), avoids holding the mutex during potentially slow action callbacks. This prevents deadlocks if actions need to acquire locks themselves.
src-tauri/src/shortcut/mod.rs (7)
15-21: Clean routing helper for fn binding detection.The
is_fn_bindingfunction provides a clear abstraction for identifying fn-key-only bindings, keeping the routing logic inregister_binding/unregister_bindingreadable.
23-46: Binding routing correctly separates fn and regular shortcuts.The cfg-gated routing to
fn_monitoron macOS with fallback to standard shortcut handling maintains clean separation of concerns.
53-68: Dynamic binding exclusion during init is correct.Skipping dynamic bindings at startup (lines 61-64) and deferring their registration to runtime aligns with the documented lifecycle for bindings like the cancel shortcut.
728-735: Platform-specific fn validation is correctly implemented.The cfg-gated validation returns success on macOS and a clear error message on other platforms, preventing accidental use of fn bindings where unsupported.
778-824: Idempotent dynamic registration prevents deadlocks.The design decision to unregister before registering (line 820) allows safe re-registration from within action callbacks without explicit cleanup. The detailed doc comment explaining the deadlock avoidance rationale is helpful.
876-893: Lock discipline in toggle mode prevents deadlocks.The explicit scoping to release the lock before calling action methods (comment at line 893) is crucial, as documented. This pattern correctly prevents deadlocks when actions need to acquire locks themselves.
910-945: Internal shortcut registration with proper validation.The duplicate registration check (line 924-927) and human-friendly validation before parsing provide good error messages. The underscore prefix convention clearly marks this as an internal implementation detail.
Feature 1: fn Key Support (macOS)
How it works
The fn/Globe key is a modifier key that generates
NSEventType::FlagsChangedevents withNSEventModifierFlags::Function. Standard shortcut libraries (liketauri-plugin-global-shortcut) cannot capture modifier-only keys.Implementation
Parallel input system for fn key:
src-tauri/src/shortcut/fn_monitor.rs(macOS-only)NSEvent::addGlobalMonitorForEventsMatchingMask_handlerfrom objc2NSEventMask::FlagsChangedeventsNSEventModifierFlags::Functionto detect fn press/releasedispatch_binding_event()function as regular shortcutssrc-tauri/src/shortcut/mod.rsfn_monitorinstead oftauri-plugin-global-shortcutis_fn_binding()helper checks for fn-only bindingsvalidate_shortcut_string()allows "fn" as valid on macOSDependencies (macOS only in Cargo.toml):
Frontend (optional): "Use fn" button in
HandyShortcut.tsx- see Reviewer NotesPermissions
Requires Accessibility permission (same as already needed for
enigopasting). No additional permission prompts for users.Feature 2: Escape to Cancel Recording
How it works
The cancel shortcut is a dynamic binding - only registered while recording is active.
Implementation
CancelActioninactions.rs:cancel_current_operation()to discard recordingCancel binding in
settings.rs:dynamic: true- not registered at startupDynamic registration in
shortcut/mod.rs:register_dynamic_binding()- idempotent (unregisters first if already registered)unregister_dynamic_binding()- removes binding at runtimeinit_shortcuts()skips dynamic bindingsLifecycle:
TranscribeAction::start()registers cancel viarun_on_main_thread()TranscribeAction::stop()unregisters cancel viarun_on_main_thread()CancelAction::start()does NOT unregister (next registration handles cleanup)Key design decisions
Why idempotent registration?
Unregistering from inside the shortcut's own callback causes a deadlock (global_shortcut holds internal locks). Instead,
register_dynamic_binding()unregisters first, soCancelActiondoesn't need to unregister itself.Why release toggle lock before calling action?
dispatch_binding_event()releases the toggle state lock BEFORE callingaction.start()/stop(). This prevents deadlock whenCancelActioncallscancel_current_operation()which also needs the lock.Linux Notes
Dynamic shortcut registration (used for the cancel shortcut) is disabled on Linux due to
instability with the
tauri-plugin-global-shortcutplugin. See PR cjpais#392.This means the Escape-to-cancel feature is not available on Linux. The cancel shortcut will
silently be a no-op.
Potential future improvement: The
dynamicbinding architecture in this branch providesa cleaner foundation than the original async-spawn approach. If the underlying Linux shortcut
issues are resolved upstream, enabling dynamic bindings on Linux would only require removing
the
#[cfg(target_os = "linux")]guards inregister_dynamic_binding()andunregister_dynamic_binding()inshortcut/mod.rs.Reviewer Notes: fn Key UI
The fn key backend works regardless of UI changes. Users can always configure fn key manually
by editing
settings_store.json. Eg:UI Visibility Option
The commit "Remove fn key UI (manual config only)" removes the "Use fn" button from the
settings UI. This commit is structured to be easily included or excluded:
The UI additions that commit removes:
isFnBinding()helper functionReference PRs
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.