macos: make QuickTerminalSize config C ABI compatible (#9837)
Hello! I did read `CONTRIBUTING.md`. I understand that the preference is to receive pull requests for existing open issues and acknowledge that I'm skipping a step wrt the preferred workflow since there aren't any open issues yet for #8419. Living dangerously and hoping for the best here. I hope you'll see that I'm acting in good faith 🤞. In my attempts to debug why `quick-terminal-size` config wasn't working reliably for me, I discovered that the [`ghostty_config_quick_terminal_size_s`](pull/9572/head08c9661683/include/ghostty.h (L460-L480)) struct used to initialize `QuickTerminalSize` in Swift-land, more often than not, didn't match what I had in my config file (e.g. `quick-terminal-size = 75%, 50%`) <details>08c9661683/include/ghostty.h (L460-L480)08c9661683/macos/Sources/Ghostty/Ghostty.Config.swift (L507-L510)</details> Almost all mismatches seemed to be downstream of `tag` (for `primary` and/or `secondary` `Size`) being an unexpected value for the `ghostty_quick_terminal_size_tag_e` enum type, which led runtime execution to fall into the `default` branch in this `switch` control flow.08c9661683/macos/Sources/Features/QuickTerminal/QuickTerminalSize.swift (L27-L38)Looking at `src/config/CApi.zig`, `src/config/c_get.zig`, `src/config/Config.zig`, and [Zig documentation for `extern enum`](https://ziglang.org/documentation/master/#extern-enum), led me to conclude that the crux of the issue is lack of guaranteed C ABI compatibility for `Tag`08c9661683/src/config/Config.zig (L7986-L7990)08c9661683/include/ghostty.h (L461-L465)Further research revealed that based on C language spec alone, one cannot assume a fixed width for `enum` and that the behaviour is compiler dependant. But given how the Zig documentation suggests using `enum(c_int)` for C-ABI-compatible enums and extern enums + comments elsewhere in `Config.zig` suggesting that `enum(c_int)` was chosen for extern compatibility,08c9661683/src/config/Config.zig (L4877)I think that this fix (changing `enum(u8)` to `enum(c_int)`) is likely the most straightforward and appropriate one.
commit
049b8826f6
|
|
@ -33,6 +33,7 @@ struct QuickTerminalSize {
|
|||
case GHOSTTY_QUICK_TERMINAL_SIZE_PIXELS:
|
||||
self = .pixels(cStruct.value.pixels)
|
||||
default:
|
||||
assertionFailure()
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7987,7 +7987,8 @@ pub const QuickTerminalSize = struct {
|
|||
tag: Tag,
|
||||
value: Value,
|
||||
|
||||
pub const Tag = enum(u8) { none, percentage, pixels };
|
||||
/// c_int because it needs to be extern compatible
|
||||
pub const Tag = enum(c_int) { none, percentage, pixels };
|
||||
|
||||
pub const Value = extern union {
|
||||
percentage: f32,
|
||||
|
|
|
|||
Loading…
Reference in New Issue