Skip to content

Commit 655b2cc

Browse files
authored
fix(settings): reserve every hardcoded menu shortcut against user rebinding (#2459)
Claude-Session: https://claude.ai/code/session_014MXT7tDsb7Rsm1HhBnwhSj
1 parent 3daf119 commit 655b2cc

3 files changed

Lines changed: 39 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2222

2323
- A data grid shortcut bound to `Cmd+G` silently taking the key from Find Next, with no conflict warning.
2424
- Stale cells after fitting, hiding or reordering a data grid column on a result narrower than the window. (#2446)
25+
- A rebound shortcut silently killing Quit, Minimize, Hide, Settings, Show Toolbar or Enter Full Screen.
26+
- `Ctrl+Cmd+J` accepted in Settings > Keyboard while the SQL editor kept it for Jump to Definition.
2527
- Autocomplete keeping an earlier prefix's ordering after the typed word becomes an exact match. (#2444)
2628
- Whole MySQL and MariaDB result set fetched before a capped query returned its first rows. (#2427)
2729
- KILL sent to a different server when a MySQL or MariaDB connection's host is spelled `localhost`.

TablePro/Models/UI/KeyboardShortcutModels.swift

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ extension ShortcutAction {
292292
(.special(.space, control: true), String(localized: "Show Completions")),
293293
(.special(.upArrow, option: true), String(localized: "Move Line Up")),
294294
(.special(.downArrow, option: true), String(localized: "Move Line Down")),
295+
(.character("j", command: true, control: true), String(localized: "Jump to Definition")),
295296
(.special(.upArrow, shift: true, option: true), String(localized: "Extend Selection to Previous Statement")),
296297
(.special(.downArrow, shift: true, option: true), String(localized: "Extend Selection to Next Statement"))
297298
]
@@ -314,13 +315,21 @@ extension ShortcutAction {
314315
(.special(.rightArrow, option: true), String(localized: "Move Word Right"))
315316
]
316317

317-
/// App-level shortcuts that are wired directly in the menu and are not
318-
/// customizable: tab selection (Cmd+1 through Cmd+9) and editor zoom. These
319-
/// fire regardless of focus, so a user binding would silently collide.
318+
/// Every key equivalent a menu builder hardcodes, and so every combo no `ShortcutAction`
319+
/// can be bound to. These fire regardless of focus, and AppKit blanks the loser when two
320+
/// menu items claim one combo, so a binding the recorder let through would silently kill
321+
/// the hardcoded command instead. An entry added to a menu builder belongs here too.
320322
static let reservedAppShortcuts: [(key: BoundKey, name: String)] = {
321323
var shortcuts: [(key: BoundKey, name: String)] = [
322324
(.character("=", command: true), String(localized: "Zoom In")),
323-
(.character("-", command: true), String(localized: "Zoom Out"))
325+
(.character("-", command: true), String(localized: "Zoom Out")),
326+
(.character(",", command: true), String(localized: "Settings…")),
327+
(.character("h", command: true), String(localized: "Hide TablePro")),
328+
(.character("h", command: true, option: true), String(localized: "Hide Others")),
329+
(.character("q", command: true), String(localized: "Quit TablePro")),
330+
(.character("m", command: true), String(localized: "Minimize")),
331+
(.character("t", command: true, option: true), String(localized: "Show Toolbar")),
332+
(.character("f", command: true, control: true), String(localized: "Enter Full Screen"))
324333
]
325334
for number in 1...9 {
326335
shortcuts.append((

TableProTests/Core/Menu/MainMenuBuilderTests.swift

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,30 @@ struct MainMenuStructureTests {
7272
#expect(duplicates.isEmpty, "AppKit blanks the loser when two items claim one combo: \(duplicates)")
7373
}
7474

75+
/// A menu builder that hardcodes a key equivalent takes that combo off the table for every
76+
/// `ShortcutAction`, and only `reservedAppShortcuts` tells the recorder so. Nothing else
77+
/// forces the two to agree, so a hardcoded item added without a matching entry ships a
78+
/// binding the recorder accepts and AppKit then blanks.
79+
@Test("Every hardcoded menu key equivalent is reserved against user binding")
80+
func hardcodedKeyEquivalentsAreReserved() {
81+
func canonical(_ key: BoundKey) -> String? {
82+
key.menuKeyEquivalent.map { "\(key.modifierFlags.rawValue)-\($0)" }
83+
}
84+
85+
let keyboard = KeyboardSettings()
86+
let customizable = Set(ShortcutAction.allCases.compactMap { keyboard.shortcut(for: $0).flatMap(canonical) })
87+
let reserved = Set(ShortcutAction.reservedAppShortcuts.compactMap { canonical($0.key) })
88+
89+
let hardcoded = flatten(buildMenu())
90+
.filter { !$0.keyEquivalent.isEmpty }
91+
.map { (combo: "\($0.keyEquivalentModifierMask.rawValue)-\($0.keyEquivalent)", title: $0.title) }
92+
.filter { !customizable.contains($0.combo) }
93+
#expect(!hardcoded.isEmpty, "Found no hardcoded menu shortcuts, so this guard would pass vacuously")
94+
95+
let unreserved = hardcoded.filter { !reserved.contains($0.combo) }.map(\.title)
96+
#expect(unreserved.isEmpty, "Hardcoded menu shortcuts missing from reservedAppShortcuts: \(unreserved)")
97+
}
98+
7599
@Test("Every menu item carries an action")
76100
func everyItemHasAnAction() {
77101
let dead = flatten(buildMenu())

0 commit comments

Comments
 (0)