Repository navigation
Allow cmux sidebar theming from Ghostty config - #4749
austinywang wants to merge 13 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughExtends ChangesSidebar Theme Configuration & Rendering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (19 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 95f67b7. Configure here.
Greptile SummaryAdds Ghostty-config-driven sidebar color theming: 8 new keys (
Confidence Score: 4/5Safe to merge with one issue to verify: a test covering the paired-theme + top-level foreground override interaction was silently deleted. The managed-defaults logic is well-tested for the new paths. The deletion of cmuxTests/GhosttyConfigTests.swift — deleted test with no corresponding production change or replacement. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Ghostty config file\n(sidebar-foreground, sidebar-border-color, …)"] --> B["GhosttyConfig.parse()"]
B --> C["rawSidebar* fields set"]
C --> D["resolveSidebarAppearance(preferredColorScheme:)"]
D --> E["sidebarForeground, sidebarBorderColor, … NSColor fields"]
E --> F["applySidebarAppearanceToUserDefaults()"]
F --> G{{"key in appliedValues?"}}
G -->|"Yes — managed value\nmatches current"| H["UserDefaults.removeObject(forKey:)"]
G -->|"No — user-edited\nor new value"| I["UserDefaults.set(hex, forKey:)"]
I --> J["appliedValues dict updated"]
J --> K["cmux.ghosttyConfig.sidebarAppearance\n.appliedValues.v1 persisted"]
K --> L["SidebarTabItemSettingsSnapshot\nreads foregroundColorHex,\nborderColorHex, etc."]
L --> M["TabItemView computed colors\nactivePrimaryTextColor / activeSecondaryColor\nsidebarAccentColor / WindowChromeBorder.colorOverride"]
Reviews (10): Last reviewed commit: "fix: apply muted sidebar detail color" | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…ing-of-cmux-sidebar-via
There was a problem hiding this comment.
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 `@docs/configuration.md`:
- Around line 17-19: Update the "Ghostty sidebar theme keys" section to
explicitly state these are cmux-specific extensions (not standard Ghostty keys)
and add concrete format examples; mention that values use Ghostty-style hex
(e.g., `#RRGGBB`) and show example usages including a single-mode example like
"sidebar-bg: `#123ABC`" and a light/dark pair like "sidebar-text:
light:`#111111`,dark:`#FFFFFF`" so theme authors understand scope and exact formats.
In `@Sources/GhosttyConfig.swift`:
- Around line 209-303: applySidebarAppearanceToUserDefaults creates a local
defaults = UserDefaults.standard but its helpers (applySidebarBackgroundColor,
applySidebarColorIfConfigured, clearManagedSidebarAppearanceValue and any other
helper that reads/writes defaults) call UserDefaults.standard directly, causing
inconsistent access; update those helper signatures to accept a defaults:
UserDefaults parameter (or alternatively return to using UserDefaults.standard
everywhere) and change calls in applySidebarAppearanceToUserDefaults to pass the
local defaults variable (also update any other call sites to the new
signatures), so all reads/writes use the same UserDefaults instance.
🪄 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: f0c2a2d7-05b0-4b35-b1a1-b468be960d9b
📒 Files selected for processing (5)
Sources/ContentView.swiftSources/GhosttyConfig.swiftSources/Sidebar/SidebarAppearanceSupport.swiftcmuxTests/GhosttyConfigTests.swiftdocs/configuration.md
Dismissed after addressing the two CodeRabbit comments in fb2504d; both CodeRabbit review threads are resolved and the follow-up CodeRabbit summary completed with pre-merge checks passing.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -721,70 +721,6 @@ final class GhosttyConfigTests: XCTestCase { | |||
| XCTAssertEqual(rgb255(darkConfig.backgroundColor), RGB(red: 0, green: 43, blue: 54)) | |||
There was a problem hiding this comment.
testLoadHonorsPairedThemeAndTopLevelForegroundOverrideByColorScheme was removed in this diff. The test exercised a specific interaction: a config that sets theme = light:X,dark:Y followed by a top-level foreground = #rrggbb override, asserting the override wins for both color schemes. No production code in GhosttyConfig.swift changed this behavior — the case "foreground" parse branch and paired-theme loadTheme call are untouched — so the test should still be valid. Silently dropping it without a comment or a replacement reduces confidence that this interaction continues to work if the parse order changes in a future PR.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
3 participants
Closes #1759.
Summary
docs/configuration.md.Scope
Verification
git diff --checkNeed help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches appearance persistence in UserDefaults and broad sidebar rendering paths; logic is heavily tested but wrong managed-key clearing could surprise users who mix manual and config-driven colors.
Overview
Adds Ghostty-driven sidebar color theming: new cmux-specific config keys (foreground, muted text, selection foreground, border, accent, notification badge, plus existing selection background) are parsed, resolved for light/dark pairs, and written into
UserDefaultsthrough an expandedapplySidebarAppearanceToUserDefaultspath that records prior writes undercmux.ghosttyConfig.sidebarAppearance.appliedValues.v1so removing or changing config only clears values cmux still owns—not user-edited hex overrides.The sidebar settings snapshot gains the new hex fields, and workspace rows use them for primary/muted text, selection foreground, active borders, progress tracks/fills, unread badges, drop indicators, and the trailing divider (via
WindowChromeBordercolor override). Accent and border fall back to existing cmux defaults when unset.Docs (
docs/configuration.md) document the keys; tests cover parsing, resolution, managed-default clearing, and invalid-config preservation.Reviewed by Cursor Bugbot for commit bc51165. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds Ghostty-driven theming for the cmux sidebar with light/dark support and preserved user overrides. Also bounds sidebar row text to fixed line limits to prevent layout overflow and fixes muted detail color. Addresses Linear #1759.
New Features
sidebar-background,sidebar-tint-opacity,sidebar-selection-background,sidebar-selection-foreground,sidebar-foreground,sidebar-muted-foreground,sidebar-border-color,sidebar-accent-color,sidebar-notification-badge-background(alias:sidebar-notification-badge-color).WindowChromeBorderoverride), progress track/fill, drop indicators, and badges; selection/muted text fall back to configured foreground when unset.Bug Fixes
cmux.ghosttyConfig.sidebarAppearance.appliedValues.v1, and clears only keys cmux set (including opacity), including when config keys are removed or switch from paired to single/dark-only.Written for commit 4455b08. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
sidebar-notification-badge-coloras an alias.