diff --git a/crates/ghostty-cli/src/list_themes.rs b/crates/ghostty-cli/src/list_themes.rs index 1a9e06c..dcb612e 100644 --- a/crates/ghostty-cli/src/list_themes.rs +++ b/crates/ghostty-cli/src/list_themes.rs @@ -233,6 +233,8 @@ pub fn run(args: &[impl AsRef<[u8]>]) -> Result { crate::parse_options(&mut opts, args)?; let stdout = std::io::stdout(); + // Zig's stderr is a 4096 byte buffer that it never flushes (the messages + // below are lost). That is an upstream bug: we flush when it is dropped. let mut stderr = crate::zig_writer::ZigWriter::new(crate::zig_writer::stderr_sink(), 4096); let resources_dir = resources_dir(); diff --git a/crates/ghostty-cli/src/zig_writer.rs b/crates/ghostty-cli/src/zig_writer.rs index f8bb532..b0737f5 100644 --- a/crates/ghostty-cli/src/zig_writer.rs +++ b/crates/ghostty-cli/src/zig_writer.rs @@ -4,6 +4,13 @@ //! file when it overflows or when the action calls `flush`, so two writers on //! the same file (as in `+show-face`) interleave in flush order, not call //! order. This type keeps that ordering. +//! +//! Two upstream data-loss behaviours are deliberately NOT replicated (see +//! `docs/rewrite/notes/final-Z17.md`): Zig's positional `pwrite` at the +//! writer's own offset 0 (writers overwrite each other and existing content +//! when stdout is a regular file) and buffers that are never flushed. Data is +//! always appended at the current end and anything still buffered is flushed +//! when the writer is dropped. #[cfg(unix)] use std::os::fd::AsFd; @@ -69,7 +76,7 @@ impl ZigWriter { } } - /// `File.Writer.end`. + /// `File.Writer.end`: a flush (Zig also truncates; not replicated). pub fn end(&mut self) -> io::Result<()> { self.flush() } @@ -91,6 +98,12 @@ impl ZigWriter { } } +impl Drop for ZigWriter { + fn drop(&mut self) { + let _ = self.flush(); + } +} + impl Write for ZigWriter { fn write(&mut self, data: &[u8]) -> io::Result { self.write_all(data)?; @@ -154,4 +167,32 @@ mod tests { w.write_all(b"h").unwrap(); assert_eq!(&*mem.0.borrow(), b"abcdefg"); } + + #[cfg(unix)] + #[test] + fn regular_file_output_is_complete_and_appended() { + let mut tmp = tempfile::tempfile().unwrap(); + std::io::Write::write_all(&mut tmp, b"pre\n").unwrap(); + let sink: Sink = Rc::new(RefCell::new(Target::Fd(tmp.try_clone().unwrap()))); + let mut a = ZigWriter::new(sink.clone(), 1024); + let mut b = ZigWriter::new(sink, 1024); + a.write_all(b"0123456789\n").unwrap(); + b.write_all(b"abc\n").unwrap(); + a.flush().unwrap(); + b.flush().unwrap(); + let mut out = String::new(); + std::io::Seek::rewind(&mut tmp).unwrap(); + std::io::Read::read_to_string(&mut tmp, &mut out).unwrap(); + assert_eq!(out, "pre\n0123456789\nabc\n"); + } + + #[test] + fn unflushed_buffer_is_flushed_on_drop() { + let mem = Mem::default(); + let sink: Sink = stream_sink(mem.clone()); + let mut w = ZigWriter::new(sink, 4096); + w.write_all(b"kept").unwrap(); + drop(w); + assert_eq!(&*mem.0.borrow(), b"kept"); + } } diff --git a/docs/rewrite/notes/final-Z17.md b/docs/rewrite/notes/final-Z17.md new file mode 100644 index 0000000..9ffe449 --- /dev/null +++ b/docs/rewrite/notes/final-Z17.md @@ -0,0 +1,30 @@ +# Z17: RV14 findings + +1. `+show-face --cp` is `Option` (new `ghostty_config::cli_args::U21`, Zig `u21`); out of range gives an + `invalid value` diagnostic and exit 1 (no panic). Audit of Zig int-typed CLI/config fields against Rust + (Config.zig, cli/*.zig, nested types): all other widths already match. +2. `--string` is `Option` (raw bytes at option parse). Invalid UTF-8 is detected after the `--cp` + lookup: `Unable to parse string as unicode: error.InvalidUtf8` (no newline), exit 1. +3. New `crates/ghostty-cli/src/zig_writer.rs` models `File.writer(&buf)`: fixed buffer, drain-on-overflow, + explicit flush, so stdout/stderr interleave in Zig's flush order (`+show-face` creates both writers on + stdout; stdout is flushed first, then stderr; verified against the oracle on pipes). Anything still + buffered is flushed on drop. Applied to every Zig action using `File.writer`: show-face, help (+ any + `--help`), version, validate-config, show-config, list-actions, list-keybinds (plain), list-fonts, + list-themes, ssh-cache, crash-report, edit-config, new-window. +4. Windows basename: already Zig 0.15.2 (colon only at byte index 1); `λ:theme` stays whole. Added test. +5. `ghostty_os::osbytes::zig_is_absolute{,_posix,_windows}` + `EncodedOs::is_zig_absolute` (0.15.2 + `isAbsolute`: `\x`, `/x`, `X:\`, `X:/`). Used by theme open, config path expand, file_load, Config + asserts, surface mouse path, GTK bell path (every Zig `isAbsolute` site). +6. `xev::available(IoUring)` is exactly `IoUring::new(256).is_ok()`. The termio reactor now handles a ring + without `EXT_ARG` by using `IORING_OP_TIMEOUT` (like libxev timers); test forces that path. + +## Upstream bugs fixed (not replicated) + +- Positional output overwrite: `std.fs.File.writer` (Zig 0.15.2 `fs/File.zig` `Writer.init`/`drain`) + `pwrite`s at the writer's own offset starting at 0. With stdout a regular file, `+show-face` + (`cli/show_face.zig:68-73`, two writers on `File.stdout()`) overwrites its own output, and any action + (`cli/help.zig:34`, `cli/validate_config.zig:43`, ...) overwrites pre-existing file content; `Writer.end` + (`validate_config.zig:46`, `show_config.zig:84`) also truncates it. Rust always appends at the current end. +- Never-flushed buffer: `cli/list_themes.zig:124-126,129-131,171` writes "Could not find the Ghostty + resources directory" / "No themes found..." to a 4096 byte stderr buffer that is never flushed, so the + messages are lost. Rust flushes it when the action ends.