Repository navigation
Auto-assign distinct colors to split panes - #6981
Closed
austinywang wants to merge 22 commits into
Closed
austinywang wants to merge 22 commits into
austinywang wants to merge 22 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds automatic pane-level tinting for newly created terminal splits, with a new persisted setting, runtime and tmux wiring, snapshot persistence, settings-file support, and schema/docs/localization updates. ChangesAuto-tint split panes feature
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 2 warnings)
✅ Passed checks (21 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
added 2 commits
June 26, 2026 16:40
…n-distinct-colors-to-split-panes-cr # Conflicts: # .github/swift-file-length-budget.tsv
Contributor
Greptile SummaryThis PR introduces
Confidence Score: 5/5Safe to merge — the feature is well-isolated, fully togglable, and the persistence provenance guard correctly prevents terminal-controlled backgrounds from leaking into session restore as cmux-assigned tints. The tinting logic is pure computation with no mutable global state risk, the session snapshot field is optional and backward-compatible, and 11 targeted unit tests cover the planner's assignment, wrap-around, and provenance-guard behavior. The one architectural note (TerminalSplitPaneTintPlanner as a caseless enum namespace) is a style concern matching a known team rule, not a runtime defect. Sources/Workspace.swift — contains the caseless-enum namespace and the unused static forwarder worth a follow-up cleanup. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[newTerminalSplit called] --> B[applyAutomaticSplitPaneTints]
B --> C{autoTintSplitPanes enabled?}
C -- No --> Z[done]
C -- Yes --> D[Resolve true background\nWorkspaceContentView.resolveGhosttyAppearanceConfig]
D --> E[Collect usedHexes\nfrom all pane overrides]
E --> F[TerminalSplitPaneTintPlanner\n.assignmentForTerminalSplit]
F --> G{source needs tint?}
G -- Yes --> H[nextColor → sourceColor\nadd to usedHexes + selectedHexes]
G -- No --> I[sourceColor = nil]
H --> J{newPane needs tint?}
I --> J
J -- Yes --> K[nextColor excluding selectedHexes\n→ newPaneColor]
J -- No --> L[newPaneColor = nil]
K --> M[Apply colors\nset autoAssignedSplitTintHex]
L --> M
N[Session snapshot] --> O[persistableTintHex\nliveOverride == autoAssigned?]
O -- Yes --> P[Write backgroundColorHex]
O -- No --> Q[Skip - terminal-controlled]
R[Session restore] --> S{autoTintSplitPanes enabled?\n& backgroundColorHex present?}
S -- Yes --> T[Restore paneBackgroundOverrideColor\nRe-record autoAssignedSplitTintHex]
S -- No --> U[Skip restore]
V[RemoteTmuxWindowMirror\nreconcile] --> W[applyAutomaticPaneTints]
W --> X[Assign tints to untinted\nremote panes in order]
%%{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[newTerminalSplit called] --> B[applyAutomaticSplitPaneTints]
B --> C{autoTintSplitPanes enabled?}
C -- No --> Z[done]
C -- Yes --> D[Resolve true background\nWorkspaceContentView.resolveGhosttyAppearanceConfig]
D --> E[Collect usedHexes\nfrom all pane overrides]
E --> F[TerminalSplitPaneTintPlanner\n.assignmentForTerminalSplit]
F --> G{source needs tint?}
G -- Yes --> H[nextColor → sourceColor\nadd to usedHexes + selectedHexes]
G -- No --> I[sourceColor = nil]
H --> J{newPane needs tint?}
I --> J
J -- Yes --> K[nextColor excluding selectedHexes\n→ newPaneColor]
J -- No --> L[newPaneColor = nil]
K --> M[Apply colors\nset autoAssignedSplitTintHex]
L --> M
N[Session snapshot] --> O[persistableTintHex\nliveOverride == autoAssigned?]
O -- Yes --> P[Write backgroundColorHex]
O -- No --> Q[Skip - terminal-controlled]
R[Session restore] --> S{autoTintSplitPanes enabled?\n& backgroundColorHex present?}
S -- Yes --> T[Restore paneBackgroundOverrideColor\nRe-record autoAssignedSplitTintHex]
S -- No --> U[Skip restore]
V[RemoteTmuxWindowMirror\nreconcile] --> W[applyAutomaticPaneTints]
W --> X[Assign tints to untinted\nremote panes in order]
Reviews (18): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
added 3 commits
June 28, 2026 20:21
…n-distinct-colors-to-split-panes-cr # Conflicts: # .github/swift-file-length-budget.tsv # Sources/Workspace.swift
…n-distinct-colors-to-split-panes-cr # Conflicts: # .github/swift-file-length-budget.tsv
…n-distinct-colors-to-split-panes-cr # Conflicts: # .github/swift-file-length-budget.tsv
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@cmuxTests/RemoteTmuxMirrorSplitRoutingTests.swift`:
- Around line 145-148: The current test only checks that
SessionTerminalPanelSnapshot stores backgroundColorHex in memory, so it does not
verify persistence behavior. Update terminalSnapshotPersistsPaneBackgroundHex to
exercise a full snapshot round-trip by encoding and decoding the snapshot
through its Codable path, then assert the restored SessionTerminalPanelSnapshot
still has the same backgroundColorHex. Use the existing
SessionTerminalPanelSnapshot initializer and verify the field survives via
CodingKeys/restore logic rather than only the direct initializer.
In `@Sources/App/WorkspaceRuntimeSettings.swift`:
- Around line 202-210: The `TerminalSplitPaneTintSettings` helper is duplicating
the auto-tint split panes key and default instead of reusing the catalog’s
`DefaultsKey` source of truth. Update `isEnabled(defaults:)` to read the key and
default from `TerminalCatalogSection.autoTintSplitPanes` (the same catalog value
used by `CmuxSettingsJSONPathSupport.swift`) so the runtime setting stays in
sync with the settings catalog and import path.
In `@Sources/Workspace.swift`:
- Around line 7087-7100: The split-pane tint logic in
applyAutomaticSplitPaneTints is using the app-wide default background instead of
the source pane’s actual effective terminal background. Update baseColor to come
from the source TerminalPanel’s resolved background (via terminalPanel(for:) and
its surface/background config) so assignmentForTerminalSplit tints against the
pane’s real color, preserving per-pane themes and correct opacity selection.
- Around line 88-96: The tint selection in Workspace’s split-pane flow can reuse
the same wrapped palette color for both panes because `usedHexes` only tracks
historical colors and `nextColor(baseColor:usedHexes:)` is called twice. Update
the `sourceColor`/`newPaneColor` assignment path to track the color chosen in
the current operation separately, and make the second `nextColor` call exclude
the already selected tint so both panes never receive the same color.
🪄 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: 3c612c29-0813-49e0-a987-2a4330c0f11b
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (33)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Keys/TerminalCatalogSection.swiftSources/App/WorkspaceRuntimeSettings.swiftSources/CmuxSettingsJSONPathSupport.swiftSources/GhosttyTerminalView.swiftSources/KeyboardShortcutSettingsFileStore+Template.swiftSources/RemoteTmuxSessionMirror.swiftSources/RemoteTmuxWindowMirror.swiftSources/SessionPersistence.swiftSources/Workspace.swiftcmuxTests/RemoteTmuxMirrorSplitRoutingTests.swiftcmuxTests/TerminalScrollSpeedSettingsFileStoreTests.swiftweb/app/[locale]/docs/configuration/page.tsxweb/data/cmux.schema.jsonweb/messages/ar.jsonweb/messages/bs.jsonweb/messages/da.jsonweb/messages/de.jsonweb/messages/en.jsonweb/messages/es.jsonweb/messages/fr.jsonweb/messages/it.jsonweb/messages/ja.jsonweb/messages/km.jsonweb/messages/ko.jsonweb/messages/no.jsonweb/messages/pl.jsonweb/messages/pt-BR.jsonweb/messages/ru.jsonweb/messages/th.jsonweb/messages/tr.jsonweb/messages/uk.jsonweb/messages/zh-CN.jsonweb/messages/zh-TW.json
…n-distinct-colors-to-split-panes-cr # Conflicts: # .github/swift-file-length-budget.tsv
Add Swift-DocC documentation to the newly introduced public autoTintSplitPanes DefaultsKey, matching the documentation pattern used by sibling catalog keys (e.g. rendererRealizationEnabled). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…n-distinct-colors-to-split-panes-cr # Conflicts: # .github/swift-file-length-budget.tsv
…n-distinct-colors-to-split-panes-cr # Conflicts: # .github/swift-file-length-budget.tsv
Auto split-pane tints are stored in the surface's paneBackgroundOverrideColor slot (shared with Ghostty OSC/config background overrides), so a config/theme reload clears them until the next split. Recorded as an accepted v1 limitation per PR review rather than reworking the shared-slot storage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
Autoreview note — accepted v1 limitation Automatic split-pane tints are stored in the surface's
All verified real, but accepted as a v1 limitation rather than reworking the shared-slot storage now: a correct fix is source-tagging |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
`nextColor` is fed the set of distinct colors already used by live panes, so `usedHexes` saturates at the eight-color palette: once all eight colors are present the Set can no longer grow, and the `usedHexes.count % palette.count` wrap resolves to index 0 for every subsequent request. The test asserted clean modular cycling (`tints[9] == tints[1]`), which the planner never produces from a saturating Set. Correct it to `tints[9] == tints[0]`, rename the test, and document why, matching the real call site's behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The auto-tint feature stored assigned colors in `paneBackgroundOverrideColor`,
a surface slot shared with terminal OSC background changes
(GHOSTTY_ACTION_COLOR_CHANGE) and Ghostty config reloads
(GHOSTTY_ACTION_CONFIG_CHANGE). Session persistence snapshotted and restored
that slot unconditionally, so:
* a transient terminal/OSC background present at save time was persisted and
resurrected later as a sticky cmux pane tint, overriding the user's theme;
* restore reapplied tints even when `terminal.autoTintSplitPanes` was
disabled, so the opt-out did not survive a restart.
Record the cmux-assigned hex as provenance on the surface
(`autoAssignedSplitTintHex`) at every assignment site (split, remote-tmux
mirror, and restore). Persist `backgroundColorHex` only when the live override
still equals that provenance -- a terminal overwrite or config-reload clear
makes them diverge and drops the value -- and gate restore on the
`terminal.autoTintSplitPanes` setting so a disabled feature is not resurrected.
Adds pure regression coverage for the persistence gate.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… tint The local split path fed the source pane's live paneBackgroundOverrideColor into TerminalSplitPaneTintPlanner as the base color. The planner blends its palette *into* the base, so once a source pane was tinted, planning the next split against that tinted color generated off-palette tint-of-a-tint colors; repeated splits from the same pane drifted/compounded instead of cycling the distinct palette (PR #6981 autoreview). Always use the resolved Ghostty background as the base, matching RemoteTmuxWindowMirror.applyAutomaticPaneTints; existing overrides remain represented via usedHexes and sourceNeedsTint. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…n-distinct-colors-to-split-panes-cr # Conflicts: # .github/swift-file-length-budget.tsv
…n-distinct-colors-to-split-panes-cr Resolve .github/swift-file-length-budget.tsv via scripts/swift_file_length_budget.py --write-budget (only textual conflict; numeric drift on both sides). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/GhosttyTerminalView.swift (1)
8049-8066: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftShared background slot mixes cmux tint provenance with OSC/config state.
paneBackgroundOverrideColoris the sameNSColor?slot mutated by OSC 11 background changes (GHOSTTY_ACTION_COLOR_CHANGE) and cleared by Ghostty config reloads (GHOSTTY_ACTION_CONFIG_CHANGE) elsewhere in this file.autoAssignedSplitTintHexis only a "last assigned" marker, not real provenance tagging, so a transient OSC background write or a config-reload clear can desync it from the actualbackgroundColor, and a staleautoAssignedSplitTintHexcan be persisted into session snapshots as if it were a genuine cmux tint.This is the exact scenario called out by the single-source-of-truth guideline for pane tint provenance. Per the PR's own comments, this is an acknowledged v1 limitation (fixing it needs source-tagging
backgroundColoracross OSC/config/setter/persistence paths) accepted for now as a documented follow-up, so I'm not blocking on it — just flagging so the follow-up tracking isn't lost.Based on the coding guideline requiring "pane background tint provenance ... come from the stored cmux-owned fields ... not from string/title matching or 'best effort' branches," and the PR objectives, which state this tradeoff is accepted for v1 pending a follow-up.
🤖 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 `@Sources/GhosttyTerminalView.swift` around lines 8049 - 8066, The split-pane tint provenance is still derived from the shared `paneBackgroundOverrideColor` slot, which can be overwritten by OSC 11 and config reloads, so `autoAssignedSplitTintHex` can drift from the actual background color. Update the persistence logic around `TerminalSplitPaneTintPlanner.persistableTintHex` and the `paneBackgroundOverrideColor`/`autoAssignedSplitTintHex` fields so only cmux-owned tint state is persisted, or otherwise add explicit source tagging to distinguish cmux tint from OSC/config writes. If this is intentionally deferred for v1, keep the limitation clearly documented in the relevant symbols so the follow-up is tracked.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 8049-8066: The split-pane tint provenance is still derived from
the shared `paneBackgroundOverrideColor` slot, which can be overwritten by OSC
11 and config reloads, so `autoAssignedSplitTintHex` can drift from the actual
background color. Update the persistence logic around
`TerminalSplitPaneTintPlanner.persistableTintHex` and the
`paneBackgroundOverrideColor`/`autoAssignedSplitTintHex` fields so only
cmux-owned tint state is persisted, or otherwise add explicit source tagging to
distinguish cmux tint from OSC/config writes. If this is intentionally deferred
for v1, keep the limitation clearly documented in the relevant symbols so the
follow-up is tracked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4ba6b525-af2d-47fe-9504-96ab25b86b71
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (4)
Sources/CmuxSettingsJSONPathSupport.swiftSources/GhosttyTerminalView.swiftSources/Workspace.swiftweb/app/[locale]/docs/configuration/page.tsx
💤 Files with no reviewable changes (2)
- web/app/[locale]/docs/configuration/page.tsx
- Sources/Workspace.swift
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5975
Summary
Validation
No local dev build or xcodebuild run per issue instructions.
Summary by cubic
Auto-assigns subtle, distinct background tints to terminal split panes and restores them across sessions and remote tmux mirrors (fixes #5975). Known v1 limitation: reloading Ghostty config/themes clears pane tints until the next split re-assigns them.
New Features
terminal.autoTintSplitPanesso disabled stays disabled after restart.terminal.autoTintSplitPanesto settings (UserDefaults mapping,web/data/cmux.schema.json, template, and localized docs).Migration
terminal.autoTintSplitPanesto false incmux.json.Written for commit 50a3d36. Summary will update on new commits.
Summary by CodeRabbit
terminal.autoTintSplitPanes(default: enabled) to automatically apply subtle, distinct background tints to newly created terminal split panes.terminal.autoTintSplitPanes.