diff --git a/rootshell/App/AppCommands.swift b/rootshell/App/AppCommands.swift index 61289d1e0..2942a6023 100644 --- a/rootshell/App/AppCommands.swift +++ b/rootshell/App/AppCommands.swift @@ -23,6 +23,9 @@ final class MenuShortcutState: ObservableObject { static let shared = MenuShortcutState() @Published var shortcuts: [KeybindAction: KeyboardShortcut] = [:] + /// Nested count so overlapping capture views don't restore the menu rail + /// while another is still recording. + private var recordingCaptureCount = 0 /// Whether a menu bar exists to carry app shortcuts. Both menu rails dispatch /// through UIApplication notifications rather than the responder chain, so a @@ -56,7 +59,32 @@ final class MenuShortcutState: ObservableObject { .store(in: &cancellables) } + /// Drop menu key equivalents while a shortcut is being recorded so the + /// capture view's `keyCommands` see the physical chord. Otherwise the menu + /// rail steals registered shortcuts (⌘T, ⌘N, …) and they never reach the + /// editor. ⌘. stays on its dedicated Send Escape item — that reserved + /// chord never arrives as a key event on Catalyst. + func beginRecordingCapture() { + recordingCaptureCount += 1 + if recordingCaptureCount == 1 { + shortcuts = [:] + } + } + + func endRecordingCapture() { + guard recordingCaptureCount > 0 else { return } + recordingCaptureCount -= 1 + if recordingCaptureCount == 0 { + rebuildShortcuts() + } + } + private func rebuildShortcuts() { + guard recordingCaptureCount == 0 else { + shortcuts = [:] + return + } + var newShortcuts: [KeybindAction: KeyboardShortcut] = [:] for binding in KeybindManager.shared.activeBindings { diff --git a/rootshell/Core/Keybinds/Keybind.swift b/rootshell/Core/Keybinds/Keybind.swift index 6beb95092..82d212336 100644 --- a/rootshell/Core/Keybinds/Keybind.swift +++ b/rootshell/Core/Keybinds/Keybind.swift @@ -21,6 +21,11 @@ struct Keybind: Codable, Identifiable, Hashable, Sendable { /// Optional parameter for the action (e.g., "1" for "increase_font_size:1") let actionParameter: String? + /// For an unbind targeting a parameterized action, the displaced binding's + /// parameter. Together with `sequence`, this identifies only that binding. + /// Optional so saved overrides from before parameterized unbinds still decode. + let unboundActionParameter: String? + /// Whether this is a user override (vs default) let isUserOverride: Bool @@ -34,6 +39,7 @@ struct Keybind: Codable, Identifiable, Hashable, Sendable { sequence: KeySequence, action: KeybindAction, actionParameter: String? = nil, + unboundActionParameter: String? = nil, isUserOverride: Bool = false, source: KeybindSource = .default ) { @@ -41,6 +47,7 @@ struct Keybind: Codable, Identifiable, Hashable, Sendable { self.sequence = sequence self.action = action self.actionParameter = actionParameter + self.unboundActionParameter = unboundActionParameter self.isUserOverride = isUserOverride self.source = source } @@ -58,6 +65,7 @@ struct Keybind: Codable, Identifiable, Hashable, Sendable { self.sequence = KeySequence(key: key, modifiers: modifiers) self.action = action self.actionParameter = actionParameter + self.unboundActionParameter = nil self.isUserOverride = isUserOverride self.source = source } @@ -74,6 +82,7 @@ struct Keybind: Codable, Identifiable, Hashable, Sendable { self.sequence = KeySequence(trigger: trigger) self.action = action self.actionParameter = actionParameter + self.unboundActionParameter = nil self.isUserOverride = isUserOverride self.source = source } @@ -124,6 +133,7 @@ struct Keybind: Codable, Identifiable, Hashable, Sendable { self.sequence = sequence self.action = action self.actionParameter = parameter + self.unboundActionParameter = nil self.isUserOverride = (source == .userOverride) self.source = source } @@ -136,6 +146,16 @@ struct Keybind: Codable, Identifiable, Hashable, Sendable { return "keybind = \(sequence.ghosttyFormat)=\(action.rawValue)" } + /// Ordinary unbinds suppress an entire action. Parameterized unbinds + /// suppress only the recorded sequence and parameter, preserving siblings. + func unbinds(_ binding: Keybind) -> Bool { + guard action == .unbind, actionParameter == binding.action.rawValue else { + return false + } + return !binding.action.isParameterized + || (unboundActionParameter == binding.actionParameter && sequence == binding.sequence) + } + // MARK: - Escape Sequence Decoding /// Decode escape sequences in text action parameters (Ghostty config format). diff --git a/rootshell/Core/Keybinds/KeybindManager.swift b/rootshell/Core/Keybinds/KeybindManager.swift index 5be04a035..873543a09 100644 --- a/rootshell/Core/Keybinds/KeybindManager.swift +++ b/rootshell/Core/Keybinds/KeybindManager.swift @@ -277,6 +277,11 @@ final class KeybindManager: ObservableObject { let paramLabel = parameter.map { ":\($0)" } ?? "" Self.logger.info("Setting override: \(sequence.ghosttyFormat) -> \(action.rawValue)\(paramLabel)") + // Snapshot who we are about to displace, before userOverrides change. + let victims = action == .unbind + ? [] + : conflicts(for: sequence, excluding: action, excludingParameter: parameter) + if action == .unbind { // Unbind is special: multiple actions can be unbound simultaneously. // Only remove duplicate unbinds for the same sequence. Keep non-unbind @@ -298,7 +303,27 @@ final class KeybindManager: ObservableObject { } } - // Add new override + if action != .unbind { + // Unbind the previous owners so they stay empty instead of + // falling back to a free default. Applied before the new + // binding so the new chord is not stripped. + for victim in victims { + let unbind = Keybind( + sequence: victim.sequence, + action: .unbind, + actionParameter: victim.action.rawValue, + unboundActionParameter: victim.action.isParameterized ? victim.actionParameter : nil, + isUserOverride: true, + source: .userOverride + ) + userOverrides.removeAll { + unbind.unbinds($0) || $0.unbinds(victim) + } + userOverrides.append(unbind) + } + } + + // New binding last so it wins over any victim unbind for this sequence. let override = Keybind( sequence: sequence, action: action, @@ -644,6 +669,14 @@ final class KeybindManager: ObservableObject { // Apply user overrides (highest priority) for override in userOverrides { + if override.action == .unbind { + // Match the displaced owner, not every binding using its + // sequence. Parameterized owners also match their parameter + // and sequence so sibling bindings remain available. + bindings.removeAll { override.unbinds($0) } + continue + } + // Remove any existing binding for this action (skip parameterized) if !override.action.isParameterized { bindings.removeAll { $0.action == override.action } @@ -651,9 +684,7 @@ final class KeybindManager: ObservableObject { // Remove any existing binding for this sequence (handle conflicts) bindings.removeAll { $0.sequence == override.sequence } - if override.action != .unbind { - bindings.append(override) - } + bindings.append(override) } // Sort by category and name for consistent ordering @@ -798,10 +829,17 @@ final class KeybindManager: ObservableObject { // MARK: - Conflict Detection /// Check if a sequence would conflict with existing bindings - func conflicts(for sequence: KeySequence, excluding action: KeybindAction? = nil) -> [Keybind] { + func conflicts( + for sequence: KeySequence, + excluding action: KeybindAction? = nil, + excludingParameter parameter: String? = nil + ) -> [Keybind] { activeBindings.filter { binding in - // Skip the action we're checking for - if let excludedAction = action, binding.action == excludedAction { + // A parameterized editor owns only its matching parameter. A new + // profile has no parameter yet, so all existing profiles conflict. + if let excludedAction = action, binding.action == excludedAction, + !excludedAction.isParameterized + || (parameter != nil && binding.actionParameter == parameter) { return false } diff --git a/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift b/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift index 45b1fd3a1..8f7f09013 100644 --- a/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift +++ b/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift @@ -23,6 +23,8 @@ struct KeybindEditorView: View { @Environment(\.sheetThemeColors) private var sheetThemeColors @ObservedObject private var keybindManager = KeybindManager.shared + /// Action the editor was opened for. `currentAction` may change if the user + /// jumps to a conflicting shortcut without dismissing the sheet. let action: KeybindAction /// Optional parameter for parameterized actions (e.g. profile UUID for `open_profile`) var actionParameter: String? = nil @@ -37,34 +39,98 @@ struct KeybindEditorView: View { /// `KeybindManager` route through this callback so the actual write /// happens in the parent's sheet-onDismiss closure. var onOutcome: (KeybindEditorOutcome) -> Void = { _ in } + /// Enables in-sheet action switching. The parent must apply subsequent + /// outcomes to this action; callers without this callback stay on the + /// action the editor was opened for. + var onSwitchAction: ((KeybindAction) -> Void)? + @State private var currentAction: KeybindAction @State private var isCapturing = false @State private var showSequenceCapture = false @State private var captureError: String? + @State private var pendingCapture: KeySequence? + @State private var conflictingBindings: [Keybind] = [] + + init( + action: KeybindAction, + actionParameter: String? = nil, + titleOverride: String? = nil, + allowsRestoreDefault: Bool = true, + draftSequence: KeySequence?? = nil, + onOutcome: @escaping (KeybindEditorOutcome) -> Void = { _ in }, + onSwitchAction: ((KeybindAction) -> Void)? = nil + ) { + self.action = action + self.actionParameter = actionParameter + self.titleOverride = titleOverride + self.allowsRestoreDefault = allowsRestoreDefault + self.draftSequence = draftSequence + self.onOutcome = onOutcome + self.onSwitchAction = onSwitchAction + _currentAction = State(initialValue: action) + } /// Current binding for this action (may be nil if displaced by external config) private var managerBinding: Keybind? { if let actionParameter { - keybindManager.keybind(for: action, parameter: actionParameter) + keybindManager.keybind(for: currentAction, parameter: actionParameter) } else { - keybindManager.keybind(for: action) + keybindManager.keybind(for: currentAction) } } /// Sequence shown in the "Current Shortcut" section private var displayedSequence: KeySequence? { - if let draftSequence { + if currentAction == action, let draftSequence { return draftSequence } return managerBinding?.sequence } private var showsCustomBadge: Bool { - draftSequence == nil && (managerBinding?.isUserOverride == true) + (currentAction != action || draftSequence == nil) && managerBinding?.isUserOverride == true } private var displayTitle: String { - titleOverride ?? action.displayName + if currentAction == action, let titleOverride { + return titleOverride + } + return currentAction.displayName + } + + /// Single conflicting action the user can jump to from the warning, if any. + /// Profile (parameterized) editors stay on the profile instead of jumping. + private var editableConflictAction: KeybindAction? { + guard onSwitchAction != nil, + !action.isParameterized, + actionParameter == nil, + conflictingBindings.count == 1, + let conflict = conflictingBindings.first?.action, + conflict != currentAction, + KeybindAction.customizableActions.contains(conflict) + else { return nil } + return conflict + } + + private var overrideButtonTitle: String { + if conflictingBindings.count == 1, let name = conflictingBindings.first?.action.displayName { + return "Unbind \(name)" + } + return "Unbind Other Shortcuts" + } + + private var conflictMessage: String { + let chord = pendingCapture?.symbolDescription ?? "This shortcut" + if conflictingBindings.count == 1, let conflict = conflictingBindings.first { + if conflict.sequence == pendingCapture { + return "\(chord) is currently bound to \(conflict.action.displayName). Overriding will remove it from that action." + } + return "\(chord) conflicts with \(conflict.sequence.symbolDescription) (\(conflict.action.displayName)). Overriding will unbind that shortcut." + } + let details = conflictingBindings + .map { "\($0.action.displayName) (\($0.sequence.symbolDescription))" } + .joined(separator: ", ") + return "\(chord) conflicts with: \(details). Overriding will unbind those shortcuts." } private var sheetBackground: Color { @@ -84,7 +150,7 @@ struct KeybindEditorView: View { .font(.title2) .fontWeight(.semibold) - Text(action.category.displayName) + Text(currentAction.category.displayName) .font(.subheadline) .foregroundColor(.secondary) .padding(.horizontal, 12) @@ -125,6 +191,27 @@ struct KeybindEditorView: View { .background(rowBackground) .cornerRadius(12) } + + if conflictingBindings.isEmpty { + if allowsRestoreDefault, + (currentAction != action || draftSequence == nil), + (managerBinding != nil && managerBinding!.isUserOverride) + || keybindManager.isActionUnbound(currentAction) { + Button("Restore Default") { + onOutcome(.restoreDefault) + dismiss() + } + .foregroundColor(.orange) + } + + if displayedSequence != nil { + Button(allowsRestoreDefault ? "Unbind Shortcut" : "Clear Shortcut") { + onOutcome(.unbind) + dismiss() + } + .foregroundColor(.red) + } + } } // Capture area @@ -139,23 +226,22 @@ struct KeybindEditorView: View { } ) .frame(height: 120) + } else if pendingCapture != nil, !conflictingBindings.isEmpty { + conflictWarningCard + .padding(.horizontal) } else { VStack(spacing: 12) { Button { - captureError = nil - isCapturing = true - showSequenceCapture = false + beginCapture(sequenceMode: false) } label: { Label("Record New Shortcut", systemImage: "keyboard") .frame(maxWidth: .infinity) } .buttonStyle(.borderedProminent) - if action != .toggle_visor { + if currentAction != .toggle_visor { Button { - captureError = nil - isCapturing = true - showSequenceCapture = true + beginCapture(sequenceMode: true) } label: { Label("Record Key Sequence", systemImage: "keyboard.badge.ellipsis") .frame(maxWidth: .infinity) @@ -175,29 +261,9 @@ struct KeybindEditorView: View { } Spacer() - - // Action buttons - VStack(spacing: 8) { - if allowsRestoreDefault, - draftSequence == nil, - (managerBinding != nil && managerBinding!.isUserOverride) || keybindManager.isActionUnbound(action) { - Button("Restore Default") { - onOutcome(.restoreDefault) - dismiss() - } - .foregroundColor(.orange) - } - - if displayedSequence != nil { - Button(allowsRestoreDefault ? "Unbind Shortcut" : "Clear Shortcut") { - onOutcome(.unbind) - dismiss() - } - .foregroundColor(.red) - } - } - .padding() } + .animation(.easeInOut(duration: 0.2), value: currentAction) + .animation(.easeInOut(duration: 0.2), value: conflictingBindings.isEmpty) .frame(maxWidth: .infinity, maxHeight: .infinity) .background(sheetBackground.ignoresSafeArea()) .navigationTitle("Edit Shortcut") @@ -210,8 +276,70 @@ struct KeybindEditorView: View { } } + private var conflictWarningCard: some View { + VStack(alignment: .leading, spacing: 12) { + Label("Shortcut Already in Use", systemImage: "exclamationmark.triangle.fill") + .font(.headline) + .foregroundStyle(.orange) + + Text(conflictMessage) + .font(.subheadline) + .foregroundColor(.primary) + .fixedSize(horizontal: false, vertical: true) + + if let pendingCapture { + Text(pendingCapture.symbolDescription) + .font(.system(.title3, design: .monospaced).weight(.medium)) + .frame(maxWidth: .infinity) + .padding(.vertical, 8) + .background(rowBackground) + .cornerRadius(8) + } + + VStack(spacing: 8) { + Button(role: .destructive) { + confirmOverride() + } label: { + Text(overrideButtonTitle) + .frame(maxWidth: .infinity) + } + .buttonStyle(.borderedProminent) + .tint(.orange) + + if let editableConflictAction { + Button { + openConflictingAction(editableConflictAction) + } label: { + Text("Edit \(editableConflictAction.displayName) Instead") + .frame(maxWidth: .infinity) + } + .buttonStyle(.bordered) + } + + Button { + clearPendingConflict() + } label: { + Text("Cancel") + .frame(maxWidth: .infinity) + } + .buttonStyle(.bordered) + } + } + .padding(16) + .frame(maxWidth: .infinity, alignment: .leading) + .background(Color.orange.opacity(0.12)) + .cornerRadius(12) + } + // MARK: - Capture Handler + private func beginCapture(sequenceMode: Bool) { + captureError = nil + clearPendingConflict() + isCapturing = true + showSequenceCapture = sequenceMode + } + private func handleCapture(_ sequence: KeySequence) { // Reject sequences whose first trigger is a default control-character // binding. `KeySequenceTracker.processFirstKey` passes those through @@ -228,6 +356,48 @@ struct KeybindEditorView: View { showSequenceCapture = false return } + + let conflicts = keybindManager.conflicts( + for: sequence, + excluding: currentAction, + excludingParameter: actionParameter + ) + isCapturing = false + showSequenceCapture = false + + if !conflicts.isEmpty { + // Keep the sheet open and ask before stealing another action's chord. + pendingCapture = sequence + conflictingBindings = conflicts + return + } + + commitCapture(sequence) + } + + private func confirmOverride() { + guard let sequence = pendingCapture else { return } + clearPendingConflict() + commitCapture(sequence) + } + + private func openConflictingAction(_ conflict: KeybindAction) { + withAnimation(.easeInOut(duration: 0.2)) { + clearPendingConflict() + captureError = nil + isCapturing = false + showSequenceCapture = false + currentAction = conflict + } + onSwitchAction?(conflict) + } + + private func clearPendingConflict() { + pendingCapture = nil + conflictingBindings = [] + } + + private func commitCapture(_ sequence: KeySequence) { // Hand the outcome to the parent. The parent applies it in the sheet's // onDismiss closure — i.e. after the sheet has fully dismissed — so the // @Published cascade in setOverride runs in a quiescent view hierarchy @@ -255,6 +425,7 @@ struct ShortcutCaptureView: UIViewRepresentable { func updateUIView(_ uiView: ShortcutCaptureUIView, context: Context) { uiView.configure(isSequenceMode: isSequenceMode, themeColors: themeColors) + uiView.claimFirstResponder() } } @@ -267,6 +438,7 @@ class ShortcutCaptureUIView: UIView { private var firstTrigger: KeyTrigger? private var firstTriggerTime: Date? private var hasCompleted = false + private var isSuppressingMenuShortcuts = false private let instructionLabel = UILabel() private let captureLabel = UILabel() private var themeColors: SheetThemeColors? @@ -382,10 +554,22 @@ class ShortcutCaptureUIView: UIView { override func didMoveToWindow() { super.didMoveToWindow() if window != nil { - becomeFirstResponder() + claimFirstResponder() + suppressMenuShortcutsForCapture() + } else { + restoreMenuShortcutsAfterCapture() } } + deinit { + restoreMenuShortcutsAfterCapture() + } + + func claimFirstResponder() { + guard window != nil, !isFirstResponder else { return } + _ = becomeFirstResponder() + } + // MARK: - Key Commands for Capturing Shortcuts /// Override keyCommands to intercept system shortcuts on Mac Catalyst @@ -511,13 +695,35 @@ class ShortcutCaptureUIView: UIView { } /// Catalyst delivers the reserved Cmd+Period chord only through the menu - /// rail (nil-target menuSystemCancel action). The action reaches this view - /// first while it owns first responder, so recording wins over the - /// terminal handler. + /// rail (nil-target menuSystemCancel action). That reserved chord never + /// arrives as a key event, so recording still has to implement this one + /// selector. Every other menu-owned shortcut is handled by temporarily + /// clearing `MenuShortcutState` so `keyCommands` sees the physical press. @objc func menuSystemCancel(_ sender: Any?) { processCapture(trigger: .commandPeriod) } + private func suppressMenuShortcutsForCapture() { + guard !isSuppressingMenuShortcuts else { return } + isSuppressingMenuShortcuts = true + let apply = { + MenuShortcutState.shared.beginRecordingCapture() + } + if Thread.isMainThread { + MainActor.assumeIsolated(apply) + } else { + DispatchQueue.main.async(execute: apply) + } + } + + private func restoreMenuShortcutsAfterCapture() { + guard isSuppressingMenuShortcuts else { return } + isSuppressingMenuShortcuts = false + DispatchQueue.main.async { + MenuShortcutState.shared.endRecordingCapture() + } + } + override func pressesBegan(_ presses: Set, with event: UIPressesEvent?) { for press in presses { guard let key = press.key else { continue } @@ -535,8 +741,7 @@ class ShortcutCaptureUIView: UIView { // Command stripped or as a translated Escape. Normalize either // representation so the chord is recordable; the twin keyCommands // delivery dedups via duplicateDeliveryWindow since both produce - // the identical trigger. For stripped events, use the physical - // chord proven by GCKeyboard; layout text has lost Command too. + // the identical trigger. if (key.keyCode != .keyboardEscape && KeyCode.sentinelKey(for: key.characters) == .escape) || ((key.keyCode == .keyboardPeriod || key.keyCode == .keyboardEscape) && KeyboardTracker.isSystemCancelChordPhysicallyDown()) { diff --git a/rootshell/UI/Settings/Keyboard/KeyboardShortcutsSettingsView.swift b/rootshell/UI/Settings/Keyboard/KeyboardShortcutsSettingsView.swift index 4a832f84e..de4e53fad 100644 --- a/rootshell/UI/Settings/Keyboard/KeyboardShortcutsSettingsView.swift +++ b/rootshell/UI/Settings/Keyboard/KeyboardShortcutsSettingsView.swift @@ -20,6 +20,9 @@ struct KeyboardShortcutsSettingsView: View { /// capture, restore-default, unbind — so every mutation is deferred /// uniformly. @State private var pendingOutcome: (action: KeybindAction, outcome: KeybindEditorOutcome)? + /// Tracks in-sheet jumps via “Edit [action] Instead” so dismiss applies to + /// the action currently on screen, not the one the sheet was opened for. + @State private var editorAction: KeybindAction? @State private var showConfigFilePicker = false @State private var showConfigEditor = false @State private var showResetConfirmation = false @@ -89,9 +92,17 @@ struct KeyboardShortcutsSettingsView: View { item: $editingAction, onDismiss: applyPendingOutcome ) { action in - KeybindEditorView(action: action, onOutcome: { outcome in - pendingOutcome = (action, outcome) - }) + KeybindEditorView( + action: action, + onOutcome: { outcome in + pendingOutcome = (editorAction ?? action, outcome) + }, + onSwitchAction: { conflict in + editorAction = conflict + selectedCategory = conflict.category + } + ) + .onAppear { editorAction = action } .themedSubSheet(sheetThemeColors) } .sheet(isPresented: $showConfigEditor) {