-
Notifications
You must be signed in to change notification settings - Fork 67
Add ghostty_config_set_color_scheme C API #39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,6 +3,7 @@ const inputpkg = @import("../input.zig"); | |||||||||||||||||||||||||||||||||||||||||||||||
| const state = &@import("../global.zig").state; | ||||||||||||||||||||||||||||||||||||||||||||||||
| const c = @import("../main_c.zig"); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const conditional = @import("conditional.zig"); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const conditional = @import("conditional.zig"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify whether `conditional` is referenced beyond its declaration.
rg -n '\bconditional\b' src/config/CApi.zigRepository: manaflow-ai/ghostty
Length of output: 260
🏁 Script executed:
cat -n src/config/CApi.zig | sed -n '85,115p'Repository: manaflow-ai/ghostty
Length of output: 1216
🏁 Script executed:
# Check full file for any actual usage of conditional (excluding just the import line)
wc -l src/config/CApi.zig && echo "---" && rg -n 'conditional\.' src/config/CApi.zigRepository: manaflow-ai/ghostty
Length of output: 140
🏁 Script executed:
# Search for _conditional_state and conditional type references in Config
rg -n '_conditional_state|ConditionalState' src/config/CApi.zig -A 2 -B 2Repository: manaflow-ai/ghostty
Length of output: 323
🏁 Script executed:
# Search for Config struct definition
rg -n 'struct.*Config|pub.*const.*Config' src/config/CApi.zig -A 5 | head -30Repository: manaflow-ai/ghostty
Length of output: 45
🏁 Script executed:
head -20 src/config/CApi.zigRepository: manaflow-ai/ghostty
Length of output: 708
🏁 Script executed:
rg -n '_conditional_state' src/config/Config.zig -A 1 -B 1 | head -30Repository: manaflow-ai/ghostty
Length of output: 1228
Remove unused conditional import (compile blocker).
Line 6 adds const conditional = @import("conditional.zig");, but this symbol is not referenced anywhere in the file. The _conditional_state field uses the conditional.State type, which is already defined in Config.zig; CApi.zig does not need to directly import this module since it only uses the Config type. In Zig, unused imports cause compilation failures.
💡 Proposed fix
-const conditional = `@import`("conditional.zig");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const conditional = @import("conditional.zig"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/config/CApi.zig` at line 6, Remove the unused import by deleting the line
that declares const conditional = `@import`("conditional.zig"); in CApi.zig; the
file only needs the Config type and references _conditional_state which uses
conditional.State indirectly via Config.zig, so remove the unused conditional
symbol to fix the compile error caused by an unused import.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Added conditional import is unused and can fail Zig compilation due to unused top-level const.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/config/CApi.zig, line 6:
<comment>Added `conditional` import is unused and can fail Zig compilation due to unused top-level const.</comment>
<file context>
@@ -3,6 +3,7 @@ const inputpkg = @import("../input.zig");
const state = &@import("../global.zig").state;
const c = @import("../main_c.zig");
+const conditional = @import("conditional.zig");
const Config = @import("Config.zig");
const c_get = @import("c_get.zig");
</file context>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use
intToEnum for consistency with existing color-scheme functions
The sibling functions ghostty_app_set_color_scheme and ghostty_surface_set_color_scheme in embedded.zig both use std.meta.intToEnum(apprt.ColorScheme, scheme_raw) for conversion. Using hardcoded 0/1 here diverges from that pattern and means the mapping stays in sync with conditional.State.Theme's field order only by convention. Prefer the same idiom so any future enum evolution is caught at compile time.
| 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 { | |
| const 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; | |
| }; | |
| self._conditional_state.theme = theme; | |
| } |
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!
Copilot
AI
Apr 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This adds a new exported C API that affects conditional theme resolution, but there is no unit test covering the new behavior (unlike other functions in this file which have Zig tests). Add a Zig test that sets the scheme via ghostty_config_set_color_scheme before finalize and asserts a conditional theme resolves according to the selected scheme, to prevent regressions.
| test "ghostty_config_set_color_scheme resolves conditional theme during finalize" { | |
| const config = ghostty_config_new() orelse return error.OutOfMemory; | |
| defer { | |
| config.deinit(); | |
| state.alloc.destroy(config); | |
| } | |
| try edit.set(config, .theme, "light:Foo,dark:Bar"); | |
| ghostty_config_set_color_scheme(config, 1); | |
| try config.finalize(); | |
| try std.testing.expectEqualStrings("Bar", config.theme.?); | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Zig implementation documents that this must be called before
ghostty_config_finalize, but the C header has no such note. Embedders working fromghostty.halone won't see the constraint. If called after finalization the conditional state is updated but the already-resolved theme string is unchanged — a silent no-op that produces wrong colors. A brief comment here would close that gap.