fix(app): rebind sidebar-hotkey as an in-app keybind, not a global hotkey#25
Conversation
…tkey
sidebar-hotkey was registered via Carbon RegisterEventHotKey, grabbing
its chord (default cmd+shift+s, i.e. "Save As…") system-wide from every
other application. Unlike the quick terminal, the sidebar toggle has no
reason to fire while noa is unfocused.
Feed the value into KeybindEngine::from_config as an in-app rebind of
ToggleSidebar instead, with one coherent contract across the engine and
the native menu:
- Explicit `keybind` entries still win over the rebind; the empty
sentinel (`none`) keeps the default chord (it only ever disabled the
old global registration). Config reload rebuilds the engine.
- KeyTrigger::parse accepts the multi-char token aliases the
global-hotkey syntax already allowed (semicolon, backslash, tab,
space, escape, JIS yen/ro, punctuation names), so pre-existing config
values keep replacing the default chord.
- Dedup and conflict checks share the runtime match relation
(KeyTrigger::overlaps over the KeyToken::alternates table — [/{,
]/}, and backslash's JIS variants \/¥/_), so a chord can never match
a keypress the checks treated as unrelated.
- Chords already serving another command (e.g. cmd+t) are rejected
with a diagnostic: the fixed menu accelerators mirror those defaults
and AppKit dispatches them before winit, so the other command's menu
item would win anyway. cmd+, (Preferences, menu-only) is reserved.
- The Sidebar menu item's accelerator tracks the engine's *effective*
ToggleSidebar chord (chord_for) and resyncs on any engine rebuild;
an unbound/unrepresentable chord clears the native key equivalent
explicitly (muda 0.19's set_accelerator(None) never touches the
NSMenuItem).
Drop the now-unused HotkeyAction::Sidebar / UserEvent::ToggleSidebar
plumbing; only quick-terminal-hotkey remains a global hotkey.
CONFIGURATION.md and KEYBINDINGS.md described sidebar-hotkey as a system-wide Carbon hotkey disabled by none/empty — the opposite of the new contract. Sync both tables and move it out of the Global System Hotkeys section.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bdb1dad8a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | (Self::End, NamedKey::End) | ||
| | (Self::Enter, NamedKey::Enter) | ||
| | (Self::Tab, NamedKey::Tab) | ||
| | (Self::Space, NamedKey::Space) |
There was a problem hiding this comment.
When winit reports Space as Key::Character(" ") (the input layer tests already handle that platform variation), a space trigger parsed here never matches because NamedKeyToken::Space only accepts NamedKey::Space. That makes sidebar-hotkey = cmd+space or any configured keybind using space silently ineffective in those environments unless a native menu accelerator happens to catch it; please match the character-space form too.
Useful? React with 👍 / 👎.
1 participant
Summary
sidebar-hotkeywas registered via CarbonRegisterEventHotKey, grabbing its chord (default ⇧⌘S, "Save As…") system-wide from every other application. It is now an in-app rebind ofToggleSidebarfed intoKeybindEngine::from_config, with one coherent contract across the engine and the native menu; onlyquick-terminal-hotkeyremains a global hotkey.Changes
keybindentries win over the rebind;none/empty keeps the default chord; config reload rebuilds the engineKeyTrigger::parseaccepts the multi-char aliases the global-hotkey syntax allowed (semicolon, backslash, tab/space/escape, JIS yen/ro, …), so existing config values stay validKeyTrigger::overlapsover theKeyToken::alternatestable —[/{,]/}, backslash's JIS variants\/¥/_)cmd+t) are rejected with a diagnostic — AppKit dispatches the fixed menu accelerators before winit, so the other command's menu item would win anywayset_accelerator(None)never touches the NSMenuItem)Test plan
cargo test -p noa-app(957 tests; adds coverage for alias parsing, conflict rejection, JIS variants, overlap-based dedup, accelerator derivation)cargo clippy/cargo fmtclean; manual: ⇧⌘S no longer fires in other apps while noa runs