From 16eb1fba6140ef8a81dd42a621911335ae031d9f Mon Sep 17 00:00:00 2001 From: ddidderr Date: Mon, 12 Oct 2026 12:45:55 +0200 Subject: [PATCH] fix: use Zig's numeric parsing in protocol and escape-sequence parsers Rust's str::parse accepts and rejects different inputs than Zig's std.fmt.parseInt/parseFloat (underscores, base prefixes, signs). The config layer already used the Zig ports; now the protocol parsers do too (call site -> Zig reference -> status in final-Z12.md): - zig_fmt moves into the leaf crate ghostty-zigfmt (ghostty-vt cannot depend on ghostty-os); ghostty-os re-exports it unchanged. - ghostty-vt: kitty graphics keys, kitty text sizing, OSC 4/5/10-19 color indices, hex color components and intensities, OSC 9 sleep and progress, OSC 133 exit codes, OSC 3008 fields, kitty color palette. - ghostty-terminal: tmux layout, format variables and pane tabs. - ghostty-os: URI ports and %XX decoding; ghostty-bench CLI arguments. Expected values were cross-checked against the real Zig toolchain. Test Plan: - cargo test --workspace --no-fail-fast - just check-macos (both), just check-windows, just check-wasm --- Cargo.lock | 7 +++ Cargo.toml | 1 + crates/ghostty-bench/src/args.rs | 48 +++++++++++++-- crates/ghostty-os/Cargo.toml | 1 + crates/ghostty-os/src/lib.rs | 2 +- crates/ghostty-os/src/uri.rs | 38 +++++++----- crates/ghostty-terminal/Cargo.toml | 1 + crates/ghostty-terminal/src/tmux/layout.rs | 35 ++++++----- crates/ghostty-terminal/src/tmux/output.rs | 51 +++++++--------- crates/ghostty-terminal/src/tmux/viewer.rs | 2 +- crates/ghostty-vt/Cargo.toml | 1 + crates/ghostty-vt/src/color.rs | 51 ++++++++-------- crates/ghostty-vt/src/kitty/color.rs | 24 ++++---- .../ghostty-vt/src/kitty/graphics_command.rs | 58 ++++++++----------- crates/ghostty-vt/src/osc/parsers/color.rs | 41 ++++++------- .../src/osc/parsers/context_signal.rs | 3 +- .../src/osc/parsers/kitty_text_sizing.rs | 38 +++++------- crates/ghostty-vt/src/osc/parsers/osc9.rs | 16 ++++- .../src/osc/parsers/semantic_prompt.rs | 13 ++++- crates/ghostty-zigfmt/Cargo.toml | 11 ++++ .../zig_fmt.rs => ghostty-zigfmt/src/lib.rs} | 2 + docs/rewrite/notes/final-Z12.md | 32 ++++++++++ 22 files changed, 290 insertions(+), 186 deletions(-) create mode 100644 crates/ghostty-zigfmt/Cargo.toml rename crates/{ghostty-os/src/zig_fmt.rs => ghostty-zigfmt/src/lib.rs} (99%) create mode 100644 docs/rewrite/notes/final-Z12.md diff --git a/Cargo.lock b/Cargo.lock index 9defede..0e5d897 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1618,6 +1618,7 @@ dependencies = [ "crossbeam-channel", "gettext-rs", "ghostty-vt", + "ghostty-zigfmt", "libc", "log", "nix 0.31.3", @@ -1698,6 +1699,7 @@ dependencies = [ "ghostty-os", "ghostty-unicode", "ghostty-vt", + "ghostty-zigfmt", "log", "onig", "png", @@ -1740,6 +1742,7 @@ name = "ghostty-vt" version = "0.1.0" dependencies = [ "base64 0.23.1", + "ghostty-zigfmt", "log", "memchr", "thiserror 2.0.21", @@ -1766,6 +1769,10 @@ dependencies = [ "temp-env", ] +[[package]] +name = "ghostty-zigfmt" +version = "0.1.0" + [[package]] name = "gio" version = "0.22.10" diff --git a/Cargo.toml b/Cargo.toml index 09b9a15..f8800ba 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,6 +56,7 @@ ghostty-font = { path = "crates/ghostty-font" } ghostty-renderer = { path = "crates/ghostty-renderer" } ghostty-termio = { path = "crates/ghostty-termio" } ghostty-os = { path = "crates/ghostty-os" } +ghostty-zigfmt = { path = "crates/ghostty-zigfmt" } ghostty-onig = { path = "crates/ghostty-onig" } ghostty-datastruct = { path = "crates/ghostty-datastruct" } ghostty-core = { path = "crates/ghostty-core" } diff --git a/crates/ghostty-bench/src/args.rs b/crates/ghostty-bench/src/args.rs index 5c88ed8..55c9d4c 100644 --- a/crates/ghostty-bench/src/args.rs +++ b/crates/ghostty-bench/src/args.rs @@ -6,11 +6,38 @@ use std::{ fs::File, io::{self, Read}, path::{Path, PathBuf}, - str::FromStr, }; use ghostty_os::osbytes::{os_bytes, os_from_bytes}; +/// Typed option value parsed like `cli/args.zig` `parseIntoField` +/// (`std.fmt.parseInt(T, v, 0)` / `std.fmt.parseFloat`). +pub trait ZigArg: Sized { + fn parse_zig(v: &[u8]) -> Option; +} + +macro_rules! zig_arg_int { + ($($t:ty),*) => {$( + impl ZigArg for $t { + fn parse_zig(v: &[u8]) -> Option { + ghostty_os::zig_fmt::parse_int::<$t>(v, 0).ok() + } + } + )*}; +} +zig_arg_int!(u8, u16, u32, u64, usize, i8, i16, i32, i64, isize); + +impl ZigArg for f64 { + fn parse_zig(v: &[u8]) -> Option { + ghostty_os::zig_fmt::parse_float::(v).ok() + } +} +impl ZigArg for f32 { + fn parse_zig(v: &[u8]) -> Option { + ghostty_os::zig_fmt::parse_float::(v).ok() + } +} + /// Parsed `--key[=value]` arguments. #[derive(Debug, Default)] pub struct Args { @@ -65,12 +92,10 @@ impl Args { } /// Take a typed value, falling back to `default`. - pub fn take(&mut self, key: &str, default: T) -> Result { + pub fn take(&mut self, key: &str, default: T) -> Result { match self.take_os(key)? { None => Ok(default), - Some(v) => v - .to_str() - .and_then(|s| s.parse().ok()) + Some(v) => T::parse_zig(&os_bytes(&v)) .ok_or_else(|| format!("invalid value for --{key}: {}", v.to_string_lossy())), } } @@ -145,6 +170,19 @@ mod tests { use super::*; + #[test] + fn take_uses_zig_parse_int_base_0() { + let args = ["--terminal-rows=0x10", "--terminal-cols=1_0", "--p=0x1p-1"]; + let mut a = Args::parse(args.map(OsString::from)).unwrap(); + assert_eq!(a.take("terminal-rows", 0u16).unwrap(), 16); + assert_eq!(a.take("terminal-cols", 0u16).unwrap(), 10); + assert_eq!(a.take("p", 0f64).unwrap(), 0.5); + let mut a = Args::parse([OsString::from("--n=+5")]).unwrap(); + assert_eq!(a.take("n", 0u16).unwrap(), 5); + let mut a = Args::parse([OsString::from("--n=-1")]).unwrap(); + assert!(a.take("n", 0u16).is_err()); + } + #[test] fn data_path_keeps_non_utf8_bytes() { let arg = OsString::from_vec(b"--data=/tmp/a\xffb".to_vec()); diff --git a/crates/ghostty-os/Cargo.toml b/crates/ghostty-os/Cargo.toml index d16526e..12bb7ed 100644 --- a/crates/ghostty-os/Cargo.toml +++ b/crates/ghostty-os/Cargo.toml @@ -5,6 +5,7 @@ edition.workspace = true license.workspace = true [dependencies] +ghostty-zigfmt.workspace = true ghostty-vt.workspace = true log = "0.4.34" thiserror = "2.0.21" diff --git a/crates/ghostty-os/src/lib.rs b/crates/ghostty-os/src/lib.rs index 479f7dd..d127f43 100644 --- a/crates/ghostty-os/src/lib.rs +++ b/crates/ghostty-os/src/lib.rs @@ -32,7 +32,7 @@ pub mod file_type; #[cfg(unix)] pub mod flatpak; pub mod osbytes; -pub mod zig_fmt; +pub use ghostty_zigfmt as zig_fmt; #[cfg(windows)] pub mod flatpak { //! Flatpak does not exist on Windows. diff --git a/crates/ghostty-os/src/uri.rs b/crates/ghostty-os/src/uri.rs index 0d75931..f7242b9 100644 --- a/crates/ghostty-os/src/uri.rs +++ b/crates/ghostty-os/src/uri.rs @@ -89,9 +89,10 @@ fn percent_decode(s: &str) -> Cow<'_, [u8]> { while i < b.len() { if b[i] == b'%' && i + 2 < b.len() - && let (Some(h), Some(l)) = (hexval(b[i + 1]), hexval(b[i + 2])) + // `std.fmt.parseInt(u8, "XX", 16)`: accepts a sign and `_`. + && let Ok(v) = crate::zig_fmt::parse_int::(&b[i + 1..i + 3], 16) { - out.push((h << 4) | l); + out.push(v); i += 3; continue; } @@ -101,15 +102,6 @@ fn percent_decode(s: &str) -> Cow<'_, [u8]> { Cow::Owned(out) } -fn hexval(c: u8) -> Option { - match c { - b'0'..=b'9' => Some(c - b'0'), - b'a'..=b'f' => Some(c - b'a' + 10), - b'A'..=b'F' => Some(c - b'A' + 10), - _ => None, - } -} - /// `std.Uri`. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Uri<'a> { @@ -217,8 +209,7 @@ fn parse_authority<'a>(authority: &'a str, uri: &mut Uri<'a>) -> Result<(), Pars // if not part of the V6 address field end_of_host = end_of_host.min(index); uri.port = Some( - authority[index + 1..] - .parse::() + crate::zig_fmt::parse_int::(&authority.as_bytes()[index + 1..], 10) .map_err(|_| ParseError::InvalidPort)?, ); } @@ -228,8 +219,7 @@ fn parse_authority<'a>(authority: &'a str, uri: &mut Uri<'a>) -> Result<(), Pars // if not part of the userinfo field end_of_host = end_of_host.min(index); uri.port = Some( - authority[index + 1..] - .parse::() + crate::zig_fmt::parse_int::(&authority.as_bytes()[index + 1..], 10) .map_err(|_| ParseError::InvalidPort)?, ); } @@ -411,6 +401,24 @@ fn is_valid_mac_address(s: &str) -> bool { mod tests { use super::*; + #[test] + fn zig_parse_int_port_and_percent() { + let u = parse("http://host:8_0/p", ParseOptions::default()).unwrap(); + assert_eq!(u.port, Some(80)); + let u = parse("http://host:+80/p", ParseOptions::default()).unwrap(); + assert_eq!(u.port, Some(80)); + assert!(parse("http://host:0x50/p", ParseOptions::default()).is_err()); + assert!(parse("http://host:65536/p", ParseOptions::default()).is_err()); + assert!(parse("http://host:-1/p", ParseOptions::default()).is_err()); + // `%XX` is decoded with `parseInt(u8, XX, 16)`: sign and nothing else. + let u = parse( + "file://localhost/a%+Ab%-0c%_1d%1_e%41", + ParseOptions::default(), + ) + .unwrap(); + assert_eq!(&*u.path.to_raw_bytes(), b"/a\x0ab\x00c%_1d%1_eA"); + } + const MAC: ParseOptions = ParseOptions { mac_address: true, raw_path: false, diff --git a/crates/ghostty-terminal/Cargo.toml b/crates/ghostty-terminal/Cargo.toml index 04abf44..5c2d9b7 100644 --- a/crates/ghostty-terminal/Cargo.toml +++ b/crates/ghostty-terminal/Cargo.toml @@ -11,6 +11,7 @@ flate2 = "1.1.10" ghostty-datastruct.workspace = true ghostty-unicode.workspace = true ghostty-vt.workspace = true +ghostty-zigfmt.workspace = true log = "0.4.34" png = "0.18.1" thiserror = "2.0.21" diff --git a/crates/ghostty-terminal/src/tmux/layout.rs b/crates/ghostty-terminal/src/tmux/layout.rs index c87d6b4..6ad5a88 100644 --- a/crates/ghostty-terminal/src/tmux/layout.rs +++ b/crates/ghostty-terminal/src/tmux/layout.rs @@ -41,22 +41,8 @@ pub enum ParseError { } fn parse_usize(s: &[u8]) -> Result { - // std.fmt.parseInt(usize, s, 10): optional '+', digits only. - let s = s.strip_prefix(b"+").unwrap_or(s); - if s.is_empty() { - return Err(ParseError::SyntaxError); - } - let mut v: usize = 0; - for &c in s { - if !c.is_ascii_digit() { - return Err(ParseError::SyntaxError); - } - v = v - .checked_mul(10) - .and_then(|v| v.checked_add(usize::from(c - b'0'))) - .ok_or(ParseError::SyntaxError)?; - } - Ok(v) + // std.fmt.parseInt(usize, s, 10) (underscores, sign rules). + ghostty_zigfmt::parse_int::(s, 10).map_err(|_| ParseError::SyntaxError) } fn index_of_any(s: &[u8], set: &[u8]) -> Option { @@ -191,6 +177,23 @@ impl Checksum { mod tests { use super::*; + #[test] + fn zig_parse_int_differences() { + // `std.fmt.parseInt(usize, _, 10)`: `_` separators, `+`/`-0` signs. + let l = p("1_0x2_0,+3,-0,4").unwrap(); + assert_eq!((l.width, l.height, l.x, l.y), (10, 20, 3, 0)); + assert_eq!(pane(&l), 4); + assert_eq!(p("1_x2,3,4,5").err(), Some(ParseError::SyntaxError)); + assert_eq!(p("0x10x2,3,4,5").err(), Some(ParseError::SyntaxError)); + assert_eq!(p("10x2,-1,4,5").err(), Some(ParseError::SyntaxError)); + assert_eq!(parse_usize(b"1__0"), Ok(10)); + assert_eq!(parse_usize(b"_1"), Err(ParseError::SyntaxError)); + assert_eq!( + parse_usize(b"99999999999999999999999"), + Err(ParseError::SyntaxError) + ); + } + fn p(s: &str) -> Result { Layout::parse(s.as_bytes()) } diff --git a/crates/ghostty-terminal/src/tmux/output.rs b/crates/ghostty-terminal/src/tmux/output.rs index 7a3bb6b..eaa7191 100644 --- a/crates/ghostty-terminal/src/tmux/output.rs +++ b/crates/ghostty-terminal/src/tmux/output.rs @@ -55,36 +55,11 @@ pub enum Value { Str(String), } -/// Equivalent of `std.fmt.parseInt(usize, s, 10)`. fn parse_int(s: &str) -> Result { - let b = s.as_bytes(); - let (neg, digits) = match b.first() { - Some(b'-') => (true, &b[1..]), - Some(b'+') => (false, &b[1..]), - _ => (false, b), - }; - if digits.is_empty() { - return Err(VariableError::InvalidCharacter); - } - let mut v: usize = 0; - let mut overflow = false; - for &c in digits { - if !c.is_ascii_digit() { - return Err(VariableError::InvalidCharacter); - } - match v - .checked_mul(10) - .and_then(|v| v.checked_add(usize::from(c - b'0'))) - { - Some(n) => v = n, - None => overflow = true, - } - } - // Negative values (other than zero) overflow an unsigned type. - if overflow || (neg && v != 0) { - return Err(VariableError::Overflow); - } - Ok(v) + ghostty_zigfmt::parse_int::(s.as_bytes(), 10).map_err(|e| match e { + ghostty_zigfmt::ParseIntError::Overflow => VariableError::Overflow, + ghostty_zigfmt::ParseIntError::InvalidCharacter => VariableError::InvalidCharacter, + }) } fn parse_prefixed(prefix: u8, value: &str) -> Result { @@ -320,6 +295,24 @@ mod tests { use super::*; + #[test] + fn zig_parse_int_differences() { + assert_eq!(parse_int("1_0"), Ok(10)); + assert_eq!(parse_int("+5"), Ok(5)); + assert_eq!(parse_int("-0"), Ok(0)); + assert_eq!(parse_int("-1"), Err(VariableError::Overflow)); + assert_eq!(parse_int("0x10"), Err(VariableError::InvalidCharacter)); + assert_eq!(parse_int("1_"), Err(VariableError::InvalidCharacter)); + assert_eq!(parse_int(""), Err(VariableError::InvalidCharacter)); + assert_eq!( + parse_int("99999999999999999999999"), + Err(VariableError::Overflow) + ); + assert_eq!(PaneId.parse("%1_2"), Ok(Value::Usize(12))); + assert_eq!(WindowId.parse("@+7"), Ok(Value::Usize(7))); + assert_eq!(WindowWidth.parse("8_0"), Ok(Value::Usize(80))); + } + fn flag(v: Variable) { assert_eq!(v.parse("1"), Ok(Value::Bool(true))); assert_eq!(v.parse("0"), Ok(Value::Bool(false))); diff --git a/crates/ghostty-terminal/src/tmux/viewer.rs b/crates/ghostty-terminal/src/tmux/viewer.rs index c3ac295..a06e3e3 100644 --- a/crates/ghostty-terminal/src/tmux/viewer.rs +++ b/crates/ghostty-terminal/src/tmux/viewer.rs @@ -976,7 +976,7 @@ impl Viewer { let tabs = data.str(Variable::PaneTabs); if !tabs.is_empty() { for tab_str in tabs.split(',') { - let Ok(col) = tab_str.parse::() else { + let Ok(col) = ghostty_zigfmt::parse_int::(tab_str.as_bytes(), 10) else { continue; }; let Some(col_cell) = to_cell(col) else { diff --git a/crates/ghostty-vt/Cargo.toml b/crates/ghostty-vt/Cargo.toml index 8bffe37..ce2a0c3 100644 --- a/crates/ghostty-vt/Cargo.toml +++ b/crates/ghostty-vt/Cargo.toml @@ -5,6 +5,7 @@ edition.workspace = true license.workspace = true [dependencies] +ghostty-zigfmt.workspace = true base64 = "0.23.1" log = "0.4.34" memchr = "2.8.3" diff --git a/crates/ghostty-vt/src/color.rs b/crates/ghostty-vt/src/color.rs index c089763..0aabb3a 100644 --- a/crates/ghostty-vt/src/color.rs +++ b/crates/ghostty-vt/src/color.rs @@ -463,29 +463,9 @@ pub struct Rgb { /// Alias matching the Zig name. pub type RGB = Rgb; -/// Zig `std.fmt.parseUnsigned(u16, value, 16)` semantics (optional leading -/// `+`; a leading `-` is only valid if the value is zero; no underscores). +/// Zig `std.fmt.parseUnsigned(u16, value, 16)`. fn parse_unsigned_hex_u16(value: &[u8]) -> Option { - let (neg, digits) = match value.first()? { - b'+' => (false, &value[1..]), - b'-' => (true, &value[1..]), - _ => (false, value), - }; - if digits.is_empty() { - return None; - } - let mut acc: u32 = 0; - for &c in digits { - let d = (c as char).to_digit(16)?; - acc = acc * 16 + d; - if acc > u16::MAX as u32 { - return None; - } - } - if neg && acc != 0 { - return None; - } - Some(acc as u16) + ghostty_zigfmt::parse_unsigned::(value, 16).ok() } impl Rgb { @@ -537,8 +517,7 @@ impl Rgb { /// /// The value should be between 0.0 and 1.0, inclusive. fn from_intensity(value: &[u8]) -> Result { - let s = std::str::from_utf8(value).map_err(|_| InvalidFormat)?; - let i: f64 = s.parse().map_err(|_| InvalidFormat)?; + let i: f64 = ghostty_zigfmt::parse_float::(value).map_err(|_| InvalidFormat)?; if !(0.0..=1.0).contains(&i) && !i.is_nan() { return Err(InvalidFormat); } @@ -854,6 +833,30 @@ mod tests { #![allow(clippy::unwrap_used)] use super::*; + // Zig `std.fmt.parseUnsigned` / `parseFloat` differences from Rust's. + #[test] + fn zig_parse_hex_u16_underscore_and_sign() { + assert_eq!(Some(0x1f), parse_unsigned_hex_u16(b"1_f")); + assert_eq!(Some(0x1f), parse_unsigned_hex_u16(b"1__f")); + assert_eq!(None, parse_unsigned_hex_u16(b"+f")); + assert_eq!(None, parse_unsigned_hex_u16(b"-0")); + assert_eq!(None, parse_unsigned_hex_u16(b"_1")); + assert_eq!(None, parse_unsigned_hex_u16(b"f_")); + assert_eq!(None, parse_unsigned_hex_u16(b"0x1")); + } + + #[test] + fn zig_parse_float_intensity() { + // hex floats and `_` separators are valid Zig floats. + assert_eq!(Ok(127), Rgb::from_intensity(b"0x1p-1")); + assert_eq!(Ok(127), Rgb::from_intensity(b"0.5_0")); + assert!(Rgb::from_intensity(b"0_.5").is_err()); + assert_eq!(Ok(127), Rgb::from_intensity(b"+.5")); + assert_eq!(Ok(255), Rgb::from_intensity(b"1.")); + assert!(Rgb::from_intensity(b"1.5").is_err()); + assert!(Rgb::from_intensity(b"").is_err()); + } + fn rgb(r: u8, g: u8, b: u8) -> Rgb { Rgb { r, g, b } } diff --git a/crates/ghostty-vt/src/kitty/color.rs b/crates/ghostty-vt/src/kitty/color.rs index e1a74f6..bdd0094 100644 --- a/crates/ghostty-vt/src/kitty/color.rs +++ b/crates/ghostty-vt/src/kitty/color.rs @@ -166,16 +166,10 @@ impl Kind { if let Some(s) = Special::from_name(key) { return Some(Kind::Special(s)); } - // Zig parseUnsigned(u8, key, 10): optional '+', digits only. - let digits = match key.first()? { - b'+' => &key[1..], - _ => key, - }; - if digits.is_empty() || !digits.iter().all(u8::is_ascii_digit) { - return None; - } - let s = std::str::from_utf8(digits).ok()?; - s.parse::().ok().map(Kind::Palette) + // Zig parseUnsigned(u8, key, 10). + ghostty_zigfmt::parse_unsigned::(key, 10) + .ok() + .map(Kind::Palette) } } @@ -233,6 +227,16 @@ mod tests { #![allow(clippy::unwrap_used)] use super::*; + #[test] + fn zig_parse_unsigned_palette_key() { + assert!(matches!(Kind::parse(b"1_0"), Some(Kind::Palette(10)))); + assert!(Kind::parse(b"+5").is_none()); + assert!(Kind::parse(b"-0").is_none()); + assert!(Kind::parse(b"0x10").is_none()); + assert!(Kind::parse(b"256").is_none()); + assert!(Kind::parse(b"1_").is_none()); + } + #[test] fn request_records_round_trip() { let mut reqs = vec![]; diff --git a/crates/ghostty-vt/src/kitty/graphics_command.rs b/crates/ghostty-vt/src/kitty/graphics_command.rs index 60ef6fb..6e51f42 100644 --- a/crates/ghostty-vt/src/kitty/graphics_command.rs +++ b/crates/ghostty-vt/src/kitty/graphics_command.rs @@ -24,45 +24,20 @@ pub enum Error { Overflow, } -/// Mirrors `std.fmt.parseInt(T, buf, 10)` for 32-bit targets: optional sign, -/// at least one digit, range checked. +/// Mirrors `std.fmt.parseInt(u32|i32, buf, 10)` (Zig `kv` value parsing), +/// including `_` digit separators and Zig's sign rules. fn parse_int(buf: &[u8], signed: bool) -> Result { - // kv_temp is at most 11 bytes so an i64 always suffices unless there - // are overflow digits, which we detect via checked arithmetic. - let (neg, digits) = match buf.first() { - Some(b'+') if buf.len() > 1 => (false, &buf[1..]), - Some(b'-') if buf.len() > 1 => (true, &buf[1..]), - _ => (false, buf), + use ghostty_zigfmt::ParseIntError; + let map = |e| match e { + ParseIntError::Overflow => Error::Overflow, + ParseIntError::InvalidCharacter => Error::InvalidCharacter, }; - if digits.is_empty() { - return Err(Error::InvalidCharacter); - } - let mut v: i64 = 0; - let mut overflow = false; - for &c in digits { - if !c.is_ascii_digit() { - return Err(Error::InvalidCharacter); - } - match v - .checked_mul(10) - .and_then(|x| x.checked_add(i64::from(c - b'0'))) - { - Some(x) => v = x, - None => overflow = true, - } - } - if overflow { - return Err(Error::Overflow); - } - if neg { - v = -v; - } if signed { - i32::try_from(v) + ghostty_zigfmt::parse_int::(buf, 10) .map(|x| x as u32) - .map_err(|_| Error::Overflow) + .map_err(map) } else { - u32::try_from(v).map_err(|_| Error::Overflow) + ghostty_zigfmt::parse_int::(buf, 10).map_err(map) } } @@ -1158,6 +1133,21 @@ mod tests { use super::*; + #[test] + fn zig_parse_int_values() { + // `_` separators, explicit `+`, negative zero for unsigned. + let c = parse("a=p,i=1_0,I=+7,r=-0,z=-1_0").unwrap(); + let Control::Display(d) = c.control else { + panic!("expected display") + }; + assert_eq!((d.image_id, d.image_number, d.rows, d.z), (10, 7, 0, -10)); + // Signed keys accept negatives, unsigned do not. + assert!(matches!(parse("a=p,i=-1"), Err(Error::Overflow))); + assert!(matches!(parse("a=p,i=0x10"), Err(Error::InvalidCharacter))); + assert!(matches!(parse("a=p,i=1_"), Err(Error::InvalidCharacter))); + assert!(matches!(parse("a=p,i=4294967296"), Err(Error::Overflow))); + } + fn parse(input: &str) -> Result { let mut p = Parser::new(); for c in input.bytes() { diff --git a/crates/ghostty-vt/src/osc/parsers/color.rs b/crates/ghostty-vt/src/osc/parsers/color.rs index 68d2e1e..c548793 100644 --- a/crates/ghostty-vt/src/osc/parsers/color.rs +++ b/crates/ghostty-vt/src/osc/parsers/color.rs @@ -141,30 +141,10 @@ fn tokenize(buf: &[u8]) -> impl Iterator { buf.split(|&c| c == b';').filter(|t| !t.is_empty()) } -/// Zig `std.fmt.parseInt(u9, s, 10)`: optional sign, digits only. +/// Zig `std.fmt.parseInt(u9, s, 10)`. fn parse_u9(s: &[u8]) -> Option { - let (neg, digits) = match s.first()? { - b'+' => (false, &s[1..]), - b'-' => (true, &s[1..]), - _ => (false, s), - }; - if digits.is_empty() { - return None; - } - let mut acc: u32 = 0; - for &c in digits { - if !c.is_ascii_digit() { - return None; - } - acc = acc * 10 + (c - b'0') as u32; - if acc > 511 { - return None; - } - } - if neg && acc != 0 { - return None; - } - Some(acc as u16) + let v = ghostty_zigfmt::parse_int::(s, 10).ok()?; + u16::try_from(v).ok().filter(|v| *v <= 511) } /// Parse any color operation string. This should NOT include the operation @@ -542,6 +522,21 @@ mod tests { #![allow(clippy::unwrap_used)] use super::*; + #[test] + fn zig_parse_u9_differences() { + assert_eq!(Some(10), parse_u9(b"1_0")); + assert_eq!(Some(500), parse_u9(b"5_0_0")); + assert_eq!(Some(5), parse_u9(b"+5")); + assert_eq!(Some(0), parse_u9(b"-0")); + assert_eq!(None, parse_u9(b"-1")); + assert_eq!(None, parse_u9(b"0x10")); + assert_eq!(None, parse_u9(b"1_")); + assert_eq!(None, parse_u9(b"_1")); + assert_eq!(Some(511), parse_u9(b"511")); + assert_eq!(None, parse_u9(b"512")); + assert_eq!(None, parse_u9(b"99999999999")); + } + const RED: Rgb = Rgb { r: 255, g: 0, b: 0 }; const BLUE: Rgb = Rgb { r: 0, g: 0, b: 255 }; diff --git a/crates/ghostty-vt/src/osc/parsers/context_signal.rs b/crates/ghostty-vt/src/osc/parsers/context_signal.rs index 78f20ed..0c44c3c 100644 --- a/crates/ghostty-vt/src/osc/parsers/context_signal.rs +++ b/crates/ghostty-vt/src/osc/parsers/context_signal.rs @@ -216,9 +216,8 @@ impl Field { if value.iter().any(|c| !c.is_ascii_digit()) { return None; } - std::str::from_utf8(value) + ghostty_zigfmt::parse_int::(value, 10) .ok() - .and_then(|s| s.parse::().ok()) .map(FieldValue::U64) } // String fields diff --git a/crates/ghostty-vt/src/osc/parsers/kitty_text_sizing.rs b/crates/ghostty-vt/src/osc/parsers/kitty_text_sizing.rs index aa5cf7c..c6d9dee 100644 --- a/crates/ghostty-vt/src/osc/parsers/kitty_text_sizing.rs +++ b/crates/ghostty-vt/src/osc/parsers/kitty_text_sizing.rs @@ -67,28 +67,8 @@ enum UpdateError { /// Equivalent of Zig `std.fmt.parseInt(u4, value, 10)`. fn parse_u4(value: &[u8]) -> Option { - let (neg, digits) = match value.first() { - Some(b'+') => (false, &value[1..]), - Some(b'-') => (true, &value[1..]), - _ => (false, value), - }; - if digits.is_empty() { - return None; - } - let mut acc: u32 = 0; - for &c in digits { - if !c.is_ascii_digit() { - return None; - } - acc = acc * 10 + (c - b'0') as u32; - if acc > 15 { - return None; - } - } - if neg && acc != 0 { - return None; - } - Some(acc as u8) + let v = ghostty_zigfmt::parse_int::(value, 10).ok()?; + u8::try_from(v).ok().filter(|v| *v <= 15) } impl OSC<'_> { @@ -213,6 +193,20 @@ pub fn parse( mod tests { use super::*; + #[test] + fn zig_parse_u4_differences() { + assert_eq!(Some(10), parse_u4(b"1_0")); + assert_eq!(Some(5), parse_u4(b"+5")); + assert_eq!(Some(0), parse_u4(b"-0")); + assert_eq!(Some(15), parse_u4(b"15")); + assert_eq!(None, parse_u4(b"16")); + assert_eq!(None, parse_u4(b"5_0_0")); + assert_eq!(None, parse_u4(b"-1")); + assert_eq!(None, parse_u4(b"0x1")); + assert_eq!(None, parse_u4(b"1_")); + assert_eq!(None, parse_u4(b"99999999999")); + } + fn run(input: &[u8]) -> Option> { let mut p = Parser::init(false); for &ch in input { diff --git a/crates/ghostty-vt/src/osc/parsers/osc9.rs b/crates/ghostty-vt/src/osc/parsers/osc9.rs index b7f08ea..e04870c 100644 --- a/crates/ghostty-vt/src/osc/parsers/osc9.rs +++ b/crates/ghostty-vt/src/osc/parsers/osc9.rs @@ -12,8 +12,8 @@ use crate::osc::{ }; /// Parse a base-10 unsigned integer from raw bytes (Zig `parseUnsigned`). -fn parse_unsigned(bytes: &[u8]) -> Option { - std::str::from_utf8(bytes).ok()?.parse().ok() +fn parse_unsigned(bytes: &[u8]) -> Option { + ghostty_zigfmt::parse_unsigned::(bytes, 10).ok() } /// Parse OSC 9, which could be an iTerm2 notification or a ConEmu extension. @@ -221,6 +221,18 @@ fn conemu_inner(data: &[u8]) -> Option> { mod tests { use super::*; + #[test] + fn zig_parse_unsigned_differences() { + assert_eq!(run("9;1;4_2"), Command::ConemuSleep { duration_ms: 42 }); + assert_eq!(run("9;1;+42"), Command::ConemuSleep { duration_ms: 100 }); + assert_eq!(run("9;1;-0"), Command::ConemuSleep { duration_ms: 100 }); + assert_eq!(run("9;1;0x10"), Command::ConemuSleep { duration_ms: 100 }); + assert_eq!(run("9;1;70000"), Command::ConemuSleep { duration_ms: 100 }); + assert_eq!(parse_unsigned::(b"5_0"), Some(50)); + assert_eq!(parse_unsigned::(b"+5"), None); + assert_eq!(parse_unsigned::(b"5_"), None); + } + fn run(input: &str) -> Command<'static> { let mut p = Parser::init(false); for ch in input.bytes() { diff --git a/crates/ghostty-vt/src/osc/parsers/semantic_prompt.rs b/crates/ghostty-vt/src/osc/parsers/semantic_prompt.rs index 70334ce..9f75e08 100644 --- a/crates/ghostty-vt/src/osc/parsers/semantic_prompt.rs +++ b/crates/ghostty-vt/src/osc/parsers/semantic_prompt.rs @@ -223,9 +223,8 @@ impl OptionKey { // If we're looking for exit_code we special case it. // as the first value. if self == OptionKey::ExitCode { - return std::str::from_utf8(full) + return ghostty_zigfmt::parse_int::(full, 10) .ok() - .and_then(|s| s.parse::().ok()) .map(OptionValue::Int); } @@ -428,6 +427,16 @@ pub fn parse( mod tests { use super::*; + #[test] + fn zig_parse_int_exit_code() { + assert_eq!(run("133;D;1_0").unwrap().exit_code(), Some(10)); + assert_eq!(run("133;D;+5").unwrap().exit_code(), Some(5)); + assert_eq!(run("133;D;-5").unwrap().exit_code(), Some(-5)); + assert_eq!(run("133;D;0x10").unwrap().exit_code(), None); + assert_eq!(run("133;D;1_").unwrap().exit_code(), None); + assert_eq!(run("133;D;99999999999").unwrap().exit_code(), None); + } + fn run(input: &str) -> Option> { let mut p = Parser::init(false); for ch in input.bytes() { diff --git a/crates/ghostty-zigfmt/Cargo.toml b/crates/ghostty-zigfmt/Cargo.toml new file mode 100644 index 0000000..3d5f579 --- /dev/null +++ b/crates/ghostty-zigfmt/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "ghostty-zigfmt" +version.workspace = true +edition.workspace = true +license.workspace = true +description = "Exact ports of Zig std.fmt.parseInt / parseUnsigned / parseFloat" + +[dependencies] + +[lints] +workspace = true diff --git a/crates/ghostty-os/src/zig_fmt.rs b/crates/ghostty-zigfmt/src/lib.rs similarity index 99% rename from crates/ghostty-os/src/zig_fmt.rs rename to crates/ghostty-zigfmt/src/lib.rs index 7b14cc9..892b0b8 100644 --- a/crates/ghostty-os/src/zig_fmt.rs +++ b/crates/ghostty-zigfmt/src/lib.rs @@ -8,6 +8,8 @@ //! standard library once the Zig grammar has accepted the input (Zig's //! algorithm is also correctly rounded); hex floats follow `convertHex`. +#![cfg_attr(test, allow(clippy::unwrap_used, clippy::expect_used, clippy::panic))] + /// `std.fmt.ParseIntError`. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ParseIntError { diff --git a/docs/rewrite/notes/final-Z12.md b/docs/rewrite/notes/final-Z12.md new file mode 100644 index 0000000..aa9cbb2 --- /dev/null +++ b/docs/rewrite/notes/final-Z12.md @@ -0,0 +1,32 @@ +# Z12: Zig-exact numeric parsing in protocol / non-config code + +`zig_fmt` moved to the new leaf crate `crates/ghostty-zigfmt` (no deps); +`ghostty_os::zig_fmt` is now `pub use ghostty_zigfmt as zig_fmt`, so existing +callers are unchanged. `ghostty-vt` and `ghostty-terminal` depend on it +directly (ghostty-os depends on ghostty-vt, so vt could not use os). +Expected values in the new tests were cross-checked with the real `zig 0.16` +(`std.fmt.parseInt/parseUnsigned/parseFloat`). + +| Rust call site | Zig reference | Status | +|---|---|---| +| vt `kitty/graphics_command.rs::parse_int` | graphics_command.zig:262 `parseInt(i32/u32, _, 10)` | fixed (was hand-rolled: no `_`) | +| vt `osc/parsers/kitty_text_sizing.rs::parse_u4` | kitty_text_sizing.zig:52 `parseInt(u4,10)` | fixed | +| vt `osc/parsers/color.rs::parse_u9` | parsers/color.zig:176,249 `parseInt(u9,10)` | fixed | +| vt `color.rs::parse_unsigned_hex_u16` | color.zig:505 `parseUnsigned(u16,16)` | fixed (Rust wrongly allowed `+`, `-0`) | +| vt `color.rs::from_intensity` | color.zig:483 `parseFloat(f64)` | fixed (`str::parse` -> `parse_float`: hex floats, `_`) | +| vt `osc/parsers/osc9.rs::parse_unsigned` | osc9.zig:27,195 `parseUnsigned(u16/usize,10)` | fixed (was `str::parse`, accepted `+`) | +| vt `osc/parsers/semantic_prompt.rs` exit_code | semantic_prompt.zig:155 `parseInt(i32,10)` | fixed | +| vt `osc/parsers/context_signal.rs` pid/status | context_signal.zig:167 (digit pre-check, `parseInt(u64)`) | fixed | +| vt `kitty/color.rs::Kind::parse` | kitty/color.zig:51 `parseUnsigned(u8,10)` | fixed (no `+` accepted) | +| vt `tmux/control.rs::digits` | control.zig regex `[0-9]+` then `parseInt(usize)` | unchanged: regex guarantees digits only | +| terminal `tmux/layout.rs::parse_usize` | layout.zig:99-163 `parseInt(usize,10)` | fixed (`_`, `-0`) | +| terminal `tmux/output.rs::parse_int` | output.zig:200-214 | fixed (`_`) | +| terminal `tmux/viewer.rs` pane_tabs | viewer.zig:1026 `parseInt(usize,10) catch continue` | fixed | +| os `uri.rs` port | std.Uri `parseInt(u16,10)` | fixed (`_`, no behaviour change for `+`) | +| os `uri.rs::percent_decode` | std.Uri formatRaw `parseInt(u8,"XX",16)` | fixed (accepts `%+A`, `%-0`; rejects nothing new) | +| bench `args.rs::take` | cli/args.zig:429 `parseInt(T,_,0)` / `parseFloat` | fixed (base 0 prefixes, `_`, hex float) | +| vt `parser.rs` / `stream.rs` param_acc | Parser.zig:353, stream.zig:673 saturating digit loop | already identical | +| font `shaper/feature.rs` | feature.zig:133-153 digit loop (u32 mul) | already identical | +| config / input binding / cli ssh-cache / os desktop | Z11 | already using zig_fmt | +| vt `x11_color.rs`, `build.rs` (rgb.txt) | x11_color.zig:30 comptime on fixed data | n/a (data file) | +| cli `list_themes/preview.rs` COLORFGBG, core `GHOSTTY_DEBUG_SCREENSHOT_MS`, apprt-gtk debug_input, build-info/fake_version, renderer shadertoy | no Zig counterpart (Rust-only) | left on `str::parse` |