diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c7cd93c0..b2681d6eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Reassigning the Execute Query, Execute All Statements, and Cancel Query shortcuts now takes effect, and the Query menu shows the new keys (#1357) +- Custom shortcuts now require a modifier key, so a plain key like Space is no longer accepted and then silently ignored (#1357) - Cancelling a pending connection no longer lets the abandoned attempt overwrite or drop a later successful connection to the same database (#1358) - Importing connections from DBeaver now brings over the username (#1355) diff --git a/TablePro/Core/Storage/AppSettingsStorage.swift b/TablePro/Core/Storage/AppSettingsStorage.swift index 1a705ac3c..8afa1ca35 100644 --- a/TablePro/Core/Storage/AppSettingsStorage.swift +++ b/TablePro/Core/Storage/AppSettingsStorage.swift @@ -103,7 +103,7 @@ final class AppSettingsStorage { // MARK: - Keyboard Settings func loadKeyboard() -> KeyboardSettings { - load(key: Keys.keyboard, default: .default) + load(key: Keys.keyboard, default: KeyboardSettings.default).sanitized() } func saveKeyboard(_ settings: KeyboardSettings) { diff --git a/TablePro/Models/UI/KeyboardShortcutModels.swift b/TablePro/Models/UI/KeyboardShortcutModels.swift index 2aaa58709..108a3a305 100644 --- a/TablePro/Models/UI/KeyboardShortcutModels.swift +++ b/TablePro/Models/UI/KeyboardShortcutModels.swift @@ -47,6 +47,8 @@ enum ShortcutAction: String, Codable, CaseIterable, Identifiable { case closeTab case refresh case executeQuery + case executeAllStatements + case cancelQuery case explainQuery case formatQuery case export @@ -101,7 +103,8 @@ enum ShortcutAction: String, Codable, CaseIterable, Identifiable { switch self { case .manageConnections, .newTab, .openDatabase, .openFile, .switchConnection, .saveChanges, .saveAs, .previewSQL, .closeTab, .refresh, - .executeQuery, .explainQuery, .formatQuery, .export, .importData, .quickSwitcher, + .executeQuery, .executeAllStatements, .cancelQuery, .explainQuery, .formatQuery, + .export, .importData, .quickSwitcher, .previousPage, .nextPage, .saveAsFavorite, .openTerminal: return .file case .undo, .redo, .cut, .copy, .copyRowsExplicit, .copyWithHeaders, .copyAsJson, .paste, @@ -118,10 +121,21 @@ enum ShortcutAction: String, Codable, CaseIterable, Identifiable { } } + var allowsBareKey: Bool { + switch self { + case .previewFKReference, .clearSelection, .delete: + return true + default: + return false + } + } + var displayName: String { switch self { case .manageConnections: return String(localized: "Manage Connections") case .executeQuery: return String(localized: "Execute Query") + case .executeAllStatements: return String(localized: "Execute All Statements") + case .cancelQuery: return String(localized: "Cancel Query") case .newTab: return String(localized: "New Tab") case .openDatabase: return String(localized: "Open Database") case .openFile: return String(localized: "Open File") @@ -282,6 +296,10 @@ struct KeyCombo: Codable, Equatable, Hashable { return modifiers } + var hasModifier: Bool { + command || shift || option || control + } + /// Human-readable display string (e.g. "⌘S", "⇧⌘P") var displayString: String { var parts: [String] = [] @@ -445,6 +463,19 @@ struct KeyboardSettings: Codable, Equatable { shortcuts.removeValue(forKey: action.rawValue) } + /// Drop overrides that can never dispatch (bare keys on menu-driven actions), + /// reverting them to their default. Cleared and unknown overrides are kept. + func sanitized() -> KeyboardSettings { + var cleaned = shortcuts + for (rawValue, combo) in shortcuts { + guard let action = ShortcutAction(rawValue: rawValue), !combo.isCleared else { continue } + if !combo.hasModifier, !action.allowsBareKey { + cleaned.removeValue(forKey: rawValue) + } + } + return KeyboardSettings(shortcuts: cleaned) + } + /// Build a SwiftUI KeyboardShortcut for the given action. /// Returns nil if the user has cleared (unassigned) the shortcut. func keyboardShortcut(for action: ShortcutAction) -> KeyboardShortcut? { @@ -461,6 +492,8 @@ struct KeyboardSettings: Codable, Equatable { // File .manageConnections: KeyCombo(key: "n", command: true), .executeQuery: KeyCombo(key: "return", command: true, isSpecialKey: true), + .executeAllStatements: KeyCombo(key: "return", command: true, shift: true, isSpecialKey: true), + .cancelQuery: KeyCombo(key: ".", command: true), .newTab: KeyCombo(key: "t", command: true), .openDatabase: KeyCombo(key: "k", command: true), .openFile: KeyCombo(key: "o", command: true), diff --git a/TablePro/TableProApp.swift b/TablePro/TableProApp.swift index 6d2d4b5b3..94783e473 100644 --- a/TablePro/TableProApp.swift +++ b/TablePro/TableProApp.swift @@ -362,13 +362,13 @@ struct AppMenuCommands: Commands { Button("Execute Query") { actions?.runQuery() } - .keyboardShortcut(.return, modifiers: .command) + .optionalKeyboardShortcut(shortcut(for: .executeQuery)) .disabled(!(actions?.isConnected ?? false) || !(actions?.hasQueryText ?? false)) Button(String(localized: "Execute All Statements")) { actions?.runAllStatements() } - .keyboardShortcut(.return, modifiers: [.command, .shift]) + .optionalKeyboardShortcut(shortcut(for: .executeAllStatements)) .disabled(!(actions?.isConnected ?? false) || !(actions?.hasQueryText ?? false)) Button("Explain Query") { @@ -403,7 +403,7 @@ struct AppMenuCommands: Commands { Button(String(localized: "Cancel Query")) { actions?.cancelCurrentQuery() } - .keyboardShortcut(".", modifiers: .command) + .optionalKeyboardShortcut(shortcut(for: .cancelQuery)) .disabled(!(actions?.isQueryExecuting ?? false)) Button("Refresh") { diff --git a/TablePro/Views/Editor/QueryEditorView.swift b/TablePro/Views/Editor/QueryEditorView.swift index 8107f7fa6..f4e4892c3 100644 --- a/TablePro/Views/Editor/QueryEditorView.swift +++ b/TablePro/Views/Editor/QueryEditorView.swift @@ -120,7 +120,7 @@ struct QueryEditorView: View { } .buttonStyle(.borderedProminent) .controlSize(.small) - .keyboardShortcut(.return, modifiers: .command) + .optionalKeyboardShortcut(AppSettingsManager.shared.keyboard.keyboardShortcut(for: .executeQuery)) } .padding(.horizontal, 12) .padding(.vertical, 8) diff --git a/TablePro/Views/Settings/KeyboardSettingsView.swift b/TablePro/Views/Settings/KeyboardSettingsView.swift index 1ce07bb0c..bd2c318b3 100644 --- a/TablePro/Views/Settings/KeyboardSettingsView.swift +++ b/TablePro/Views/Settings/KeyboardSettingsView.swift @@ -15,6 +15,7 @@ struct KeyboardSettingsView: View { @State private var searchText = "" @State private var conflictAlert: ConflictAlertState? @State private var systemReservedAlert: ShortcutAction? + @State private var needsModifierAlert: ShortcutAction? var body: some View { VStack(spacing: 0) { @@ -90,6 +91,19 @@ struct KeyboardSettingsView: View { } message: { Text(String(localized: "This shortcut is reserved by macOS and cannot be assigned.")) } + .alert( + String(localized: "Modifier Key Required"), + isPresented: Binding( + get: { needsModifierAlert != nil }, + set: { if !$0 { needsModifierAlert = nil } } + ) + ) { + Button(String(localized: "OK"), role: .cancel) { + needsModifierAlert = nil + } + } message: { + Text(String(localized: "This action needs a modifier key like ⌘ or ⌥. A plain key won't reach the menu reliably.")) + } } // MARK: - Shortcut Row @@ -128,13 +142,16 @@ struct KeyboardSettingsView: View { } private func handleRecord(_ combo: KeyCombo, for action: ShortcutAction) { - // Check system-reserved shortcuts if combo.isSystemReserved { systemReservedAlert = action return } - // Check for conflicts + if !combo.hasModifier, !action.allowsBareKey { + needsModifierAlert = action + return + } + if let conflict = settings.findConflict(for: combo, excluding: action) { conflictAlert = ConflictAlertState( action: action, @@ -144,7 +161,6 @@ struct KeyboardSettingsView: View { return } - // No conflict — assign directly settings.setShortcut(combo, for: action) } } diff --git a/TableProTests/Models/KeyboardShortcutTests.swift b/TableProTests/Models/KeyboardShortcutTests.swift new file mode 100644 index 000000000..5b4585d29 --- /dev/null +++ b/TableProTests/Models/KeyboardShortcutTests.swift @@ -0,0 +1,114 @@ +// +// KeyboardShortcutTests.swift +// TableProTests +// +// Pins the shortcut customization fixes for #1357: Execute Query / Cancel Query +// are customizable, bare keys are rejected for menu-driven actions, and stale +// bare-key overrides self-heal on load. +// + +import Foundation +@testable import TablePro +import Testing + +@Suite("ShortcutAction defaults") +struct ShortcutActionDefaultsTests { + @Test("Execute Query default is Cmd+Return") + func executeQueryDefault() { + #expect(KeyboardSettings.defaultShortcuts[.executeQuery] == KeyCombo(key: "return", command: true, isSpecialKey: true)) + } + + @Test("Execute All Statements default is Cmd+Shift+Return") + func executeAllStatementsDefault() { + #expect( + KeyboardSettings.defaultShortcuts[.executeAllStatements] + == KeyCombo(key: "return", command: true, shift: true, isSpecialKey: true) + ) + } + + @Test("Cancel Query default is Cmd+.") + func cancelQueryDefault() { + #expect(KeyboardSettings.defaultShortcuts[.cancelQuery] == KeyCombo(key: ".", command: true)) + } +} + +@Suite("Bare-key validation") +struct BareKeyValidationTests { + @Test("Grid actions allow bare keys") + func gridActionsAllowBareKeys() { + #expect(ShortcutAction.previewFKReference.allowsBareKey) + #expect(ShortcutAction.clearSelection.allowsBareKey) + #expect(ShortcutAction.delete.allowsBareKey) + } + + @Test("Menu actions reject bare keys") + func menuActionsRejectBareKeys() { + #expect(!ShortcutAction.toggleInspector.allowsBareKey) + #expect(!ShortcutAction.executeQuery.allowsBareKey) + } + + @Test("hasModifier reflects the combo") + func hasModifierReflectsCombo() { + #expect(KeyCombo(key: "r", command: true).hasModifier) + #expect(!KeyCombo(key: "space", isSpecialKey: true).hasModifier) + } + + @Test("Every bare-key default belongs to an action that allows bare keys") + func bareKeyDefaultsAreAllowed() { + for (action, combo) in KeyboardSettings.defaultShortcuts where !combo.hasModifier { + #expect(action.allowsBareKey, "\(action.rawValue) ships a bare-key default but does not allow bare keys") + } + } +} + +@Suite("Shortcut conflict detection") +struct ShortcutConflictTests { + @Test("Assigning Cmd+R to Execute Query conflicts with Refresh") + func cmdRConflictsWithRefresh() { + let settings = KeyboardSettings.default + let conflict = settings.findConflict(for: KeyCombo(key: "r", command: true), excluding: .executeQuery) + #expect(conflict == .refresh) + } +} + +@Suite("Keyboard settings sanitization") +struct KeyboardSettingsSanitizeTests { + @Test("Bare-Space override on a menu action is dropped on load") + func dropsBareSpaceMenuOverride() { + let settings = KeyboardSettings(shortcuts: [ + ShortcutAction.toggleInspector.rawValue: KeyCombo(key: "space", isSpecialKey: true) + ]) + let sanitized = settings.sanitized() + #expect(!sanitized.isCustomized(.toggleInspector)) + #expect(sanitized.shortcut(for: .toggleInspector) == KeyboardSettings.defaultShortcuts[.toggleInspector]) + } + + @Test("Bare-key override on a grid action survives") + func keepsBareKeyGridOverride() { + let space = KeyCombo(key: "space", isSpecialKey: true) + let settings = KeyboardSettings(shortcuts: [ShortcutAction.previewFKReference.rawValue: space]) + #expect(settings.sanitized().shortcut(for: .previewFKReference) == space) + } + + @Test("Cleared sentinel survives") + func keepsClearedSentinel() { + let settings = KeyboardSettings(shortcuts: [ShortcutAction.executeQuery.rawValue: .cleared]) + let sanitized = settings.sanitized() + #expect(sanitized.isCustomized(.executeQuery)) + #expect(sanitized.keyboardShortcut(for: .executeQuery) == nil) + } + + @Test("Modifier override survives") + func keepsModifierOverride() { + let combo = KeyCombo(key: "r", command: true, shift: true) + let settings = KeyboardSettings(shortcuts: [ShortcutAction.toggleInspector.rawValue: combo]) + #expect(settings.sanitized().shortcut(for: .toggleInspector) == combo) + } + + @Test("Unknown action raw value survives sanitization") + func keepsUnknownRawValue() { + let combo = KeyCombo(key: "x", command: true) + let settings = KeyboardSettings(shortcuts: ["future.unknown.action": combo]) + #expect(settings.sanitized().shortcuts["future.unknown.action"] == combo) + } +} diff --git a/docs/features/keyboard-shortcuts.mdx b/docs/features/keyboard-shortcuts.mdx index 9c0584479..e6435d88a 100644 --- a/docs/features/keyboard-shortcuts.mdx +++ b/docs/features/keyboard-shortcuts.mdx @@ -324,6 +324,10 @@ Most menu shortcuts are rebindable in **Settings** > **Keyboard**. 4. Press the new key combination 5. The shortcut updates immediately in the menu bar + +Menu actions need a modifier key (`Cmd`, `Option`, `Control`, or `Shift`). A plain key like `Space` won't reach the menu, so TablePro asks you to add a modifier. Data grid actions that read the key directly, like Preview FK Reference (`Space`) and Cancel edit (`Escape`), are the exception. + + ### Clearing a Shortcut 1. Click the shortcut field for the action