Add ghostty_config_set_color_scheme C API#37
Add ghostty_config_set_color_scheme C API#37rodchristiansen wants to merge 1 commit intomanaflow-ai:mainfrom
Conversation
Allows setting the config's conditional theme state before finalization so that conditional theme resolution (e.g. `theme = light:Foo,dark:Bar`) uses the correct scheme during ghostty_config_finalize. Without this, finalization always uses the ConditionalState default (.light), causing apps to load the wrong theme colors until a surface color scheme change triggers re-derivation.
📝 WalkthroughWalkthroughA new public C API function Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR adds Confidence Score: 5/5Safe to merge; all findings are P2 style suggestions that do not affect correctness. The implementation is functionally correct — the switch values (0=light, 1=dark) match the C header constants, the warning path on invalid input is handled, and the approach is consistent with how the rest of Config.zig directly manipulates _conditional_state. Both remaining comments are purely stylistic (enum conversion pattern and missing test) and do not introduce any runtime defect. src/config/CApi.zig — enum conversion style and missing test coverage.
|
| Filename | Overview |
|---|---|
| src/config/CApi.zig | Adds ghostty_config_set_color_scheme export; uses hardcoded switch on raw integers instead of std.meta.intToEnum, inconsistent with the pattern in embedded.zig; missing unit test. |
| include/ghostty.h | Adds ghostty_config_set_color_scheme declaration using the pre-existing ghostty_color_scheme_e typedef; placed correctly among other config API declarations. |
Sequence Diagram
sequenceDiagram
participant Embedder as Embedder (C)
participant CApi as CApi.zig
participant Config as Config._conditional_state
Embedder->>CApi: ghostty_config_new()
CApi-->>Embedder: config*
Embedder->>CApi: ghostty_config_load_default_files(config)
Embedder->>CApi: ghostty_config_set_color_scheme(config, DARK)
CApi->>Config: theme = .dark
Embedder->>CApi: ghostty_config_finalize(config)
Note over Config: Conditional theme resolution uses .dark → loads dark theme values
Embedder->>CApi: ghostty_config_get(config, "background", ...)
CApi-->>Embedder: dark background color
Reviews (1): Last reviewed commit: "Add ghostty_config_set_color_scheme C AP..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export fn ghostty_config_set_color_scheme(self: *Config, scheme_raw: c_int) void { | ||
| self._conditional_state.theme = switch (scheme_raw) { | ||
| 0 => .light, | ||
| 1 => .dark, | ||
| else => { | ||
| log.warn( | ||
| "invalid color scheme to ghostty_config_set_color_scheme value={}", | ||
| .{scheme_raw}, | ||
| ); | ||
| return; | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
The analogous functions ghostty_app_set_color_scheme and ghostty_surface_set_color_scheme in src/apprt/embedded.zig both use std.meta.intToEnum to validate and convert the raw integer, keeping a single conversion point that will produce a compile error if the enum grows. The new function hard-codes the integer literals instead, which diverges from that convention and silently becomes stale if conditional.State.Theme ever adds a variant with a non-sequential value.
| export fn ghostty_config_set_color_scheme(self: *Config, scheme_raw: c_int) void { | |
| self._conditional_state.theme = switch (scheme_raw) { | |
| 0 => .light, | |
| 1 => .dark, | |
| else => { | |
| log.warn( | |
| "invalid color scheme to ghostty_config_set_color_scheme value={}", | |
| .{scheme_raw}, | |
| ); | |
| return; | |
| }, | |
| }; | |
| } | |
| export fn ghostty_config_set_color_scheme(self: *Config, scheme_raw: c_int) void { | |
| self._conditional_state.theme = std.meta.intToEnum( | |
| conditional.State.Theme, | |
| scheme_raw, | |
| ) catch { | |
| log.warn( | |
| "invalid color scheme to ghostty_config_set_color_scheme value={}", | |
| .{scheme_raw}, | |
| ); | |
| return; | |
| }; | |
| } |
| export fn ghostty_config_set_color_scheme(self: *Config, scheme_raw: c_int) void { | ||
| self._conditional_state.theme = switch (scheme_raw) { | ||
| 0 => .light, | ||
| 1 => .dark, | ||
| else => { | ||
| log.warn( | ||
| "invalid color scheme to ghostty_config_set_color_scheme value={}", | ||
| .{scheme_raw}, | ||
| ); | ||
| return; | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Every other exported function in this file has at least one Zig test (e.g. ghostty_config_get has six). A test that sets the scheme to dark before finalization and verifies _conditional_state.theme == .dark — and a second confirming the warning path leaves the state unchanged — would match the existing test pattern here.
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!
There was a problem hiding this comment.
Pull request overview
Adds a new C API entry point to allow embedders to set the config’s conditional theme (light/dark) prior to config finalization, so conditional theme values resolve correctly from the start.
Changes:
- Add
ghostty_config_set_color_schemeZig export to setConfig._conditional_state.themebased on a C enum value. - Expose the new function in the public C header (
include/ghostty.h).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/config/CApi.zig | Adds the new exported C API function that sets the conditional theme before finalize. |
| include/ghostty.h | Declares the new C API function for external consumers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const state = &@import("../global.zig").state; | ||
| const c = @import("../main_c.zig"); | ||
|
|
||
| const conditional = @import("conditional.zig"); |
There was a problem hiding this comment.
const conditional = @import("conditional.zig"); is currently unused in this file, which will fail Zig compilation (unused top-level declaration). Either remove the import or use it (e.g., parse scheme_raw via std.meta.intToEnum(conditional.State.Theme, …) and assign the result).
| const conditional = @import("conditional.zig"); |
| /// Set the color scheme on a configuration's conditional state before | ||
| /// finalization. This ensures that conditional theme resolution (e.g. | ||
| /// `theme = light:Foo,dark:Bar`) uses the correct scheme during | ||
| /// `ghostty_config_finalize`. Must be called before finalize. | ||
| export fn ghostty_config_set_color_scheme(self: *Config, scheme_raw: c_int) void { |
There was a problem hiding this comment.
This new C API entry point isn’t covered by tests in this file. Since src/config/CApi.zig already has unit tests, add tests for ghostty_config_set_color_scheme verifying: (1) 0/1 set _conditional_state.theme to light/dark respectively, and (2) invalid values log a warning and do not modify the existing theme value.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/config/CApi.zig (1)
93-109: Add focused tests for the new color-scheme setter behavior.Please add coverage for DARK, LIGHT, and invalid inputs (including “state unchanged on invalid”) to prevent regressions in pre-finalize theme resolution.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config/CApi.zig` around lines 93 - 109, Add scoped unit tests exercising ghostty_config_set_color_scheme: verify that passing 0 sets Config._conditional_state.theme to .light, passing 1 sets it to .dark, and passing an invalid integer leaves _conditional_state.theme unchanged and logs a warning; specifically, create tests that initialize a Config with a known starting theme, call ghostty_config_set_color_scheme for LIGHT (0) and DARK (1) and assert the theme value changes accordingly, then call it with an invalid value and assert the theme remains the pre-call value while a warning was emitted. Ensure tests invoke ghostty_config_set_color_scheme directly and assert on Config._conditional_state.theme and the logging side-effect to cover all three behaviors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/config/CApi.zig`:
- Around line 93-109: Add scoped unit tests exercising
ghostty_config_set_color_scheme: verify that passing 0 sets
Config._conditional_state.theme to .light, passing 1 sets it to .dark, and
passing an invalid integer leaves _conditional_state.theme unchanged and logs a
warning; specifically, create tests that initialize a Config with a known
starting theme, call ghostty_config_set_color_scheme for LIGHT (0) and DARK (1)
and assert the theme value changes accordingly, then call it with an invalid
value and assert the theme remains the pre-call value while a warning was
emitted. Ensure tests invoke ghostty_config_set_color_scheme directly and assert
on Config._conditional_state.theme and the logging side-effect to cover all
three behaviors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b5a5606f-fe26-4ad8-b525-d9c8229c47ba
📒 Files selected for processing (2)
include/ghostty.hsrc/config/CApi.zig
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
The previous fix only handled the config reload path. If macOS auto- switched dark/light while cmux was in the background (e.g. overnight via the auto appearance schedule), per-view viewDidChangeEffectiveAppearance callbacks did not always fire reliably for hidden views, leaving the surfaces stuck in the wrong theme. The root cause runs deeper: Ghostty's ConditionalState.theme defaults to .light, so ghostty_config_finalize always resolved conditional themes (e.g. `theme = light:Foo,dark:Bar`) with the light variant even on systems running dark mode. This made defaultBackgroundColor and the surface-level config wrong from the start. Fix: 1. New ghostty C API: ghostty_config_set_color_scheme, called before ghostty_config_finalize in loadDefaultConfigFilesWithLegacyFallback so the config is finalized with the correct theme variant from the beginning. (Requires ghostty submodule bump with the new export.) 2. Deferred ghostty_app_set_color_scheme after ghostty_app_new so app.config_conditional_state matches the current appearance. New surfaces then inherit the right scheme instead of .light. Deferred via DispatchQueue.main.async to avoid re-entrant reloads during GhosttyApp initialization. 3. ghostty_app_set_color_scheme in synchronizeThemeWithAppearance to keep the app-level state in sync with the new appearance before the config reload runs. 4. KVO observer on NSApp.effectiveAppearance so appearance changes are detected even when no terminal view receives viewDidChangeEffectiveAppearance. 5. synchronizeThemeWithAppearance call in applicationDidBecomeActive as a backstop for when cmux regains focus after macOS switched appearance modes while it was inactive. .gitmodules temporarily points ghostty at the rodchristiansen fork while the upstream ghostty C API PR (manaflow-ai/ghostty#37) is in review. Revert to manaflow-ai/ghostty once that merges.
The previous fix only handled the config reload path. If macOS auto- switched dark/light while cmux was in the background (e.g. overnight via the auto appearance schedule), per-view viewDidChangeEffectiveAppearance callbacks did not always fire reliably for hidden views, leaving the surfaces stuck in the wrong theme. The root cause runs deeper: Ghostty's ConditionalState.theme defaults to .light, so ghostty_config_finalize always resolved conditional themes (e.g. `theme = light:Foo,dark:Bar`) with the light variant even on systems running dark mode. This made defaultBackgroundColor and the surface-level config wrong from the start. Fix: 1. New ghostty C API: ghostty_config_set_color_scheme, called before ghostty_config_finalize in loadDefaultConfigFilesWithLegacyFallback so the config is finalized with the correct theme variant from the beginning. (Requires ghostty submodule bump with the new export.) 2. Deferred ghostty_app_set_color_scheme after ghostty_app_new so app.config_conditional_state matches the current appearance. New surfaces then inherit the right scheme instead of .light. Deferred via DispatchQueue.main.async to avoid re-entrant reloads during GhosttyApp initialization. 3. ghostty_app_set_color_scheme in synchronizeThemeWithAppearance to keep the app-level state in sync with the new appearance before the config reload runs. 4. KVO observer on NSApp.effectiveAppearance so appearance changes are detected even when no terminal view receives viewDidChangeEffectiveAppearance. 5. synchronizeThemeWithAppearance call in applicationDidBecomeActive as a backstop for when cmux regains focus after macOS switched appearance modes while it was inactive. .gitmodules temporarily points ghostty at the rodchristiansen fork while the upstream ghostty C API PR (manaflow-ai/ghostty#37) is in review. Revert to manaflow-ai/ghostty once that merges.
Summary
Adds a new C API function
ghostty_config_set_color_schemethat allows setting the config's conditional theme state before finalization.Why
Without this,
ghostty_config_finalizealways usesConditionalState.theme = .light(the Zig struct default) to resolve conditional theme configs liketheme = light:Foo,dark:Bar. Apps end up loading the light theme colors even when the system is in dark mode, and the background/foreground state can only be corrected after a surface color scheme change triggers a cascade re-derivation — which has race conditions and doesn't fully update the app-level default background color used by embedding apprts.With this API, embedders can set the scheme before finalization so the config resolves the correct theme from the start.
Test plan
ghostty_config_set_color_scheme(config, GHOSTTY_COLOR_SCHEME_DARK)beforeghostty_config_finalize(config)results in the config loading dark theme conditional valuesghostty_config_set_color_scheme(config, GHOSTTY_COLOR_SCHEME_LIGHT)loads light theme conditional valuesSummary by cubic
Adds a C API to set the config’s color scheme before finalization so conditional themes resolve correctly from the start. This prevents apps from defaulting to light colors when the system is in dark mode.
New Features
Migration
Written for commit e0f6eba. Summary will update on new commits.
Summary by CodeRabbit