Menu shortcut reads live hotkey + dev-run teardown on Ctrl-C#4
Merged
Conversation
The "Search Screenshots…" item hardcoded ⇧⌘S, so rebinding the invoke hotkey to anything else (Hyper+S in my case) left the menu displaying the old default — a quiet lie to anyone who's touched Preferences. Read the live HotKeyChord instead and map it to a KeyboardShortcut?. Chords SwiftUI can't represent (empty, non-mappable keycodes) return nil, so the menu drops the glyph instead of showing the wrong one. Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com>
Ctrl-C out of the log tail now means "done dev-running": pkill -x Vista, then rm -rf Distribution/Vista.app. The dev bundle shares com.gordonbeeming.vista with the brew-installed prod copy, so a stale dev build left on disk silently wins the hotkey and UserDefaults on the next launch. The teardown closes that hazard. Scoped to the tailing branch; --no-tail is still fire-and-forget. Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com>
There was a problem hiding this comment.
Pull request overview
Updates the menu bar “Search Screenshots…” item to display the currently configured global hotkey, and improves the dev-run.sh developer workflow by cleaning up the dev app bundle/process on Ctrl-C when tailing logs.
Changes:
- Map the live
HotKeyChordpreference into a SwiftUIKeyboardShortcut?for menu rendering. - Add keyboard-layout-aware keycode→glyph translation for menu shortcut display.
- Add an
EXITtrap inScripts/dev-run.sh(tailing mode) to stop Vista and removeDistribution/Vista.appon exit/Ctrl-C.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Sources/Vista/MenuBarContentView.swift | Replaces hardcoded shortcut with a live HotKeyChord→KeyboardShortcut? mapping for accurate menu glyphs. |
| Scripts/dev-run.sh | Adds Ctrl-C/exit cleanup in the log-tailing branch to prevent stale dev bundles/processes from lingering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Two bot-review follow-ups on the menu shortcut change: - `HotKeyChord.layoutCharacter(for:)` now owns the UCKeyTranslate → String conversion. `KeyRecorderView.layoutKeyName` and the menu shortcut extension both call into it, each applying its own casing rule at the call site (uppercase for the recorder badge, lowercase for SwiftUI's `KeyEquivalent`). Kills the two near-identical copies of the layout-translation plumbing. - The "cleared chord" sentinel was `keyCode != 0`, but Carbon virtual keycode 0 is `kVK_ANSI_A`, so any chord on the A key (e.g. Hyper+A) was silently returning nil and dropping the menu glyph. Guard on `(keyCode == 0 && modifiers == 0)` instead — only the fully-empty chord means "no shortcut". Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com>
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.
Summary
HotKeyChordto a SwiftUIKeyboardShortcut?. Chords SwiftUI can't represent (empty, non-mappable keycodes) returnnilso the menu drops the glyph rather than show the wrong one — the global Carbon hotkey still fires regardless.dev-run.shcleans up on Ctrl-C. The tail banner used to say "Ctrl-C to stop — app keeps running", which meant every./Scripts/dev-run.shsession leftDistribution/Vista.appon disk and the dev app running in the menu bar. Since the dev bundle sharescom.gordonbeeming.vistawith the brew-installed prod copy, a stale dev build silently wins the hotkey and UserDefaults on the next login. Ctrl-C now installs anEXITtrap thatpkill -x Vistaandrm -rf Distribution/Vista.app. Scoped to the tailing branch —--no-tailstays fire-and-forget.Test plan
swift build— compiles clean⇧⌘S⌃⌥⇧⌘S./Scripts/dev-run.sh, Ctrl-C → expect🧹 Stopping Vista and removing …/Distribution/Vista.appbanner, menu-bar icon gone,Distribution/Vista.appdeleted./Scripts/dev-run.sh --no-tail→ app launches, script exits, app + bundle both still present