Label canvas-layout shortcuts in Settings so the ⌘=/⌘-/⌘0 zoom duplicates read as context-scoped (#5810)

[austinywang]

Fixes #5810

Problem

Issue #5810 reports that Zoom In/Out (⌘= / ⌘-) and Markdown Viewer: Zoom In/Out ship the same default chords, so Settings → Keyboard Shortcuts looks like it has duplicate-default collisions. The endorsed resolutions are either to re-scope the defaults or to label the context-scoped duplicates in the UI.

These zoom bindings are intentionally context-scoped — ⌘= zooms the browser when a browser pane is focused and the markdown viewer when a markdown preview is focused; they never both fire — so unbinding would regress the feature. The right fix is the UI labeling.

That labeling already exists for most cases: #5196 added a per-row scope caption ("Only while a browser pane is focused" / "Only while a markdown preview is focused" / "…right sidebar…"), and #5394 fixed the browser/markdown non‑US‑layout zoom path. But there's a remaining gap in the same mechanism:

• The ⌘0 "Actual Size" trio ships on the browser, the markdown viewer, and the canvas (Canvas: Actual Size). The canvas member — like every canvas-layout-scoped shortcut — fell through to the catch-all and was mislabeled "Only while a terminal pane is focused", which is wrong and undercuts the very indication this issue asks for.

Fix

• Extract the built-in scope→caption mapping out of KeyboardShortcutsSection.scopeCaption into a testable builtInScopeCaption(for:) seam.
• Map the workspaceCanvasLayout clause (and the compound canvas && !browser && !markdown clause used by Canvas: Actual Size) to a new, accurate "Only while the canvas layout is active" caption.

After this, the whole ⌘= / ⌘- / ⌘0 zoom-shortcut family is accurately scoped in Settings: browser, markdown, and canvas each read with their own context instead of looking like a raw collision.

Tests (two-commit red → green)

ShortcutScopeCaptionTests (package test target, auto-discovered by SwiftPM):

• Commit 1 adds the failing regression: canvas-scoped bindings (incl. Canvas: Actual Size, ⌘0) assert a canvas caption while the code still returns the terminal caption → red.
• Commit 2 adds the canvas mapping + localization → green.

Coverage asserts: the clause→caption mapping for every built-in scope; each zoom action's caption (browser/markdown/canvas) including the explicit "canvas must not be the terminal caption" regression; and that the ⌘=/⌘-/⌘0 duplicate defaults share a keystroke yet are context-disjoint (bindingsCollide == false), which is exactly why the scope caption is required.

Scope notes

• Problem 2 (zoom-in unreachable on non-US layouts) is tracked elsewhere: Fix markdown/browser Cmd zoom on non-US keyboard layouts #5394 (merged) fixed the browser/markdown path; Fix terminal zoom on non-US ISO keyboard layouts #4302 (open) fixes the terminal path. This PR does not touch keystroke translation.
• No behavior change for browser/markdown/sidebar/terminal captions — only canvas was mislabeled.

Localization audit

Added one new user-facing string, shortcut.when.caption.canvasLayout, to Resources/Localizable.xcstrings with translations for all 20 supported locales (verified at locale parity with the sibling shortcut.when.caption.terminalFocus key). No web/ or docs message catalogs are involved. No new bare English literals — all captions go through String(localized:).

---

Summary by cubic

Label canvas-layout shortcuts in Settings with “Only while the canvas layout is active” so the ⌘=/⌘-/⌘0 zoom defaults read as context-scoped, not duplicates. Fixes #5810; detection now handles the canvas key regardless of clause order.

• Bug Fixes

  • Map workspaceCanvasLayout and any clause that requires that key (at any && depth) to the new caption; negated or or cases are excluded to avoid mislabels.
  • Applies to canvas zoom and “Actual Size” (⌘0); browser/markdown captions unchanged.
  • Added shortcut.when.caption.canvasLayout in Localizable.xcstrings with translations for all supported locales.

• Refactors

  • Moved built-in scope→caption logic into ShortcutScopeCaptionResolver and used it in KeyboardShortcutsSection.scopeCaption.
  • Added ShortcutScopeCaptionTests; covers caption mapping (incl. the combined browser-or-text-preview caption), zoom defaults’ context-disjointness, and right-placed/nested/negated canvas conjuncts.

^{Written for commit e24bfe2. Summary will update on new commits.}

Summary by CodeRabbit

• New Features

  • Added clearer keyboard shortcut scope captions across more contexts, including canvas layout and combined browser/file preview scopes.
  • Localized the new canvas-layout caption for multiple languages.

• Bug Fixes

  • Improved scope labeling for shortcuts so zoom and reset actions now show the correct caption for their active context.
  • Fixed canvas-related shortcuts to display the right scope even when conditions are combined in different ways.

• Tests

  • Added coverage for shortcut scope caption behavior and several regression cases.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	Jul 5, 2026 12:38am
cmux-staging	Building	Preview, Comment	Jul 5, 2026 12:38am

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors keyboard shortcut "when" scope caption logic by extracting it into a new centralized builtInScopeCaption(for:) function with helper logic for canvas-layout detection, wires it into the existing settings UI, adds a new test suite, and adds a supporting localization string.

Changes

Scope caption centralization

Layer / File(s)	Summary
Built-in scope caption function and helpers Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/ShortcutScopeCaption.swift, Resources/Localizable.xcstrings	New file introduces builtInScopeCaption(for:) mapping focus/layout clauses to localized captions, plus clauseRequiresKey and canvasLayoutCaption helpers, and a new shortcut.when.caption.canvasLayout localization entry across many locales.
Wire caption function into KeyboardShortcutsSection Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/KeyboardShortcutsSection.swift	scopeCaption(for:) now delegates to builtInScopeCaption(for:) instead of an inline switch, with updated documentation clarifying the centralized mapping and context-separated duplicate default behavior.
Scope caption test coverage Packages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/ShortcutScopeCaptionTests.swift	New test suite validates caption generation for always/atom/OR/canvas-layout clauses, zoom-family regression captions, and duplicate default chord coexistence via bindingsCollide.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related PRs

• manaflow-ai/cmux#7157: Extends builtInScopeCaption(for:) for new focus clause combinations introduced alongside zoom-shortcut changes.

Suggested reviewers: azooz2003-bit

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
Cmux No Ambient Global State	❌ Error	Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/ShortcutScopeCaption.swift:24 adds a cross-file top-level helper, which the rule forbids.	Move the scope-caption logic onto ShortcutWhenClause (e.g. an extension method/property) or another owning type, and call that from KeyboardShortcutsSection.

✅ Passed checks (24 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation	✅ Passed	PASS: The PR only adds a pure caption helper and tests; the modified UI type remains @MainActor, and no Sendable/reference-store isolation changes were introduced.
Cmux Swift Blocking Runtime	✅ Passed	Touched Swift files add pure caption mapping/tests only; no semaphores, sleeps, sync waits, delayed dispatch, polling, or locks were introduced.
Cmux Browser Automation Off-Main	✅ Passed	Commit only changes CmuxSettingsUI tests; no browser automation files or browser.* routing changed, so the off-main WebKit policy isn't implicated.
Cmux Expensive Synchronous Load	✅ Passed	Diff only refactors keyboard-shortcut scope captions, adds tests/localization, and contains no agent-history loads, JSON scans, or main-actor interactive file parsing.
Cmux Cache Substitution Correctness	✅ Passed	Only transient UI scope-caption rendering changed; no persistence/history/undo/snapshot path or fresh-vs-cache substitution was introduced.
Cmux No Hacky Sleeps	✅ Passed	PR only changes Swift, tests, localization, and a TSV; no non-Swift runtime code uses sleeps/timers/polling.
Cmux Algorithmic Complexity	✅ Passed	PASS: The new caption logic only switches over a small enum and recursively checks a tiny ShortcutWhenClause tree; no scalable collection scans or repeated rescans were added.
Cmux Swift Concurrency	✅ Passed	PASS: Diff only refactors scope-caption logic and adds tests/localization; no new DispatchQueue, Combine, completion-handler APIs, or lifecycle Tasks were introduced.
Cmux Swift @Concurrent	✅ Passed	The PR adds only synchronous pure caption helpers/tests; no new @concurrent, nonisolated async, or heavy async UI call sites appear in the diff.
Cmux Swift File And Package Boundaries	✅ Passed	Focused bug fix in a small CmuxSettingsUI package file plus tests; no oversized growth, no mixed responsibilities, and the new logic stays behind a SwiftPM package boundary.
Cmux Swiftpm Lockfiles	✅ Passed	PR changes only Swift sources, tests, and localization; no .gitignore, Package.resolved, or Xcode package-reference files changed.
Cmux Swift Logging	✅ Passed	Branch diff adds scope-caption code, tests, and localization only; no added print/debugPrint/NSLog/Logger/stdout/stderr in production Swift.
Cmux User-Facing Error Privacy	✅ Passed	Patch only adds/rewires safe shortcut-scope captions and tests; no user-facing errors, alerts, recovery copy, or sensitive implementation details were introduced.
Cmux Full Internationalization	✅ Passed	PASS: New canvas caption uses String(localized:defaultValue:) and Resources/Localizable.xcstrings adds shortcut.when.caption.canvasLayout with 20 locales; no unlocalized user-facing text introduced.
Cmux Swiftui State Layout	✅ Passed	No new @Observable/@published, GeometryReader, row-store refs, or render-time state writes were introduced; only caption logic/tests changed.
Cmux Architecture Rethink	✅ Passed	Pure caption refactor with one owner/invariant; no timing, observer, cache, or lifecycle-splitting patterns were introduced.
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PR only changes shortcut-caption helpers, tests, and localization; no NSWindow/WindowGroup or cmuxAuxiliaryWindowIdentifiers changes.
Cmux Source Artifacts	✅ Passed	All changed paths are intentional source/config/test/localization assets; no logs, build outputs, caches, or scratch artifacts were added.
Cmux No Test Or Debug Seam In Production Source	✅ Passed	PASS: the new helper is production logic with a real caller, and I found no #if DEBUG/test-hook/seam-style members in Sources.
Title check	✅ Passed	The title clearly summarizes the main UI change: labeling canvas-layout shortcuts so zoom duplicates read as context-scoped.
Description check	✅ Passed	The description covers the problem, fix, tests, and localization, but omits the demo video, review-trigger block, and checklist items from the template.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-5810-duplicate-default-shortcuts-bound-to-both

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR fixes issue #5810 by adding an accurate "Only while the canvas layout is active" scope caption for canvas-scoped keyboard shortcuts in Settings, so the ⌘=/⌘-/⌘0 zoom shortcut family reads as context-disjoint rather than a raw duplicate-default collision.

• Extraction: The built-in scope→caption switch is moved from an inline switch in KeyboardShortcutsSection.scopeCaption() to a new ShortcutScopeCaptionResolver struct, keeping the logic unit-testable independently of the SwiftUI view.
• Canvas fix: A clauseRequiresKey helper detects the workspaceCanvasLayout key as a required AND-conjunct at any depth (not positionally), correctly labeling both plain canvas actions and the Canvas: Actual Size compound clause (canvas && !browser && !markdown) that previously fell through to the terminal caption.
• Localization: One new string (shortcut.when.caption.canvasLayout) is added to Localizable.xcstrings with translations for all 20 supported locales, at parity with the sibling terminalFocus key.

Confidence Score: 5/5

Safe to merge — a pure UI label fix with no changes to shortcut routing, binding resolution, or focus logic.

The change is narrow and self-contained: it extracts an inline switch into a testable struct, adds one new localized string, and fixes a mislabel in the Settings UI. No shortcut dispatch, binding storage, or runtime behavior is touched. The clauseRequiresKey helper correctly handles all AND-tree depths, the OR/NOT exclusions are tested explicitly, and locale parity with the sibling key is confirmed at 20 locales.

No files require special attention.

Important Files Changed

Filename	Overview
Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/ShortcutScopeCaption.swift	New file introducing ShortcutScopeCaptionResolver struct; logic is correct, clauseRequiresKey handles all AND-tree depths and correctly excludes OR/NOT/negated branches. No issues.
Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/KeyboardShortcutsSection.swift	scopeCaption() now delegates to ShortcutScopeCaptionResolver(); doc comment updated and call site is straightforward. No issues.
Packages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/ShortcutScopeCaptionTests.swift	Comprehensive test suite covering all built-in scope captions, canvas positional/nested/negated conjunct cases, per-zoom-action captions, and the duplicate-default coexistence property.
Resources/Localizable.xcstrings	Adds shortcut.when.caption.canvasLayout with translations for all 20 locales matching the terminalFocus sibling key. Locale parity confirmed.
.github/swift-file-length-budget.tsv	Reflects KeyboardShortcutsSection.swift shrinking from 620→610 lines after the extraction. Routine budget update.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["scopeCaption(for: action)"] --> B{overrideClause present?}
    B -- yes --> C[Return formatted override string or nil]
    B -- no --> D["ShortcutScopeCaptionResolver().caption(for: clause)"]

    D --> E{clause type?}
    E -- ".always" --> F["nil (no caption)"]
    E -- ".atom(.sidebarFocus)" --> G["'Only while the right sidebar is focused'"]
    E -- ".atom(.browserFocus)" --> H["'Only while a browser pane is focused'"]
    E -- ".atom(.filePreviewTextEditorFocus)" --> I["'Only while a text file preview is focused'"]
    E -- ".or(browser, filePreview) or reversed" --> J["'Only while a browser pane or text file preview is focused'"]
    E -- ".atom(.markdownFocus)" --> K["'Only while a markdown preview is focused'"]
    E -- "clauseRequiresKey(canvas) == true" --> L["'Only while the canvas layout is active' NEW"]
    E -- default --> M["'Only while a terminal pane is focused'"]

    L --> N["clauseRequiresKey(clause, key)"]
    N --> O{".key(name)==canvas?"}
    O -- yes --> P[true]
    O -- ".and(lhs, rhs)" --> Q["recurse on lhs OR rhs (any depth)"]
    O -- default --> R[false]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["scopeCaption(for: action)"] --> B{overrideClause present?}
    B -- yes --> C[Return formatted override string or nil]
    B -- no --> D["ShortcutScopeCaptionResolver().caption(for: clause)"]

    D --> E{clause type?}
    E -- ".always" --> F["nil (no caption)"]
    E -- ".atom(.sidebarFocus)" --> G["'Only while the right sidebar is focused'"]
    E -- ".atom(.browserFocus)" --> H["'Only while a browser pane is focused'"]
    E -- ".atom(.filePreviewTextEditorFocus)" --> I["'Only while a text file preview is focused'"]
    E -- ".or(browser, filePreview) or reversed" --> J["'Only while a browser pane or text file preview is focused'"]
    E -- ".atom(.markdownFocus)" --> K["'Only while a markdown preview is focused'"]
    E -- "clauseRequiresKey(canvas) == true" --> L["'Only while the canvas layout is active' NEW"]
    E -- default --> M["'Only while a terminal pane is focused'"]

    L --> N["clauseRequiresKey(clause, key)"]
    N --> O{".key(name)==canvas?"}
    O -- yes --> P[true]
    O -- ".and(lhs, rhs)" --> Q["recurse on lhs OR rhs (any depth)"]
    O -- default --> R[false]

Loading

_{Reviews (7): Last reviewed commit: "Scope shortcut caption resolver to ownin..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/ShortcutScopeCaption.swift`:
- Around line 24-73: The issue is that builtInScopeCaption(for:) is a file-scope
helper being used across files, which violates the top-level API rule. Move this
behavior onto the owning type by adding an extension method on
ShortcutWhenClause (for example, a builtInScopeCaption property or method) and
update KeyboardShortcutsSection.swift to call it through the clause instance.
Keep any remaining file-scope helper such as canvasLayoutCaption
private/fileprivate if needed, and remove the free cross-file function.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 622b6608-ef59-4178-a540-979bc21c5d39

📥 Commits

Reviewing files that changed from the base of the PR and between eeded15 and 4931c61.

⛔ Files ignored due to path filters (1)

• .github/swift-file-length-budget.tsv is excluded by !**/*.tsv

📒 Files selected for processing (4)

• Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/KeyboardShortcutsSection.swift
• Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/ShortcutScopeCaption.swift
• Packages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/ShortcutScopeCaptionTests.swift
• Resources/Localizable.xcstrings