feat: add cmux theme picker live preview hooks#42
Conversation
When CMUX_THEME_PICKER_* environment variables are set, the theme picker writes theme choices to a config file and sends an NSDistributedNotification so the cmux app reloads themes in real-time. Without those env vars, behavior is unchanged from upstream. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 59 minutes and 50 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ Finishing Touches🧪 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/cli/list_themes.zig">
<violation number="1" location="src/cli/list_themes.zig:718">
P1: Unused local constant `cmux` in new code can cause Zig compile failure.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| fn applyCmuxSelectionForCurrentTheme(self: *Preview) !void { | ||
| const cmux = self.cmux orelse return; |
There was a problem hiding this comment.
P1: Unused local constant cmux in new code can cause Zig compile failure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/cli/list_themes.zig, line 718:
<comment>Unused local constant `cmux` in new code can cause Zig compile failure.</comment>
<file context>
@@ -411,6 +700,72 @@ const Preview = struct {
+ }
+
+ fn applyCmuxSelectionForCurrentTheme(self: *Preview) !void {
+ const cmux = self.cmux orelse return;
+ if (self.filtered.items.len == 0) return;
+
</file context>
| const cmux = self.cmux orelse return; | |
| if (self.cmux == null) return; |
Greptile SummaryThis PR adds an opt-in live theme preview mechanism to
Confidence Score: 2/5Not safe to merge — a compile error in the new code breaks +list-themes for all users. The unused local constant src/cli/list_themes.zig — compile error on line 718, directory creation on line 324, help overlay loop on lines 1194-1196. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Preview as list_themes TUI
participant FS as Config File
participant NS as NSDistributedNotification
participant cmux as cmux App
User->>Preview: launch with CMUX_THEME_PICKER_CONFIG set
Preview->>FS: readOptionalFile (capture original_contents)
Preview->>Preview: CmuxThemePicker.load() — read env vars
loop Render loop (every frame)
User->>Preview: navigate (j/k/scroll)
Preview->>Preview: applyCmuxSelectionForCurrentTheme()
Preview->>Preview: syncCmuxPreview() — dedup check
alt theme changed
Preview->>FS: writeCmuxThemeOverride (managed block)
Preview->>NS: postNotificationName:object:deliverImmediately:
NS-->>cmux: com.cmuxterm.themes.reload-config
cmux-->>User: live theme update
end
Preview->>User: draw TUI frame
end
alt User presses Enter (apply)
Preview->>Preview: outcome = .apply, should_quit = true
Note over FS: managed block stays in config
else User presses q/Esc (cancel)
Preview->>Preview: outcome = .cancel, should_quit = true
Preview->>FS: restoreCmuxThemeOverride (original_contents)
Preview->>NS: postNotificationName (reload)
NS-->>cmux: revert to original theme
end
Reviews (1): Last reviewed commit: "feat: add cmux theme picker helper hooks" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
|
|
||
| fn applyCmuxSelectionForCurrentTheme(self: *Preview) !void { | ||
| const cmux = self.cmux orelse return; |
There was a problem hiding this comment.
const cmux = self.cmux orelse return; binds the unwrapped value but it is never referenced; self.cmux.? is used in the switch below instead. Zig treats unused local constants as a hard compile error ("pointless discard of local constant"), so this entire file will not compile, breaking +list-themes for all users regardless of whether the env vars are set.
Fix: use the bound cmux value in the switch body to eliminate the unused binding.
| const cmux = self.cmux orelse return; | |
| fn applyCmuxSelectionForCurrentTheme(self: *Preview) !void { | |
| const cmux = self.cmux orelse return; | |
| if (self.filtered.items.len == 0) return; | |
| const theme = self.themes[self.filtered.items[self.current]].theme; | |
| switch (cmux.target_mode) { |
| if (std.fs.path.dirname(path)) |dir| { | ||
| std.fs.makeDirAbsolute(dir) catch |err| switch (err) { | ||
| error.PathAlreadyExists => {}, | ||
| else => return err, | ||
| }; | ||
| } |
There was a problem hiding this comment.
std.fs.makeDirAbsolute creates only one directory level. If the user-supplied CMUX_THEME_PICKER_CONFIG path sits under directories that do not exist yet (e.g. ~/.config/cmux/theme.conf when ~/.config/cmux/ is missing), this will propagate error.FileNotFound back to the caller and silently prevent live preview from working on first run.
Replace with std.fs.cwd().makePath(dir), which creates the full directory chain and is already idempotent for existing paths:
| if (std.fs.path.dirname(path)) |dir| { | |
| std.fs.makeDirAbsolute(dir) catch |err| switch (err) { | |
| error.PathAlreadyExists => {}, | |
| else => return err, | |
| }; | |
| } | |
| if (std.fs.path.dirname(path)) |dir| { | |
| try std.fs.cwd().makePath(dir); | |
| } |
| for (key_help, 0..) |help, captured_i| { | ||
| if (help.help.len == 0) continue; | ||
| const i: u16 = @intCast(captured_i); |
There was a problem hiding this comment.
captured_i keeps incrementing for every element in key_help, including the ones that get continued. Because row_offset = i + 1 uses this raw index, any skipped entry leaves a blank row in the overlay. In cmux mode, the "w" entry (index 18) is skipped, so the rendered "t" entry appears one row too low and a visible gap is left above it.
Track a separate render counter to keep rows contiguous:
| for (key_help, 0..) |help, captured_i| { | |
| if (help.help.len == 0) continue; | |
| const i: u16 = @intCast(captured_i); | |
| var row: u16 = 0; | |
| for (key_help) |help| { | |
| if (help.help.len == 0) continue; | |
| defer row += 1; |
Then replace the three row_offset = i + 1 references below with row_offset = row + 1.
| pub fn run(self: *Preview) !void { | ||
| errdefer self.restoreCmuxOriginal() catch {}; | ||
| defer if (self.outcome == .cancel) { | ||
| self.restoreCmuxOriginal() catch {}; | ||
| }; |
There was a problem hiding this comment.
When run() returns an error and outcome is still .cancel (the initial value), both the errdefer on line 573 and the defer on lines 574-576 fire, calling restoreCmuxOriginal twice. The second write is redundant. Consider removing the errdefer and relying solely on the defer, which already handles cancel correctly:
| pub fn run(self: *Preview) !void { | |
| errdefer self.restoreCmuxOriginal() catch {}; | |
| defer if (self.outcome == .cancel) { | |
| self.restoreCmuxOriginal() catch {}; | |
| }; | |
| defer if (self.outcome == .cancel) { | |
| self.restoreCmuxOriginal() catch {}; | |
| }; |
1 participant
Summary
CMUX_THEME_PICKER_*environment variables are set, the+list-themesTUI writes theme choices to a config file and sends anNSDistributedNotificationso the cmux app reloads themes in real-timeCMUX_THEME_PICKER_TARGETEnvironment variables
CMUX_THEME_PICKER_CONFIG— path to the config file to write theme overridesCMUX_THEME_PICKER_BUNDLE_ID— bundle ID for the notification (default:com.cmuxterm.app)CMUX_THEME_PICKER_INITIAL_LIGHT/CMUX_THEME_PICKER_INITIAL_DARK— current theme names for initial selectionCMUX_THEME_PICKER_TARGET—both,light, ordarkChanges from upstream
list_themes.zigCmuxThemePickerstruct loaded from env varsNSDistributedNotificationto signal cmux to reload configtkey to cycle target mode (both/light/dark)Test plan
ghostty +list-themeswithout env vars — behavior identical to upstreamCMUX_THEME_PICKER_CONFIG=/tmp/test-theme.conf ghostty +list-themes— verify live preview writes config file on navigationtkey cycles target mode in footer bar🤖 Generated with Claude Code
Summary by cubic
Adds live cmux theme preview to
ghostty +list-themes. WhenCMUX_THEME_PICKER_*vars are set, the picker writes a managed theme block to a config file and sends a macOS distributed notification so cmux reloads instantly. Unset vars keep upstream behavior.New Features
CmuxThemePicker): writes a managed block toCMUX_THEME_PICKER_CONFIGand postscom.cmuxterm.themes.reload-configviaNSDistributedNotificationCenter(bundle fromCMUX_THEME_PICKER_BUNDLE_ID, defaultcom.cmuxterm.app).both/light/darkviaCMUX_THEME_PICKER_TARGET; presstto cycle. Enter applies and exits; q/Esc cancels and restores the original file. Initial selection can be seeded byCMUX_THEME_PICKER_INITIAL_LIGHT/CMUX_THEME_PICKER_INITIAL_DARK.Migration
CMUX_THEME_PICKER_CONFIG=/path/to/cmux.conf.CMUX_THEME_PICKER_BUNDLE_ID,CMUX_THEME_PICKER_INITIAL_LIGHT,CMUX_THEME_PICKER_INITIAL_DARK,CMUX_THEME_PICKER_TARGET=both|light|dark.ghostty +list-themes; usetto change target, Enter to apply, q/Esc to cancel.Written for commit 45c4266. Summary will update on new commits.