fix: last RV11 gaps - bench enum args, theme NUL paths, Zig error names

Resolves the 3 remaining gaps in docs/rewrite/notes/review-rv11-codex.md
(final-Z14.md):
- ghostty-bench validates every occurrence of enum options in order
  (Args::take_enum; empty value resets) like cli/args.zig, so
  `--mode=bad --mode=noop` fails as in Zig.
- Theme paths with an interior NUL open the prefix like Zig's C-string
  path handling (verified against /usr/bin/ghostty).
- Filesystem diagnostics print Zig's error names (error.FileNotFound,
  NotDir, AccessDenied, IsDir, NameTooLong, SymLinkLoop, ...) via a
  shared ghostty_os::zig_error mapping used by theme/path/config-file/
  file_load/edit-config/list-themes; messages match the oracle.

Test Plan:
- cargo test --workspace --no-fail-fast
- just check-macos (both), just check-windows, just check-wasm
This commit is contained in:
ddidderr committed 2026-10-12 13:54:15 +02:00
1 parent 64aa8b6793
commit 4bbe0d7fe9
12 files changed
+215 -89

No files matched your search

+36
View File
@@ -122,6 +122,27 @@ impl Args {
}) })
} }
/// Take an enum value (`parseIntoField` on an enum): every occurrence is
/// validated against `variants` in order (a bad value is an error even if
/// a later one is fine); an empty value resets to `default`.
pub fn take_enum<T: Copy>(
&mut self,
key: &str,
default: T,
variants: &[(&str, T)],
) -> Result<T, String> {
self.apply(key, default, |v| {
let v = v.ok_or_else(|| format!("option --{key} requires a value"))?;
let bad = || format!("invalid value for --{key}: {}", v.to_string_lossy());
let v = v.to_str().ok_or_else(bad)?;
variants
.iter()
.find(|(n, _)| *n == v)
.map(|(_, t)| *t)
.ok_or_else(bad)
})
}
/// Take a file path value (any bytes are allowed). /// Take a file path value (any bytes are allowed).
pub fn take_opt_path(&mut self, key: &str) -> Result<Option<PathBuf>, String> { pub fn take_opt_path(&mut self, key: &str) -> Result<Option<PathBuf>, String> {
self.apply(key, None, |v| { self.apply(key, None, |v| {
@@ -197,6 +218,21 @@ mod tests {
assert!(a.take("n", 0u16).is_err()); assert!(a.take("n", 0u16).is_err());
} }
#[test]
fn take_enum_validates_every_occurrence() {
let t = |v: &[&str]| {
Args::parse(v.iter().map(OsString::from))
.unwrap()
.take_enum("mode", 0u8, &[("noop", 0), ("table", 1)])
};
assert!(t(&["--mode=bad", "--mode=noop"]).is_err());
assert!(t(&["--mode=noop", "--mode=bad"]).is_err());
assert!(t(&["--mode=bad", "--mode="]).is_err());
assert_eq!(t(&["--mode=table", "--mode="]).unwrap(), 0);
assert_eq!(t(&["--mode=noop", "--mode=table"]).unwrap(), 1);
assert_eq!(t(&[]).unwrap(), 0);
}
#[test] #[test]
fn duplicates_apply_in_order_and_first_error_wins() { fn duplicates_apply_in_order_and_first_error_wins() {
let a = |v: &[&str]| Args::parse(v.iter().map(OsString::from)).unwrap(); let a = |v: &[&str]| Args::parse(v.iter().map(OsString::from)).unwrap();
+40 -28
View File
@@ -104,11 +104,7 @@ impl TerminalStream {
let rows = args.take("terminal-rows", 80u16)?; let rows = args.take("terminal-rows", 80u16)?;
let cols = args.take("terminal-cols", 120u16)?; let cols = args.take("terminal-cols", 120u16)?;
let data = args.take_opt_path("data")?; let data = args.take_opt_path("data")?;
let full = match args.take_opt_string("handler")?.as_deref() { let full = args.take_enum("handler", false, &[("print", false), ("full", true)])?;
None | Some("print") => false,
Some("full") => true,
Some(m) => return Err(format!("invalid value for --handler: {m}")),
};
let t = Terminal::new(TermOptions::new(cols, rows)).map_err(|e| format!("{e:?}"))?; let t = Terminal::new(TermOptions::new(cols, rows)).map_err(|e| format!("{e:?}"))?;
Ok(TerminalStream { Ok(TerminalStream {
data: Data::new(data), data: Data::new(data),
@@ -259,13 +255,16 @@ pub struct CodepointWidth {
impl CodepointWidth { impl CodepointWidth {
pub fn create(args: &mut Args) -> std::result::Result<Self, String> { pub fn create(args: &mut Args) -> std::result::Result<Self, String> {
let mode = match args.take_opt_string("mode")?.as_deref() { let mode = args.take_enum(
None | Some("noop") => WidthMode::Noop, "mode",
Some("wcwidth") => WidthMode::Wcwidth, WidthMode::Noop,
Some("simd") => WidthMode::Simd, &[
Some("table") => WidthMode::Table, ("noop", WidthMode::Noop),
Some(m) => return Err(format!("invalid value for --mode: {m}")), ("wcwidth", WidthMode::Wcwidth),
}; ("simd", WidthMode::Simd),
("table", WidthMode::Table),
],
)?;
Ok(CodepointWidth { Ok(CodepointWidth {
data: Data::new(args.take_opt_path("data")?), data: Data::new(args.take_opt_path("data")?),
mode, mode,
@@ -384,11 +383,7 @@ pub struct GraphemeBreak {
impl GraphemeBreak { impl GraphemeBreak {
pub fn create(args: &mut Args) -> std::result::Result<Self, String> { pub fn create(args: &mut Args) -> std::result::Result<Self, String> {
let noop = match args.take_opt_string("mode")?.as_deref() { let noop = args.take_enum("mode", false, &[("noop", true), ("table", false)])?;
None | Some("table") => false,
Some("noop") => true,
Some(m) => return Err(format!("invalid value for --mode: {m}")),
};
Ok(GraphemeBreak { Ok(GraphemeBreak {
data: Data::new(args.take_opt_path("data")?), data: Data::new(args.take_opt_path("data")?),
noop, noop,
@@ -473,11 +468,11 @@ pub fn is_symbol_computed(cp: u32) -> bool {
impl IsSymbol { impl IsSymbol {
pub fn create(args: &mut Args) -> std::result::Result<Self, String> { pub fn create(args: &mut Args) -> std::result::Result<Self, String> {
let mode = match args.take_opt_string("mode")?.as_deref() { let mode = args.take_enum(
None | Some("uucode") => SymbolMode::Uucode, "mode",
Some("table") => SymbolMode::Table, SymbolMode::Uucode,
Some(m) => return Err(format!("invalid value for --mode: {m}")), &[("uucode", SymbolMode::Uucode), ("table", SymbolMode::Table)],
}; )?;
Ok(IsSymbol { Ok(IsSymbol {
data: Data::new(args.take_opt_path("data")?), data: Data::new(args.take_opt_path("data")?),
mode, mode,
@@ -536,12 +531,15 @@ pub struct ScreenClone {
impl ScreenClone { impl ScreenClone {
pub fn create(args: &mut Args) -> std::result::Result<Self, String> { pub fn create(args: &mut Args) -> std::result::Result<Self, String> {
let mode = match args.take_opt_string("mode")?.as_deref() { let mode = args.take_enum(
None | Some("clone") => CloneMode::Clone, "mode",
Some("noop") => CloneMode::Noop, CloneMode::Clone,
Some("render") => CloneMode::Render, &[
Some(m) => return Err(format!("invalid value for --mode: {m}")), ("clone", CloneMode::Clone),
}; ("noop", CloneMode::Noop),
("render", CloneMode::Render),
],
)?;
let rows = args.take("terminal-rows", 80u16)?; let rows = args.take("terminal-rows", 80u16)?;
let cols = args.take("terminal-cols", 120u16)?; let cols = args.take("terminal-cols", 120u16)?;
let data = args.take_opt_path("data")?; let data = args.take_opt_path("data")?;
@@ -718,6 +716,20 @@ mod tests {
let _ = std::fs::remove_file(p); let _ = std::fs::remove_file(p);
} }
#[test]
fn duplicate_bad_enum_modes_fail() {
let mk = |v: &[&str]| Args::parse(v.iter().map(|s| (*s).to_owned())).unwrap();
for v in [
&["--mode=bad", "--mode=noop"][..],
&["--mode=bad", "--mode="][..],
] {
assert!(CodepointWidth::create(&mut mk(v)).is_err());
assert!(GraphemeBreak::create(&mut mk(v)).is_err());
assert!(IsSymbol::create(&mut mk(v)).is_err());
assert!(ScreenClone::create(&mut mk(v)).is_err());
}
}
#[test] #[test]
fn is_symbol_modes_run() { fn is_symbol_modes_run() {
let p = sample_file("sym"); let p = sample_file("sym");
@@ -40,11 +40,14 @@ fn run() -> Result<(), String> {
} }
"osc" => { "osc" => {
let p_valid = args.take("p-valid", 0.5f64)?; let p_valid = args.take("p-valid", 0.5f64)?;
let style = match args.take_opt_string("style")?.as_deref() { let style = args.take_enum(
None | Some("streaming") => OscStyle::Streaming, "style",
Some("parser") => OscStyle::Parser, OscStyle::Streaming,
Some(s) => return Err(format!("invalid value for --style: {s}")), &[
}; ("streaming", OscStyle::Streaming),
("parser", OscStyle::Parser),
],
)?;
args.finish()?; args.finish()?;
run_osc(&mut out, &mut rng, p_valid, style) run_osc(&mut out, &mut rng, p_valid, style)
} }
+1 -5
View File
@@ -122,11 +122,7 @@ fn exec_editor(stderr: &mut dyn Write, path: &EncodedOs) -> Result<u8, Error> {
.exec(); .exec();
// If we reached this point then exec failed. // If we reached this point then exec failed.
let err = match err.kind() { let err = ghostty_os::zig_error::io_error_name(&err);
std::io::ErrorKind::NotFound => "error.FileNotFound".to_string(),
std::io::ErrorKind::PermissionDenied => "error.AccessDenied".to_string(),
_ => err.to_string(),
};
let msg = format!( let msg = format!(
"Failed to execute the editor. Error code={err}.\n\ "Failed to execute the editor. Error code={err}.\n\
\n\ \n\
@@ -1396,11 +1396,9 @@ impl Preview {
fn error_name(e: &ghostty_config::config::Error) -> String { fn error_name(e: &ghostty_config::config::Error) -> String {
use ghostty_config::config::Error; use ghostty_config::config::Error;
match e { match e {
Error::Io(io) => match io.kind() { Error::Io(io) => ghostty_os::zig_error::io_error_name(io)
std::io::ErrorKind::NotFound => "FileNotFound".to_string(), .trim_start_matches("error.")
std::io::ErrorKind::PermissionDenied => "AccessDenied".to_string(), .to_owned(),
_ => io.to_string(),
},
e => e.to_string(), e => e.to_string(),
} }
} }
+2 -36
View File
@@ -3752,7 +3752,7 @@ crate::config_struct! {
/// Errors from loading configuration. /// Errors from loading configuration.
#[derive(Debug, thiserror::Error)] #[derive(Debug, thiserror::Error)]
pub enum Error { pub enum Error {
#[error("{0}")] #[error("{}", ghostty_os::zig_error::io_error_name(.0))]
Io(#[from] std::io::Error), Io(#[from] std::io::Error),
#[error("{0}")] #[error("{0}")]
Arg(#[from] ArgError), Arg(#[from] ArgError),
@@ -4273,7 +4273,7 @@ impl Config {
message: format!( message: format!(
"error opening config-file {}: {}", "error opening config-file {}: {}",
path.as_encoded(), path.as_encoded(),
zig_io_error_name(&err) ghostty_os::zig_error::io_error_name(&err)
), ),
..Diagnostic::default() ..Diagnostic::default()
}; };
@@ -4806,40 +4806,6 @@ impl Iterator for ChangeIterator<'_> {
} }
} }
/// Name of an IO error like Zig's `{}` formatting of an error value
/// (`error.FileNotFound`), used in user-visible diagnostics.
fn zig_io_error_name(err: &std::io::Error) -> String {
use std::io::ErrorKind;
#[cfg(unix)]
if let Some(errno) = err.raw_os_error() {
let name = match errno {
2 => Some("FileNotFound"),
13 => Some("AccessDenied"),
1 => Some("PermissionDenied"),
21 => Some("IsDir"),
20 => Some("NotDir"),
40 => Some("SymLinkLoop"),
36 => Some("NameTooLong"),
24 => Some("ProcessFdQuotaExceeded"),
23 => Some("SystemFdQuotaExceeded"),
12 => Some("SystemResources"),
19 => Some("NoDevice"),
16 => Some("DeviceBusy"),
26 => Some("FileBusy"),
30 => Some("ReadOnlyFileSystem"),
_ => None,
};
if let Some(name) = name {
return format!("error.{name}");
}
}
match err.kind() {
ErrorKind::NotFound => "error.FileNotFound".to_string(),
ErrorKind::PermissionDenied => "error.AccessDenied".to_string(),
_ => "error.Unexpected".to_string(),
}
}
fn file_kind_name(meta: &std::fs::Metadata) -> &'static str { fn file_kind_name(meta: &std::fs::Metadata) -> &'static str {
let ft = meta.file_type(); let ft = meta.file_type();
if ft.is_dir() { if ft.is_dir() {
+8 -3
View File
@@ -170,15 +170,20 @@ pub fn open(path: &EncodedOs) -> Result<File, OpenFileError> {
OpenFileError::FileNotFound OpenFileError::FileNotFound
} else { } else {
log::warn!( log::warn!(
"unexpected file open error path={} err={err}", "unexpected file open error path={} err={}",
real.display() real.display(),
ghostty_os::zig_error::io_error_name(&err)
); );
OpenFileError::FileOpenFailed OpenFileError::FileOpenFailed
} }
})?; })?;
let meta = file.metadata().map_err(|err| { let meta = file.metadata().map_err(|err| {
log::warn!("error getting file stat path={} err={err}", real.display()); log::warn!(
"error getting file stat path={} err={}",
real.display(),
ghostty_os::zig_error::io_error_name(&err)
);
OpenFileError::FileOpenFailed OpenFileError::FileOpenFailed
})?; })?;
if !meta.is_file() { if !meta.is_file() {
+5 -1
View File
@@ -164,7 +164,11 @@ impl Path {
} }
Err(err) => { Err(err) => {
diags.append(Diagnostic { diags.append(Diagnostic {
message: format!("error resolving file path {}: {err}", path.as_encoded()), message: format!(
"error resolving file path {}: {}",
path.as_encoded(),
ghostty_os::zig_error::io_error_name(&err)
),
..Diagnostic::default() ..Diagnostic::default()
}); });
+43 -6
View File
@@ -128,7 +128,13 @@ pub(crate) fn special_kind_name(_: std::fs::FileType) -> &'static str {
fn check_regular(file: &File, theme: &str, diags: &mut DiagnosticList) -> bool { fn check_regular(file: &File, theme: &str, diags: &mut DiagnosticList) -> bool {
match file.metadata() { match file.metadata() {
Err(err) => { Err(err) => {
diag(diags, format!("not reading theme from \"{theme}\": {err}")); diag(
diags,
format!(
"not reading theme from \"{theme}\": {}",
ghostty_os::zig_error::io_error_name(&err)
),
);
false false
} }
Ok(m) if !m.is_file() => { Ok(m) if !m.is_file() => {
@@ -197,7 +203,7 @@ pub fn open(
let mut it = LocationIterator::new(resources_dir); let mut it = LocationIterator::new(resources_dir);
while let Some((_, dir)) = it.next_location() { while let Some((_, dir)) = it.next_location() {
let path = EncodedOs::from_path(&dir.to_path().ok()?.join(&theme_path)); let path = EncodedOs::from_path(&dir.to_path().ok()?.join(&theme_path));
match path.to_path().and_then(File::open) { match path.to_syscall_path().and_then(File::open) {
Ok(file) => { Ok(file) => {
if !check_regular(&file, theme_text, diags) { if !check_regular(&file, theme_text, diags) {
return None; return None;
@@ -211,8 +217,9 @@ pub fn open(
diag( diag(
diags, diags,
format!( format!(
"failed to load theme \"{theme_text}\" from the file \"{}\": {err}", "failed to load theme \"{theme_text}\" from the file \"{}\": {}",
path.as_encoded() path.as_encoded(),
ghostty_os::zig_error::io_error_name(&err)
), ),
); );
return None; return None;
@@ -241,7 +248,7 @@ pub fn open(
/// then messages will be appended to the given error list and None is /// then messages will be appended to the given error list and None is
/// returned. /// returned.
pub fn open_absolute(theme: &EncodedOs, diags: &mut DiagnosticList) -> Option<File> { pub fn open_absolute(theme: &EncodedOs, diags: &mut DiagnosticList) -> Option<File> {
let path = theme.to_path(); let path = theme.to_syscall_path();
let theme = theme.as_encoded(); let theme = theme.as_encoded();
match path.and_then(File::open) { match path.and_then(File::open) {
Ok(f) => Some(f), Ok(f) => Some(f),
@@ -254,7 +261,10 @@ pub fn open_absolute(theme: &EncodedOs, diags: &mut DiagnosticList) -> Option<Fi
} else { } else {
diag( diag(
diags, diags,
format!("failed to load theme from the path \"{theme}\": {err}"), format!(
"failed to load theme from the path \"{theme}\": {}",
ghostty_os::zig_error::io_error_name(&err)
),
); );
} }
None None
@@ -282,6 +292,33 @@ mod tests {
); );
} }
#[cfg(unix)]
#[test]
fn theme_path_errors_use_zig_names() {
let mut d = DiagnosticList::default();
assert!(open(&e("/etc/passwd/x"), None, &mut d).is_none());
assert_eq!(
d.items()[0].message,
"failed to load theme from the path \"/etc/passwd/x\": error.NotDir"
);
}
#[cfg(unix)]
#[test]
fn theme_absolute_interior_nul_opens_prefix() {
let dir = std::env::temp_dir().join(format!("gtheme-nul-{}", std::process::id()));
let _ = std::fs::create_dir_all(&dir);
let f = dir.join("t");
std::fs::write(&f, "background = #112233\n").unwrap();
let mut bytes = f.as_os_str().as_encoded_bytes().to_vec();
bytes.extend_from_slice(b"\0tail");
let t = EncodedOs::from_bytes(&bytes);
let mut d = DiagnosticList::default();
assert!(open(&t, None, &mut d).is_some());
assert_eq!(d.items().len(), 0);
let _ = std::fs::remove_dir_all(&dir);
}
#[test] #[test]
fn theme_absolute_missing_and_dir() { fn theme_absolute_missing_and_dir() {
let mut d = DiagnosticList::default(); let mut d = DiagnosticList::default();
+1
View File
@@ -123,6 +123,7 @@ pub use unsupported::homedir::expand_home;
pub use unsupported::{ResourcesDir, homedir, is_flatpak, passwd, resources_dir, xdg}; pub use unsupported::{ResourcesDir, homedir, is_flatpak, passwd, resources_dir, xdg};
#[cfg(not(target_arch = "wasm32"))] #[cfg(not(target_arch = "wasm32"))]
pub mod xdg; pub mod xdg;
pub mod zig_error;
#[cfg(test)] #[cfg(test)]
pub(crate) mod testutil; pub(crate) mod testutil;
+63
View File
@@ -0,0 +1,63 @@
//! `std::io::Error` -> Zig error name (`{}` of an error value, e.g.
//! `error.FileNotFound`), following Zig std's posix errno mapping for the
//! calls Ghostty makes (`openat`, `fstat`, `realpath`, `execve`, ...).
/// Zig error name (without the `error.` prefix) for a Linux errno.
#[must_use]
pub fn errno_name(errno: i32) -> &'static str {
match errno {
1 => "PermissionDenied", // EPERM
2 => "FileNotFound", // ENOENT
3 => "ProcessNotFound", // ESRCH
6 | 19 => "NoDevice", // ENXIO, ENODEV
11 => "WouldBlock", // EAGAIN
12 => "SystemResources", // ENOMEM
13 => "AccessDenied", // EACCES
16 => "DeviceBusy", // EBUSY
17 => "PathAlreadyExists", // EEXIST
20 => "NotDir", // ENOTDIR
21 => "IsDir", // EISDIR
23 => "SystemFdQuotaExceeded", // ENFILE
24 => "ProcessFdQuotaExceeded", // EMFILE
26 => "FileBusy", // ETXTBSY
27 | 75 => "FileTooBig", // EFBIG, EOVERFLOW
28 => "NoSpaceLeft", // ENOSPC
30 => "ReadOnlyFileSystem", // EROFS
36 => "NameTooLong", // ENAMETOOLONG
40 => "SymLinkLoop", // ELOOP
84 => "InvalidWtf8", // EILSEQ
95 => "FileLocksNotSupported", // EOPNOTSUPP
_ => "Unexpected",
}
}
/// `error.Name` text for an I/O error.
#[must_use]
pub fn io_error_name(err: &std::io::Error) -> String {
if let Some(errno) = err.raw_os_error() {
return format!("error.{}", errno_name(errno));
}
match err.kind() {
std::io::ErrorKind::NotFound => "error.FileNotFound".into(),
std::io::ErrorKind::PermissionDenied => "error.AccessDenied".into(),
std::io::ErrorKind::InvalidInput => "error.BadPathName".into(),
_ => "error.Unexpected".into(),
}
}
#[cfg(all(test, unix))]
mod tests {
use super::*;
#[test]
fn names() {
let n = |e| io_error_name(&std::io::Error::from_raw_os_error(e));
assert_eq!(n(2), "error.FileNotFound");
assert_eq!(n(20), "error.NotDir");
assert_eq!(n(13), "error.AccessDenied");
assert_eq!(n(21), "error.IsDir");
assert_eq!(n(36), "error.NameTooLong");
assert_eq!(n(40), "error.SymLinkLoop");
assert_eq!(n(9999), "error.Unexpected");
}
}
+5
View File
@@ -0,0 +1,5 @@
# Z14 (RV11 gaps)
1. ghostty-bench: `Args::take_enum` validates every occurrence of an enum option in order (empty value resets to default), used by codepoint-width, grapheme-break, is-symbol, screen-clone, terminal-stream `--handler` and synthetic `+osc --style`. Tests in args.rs / benches.rs.
2. Theme open (named and absolute) uses `to_syscall_path` so an interior NUL truncates the path like Zig's C string. Test `theme_absolute_interior_nul_opens_prefix`; checked against /usr/bin/ghostty (no diagnostics, loads).
3. New `ghostty_os::zig_error::{errno_name, io_error_name}` (Zig std posix errno mapping). Used by theme.rs, path.rs, config.rs (config-file open, `config::Error::Io` Display), file_load.rs logs, edit_config.rs, list_themes preview. Verified identical to oracle for FileNotFound, NotDir, AccessDenied, IsDir ("it is a directory" path), NameTooLong, SymLinkLoop.