Skip to content

Fix file-editor arrow-key navigation and add a word-wrap setting (#5227) - #5247

Merged
austinywang merged 6 commits into
mainfrom
issue-5227-file-editor-arrow-keys
Jun 2, 2026
Merged

austinywang merged 6 commits into
mainfrom
issue-5227-file-editor-arrow-keys

Conversation

@austinywang

@austinywang austinywang commented Jun 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #5227.

Two parts, as requested in the issue.

Part 1 — Bug: arrow keys don't move the cursor in the file editor

Root cause

In a cmux window, every keyDown flows through the swizzled NSWindow.performKeyEquivalent (cmux_performKeyEquivalent), where the original AppKit implementation swallows plain arrow keys (keyCodes 123–126) before they reach the focused view's keyDown. To compensate, cmux_performKeyEquivalent re-routes arrows to firstResponder.keyDown(with:) — but only for a hand-maintained set of responder types (browser web view, browser omnibar, command-palette field editor, TextBoxInputTextView).

The file editor's SavingTextView (a standalone editable NSTextView) was not in that set, so its arrow keyDowns fell through to the original performKeyEquivalent, which claimed them — the cursor never moved. Mouse cursor placement worked because that's mouseDown, not keyDown. This is structural, not macOS-26-specific.

Fix (general, not a 5th narrow case)

Generalize the seam: a new predicate shouldDispatchEditableTextViewArrowViaFirstResponderKeyDown, gated on "first responder is an editable, non-field-editor NSTextView", routes arrows to the view's keyDown. This covers SavingTextView and any future standalone editable text view cmux hosts — eliminating the whole class, not just the file editor. Modifier policy matches TextBoxInput (plain / Shift / Option / Cmd + Shift combos for move / select / word / line); Cmd+Option+Arrow is left for pane focus.

Home/End/PageUp/PageDown (keyCodes 115/116/119/121) are not in the swallowed set and already reach keyDown, so they were unaffected and need no change.

Test

Two-commit red→green: the regression commit adds EditableTextViewArrowKeyForwardingTests (and the routing seam as a stub returning false) so CI shows it red; the fix commit implements the predicate + wiring and turns it green. The test pins the real routing decision (the executable path that governs whether arrows reach the editor). A full XCUITest asserting cursor movement isn't practical — AppKit exposes no reliable accessibility hook for the insertion-point index — so coverage is the routing-decision unit test plus the manual dogfood path below.

Part 2 — Feature: word-wrap option for the file editor

New persisted setting fileEditor.wordWrap, default off (preserves the editor's established no-wrap + horizontal-scroll behavior; users opt in to wrapping).

  • Catalog (CmuxSettings): new FileEditorCatalogSection.
  • Settings window (CmuxSettingsUI): toggle row under App + curated search entry; also a command-palette toggle and legacy settings search/navigation entries.
  • ~/.config/cmux/cmux.json: parser, JSON-path allowlist, config template, and cmux.schema.json.
  • Editor: FilePreviewTextEditor configures the text container for soft wrap, driven live by @AppStorage in the file-preview and markdown-source hosts (toggling Settings reflows open editors).
  • Localized en + ja (Localizable.xcstrings, web/messages/*); documented on the configuration docs page.
  • Test: cmux.json fileEditor.wordWrap parses into UserDefaults.

Localization audit

New user-facing strings: settings.app.fileEditorWordWrap, settings.app.fileEditorWordWrap.subtitle, and the search alias settings.search.alias.setting.app.file-editor-word-wrap — all added to Resources/Localizable.xcstrings with en + ja. Docs/schema descriptions added to web/messages/en.json and web/messages/ja.json (docs.configuration.exampleFileEditorWordWrap, docs.configuration.schemaDescriptions.fileEditor.wordWrap). No bare English introduced in the changed Swift/TSX surfaces.

🤖 Generated with Claude Code


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Note

Medium Risk
Changes global window keyboard routing for all standalone editable NSTextViews; modifier policy is narrow but worth regression-testing other text surfaces.

Overview
Fixes arrow-key navigation in the built-in plain-text file editor and adds an optional fileEditor.wordWrap preference (default off).

Arrow keys: cmux’s window performKeyEquivalent path was swallowing arrows before they reached the file editor’s NSTextView. The PR forwards arrow keyDowns (plain, Shift, Option, Command combos; not Cmd+Option) to firstResponder.keyDown for any editable, non–field-editor NSTextView, shared with the existing text-box policy via standaloneTextResponderOwnsArrowKeyDown, with unit tests.

Word wrap: New fileEditor.* catalog key, Settings toggle, command palette entry, cmux.json / schema / docs, and live reflow in FilePreviewTextEditor (horizontal scroller hidden when wrap is on). Markdown source mode uses the same editor path.

Reviewed by Cursor Bugbot for commit 0e6b7a0. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Fixes arrow-key cursor movement in the file editor and adds a new word-wrap setting (off by default) that users can toggle in Settings or the command palette.

  • Bug Fixes

    • Routes arrow keys (plain, Shift/Option/Command combos) to first responder keyDown for editable, non–field-editor NSTextViews.
    • Applies to the file editor (SavingTextView), restoring cursor movement; keeps Cmd+Option+Arrow for pane focus.
    • Added a focused routing-decision test using Swift Testing and deduped the modifier logic into a shared helper.
    • Guarded word-wrap initialization to avoid a zero-width container on first layout; fixed Xcode project reference collision; restored sorted placement for fileEditor.wordWrap in the JSON-path allowlist.
  • New Features

    • Added fileEditor.wordWrap (default: false) persisted via UserDefaults/cmux.json.
    • Toggle in Settings, command palette, and settings search; localized (en/ja).
    • Live reflow in open editors (including markdown source); hides the horizontal scrollbar when enabled.
    • Updated cmux.schema.json and docs to include the new key.

Written for commit 0e6b7a0. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Added a persistent "File Editor Word Wrap" toggle for the plain-text preview/editor, surfaced in Settings UI, Command Palette, and config files; preview panels now respect this setting in real time.
  • Tests

    • Added tests for arrow-key forwarding to editable text views and for parsing the word-wrap setting from config files.
  • Documentation

    • Added localized strings, search aliases, docs, and schema entries for the new setting.

austinywang and others added 3 commits June 2, 2026 14:28
The file-editor text view (SavingTextView) is a standalone editable
NSTextView. Like the browser, omnibar, command palette, and text-box
input responders, plain arrow keyDowns are swallowed by the original
NSWindow.performKeyEquivalent before they reach the focused view, so the
window swizzle must re-route arrows to firstResponder.keyDown for it too.

This commit adds the routing-decision seam
shouldDispatchEditableTextViewArrowViaFirstResponderKeyDown plus its
regression test, with the predicate left unimplemented (returns false) so
CI shows the test red. The implementation and wiring land next.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implements shouldDispatchEditableTextViewArrowViaFirstResponderKeyDown and
wires it into the cmux_performKeyEquivalent window swizzle. A standalone
editable NSTextView (the file-preview editor's SavingTextView, and any
future hosted text view) now receives plain/selection/word/line arrow
keyDowns instead of having them swallowed by the original
NSWindow.performKeyEquivalent. Cmd+Option+Arrow is left for pane focus.

This generalizes the existing per-surface arrow-forwarding seam (browser,
omnibar, command palette, text-box input) to cover the whole class, fixing
the file-editor cursor-navigation bug. Turns EditableTextViewArrowKeyForwardingTests green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds fileEditor.wordWrap as a real, configurable setting:
- CmuxSettings catalog: new FileEditorCatalogSection (fileEditor.wordWrap, default off)
- CmuxSettingsUI: Settings window toggle row + curated search entry
- Command palette toggle + legacy settings search/navigation entries
- cmux.json: parser, JSON-path allowlist, and config template
- FilePreviewTextEditor: soft-wrap container configuration, driven live by
  @AppStorage in the file-preview and markdown-source hosts
- Localized en + ja (Localizable.xcstrings, web messages); documented in the
  configuration docs page and cmux.schema.json
- Unit test: cmux.json fileEditor.wordWrap parses into UserDefaults

Default is off to preserve the editor's established no-wrap + horizontal
scroll behavior; users opt in to wrapping.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 2, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Jun 2, 2026 10:22pm
cmux-staging Building Building Preview, Comment Jun 2, 2026 10:22pm

@coderabbitai

coderabbitai Bot commented Jun 2, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a persistent file-editor word-wrap toggle with UI, persistence, docs, and live application in file previews, and forwards arrow-key keyDown events to standalone editable NSTextView responders to restore cursor navigation.

Changes

File Editor Configuration and Features

Layer / File(s) Summary
Settings catalog and localization
Packages/CmuxSettings/Sources/CmuxSettings/Keys/FileEditorCatalogSection.swift, Packages/CmuxSettings/Sources/CmuxSettings/Keys/SettingCatalog.swift, Resources/Localizable.xcstrings
Introduce FileEditorCatalogSection with a wordWrap DefaultsKey and add English/Japanese localization and search alias entries.
Word-wrap core implementation
Sources/Panels/FilePreviewWordWrapSettings.swift, Sources/Panels/FilePreviewTextEditor.swift
FilePreviewWordWrapSettings defines the persisted fileEditor.wordWrap key/default; FilePreviewTextEditor gains wordWrap and applies it via NSTextView.applyFilePreviewWordWrap(_:scrollView:) to toggle wrapping and horizontal scrolling.
Panel wiring
Sources/Panels/FilePreviewPanel.swift, Sources/Panels/MarkdownPanelView.swift
Panels use @AppStorage to source fileEditor.wordWrap and pass it into FilePreviewTextEditor for live persisted behavior.
Settings UI & registration
Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/AppSection.swift, Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swift, Sources/SettingsNavigation.swift, Sources/SettingsSearchAliases.swift, Sources/CommandPalette/CommandPaletteSettingsToggle.swift
Add a DefaultsValueModel-backed toggle row, curated entry, settings-index/search alias mapping, and command-palette toggle descriptor for file-editor-word-wrap.
Settings-file parsing, template, and tests
Sources/KeyboardShortcutSettingsFileStore+Template.swift, Sources/KeyboardShortcutSettingsFileStore.swift, Sources/CmuxSettingsJSONPathSupport.swift, Packages/CmuxSettingsUI/Tests/CmuxSettingsUITests/SettingsRowAnchorResolutionTests.swift, cmuxTests/KeyboardShortcutSettingsFileStoreStartupTests.swift
Template includes fileEditor; parser validates/persists fileEditor.wordWrap; supported JSON paths updated; tests validate anchor resolution and startup parsing.
Arrow-key routing to editable text views
Sources/App/ShortcutRoutingSupport.swift, Sources/AppDelegate.swift, cmuxTests/EditableTextViewArrowKeyForwardingTests.swift
Centralize standalone editable text responder eligibility; add shouldDispatchEditableTextViewArrowViaFirstResponderKeyDown and AppDelegate forwarding with recursion guard; tests verify forwarding and reserved-shortcut exclusion.
Build, schema, docs, and locale updates
cmux.xcodeproj/project.pbxproj, web/data/cmux.schema.json, web/app/[locale]/docs/configuration/page.tsx, web/messages/en.json, web/messages/ja.json
Xcode project entries added; JSON schema defines fileEditor.wordWrap; docs and translation messages include descriptions and examples for English/Japanese.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant NSApp
  participant AppDelegate
  participant NSTextView
  participant FilePreviewTextEditor
  participant UserDefaults
  
  User->>NSApp: Press arrow key
  NSApp->>AppDelegate: performKeyEquivalent
  AppDelegate->>AppDelegate: shouldDispatchEditableTextViewArrowViaFirstResponderKeyDown
  alt Arrow key forwarded
    AppDelegate->>NSTextView: keyDown (forwarded)
    NSTextView->>NSTextView: Update cursor
  else Normal handling
    AppDelegate->>NSApp: Continue handling
  end
  
  User->>FilePreviewTextEditor: Toggle word-wrap
  FilePreviewTextEditor->>FilePreviewTextEditor: applyFilePreviewWordWrap
  FilePreviewTextEditor->>NSTextView: Configure textContainer width tracking
  FilePreviewTextEditor->>NSScrollView: Update horizontal scroller state
  FilePreviewTextEditor->>UserDefaults: Persist wordWrap value
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • manaflow-ai/cmux#5012: Adds settings-row anchor-resolution tests that directly validate the new fileEditor.wordWrap settings path and navigation mappings.

Poem

🐰 I hop through lines both long and tight,

Arrow keys now move with quiet delight.
Soft wrap hugs each sentence new,
Saved in settings, snug and true.
🥕


Caution

Pre-merge checks failed

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

  • Ignore

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Cmux Full Internationalization ❌ Error Web messages for fileEditor.wordWrap only exist in en.json and ja.json, but missing from 16 other supported locales per web/i18n/routing.ts. Add fileEditor.wordWrap and exampleFileEditorWordWrap entries to web/messages for all 16 missing locales.
Docstring Coverage ⚠️ Warning Docstring coverage is 17.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (16 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix file-editor arrow-key navigation and add a word-wrap setting (#5227)' clearly summarizes the two main changes: fixing arrow-key navigation and adding a word-wrap feature for the file editor.
Linked Issues check ✅ Passed The PR successfully addresses both objectives from issue #5227: restores arrow-key navigation in the file editor by routing keyDown events to editable NSTextView instances, and adds a persisted fileEditor.wordWrap setting (default off) with full Settings/schema/localization support.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the two stated objectives: arrow-key routing infrastructure, file-editor word-wrap implementation, supporting settings catalog/UI/parsing, localization, tests, and documentation—no unrelated modifications detected.
Cmux Swift Actor Isolation ✅ Passed PR introduces no actor isolation mistakes: new types are Sendable/static utilities, AppDelegate changes are @MainActor-isolated, SwiftUI uses proper property wrappers.
Cmux Swift Blocking Runtime ✅ Passed No blocking/timing synchronization added to production code. Arrow routing uses reentrancy counter, word-wrap applies properties directly, settings use @AppStorage.
Cmux No Hacky Sleeps ✅ Passed PR contains no production TypeScript/JavaScript/shell changes; all non-Swift files are configuration, data, or documentation without timing/polling logic.
Cmux Algorithmic Complexity ✅ Passed All changes are O(1): arrow routing uses fixed enum/range checks, settings parsing uses dict lookups, view updates are single-object. Search filtering unchanged on ~120-entry collection.
Cmux Swift Concurrency ✅ Passed All new code is synchronous: settings structures, routing functions, UserDefaults enum, view config. No Dispatch queues, Combine, completion handlers, or fire-and-forget Tasks added.
Cmux Swift @Concurrent ✅ Passed No Swift @concurrent annotation violations found. All new functions added are synchronous; no nonisolated async functions or invalid @concurrent annotations introduced.
Cmux Swift File And Package Boundaries ✅ Passed New files are small (21-100 lines) with single responsibilities; no oversized files exceed 250-line additions; no mixed UI/persistence/parsing; appropriate file/package boundaries throughout.
Cmux Swift Logging ✅ Passed All logging in this PR uses Apple's Logger (os.log) with proper nonisolated private declaration, correct subsystem/category, and redacted sensitive values—complying fully with swift-logging.md rules.
Cmux User-Facing Error Privacy ✅ Passed PR does not violate user-facing error privacy rules. All user-facing text uses safe generic terms and error logging is properly privacy-masked.
Cmux Swiftui State Layout ✅ Passed PR uses modern @Observable+@State pattern in AppSection; standard @AppStorage in SwiftUI views; @Published only in AppKit bridge. No swiftui-state-layout.md violations.
Cmux Architecture Rethink ✅ Passed PR uses required platform bridge code (depth guards for performKeyEquivalent reentry) following existing patterns, with clear invariants and single-owner architecture for word-wrap setting.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR creates no new standalone windows. Arrow-key routing modifies existing NSWindow swizzle; word-wrap is a settings-only feature. Lint script passes with 26 windows checked.
Description check ✅ Passed The pull request description is comprehensive and well-structured, covering both parts of the fix (arrow-key navigation bug and word-wrap feature) with clear explanations of root cause, solution, testing approach, and localization details.
✨ 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-5227-file-editor-arrow-keys

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@socket-security

socket-security Bot commented Jun 2, 2026 •

Copy link
Copy Markdown

Comment thread cmux.xcodeproj/project.pbxproj Outdated
@greptile-apps

greptile-apps Bot commented Jun 2, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a structural bug where arrow keys failed to move the cursor in the plain-text file editor, and adds a persisted fileEditor.wordWrap toggle (default off). Both changes are well-scoped and consistently layered.

  • Arrow-key fix: Generalizes the existing per-surface arrow-forwarding seam in cmux_performKeyEquivalent to cover any standalone editable, non-field-editor NSTextView. The standaloneTextResponderOwnsArrowKeyDown helper shares the modifier policy (plain / Shift / Option / Cmd+Shift for move / select / word / line; Cmd+Option excluded for pane focus) between the existing TextBoxInput path and the new shouldDispatchEditableTextViewArrowViaFirstResponderKeyDown path, with a matching unit-test suite.
  • Word-wrap toggle: Wired end-to-end through CmuxSettings catalog, Settings UI, command palette, cmux.json parser + allowlist, cmux.schema.json, localized strings (en + ja in both Localizable.xcstrings and web/messages/), docs page, and live reflow in FilePreviewTextEditor/MarkdownPanelView via @AppStorage. The zero-width-container edge case during makeNSView is guarded with visibleWidth > 0. Xcode project UUIDs for both new files are unique (the previously flagged collision is resolved).

Confidence Score: 5/5

Safe to merge — the arrow-key routing change is narrowly gated on editable, non-field-editor NSTextViews, consistent with all existing forwarding patterns, and the word-wrap toggle is default-off with live reflow.

The arrow-key fix correctly generalizes the existing per-surface forwarding seam without touching any existing routing paths. The reentrancy depth guard and Cmd+Option exclusion match every other forwarding block in the file. The word-wrap feature is wired consistently across all layers and the zero-width-container edge case during makeNSView is properly guarded. The previously flagged Xcode project UUID collision is resolved with unique IDs for both new files.

No files require special attention.

Important Files Changed

Filename Overview
Sources/App/ShortcutRoutingSupport.swift Refactors arrow-routing predicate into a shared standaloneTextResponderOwnsArrowKeyDown helper; adds shouldDispatchEditableTextViewArrowViaFirstResponderKeyDown for any standalone editable NSTextView. Modifier policy is correct; Cmd+Option exclusion for pane-focus is preserved.
Sources/AppDelegate.swift Adds cmuxEditableTextViewArrowForwardingDepth reentrancy guard and firstResponderIsStandaloneEditableTextView predicate; wires the new routing path after the TextBoxInput block, consistent with existing forwarding patterns.
Sources/Panels/FilePreviewTextEditor.swift Adds wordWrap property and applyFilePreviewWordWrap extension; called in both makeNSView and updateNSView. Zero-width guard (visibleWidth > 0) correctly avoids collapsing the text container before layout.
cmux.xcodeproj/project.pbxproj Adds two new files with unique UUIDs; the previously flagged UUID collision with MarkdownTypographyDefaults is resolved.
cmuxTests/EditableTextViewArrowKeyForwardingTests.swift Pins the routing predicate for all four arrow keyCodes, plain/modified combos, non-editable responder, marked text (IME), and Cmd+Option exclusion — covers the full decision surface.
Resources/Localizable.xcstrings Adds en+ja translations for settings.app.fileEditorWordWrap, settings.app.fileEditorWordWrap.subtitle, and the search alias — complete for the two locales supported by the touched catalog.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["NSWindow.cmux_performKeyEquivalent(event)"] --> B{Arrow keyCode 123-126?}
    B -->|No| Z["Fall through to original performKeyEquivalent"]
    B -->|Yes| C{firstResponder type}
    C -->|BrowserWebView| D["shouldDispatchBrowserArrow - firstResponder.keyDown"]
    C -->|OmnibarFieldEditor| E["shouldDispatchBrowserOmnibar - firstResponder.keyDown"]
    C -->|CommandPaletteFieldEditor| F["shouldDispatchCommandPalette - firstResponder.keyDown"]
    C -->|TextBoxInputTextView| G["shouldDispatchTextBoxInput - firstResponder.keyDown"]
    C -->|"Standalone editable NSTextView (e.g. SavingTextView) NEW"| H["shouldDispatchEditableTextView - firstResponder.keyDown"]
    H --> I{standaloneTextResponderOwnsArrowKeyDown}
    G --> I
    I -->|Modifier allowed| J["Forward to keyDown - Cursor moves"]
    I -->|Cmd+Option| K["Return false - pane-focus shortcut"]
    I -->|IME marked text| K
Loading

Reviews (4): Last reviewed commit: "Address review: Swift Testing for new te..." | Re-trigger Greptile

Comment on lines +186 to +192
if wrap {
let visibleWidth = scrollView.contentSize.width
textContainer.widthTracksTextView = true
textContainer.size = NSSize(width: visibleWidth, height: .greatestFiniteMagnitude)
// Snap the view to the visible width so wrapping reflows immediately
// instead of waiting for the next layout pass.
setFrameSize(NSSize(width: visibleWidth, height: frame.height))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Word-wrap snap may use zero width before the scroll view is laid out

In makeNSView, applyFilePreviewWordWrap is called right after the scroll view is created but before it has been added to a window or measured. When wrap == true, visibleWidth = scrollView.contentSize.width will be 0 (no layout pass has occurred yet), so setFrameSize and the text-container width are both set to 0. The subsequent updateNSView call will correct this, but there will be a brief zero-width state during initial view creation with word-wrap enabled — which can cause an empty first-pass layout for large files. Consider guarding against visibleWidth <= 0 or deferring the snap to updateNSView.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — the frame/container snap now only runs when the scroll view has a measured width (>0). widthTracksTextView = true already keeps wrapping correct before layout, and updateNSView re-applies once the clip view has a size, so there's no longer a zero-width container during makeNSView. (d6bc278)

— Claude Code

The build-file/file-ref UUIDs initially chosen collided with
MarkdownTypographyDefaults.swift, so FilePreviewWordWrapSettings.swift was
never compiled into the app module and MarkdownPanelView failed with
'cannot find FilePreviewWordWrapSettings in scope'. Reassign unique UUIDs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@cmux.xcodeproj/project.pbxproj`:
- Line 258: The PBX object IDs F5168A060000000000000001 and
F5168A060000000000002 are duplicated between FilePreviewWordWrapSettings.swift
and MarkdownTypographyDefaults.swift; regenerate new unique IDs for the
FilePreviewWordWrapSettings.swift entries (both the PBXBuildFile entry and its
PBXFileReference) and replace every occurrence so the build file, fileRef, and
any group/source list entries referencing FilePreviewWordWrapSettings.swift are
updated to the new IDs; ensure the new IDs do not collide with existing ones and
keep the existing MarkdownTypographyDefaults.swift IDs untouched so both files
are wired correctly in the project.pbxproj.

In
`@Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry`+Default.swift:
- Line 44: The new CuratedSettingEntry added in
CuratedSettingEntry+Default.swift (the .init with section: .app, id:
"file-editor-word-wrap") is hardcoded in English; replace the literal title and
synonyms with localized values by routing them through your localization system
(e.g., NSLocalizedString keys or the app's localized resource helper) or by
deriving them from the existing localized search/index source used for other
curated entries; ensure you add matching keys to the appropriate .strings files
for all supported locales and reference those keys when constructing the
CuratedSettingEntry so the title and synonyms are localized consistently.
🪄 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: 812faf65-1e8d-4b3c-abd6-4462ca3fa1e1

📥 Commits

Reviewing files that changed from the base of the PR and between 67014e3 and 249a325.

📒 Files selected for processing (25)
  • Packages/CmuxSettings/Sources/CmuxSettings/Keys/FileEditorCatalogSection.swift
  • Packages/CmuxSettings/Sources/CmuxSettings/Keys/SettingCatalog.swift
  • Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swift
  • Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/AppSection.swift
  • Packages/CmuxSettingsUI/Tests/CmuxSettingsUITests/SettingsRowAnchorResolutionTests.swift
  • Resources/Localizable.xcstrings
  • Sources/App/ShortcutRoutingSupport.swift
  • Sources/AppDelegate.swift
  • Sources/CmuxSettingsJSONPathSupport.swift
  • Sources/CommandPalette/CommandPaletteSettingsToggle.swift
  • Sources/KeyboardShortcutSettingsFileStore+Template.swift
  • Sources/KeyboardShortcutSettingsFileStore.swift
  • Sources/Panels/FilePreviewPanel.swift
  • Sources/Panels/FilePreviewTextEditor.swift
  • Sources/Panels/FilePreviewWordWrapSettings.swift
  • Sources/Panels/MarkdownPanelView.swift
  • Sources/SettingsNavigation.swift
  • Sources/SettingsSearchAliases.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/EditableTextViewArrowKeyForwardingTests.swift
  • cmuxTests/KeyboardShortcutSettingsFileStoreStartupTests.swift
  • web/app/[locale]/docs/configuration/page.tsx
  • web/data/cmux.schema.json
  • web/messages/en.json
  • web/messages/ja.json

Comment thread cmux.xcodeproj/project.pbxproj Outdated
.init(section: .app, id: "preferred-editor", title: "Open Files With", synonyms: "app.preferredEditor editor open file code vscode visual studio zed sublime subl cursor"),
.init(section: .app, id: "supported-file-previews", title: "Open Supported Files in cmux", synonyms: "app.openSupportedFilesInCmux cmd click file preview pdf image video audio quicklook quick look editor external"),
.init(section: .app, id: "markdown-viewer", title: "Open Markdown in cmux Viewer", synonyms: "app.openMarkdownInCmuxViewer md markdown mdx viewer preview readme"),
.init(section: .app, id: "file-editor-word-wrap", title: "File Editor Word Wrap", synonyms: "fileEditor.wordWrap file editor word wrap soft wrap reflow lines text horizontal scroll preview"),

@coderabbitai coderabbitai Bot Jun 2, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Localize this curated search entry before shipping.

This adds a new user-visible curated result with an English-only title and English-only search terms, so localized builds will surface/search this setting differently from the rest of the PR. Route the title/synonyms through localized resources or derive them from an already localized search/index source instead of hardcoding another English entry here. As per coding guidelines, "Any new or changed user-facing text must be fully localized across all supported locales."

🤖 Prompt for 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.

In
`@Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry`+Default.swift
at line 44, The new CuratedSettingEntry added in
CuratedSettingEntry+Default.swift (the .init with section: .app, id:
"file-editor-word-wrap") is hardcoded in English; replace the literal title and
synonyms with localized values by routing them through your localization system
(e.g., NSLocalizedString keys or the app's localized resource helper) or by
deriving them from the existing localized search/index source used for other
curated entries; ensure you add matching keys to the appropriate .strings files
for all supported locales and reference those keys when constructing the
CuratedSettingEntry so the title and synonyms are localized consistently.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentionally consistent with the existing convention: every entry in CuratedSettingEntry+Default.swift uses English-literal titles/synonyms (the curated table is the search synonym index). The user-facing Settings control itself is localized — AppSection renders String(localized: "settings.app.fileEditorWordWrap") (en + ja in Localizable.xcstrings), and the legacy search alias has en + ja entries too. Localizing this one curated entry while ~60 siblings stay English would be inconsistent; happy to localize the whole table in a separate pass if desired.

— Claude Code

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@austinywang, that makes complete sense — the file-level doc comment even states "Strings are English-only until the package ships an xcstrings catalog", and every existing sibling entry follows the same pattern. The rendered settings label and legacy search alias are already localized (en + ja), so the concern doesn't apply here. Acknowledged, and happy to see the whole table localized in a dedicated pass when the package gets its own catalog.


✏️ Learnings added
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 5247
File: Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swift:44-44
Timestamp: 2026-06-02T22:04:30.993Z
Learning: In manaflow-ai/cmux, `Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swift` is an internal search-synonym index table. All entries (title and synonyms fields) are intentionally hardcoded in English — the file-level doc comment explicitly states "Strings are English-only until the package ships an xcstrings catalog." Do NOT flag individual entries in this file for missing localization; the rendered user-facing Settings UI labels are localized separately via `String(localized:)` with keys in `Resources/Localizable.xcstrings`, and the legacy search aliases have their own en/ja entries.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: pgbezerra
Repo: manaflow-ai/cmux PR: 3307
File: Sources/cmuxApp.swift:6413-6417
Timestamp: 2026-04-30T11:55:31.575Z
Learning: In this repo (manaflow-ai/cmux), when adding a new Settings section in SwiftUI (e.g., in Sources/cmuxApp.swift or related Views), don’t wire navigation/search with a raw anchor string alone. Instead: (1) create a corresponding SettingsNavigationTarget enum case (e.g., .workspaces); (2) provide the localized title, symbol, search text, and aliases for that case; (3) add/update the matching entry in SettingsSearchIndex so the sidebar/search can navigate to it; and (4) apply .settingsSearchAnchor(SettingsSearchIndex.sectionID(for: <target>)) to the section header. This prevents broken jump-to behavior by ensuring the navigation anchor and the search index stay consistent.

Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 5174
File: Sources/SettingsSearchAliases.swift:15-15
Timestamp: 2026-06-02T06:43:09.571Z
Learning: In manaflow-ai/cmux, the `settings.search.alias.section.*` and `settings.search.alias.setting.*` keys in `Sources/SettingsSearchAliases.swift` are intentionally absent from `Resources/Localizable.xcstrings`. They are internal fuzzy-match tokens used for Settings search, not displayed UI strings, and they resolve via their English `defaultValue` synonyms at runtime. For example, `settings.search.alias.section.betaFeatures` and `settings.search.alias.setting.betaFeatures.dock` (and now `settings.search.alias.setting.betaFeatures.feed`) are all intentionally uncataloged. Do not flag missing catalog entries for these `settings.search.alias.*` keys; cataloging them is a separate, coordinated cleanup task covering all sections together.

Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 4534
File: Sources/AppDelegate.swift:11307-11311
Timestamp: 2026-05-22T05:05:58.658Z
Learning: Repo: manaflow-ai/cmux
File/Area: Sources/AppDelegate.swift (menu wiring)
Learning: For the “Reload Configuration” menu item, it’s acceptable to use a localized title-based fallback only during the first configuration pass (before the NSMenuItem identifier is assigned). The stable identifier is set immediately afterward and used for subsequent lookups. Do not flag this pattern as fragile when the identifier path is present.

Learnt from: Znboston
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-03-26T19:00:12.434Z
Learning: Repo: manaflow-ai/cmux — Sources/GroupHeaderView.swift and Sources/ContentView.swift — Palette color entry labels (`Text(entry.name)`) use dynamic palette data loaded at runtime and cannot be statically keyed with `String(localized:)`. This matches the existing workspace color picker pattern in ContentView.swift. Palette name localization would require restructuring the color palette system and is intentionally deferred; do not flag `entry.name` as a missing localization in this codebase.

Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 4831
File: Sources/cmuxApp.swift:5350-5353
Timestamp: 2026-06-01T00:51:58.772Z
Learning: In `manafow-ai/cmux` Swift settings code, `Sources/cmuxApp.swift`'s legacy in-app `SettingsView` is no longer the presented settings window for the Kiro settings UI; the live UI is `CmuxSettingsUI`'s `AutomationSection`, and `SettingsWindowRootView` is not instantiated. For `KiroIntegrationSettings.notificationLevel`, normal write paths validate against `KiroIntegrationSettings.NotificationLevel` (picker tags are valid and the `cmux.json` parser rejects invalid values), while runtime/terminal environment readers normalize via `KiroIntegrationSettings.notificationLevel(defaults:)`. Do not flag the legacy `AppStorage` string binding for `kiroNotificationLevel` solely because it could theoretically display an invalid raw value.

Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 3527
File: Sources/cmuxApp.swift:0-0
Timestamp: 2026-05-05T02:03:05.654Z
Learning: In manaflow-ai/cmux, the Settings terminal-theme picker (Sources/TerminalThemePickerRow.swift) must always include an “Adaptive” option even when the current selection is .custom or .named. When not already adaptive, build the adaptive choice from GhosttyConfig.cmuxDefaultLightThemeName and GhosttyConfig.cmuxDefaultDarkThemeName. Keep named options de-duplicated case/diacritic-insensitively.

Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-06-02T19:47:28.596Z
Learning: Applies to **/*.swift : Every new cmux-owned keyboard shortcut must be added to `KeyboardShortcutSettings`, visible/editable in Settings, supported in `~/.config/cmux/cmux.json`, and documented in keyboard shortcut and configuration docs

Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2528
File: Sources/cmuxApp.swift:6439-6444
Timestamp: 2026-04-03T03:37:25.743Z
Learning: Repo: manaflow-ai/cmux — In Sources/KeyboardShortcutSettings.swift, KeyboardShortcutSettings.setShortcut(_:, for:) is a no‑op when the action is managed by settings.json (isManagedBySettingsFile(action) == true), preventing UserDefaults backfill for file‑managed shortcuts.

Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2575
File: Sources/cmuxApp.swift:0-0
Timestamp: 2026-04-06T09:01:51.979Z
Learning: Repo: manaflow-ai/cmux — File: Sources/cmuxApp.swift — SettingsView now uses SwiftUI `.searchable(text:placement:prompt:)` for the sidebar search, which provides a native (accessible) clear button. Do not flag missing accessibility on a custom clear button in this view going forward.

Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2528
File: Sources/ContentView.swift:8917-8921
Timestamp: 2026-04-03T03:35:56.499Z
Learning: Repo: manaflow-ai/cmux — Sources/ContentView.swift — ShortcutHintModifierPolicy.shouldShowHints(for:) reveals sidebar/titlebar shortcut hints when the current modifier flags exactly equal KeyboardShortcutSettings.shortcut(for: .selectWorkspaceByNumber).modifierFlags, and returns false for chorded-number mappings. This follows the configured workspace-number modifier (not strictly Command-only). TabItemView’s close-button suppression via showsModifierShortcutHints therefore tracks the configured modifier. The debug toggle showHintsOnCommandHold predates customizable shortcuts and conceptually means “show hints on holding the configured workspace-number modifier(s).”

Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 5178
File: web/data/cmux-shortcuts.ts:223-227
Timestamp: 2026-06-02T08:18:29.897Z
Learning: In the manaflow-ai/cmux project, `LocalizedText` in `web/data/cmux-shortcuts.ts` is intentionally typed as `{ en: string; ja: string }` — every shortcut entry in that file provides only English and Japanese descriptions. The keyboard-shortcuts docs renderer has an explicit English fallback for all other locales. This is the established design for the entire shortcuts data file. Reviewers should NOT flag individual shortcut entries for missing locale coverage beyond en/ja in this file; extending to additional locales would require a separate project-wide change to the shortcuts i18n system. The native macOS Settings UI label localization (via Localizable.xcstrings) is separate and does cover all supported catalog locales.

Learnt from: HamptonMakes
Repo: manaflow-ai/cmux PR: 4443
File: Sources/cmuxApp.swift:6678-6695
Timestamp: 2026-05-22T15:04:55.554Z
Learning: In manaflow-ai/cmux, agent/automation settings strings (settings.automation.*) follow AGENTS.md, which scopes supported locales to English and Japanese; sibling toggles on main (Cursor/Gemini) also include Korean. For new entries like settings.automation.amp.*, matching en/ja/ko is acceptable and we should not require expansion to all app locales in the touched catalog unless maintainers request it.

Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 3971
File: Resources/Localizable.xcstrings:64948-64951
Timestamp: 2026-05-13T21:22:34.162Z
Learning: In `Resources/Localizable.xcstrings` for the `manaflow-ai/cmux` repo, `settings.betaFeatures.*` localization keys (e.g., `settings.betaFeatures.dock`, `settings.betaFeatures.feed`, and their subtitle/warning variants) intentionally include only English ("en") and Japanese ("ja") locales while the feature is in beta. Do not flag the absence of other locales for these beta feature keys as a localization gap — this is the established convention for beta-gated features in this project.

Learnt from: HamptonMakes
Repo: manaflow-ai/cmux PR: 4443
File: Resources/Localizable.xcstrings:59654-59745
Timestamp: 2026-05-21T21:34:34.427Z
Learning: For `settings.automation.*` integration-toggle keys in `Resources/Localizable.xcstrings` (e.g., `settings.automation.cursor`, `settings.automation.gemini`, `settings.automation.amp` and their `.note`, `.subtitleOff`, `.subtitleOn` sub-keys), the established repo pattern is to provide only `en`, `ja`, and `ko` locales with `extractionState: "manual"` and `state: "translated"`. Do NOT flag these as partial-localization violations — the en/ja/ko-only set is intentional for automation integration settings and matches the existing sibling-key precedent.

Learnt from: azooz2003-bit
Repo: manaflow-ai/cmux PR: 4771
File: CLI/cmux.swift:6013-6047
Timestamp: 2026-05-26T05:16:37.569Z
Learning: In the manaflow-ai/cmux project, CLI-facing strings (CLIError messages, per-subcommand help/usage text, and diagnostic summary output in CLI/cmux.swift) are intentionally kept as hard-coded English and are NOT subject to the xcstrings/`String(localized:)` localization requirement. The localization rule (full-internationalization.md) targets in-app user-facing UI strings only. Existing CLI subcommands such as reorder-workspace follow the same hard-coded English pattern. Do not flag missing localization for CLI error messages, help text, or diagnostic output in CLI/**/*.swift.

Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3393
File: web/messages/zh-TW.json:0-0
Timestamp: 2026-05-01T08:41:57.216Z
Learning: Repo: manaflow-ai/cmux — In `web/messages/*.json` locale files, the `docs.dock.agentPrompt` key is intentionally kept in English across all locales (including high-confidence locales like zh-TW, ja, ko, de, fr, etc.) because it is a verbatim prompt designed to be copied into coding agents. Do not flag the English value of `agentPrompt` as a missing translation for any locale. Only `agentPromptIntro` and all other surrounding dock doc strings are expected to be translated for high-confidence locales.

Learnt from: zlatkoc
Repo: manaflow-ai/cmux PR: 1368
File: Sources/Panels/BrowserPanel.swift:69-69
Timestamp: 2026-03-13T13:46:07.021Z
Learning: In manaflow-ai/cmux (Sources/Panels/BrowserPanel.swift), search engine `displayName` values (e.g. "Google", "DuckDuckGo", "Bing", "Kagi", "Startpage") are intentionally bare string literals and must NOT be wrapped with `String(localized:...)`. They are proper brand/product names, not translatable UI text. The localization guideline applies only to generic UI strings (labels, buttons, error messages, etc.), not to engine/brand names.

Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 5174
File: Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BetaFeaturesSection.swift:5-6
Timestamp: 2026-06-02T06:43:13.599Z
Learning: In manaflow-ai/cmux, all `settings.betaFeatures.*` localization keys (including `settings.betaFeatures.dock`, `.extensions`, `.warning`, `.feed`, and their `.subtitleOn`/`.subtitleOff` variants) are intentionally translated only into `en` and `ja`. This matches the project's documented supported languages in CLAUDE.md ("currently English and Japanese"). Do NOT flag missing translations for other locales in the `settings.betaFeatures.*` key family — adding other locales to only one key would make it inconsistent with the rest of the family. Broadening locale coverage for the whole beta-features string family is a separate, intentional change.

Learnt from: nanami-he
Repo: manaflow-ai/cmux PR: 4633
File: Resources/Localizable.xcstrings:38310-38310
Timestamp: 2026-05-23T08:40:54.816Z
Learning: In the manaflow-ai/cmux repository, the only required locales for newly added keys in `Resources/Localizable.xcstrings` are **English (`en`) and Japanese (`ja`)**. This is explicitly documented in `CLAUDE.md` ("Keys go in `Resources/Localizable.xcstrings` with translations for all supported languages (currently English and Japanese)"). Approximately 42% of catalog keys carry only `en`+`ja`. Keys with additional locales (up to 19) are populated by a separate batch-translation pass and are NOT a requirement for newly added keys. Do NOT flag `en`+`ja`-only entries as incomplete localization.

Learnt from: nanami-he
Repo: manaflow-ai/cmux PR: 4633
File: Resources/Localizable.xcstrings:38310-38310
Timestamp: 2026-05-23T08:40:54.816Z
Learning: In the manaflow-ai/cmux repository, the only required locales for newly added keys in `Resources/Localizable.xcstrings` are **English (`en`) and Japanese (`ja`)**. This is explicitly documented in `CLAUDE.md`: "Keys go in `Resources/Localizable.xcstrings` with translations for all supported languages (currently English and Japanese)." Approximately 42% of catalog keys (886 of 2086) carry only `en`+`ja`. Keys with additional locales (up to ~19) are populated by a separate batch-translation pass and are NOT a requirement for newly added keys. Do NOT flag `en`+`ja`-only entries as incomplete or partial localization.

Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T04:48:00.216Z
Learning: Applies to **/*.swift : All user-facing strings must be localized using `String(localized: "key.name", defaultValue: "English text")` for every string shown in the UI (labels, buttons, menus, dialogs, tooltips, error messages). Keys must go in `Resources/Localizable.xcstrings` with translations for all supported languages (English and Japanese). Never use bare string literals in SwiftUI `Text()`, `Button()`, alert titles, etc.

Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/Update/UpdateTitlebarAccessory.swift:924-926
Timestamp: 2026-03-04T14:04:40.577Z
Learning: In manaflow-ai/cmux (Swift/SwiftUI macOS app), the notifications empty-state localization keys are intentionally different across two views:
- `NotificationsPage.swift` uses key `"notifications.empty.description"` with text "Desktop notifications will appear here for quick review." (full-page view).
- `UpdateTitlebarAccessory.swift` uses key `"notifications.empty.subtitle"` with text "Desktop notifications will appear here." (space-constrained titlebar popover).
Both keys have correct Japanese translations in the .xcstrings catalog. The difference in key names and text is by design, not an inconsistency.

Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2475
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-03T07:19:36.497Z
Learning: Repo: manaflow-ai/cmux — In Sources/ContentView.swift, the command‑palette rename flow uses a single-line SwiftUI TextField via commandPaletteEditorField(style: .singleLine(...)); the multiline NSTextView editor is only used for the workspace description input. Do not flag newline persistence for rename.

Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2514
File: Sources/GhosttyTerminalView.swift:3759-3761
Timestamp: 2026-04-01T22:57:41.165Z
Learning: Repo: manaflow-ai/cmux — In Sources/cmuxApp.swift, ClaudeCodeIntegrationSettings.customClaudePath(defaults:) trims surrounding whitespace and returns nil for empty/whitespace-only values; callers (e.g., TerminalSurface.createSurface(for:)) can safely set CMUX_CUSTOM_CLAUDE_PATH without additional trimming.

Learnt from: tayl0r
Repo: manaflow-ai/cmux PR: 1909
File: Sources/ContentView.swift:2156-2171
Timestamp: 2026-03-23T06:08:14.740Z
Learning: Repo: manaflow-ai/cmux — In Sources/ContentView.swift, openFileInTextEditor(_:) must attempt workspace.newTextEditorSplit(from:orientation:filePath:focus:) and, if that returns nil, fall back to workspace.newTextEditorSurface(inPane:filePath:focus:) using bonsplitController.focusedPaneId. Rationale: avoid dropping file-open requests when the focused panel is not pane-backed.

Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3182
File: Sources/ContentView.swift:10850-10875
Timestamp: 2026-04-27T10:11:40.167Z
Learning: Repo: manaflow-ai/cmux — When measuring NSTextView content height (e.g., in FeedbackComposerMessageEditorView.naturalDocumentHeight(for:)), include layoutManager.extraLineFragmentRect.height when extraLineFragmentTextContainer === textContainer to account for a trailing newline; otherwise the caret on the final blank line can be clipped. Apply this pattern to future NSTextView-based editors in this repo.

Learnt from: HamptonMakes
Repo: manaflow-ai/cmux PR: 2007
File: Sources/cmuxApp.swift:3798-3799
Timestamp: 2026-03-23T16:36:55.259Z
Learning: Repo: manaflow-ai/cmux — In Sources/cmuxApp.swift, SettingsView.resetAllSettings() must reset newly added AppStorage toggles to defaults. Specifically, ensure ampHooksEnabled is set to AmpIntegrationSettings.defaultHooksEnabled so the Amp integration toggle resets correctly.

Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2525
File: Sources/GhosttyTerminalView.swift:481-513
Timestamp: 2026-04-02T10:13:39.235Z
Learning: Repo: manaflow-ai/cmux — In Sources/GhosttyTerminalView.swift, terminal file-link resolution trims trailing unmatched closing delimiters “) ] } >” only when they are dangling (more closers than openers), preserving wrapped tokens like “(file:///tmp/a.png)”. Implemented via terminalFileLinkTrailingClosingDelimiters and count comparison inside trimTrailingTerminalFileLinkPunctuation(_:) and exercised by a regression test (PR `#2525`, commit 3f5c5b6d).

Learnt from: qkrwpdlr
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-03-23T07:12:42.553Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift and Sources/Panels/BrowserPanel.swift, BiDi override (U+202A–202E, U+2066–2069) and zero-width char (U+200B–200F, U+FEFF) filtering is implemented via a shared `dangerousScalars: Set<UInt32>` in each class. v2SanitizeWebText() truncates to 200 chars; v2SanitizeXPath() caps at 2000 chars for selector fidelity. Both use a shared v2SanitizeScalar() predicate. BrowserPickerMessageHandler.sanitize() uses the same dangerousScalars pattern with a 200-char cap.

Learnt from: arieltobiana
Repo: manaflow-ai/cmux PR: 1873
File: Sources/TerminalController.swift:4071-4087
Timestamp: 2026-03-20T17:18:30.333Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift, v2WorkspaceAction(params:) -> case "set_color": palette names are resolved via WorkspaceTabColorSettings.defaultPaletteWithOverrides(), whose entries are always valid hex (validated by the UI). Therefore, additional normalization of entry.hex is unnecessary.

Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 3843
File: Sources/AppDelegate.swift:14237-14239
Timestamp: 2026-05-11T06:23:21.666Z
Learning: In manaflow-ai/cmux (Sources/AppDelegate.swift), within NSWindow.cmux_performKeyEquivalent, when forwarding Return/Enter or plain arrow keys for browser inputs, the keyDown target must be the resolved owning web view: prefer firstResponderWebView (CmuxWebView), else firstResponderEmbeddedWebView (WKWebView), else fall back to firstResponder. This avoids AppKit beeps on form submit and preserves arrow handling in embedded auth web views. Tests verify embedded WKWebView delivery.

Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 2505
File: Sources/AppDelegate.swift:5636-5642
Timestamp: 2026-04-06T07:18:41.310Z
Learning: Repo: manaflow-ai/cmux — In AppDelegate’s .keyDown focus-repair path, never dereference NSTextView.delegate (unsafe-unretained). Resolve field-editor ownership via cmuxFieldEditorOwnerView(_), and prefer superview/nextResponder traversal or hostedView.responderMatchesPreferredKeyboardFocus(...) for matching, as applied in AppDelegate.swift and GhosttySurfaceScrollView.

Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3244
File: Sources/AppDelegate.swift:6601-6614
Timestamp: 2026-04-29T06:25:44.652Z
Learning: Repo: manaflow-ai/cmux — Settings window behavior (PR `#3244`): Settings now uses a singleton SwiftUI Window scene. SettingsWindowPresenter.show(navigationTarget:) first locates any existing Settings NSWindow and, if found, deminiaturizes it when isMiniaturized and brings it to the front (makeKeyAndOrderFront/activation) instead of creating a duplicate; only opens a new window when none exists. AppDelegate.presentPreferencesWindow correctly delegates to SettingsWindowPresenter.show and should not add extra re-front/dedup logic.

Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3784
File: README.md:161-161
Timestamp: 2026-05-09T04:48:35.413Z
Learning: In the cmux project, the right-sidebar keyboard shortcut labels were intentionally swapped (per PR `#3784`). Reviewers should NOT flag the ⌘⇧E (Cmd+Shift+E) label as “Open file explorer.” Use these mappings consistently: ⌘⇧E → `focusRightSidebar` with the user-facing label “Toggle right sidebar focus”; ⌘⌥B (Cmd+Option+B) → `toggleFileExplorer` with the user-facing label “Open file explorer.”

Learnt from: jt-hsiao
Repo: manaflow-ai/cmux PR: 1423
File: Sources/AppDelegate.swift:11220-11226
Timestamp: 2026-03-14T07:06:01.466Z
Learning: Repo: manaflow-ai/cmux — In Sources/Panels/CmuxWebView.swift, performKeyEquivalent(with:) handles Command-key routing end-to-end: (1) if allowed, route to NSApp.mainMenu.performKeyEquivalent; (2) fall back to AppDelegate.shared?.handleBrowserSurfaceKeyEquivalent(event) for non–menu-backed app shortcuts; (3) fall back to super.performKeyEquivalent. For non-Command keys it calls super directly. Therefore, in NSWindow.cmux_performKeyEquivalent (Sources/AppDelegate.swift), it is correct to call firstResponderWebView.performKeyEquivalent and return its Bool unconditionally to avoid re-entering SwiftUI’s performKeyEquivalent path that can swallow keys after WKWebView focus.

Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3244
File: Sources/WindowDecorationsController.swift:71-73
Timestamp: 2026-04-29T06:24:53.283Z
Learning: Repo: manaflow-ai/cmux — Sources/WindowDecorationsController.swift — `trafficLightOffset(for:)` returned `.zero` for all non-settings windows on `main`; the only non-zero offset was for `cmux.settings` (nudge right/down to align with the custom Settings title row). PR `#3244` intentionally removes that offset because Settings now uses a SwiftUI WindowGroup with a native macOS system titlebar, making the nudge unnecessary. Do not flag `trafficLightOffset(for:)` always returning `.zero` as a regression.

Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-04T14:05:42.574Z
Learning: Guideline: In Swift files (cmux project), when handling pluralized strings, prefer using localization keys with the ICU-style plural forms .one and .other. For example, use keys like statusMenu.unreadCount.one for the singular case (1) and statusMenu.unreadCount.other for all other counts, and similarly for statusMenu.tooltip.unread.one/other. Rationale: ensures correct pluralization across locales and makes localization keys explicit. Review code to ensure any unread count strings and related tooltips follow this .one/.other key pattern and verify the correct value is chosen based on the count.

Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 954
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-05T22:04:34.712Z
Learning: Adopt the convention: for health/telemetry tri-state values in Swift, prefer Optionals (Bool?) over sentinel booleans. In TerminalController.swift, socketConnectable is Bool? and only set when socketProbePerformed is true; downstream logic must treat nil as 'not probed'. Ensure downstream code checks for nil before using a value and uses explicit non-nil checks to determine state, improving clarity and avoiding misinterpretation of default false.

Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-16T08:02:06.824Z
Learning: In Swift sources, for any panel_id-only route handling in v2PanelMarkBackground(params:) and v2PanelMarkForeground(params:), first attempt v2ResolveTabManager(params:). Use the manager only if it actually owns the panelId; otherwise fall back to AppDelegate.shared?.locateSurface(surfaceId:) to locate the correct TabManager across windows. Apply this pattern to all panel_id-only routes to avoid active-window bias.

Learnt from: pratikpakhale
Repo: manaflow-ai/cmux PR: 2011
File: Resources/Localizable.xcstrings:15256-15368
Timestamp: 2026-03-23T21:39:50.795Z
Learning: When reviewing this repo’s Swift localization usage, do not flag missing `String.localizedStringWithFormat` for calls that use the modern overload `String(localized: "key", defaultValue: "...\(variable)")` (where `defaultValue` is a `String.LocalizationValue` built with `\(…)`). That overload natively supports interpolation and the xcstrings/runtime substitution handles the resulting placeholders automatically. Only require `String.localizedStringWithFormat` when using the older `String(localized:)` overload that takes a plain `String` (i.e., where format arguments must be passed separately), such as for keys like `clipboard.sshError.single`.

Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: Sources/TerminalController.swift:3620-3622
Timestamp: 2026-03-25T00:32:54.735Z
Learning: When validating or reporting workspace/tab colors in this repo, only accept and use 6-digit hex colors in the form `#RRGGBB` (no alpha, i.e., do not allow `#RRGGBBAA`). Ensure validation logic matches the existing behavior (e.g., WorkspaceTabColorSettings.normalizedHex(...) and TabManager.setTabColor(tabId:color:) as well as CLI/cmux.swift). Update any error/help text for workspace color to reference only `#RRGGBB` (not `#RRGGBBAA`).

Learnt from: ryouka0731
Repo: manaflow-ai/cmux PR: 3415
File: Sources/Worktree/WorktreeManager.swift:1-10
Timestamp: 2026-05-06T19:24:46.399Z
Learning: Do not recommend extracting Foundation-only Swift files into a new SwiftPM package target in this repo. The established pattern is to keep app-internal helper modules under Sources/ unless the repository introduces a dedicated package (e.g., CMUXAuthCore, CMUXDebugLog, CMUXWorkstream) with a clear architectural rationale. Speculative SwiftPM extractions for individual Foundation-only modules expand PR scope without documented justification. Apply this guideline when reviewing changes to Swift files under Sources/; require a documented package architecture justification before considering packaging changes.

Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 3626
File: Sources/TabManager.swift:1817-1818
Timestamp: 2026-05-07T08:37:03.967Z
Learning: In this Swift repo (manafow-ai/cmux), when cleaning up stale agent process entries, use `Workspace.clearAgentPID(key:panelId:)` as the single cleanup path. Do not directly mutate `Workspace.statusEntries` or `Workspace.agentPIDs` from outside the dedicated helpers; for example, `TabManager.sweepStaleAgentPIDs` should only call `clearAgentPID` rather than performing its own mutations. This ensures panel-scoped side effects and port/refresh logic run consistently.

Learnt from: psh4607
Repo: manaflow-ai/cmux PR: 3696
File: cmuxTests/ShortcutAndCommandPaletteTests.swift:1716-1771
Timestamp: 2026-05-07T10:56:50.266Z
Learning: In the manaflow-ai/cmux repo, SwiftLint does not enforce a `required_deinit` rule (no project `.swiftlint.yml` in cmux itself, no `required_deinit` in `.github/review-bot-rules/`, and no SwiftLint CI run in `.github/workflows/`). During code reviews, do not raise findings for missing `deinit` on `XCTestCase` subclasses or other Swift classes based on a `required_deinit` rule.

Comment thread Sources/App/ShortcutRoutingSupport.swift
…5227)

- Extract standaloneTextResponderOwnsArrowKeyDown shared by the text-box-input
  and editable-text-view arrow predicates (removes duplicated body; cursor).
- Only snap the wrapped text view to a measured width when the scroll view has
  been laid out; widthTracksTextView already keeps wrapping correct before
  then, avoiding a zero-width container in makeNSView (greptile).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d6bc278. Configure here.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 25 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread cmuxTests/EditableTextViewArrowKeyForwardingTests.swift Outdated
…ist (#5227)

- Convert EditableTextViewArrowKeyForwardingTests to Swift Testing
  (@Suite/@Test/#expect) per the repo test-framework policy (cubic).
- Move fileEditor.wordWrap to its correct alphabetical slot (after the
  browser.* block) in rowConfigPaths (cursor).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

This branch was successfully deployed

1 active deployment
Preview – cmux — 0e6b7a02 Deployed Jun 2, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File editor: arrow keys don't move the cursor + add word-wrap option

1 participant