fix(tauri): gracefully handle Alt+Space global shortcut conflicts#3067
fix(tauri): gracefully handle Alt+Space global shortcut conflicts#3067brbrainerd wants to merge 1 commit into
Conversation
WalkthroughMove Changes
Sequence Diagram(s)sequenceDiagram
participant Startup as App Startup
participant App as Tauri App
participant GS as GlobalShortcut API
participant Log as Logger
Startup->>App: initialize (builder completed)
App->>GS: register("Alt+Space")
alt registration OK
GS-->>App: Ok
App->>Log: info!("registered Alt+Space")
else registration Err
GS-->>App: Err(e)
App->>Log: warn!("voice overlay shortcut disabled", e)
end
Startup->>App: continue boot (non-fatal)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/tauri/src-tauri/src/main.rs (1)
2010-2013: Make the warning text error-agnostic.
register()can fail for reasons other than another app owning the shortcut, so the log should avoid diagnosing every failure as “already in use”.Suggested log wording
- match app.handle().global_shortcut().register("Alt+Space") { - Ok(_) => tracing::info!("Registered Alt+Space global shortcut"), - Err(e) => tracing::warn!("Alt+Space already in use, voice overlay shortcut disabled: {}", e), - } + match app.handle().global_shortcut().register("Alt+Space") { + Ok(_) => tracing::info!("Registered Alt+Space global shortcut"), + Err(error) => tracing::warn!( + ?error, + "Failed to register Alt+Space global shortcut; voice overlay shortcut disabled" + ), + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src-tauri/src/main.rs` around lines 2010 - 2013, The current tracing::warn! call in the app.handle().global_shortcut().register("Alt+Space") branch assumes the failure means the shortcut is already in use; change that message to be error-agnostic and include the error details instead. Update the Err(e) arm that calls tracing::warn! to something like "Failed to register Alt+Space global shortcut, voice overlay disabled: {}" (or similar neutral wording) and pass the error so logs show the actual failure; keep the Ok(_) arm unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/tauri/src-tauri/src/main.rs`:
- Around line 2010-2013: The current tracing::warn! call in the
app.handle().global_shortcut().register("Alt+Space") branch assumes the failure
means the shortcut is already in use; change that message to be error-agnostic
and include the error details instead. Update the Err(e) arm that calls
tracing::warn! to something like "Failed to register Alt+Space global shortcut,
voice overlay disabled: {}" (or similar neutral wording) and pass the error so
logs show the actual failure; keep the Ok(_) arm unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: adddf4e1-baa4-4b9c-b630-5457603af334
📒 Files selected for processing (2)
.tasks/interface/TAURI-001-graceful-shortcut-conflicts.mdapps/tauri/src-tauri/src/main.rs
brbrainerd
force-pushed
the
fix/tauri-shortcut-conflict
branch
from
925101d to
e022221
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/tauri/src-tauri/src/main.rs (1)
1930-1940: Handler will fire for any registered shortcut, not just Alt+Space (future-proofing).The handler ignores
_shortcutand unconditionally toggles the voice overlay on anyPressedevent. Today onlyAlt+Spaceis registered via runtime registration on non-Linux platforms, so this is benign. However, if another component registers additional global shortcuts through this plugin in the future, they would all trigger the voice overlay. Consider matching againstAlt+Spaceexplicitly to make this future‑proof.♻️ Example guard
- .with_handler(|app, _shortcut, event| { - if event.state() == tauri_plugin_global_shortcut::ShortcutState::Pressed { + .with_handler(|app, shortcut, event| { + use tauri_plugin_global_shortcut::{ShortcutState, Code, Modifiers}; + if event.state() == ShortcutState::Pressed + && shortcut.matches(Modifiers::ALT, Code::Space) + { if let Err(error) = windows::toggle_voice_overlay_internal(app.clone()) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src-tauri/src/main.rs` around lines 1930 - 1940, The global-shortcut handler currently ignores the second parameter and toggles the overlay on any Pressed event; change the closure signature from |app, _shortcut, event| to capture the shortcut name (e.g., |app, shortcut, event|) and guard the toggle by matching that shortcut against the Alt+Space identifier used at registration (only call windows::toggle_voice_overlay_internal(app.clone()) when event.state() == tauri_plugin_global_shortcut::ShortcutState::Pressed AND the passed shortcut equals the Alt+Space string), so other shortcuts won't trigger the voice overlay.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/tauri/src-tauri/src/main.rs`:
- Around line 1930-1940: The global-shortcut handler currently ignores the
second parameter and toggles the overlay on any Pressed event; change the
closure signature from |app, _shortcut, event| to capture the shortcut name
(e.g., |app, shortcut, event|) and guard the toggle by matching that shortcut
against the Alt+Space identifier used at registration (only call
windows::toggle_voice_overlay_internal(app.clone()) when event.state() ==
tauri_plugin_global_shortcut::ShortcutState::Pressed AND the passed shortcut
equals the Alt+Space string), so other shortcuts won't trigger the voice
overlay.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 524f1ccf-93a6-4d7d-8e54-3e0446649845
📒 Files selected for processing (2)
.tasks/interface/TAURI-001-graceful-shortcut-conflicts.mdapps/tauri/src-tauri/src/main.rs
✅ Files skipped from review due to trivial changes (1)
- .tasks/interface/TAURI-001-graceful-shortcut-conflicts.md
brbrainerd
force-pushed
the
fix/tauri-shortcut-conflict
branch
2 times, most recently
from
a16aabd to
585c1c0
Compare
|
Addressed automated review feedback: improved the Alt+Space failure log to be error-agnostic and surface the underlying error details. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.tasks/interface/TAURI-001-graceful-shortcut-conflicts.md (2)
41-41: Use structured logging format consistent with the actual implementation.The example shows string interpolation (
"...: {}", e), but the actual implementation inmain.rsuses structured logging with the?errorfield syntax, which is the preferred pattern in the tracing ecosystem. The documentation example should match the actual implementation for consistency.🔧 Suggested fix for consistency
- Err(e) => tracing::warn!("Failed to register Alt+Space global shortcut, voice overlay disabled: {}", e), + Err(error) => tracing::warn!( + ?error, + "Failed to register Alt+Space global shortcut, voice overlay disabled" + ),This structured format:
- Matches the pattern used in
apps/tauri/src-tauri/src/main.rs:1928-1941- Provides better structured log output for log aggregation tools
- Follows tracing best practices for error logging
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.tasks/interface/TAURI-001-graceful-shortcut-conflicts.md at line 41, The logging should use tracing's structured field syntax instead of string interpolation; locate the Err(e) branch that currently logs "Failed to register Alt+Space global shortcut, voice overlay disabled: {}", e and change it to use a structured field like tracing::warn!("Failed to register Alt+Space global shortcut, voice overlay disabled"; ?error = e) so it matches the pattern used in main.rs and the project's tracing conventions.
36-36: Document why Linux is excluded from global shortcut registration.The example includes
#[cfg(not(target_os = "linux"))]but doesn't explain why Linux is excluded from this feature. Adding a brief explanation would help readers understand this platform-specific behavior.📝 Suggested documentation addition
Consider adding a note after line 25 or within the diff explanation:
> **Note**: The implementation uses `#[cfg(not(target_os = "linux"))]` because [reason for Linux exclusion - e.g., "global shortcuts are handled differently on Linux" or "this feature is not supported on Linux due to X11/Wayland limitations"].🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.tasks/interface/TAURI-001-graceful-shortcut-conflicts.md at line 36, Add a short explanatory note in the documentation next to the #[cfg(not(target_os = "linux"))] attribute (or after the example block) that explicitly states why Linux is excluded from global shortcut registration; reference the cfg symbol in your note and describe the platform limitation (e.g., global shortcuts are handled differently or not reliably supported on Linux due to X11/Wayland or desktop-environment differences) so readers understand the conditional compilation choice.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.tasks/interface/TAURI-001-graceful-shortcut-conflicts.md:
- Line 41: The logging should use tracing's structured field syntax instead of
string interpolation; locate the Err(e) branch that currently logs "Failed to
register Alt+Space global shortcut, voice overlay disabled: {}", e and change it
to use a structured field like tracing::warn!("Failed to register Alt+Space
global shortcut, voice overlay disabled"; ?error = e) so it matches the pattern
used in main.rs and the project's tracing conventions.
- Line 36: Add a short explanatory note in the documentation next to the
#[cfg(not(target_os = "linux"))] attribute (or after the example block) that
explicitly states why Linux is excluded from global shortcut registration;
reference the cfg symbol in your note and describe the platform limitation
(e.g., global shortcuts are handled differently or not reliably supported on
Linux due to X11/Wayland or desktop-environment differences) so readers
understand the conditional compilation choice.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 84bba6a6-8c1a-44e0-905f-0cee8e416b52
📒 Files selected for processing (2)
.tasks/interface/TAURI-001-graceful-shortcut-conflicts.mdapps/tauri/src-tauri/src/main.rs
✅ Files skipped from review due to trivial changes (1)
- apps/tauri/src-tauri/src/main.rs
brbrainerd
force-pushed
the
fix/tauri-shortcut-conflict
branch
from
585c1c0 to
fa5f763
Compare
|
Addressed additional CodeRabbit nitpicks: updated the shortcut-handler closure to match against the specific 'Alt+Space' identifier before toggling, added tracing's structured ?error field syntax to the documentation diff, and added a clarifying note about why Linux is excluded. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/tauri/src-tauri/src/main.rs (1)
1930-1941:⚠️ Potential issue | 🟠 MajorUse
shortcut.matches()instead of string comparison;to_string()produces lowercase modifiers.The handler's check
shortcut.to_string() == "Alt+Space"will never succeed. TheHotKey::into_string()implementation in global-hotkey 0.7.0 produces lowercase modifier names ("alt+","control+", etc.), so the comparison against the uppercase"Alt+Space"always fails. The voice overlay toggle will silently not fire—a regression vs. the prior flow.Use
shortcut.matches()instead:Proposed fix
-.with_handler(|app, shortcut, event| { - if event.state() == tauri_plugin_global_shortcut::ShortcutState::Pressed - && shortcut.to_string() == "Alt+Space" - { +.with_handler(|app, shortcut, event| { + use tauri_plugin_global_shortcut::{Code, Modifiers, ShortcutState}; + if event.state() == ShortcutState::Pressed && shortcut.matches(Modifiers::ALT, Code::Space) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src-tauri/src/main.rs` around lines 1930 - 1941, The global shortcut check is using shortcut.to_string() == "Alt+Space" which fails because HotKey strings use lowercase modifiers; replace the string equality with shortcut.matches("Alt+Space") (or appropriate pattern) inside the handler closure so the branch invoking windows::toggle_voice_overlay_internal(app.clone()) actually runs; update the closure that currently compares shortcut.to_string() to call shortcut.matches(...) and keep the existing error handling/tracing around windows::toggle_voice_overlay_internal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/tauri/src-tauri/src/main.rs`:
- Around line 1930-1941: The global shortcut check is using shortcut.to_string()
== "Alt+Space" which fails because HotKey strings use lowercase modifiers;
replace the string equality with shortcut.matches("Alt+Space") (or appropriate
pattern) inside the handler closure so the branch invoking
windows::toggle_voice_overlay_internal(app.clone()) actually runs; update the
closure that currently compares shortcut.to_string() to call
shortcut.matches(...) and keep the existing error handling/tracing around
windows::toggle_voice_overlay_internal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3ab7e9bd-d9a8-48e7-847e-82d02451b370
📒 Files selected for processing (2)
.tasks/interface/TAURI-001-graceful-shortcut-conflicts.mdapps/tauri/src-tauri/src/main.rs
1 participant
This PR prevents the Tauri app from panicking on startup if the Alt+Space global shortcut is already bound by another application (e.g., PowerToys Run).
Key changes:
Task File: .tasks/interface/TAURI-001-graceful-shortcut-conflicts.md
Closes TAURI-001