Conversation
![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: fdae7eb50c
ℹ️ 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".
| private func startRecording() { | ||
| isRecording = true | ||
| eventMonitor = NSEvent.addLocalMonitorForEvents(matching: .keyDown) { event in | ||
| handleKeyEvent(event) |
There was a problem hiding this comment.
startRecording() always adds a new local keyDown monitor and overwrites eventMonitor without removing any existing monitor first. If the user clicks the shortcut field twice before pressing a key, the first monitor is leaked; stopRecording() removes only the most recent one, leaving a stale monitor that keeps consuming key events and can trigger unexpected shortcut recordings/actions afterward.
Useful? React with 👍 / 👎.
| self.overrides = storage.loadOverrides() | ||
| self.bindings = Self.merge(defaults: defaults, overrides: storage.loadOverrides()) |
There was a problem hiding this comment.
The initializer merges persisted overrides directly into active bindings without any conflict/lock validation. If keybindings.json contains a stale or hand-edited override that reuses a locked shortcut (for example ⌘C or ⌘Q), that invalid binding becomes active at startup and the app-level key monitor can intercept the shortcut before responder-chain/system behavior, effectively breaking reserved commands until the file is cleaned up.
Useful? React with 👍 / 👎.
…onflictResult types and KeyBindingStorageProtocol
…onfigurable + 11 locked system)
Implements KeyBindingStorageProtocol with thread-safe NSLock-based JSON file storage. Only user-overridden bindings are persisted. Gracefully handles missing and corrupt files.
22 new assertions covering round-trip, sparse storage, missing file, corrupt file, and overwrite scenarios.
- Fix window.toggleSidebar default from ⌘0 to ⌘B to match AppDelegate and docs - Add testKeyBindingDefaultsNoShortcutConflicts to catch duplicate shortcuts
…ridging Create KeyBindingStore (@mainactor @observable) that merges defaults with user overrides, supports conflict validation, displacement on update, and single/bulk reset. Add KeyBinding+AppKit extension for NSEvent matching and NSMenuItem key equivalents.
Test KeyBindingStore lifecycle (init, update, validate, displacement, reset), conflict detection (locked vs configurable), sparse persistence, and KeyModifiers<->NSEvent.ModifierFlags round-trips.
Add MoriKeybindings as dependency of Mori executable target and add test:keybindings task to mise test suite.
…hortcuts Replace hardcoded keyboard shortcuts in setupMainMenu() and setupCommandPalette() with KeyBindingStore-driven lookups. Menu items and the key monitor now read shortcuts from the store, enabling runtime customization. Locked system shortcuts (Edit menu, Quit, Hide, Minimize, Fullscreen) remain hardcoded. The rebuildMenuKeyBindings() callback updates menu items when bindings change.
SwiftUI view that displays current shortcut (formatted with modifier symbols) and captures new key combinations via NSEvent local monitor. Supports locked state, clear/unassign, and Escape to cancel.
Pure data + callbacks view that groups bindings by category, shows per-row reset buttons for overridden bindings, handles locked conflicts (error, rejected) and configurable conflicts (warning + Assign Anyway).
Replace static moriKeybinds list with editable KeyBindingsSettingsView. Thread key binding callbacks through GhosttySettingsView and SettingsWindowContent to KeyBindingStore in AppDelegate.
Add en + zh-Hans translations for: shortcut recorder UI strings, category names, conflict messages, and all 50 keybinding display names.
…UI in sync The @State keyBindings in SettingsWindowContent was a snapshot that never refreshed after store mutations, causing the UI to show stale data and making customized shortcuts appear to reset immediately.
Use opacity(0) instead of conditional placeholder so the Reset button always occupies the same space, preventing layout shift when a shortcut is customized.
…load - Guard against double-click in ShortcutRecorderView creating leaked monitors - Validate persisted overrides at startup: strip entries that conflict with locked shortcuts or override locked bindings, preventing hand-edited JSON from breaking system shortcuts like ⌘C or ⌘Q
- Cache defaultsById in KeyBindingStore to avoid rebuilding on every persist() - Convert KeyBindingDefaults.byId from computed var to stored let - Extract managerAction helper to deduplicate 16 repetitive closures in keyMonitorActionMap
vaayne
force-pushed
the
feat-customizable-keyboards
branch
from
892e1a3 to
01aee45
Compare
The AppDelegate key monitor was intercepting events (e.g. CMD+T) before the ShortcutRecorderView could capture them for rebinding. Added a @mainactor global flag isRecordingShortcut that the AppDelegate checks to pass events through during shortcut recording.
Add other.projectSwitcher binding (CMD+P) to default keybindings, action map, and localization strings (en + zh-Hans).
… category Group CMD+Shift+P (command palette) and CMD+P (project switcher) together under the Other category. Remove now-empty commandPalette category from settings view ordering.
1 participant
Summary
~/Library/Application Support/Mori/keybindings.json) — only overrides are stored, defaults are never writtenMoriKeybindingspackage (macOS-only) housesKeyBindingStore(@mainactor @observable) and AppKit bridging (matchesEvent, menu helpers)Fix: #32
Test plan