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:
1 parent
ce9373aa7a
commit
f9b333545a
12 files changed
+215
-89
No files matched your search
@@ -3752,7 +3752,7 @@ crate::config_struct! {
|
||||
/// Errors from loading configuration.
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub enum Error {
|
||||
#[error("{0}")]
|
||||
#[error("{}", ghostty_os::zig_error::io_error_name(.0))]
|
||||
Io(#[from] std::io::Error),
|
||||
#[error("{0}")]
|
||||
Arg(#[from] ArgError),
|
||||
@@ -4273,7 +4273,7 @@ impl Config {
|
||||
message: format!(
|
||||
"error opening config-file {}: {}",
|
||||
path.as_encoded(),
|
||||
zig_io_error_name(&err)
|
||||
ghostty_os::zig_error::io_error_name(&err)
|
||||
),
|
||||
..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 {
|
||||
let ft = meta.file_type();
|
||||
if ft.is_dir() {
|
||||
|
||||
@@ -170,15 +170,20 @@ pub fn open(path: &EncodedOs) -> Result<File, OpenFileError> {
|
||||
OpenFileError::FileNotFound
|
||||
} else {
|
||||
log::warn!(
|
||||
"unexpected file open error path={} err={err}",
|
||||
real.display()
|
||||
"unexpected file open error path={} err={}",
|
||||
real.display(),
|
||||
ghostty_os::zig_error::io_error_name(&err)
|
||||
);
|
||||
OpenFileError::FileOpenFailed
|
||||
}
|
||||
})?;
|
||||
|
||||
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
|
||||
})?;
|
||||
if !meta.is_file() {
|
||||
|
||||
@@ -164,7 +164,11 @@ impl Path {
|
||||
}
|
||||
Err(err) => {
|
||||
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()
|
||||
});
|
||||
|
||||
|
||||
@@ -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 {
|
||||
match file.metadata() {
|
||||
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
|
||||
}
|
||||
Ok(m) if !m.is_file() => {
|
||||
@@ -197,7 +203,7 @@ pub fn open(
|
||||
let mut it = LocationIterator::new(resources_dir);
|
||||
while let Some((_, dir)) = it.next_location() {
|
||||
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) => {
|
||||
if !check_regular(&file, theme_text, diags) {
|
||||
return None;
|
||||
@@ -211,8 +217,9 @@ pub fn open(
|
||||
diag(
|
||||
diags,
|
||||
format!(
|
||||
"failed to load theme \"{theme_text}\" from the file \"{}\": {err}",
|
||||
path.as_encoded()
|
||||
"failed to load theme \"{theme_text}\" from the file \"{}\": {}",
|
||||
path.as_encoded(),
|
||||
ghostty_os::zig_error::io_error_name(&err)
|
||||
),
|
||||
);
|
||||
return None;
|
||||
@@ -241,7 +248,7 @@ pub fn open(
|
||||
/// then messages will be appended to the given error list and None is
|
||||
/// returned.
|
||||
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();
|
||||
match path.and_then(File::open) {
|
||||
Ok(f) => Some(f),
|
||||
@@ -254,7 +261,10 @@ pub fn open_absolute(theme: &EncodedOs, diags: &mut DiagnosticList) -> Option<Fi
|
||||
} else {
|
||||
diag(
|
||||
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
|
||||
@@ -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]
|
||||
fn theme_absolute_missing_and_dir() {
|
||||
let mut d = DiagnosticList::default();
|
||||
|
||||
Reference in new issue
Block a user