From cfb73244e94049cf277e1ab714b3324ceaa041d2 Mon Sep 17 00:00:00 2001 From: Joshua Van Deren Date: Sat, 12 Sep 2026 22:54:14 -0600 Subject: [PATCH 1/5] Warn before stealing a keyboard shortcut that's already in use. Recording a chord that another action already owns used to succeed silently, so users couldn't tell who held a shortcut or that the old binding would disappear. Co-authored-by: Cursor --- .../Features/Visor/iPadVisorSettings.swift | 2 +- .../Settings/Keyboard/KeybindEditorView.swift | 339 ++++++++++++++---- .../KeyboardShortcutsSettingsView.swift | 13 +- 3 files changed, 279 insertions(+), 75 deletions(-) diff --git a/rootshell/Features/Visor/iPadVisorSettings.swift b/rootshell/Features/Visor/iPadVisorSettings.swift index b9f69e8dd..f9df05fb8 100644 --- a/rootshell/Features/Visor/iPadVisorSettings.swift +++ b/rootshell/Features/Visor/iPadVisorSettings.swift @@ -87,7 +87,7 @@ struct iPadVisorSettingsView: View { .themedList() .navigationTitle("Visor") .sheet(isPresented: $editing, onDismiss: applyOutcome) { - KeybindEditorView(action: .toggle_visor) { outcome = $0 } + KeybindEditorView(action: .toggle_visor) { _, result in outcome = result } .themedSubSheet(sheetThemeColors) } } diff --git a/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift b/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift index 45b1fd3a1..bd5216fe9 100644 --- a/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift +++ b/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift @@ -23,48 +23,67 @@ struct KeybindEditorView: View { @Environment(\.sheetThemeColors) private var sheetThemeColors @ObservedObject private var keybindManager = KeybindManager.shared - let action: KeybindAction - /// Optional parameter for parameterized actions (e.g. profile UUID for `open_profile`) - var actionParameter: String? = nil - /// Optional title override (e.g. profile name). Falls back to `action.displayName`. - var titleOverride: String? = nil - /// When false, hide "Restore Default" (used for profile shortcuts whose default is none). - var allowsRestoreDefault: Bool = true - /// When non-nil, display this sequence instead of looking up the live KeybindManager - /// binding. Lets parents (e.g. profile editor) keep a draft until Save. - var draftSequence: KeySequence?? = nil /// Reports the user's choice to the parent. All paths that mutate /// `KeybindManager` route through this callback so the actual write - /// happens in the parent's sheet-onDismiss closure. - var onOutcome: (KeybindEditorOutcome) -> Void = { _ in } - + /// happens in the parent's sheet-onDismiss closure. Includes the action + /// currently on screen, which may have changed if the user jumped to a + /// conflicting shortcut without dismissing the sheet. + var onOutcome: (KeybindAction, KeybindEditorOutcome) -> Void = { _, _ in } + /// Optional: parent can follow an in-sheet jump to another action (e.g. to + /// keep the shortcuts list on the matching category). The sheet stays open. + 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, + onOutcome: @escaping (KeybindAction, KeybindEditorOutcome) -> Void = { _, _ in }, + onSwitchAction: ((KeybindAction) -> Void)? = nil + ) { + 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) - } else { - keybindManager.keybind(for: action) - } + private var binding: Keybind? { + keybindManager.keybind(for: currentAction) } - /// Sequence shown in the "Current Shortcut" section - private var displayedSequence: KeySequence? { - if let draftSequence { - return draftSequence - } - return managerBinding?.sequence + /// Single conflicting action the user can jump to from the warning, if any. + private var editableConflictAction: KeybindAction? { + guard conflictingBindings.count == 1, + let conflict = conflictingBindings.first?.action, + conflict != currentAction, + KeybindAction.customizableActions.contains(conflict) + else { return nil } + return conflict } - private var showsCustomBadge: Bool { - draftSequence == nil && (managerBinding?.isUserOverride == true) + private var overrideButtonTitle: String { + if conflictingBindings.count == 1, let name = conflictingBindings.first?.action.displayName { + return "Unbind \(name)" + } + return "Unbind Other Shortcuts" } - private var displayTitle: String { - titleOverride ?? action.displayName + 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 { @@ -80,11 +99,11 @@ struct KeybindEditorView: View { VStack(spacing: 24) { // Action info VStack(spacing: 8) { - Text(displayTitle) + Text(currentAction.displayName) .font(.title2) .fontWeight(.semibold) - Text(action.category.displayName) + Text(currentAction.category.displayName) .font(.subheadline) .foregroundColor(.secondary) .padding(.horizontal, 12) @@ -102,15 +121,15 @@ struct KeybindEditorView: View { .font(.headline) .foregroundColor(.secondary) - if let displayedSequence { - Text(displayedSequence.symbolDescription) + if let binding { + Text(binding.sequence.symbolDescription) .font(.system(size: 28, weight: .medium, design: .monospaced)) .padding(.horizontal, 24) .padding(.vertical, 16) .background(rowBackground) .cornerRadius(12) - if showsCustomBadge { + if binding.isUserOverride { Label("Custom", systemImage: "star.fill") .font(.caption) .foregroundStyle(.tint) @@ -125,6 +144,24 @@ struct KeybindEditorView: View { .background(rowBackground) .cornerRadius(12) } + + if conflictingBindings.isEmpty { + if (binding != nil && binding!.isUserOverride) || keybindManager.isActionUnbound(currentAction) { + Button("Restore Default") { + onOutcome(currentAction, .restoreDefault) + dismiss() + } + .foregroundColor(.orange) + } + + if binding != nil { + Button("Unbind Shortcut") { + onOutcome(currentAction, .unbind) + dismiss() + } + .foregroundColor(.red) + } + } } // Capture area @@ -139,23 +176,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 +211,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 +226,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,11 +306,49 @@ struct KeybindEditorView: View { showSequenceCapture = false return } + + let conflicts = keybindManager.conflicts(for: sequence, excluding: currentAction) + 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 // rather than mid-dismissal. - onOutcome(.captured(sequence)) + onOutcome(currentAction, .captured(sequence)) dismiss() } } @@ -255,6 +371,7 @@ struct ShortcutCaptureView: UIViewRepresentable { func updateUIView(_ uiView: ShortcutCaptureUIView, context: Context) { uiView.configure(isSequenceMode: isSequenceMode, themeColors: themeColors) + uiView.claimFirstResponder() } } @@ -382,10 +499,15 @@ class ShortcutCaptureUIView: UIView { override func didMoveToWindow() { super.didMoveToWindow() if window != nil { - becomeFirstResponder() + claimFirstResponder() } } + func claimFirstResponder() { + guard window != nil, !isFirstResponder else { return } + _ = becomeFirstResponder() + } + // MARK: - Key Commands for Capturing Shortcuts /// Override keyCommands to intercept system shortcuts on Mac Catalyst @@ -510,14 +632,90 @@ class ShortcutCaptureUIView: UIView { onCancel?() } - /// 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. + /// Catalyst delivers reserved and menu-owned chords (⌘T, ⌘N, ⌘.) through + /// the menu rail as a nil-target `sendAction`. Those never reach + /// `keyCommands` or `pressesBegan`. The walk starts at first responder, so + /// implementing the same selectors here records the chord instead of + /// creating a tab / firing the bound action. @objc func menuSystemCancel(_ sender: Any?) { processCapture(trigger: .commandPeriod) } + @objc func menuCreateLocalShell(_ sender: Any?) { captureMenuBinding(.new_local_shell) } + @objc func menuNewTab(_ sender: Any?) { captureMenuBinding(.new_tab) } + @objc func menuNewWindow(_ sender: Any?) { captureMenuBinding(.new_window) } + @objc func menuDuplicateTabWithSSH(_ sender: Any?) { captureMenuBinding(.duplicate_ssh_tab) } + @objc func menuClearScreen(_ sender: Any?) { captureMenuBinding(.clear_screen) } + @objc func findInTerminal(_ sender: Any?) { captureMenuBinding(.start_search) } + @objc func increaseFontSize(_ sender: Any?) { captureMenuBinding(.increase_font_size) } + @objc func decreaseFontSize(_ sender: Any?) { captureMenuBinding(.decrease_font_size) } + @objc func resetFontSizeToDefault(_ sender: Any?) { captureMenuBinding(.reset_font_size) } + @objc func menuSplitRight(_ sender: Any?) { captureMenuBinding(.split_right) } + @objc func menuSplitDown(_ sender: Any?) { captureMenuBinding(.split_down) } + @objc func menuNavigateSplitLeft(_ sender: Any?) { captureMenuBinding(.navigate_split_left) } + @objc func menuNavigateSplitRight(_ sender: Any?) { captureMenuBinding(.navigate_split_right) } + @objc func menuNavigateSplitUp(_ sender: Any?) { captureMenuBinding(.navigate_split_up) } + @objc func menuNavigateSplitDown(_ sender: Any?) { captureMenuBinding(.navigate_split_down) } + @objc func menuToggleSplitZoom(_ sender: Any?) { captureMenuBinding(.toggle_split_zoom) } + @objc func menuEqualizeSplits(_ sender: Any?) { captureMenuBinding(.equalize_splits) } + @objc func menuToggleTabBar(_ sender: Any?) { captureMenuBinding(.toggle_tab_bar) } + @objc func menuToggleGroupMode(_ sender: Any?) { captureMenuBinding(.toggle_group_mode) } + @objc func menuToggleTabSwitcher(_ sender: Any?) { captureMenuBinding(.toggle_tab_switcher) } + @objc func menuToggleTabExpose(_ sender: Any?) { captureMenuBinding(.toggle_tab_expose) } + @objc func menuPreviousTab(_ sender: Any?) { captureMenuBinding(.previous_tab) } + @objc func menuNextTab(_ sender: Any?) { captureMenuBinding(.next_tab) } + @objc func menuSelectTab1(_ sender: Any?) { captureMenuBinding(.select_tab_1) } + @objc func menuSelectTab2(_ sender: Any?) { captureMenuBinding(.select_tab_2) } + @objc func menuSelectTab3(_ sender: Any?) { captureMenuBinding(.select_tab_3) } + @objc func menuSelectTab4(_ sender: Any?) { captureMenuBinding(.select_tab_4) } + @objc func menuSelectTab5(_ sender: Any?) { captureMenuBinding(.select_tab_5) } + @objc func menuSelectTab6(_ sender: Any?) { captureMenuBinding(.select_tab_6) } + @objc func menuSelectTab7(_ sender: Any?) { captureMenuBinding(.select_tab_7) } + @objc func menuSelectTab8(_ sender: Any?) { captureMenuBinding(.select_tab_8) } + @objc func menuSelectTab9(_ sender: Any?) { captureMenuBinding(.select_tab_9) } + @objc func menuBrowseHosts(_ sender: Any?) { captureMenuBinding(.browse_hosts) } + @objc func menuBrowseProfiles(_ sender: Any?) { captureMenuBinding(.browse_profiles) } + @objc func menuToggleAIAgent(_ sender: Any?) { captureMenuBinding(.toggle_ai_agent) } + @objc func menuToggleVoiceAgent(_ sender: Any?) { captureMenuBinding(.toggle_voice_agent) } + @objc func menuOpenSettings(_ sender: Any?) { captureMenuBinding(.open_settings) } + @objc func menuShowTmuxSessions(_ sender: Any?) { captureMenuBinding(.show_tmux_sessions) } + @objc func menuDetachSession(_ sender: Any?) { captureMenuBinding(.detach_session) } + @objc func menuDetachAllSessions(_ sender: Any?) { captureMenuBinding(.detach_all_sessions) } + @objc func menuDetachOtherClients(_ sender: Any?) { captureMenuBinding(.detach_other_clients) } + @objc func menuToggleTransparency(_ sender: Any?) { captureMenuBinding(.toggle_transparency) } + @objc func menuToggleTitleBar(_ sender: Any?) { captureMenuBinding(.toggle_titlebar) } + @objc func menuToggleAutoRedact(_ sender: Any?) { captureMenuBinding(.toggle_auto_redact) } + @objc func menuToggleBackgroundEffect(_ sender: Any?) { captureMenuBinding(.toggle_background_effect) } + @objc func menuToggleFullScreen(_ sender: Any?) { captureMenuBinding(.toggle_full_screen) } + @objc func menuToggleCompose(_ sender: Any?) { captureMenuBinding(.toggle_compose) } + @objc func menuToggleMouseCapture(_ sender: Any?) { captureMenuBinding(.toggle_mouse_capture) } + @objc func menuToggleClipboardManager(_ sender: Any?) { captureMenuBinding(.toggle_clipboard_manager) } + @objc func menuToggleThemePicker(_ sender: Any?) { captureMenuBinding(.toggle_theme_picker) } + @objc func menuToggleQuickSettings(_ sender: Any?) { captureMenuBinding(.toggle_quick_settings) } + @objc func menuScrollPageUp(_ sender: Any?) { captureMenuBinding(.scroll_page_up) } + @objc func menuScrollPageDown(_ sender: Any?) { captureMenuBinding(.scroll_page_down) } + @objc func menuScrollToTop(_ sender: Any?) { captureMenuBinding(.scroll_to_top) } + @objc func menuScrollToBottom(_ sender: Any?) { captureMenuBinding(.scroll_to_bottom) } + @objc func menuBrightnessBoost(_ sender: Any?) { captureMenuBinding(.brightness_boost) } + @objc func menuCycleInputSource(_ sender: Any?) { captureMenuBinding(.cycle_input_source) } + @objc func menuPreviousGroup(_ sender: Any?) { captureMenuBinding(.previous_group) } + @objc func menuNextGroup(_ sender: Any?) { captureMenuBinding(.next_group) } + + /// Record the chord that just fired a menu item. Use the bound key plus + /// modifiers that are physically held so Shift+⌘T records as Shift+⌘T + /// even when the menu item itself is ⌘T. + private func captureMenuBinding(_ action: KeybindAction) { + guard let binding = KeybindManager.shared.keybind(for: action), + let first = binding.sequence.first, + !binding.sequence.isSequence + else { return } + + var modifiers = first.modifiers + let hardware = KeybindModifiers(uiModifierFlags: KeyboardTracker.shared.hardwareModifierFlags) + modifiers.formUnion(hardware) + processCapture(trigger: KeyTrigger(key: first.key, modifiers: modifiers)) + } + override func pressesBegan(_ presses: Set, with event: UIPressesEvent?) { for press in presses { guard let key = press.key else { continue } @@ -535,8 +733,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..499a51704 100644 --- a/rootshell/UI/Settings/Keyboard/KeyboardShortcutsSettingsView.swift +++ b/rootshell/UI/Settings/Keyboard/KeyboardShortcutsSettingsView.swift @@ -89,9 +89,16 @@ struct KeyboardShortcutsSettingsView: View { item: $editingAction, onDismiss: applyPendingOutcome ) { action in - KeybindEditorView(action: action, onOutcome: { outcome in - pendingOutcome = (action, outcome) - }) + KeybindEditorView( + action: action, + onOutcome: { editedAction, outcome in + pendingOutcome = (editedAction, outcome) + }, + onSwitchAction: { conflict in + // Stay in the same sheet; just keep the list behind it in sync. + selectedCategory = conflict.category + } + ) .themedSubSheet(sheetThemeColors) } .sheet(isPresented: $showConfigEditor) { From 49b9b38df79244a94637785d8a7a96b84a1353e1 Mon Sep 17 00:00:00 2001 From: Joshua Van Deren Date: Sat, 12 Sep 2026 23:05:19 -0600 Subject: [PATCH 2/5] Record menu-owned shortcuts by pausing menu key equivalents. A hardcoded list of menu selectors would drift whenever a new File or View shortcut is added; clearing MenuShortcutState during capture lets the editor see the physical chord from the live binding table instead. Co-authored-by: Cursor --- rootshell/App/AppCommands.swift | 50 ++++++-- .../Settings/Keyboard/KeybindEditorView.swift | 110 +++++------------- 2 files changed, 71 insertions(+), 89 deletions(-) diff --git a/rootshell/App/AppCommands.swift b/rootshell/App/AppCommands.swift index 61289d1e0..72da9c2e0 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 { @@ -556,14 +584,6 @@ struct ShellCommands: Commands { ) } .modifier(DynamicShortcut(action: .open_settings, shortcuts: shortcutState.shortcuts)) - - Button("Quick Settings…") { - UIApplication.shared.sendAction( - #selector(UIApplication.menuToggleQuickSettings(_:)), - to: nil, from: nil, for: nil - ) - } - .modifier(DynamicShortcut(action: .toggle_quick_settings, shortcuts: shortcutState.shortcuts)) } } } @@ -625,13 +645,21 @@ struct WindowCommands: Commands { } .modifier(DynamicShortcut(action: .show_tmux_sessions, shortcuts: shortcutState.shortcuts)) - Button("Discover Sessions") { + Button("Detach Session") { + UIApplication.shared.sendAction( + #selector(Ghostty.TerminalView.menuDetachSession(_:)), + to: nil, from: nil, for: nil + ) + } + .modifier(DynamicShortcut(action: .detach_session, shortcuts: shortcutState.shortcuts)) + + Button("Detach All Sessions") { UIApplication.shared.sendAction( - #selector(Ghostty.TerminalView.menuDiscoverSessions(_:)), + #selector(Ghostty.TerminalView.menuDetachAllSessions(_:)), to: nil, from: nil, for: nil ) } - .modifier(DynamicShortcut(action: .discover_sessions, shortcuts: shortcutState.shortcuts)) + .modifier(DynamicShortcut(action: .detach_all_sessions, shortcuts: shortcutState.shortcuts)) Button("Detach Other Clients") { UIApplication.shared.sendAction( diff --git a/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift b/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift index bd5216fe9..bd2083122 100644 --- a/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift +++ b/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift @@ -384,6 +384,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? @@ -500,9 +501,16 @@ class ShortcutCaptureUIView: UIView { super.didMoveToWindow() if window != nil { claimFirstResponder() + suppressMenuShortcutsForCapture() + } else { + restoreMenuShortcutsAfterCapture() } } + deinit { + restoreMenuShortcutsAfterCapture() + } + func claimFirstResponder() { guard window != nil, !isFirstResponder else { return } _ = becomeFirstResponder() @@ -632,88 +640,34 @@ class ShortcutCaptureUIView: UIView { onCancel?() } - /// Catalyst delivers reserved and menu-owned chords (⌘T, ⌘N, ⌘.) through - /// the menu rail as a nil-target `sendAction`. Those never reach - /// `keyCommands` or `pressesBegan`. The walk starts at first responder, so - /// implementing the same selectors here records the chord instead of - /// creating a tab / firing the bound action. + /// Catalyst delivers the reserved Cmd+Period chord only through the menu + /// 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) } - @objc func menuCreateLocalShell(_ sender: Any?) { captureMenuBinding(.new_local_shell) } - @objc func menuNewTab(_ sender: Any?) { captureMenuBinding(.new_tab) } - @objc func menuNewWindow(_ sender: Any?) { captureMenuBinding(.new_window) } - @objc func menuDuplicateTabWithSSH(_ sender: Any?) { captureMenuBinding(.duplicate_ssh_tab) } - @objc func menuClearScreen(_ sender: Any?) { captureMenuBinding(.clear_screen) } - @objc func findInTerminal(_ sender: Any?) { captureMenuBinding(.start_search) } - @objc func increaseFontSize(_ sender: Any?) { captureMenuBinding(.increase_font_size) } - @objc func decreaseFontSize(_ sender: Any?) { captureMenuBinding(.decrease_font_size) } - @objc func resetFontSizeToDefault(_ sender: Any?) { captureMenuBinding(.reset_font_size) } - @objc func menuSplitRight(_ sender: Any?) { captureMenuBinding(.split_right) } - @objc func menuSplitDown(_ sender: Any?) { captureMenuBinding(.split_down) } - @objc func menuNavigateSplitLeft(_ sender: Any?) { captureMenuBinding(.navigate_split_left) } - @objc func menuNavigateSplitRight(_ sender: Any?) { captureMenuBinding(.navigate_split_right) } - @objc func menuNavigateSplitUp(_ sender: Any?) { captureMenuBinding(.navigate_split_up) } - @objc func menuNavigateSplitDown(_ sender: Any?) { captureMenuBinding(.navigate_split_down) } - @objc func menuToggleSplitZoom(_ sender: Any?) { captureMenuBinding(.toggle_split_zoom) } - @objc func menuEqualizeSplits(_ sender: Any?) { captureMenuBinding(.equalize_splits) } - @objc func menuToggleTabBar(_ sender: Any?) { captureMenuBinding(.toggle_tab_bar) } - @objc func menuToggleGroupMode(_ sender: Any?) { captureMenuBinding(.toggle_group_mode) } - @objc func menuToggleTabSwitcher(_ sender: Any?) { captureMenuBinding(.toggle_tab_switcher) } - @objc func menuToggleTabExpose(_ sender: Any?) { captureMenuBinding(.toggle_tab_expose) } - @objc func menuPreviousTab(_ sender: Any?) { captureMenuBinding(.previous_tab) } - @objc func menuNextTab(_ sender: Any?) { captureMenuBinding(.next_tab) } - @objc func menuSelectTab1(_ sender: Any?) { captureMenuBinding(.select_tab_1) } - @objc func menuSelectTab2(_ sender: Any?) { captureMenuBinding(.select_tab_2) } - @objc func menuSelectTab3(_ sender: Any?) { captureMenuBinding(.select_tab_3) } - @objc func menuSelectTab4(_ sender: Any?) { captureMenuBinding(.select_tab_4) } - @objc func menuSelectTab5(_ sender: Any?) { captureMenuBinding(.select_tab_5) } - @objc func menuSelectTab6(_ sender: Any?) { captureMenuBinding(.select_tab_6) } - @objc func menuSelectTab7(_ sender: Any?) { captureMenuBinding(.select_tab_7) } - @objc func menuSelectTab8(_ sender: Any?) { captureMenuBinding(.select_tab_8) } - @objc func menuSelectTab9(_ sender: Any?) { captureMenuBinding(.select_tab_9) } - @objc func menuBrowseHosts(_ sender: Any?) { captureMenuBinding(.browse_hosts) } - @objc func menuBrowseProfiles(_ sender: Any?) { captureMenuBinding(.browse_profiles) } - @objc func menuToggleAIAgent(_ sender: Any?) { captureMenuBinding(.toggle_ai_agent) } - @objc func menuToggleVoiceAgent(_ sender: Any?) { captureMenuBinding(.toggle_voice_agent) } - @objc func menuOpenSettings(_ sender: Any?) { captureMenuBinding(.open_settings) } - @objc func menuShowTmuxSessions(_ sender: Any?) { captureMenuBinding(.show_tmux_sessions) } - @objc func menuDetachSession(_ sender: Any?) { captureMenuBinding(.detach_session) } - @objc func menuDetachAllSessions(_ sender: Any?) { captureMenuBinding(.detach_all_sessions) } - @objc func menuDetachOtherClients(_ sender: Any?) { captureMenuBinding(.detach_other_clients) } - @objc func menuToggleTransparency(_ sender: Any?) { captureMenuBinding(.toggle_transparency) } - @objc func menuToggleTitleBar(_ sender: Any?) { captureMenuBinding(.toggle_titlebar) } - @objc func menuToggleAutoRedact(_ sender: Any?) { captureMenuBinding(.toggle_auto_redact) } - @objc func menuToggleBackgroundEffect(_ sender: Any?) { captureMenuBinding(.toggle_background_effect) } - @objc func menuToggleFullScreen(_ sender: Any?) { captureMenuBinding(.toggle_full_screen) } - @objc func menuToggleCompose(_ sender: Any?) { captureMenuBinding(.toggle_compose) } - @objc func menuToggleMouseCapture(_ sender: Any?) { captureMenuBinding(.toggle_mouse_capture) } - @objc func menuToggleClipboardManager(_ sender: Any?) { captureMenuBinding(.toggle_clipboard_manager) } - @objc func menuToggleThemePicker(_ sender: Any?) { captureMenuBinding(.toggle_theme_picker) } - @objc func menuToggleQuickSettings(_ sender: Any?) { captureMenuBinding(.toggle_quick_settings) } - @objc func menuScrollPageUp(_ sender: Any?) { captureMenuBinding(.scroll_page_up) } - @objc func menuScrollPageDown(_ sender: Any?) { captureMenuBinding(.scroll_page_down) } - @objc func menuScrollToTop(_ sender: Any?) { captureMenuBinding(.scroll_to_top) } - @objc func menuScrollToBottom(_ sender: Any?) { captureMenuBinding(.scroll_to_bottom) } - @objc func menuBrightnessBoost(_ sender: Any?) { captureMenuBinding(.brightness_boost) } - @objc func menuCycleInputSource(_ sender: Any?) { captureMenuBinding(.cycle_input_source) } - @objc func menuPreviousGroup(_ sender: Any?) { captureMenuBinding(.previous_group) } - @objc func menuNextGroup(_ sender: Any?) { captureMenuBinding(.next_group) } - - /// Record the chord that just fired a menu item. Use the bound key plus - /// modifiers that are physically held so Shift+⌘T records as Shift+⌘T - /// even when the menu item itself is ⌘T. - private func captureMenuBinding(_ action: KeybindAction) { - guard let binding = KeybindManager.shared.keybind(for: action), - let first = binding.sequence.first, - !binding.sequence.isSequence - else { return } - - var modifiers = first.modifiers - let hardware = KeybindModifiers(uiModifierFlags: KeyboardTracker.shared.hardwareModifierFlags) - modifiers.formUnion(hardware) - processCapture(trigger: KeyTrigger(key: first.key, modifiers: modifiers)) + 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?) { From ea39a045250f4a2121735ce2245f98d9012bd613 Mon Sep 17 00:00:00 2001 From: Joshua Van Deren Date: Sat, 12 Sep 2026 23:27:44 -0600 Subject: [PATCH 3/5] Unbind the previous owner when a shortcut is taken. Stealing a chord used to leave a stale remap or wipe the new binding; the old action now stays unbound and the new one keeps the shortcut. Co-authored-by: Cursor --- rootshell/Core/Keybinds/KeybindManager.swift | 44 ++++++++++++++++++-- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/rootshell/Core/Keybinds/KeybindManager.swift b/rootshell/Core/Keybinds/KeybindManager.swift index 5be04a035..d90a9da27 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) + 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,28 @@ 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 where !victim.action.isParameterized { + userOverrides.removeAll { $0.action == victim.action } + userOverrides.removeAll { + $0.action == .unbind && $0.actionParameter == victim.action.rawValue + } + userOverrides.append( + Keybind( + sequence: victim.sequence, + action: .unbind, + actionParameter: victim.action.rawValue, + isUserOverride: true, + source: .userOverride + ) + ) + } + } + + // New binding last so it wins over any victim unbind for this sequence. let override = Keybind( sequence: sequence, action: action, @@ -644,6 +670,18 @@ final class KeybindManager: ObservableObject { // Apply user overrides (highest priority) for override in userOverrides { + if override.action == .unbind { + // Unbind targets the action in `actionParameter`, not the + // sequence. Clearing the sequence here would also strip a + // newer override that just took that chord. + if let raw = override.actionParameter, + let unbound = KeybindAction(rawValue: raw), + !unbound.isParameterized { + bindings.removeAll { $0.action == unbound } + } + continue + } + // Remove any existing binding for this action (skip parameterized) if !override.action.isParameterized { bindings.removeAll { $0.action == override.action } @@ -651,9 +689,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 From f4b8d80ab3d1bb73fb1f789bfb498fe40ca4d9f0 Mon Sep 17 00:00:00 2001 From: Joshua Van Deren Date: Sun, 13 Sep 2026 11:12:45 -0600 Subject: [PATCH 4/5] Keep KeybindEditorView compatible with profile shortcuts on main. The editor API on main already takes a profile parameter and a single-argument outcome; restore those so this branch builds without the mux PRs. --- rootshell/App/AppCommands.swift | 22 ++--- .../Features/Visor/iPadVisorSettings.swift | 2 +- .../Settings/Keyboard/KeybindEditorView.swift | 93 +++++++++++++++---- .../KeyboardShortcutsSettingsView.swift | 10 +- 4 files changed, 93 insertions(+), 34 deletions(-) diff --git a/rootshell/App/AppCommands.swift b/rootshell/App/AppCommands.swift index 72da9c2e0..2942a6023 100644 --- a/rootshell/App/AppCommands.swift +++ b/rootshell/App/AppCommands.swift @@ -584,6 +584,14 @@ struct ShellCommands: Commands { ) } .modifier(DynamicShortcut(action: .open_settings, shortcuts: shortcutState.shortcuts)) + + Button("Quick Settings…") { + UIApplication.shared.sendAction( + #selector(UIApplication.menuToggleQuickSettings(_:)), + to: nil, from: nil, for: nil + ) + } + .modifier(DynamicShortcut(action: .toggle_quick_settings, shortcuts: shortcutState.shortcuts)) } } } @@ -645,21 +653,13 @@ struct WindowCommands: Commands { } .modifier(DynamicShortcut(action: .show_tmux_sessions, shortcuts: shortcutState.shortcuts)) - Button("Detach Session") { - UIApplication.shared.sendAction( - #selector(Ghostty.TerminalView.menuDetachSession(_:)), - to: nil, from: nil, for: nil - ) - } - .modifier(DynamicShortcut(action: .detach_session, shortcuts: shortcutState.shortcuts)) - - Button("Detach All Sessions") { + Button("Discover Sessions") { UIApplication.shared.sendAction( - #selector(Ghostty.TerminalView.menuDetachAllSessions(_:)), + #selector(Ghostty.TerminalView.menuDiscoverSessions(_:)), to: nil, from: nil, for: nil ) } - .modifier(DynamicShortcut(action: .detach_all_sessions, shortcuts: shortcutState.shortcuts)) + .modifier(DynamicShortcut(action: .discover_sessions, shortcuts: shortcutState.shortcuts)) Button("Detach Other Clients") { UIApplication.shared.sendAction( diff --git a/rootshell/Features/Visor/iPadVisorSettings.swift b/rootshell/Features/Visor/iPadVisorSettings.swift index f9df05fb8..b9f69e8dd 100644 --- a/rootshell/Features/Visor/iPadVisorSettings.swift +++ b/rootshell/Features/Visor/iPadVisorSettings.swift @@ -87,7 +87,7 @@ struct iPadVisorSettingsView: View { .themedList() .navigationTitle("Visor") .sheet(isPresented: $editing, onDismiss: applyOutcome) { - KeybindEditorView(action: .toggle_visor) { _, result in outcome = result } + KeybindEditorView(action: .toggle_visor) { outcome = $0 } .themedSubSheet(sheetThemeColors) } } diff --git a/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift b/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift index bd2083122..4a2bc1542 100644 --- a/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift +++ b/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift @@ -23,12 +23,22 @@ 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 + /// Optional title override (e.g. profile name). Falls back to `action.displayName`. + var titleOverride: String? = nil + /// When false, hide "Restore Default" (used for profile shortcuts whose default is none). + var allowsRestoreDefault: Bool = true + /// When non-nil, display this sequence instead of looking up the live KeybindManager + /// binding. Lets parents (e.g. profile editor) keep a draft until Save. + var draftSequence: KeySequence?? = nil /// Reports the user's choice to the parent. All paths that mutate /// `KeybindManager` route through this callback so the actual write - /// happens in the parent's sheet-onDismiss closure. Includes the action - /// currently on screen, which may have changed if the user jumped to a - /// conflicting shortcut without dismissing the sheet. - var onOutcome: (KeybindAction, KeybindEditorOutcome) -> Void = { _, _ in } + /// happens in the parent's sheet-onDismiss closure. + var onOutcome: (KeybindEditorOutcome) -> Void = { _ in } /// Optional: parent can follow an in-sheet jump to another action (e.g. to /// keep the shortcuts list on the matching category). The sheet stays open. var onSwitchAction: ((KeybindAction) -> Void)? @@ -42,22 +52,56 @@ struct KeybindEditorView: View { init( action: KeybindAction, - onOutcome: @escaping (KeybindAction, KeybindEditorOutcome) -> Void = { _, _ in }, + 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 binding: Keybind? { - keybindManager.keybind(for: currentAction) + private var managerBinding: Keybind? { + if let actionParameter { + keybindManager.keybind(for: currentAction, parameter: actionParameter) + } else { + keybindManager.keybind(for: currentAction) + } + } + + /// Sequence shown in the "Current Shortcut" section + private var displayedSequence: KeySequence? { + if currentAction == action, let draftSequence { + return draftSequence + } + return managerBinding?.sequence + } + + private var showsCustomBadge: Bool { + (currentAction != action || draftSequence == nil) && managerBinding?.isUserOverride == true + } + + private var displayTitle: String { + 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 conflictingBindings.count == 1, + guard actionParameter == nil, + conflictingBindings.count == 1, let conflict = conflictingBindings.first?.action, conflict != currentAction, KeybindAction.customizableActions.contains(conflict) @@ -99,7 +143,7 @@ struct KeybindEditorView: View { VStack(spacing: 24) { // Action info VStack(spacing: 8) { - Text(currentAction.displayName) + Text(displayTitle) .font(.title2) .fontWeight(.semibold) @@ -121,15 +165,15 @@ struct KeybindEditorView: View { .font(.headline) .foregroundColor(.secondary) - if let binding { - Text(binding.sequence.symbolDescription) + if let displayedSequence { + Text(displayedSequence.symbolDescription) .font(.system(size: 28, weight: .medium, design: .monospaced)) .padding(.horizontal, 24) .padding(.vertical, 16) .background(rowBackground) .cornerRadius(12) - if binding.isUserOverride { + if showsCustomBadge { Label("Custom", systemImage: "star.fill") .font(.caption) .foregroundStyle(.tint) @@ -146,17 +190,20 @@ struct KeybindEditorView: View { } if conflictingBindings.isEmpty { - if (binding != nil && binding!.isUserOverride) || keybindManager.isActionUnbound(currentAction) { + if allowsRestoreDefault, + (currentAction != action || draftSequence == nil), + (managerBinding != nil && managerBinding!.isUserOverride) + || keybindManager.isActionUnbound(currentAction) { Button("Restore Default") { - onOutcome(currentAction, .restoreDefault) + onOutcome(.restoreDefault) dismiss() } .foregroundColor(.orange) } - if binding != nil { - Button("Unbind Shortcut") { - onOutcome(currentAction, .unbind) + if displayedSequence != nil { + Button(allowsRestoreDefault ? "Unbind Shortcut" : "Clear Shortcut") { + onOutcome(.unbind) dismiss() } .foregroundColor(.red) @@ -307,7 +354,15 @@ struct KeybindEditorView: View { return } - let conflicts = keybindManager.conflicts(for: sequence, excluding: currentAction) + let conflicts = keybindManager.conflicts( + for: sequence, + excluding: actionParameter == nil ? currentAction : nil + ).filter { binding in + if let actionParameter { + return !(binding.action == currentAction && binding.actionParameter == actionParameter) + } + return true + } isCapturing = false showSequenceCapture = false @@ -348,7 +403,7 @@ struct KeybindEditorView: View { // onDismiss closure — i.e. after the sheet has fully dismissed — so the // @Published cascade in setOverride runs in a quiescent view hierarchy // rather than mid-dismissal. - onOutcome(currentAction, .captured(sequence)) + onOutcome(.captured(sequence)) dismiss() } } diff --git a/rootshell/UI/Settings/Keyboard/KeyboardShortcutsSettingsView.swift b/rootshell/UI/Settings/Keyboard/KeyboardShortcutsSettingsView.swift index 499a51704..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 @@ -91,14 +94,15 @@ struct KeyboardShortcutsSettingsView: View { ) { action in KeybindEditorView( action: action, - onOutcome: { editedAction, outcome in - pendingOutcome = (editedAction, outcome) + onOutcome: { outcome in + pendingOutcome = (editorAction ?? action, outcome) }, onSwitchAction: { conflict in - // Stay in the same sheet; just keep the list behind it in sync. + editorAction = conflict selectedCategory = conflict.category } ) + .onAppear { editorAction = action } .themedSubSheet(sheetThemeColors) } .sheet(isPresented: $showConfigEditor) { From 66a5e27239cd1d0d2f7426f9389703ace0df5905 Mon Sep 17 00:00:00 2001 From: Kit Knox Date: Sun, 13 Sep 2026 11:11:32 -0700 Subject: [PATCH 5/5] Fix shortcut conflict handling for profiles and action switching Require switch-aware parents for in-sheet action changes. Check new profile shortcuts against existing profiles and persist targeted unbinds for displaced parameterized bindings. --- rootshell/Core/Keybinds/Keybind.swift | 20 ++++++++ rootshell/Core/Keybinds/KeybindManager.swift | 50 ++++++++++--------- .../Settings/Keyboard/KeybindEditorView.swift | 19 ++++--- 3 files changed, 55 insertions(+), 34 deletions(-) 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 d90a9da27..873543a09 100644 --- a/rootshell/Core/Keybinds/KeybindManager.swift +++ b/rootshell/Core/Keybinds/KeybindManager.swift @@ -280,7 +280,7 @@ final class KeybindManager: ObservableObject { // Snapshot who we are about to displace, before userOverrides change. let victims = action == .unbind ? [] - : conflicts(for: sequence, excluding: action) + : conflicts(for: sequence, excluding: action, excludingParameter: parameter) if action == .unbind { // Unbind is special: multiple actions can be unbound simultaneously. @@ -307,20 +307,19 @@ final class KeybindManager: ObservableObject { // 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 where !victim.action.isParameterized { - userOverrides.removeAll { $0.action == victim.action } + 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 { - $0.action == .unbind && $0.actionParameter == victim.action.rawValue + unbind.unbinds($0) || $0.unbinds(victim) } - userOverrides.append( - Keybind( - sequence: victim.sequence, - action: .unbind, - actionParameter: victim.action.rawValue, - isUserOverride: true, - source: .userOverride - ) - ) + userOverrides.append(unbind) } } @@ -671,14 +670,10 @@ final class KeybindManager: ObservableObject { // Apply user overrides (highest priority) for override in userOverrides { if override.action == .unbind { - // Unbind targets the action in `actionParameter`, not the - // sequence. Clearing the sequence here would also strip a - // newer override that just took that chord. - if let raw = override.actionParameter, - let unbound = KeybindAction(rawValue: raw), - !unbound.isParameterized { - bindings.removeAll { $0.action == unbound } - } + // 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 } @@ -834,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 4a2bc1542..8f7f09013 100644 --- a/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift +++ b/rootshell/UI/Settings/Keyboard/KeybindEditorView.swift @@ -39,8 +39,9 @@ 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 } - /// Optional: parent can follow an in-sheet jump to another action (e.g. to - /// keep the shortcuts list on the matching category). The sheet stays open. + /// 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 @@ -100,7 +101,9 @@ struct KeybindEditorView: View { /// 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 actionParameter == nil, + guard onSwitchAction != nil, + !action.isParameterized, + actionParameter == nil, conflictingBindings.count == 1, let conflict = conflictingBindings.first?.action, conflict != currentAction, @@ -356,13 +359,9 @@ struct KeybindEditorView: View { let conflicts = keybindManager.conflicts( for: sequence, - excluding: actionParameter == nil ? currentAction : nil - ).filter { binding in - if let actionParameter { - return !(binding.action == currentAction && binding.actionParameter == actionParameter) - } - return true - } + excluding: currentAction, + excludingParameter: actionParameter + ) isCapturing = false showSequenceCapture = false