Skip to content
Closed
4 changes: 2 additions & 2 deletions .github/swift-file-length-budget.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
4483 Sources/cmuxApp.swift
4482 Sources/Panels/FilePreviewPanel.swift
4367 cmuxTests/BrowserPanelTests.swift
4283 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
4121 Sources/BrowserWindowPortal.swift
3964 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
3934 Sources/Feed/FeedPanelView.swift
3926 cmuxTests/TabManagerUnitTests.swift
3896 cmuxTests/WindowAndDragTests.swift
Expand Down Expand Up @@ -173,11 +173,11 @@
637 Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileShellRenderGridLivenessTestSupport.swift
635 cmuxUITests/RightSidebarChromeHeightUITests.swift
630 Packages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutWhenClause.swift
620 Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/KeyboardShortcutsSection.swift
620 cmuxTests/FinderFileDropRegressionTests.swift
620 cmuxTests/TerminalNotificationQueueTests.swift
617 Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileShellRenderGridLivenessTests.swift
617 Sources/SettingsNavigation.swift
610 Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/KeyboardShortcutsSection.swift
608 cmuxUITests/FeedSidebarUITests.swift
607 Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface.swift
607 Sources/SessionIndexModels.swift
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,12 @@ public struct KeyboardShortcutsSection: View {
/// `shortcuts.when` override when one is configured, otherwise a localized
/// description of the action's built-in focus scope. `nil` for ordinary
/// always-on shortcuts so unscoped rows stay uncluttered.
///
/// The built-in scope mapping lives in ``ShortcutScopeCaptionResolver`` so it
/// can be unit-tested directly; it labels context-scoped duplicate defaults
/// (e.g. the ⌘=/⌘-/⌘0 zoom family shared across browser, markdown, and canvas)
/// so they read as deliberately context-separated rather than as a conflict
/// (issue #5810).
private func scopeCaption(for action: ShortcutAction) -> String? {
if let overrideClause = whenOverrideClauses[action.rawValue] {
// An explicit empty/`true` override means "no restriction" — show
Expand All @@ -327,23 +333,7 @@ public struct KeyboardShortcutsSection: View {
let format = String(localized: "shortcut.when.caption.override", defaultValue: "When: %@")
return String.localizedStringWithFormat(format, raw)
}
switch action.defaultFocusWhenClause {
case .always:
return nil
case .atom(.sidebarFocus):
return String(localized: "shortcut.when.caption.sidebarFocus", defaultValue: "Only while the right sidebar is focused")
case .atom(.browserFocus):
return String(localized: "shortcut.when.caption.browserFocus", defaultValue: "Only while a browser pane is focused")
case .atom(.filePreviewTextEditorFocus):
return String(localized: "shortcut.when.caption.filePreviewTextEditorFocus", defaultValue: "Only while a text file preview is focused")
case .or(.atom(.browserFocus), .atom(.filePreviewTextEditorFocus)),
.or(.atom(.filePreviewTextEditorFocus), .atom(.browserFocus)):
return String(localized: "shortcut.when.caption.browserOrFilePreviewTextEditorFocus", defaultValue: "Only while a browser pane or text file preview is focused")
case .atom(.markdownFocus):
return String(localized: "shortcut.when.caption.markdownFocus", defaultValue: "Only while a markdown preview is focused")
default:
return String(localized: "shortcut.when.caption.terminalFocus", defaultValue: "Only while a terminal pane is focused")
}
return ShortcutScopeCaptionResolver().caption(for: action.defaultFocusWhenClause)
}

private func detectConflict(for action: ShortcutAction, stroke: StoredShortcut) -> ShortcutAction? {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import CmuxSettings
import Foundation

/// Resolves localized captions for built-in keyboard shortcut focus scopes.
struct ShortcutScopeCaptionResolver {
/// Returns a caption for the built-in focus/layout scope represented by `clause`.
func caption(for clause: ShortcutWhenClause) -> String? {
let canvasLayoutKey = ShortcutContextKnownKey.workspaceCanvasLayout.rawValue
switch clause {
case .always:
return nil
case .atom(.sidebarFocus):
return String(
localized: "shortcut.when.caption.sidebarFocus",
defaultValue: "Only while the right sidebar is focused"
)
case .atom(.browserFocus):
return String(
localized: "shortcut.when.caption.browserFocus",
defaultValue: "Only while a browser pane is focused"
)
case .atom(.filePreviewTextEditorFocus):
return String(
localized: "shortcut.when.caption.filePreviewTextEditorFocus",
defaultValue: "Only while a text file preview is focused"
)
case .or(.atom(.browserFocus), .atom(.filePreviewTextEditorFocus)),
.or(.atom(.filePreviewTextEditorFocus), .atom(.browserFocus)):
return String(
localized: "shortcut.when.caption.browserOrFilePreviewTextEditorFocus",
defaultValue: "Only while a browser pane or text file preview is focused"
)
case .atom(.markdownFocus):
return String(
localized: "shortcut.when.caption.markdownFocus",
defaultValue: "Only while a markdown preview is focused"
)
case let other where clauseRequiresKey(other, key: canvasLayoutKey):
// Any clause that *requires* the canvas-layout key is scoped to the
// canvas layout — both the plain `Canvas: …` actions (`canvas`) and
// `Canvas: Actual Size`, whose `canvas && !browser && !markdown` clause
// keeps its ⌘0 default off the browser/markdown zoom-reset bindings.
// Matching the key as a required conjunct (rather than positionally)
// avoids re-introducing the very mislabel this caption fixes if a future
// clause places the canvas predicate on the other side of an `&&`.
return canvasLayoutCaption()
default:
// The remaining built-in scope is the terminal-pane predicate used by
// Rename Tab/Workspace, Send Ctrl-F, and Clear Screen
// (`!browser && !sidebar`).
return String(
localized: "shortcut.when.caption.terminalFocus",
defaultValue: "Only while a terminal pane is focused"
)
}
}

/// Whether `clause` requires `key` to be true as a top-level `&&` conjunct.
private func clauseRequiresKey(_ clause: ShortcutWhenClause, key: String) -> Bool {
switch clause {
case let .key(name):
return name == key
case let .and(lhs, rhs):
return clauseRequiresKey(lhs, key: key) || clauseRequiresKey(rhs, key: key)
default:
return false
}
}

private func canvasLayoutCaption() -> String {
String(
localized: "shortcut.when.caption.canvasLayout",
defaultValue: "Only while the canvas layout is active"
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
import Testing
import CmuxSettings
@testable import CmuxSettingsUI

/// Coverage for the Settings → Keyboard Shortcuts scope caption that labels
/// context-scoped duplicate default shortcuts (issue #5810).
///
/// Several actions intentionally ship the *same* default chord, disambiguated
/// only by focus/layout context — most visibly the zoom family: `⌘=` / `⌘-`
/// across Zoom In/Out (browser) and Markdown Viewer: Zoom In/Out, and `⌘0`
/// across the browser, markdown, and canvas "Actual Size" actions. Each row
/// must state the scope its binding fires in so the list does not read as a raw
/// duplicate-default collision.
@Suite("Keyboard shortcut scope captions")
struct ShortcutScopeCaptionTests {
// Package tests resolve `String(localized:)` to the `defaultValue` because
// the app's `Localizable.xcstrings` is not bundled with the package test
// target, so the English defaults are the source of truth here.
private let browserCaption = "Only while a browser pane is focused"
private let filePreviewTextEditorCaption = "Only while a text file preview is focused"
private let browserOrFilePreviewTextEditorCaption = "Only while a browser pane or text file preview is focused"
private let markdownCaption = "Only while a markdown preview is focused"
private let sidebarCaption = "Only while the right sidebar is focused"
private let terminalCaption = "Only while a terminal pane is focused"
private let canvasCaption = "Only while the canvas layout is active"

private let resolver = ShortcutScopeCaptionResolver()
private var canvasLayoutKey: String { ShortcutContextKnownKey.workspaceCanvasLayout.rawValue }

@Test func alwaysClauseHasNoCaption() {
#expect(resolver.caption(for: .always) == nil)
}

@Test func focusAtomClausesMapToTheirCaption() {
#expect(resolver.caption(for: .atom(.sidebarFocus)) == sidebarCaption)
#expect(resolver.caption(for: .atom(.browserFocus)) == browserCaption)
#expect(resolver.caption(for: .atom(.filePreviewTextEditorFocus)) == filePreviewTextEditorCaption)
#expect(resolver.caption(for: .atom(.markdownFocus)) == markdownCaption)
}

@Test func browserOrFilePreviewTextEditorClauseMapsToCombinedCaption() {
let browserFirst = ShortcutWhenClause.or(.atom(.browserFocus), .atom(.filePreviewTextEditorFocus))
let filePreviewFirst = ShortcutWhenClause.or(.atom(.filePreviewTextEditorFocus), .atom(.browserFocus))

#expect(resolver.caption(for: browserFirst) == browserOrFilePreviewTextEditorCaption)
#expect(resolver.caption(for: filePreviewFirst) == browserOrFilePreviewTextEditorCaption)
}

@Test func terminalPredicateMapsToTerminalCaption() {
// Rename Tab/Workspace, Send Ctrl-F, Clear Screen gate on
// `!browser && !sidebar`.
let clause = ShortcutWhenClause.and(.not(.atom(.browserFocus)), .not(.atom(.sidebarFocus)))
#expect(resolver.caption(for: clause) == terminalCaption)
}

@Test func canvasLayoutClausesMapToCanvasCaption() {
// Plain canvas-layout actions (Canvas: Zoom In/Out, Tidy, Align, …).
#expect(resolver.caption(for: .key(canvasLayoutKey)) == canvasCaption)
// `Canvas: Actual Size` gates on `canvas && !browser && !markdown` so it
// never collides with the browser/markdown ⌘0 zoom-reset bindings.
let compound = ShortcutWhenClause.and(
.key(canvasLayoutKey),
.and(.not(.atom(.browserFocus)), .not(.atom(.markdownFocus)))
)
#expect(resolver.caption(for: compound) == canvasCaption)
}

/// The canvas key must be recognized as a required `&&` conjunct regardless
/// of which side of the `.and` it sits on, so a future clause structure can't
/// silently fall back to the terminal caption (the bug this PR fixes).
@Test func canvasLayoutKeyIsMatchedRegardlessOfConjunctPosition() {
let rightPlaced = ShortcutWhenClause.and(.not(.atom(.browserFocus)), .key(canvasLayoutKey))
#expect(resolver.caption(for: rightPlaced) == canvasCaption)

let nested = ShortcutWhenClause.and(
.not(.atom(.markdownFocus)),
.and(.not(.atom(.browserFocus)), .key(canvasLayoutKey))
)
#expect(resolver.caption(for: nested) == canvasCaption)

// A *negated* canvas predicate is not a canvas requirement, so it must
// not borrow the canvas caption.
let negated = ShortcutWhenClause.and(.not(.key(canvasLayoutKey)), .not(.atom(.sidebarFocus)))
#expect(resolver.caption(for: negated) == terminalCaption)
}

/// Regression for issue #5810: the ⌘= / ⌘- / ⌘0 zoom shortcuts ship the same
/// default across browser, markdown, and (for ⌘0) canvas. Each must carry the
/// caption for its own scope. In particular `canvasZoomReset` must read as the
/// canvas layout, not be mislabeled "Only while a terminal pane is focused".
@Test func zoomFamilyActionsCarryTheirOwnScopeCaption() {
for action in [ShortcutAction.browserZoomIn, .browserZoomOut, .browserZoomReset] {
#expect(resolver.caption(for: action.defaultFocusWhenClause) == browserOrFilePreviewTextEditorCaption)
}
for action in [ShortcutAction.markdownZoomIn, .markdownZoomOut, .markdownZoomReset] {
#expect(resolver.caption(for: action.defaultFocusWhenClause) == markdownCaption)
}
for action in [ShortcutAction.canvasZoomIn, .canvasZoomOut, .canvasZoomReset] {
#expect(resolver.caption(for: action.defaultFocusWhenClause) == canvasCaption)
}
// The specific mislabel this issue fixes: canvas must not fall through to
// the terminal caption.
#expect(resolver.caption(for: ShortcutAction.canvasZoomReset.defaultFocusWhenClause) != terminalCaption)
}

/// The duplicate default chords are intentional: each pair shares one
/// keystroke yet has context-disjoint effective clauses, so the runtime and
/// the recorder let them coexist. That coexistence is exactly why the scope
/// caption is required to disambiguate them in Settings.
@Test func zoomDuplicateDefaultsShareAStrokeButCoexist() throws {
func assertCoexisting(_ lhs: ShortcutAction, _ rhs: ShortcutAction) throws {
let lhsShortcut = try #require(lhs.defaultShortcut)
let rhsShortcut = try #require(rhs.defaultShortcut)
#expect(!lhsShortcut.isUnbound)
#expect(!rhsShortcut.isUnbound)
// Same physical keystroke…
#expect(numberedAwareStrokesConflict(
lhsShortcut.first, numbered: lhs.usesNumberedDigitMatching,
rhsShortcut.first, numbered: rhs.usesNumberedDigitMatching
))
// …but context-disjoint, so they are not a real collision.
#expect(!ShortcutWhenClause.bindingsCollide(
lhs.defaultFocusWhenClause, lhsHasPriority: lhs.hasPriorityShortcutRouting,
rhs.defaultFocusWhenClause, rhsHasPriority: rhs.hasPriorityShortcutRouting
))
}
try assertCoexisting(.browserZoomIn, .markdownZoomIn) // ⌘=
try assertCoexisting(.browserZoomOut, .markdownZoomOut) // ⌘-
try assertCoexisting(.browserZoomReset, .markdownZoomReset) // ⌘0
try assertCoexisting(.browserZoomReset, .canvasZoomReset) // ⌘0
try assertCoexisting(.markdownZoomReset, .canvasZoomReset) // ⌘0
}
}
Loading
Loading