diff --git a/Cargo.lock b/Cargo.lock index de5eec1..9defede 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1553,6 +1553,7 @@ name = "ghostty-input" version = "0.1.0" dependencies = [ "ghostty-helpgen", + "ghostty-os", "ghostty-unicode", "ghostty-vt", "indexmap", diff --git a/crates/ghostty-apprt-gtk/src/class/application.rs b/crates/ghostty-apprt-gtk/src/class/application.rs index d436a8f..752a9b9 100644 --- a/crates/ghostty-apprt-gtk/src/class/application.rs +++ b/crates/ghostty-apprt-gtk/src/class/application.rs @@ -401,10 +401,7 @@ impl Application { // Keep the inherited bytes: LANG need not be UTF-8. let saved_language = std::env::var_os("LANG"); if let Some(language) = &config.language { - set_env( - "LANG", - ghostty_os::osbytes::EncodedOs::from_encoded(language.clone()).to_os(), - ); + set_env("LANG", language.to_os()); } // Set gettext global domain to be our app so that our unqualified @@ -447,7 +444,9 @@ impl Application { // Our app ID determines uniqueness and maps to our desktop file. let app_id: String = match &config.class { - Some(class) if gio::Application::id_is_valid(class) => class.clone(), + Some(class) if gio::Application::id_is_valid(&class.to_text_lossy()) => { + class.to_text_lossy() + } Some(_) => { log::warn!("invalid 'class' in config, ignoring"); crate::APPLICATION_ID.to_owned() @@ -1102,7 +1101,7 @@ impl Application { let mut working_directory: Option = None; let mut title: Option = None; let mut command: Option = None; - let mut args: Vec = Vec::new(); + let mut args: Vec = Vec::new(); 'overrides: { // were we given a parameter? @@ -1121,7 +1120,7 @@ impl Application { log::debug!("new-window argument: {i} {s}"); if e_seen { - args.push(s.clone()); + args.push(ghostty_os::osbytes::EncodedOs::from_plain_text(s)); continue; } if s == "-e" { @@ -1138,9 +1137,9 @@ impl Application { continue; } if let Some(v) = s.strip_prefix("--working-directory=") { - // IPC carries the encoded form of the OS string. + // D-Bus strings are plain UTF-8, including literal U+FDD0. working_directory = - Some(ghostty_os::osbytes::EncodedOs::from_encoded(v.trim())); + Some(ghostty_os::osbytes::EncodedOs::from_plain_text(v.trim())); continue; } if let Some(v) = s.strip_prefix("--title=") { @@ -1449,7 +1448,7 @@ impl Application { true } Action::SetTitle(v) => { - tgt.with_surface_void("set_title", |s| s.set_title(Some(&v.title))); + tgt.with_surface_void("set_title", |s| s.set_title(Some(&v.title.to_text_lossy()))); true } Action::SetTabTitle(v) => { @@ -1462,11 +1461,8 @@ impl Application { log::warn!("surface is not in a tab, ignoring set_tab_title"); return false; }; - tab.set_title_override(if v.title.is_empty() { - None - } else { - Some(&v.title) - }); + let title = v.title.to_text_lossy(); + tab.set_title_override(if title.is_empty() { None } else { Some(&title) }); true }) } diff --git a/crates/ghostty-apprt-gtk/src/class/command_palette.rs b/crates/ghostty-apprt-gtk/src/class/command_palette.rs index 2b4b115..e654073 100644 --- a/crates/ghostty-apprt-gtk/src/class/command_palette.rs +++ b/crates/ghostty-apprt-gtk/src/class/command_palette.rs @@ -110,9 +110,7 @@ mod command_imp { fn prop_get_title(&self) -> Option { let mut data = self.data.borrow_mut(); match &mut *data { - CommandData::Regular { command } => { - Some(ghostty_os::osbytes::config_str_to_text(&command.title)) - } + CommandData::Regular { command } => Some(command.title.to_text_lossy()), CommandData::Jump { surface, title, .. } => { if let Some(t) = title { return Some(t.clone()); @@ -127,9 +125,7 @@ mod command_imp { fn prop_get_description(&self) -> Option { let mut data = self.data.borrow_mut(); match &mut *data { - CommandData::Regular { command } => Some(ghostty_os::osbytes::config_str_to_text( - &command.description, - )), + CommandData::Regular { command } => Some(command.description.to_text_lossy()), CommandData::Jump { surface, description, diff --git a/crates/ghostty-apprt-gtk/src/class/config.rs b/crates/ghostty-apprt-gtk/src/class/config.rs index 1465142..a8ea47c 100644 --- a/crates/ghostty-apprt-gtk/src/class/config.rs +++ b/crates/ghostty-apprt-gtk/src/class/config.rs @@ -8,6 +8,27 @@ use std::{cell::RefCell, sync::Arc}; use ghostty_config::config::Config as CoreConfig; use gtk::{glib, glib::subclass::prelude::*}; +/// GTK accepts UTF-8, while the diagnostic formatter transports original bytes. +/// Upstream inserts each diagnostic separately, skipping invalid text but still +/// inserting its newline, and skips diagnostics exceeding its 4095-byte buffer. +fn diagnostics_text(config: &CoreConfig) -> String { + let mut text = String::new(); + for diag in config.diagnostics.items() { + let bytes = ghostty_os::osbytes::config_str_to_bytes(&diag.to_string()); + if bytes.len() > 4095 { + log::warn!("diagnostic exceeds GTK buffer capacity"); + continue; + } + if !bytes.contains(&0) + && let Ok(line) = std::str::from_utf8(&bytes) + { + text.push_str(line); + } + text.push('\n'); + } + text +} + mod imp { use gtk::{ glib, @@ -57,15 +78,7 @@ mod imp { return None; } let buf = gtk::TextBuffer::new(None); - let mut text = String::new(); - for diag in config.diagnostics.items() { - if let Err(err) = diag.format(&mut text) { - log::warn!("error writing diagnostic to buffer err={err}"); - continue; - } - text.push('\n'); - } - buf.set_text(&text); + buf.set_text(&super::diagnostics_text(&config)); Some(buf) } } @@ -125,6 +138,20 @@ impl Default for Config { mod tests { use super::*; + #[test] + fn diagnostics_display_original_text_and_skip_invalid_utf8() { + let mut config = CoreConfig::default(); + config + .load_iter(&mut ghostty_config::cli_args::LineIterator::new( + b"font-size=\xef\xb7\x90\xc3\xa9\nfont-size=\xff\n".as_slice(), + )) + .expect("parse diagnostics"); + let text = diagnostics_text(&config); + assert!(text.contains("invalid value \"\u{fdd0}é\"")); + assert!(!text.contains("\u{fdd0}\u{fdd0}")); + assert!(text.ends_with("\n\n")); + } + #[test] fn ghostty_config() { // No display needed; GObject only. diff --git a/crates/ghostty-apprt-gtk/src/class/surface.rs b/crates/ghostty-apprt-gtk/src/class/surface.rs index f15cbf7..2492f0d 100644 --- a/crates/ghostty-apprt-gtk/src/class/surface.rs +++ b/crates/ghostty-apprt-gtk/src/class/surface.rs @@ -278,7 +278,7 @@ impl Surface { if ghostty_apprt::surface::should_inherit_working_directory(context, &config_obj.get()) && let Some(pwd) = parent.get_pwd() { - imp.pwd.replace(Some(pwd.into_encoded())); + imp.pwd.replace(Some(pwd)); self.notify("pwd"); } } @@ -391,7 +391,7 @@ impl Surface { self.notify("key-table"); match value { - KeyTable::Activate(name) => imp.key_tables.borrow_mut().push(name.clone()), + KeyTable::Activate(name) => imp.key_tables.borrow_mut().push(name.to_text_lossy()), KeyTable::Deactivate => { imp.key_tables.borrow_mut().pop(); } @@ -1615,21 +1615,13 @@ impl Surface { /// Returns the pwd property. pub fn get_pwd(&self) -> Option { - // The GObject `pwd` property is a (UTF-8) string, so it stores the - // encoded form of the OS string; this accessor is the only place it - // is converted back to the typed value. - self.imp() - .pwd - .borrow() - .clone() - .map(ghostty_os::osbytes::EncodedOs::from_encoded) + // The typed backing retains bytes; the GObject property renders text. + self.imp().pwd.borrow().clone() } /// Set the pwd for this surface, copies the value. pub fn set_pwd(&self, pwd: Option<&ghostty_os::osbytes::EncodedOs>) { - self.imp() - .pwd - .replace(pwd.map(|p| p.as_encoded().to_owned())); + self.imp().pwd.replace(pwd.cloned()); self.notify("pwd"); } @@ -1822,7 +1814,10 @@ impl Surface { let volume = config.bell_audio_volume.clamp(0.0, 1.0); // The config path is an encoded OS string: decode it first. - let path = path.to_path(); + let Ok(path) = path.to_path() else { + log::warn!("invalid bell audio path: BadPathName"); + return; + }; debug_assert!(path.is_absolute()); let media_file = gtk::MediaFile::for_filename(&path); @@ -2403,8 +2398,10 @@ impl RtSurface for RtSurfaceHandle { None } - fn get_title(&self) -> Option { - self.gobj().and_then(|s| s.get_title()) + fn get_title(&self) -> Option { + self.gobj() + .and_then(|s| s.get_title()) + .map(ghostty_os::osbytes::EncodedOs::from) } fn get_content_scale(&self) -> ContentScale { @@ -2843,8 +2840,8 @@ mod imp { /// If this is set prior to initialization then the surface will /// start in this pwd. If it is set after, it has no impact on the /// core surface. - #[property(get, set, nullable)] - pub pwd: RefCell>, + #[property(get = Self::pwd_text, set = Self::set_pwd_text, type = Option, nullable)] + pub pwd: RefCell>, /// The title of this surface, if any has been set. #[property(get, set, nullable)] @@ -3068,6 +3065,18 @@ mod imp { } impl Surface { + fn pwd_text(&self) -> Option { + self.pwd + .borrow() + .as_ref() + .map(ghostty_os::osbytes::EncodedOs::to_text_lossy) + } + + fn set_pwd_text(&self, value: Option) { + self.pwd + .replace(value.map(ghostty_os::osbytes::EncodedOs::from)); + } + fn mouse_shape_prop(&self) -> i32 { self.mouse_shape.get() as i32 } @@ -3388,7 +3397,7 @@ mod imp { // Binding payloads are config strings: the needle is plain // UI text, so escape it instead of decoding it. &ghostty_input::binding::Action::Search( - ghostty_os::osbytes::valid_str_to_config_string(&needle.unwrap_or_default()), + ghostty_os::osbytes::EncodedOs::from_plain_text(&needle.unwrap_or_default()), ), ) { log::warn!("unable to perform search action err={err}"); diff --git a/crates/ghostty-apprt-gtk/src/class/tab.rs b/crates/ghostty-apprt-gtk/src/class/tab.rs index 8fd6421..82815a5 100644 --- a/crates/ghostty-apprt-gtk/src/class/tab.rs +++ b/crates/ghostty-apprt-gtk/src/class/tab.rs @@ -315,7 +315,12 @@ mod imp { // exists, otherwise the overridden title if it exists, // otherwise the terminal title if it exists, otherwise a // default string. - let config_title = config.as_ref().and_then(|c| c.get().title.clone()); + let config_title = config.as_ref().and_then(|c| { + c.get() + .title + .as_ref() + .map(ghostty_os::osbytes::EncodedOs::to_text_lossy) + }); let plain = tab_override .or(surface_override) .or(terminal) diff --git a/crates/ghostty-apprt-gtk/src/class/window.rs b/crates/ghostty-apprt-gtk/src/class/window.rs index 8125f3a..8596490 100644 --- a/crates/ghostty-apprt-gtk/src/class/window.rs +++ b/crates/ghostty-apprt-gtk/src/class/window.rs @@ -313,7 +313,7 @@ mod imp { // so that any applications inspecting the window states see // an immediate title set when the window appears. if let Some(title) = &config.title { - obj.set_title(Some(&ghostty_os::osbytes::config_str_to_text(title))); + obj.set_title(Some(&title.to_text_lossy())); } } diff --git a/crates/ghostty-apprt-gtk/src/css.rs b/crates/ghostty-apprt-gtk/src/css.rs index 73dfdba..0c2c21e 100644 --- a/crates/ghostty-apprt-gtk/src/css.rs +++ b/crates/ghostty-apprt-gtk/src/css.rs @@ -113,6 +113,13 @@ impl CssProviders { Path::Optional(path) => (path.to_path(), true), Path::Required(path) => (path.to_path(), false), }; + let path = match path { + Ok(path) => path, + Err(error) => { + log::warn!("invalid custom CSS path: {error}"); + continue; + } + }; let shown = path.display(); let meta = match std::fs::metadata(&path) { Ok(m) => m, @@ -215,7 +222,7 @@ pub fn runtime_css(config: &Config, gtk_416: bool) -> String { } if let Some(family) = &config.window_title_font_family { - let family = ghostty_os::osbytes::config_str_to_text(family); + let family = family.to_text_lossy(); let _ = write!( out, ".window headerbar {{\n font-family: \"{family}\";\n}}\n" diff --git a/crates/ghostty-apprt-gtk/src/ipc/new_window.rs b/crates/ghostty-apprt-gtk/src/ipc/new_window.rs index cf44273..a064613 100644 --- a/crates/ghostty-apprt-gtk/src/ipc/new_window.rs +++ b/crates/ghostty-apprt-gtk/src/ipc/new_window.rs @@ -16,7 +16,7 @@ //! ``` use ghostty_apprt::ipc::{Error, NewWindow, Target}; -use gtk::prelude::*; +use gtk::glib; use super::dbus::DBus; @@ -35,9 +35,78 @@ pub fn new_window(target: &Target, value: &NewWindow) -> Result { // parameter is an array of strings that contain the arguments. They // will be sent to the main Ghostty instance and interpreted as CLI // arguments. - dbus.add_parameter(&arguments.to_variant()); + dbus.add_parameter(&arguments_variant(arguments)); } dbus.send()?; Ok(true) } + +/// Zig constructs each GVariant string from serialized bytes including its NUL. +/// Keep valid wire bytes unchanged. Zig marks these strings trusted, causing +/// GIO to abort when serializing invalid UTF-8 or an interior NUL. Fail the same +/// way explicitly: an untrusted GVariant would silently substitute an empty +/// string, while a malformed trusted variant would violate Rust GLib's contract. +fn arguments_variant(arguments: &[Vec]) -> glib::Variant { + glib::Variant::array_from_iter_with_type( + glib::VariantTy::STRING, + arguments.iter().map(|arg| { + if arg.contains(&0) || std::str::from_utf8(arg).is_err() { + log::error!("invalid D-Bus string in new-window arguments"); + std::process::abort(); + } + let mut bytes = arg.clone(); + bytes.push(0); + glib::Variant::from_bytes_with_type( + &glib::Bytes::from_owned(bytes), + glib::VariantTy::STRING, + ) + }), + ) +} + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used)] + use super::*; + #[cfg(unix)] + #[test] + fn invalid_dbus_strings_fail_without_substitution() { + use std::os::unix::process::ExitStatusExt; + const CHILD: &str = "GHOSTTY_TEST_INVALID_DBUS_VALUE"; + if let Ok(kind) = std::env::var(CHILD) { + let value: &[u8] = if kind == "utf8" { + b"--title=bad\xff" + } else { + b"--title=bad\0tail" + }; + arguments_variant(&[value.to_vec()]); + panic!("invalid D-Bus argument must abort like Zig/GIO"); + } + for kind in ["utf8", "nul"] { + // Disable core dumps in the child without changing this test process. + let result = std::process::Command::new("/bin/sh") + .args(["-c", "ulimit -c 0; exec \"$@\"", "ghostty-dbus-test"]) + .arg(std::env::current_exe().unwrap()) + .args([ + "--exact", + "ipc::new_window::tests::invalid_dbus_strings_fail_without_substitution", + ]) + .env(CHILD, kind) + .output() + .unwrap(); + assert_eq!(result.status.signal(), Some(libc::SIGABRT)); + } + } + + #[test] + fn arguments_are_plain_dbus_strings_not_escape_encoded() { + let argument = "--working-directory=/tmp/\u{fdd0}é"; + let variant = arguments_variant(&[argument.as_bytes().to_vec()]); + assert_eq!(variant.get::>().unwrap(), [argument]); + assert_eq!( + variant.child_value(0).data(), + [argument.as_bytes(), b"\0"].concat() + ); + } +} diff --git a/crates/ghostty-apprt-gtk/src/winproto/wayland.rs b/crates/ghostty-apprt-gtk/src/winproto/wayland.rs index f5786f1..edb7d75 100644 --- a/crates/ghostty-apprt-gtk/src/winproto/wayland.rs +++ b/crates/ghostty-apprt-gtk/src/winproto/wayland.rs @@ -525,7 +525,7 @@ impl Window { QuickTerminalLayer::Background => layer_shell::Layer::Background, }); window.set_namespace(Some( - ghostty_os::osbytes::config_str_to_text(&config.gtk_quick_terminal_namespace).as_str(), + config.gtk_quick_terminal_namespace.to_text_lossy().as_str(), )); window.set_keyboard_mode(match config.quick_terminal_keyboard_interactivity { diff --git a/crates/ghostty-apprt-gtk/src/winproto/x11.rs b/crates/ghostty-apprt-gtk/src/winproto/x11.rs index 80bc387..335f3e5 100644 --- a/crates/ghostty-apprt-gtk/src/winproto/x11.rs +++ b/crates/ghostty-apprt-gtk/src/winproto/x11.rs @@ -118,7 +118,7 @@ impl App { let xlib_display = unsafe { x11_display.xdisplay() }; let program_name = match &config.x11_instance_name { - Some(pn) => pn.clone(), + Some(pn) => pn.to_text_lossy(), None if cfg!(debug_assertions) => "ghostty-debug".to_string(), None => "ghostty".to_string(), }; diff --git a/crates/ghostty-apprt/src/action.rs b/crates/ghostty-apprt/src/action.rs index d3bb5b2..877fb49 100644 --- a/crates/ghostty-apprt/src/action.rs +++ b/crates/ghostty-apprt/src/action.rs @@ -638,12 +638,12 @@ pub struct CellSize { #[derive(Debug, Clone, PartialEq, Eq)] pub struct SetTitle { - pub title: String, + pub title: ghostty_os::osbytes::EncodedOs, } impl std::fmt::Display for SetTitle { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "SetTitle{{ {} }}", self.title) + write!(f, "SetTitle{{ {} }}", self.title.to_text_lossy()) } } @@ -657,7 +657,7 @@ pub struct Pwd { impl std::fmt::Display for Pwd { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "Pwd{{ {} }}", self.pwd.as_encoded()) + write!(f, "Pwd{{ {} }}", self.pwd.to_text_lossy()) } } @@ -686,7 +686,7 @@ pub enum KeySequence { #[derive(Debug, Clone, PartialEq, Eq)] pub enum KeyTable { - Activate(String), + Activate(ghostty_os::osbytes::EncodedOs), Deactivate, DeactivateAll, } diff --git a/crates/ghostty-apprt/src/ipc.rs b/crates/ghostty-apprt/src/ipc.rs index a58472a..e271c6d 100644 --- a/crates/ghostty-apprt/src/ipc.rs +++ b/crates/ghostty-apprt/src/ipc.rs @@ -29,7 +29,7 @@ pub struct NewWindow { /// shell should be launched. /// /// It is an error for this to be `Some`, but zero length. - pub arguments: Option>, + pub arguments: Option>>, } /// IPC actions. diff --git a/crates/ghostty-apprt/src/runtime_api.rs b/crates/ghostty-apprt/src/runtime_api.rs index faf5d6f..5e1b22f 100644 --- a/crates/ghostty-apprt/src/runtime_api.rs +++ b/crates/ghostty-apprt/src/runtime_api.rs @@ -291,7 +291,7 @@ pub trait RtSurface { fn close(&self, process_active: bool); /// Cgroup path of the surface (Linux, `gtk-single-instance` scopes). fn cgroup(&self) -> Option; - fn get_title(&self) -> Option; + fn get_title(&self) -> Option; fn get_content_scale(&self) -> ContentScale; fn get_size(&self) -> SurfaceSize; fn get_cursor_pos(&self) -> CursorPos; diff --git a/crates/ghostty-cli/src/action.rs b/crates/ghostty-cli/src/action.rs index 5ef856f..421d682 100644 --- a/crates/ghostty-cli/src/action.rs +++ b/crates/ghostty-cli/src/action.rs @@ -53,7 +53,7 @@ pub trait ActionEnum: Sized + Copy { /// Detect the action from the process CLI args (Zig: `detectArgs`). pub fn detect_args() -> Result, DetectError> { - detect_iter(ghostty_os::args::collect_config_strings()) + detect_iter(ghostty_os::args::collect_bytes()) } /// Detect the action from any iterator. Each iterator value should yield @@ -61,7 +61,7 @@ pub fn detect_args() -> Result, DetectError> { pub fn detect_iter(iter: I) -> Result, DetectError> where I: IntoIterator, - S: AsRef, + S: AsRef<[u8]>, { let mut fallback: Option = None; let mut pending: Option = None; @@ -69,7 +69,10 @@ where let arg = arg.as_ref(); // Allow handling of special cases. - if let Some(special) = E::detect_special_case(arg) { + if let Some(special) = std::str::from_utf8(arg) + .ok() + .and_then(E::detect_special_case) + { match special { SpecialCase::Action(a) => return Ok(Some(a)), SpecialCase::Fallback(a) => fallback = Some(a), @@ -82,13 +85,18 @@ where } // Commands must start with "+" - let Some(name) = arg.strip_prefix('+') else { + let Some(name) = arg.strip_prefix(b"+") else { continue; }; if pending.is_some() { return Err(DetectError::MultipleActions); } - pending = Some(E::from_name(name).ok_or(DetectError::InvalidAction)?); + pending = Some( + std::str::from_utf8(name) + .ok() + .and_then(E::from_name) + .ok_or(DetectError::InvalidAction)?, + ); } // If we have an action, we always return that action, even if we've diff --git a/crates/ghostty-cli/src/boo.rs b/crates/ghostty-cli/src/boo.rs index 7119cd6..33a7a48 100644 --- a/crates/ghostty-cli/src/boo.rs +++ b/crates/ghostty-cli/src/boo.rs @@ -179,7 +179,7 @@ impl Boo { } /// The `boo` command is used to display the animation from the Ghostty website in the terminal -pub fn run(args: &[String]) -> Result { +pub fn run(args: &[impl AsRef<[u8]>]) -> Result { // Disable on non-desktop systems. if cfg!(not(any( windows, diff --git a/crates/ghostty-cli/src/crash_report.rs b/crates/ghostty-cli/src/crash_report.rs index 627361c..0ce4df8 100644 --- a/crates/ghostty-cli/src/crash_report.rs +++ b/crates/ghostty-cli/src/crash_report.rs @@ -36,7 +36,7 @@ pub fn list_reports(dir: &std::path::Path) -> Result, Error> { /// /// This command currently only supports listing crash reports. Viewing /// and sending crash reports is unimplemented and will be added in the future. -pub fn run(args: &[String]) -> Result { +pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut opts = Options; crate::parse_options(&mut opts, args)?; diff --git a/crates/ghostty-cli/src/edit_config.rs b/crates/ghostty-cli/src/edit_config.rs index c01c0c1..8f84045 100644 --- a/crates/ghostty-cli/src/edit_config.rs +++ b/crates/ghostty-cli/src/edit_config.rs @@ -30,7 +30,7 @@ pub use crate::options::NoOptions as Options; /// This command prefers the `$VISUAL` environment variable over `$EDITOR`, /// if both are set. If neither are set, it will print an error /// and exit. -pub fn run(args: &[String]) -> Result { +pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut stderr = std::io::stderr().lock(); let mut opts = Options; @@ -78,14 +78,14 @@ fn run_inner(stderr: &mut dyn Write) -> Result { /// Windows branch of `run`: print the path and fail. #[cfg(windows)] fn write_windows_unsupported(stderr: &mut dyn Write, path: &EncodedOs) -> Result { - write!( - stderr, + let message = format!( "The `ghostty +edit-config` command is not supported on Windows.\n\ Please edit the configuration file manually at the following path:\n\ \n\ {}\n", path.as_encoded() - )?; + ); + stderr.write_all(&ghostty_os::osbytes::config_str_to_bytes(&message))?; Ok(1) } diff --git a/crates/ghostty-cli/src/ghostty.rs b/crates/ghostty-cli/src/ghostty.rs index 49e10f6..895df76 100644 --- a/crates/ghostty-cli/src/ghostty.rs +++ b/crates/ghostty-cli/src/ghostty.rs @@ -158,7 +158,7 @@ impl Action { /// /// If the action requests its help text (`--help`/`-h`), the help text /// is printed to stdout and 0 is returned. - pub fn run(self, args: &[String]) -> Result { + pub fn run(self, args: &[impl AsRef<[u8]>]) -> Result { match self.run_main(args) { Err(Error::ActionHelpRequested) => { // Future note: for now we just output the help text directly @@ -180,7 +180,7 @@ impl Action { } } - fn run_main(self, args: &[String]) -> Result { + fn run_main(self, args: &[impl AsRef<[u8]>]) -> Result { match self { Action::Version => crate::version::run(args), Action::Help => crate::help::run(args), @@ -239,7 +239,7 @@ impl std::fmt::Display for Action { } /// Detect the CLI action from `args` (full argv, `argv[0]` included). -pub fn detect(args: &[String]) -> Result, DetectError> { +pub fn detect(args: &[impl AsRef<[u8]>]) -> Result, DetectError> { detect_iter::(args.iter()) } @@ -248,21 +248,21 @@ pub fn detect(args: &[String]) -> Result, DetectError> { /// reversible escape encoding of `ghostty_os::osbytes`, which the config /// layer (`cli_args`) decodes back to the original bytes. #[must_use] -pub fn os_args_to_strings(args: &[std::ffi::OsString]) -> Vec { +pub fn os_args_to_bytes(args: &[std::ffi::OsString]) -> Vec> { args.iter() - .map(|a| ghostty_os::osbytes::os_to_config_string(a)) + .map(|a| ghostty_os::osbytes::os_bytes(a)) .collect() } /// [`detect`] over raw OS arguments. pub fn detect_os(args: &[std::ffi::OsString]) -> Result, DetectError> { - detect(&os_args_to_strings(args)) + detect(&os_args_to_bytes(args)) } /// [`run_from_args`] over raw OS arguments (byte-preserving). #[must_use] pub fn run_from_os_args(args: &[std::ffi::OsString]) -> Option { - run_from_args(&os_args_to_strings(args)) + run_from_args(&os_args_to_bytes(args)) } /// The CLI entrypoint logic of `main_ghostty.zig` for actions: detect the @@ -272,7 +272,7 @@ pub fn run_from_os_args(args: &[std::ffi::OsString]) -> Option { /// Returns `None` if no action was requested (the caller should launch the /// terminal), or the exit code the process should terminate with. #[must_use] -pub fn run_from_args(args: &[String]) -> Option { +pub fn run_from_args(args: &[impl AsRef<[u8]>]) -> Option { let action = match detect(args) { Ok(a) => a?, Err(err) => { @@ -370,11 +370,8 @@ mod tests { std::ffi::OsString::from("+version"), ]; assert_eq!(Some(Action::Version), detect_os(&args).unwrap()); - let strings = os_args_to_strings(&args); - assert_eq!( - ghostty_os::osbytes::config_str_to_bytes(&strings[1]), - b"--title=\xff" - ); + let strings = os_args_to_bytes(&args); + assert_eq!(strings[1], b"--title=\xff"); } #[test] diff --git a/crates/ghostty-cli/src/help.rs b/crates/ghostty-cli/src/help.rs index fbe1b0e..01299e8 100644 --- a/crates/ghostty-cli/src/help.rs +++ b/crates/ghostty-cli/src/help.rs @@ -22,7 +22,7 @@ cli_options!(Options); /// /// You can also specify `--help` or `-h` along with any action such as /// `+list-themes` to see help for a specific action. -pub fn run(args: &[String]) -> Result { +pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut opts = Options::default(); crate::parse_options(&mut opts, args)?; diff --git a/crates/ghostty-cli/src/list_actions.rs b/crates/ghostty-cli/src/list_actions.rs index a4b1527..593e1bd 100644 --- a/crates/ghostty-cli/src/list_actions.rs +++ b/crates/ghostty-cli/src/list_actions.rs @@ -22,7 +22,7 @@ cli_options!(Options, help); /// Flags: /// /// * `--docs`: will print out the documentation for each action. -pub fn run(args: &[String]) -> Result { +pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut opts = Options::default(); crate::parse_options(&mut opts, args)?; diff --git a/crates/ghostty-cli/src/list_colors.rs b/crates/ghostty-cli/src/list_colors.rs index 1231519..cf83a5f 100644 --- a/crates/ghostty-cli/src/list_colors.rs +++ b/crates/ghostty-cli/src/list_colors.rs @@ -52,7 +52,7 @@ pub fn sorted_colors() -> Vec<(String, (u8, u8, u8))> { /// /// * `--plain`: will disable formatting and make the output more /// friendly for Unix tooling. This is default when not printing to a tty. -pub fn run(args: &[String]) -> Result { +pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut opts = Options::default(); crate::parse_options(&mut opts, args)?; diff --git a/crates/ghostty-cli/src/list_fonts.rs b/crates/ghostty-cli/src/list_fonts.rs index 24bd891..310336b 100644 --- a/crates/ghostty-cli/src/list_fonts.rs +++ b/crates/ghostty-cli/src/list_fonts.rs @@ -8,10 +8,10 @@ config_struct! { pub struct Options { /// The font family to search for. If this is set, then only fonts /// matching this family will be listed. - "family" => family: Option = None, + "family" => family: Option = None, /// The style name to search for. - "style" => style: Option = None, + "style" => style: Option = None, /// Font styles to search for. If this is set, then only fonts that /// match the given styles will be listed. @@ -45,7 +45,7 @@ cli_options!(Options, help); /// * `--family`: Filter results to a specific font family. The family handling /// is identical to the `font-family` set of Ghostty configuration values, so /// this can be used to debug why your desired font may not be loading. -pub fn run(args: &[String]) -> Result { +pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut opts = Options::default(); crate::parse_options(&mut opts, args)?; diff --git a/crates/ghostty-cli/src/list_keybinds.rs b/crates/ghostty-cli/src/list_keybinds.rs index 50ef4e8..eabe5e1 100644 --- a/crates/ghostty-cli/src/list_keybinds.rs +++ b/crates/ghostty-cli/src/list_keybinds.rs @@ -57,7 +57,7 @@ cli_options!(Options, help); /// /// * `--plain`: will disable formatting and make the output more /// friendly for Unix tooling. This is default when not printing to a tty. -pub fn run(args: &[String]) -> Result { +pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut opts = Options::default(); crate::parse_options(&mut opts, args)?; @@ -274,12 +274,13 @@ pub fn collect_bindings(keybinds: &Keybinds) -> (Vec, u16, u16) { for (table_name, table_set) in &keybinds.tables { let (mut table_bindings, table_width) = iterate_bindings(table_set); for b in &mut table_bindings { - b.table_name = Some(table_name.clone()); + b.table_name = Some(table_name.to_text_lossy()); } bindings.extend(table_bindings); widest_chord = widest_chord.max(table_width); - widest_table_prefix = widest_table_prefix.max(gwidth(table_name) + gwidth("/")); + widest_table_prefix = + widest_table_prefix.max(gwidth(&table_name.to_text_lossy()) + gwidth("/")); } // std.mem.sort is stable. diff --git a/crates/ghostty-cli/src/list_themes.rs b/crates/ghostty-cli/src/list_themes.rs index 7acd867..6fee1a0 100644 --- a/crates/ghostty-cli/src/list_themes.rs +++ b/crates/ghostty-cli/src/list_themes.rs @@ -62,11 +62,21 @@ impl ThemeListElement { /// TODO: use Unicode-aware comparison (as in Zig). #[must_use] pub fn order(&self, other: &ThemeListElement) -> Ordering { - let a = self.theme.bytes().map(|c| c.to_ascii_lowercase()); - let b = other.theme.bytes().map(|c| c.to_ascii_lowercase()); + let a = config_str_to_bytes(&self.theme) + .into_iter() + .map(|c| c.to_ascii_lowercase()); + let b = config_str_to_bytes(&other.theme) + .into_iter() + .map(|c| c.to_ascii_lowercase()); a.cmp(b) } + /// Plain text for TUI display/search, separate from byte-preserving export. + #[must_use] + pub fn display_name(&self) -> String { + ghostty_os::osbytes::config_str_to_text(&self.theme) + } + /// `file://` URI of the theme file (Zig: `toUri`). #[must_use] pub fn to_uri(&self) -> String { @@ -122,11 +132,11 @@ pub fn collect_themes(resources_dir: Option<&EncodedOs>) -> Vec rd, Err(e) if e.kind() == std::io::ErrorKind::NotFound => continue, Err(e) => { - eprintln!("error trying to open {}: {e}", dir.as_encoded()); + eprintln!("error trying to open {}: {e}", dir.to_text_lossy()); continue; } }; @@ -145,7 +155,7 @@ pub fn collect_themes(resources_dir: Option<&EncodedOs>) -> Vec /// Make `path` absolute (`Config::load_file` requires absolute paths). pub(crate) fn absolute_path(path: &EncodedOs) -> EncodedOs { - std::path::absolute(path.to_path()).map_or_else(|_| path.clone(), |p| EncodedOs::from_path(&p)) + path.to_path() + .and_then(std::path::absolute) + .map_or_else(|_| path.clone(), |p| EncodedOs::from_path(&p)) } fn resources_dir() -> Option { @@ -216,7 +228,7 @@ fn resources_dir() -> Option { /// /// * `--color`: Specify the color scheme of the themes included in the list. /// This can be `dark`, `light`, or `all`. The default is `all`. -pub fn run(args: &[String]) -> Result { +pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut opts = Options::default(); crate::parse_options(&mut opts, args)?; @@ -316,12 +328,26 @@ mod tests { assert_eq!(1, themes.len()); assert_eq!(config_str_to_bytes(&themes[0].theme), b"th\xffeme".to_vec()); // The path points at the real file and the listing loads it. - assert_eq!(themes[0].path.to_path(), dir.join(raw)); + assert_eq!(themes[0].path.to_path().unwrap(), dir.join(raw)); let mut out = Vec::new(); write_plain(&mut out, &themes, &Options::default()).unwrap(); assert_eq!(out, b"th\xffeme (resources)\n"); } + #[test] + fn theme_display_and_sort_use_original_names() { + let theme = |name: EncodedOs| ThemeListElement { + theme: name.into_encoded(), + path: EncodedOs::new(), + location: Location::Resources, + rank: None, + }; + let literal = theme(EncodedOs::from_plain_text("\u{fdd0}é")); + let invalid = theme(EncodedOs::from_bytes(b"\xff")); + assert_eq!(literal.display_name(), "\u{fdd0}é"); + assert_eq!(literal.order(&invalid), Ordering::Less); + } + #[test] fn collect_sort_and_filter() { let tmp = theme_dir(&[ @@ -407,13 +433,9 @@ mod tests { #[test] fn options_color() { let mut opts = Options::default(); - crate::parse_options( - &mut opts, - &["g".into(), "--color=dark".into(), "--path".into()], - ) - .unwrap(); + crate::parse_options(&mut opts, &["g", "--color=dark", "--path"]).unwrap(); assert_eq!(ColorScheme::Dark, opts.color); assert!(opts.path); - assert!(crate::parse_options(&mut opts, &["g".into(), "--color=blue".into()]).is_err()); + assert!(crate::parse_options(&mut opts, &["g", "--color=blue"]).is_err()); } } diff --git a/crates/ghostty-cli/src/list_themes/preview.rs b/crates/ghostty-cli/src/list_themes/preview.rs index 0728a76..8f0df93 100644 --- a/crates/ghostty-cli/src/list_themes/preview.rs +++ b/crates/ghostty-cli/src/list_themes/preview.rs @@ -442,7 +442,7 @@ impl Preview { continue; } if searching { - theme.rank = rank(&theme.theme, &tokens); + theme.rank = rank(&theme.display_name(), &tokens); if theme.rank.is_some() { self.filtered.push(i); } @@ -771,7 +771,7 @@ impl Preview { theme_list.print("\u{276f} ", self.ui_selected(), row, 0); } theme_list.print_ex( - &theme.theme, + &theme.display_name(), match style { St::Normal => self.ui_standard(), St::Highlighted => self.ui_highlighted(), @@ -783,7 +783,7 @@ impl Preview { None, ); if style == St::Selected { - let name_len = gw(&theme.theme); + let name_len = gw(&theme.display_name()); if name_len + 4 < LIST_WIDTH { for i in (2 + name_len)..(LIST_WIDTH - 2) { theme_list.print(" ", self.ui_selected(), row, i); @@ -869,7 +869,7 @@ impl Preview { height, self.ui_standard(), ); - let theme_line = format!("theme = {}", theme.theme); + let theme_line = format!("theme = {}", theme.display_name()); let save_instructions: [&str; 14] = [ "To apply this theme, add the following line to your Ghostty configuration:", "", @@ -958,7 +958,7 @@ impl Preview { let child = win.child(x_off, 0, width, area.height); child.fill(ui_std); let middle = child.area.height / 2; - let text = format!("Unable to open {} from:", theme.theme); + let text = format!("Unable to open {} from:", theme.display_name()); child.print( &text, ui_err, @@ -966,10 +966,10 @@ impl Preview { (child.area.width / 2).saturating_sub(gw(&text) / 2), ); child.print_ex( - theme.path.as_encoded(), + &theme.path.to_text_lossy(), ui_err, middle, - (child.area.width / 2).saturating_sub(gw(theme.path.as_encoded()) / 2), + (child.area.width / 2).saturating_sub(gw(&theme.path.to_text_lossy()) / 2), Some(&theme.to_uri()), None, ); @@ -1017,18 +1017,18 @@ impl Preview { child.fill(standard); let uri = theme.to_uri(); child.print_ex( - &theme.theme, + &theme.display_name(), standard_bold_italic, 1, - (child.area.width / 2).saturating_sub(gw(&theme.theme) / 2), + (child.area.width / 2).saturating_sub(gw(&theme.display_name()) / 2), Some(&uri), None, ); child.print_ex( - theme.path.as_encoded(), + &theme.path.to_text_lossy(), standard, 2, - (child.area.width / 2).saturating_sub(gw(theme.path.as_encoded()) / 2), + (child.area.width / 2).saturating_sub(gw(&theme.path.to_text_lossy()) / 2), Some(&uri), None, ); @@ -1048,7 +1048,7 @@ impl Preview { ); for (i, diag) in config.diagnostics.items().iter().enumerate() { child.print( - &diag.to_string(), + &ghostty_os::osbytes::config_str_to_text(&diag.to_string()), ui_err, 2 + u16::try_from(i).unwrap_or(0), 2, @@ -1409,7 +1409,7 @@ fn error_name(e: &ghostty_config::config::Error) -> String { pub fn resolve_auto_theme_path() -> Result { let main_cfg_path = ghostty_config::file_load::preferred_default_file_path() .map_err(|e| Error::Other(e.to_string()))? - .to_path(); + .to_path()?; let base_dir = main_cfg_path .parent() .ok_or_else(|| Error::Other("BadPathName".to_string()))?; @@ -1957,6 +1957,21 @@ mod tests { assert_eq!(Mode::Normal, p.mode); } + #[test] + fn theme_preview_renders_plain_unicode_names() { + let tmp = tempfile::tempdir().unwrap(); + let mut p = app(&tmp); + let name = "Alpha\u{fdd0}é"; + p.themes[0].theme = ghostty_os::osbytes::EncodedOs::from_plain_text(name).into_encoded(); + render(&mut p, 120, 60); + assert!(p.overlays().iter().any(|overlay| overlay.text == name)); + assert!( + !p.overlays() + .iter() + .any(|overlay| overlay.text.contains("\u{fdd0}\u{fdd0}")) + ); + } + #[test] fn draw_layout() { let tmp = tempfile::tempdir().unwrap(); diff --git a/crates/ghostty-cli/src/new_window.rs b/crates/ghostty-cli/src/new_window.rs index e11c527..5bb435f 100644 --- a/crates/ghostty-cli/src/new_window.rs +++ b/crates/ghostty-cli/src/new_window.rs @@ -38,49 +38,58 @@ pub struct Options { /// All of the arguments after `+new-window`. They will be sent to Ghostty /// for processing. - pub arguments: Vec, -} - -fn is_ascii_ws(c: char) -> bool { - matches!(c, ' ' | '\t' | '\n' | '\r' | '\x0b' | '\x0c') + pub arguments: Vec>, } impl Options { - fn check_arg(&mut self, arg: &str) -> Result, ArgError> { - if let Some(rest) = arg.strip_prefix("--class=") { - self.class = Some(rest.trim_matches(is_ascii_ws).to_string()); + fn check_arg(&mut self, arg: &[u8]) -> Result>, ArgError> { + if let Some(rest) = arg.strip_prefix(b"--class=") { + self.class = Some( + std::str::from_utf8(ghostty_config::cli_args::trim_ascii_whitespace(rest)) + .map_err(|_| ArgError::InvalidValue)? + .to_owned(), + ); return Ok(None); } - if let Some(rest) = arg.strip_prefix("--working-directory=") { - let stripped = rest.trim_matches(is_ascii_ws); - if stripped == "home" || stripped == "inherit" { - return Ok(Some(arg.to_string())); + if let Some(rest) = arg.strip_prefix(b"--working-directory=") { + let stripped = ghostty_config::cli_args::trim_ascii_whitespace(rest); + if stripped == b"home" || stripped == b"inherit" { + return Ok(Some(arg.to_vec())); } - // The CLI argument is an encoded OS string. - let expanded = ghostty_os::expand_home(&EncodedOs::from_encoded(stripped)) + // Preserve the original argument bytes while expanding its path. + let expanded = ghostty_os::expand_home(&EncodedOs::from_bytes(stripped)) .map_err(|e| ArgError::Other(e.to_string()))?; - let realpath = std::fs::canonicalize(expanded.to_path()) + let realpath = expanded + .to_path() + .and_then(std::fs::canonicalize) .map_err(|e| ArgError::Other(e.to_string()))?; self.working_directory_seen = true; - return Ok(Some(format!( - "--working-directory={}", - EncodedOs::from_path(&realpath).as_encoded() - ))); + return Ok(Some( + [ + b"--working-directory=".as_slice(), + &EncodedOs::from_path(&realpath).to_bytes(), + ] + .concat(), + )); } - Ok(Some(arg.to_string())) + Ok(Some(arg.to_vec())) } } impl CliTarget for Options { - fn parse_into_field(&mut self, _key: &str, _value: Option<&str>) -> Result<(), ArgError> { + fn parse_into_field(&mut self, _key: &str, _value: Option<&[u8]>) -> Result<(), ArgError> { Err(ArgError::InvalidField) } /// Manual parse hook, collect all of the arguments after `+new-window`. - fn parse_manually_hook(&mut self, arg: &str, iter: &mut dyn ArgIter) -> Result { - let mut e_seen = arg == "-e"; + fn parse_manually_hook( + &mut self, + arg: &[u8], + iter: &mut dyn ArgIter, + ) -> Result { + let mut e_seen = arg == b"-e"; // Include the argument that triggered the manual parse hook. if let Some(a) = self.check_arg(arg)? { @@ -93,7 +102,7 @@ impl CliTarget for Options { self.arguments.push(param); continue; } - if param == "-e" { + if param == b"-e" { e_seen = true; self.arguments.push(param); continue; @@ -177,9 +186,10 @@ impl CliTarget for Options { /// default command. /// /// Available since: 1.2.0 -pub fn run(args: &[String]) -> Result { +pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut opts = Options::default(); - let mut iter = ghostty_config::cli_args::ArgsIterator::new(args.iter().skip(1).cloned()); + let mut iter = + ghostty_config::cli_args::ArgsIterator::new(args.iter().skip(1).map(AsRef::as_ref)); match ghostty_config::cli_args::parse(&mut opts, &mut iter) { Ok(()) => {} Err(ArgError::Other(n)) if n == crate::error::HELP_ERROR_NAME => { @@ -196,10 +206,11 @@ pub fn run(args: &[String]) -> Result { let wd = std::fs::canonicalize(".")?; opts.arguments.insert( 0, - format!( - "--working-directory={}", - ghostty_os::osbytes::path_to_config_string(&wd) - ), + [ + b"--working-directory=".as_slice(), + &EncodedOs::from_path(&wd).to_bytes(), + ] + .concat(), ); } @@ -259,7 +270,13 @@ mod tests { fn gathers_arguments_and_class() { let o = parse_str("--class=foo.bar --title=hi --command=ls"); assert_eq!(Some("foo.bar".to_string()), o.class); - assert_eq!(vec!["--title=hi", "--command=ls"], o.arguments); + assert_eq!( + vec!["--title=hi", "--command=ls"], + o.arguments + .iter() + .map(|v| std::str::from_utf8(v).unwrap()) + .collect::>() + ); assert!(!o.working_directory_seen); } @@ -275,6 +292,9 @@ mod tests { "--working-directory=/nonexistent" ], o.arguments + .iter() + .map(|v| std::str::from_utf8(v).unwrap()) + .collect::>() ); } @@ -282,17 +302,35 @@ mod tests { fn working_directory_is_resolved() { let o = parse_str("--working-directory=/"); assert!(o.working_directory_seen); - assert_eq!(vec!["--working-directory=/"], o.arguments); + assert_eq!( + vec!["--working-directory=/"], + o.arguments + .iter() + .map(|v| std::str::from_utf8(v).unwrap()) + .collect::>() + ); let o = parse_str("--working-directory=home"); assert!(!o.working_directory_seen); - assert_eq!(vec!["--working-directory=home"], o.arguments); + assert_eq!( + vec!["--working-directory=home"], + o.arguments + .iter() + .map(|v| std::str::from_utf8(v).unwrap()) + .collect::>() + ); } #[test] fn help_is_gathered_not_special() { // Like Zig, the manual hook runs before the help check. let o = parse_str("--help"); - assert_eq!(vec!["--help"], o.arguments); + assert_eq!( + vec!["--help"], + o.arguments + .iter() + .map(|v| std::str::from_utf8(v).unwrap()) + .collect::>() + ); } #[test] diff --git a/crates/ghostty-cli/src/options.rs b/crates/ghostty-cli/src/options.rs index 36f8158..b4e227f 100644 --- a/crates/ghostty-cli/src/options.rs +++ b/crates/ghostty-cli/src/options.rs @@ -15,9 +15,9 @@ macro_rules! cli_options { fn parse_into_field( &mut self, key: &str, - value: ::core::option::Option<&str>, + value: ::core::option::Option<&[u8]>, ) -> ::core::result::Result<(), ::ghostty_config::cli_args::ArgError> { - self.parse_field(key, value) + self.parse_field_bytes(key, value) } fn valid_values(&self, key: &str) -> ::core::option::Option<&'static [&'static str]> { Self::valid_values_for(key) @@ -43,9 +43,9 @@ macro_rules! cli_options { fn parse_into_field( &mut self, key: &str, - value: ::core::option::Option<&str>, + value: ::core::option::Option<&[u8]>, ) -> ::core::result::Result<(), ::ghostty_config::cli_args::ArgError> { - self.parse_field(key, value) + self.parse_field_bytes(key, value) } fn valid_values(&self, key: &str) -> ::core::option::Option<&'static [&'static str]> { Self::valid_values_for(key) @@ -66,9 +66,9 @@ macro_rules! cli_options { fn parse_into_field( &mut self, key: &str, - value: ::core::option::Option<&str>, + value: ::core::option::Option<&[u8]>, ) -> ::core::result::Result<(), ::ghostty_config::cli_args::ArgError> { - self.parse_field(key, value) + self.parse_field_bytes(key, value) } fn valid_values(&self, key: &str) -> ::core::option::Option<&'static [&'static str]> { Self::valid_values_for(key) @@ -87,8 +87,8 @@ macro_rules! cli_options { /// Parse the CLI arguments `args` (the full argv, including `argv[0]`, /// which is skipped; `+action` arguments are ignored) into `opts` /// (Zig: `args.argsIterator` + `args.parse`). -pub fn parse_options(opts: &mut T, args: &[String]) -> Result<(), Error> { - let mut iter = ArgsIterator::new(args.iter().skip(1).cloned()); +pub fn parse_options(opts: &mut T, args: &[impl AsRef<[u8]>]) -> Result<(), Error> { + let mut iter = ArgsIterator::new(args.iter().skip(1).map(AsRef::as_ref)); cli_args::parse(opts, &mut iter)?; Ok(()) } @@ -101,7 +101,7 @@ impl CliTarget for NoOptions { fn parse_into_field( &mut self, _key: &str, - _value: Option<&str>, + _value: Option<&[u8]>, ) -> Result<(), cli_args::ArgError> { Err(cli_args::ArgError::InvalidField) } diff --git a/crates/ghostty-cli/src/show_config.rs b/crates/ghostty-cli/src/show_config.rs index 598290c..feda24d 100644 --- a/crates/ghostty-cli/src/show_config.rs +++ b/crates/ghostty-cli/src/show_config.rs @@ -49,7 +49,7 @@ cli_options!(Options, help); /// * `--docs`: Print the documentation above each option as a comment, /// This is very noisy but is very useful to learn about available /// options, especially paired with `--default`. -pub fn run(args: &[String]) -> Result { +pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut opts = Options::default(); crate::parse_options(&mut opts, args)?; @@ -113,6 +113,81 @@ mod tests { assert!(s.contains("\nfont-size = 12\n")); } + #[test] + fn show_config_preserves_raw_and_text_fields_on_reload() { + use ghostty_config::cli_args::LineIterator; + let raw = b"title=x\xff\xef\xb7\x90\xc3\xa9\nfont-style=style\xfe\xef\xb7\x90\nfont-codepoint-map=U+0041=family\xff\nselection-word-chars=\xef\xb7\x90\xc3\xa9\nclipboard-codepoint-map=U+0041=\xef\xb7\x90\xc3\xa9\nkeybind=\xef\xb7\x90=set_surface_title:payload\ncommand-palette-entry=title:\"name\\xff\",action:\"text:payload\\xfe\"\n"; + let mut config = Config::new_default(); + config + .load_iter(&mut LineIterator::new(raw.as_slice())) + .unwrap(); + assert_eq!(config.diagnostics.items(), []); + let output = format_config_bytes(&config, &Options::default()).unwrap(); + for line in [ + b"title = x\xff\xef\xb7\x90\xc3\xa9".as_slice(), + b"font-style = style\xfe\xef\xb7\x90", + b"font-codepoint-map = U+0041=family\xff", + b"selection-word-chars = \xef\xb7\x90\xc3\xa9", + b"clipboard-codepoint-map = U+0041=\xef\xb7\x90\xc3\xa9", + b"keybind = \xef\xb7\x90=set_surface_title:payload", + ] { + assert!( + output.split(|b| *b == b'\n').any(|value| value == line), + "missing {line:?} in {output:?}" + ); + } + let mut reloaded = Config::new_default(); + reloaded + .load_iter(&mut LineIterator::new(output.as_slice())) + .unwrap(); + assert_eq!(reloaded.diagnostics.items(), []); + assert_eq!(reloaded.title, config.title); + assert_eq!(reloaded.font_style, config.font_style); + assert_eq!(reloaded.font_codepoint_map, config.font_codepoint_map); + assert_eq!(reloaded.selection_word_chars, config.selection_word_chars); + assert_eq!( + reloaded.clipboard_codepoint_map, + config.clipboard_codepoint_map + ); + let trigger = ghostty_input::binding::Trigger::parse("\u{fdd0}").unwrap(); + assert_eq!( + reloaded + .keybind + .set + .get(trigger) + .unwrap() + .as_leaf() + .unwrap() + .action, + config + .keybind + .set + .get(trigger) + .unwrap() + .as_leaf() + .unwrap() + .action, + ); + assert_eq!( + reloaded.command_palette_entry.value.last(), + config.command_palette_entry.value.last() + ); + } + + #[test] + fn show_config_action_escaping_uses_original_bytes() { + use ghostty_config::cli_args::LineIterator; + let mut config = Config::new_default(); + config + .load_iter(&mut LineIterator::new( + b"keybind=a=set_surface_title:payload\xff\xef\xb7\x90".as_slice(), + )) + .unwrap(); + let output = format_config_bytes(&config, &Options::default()).unwrap(); + // Zig formats action byte strings with std.zig.stringEscape. + assert!(output.ends_with(b"keybind = a=set_surface_title:payload\\xff\\xef\\xb7\\x90\n")); + } + #[cfg(unix)] #[test] fn show_config_round_trips_non_utf8_and_fdd0_paths() { @@ -152,7 +227,7 @@ mod tests { assert!(opts.default); assert!(!opts.changes_only); assert!(matches!( - crate::parse_options(&mut opts, &["g".into(), "--help".into()]), + crate::parse_options(&mut opts, &["g", "--help"]), Err(Error::ActionHelpRequested) )); } diff --git a/crates/ghostty-cli/src/show_face.rs b/crates/ghostty-cli/src/show_face.rs index 27c9499..c94e12a 100644 --- a/crates/ghostty-cli/src/show_face.rs +++ b/crates/ghostty-cli/src/show_face.rs @@ -78,7 +78,7 @@ cli_options!(Options, help, diagnostics); /// * `--presentation`: If set, force searching for a specific presentation /// style. Valid options are `text` and `emoji`. If unset, the presentation /// style of a codepoint will be inferred from the Unicode standard. -pub fn run(args: &[String]) -> Result { +pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut opts = Options::default(); crate::parse_options(&mut opts, args)?; diff --git a/crates/ghostty-cli/src/ssh_cache.rs b/crates/ghostty-cli/src/ssh_cache.rs index dfbcb0f..d5a1f0c 100644 --- a/crates/ghostty-cli/src/ssh_cache.rs +++ b/crates/ghostty-cli/src/ssh_cache.rs @@ -46,7 +46,7 @@ cli_options!(Options, help); /// ghostty +ssh-cache --remove=example.com # Remove host from cache /// ghostty +ssh-cache --clear # Clear entire cache /// ghostty +ssh-cache --expire-days=30 # Set custom expiration period -pub fn run(args: &[String]) -> Result { +pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut opts = Options::default(); crate::parse_options(&mut opts, args)?; diff --git a/crates/ghostty-cli/src/ssh_cache/disk_cache.rs b/crates/ghostty-cli/src/ssh_cache/disk_cache.rs index 8ad8e2c..76b3559 100644 --- a/crates/ghostty-cli/src/ssh_cache/disk_cache.rs +++ b/crates/ghostty-cli/src/ssh_cache/disk_cache.rs @@ -59,7 +59,7 @@ impl DiskCache { home: None, }) .map_err(|_| Error::XdgLookupFailed)?; - Ok(state_dir.to_path().join("ssh_cache")) + Ok(state_dir.to_path()?.join("ssh_cache")) } /// Clear all cache data stored in the disk cache. diff --git a/crates/ghostty-cli/src/validate_config.rs b/crates/ghostty-cli/src/validate_config.rs index a9265b3..a624fa7 100644 --- a/crates/ghostty-cli/src/validate_config.rs +++ b/crates/ghostty-cli/src/validate_config.rs @@ -11,7 +11,7 @@ config_struct! { pub struct Options { /// The path of the config file to validate. If this isn't specified, /// then the default config file paths will be validated. - "config-file" => config_file: Option = None, + "config-file" => config_file: Option = None, } } cli_options!(Options, help); @@ -25,7 +25,7 @@ cli_options!(Options, help); /// /// * `--config-file`: can be passed to validate a specific target config file in /// a non-default location -pub fn run(args: &[String]) -> Result { +pub fn run(args: &[impl AsRef<[u8]>]) -> Result { let mut opts = Options::default(); crate::parse_options(&mut opts, args)?; @@ -42,7 +42,7 @@ fn run_inner(opts: &Options, stdout: &mut dyn Write) -> Result { if let Some(config_path) = &opts.config_file { // Byte-preserving: the option string carries non-UTF-8 bytes via the // reversible `osbytes` escape and the canonical path may too. - let abs_path = std::fs::canonicalize(EncodedOs::from_encoded(config_path).to_path())?; + let abs_path = std::fs::canonicalize(config_path.to_path()?)?; cfg.load_file(&EncodedOs::from_path(&abs_path))?; cfg.load_recursive_files()?; } else { @@ -83,7 +83,7 @@ mod tests { fn validate_good_file() { let p = write_tmp("good", "font-size = 14\n"); let opts = Options { - config_file: Some(p), + config_file: Some(p.into()), }; let mut out = Vec::new(); assert_eq!(0, run_inner(&opts, &mut out).unwrap()); @@ -94,7 +94,7 @@ mod tests { fn validate_bad_file_prints_diagnostics() { let p = write_tmp("bad", "not-a-key = 1\nfont-size = abc\n"); let opts = Options { - config_file: Some(p.clone()), + config_file: Some(p.clone().into()), }; let mut out = Vec::new(); assert_eq!(1, run_inner(&opts, &mut out).unwrap()); @@ -117,7 +117,7 @@ mod tests { let path = tmp.path().join("config"); std::fs::write(&path, b"bad\xffkey = 1\n").unwrap(); let opts = Options { - config_file: Some(path.to_string_lossy().into_owned()), + config_file: Some(EncodedOs::from_path(&path)), }; let mut out = Vec::new(); assert_eq!(1, run_inner(&opts, &mut out).unwrap()); @@ -148,7 +148,7 @@ mod tests { let link = tmp.path().join("link"); std::os::unix::fs::symlink(&target, &link).unwrap(); let opts = Options { - config_file: Some(link.to_string_lossy().into_owned()), + config_file: Some(EncodedOs::from_path(&link)), }; let mut out = Vec::new(); assert_eq!(0, run_inner(&opts, &mut out).unwrap()); diff --git a/crates/ghostty-cli/src/version.rs b/crates/ghostty-cli/src/version.rs index ddbd0fb..cc885c0 100644 --- a/crates/ghostty-cli/src/version.rs +++ b/crates/ghostty-cli/src/version.rs @@ -9,7 +9,7 @@ use crate::{ /// The `version` command is used to display information about Ghostty. Recognized as /// either `+version` or `--version`. -pub fn run(_args: &[String]) -> Result { +pub fn run(_args: &[impl AsRef<[u8]>]) -> Result { let stdout = std::io::stdout(); let tty = stdout.is_terminal(); let mut out = std::io::BufWriter::new(stdout.lock()); diff --git a/crates/ghostty-config/src/cli_args.rs b/crates/ghostty-config/src/cli_args.rs index 55d1e9e..b49c740 100644 --- a/crates/ghostty-config/src/cli_args.rs +++ b/crates/ghostty-config/src/cli_args.rs @@ -30,6 +30,39 @@ pub fn trim_ws(s: &str) -> &str { s.trim_matches(|c| c == ' ' || c == '\t') } +/// Trim Zig's `std.ascii.whitespace`, including vertical tab. +pub fn trim_ascii_whitespace(value: &[u8]) -> &[u8] { + let is_whitespace = |b: &u8| matches!(b, b' ' | b'\t' | b'\n' | b'\r' | 0x0b | 0x0c); + let start = value + .iter() + .position(|b| !is_whitespace(b)) + .unwrap_or(value.len()); + let end = value + .iter() + .rposition(|b| !is_whitespace(b)) + .map_or(start, |i| i + 1); + &value[start..end] +} + +/// Validate at a text field's parsing boundary, never in the line/argument reader. +pub fn text_value(value: Option<&[u8]>) -> Result, ArgError> { + value + .map(|v| std::str::from_utf8(v).map_err(|_| ArgError::InvalidValue)) + .transpose() +} + +pub fn trim_bytes(value: &[u8]) -> &[u8] { + let start = value + .iter() + .position(|b| !matches!(b, b' ' | b'\t')) + .unwrap_or(value.len()); + let end = value + .iter() + .rposition(|b| !matches!(b, b' ' | b'\t')) + .map_or(start, |i| i + 1); + &value[start..end] +} + /// The base errors for arg parsing. Additional errors can be returned due /// to type-specific parsing but these are always possible. #[derive(Debug, Clone, PartialEq, Eq)] @@ -103,7 +136,12 @@ pub trait ConfigField: Clone + PartialEq + Default + fmt::Debug { /// Parse `value` (the text after `=`, `None` if there was no `=`) into /// `self`. Repeatable types accumulate; scalar types overwrite. - fn parse_cli(&mut self, value: Option<&str>) -> Result<(), ArgError>; + fn parse_bytes(&mut self, value: Option<&[u8]>) -> Result<(), ArgError>; + + /// Convenience for plain UTF-8 input. Never accepts the OS escape encoding. + fn parse_cli(&mut self, value: Option<&str>) -> Result<(), ArgError> { + self.parse_bytes(value.map(str::as_bytes)) + } /// Called when the value was set to the empty string: reset to the /// field's default value (Zig: the `init` function or the declared @@ -143,7 +181,7 @@ pub trait CliTarget { /// Parse a single key/value pair into the destination /// (Zig: `parseIntoField`). Returns `ArgError::InvalidField` if the key /// is unknown. - fn parse_into_field(&mut self, key: &str, value: Option<&str>) -> Result<(), ArgError>; + fn parse_into_field(&mut self, key: &str, value: Option<&[u8]>) -> Result<(), ArgError>; /// For enum-typed fields, the valid values (used for diagnostics). fn valid_values(&self, _key: &str) -> Option<&'static [&'static str]> { @@ -159,7 +197,7 @@ pub trait CliTarget { /// stop parsing. fn parse_manually_hook( &mut self, - _arg: &str, + _arg: &[u8], _iter: &mut dyn ArgIter, ) -> Result { Ok(true) @@ -177,14 +215,14 @@ pub trait CliTarget { /// Compatibility handler (Zig: `compatibility` decl). Called when /// parsing a field fails. Return true if handled. - fn compatibility(&mut self, _key: &str, _value: Option<&str>) -> bool { + fn compatibility(&mut self, _key: &str, _value: Option<&[u8]>) -> bool { false } } /// An iterator over arguments (Zig: any iterator with `next`). pub trait ArgIter { - fn next_arg(&mut self) -> Option; + fn next_arg(&mut self) -> Option>; /// Location for diagnostics. fn location(&self) -> Option { @@ -206,7 +244,7 @@ pub fn parse(dst: &mut T, iter: &mut dyn ArgIter) -> Resu // If the destination supports help then we check for it, call // the help function and return. - if dst.supports_help() && (arg == "--help" || arg == "-h") { + if dst.supports_help() && (arg == b"--help" || arg == b"-h") { dst.help()?; return Ok(()); } @@ -214,24 +252,27 @@ pub fn parse(dst: &mut T, iter: &mut dyn ArgIter) -> Resu // If this doesn't start with "--" then it isn't a config // flag. We don't support positional arguments or configuration // values set with spaces so this is an error. - let Some(rest) = arg.strip_prefix("--") else { + let Some(rest) = arg.strip_prefix(b"--") else { let loc = iter.location().unwrap_or_default(); let Some(diags) = dst.diagnostics_mut() else { return Err(ArgError::InvalidField); }; diags.append(Diagnostic { - key: arg.clone(), + key: ghostty_os::osbytes::bytes_to_config_string(&arg), message: "invalid field".to_string(), location: loc, }); continue; }; - let (key, value): (&str, Option<&str>) = match rest.find('=') { + let (key, value) = match rest.iter().position(|b| *b == b'=') { Some(idx) => (&rest[..idx], Some(&rest[idx + 1..])), None => (rest, None), }; + // Keys are ASCII names. Keep arbitrary unknown keys byte-faithful in diagnostics. + let key = ghostty_os::osbytes::bytes_to_config_string(key); + let key = key.as_str(); if let Err(err) = dst.parse_into_field(key, value) { // Fall back to compatibility handlers if able. if dst.compatibility(key, value) { @@ -251,7 +292,10 @@ pub fn parse(dst: &mut T, iter: &mut dyn ArgIter) -> Resu } ArgError::InvalidValue => { let mut m = match value { - Some(v) => format!("invalid value \"{v}\""), + Some(v) => format!( + "invalid value \"{}\"", + ghostty_os::osbytes::bytes_to_config_string(v) + ), None => "invalid value \"null\"".to_string(), }; format_values(dst.valid_values(key), &mut m); @@ -286,7 +330,7 @@ fn format_values(values: Option<&[&str]>, out: &mut String) { pub fn compatibility_renamed( dst: &mut T, to: &str, - value: Option<&str>, + value: Option<&[u8]>, ) -> bool { match dst.parse_into_field(to, value) { Ok(()) => true, @@ -307,6 +351,14 @@ pub fn split_tagged_union(v: &str) -> (&str, &str) { } } +/// Split an ASCII union tag while preserving arbitrary payload bytes. +pub fn split_tagged_union_bytes(v: &[u8]) -> (&[u8], &[u8]) { + match v.iter().position(|b| *b == b':') { + Some(i) => (trim_bytes(&v[..i]), &v[i + 1..]), + None => (trim_bytes(v), b""), + } +} + /// Parse an "auto struct" value like `a:hello,b:42` (Zig: `parseAutoStruct`). /// /// Entries are split by commas (with quoting / escaping, see @@ -320,21 +372,39 @@ pub fn parse_auto_struct_with( v: &str, mut set: impl FnMut(&str, &str) -> Result, ) -> Result<(), ArgError> { - let mut iter = CommaSplitter::new(v); - while let Some(entry) = iter.next_field()? { - let idx = entry.find(':').ok_or(ArgError::InvalidValue)?; - let key = trim_ws(&entry[..idx]); - let raw = trim_ws(&entry[idx + 1..]); + parse_auto_struct_bytes_with(v.as_bytes(), |k, v| { + set( + k, + std::str::from_utf8(v).map_err(|_| ArgError::InvalidValue)?, + ) + }) +} + +pub fn parse_auto_struct_bytes_with( + v: &[u8], + mut set: impl FnMut(&str, &[u8]) -> Result, +) -> Result<(), ArgError> { + let mut iter = CommaSplitter::from_bytes(v); + while let Some(entry) = iter.next_bytes()? { + let idx = entry + .iter() + .position(|b| *b == b':') + .ok_or(ArgError::InvalidValue)?; + let key = trim_bytes(&entry[..idx]); + let raw = trim_bytes(&entry[idx + 1..]); let decoded; - let value = if raw.len() >= 2 && raw.starts_with('"') && raw.ends_with('"') { - decoded = parse_zig_string_literal(raw).ok_or(ArgError::InvalidValue)?; - decoded.as_str() + let value = if raw.len() >= 2 && raw.starts_with(b"\"") && raw.ends_with(b"\"") { + decoded = parse_zig_string_literal_bytes(raw).ok_or(ArgError::InvalidValue)?; + decoded.as_slice() } else { raw }; - if !set(key, value)? { + if !set( + std::str::from_utf8(key).map_err(|_| ArgError::InvalidValue)?, + value, + )? { return Err(ArgError::InvalidValue); } } @@ -342,22 +412,18 @@ pub fn parse_auto_struct_with( } /// Decode a double-quoted Zig string literal (including the quotes). -/// Returns `None` on failure. Bytes produced by `\x` escapes that do not form -/// valid UTF-8 stay raw bytes: they are carried through the reversible -/// `ghostty_os::osbytes` escape encoding (`\xFF` -> `U+FDD0 U+00FF`, decoded back -/// to `0xFF` by `config_str_to_bytes`). See [`parse_zig_string_literal_bytes`] -/// for the raw bytes. +/// Returns `None` on failure or invalid UTF-8. Byte fields use +/// [`parse_zig_string_literal_bytes`] to retain arbitrary literal and escaped bytes. #[must_use] pub fn parse_zig_string_literal(s: &str) -> Option { - parse_zig_string_literal_bytes(s).map(|b| ghostty_os::osbytes::bytes_to_config_string(&b)) + String::from_utf8(parse_zig_string_literal_bytes(s)?).ok() } /// Byte-exact [`parse_zig_string_literal`] (Zig `std.zig.string_literal` /// decoding yields a `[]u8`). #[must_use] -pub fn parse_zig_string_literal_bytes(s: &str) -> Option> { - let b = ghostty_os::osbytes::config_str_to_bytes(s); - let b = b.as_slice(); +pub fn parse_zig_string_literal_bytes(s: impl AsRef<[u8]>) -> Option> { + let b = s.as_ref(); if b.len() < 2 || b[0] != b'"' || b[b.len() - 1] != b'"' { return None; } @@ -494,7 +560,8 @@ impl ConfigField for bool { CValue::Bool(*self) } - fn parse_cli(&mut self, value: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, value: Option<&[u8]>) -> Result<(), ArgError> { + let value = crate::cli_args::text_value(value)?; *self = parse_bool(value.unwrap_or("t"))?; Ok(()) } @@ -512,7 +579,8 @@ macro_rules! impl_int_field { f(*self) } - fn parse_cli(&mut self, value: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, value: Option<&[u8]>) -> Result<(), ArgError> { + let value = crate::cli_args::text_value(value)?; let v = value.ok_or(ArgError::ValueRequired)?; let n = parse_int_base0(v).ok_or(ArgError::InvalidValue)?; *self = <$t>::try_from(n).map_err(|_| ArgError::InvalidValue)?; @@ -546,7 +614,8 @@ macro_rules! impl_float_field { f(*self) } - fn parse_cli(&mut self, value: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, value: Option<&[u8]>) -> Result<(), ArgError> { + let value = crate::cli_args::text_value(value)?; let v = value.ok_or(ArgError::ValueRequired)?; *self = v.parse::<$t>().map_err(|_| ArgError::InvalidValue)?; Ok(()) @@ -562,20 +631,44 @@ impl_float_field!(f32 => CValue::F32, f64 => CValue::F64); impl ConfigField for String { fn c_value_optional(&self) -> CValue { - CValue::String(self.clone()) + CValue::String(ghostty_os::osbytes::valid_str_to_config_string(self)) } fn c_value_none() -> CValue { CValue::NullString } - fn parse_cli(&mut self, value: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, value: Option<&[u8]>) -> Result<(), ArgError> { + let value = crate::cli_args::text_value(value)?; *self = value.ok_or(ArgError::ValueRequired)?.to_string(); Ok(()) } fn format_entry(&self, f: &mut EntryFormatter<'_>) -> fmt::Result { - writeln!(f.writer, "{} = {}", f.name, self) + writeln!( + f.writer, + "{} = {}", + f.name, + ghostty_os::osbytes::valid_str_to_config_string(self) + ) + } +} +impl ConfigField for ghostty_os::osbytes::EncodedOs { + fn c_value_optional(&self) -> CValue { + CValue::String(self.as_encoded().to_owned()) + } + + fn c_value_none() -> CValue { + CValue::NullString + } + + fn parse_bytes(&mut self, value: Option<&[u8]>) -> Result<(), ArgError> { + *self = Self::from_bytes(value.ok_or(ArgError::ValueRequired)?); + Ok(()) + } + + fn format_entry(&self, f: &mut EntryFormatter<'_>) -> fmt::Result { + writeln!(f.writer, "{} = {}", f.name, self.as_encoded()) } } @@ -593,13 +686,13 @@ impl ConfigField for Option { } } - fn parse_cli(&mut self, value: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, value: Option<&[u8]>) -> Result<(), ArgError> { // For optional fields, we just treat it as the child type. if let Some(v) = self { - v.parse_cli(value) + v.parse_bytes(value) } else { let mut tmp = T::default(); - tmp.parse_cli(value)?; + tmp.parse_bytes(value)?; *self = Some(tmp); Ok(()) } @@ -622,7 +715,7 @@ impl ConfigField for Option { /// /// This also ignores any argument that starts with `+`. It assumes that /// actions were parsed out before this iterator was created. -pub struct ArgsIterator> { +pub struct ArgsIterator { /// The underlying args iterator. pub iterator: I, /// Our current index into the iterator. This is 1-indexed because @@ -630,28 +723,34 @@ pub struct ArgsIterator> { pub index: usize, } -impl> ArgsIterator { +impl ArgsIterator +where + I::Item: AsRef<[u8]>, +{ pub fn new(iterator: I) -> Self { Self { iterator, index: 0 } } - pub fn next(&mut self) -> Option { + pub fn next(&mut self) -> Option> { loop { let value = self.iterator.next()?; self.index += 1; // We ignore any argument that starts with "+". This is used // to indicate actions and are expected to be parsed out before // this iterator is created. - if value.starts_with('+') { + if value.as_ref().starts_with(b"+") { continue; } - return Some(value); + return Some(value.as_ref().to_vec()); } } } -impl> ArgIter for ArgsIterator { - fn next_arg(&mut self) -> Option { +impl ArgIter for ArgsIterator +where + I::Item: AsRef<[u8]>, +{ + fn next_arg(&mut self) -> Option> { self.next() } @@ -662,47 +761,46 @@ impl> ArgIter for ArgsIterator { /// Create an args iterator for the process args. This will skip argv0. #[must_use] -pub fn args_iterator() -> ArgsIterator>> { - // Byte-safe: non-UTF-8 arguments keep their bytes (reversible escape - // encoding, see `ghostty_os::osbytes`). - ArgsIterator::new( - ghostty_os::args::collect_config_strings() - .into_iter() - .skip(1), - ) +pub fn args_iterator() -> ArgsIterator>>> { + // Keep original bytes until each field chooses whether to validate UTF-8. + ArgsIterator::new(ghostty_os::args::collect_bytes().into_iter().skip(1)) } /// An iterator valid for arg parsing from a slice. -pub struct SliceIterator<'a> { - pub slice: &'a [String], +pub struct SliceIterator<'a, T> { + pub slice: &'a [T], pub idx: usize, } -impl ArgIter for SliceIterator<'_> { - fn next_arg(&mut self) -> Option { +impl> ArgIter for SliceIterator<'_, T> { + fn next_arg(&mut self) -> Option> { let v = self.slice.get(self.idx)?; self.idx += 1; - Some(v.clone()) + Some(v.as_ref().to_vec()) } } /// Construct a [`SliceIterator`] from a slice. #[must_use] -pub fn slice_iterator(slice: &[String]) -> SliceIterator<'_> { +pub fn slice_iterator>(slice: &[T]) -> SliceIterator<'_, T> { SliceIterator { slice, idx: 0 } } /// A simple whitespace-splitting iterator used in tests and for /// string-based argument parsing. pub struct VecIterator { - items: std::vec::IntoIter, + items: std::vec::IntoIter>, } impl VecIterator { #[must_use] - pub fn new(items: Vec) -> Self { + pub fn new>(items: Vec) -> Self { Self { - items: items.into_iter(), + items: items + .into_iter() + .map(|v| v.as_ref().to_vec()) + .collect::>() + .into_iter(), } } @@ -714,7 +812,7 @@ impl VecIterator { } impl ArgIter for VecIterator { - fn next_arg(&mut self) -> Option { + fn next_arg(&mut self) -> Option> { self.items.next() } } @@ -785,9 +883,9 @@ impl LineIterator { } } - pub fn next(&mut self) -> Option { + pub fn next(&mut self) -> Option> { let mut buf = Vec::new(); - let entry: String = loop { + let entry: Vec = loop { buf.clear(); match self.r.read_until(b'\n', &mut buf) { Ok(0) => return None, @@ -807,42 +905,48 @@ impl LineIterator { self.line += 1; - // Raw bytes survive via the reversible escape encoding. - let line = ghostty_os::osbytes::bytes_to_config_string(&buf); - // Trim any whitespace (including CR) around it - let trim = line.trim_matches(|c| c == ' ' || c == '\t' || c == '\r'); + // Zig trims spaces, tabs and carriage returns, without decoding text. + let start = buf + .iter() + .position(|b| !matches!(b, b' ' | b'\t' | b'\r')) + .unwrap_or(buf.len()); + let end = buf + .iter() + .rposition(|b| !matches!(b, b' ' | b'\t' | b'\r')) + .map_or(start, |i| i + 1); + let trim = &buf[start..end]; // Ignore blank lines and comments - if trim.is_empty() || trim.starts_with('#') { + if trim.is_empty() || trim.starts_with(b"#") { continue; } - break trim.to_string(); + break trim.to_vec(); }; - let mut result = String::from("--"); - if let Some(idx) = entry.find('=') { - let key = trim_ws(&entry[..idx]); - let mut value = trim_ws(&entry[idx + 1..]); + let mut result = b"--".to_vec(); + if let Some(idx) = entry.iter().position(|b| *b == b'=') { + let key = trim_bytes(&entry[..idx]); + let mut value = trim_bytes(&entry[idx + 1..]); // Detect a quoted string. - if value.len() >= 2 && value.starts_with('"') && value.ends_with('"') { + if value.len() >= 2 && value.starts_with(b"\"") && value.ends_with(b"\"") { // Trim quotes since our CLI args processor expects // quotes to already be gone. value = &value[1..value.len() - 1]; } - result.push_str(key); - result.push('='); - result.push_str(value); + result.extend_from_slice(key); + result.push(b'='); + result.extend_from_slice(value); } else { - result.push_str(&entry); + result.extend_from_slice(&entry); } Some(result) } } impl ArgIter for LineIterator { - fn next_arg(&mut self) -> Option { + fn next_arg(&mut self) -> Option> { self.next() } @@ -889,8 +993,8 @@ mod tests { } } impl CliTarget for Simple { - fn parse_into_field(&mut self, key: &str, value: Option<&str>) -> Result<(), ArgError> { - self.parse_field(key, value) + fn parse_into_field(&mut self, key: &str, value: Option<&[u8]>) -> Result<(), ArgError> { + self.parse_field_bytes(key, value) } } @@ -901,8 +1005,8 @@ mod tests { } } impl CliTarget for Quoted { - fn parse_into_field(&mut self, key: &str, value: Option<&str>) -> Result<(), ArgError> { - self.parse_field(key, value) + fn parse_into_field(&mut self, key: &str, value: Option<&[u8]>) -> Result<(), ArgError> { + self.parse_field_bytes(key, value) } } @@ -913,8 +1017,8 @@ mod tests { } } impl CliTarget for Reset { - fn parse_into_field(&mut self, key: &str, value: Option<&str>) -> Result<(), ArgError> { - self.parse_field(key, value) + fn parse_into_field(&mut self, key: &str, value: Option<&[u8]>) -> Result<(), ArgError> { + self.parse_field_bytes(key, value) } } @@ -932,8 +1036,8 @@ mod tests { } } impl CliTarget for Diag { - fn parse_into_field(&mut self, key: &str, value: Option<&str>) -> Result<(), ArgError> { - self.parse_field(key, value) + fn parse_into_field(&mut self, key: &str, value: Option<&[u8]>) -> Result<(), ArgError> { + self.parse_field_bytes(key, value) } fn valid_values(&self, key: &str) -> Option<&'static [&'static str]> { Self::valid_values_for(key) @@ -1029,15 +1133,15 @@ mod tests { pub a: bool, } impl CliTarget for Compat { - fn parse_into_field(&mut self, key: &str, value: Option<&str>) -> Result<(), ArgError> { + fn parse_into_field(&mut self, key: &str, value: Option<&[u8]>) -> Result<(), ArgError> { if key == "a" { // Fails for anything but a valid bool. - return self.a.parse_cli(value); + return self.a.parse_bytes(value); } Err(ArgError::InvalidField) } - fn compatibility(&mut self, key: &str, value: Option<&str>) -> bool { - if key == "a" && value == Some("yuh") { + fn compatibility(&mut self, key: &str, value: Option<&[u8]>) -> bool { + if key == "a" && value == Some(b"yuh") { self.a = true; return true; } @@ -1057,14 +1161,14 @@ mod tests { pub b: bool, } impl CliTarget for Renamed { - fn parse_into_field(&mut self, key: &str, value: Option<&str>) -> Result<(), ArgError> { + fn parse_into_field(&mut self, key: &str, value: Option<&[u8]>) -> Result<(), ArgError> { match key { - "a" => self.a.parse_cli(value), - "b" => self.b.parse_cli(value), + "a" => self.a.parse_bytes(value), + "b" => self.b.parse_bytes(value), _ => Err(ArgError::InvalidField), } } - fn compatibility(&mut self, key: &str, value: Option<&str>) -> bool { + fn compatibility(&mut self, key: &str, value: Option<&[u8]>) -> bool { if key == "old" { return compatibility_renamed(self, "a", value); } @@ -1086,12 +1190,12 @@ mod tests { } } impl CliTarget for Underscore { - fn parse_into_field(&mut self, key: &str, value: Option<&str>) -> Result<(), ArgError> { + fn parse_into_field(&mut self, key: &str, value: Option<&[u8]>) -> Result<(), ArgError> { // Underscore-prefixed fields are never parseable. if key.starts_with('_') { return Err(ArgError::InvalidField); } - self.parse_field(key, value) + self.parse_field_bytes(key, value) } } @@ -1099,7 +1203,7 @@ mod tests { fn parse_into_field_ignore_underscore_prefixed_fields() { let mut data = Underscore::raw_default(); assert_eq!( - data.parse_into_field("_a", Some("42")), + data.parse_into_field("_a", Some(b"42")), Err(ArgError::InvalidField) ); assert_eq!(data.a, "12"); @@ -1117,7 +1221,8 @@ mod tests { } } impl ConfigField for WithInit { - fn parse_cli(&mut self, _value: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, _value: Option<&[u8]>) -> Result<(), ArgError> { + let _value = crate::cli_args::text_value(_value)?; Ok(()) } fn reset(&mut self, _default: Self) { @@ -1272,7 +1377,8 @@ mod tests { pub v: String, } impl ConfigField for WithParse { - fn parse_cli(&mut self, _value: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, _value: Option<&[u8]>) -> Result<(), ArgError> { + let _value = crate::cli_args::text_value(_value)?; self.v = "HELLO!".into(); Ok(()) } @@ -1313,7 +1419,8 @@ mod tests { } } impl ConfigField for Basic { - fn parse_cli(&mut self, value: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, value: Option<&[u8]>) -> Result<(), ArgError> { + let value = crate::cli_args::text_value(value)?; let v = value.ok_or(ArgError::ValueRequired)?; let mut r = Basic::default(); let mut set = [false; 3]; @@ -1381,7 +1488,8 @@ mod tests { D(String), } impl ConfigField for Tu { - fn parse_cli(&mut self, value: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, value: Option<&[u8]>) -> Result<(), ArgError> { + let value = crate::cli_args::text_value(value)?; let v = value.ok_or(ArgError::ValueRequired)?; let (tag, rest) = split_tagged_union(v); *self = match tag { @@ -1454,8 +1562,8 @@ mod tests { fn args_iterator() { let child = split_args("--what +list-things --a=42").into_iter(); let mut iter = ArgsIterator::new(child); - assert_eq!(iter.next().unwrap(), "--what"); - assert_eq!(iter.next().unwrap(), "--a=42"); + assert_eq!(iter.next().unwrap(), b"--what"); + assert_eq!(iter.next().unwrap(), b"--a=42"); assert_eq!(iter.next(), None); assert_eq!(iter.next(), None); } @@ -1469,12 +1577,12 @@ mod tests { let mut iter = line_iter( "A\nB=42\nC\n\n# A comment\nD\n\n # An indented comment\n E\n\n# A quoted string with whitespace\nF= \"value \"", ); - assert_eq!(iter.next().unwrap(), "--A"); - assert_eq!(iter.next().unwrap(), "--B=42"); - assert_eq!(iter.next().unwrap(), "--C"); - assert_eq!(iter.next().unwrap(), "--D"); - assert_eq!(iter.next().unwrap(), "--E"); - assert_eq!(iter.next().unwrap(), "--F=value "); + assert_eq!(iter.next().unwrap(), b"--A"); + assert_eq!(iter.next().unwrap(), b"--B=42"); + assert_eq!(iter.next().unwrap(), b"--C"); + assert_eq!(iter.next().unwrap(), b"--D"); + assert_eq!(iter.next().unwrap(), b"--E"); + assert_eq!(iter.next().unwrap(), b"--F=value "); assert_eq!(iter.next(), None); assert_eq!(iter.next(), None); } @@ -1482,7 +1590,7 @@ mod tests { #[test] fn line_iterator_end_in_newline() { let mut iter = line_iter("A\n\n"); - assert_eq!(iter.next().unwrap(), "--A"); + assert_eq!(iter.next().unwrap(), b"--A"); assert_eq!(iter.next(), None); assert_eq!(iter.next(), None); } @@ -1490,7 +1598,7 @@ mod tests { #[test] fn line_iterator_spaces_around_equals() { let mut iter = line_iter("A = B\n\n"); - assert_eq!(iter.next().unwrap(), "--A=B"); + assert_eq!(iter.next().unwrap(), b"--A=B"); assert_eq!(iter.next(), None); assert_eq!(iter.next(), None); } @@ -1498,15 +1606,15 @@ mod tests { #[test] fn line_iterator_no_value() { let mut iter = line_iter("A = \n\n"); - assert_eq!(iter.next().unwrap(), "--A="); + assert_eq!(iter.next().unwrap(), b"--A="); assert_eq!(iter.next(), None); } #[test] fn line_iterator_with_crlf_line_endings() { let mut iter = line_iter("A\r\nB = C\r\n"); - assert_eq!(iter.next().unwrap(), "--A"); - assert_eq!(iter.next().unwrap(), "--B=C"); + assert_eq!(iter.next().unwrap(), b"--A"); + assert_eq!(iter.next().unwrap(), b"--B=C"); assert_eq!(iter.next(), None); assert_eq!(iter.next(), None); } @@ -1515,8 +1623,8 @@ mod tests { fn line_iterator_with_buffered_reader() { let r = std::io::BufReader::with_capacity(2, Cursor::new(b"A\nB = C\n".to_vec())); let mut iter = LineIterator::new(r); - assert_eq!(iter.next().unwrap(), "--A"); - assert_eq!(iter.next().unwrap(), "--B=C"); + assert_eq!(iter.next().unwrap(), b"--A"); + assert_eq!(iter.next().unwrap(), b"--B=C"); assert_eq!(iter.next(), None); assert_eq!(iter.next(), None); } @@ -1527,8 +1635,8 @@ mod tests { let mut r = std::io::BufReader::with_capacity(2, Cursor::new(b"A\nB = C\n".to_vec())); r.fill_buf().unwrap(); let mut iter = LineIterator::new(r); - assert_eq!(iter.next().unwrap(), "--A"); - assert_eq!(iter.next().unwrap(), "--B=C"); + assert_eq!(iter.next().unwrap(), b"--A"); + assert_eq!(iter.next().unwrap(), b"--B=C"); assert_eq!(iter.next(), None); assert_eq!(iter.next(), None); } @@ -1558,21 +1666,16 @@ mod tests { parse_zig_string_literal_bytes("\"a\\xFFb\"").unwrap(), vec![b'a', 0xff, b'b'] ); - let s = parse_zig_string_literal("\"a\\xFFb\"").unwrap(); - assert_eq!( - ghostty_os::osbytes::config_str_to_bytes(&s), - vec![b'a', 0xff, b'b'] - ); - assert!(!s.contains('\u{fffd}')); + assert!(parse_zig_string_literal("\"a\\xFFb\"").is_none()); // Valid multi-byte sequences spelled with escapes still decode. assert_eq!( parse_zig_string_literal("\"\\xC3\\xA9\"").unwrap(), "\u{e9}" ); - // Raw (unescaped) invalid input bytes survive the escape encoding. - let raw = ghostty_os::osbytes::bytes_to_config_string(b"\"x\xfey\""); + // Raw (unescaped) invalid input bytes survive literal parsing. + let raw = b"\"x\xfey\""; assert_eq!( - parse_zig_string_literal_bytes(&raw).unwrap(), + parse_zig_string_literal_bytes(raw).unwrap(), vec![b'x', 0xfe, b'y'] ); } @@ -1582,10 +1685,6 @@ mod tests { let data: &[u8] = b"title = a\xffb\n"; let mut it = LineIterator::new(data); let line = it.next().unwrap(); - assert_eq!( - ghostty_os::osbytes::config_str_to_bytes(&line), - b"--title=a\xffb".to_vec() - ); - assert!(!line.contains('\u{fffd}')); + assert_eq!(line, b"--title=a\xffb".to_vec()); } } diff --git a/crates/ghostty-config/src/comma_splitter.rs b/crates/ghostty-config/src/comma_splitter.rs index 864e13d..64f45d0 100644 --- a/crates/ghostty-config/src/comma_splitter.rs +++ b/crates/ghostty-config/src/comma_splitter.rs @@ -54,21 +54,31 @@ enum UnicodeState { #[derive(Debug, Clone)] pub struct CommaSplitter<'a> { /// the string that we are splitting - str: &'a str, + str: &'a [u8], /// how much of the string has been consumed so far index: usize, } impl<'a> CommaSplitter<'a> { + pub fn new(value: &'a str) -> Self { + Self::from_bytes(value.as_bytes()) + } + + pub fn next_field(&mut self) -> Result, Error> { + self.next_bytes()? + .map(|b| std::str::from_utf8(b).map_err(|_| Error::IllegalEscape)) + .transpose() + } + /// initialize a splitter with the given string #[must_use] - pub fn new(str: &'a str) -> Self { + pub fn from_bytes(str: &'a [u8]) -> Self { Self { str, index: 0 } } /// return the next field, `Ok(None)` if no more fields - pub fn next_field(&mut self) -> Result, Error> { - let bytes = self.str.as_bytes(); + pub fn next_bytes(&mut self) -> Result, Error> { + let bytes = self.str; if self.index >= bytes.len() { return Ok(None); } @@ -213,7 +223,7 @@ impl<'a> CommaSplitter<'a> { } let r = &self.str[self.index..]; self.index = self.str.len(); - Some(r) + std::str::from_utf8(r).ok() } } diff --git a/crates/ghostty-config/src/command.rs b/crates/ghostty-config/src/command.rs index d5fc252..ba4b522 100644 --- a/crates/ghostty-config/src/command.rs +++ b/crates/ghostty-config/src/command.rs @@ -2,6 +2,8 @@ use std::fmt; +use ghostty_os::osbytes::EncodedOs; + use crate::{ cli_args::{ArgError, ConfigField}, formatter::EntryFormatter, @@ -33,30 +35,30 @@ pub enum Command { /// Index zero is not guaranteed to be an absolute path, and may require /// PATH lookup. It is up to the downstream to do this, usually via /// delegation to something like `execvp`. - Direct(Vec), + Direct(Vec), /// Execute a command via shell expansion. This provides the command /// as a single string that is expected to be expanded in some way /// (up to the downstream). Usually `/bin/sh -c`. - Shell(String), + Shell(EncodedOs), } impl Default for Command { fn default() -> Self { - Command::Shell(String::new()) + Command::Shell(EncodedOs::new()) } } /// Iterates over each argument in the command. #[derive(Debug, Clone)] pub struct ArgIterator { - args: std::vec::IntoIter, + args: std::vec::IntoIter, } impl Iterator for ArgIterator { - type Item = String; + type Item = EncodedOs; - fn next(&mut self) -> Option { + fn next(&mut self) -> Option { self.args.next() } } @@ -64,8 +66,7 @@ impl Iterator for ArgIterator { /// Best-effort shell string splitter matching Zig's /// `std.process.ArgIteratorGeneral(.{})` (whitespace separated, double /// quotes group, backslashes only special before a double quote). -fn split_general(s: &str) -> Vec { - let b = s.as_bytes(); +fn split_general(b: &[u8]) -> Vec { let mut out = vec![]; let mut i = 0; loop { @@ -108,7 +109,7 @@ fn split_general(s: &str) -> Vec { } } } - out.push(String::from_utf8_lossy(&cur).into_owned()); + out.push(EncodedOs::from_bytes(&cur)); } out } @@ -120,8 +121,12 @@ impl Command { #[must_use] pub fn string(&self) -> String { match self { - Command::Shell(v) => v.clone(), - Command::Direct(v) => v.join(" "), + Command::Shell(v) => v.as_encoded().to_owned(), + Command::Direct(v) => v + .iter() + .map(EncodedOs::as_encoded) + .collect::>() + .join(" "), } } @@ -135,7 +140,7 @@ impl Command { pub fn arg_iterator(&self) -> ArgIterator { let args = match self { Command::Direct(v) => v.clone(), - Command::Shell(v) => split_general(v), + Command::Shell(v) => split_general(&v.to_bytes()), }; ArgIterator { args: args.into_iter(), @@ -143,39 +148,60 @@ impl Command { } } +fn trim_spaces(value: &[u8]) -> &[u8] { + let start = value.iter().position(|b| *b != b' ').unwrap_or(value.len()); + let end = value + .iter() + .rposition(|b| *b != b' ') + .map_or(start, |i| i + 1); + &value[start..end] +} + impl ConfigField for Command { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { // Input is required. Whitespace on the edges isn't needed. // Commands must be non-empty. let input = input.ok_or(ArgError::ValueRequired)?; - let trimmed = input.trim_matches(' '); + let trimmed = trim_spaces(input); if trimmed.is_empty() { return Err(ArgError::ValueRequired); } // If we have a `:` then we MIGHT have a prefix to specify what // tag we should use. - let (direct, str) = match trimmed.find(':') { - Some(idx) if &trimmed[..idx] == "direct" => (true, &trimmed[idx + 1..]), - Some(idx) if &trimmed[..idx] == "shell" => (false, &trimmed[idx + 1..]), + let (direct, str) = match trimmed.iter().position(|b| *b == b':') { + Some(idx) if &trimmed[..idx] == b"direct" => (true, &trimmed[idx + 1..]), + Some(idx) if &trimmed[..idx] == b"shell" => (false, &trimmed[idx + 1..]), _ => (false, trimmed), }; - let str = str.trim_matches(' '); + let str = trim_spaces(str); *self = if direct { // We're not shell expanding, so the arguments are naively // split on spaces. - Command::Direct(str.split(' ').map(str::to_string).collect()) + Command::Direct( + str.split(|b| *b == b' ') + .map(EncodedOs::from_bytes) + .collect(), + ) } else { - Command::Shell(str.to_string()) + Command::Shell(EncodedOs::from_bytes(str)) }; Ok(()) } fn format_entry(&self, f: &mut EntryFormatter<'_>) -> fmt::Result { match self { - Command::Shell(v) => writeln!(f.writer, "{} = {v}", f.name), - Command::Direct(v) => writeln!(f.writer, "{} = direct:{}", f.name, v.join(" ")), + Command::Shell(v) => writeln!(f.writer, "{} = {}", f.name, v.as_encoded()), + Command::Direct(v) => writeln!( + f.writer, + "{} = direct:{}", + f.name, + v.iter() + .map(EncodedOs::as_encoded) + .collect::>() + .join(" ") + ), } } } @@ -229,7 +255,7 @@ mod tests { #[test] fn command_arg_iterator_shell_quotes() { let v = Command::Shell("echo \"hello world\" a\\\"b".into()); - let args: Vec = v.arg_iterator().collect(); + let args: Vec = v.arg_iterator().collect(); assert_eq!(args, ["echo", "hello world", "a\"b"]); } diff --git a/crates/ghostty-config/src/config.rs b/crates/ghostty-config/src/config.rs index 08e199f..426e381 100644 --- a/crates/ghostty-config/src/config.rs +++ b/crates/ghostty-config/src/config.rs @@ -56,7 +56,7 @@ crate::config_struct! { /// /// GTK only. /// Available since 1.3.0. - "language" => language: Option = None, + "language" => language: Option = None, /// The font families to use. /// @@ -1444,7 +1444,7 @@ crate::config_struct! { /// sequence will be honored but previous changes will not retroactively /// be set. This latter case may require you to restart programs such as Neovim /// to get the new title. - "title" => title: Option = None, + "title" => title: Option = None, /// The setting that will change the application class value. /// @@ -1466,7 +1466,7 @@ crate::config_struct! { /// The default is `com.mitchellh.ghostty`. /// /// This only affects GTK builds. - "class" => class: Option = None, + "class" => class: Option = None, /// This controls the instance name field of the `WM_CLASS` X11 property when /// running under X11. It has no effect otherwise. @@ -1474,7 +1474,7 @@ crate::config_struct! { /// The default is `ghostty`. /// /// This only affects GTK builds. - "x11-instance-name" => x11_instance_name: Option = None, + "x11-instance-name" => x11_instance_name: Option = None, /// The directory to change to after starting the command. /// @@ -2049,7 +2049,7 @@ crate::config_struct! { /// required to be a fixed-width font. /// /// Available since: 1.0.0 on macOS, 1.1.0 on GTK - "window-title-font-family" => window_title_font_family: Option = None, + "window-title-font-family" => window_title_font_family: Option = None, /// The text that will be displayed in the subtitle of the window. Valid values: /// @@ -2626,7 +2626,7 @@ crate::config_struct! { /// GTK Wayland only. /// /// Available since: 1.2.0 - "gtk-quick-terminal-namespace" => gtk_quick_terminal_namespace: String = "ghostty-quick-terminal".to_string(), + "gtk-quick-terminal-namespace" => gtk_quick_terminal_namespace: EncodedOs = EncodedOs::from_plain_text("ghostty-quick-terminal"), /// The screen where the quick terminal should show up. /// @@ -3360,7 +3360,7 @@ crate::config_struct! { /// Supported formats include PNG, JPEG, and ICNS. /// /// Defaults to `~/.config/ghostty/Ghostty.icns` - "macos-custom-icon" => macos_custom_icon: Option = None, + "macos-custom-icon" => macos_custom_icon: Option = None, /// The material to use for the frame of the macOS app icon. /// @@ -3648,11 +3648,11 @@ crate::config_struct! { /// features. An option exists in vim to modify this: `:set /// keyprotocol=ghostty:kitty`, however a bug in the implementation prevents it /// from working properly. https://github.com/vim/vim/pull/13211 fixes this. - "term" => term: String = "xterm-ghostty".to_string(), + "term" => term: EncodedOs = EncodedOs::from_plain_text("xterm-ghostty"), /// String to send when we receive `ENQ` (`0x05`) from the command that we are /// running. Defaults to an empty string if not set. - "enquiry-response" => enquiry_response: String = String::new(), + "enquiry-response" => enquiry_response: EncodedOs = EncodedOs::new(), /// Configures the low-level API to use for async IO, eventing, etc. /// @@ -3764,7 +3764,7 @@ pub enum Error { #[derive(Debug, Clone, PartialEq)] pub enum ReplayStep { /// An argument to parse as if it came from the CLI or file. - Arg(String), + Arg(Vec), /// A base path to expand relative paths against. Expand(EncodedOs), @@ -3774,7 +3774,7 @@ pub enum ReplayStep { /// having multiple conditional arg entries. ConditionalArg { conditions: Vec, - arg: String, + arg: Vec, }, /// A diagnostic to be added to the new configuration when @@ -3815,7 +3815,7 @@ impl Default for Config { /// bytes are carried as encoded command text (so non-UTF-8 bytes and a /// literal U+FDD0 round-trip exactly, like Zig's byte slice). fn shell_from_env(value: &std::ffi::OsStr) -> Command { - Command::Shell(ghostty_os::osbytes::EncodedOs::from_os(value).into_encoded()) + Command::Shell(ghostty_os::osbytes::EncodedOs::from_os(value)) } impl Config { @@ -3886,13 +3886,13 @@ impl Config { /// /// `path` must be resolved and absolute. pub fn load_file(&mut self, path: &EncodedOs) -> Result<(), Error> { - assert!(path.to_path().is_absolute()); + assert!(path.to_path()?.is_absolute()); let file = match crate::file_load::open(path) { Ok(f) => f, Err(crate::file_load::OpenFileError::NotAFile) => { log::warn!( "config-file {}: not reading because it is not a file", - path.as_encoded() + path.to_text_lossy() ); return Ok(()); } @@ -3907,7 +3907,7 @@ impl Config { /// Load config from the given File. fn load_fs_file(&mut self, file: std::fs::File, path: &EncodedOs) -> Result<(), Error> { - log::info!("reading configuration file path={}", path.as_encoded()); + log::info!("reading configuration file path={}", path.to_text_lossy()); let mut reader = std::io::BufReader::with_capacity(2048, file); self.load_reader(&mut reader, path) } @@ -3929,7 +3929,7 @@ impl Config { let mut iter = cli_args::LineIterator::with_path(reader, path.as_encoded()); self.load_iter(&mut iter)?; let dir = path - .to_path() + .to_path()? .parent() .map(EncodedOs::from_path) .unwrap_or_default(); @@ -3949,7 +3949,7 @@ impl Config { Err(err) => { log::warn!( "error reading optional config file, not loading err={err} path={}", - path.as_encoded() + path.to_text_lossy() ); OptionalFileAction::Error } @@ -3971,8 +3971,11 @@ impl Config { } fn write_config_template(path: &EncodedOs) -> std::io::Result<()> { - log::info!("creating template config file: path={}", path.as_encoded()); - let os_path = path.to_path(); + log::info!( + "creating template config file: path={}", + path.to_text_lossy() + ); + let os_path = path.to_path()?; if let Some(dir) = os_path.parent() { std::fs::create_dir_all(dir)?; } @@ -3999,8 +4002,8 @@ impl Config { { log::warn!( "both config files `{}` and `{}` exist.", - legacy_xdg_path.as_encoded(), - xdg_path.as_encoded() + legacy_xdg_path.to_text_lossy(), + xdg_path.to_text_lossy() ); log::warn!("loading them both in that order"); true @@ -4034,8 +4037,8 @@ impl Config { { log::warn!( "both config files `{}` and `{}` exist.", - legacy_app_support_path.as_encoded(), - app_support_path.as_encoded() + legacy_app_support_path.to_text_lossy(), + app_support_path.to_text_lossy() ); log::warn!("loading them both in that order"); true @@ -4102,10 +4105,11 @@ impl Config { // Next, take all remaining args and use that to build up // a command to execute. - let mut builder: Vec = Vec::new(); + let mut builder: Vec = Vec::new(); for arg in args { - let copy = ghostty_os::osbytes::bytes_to_config_string(arg.to_bytes()); - self.replay_steps.push(ReplayStep::Arg(copy.clone())); + let copy = EncodedOs::from_bytes(arg.to_bytes()); + self.replay_steps + .push(ReplayStep::Arg(arg.to_bytes().to_vec())); builder.push(copy); } @@ -4207,7 +4211,7 @@ impl Config { // All paths should already be absolute at this point because // they're fixed up after each load. - assert!(path.to_path().is_absolute()); + assert!(path.to_path()?.is_absolute()); // We must only load a unique file once if !loaded.insert(path.clone()) { @@ -4220,7 +4224,7 @@ impl Config { continue; } - let file = match std::fs::File::open(path.to_path()) { + let file = match path.to_path().and_then(std::fs::File::open) { Ok(f) => f, Err(err) => { if err.kind() != std::io::ErrorKind::NotFound || !optional { @@ -4334,7 +4338,7 @@ impl Config { ReplayStep::DashE => { // The "-e" hook consumes every remaining argument. - let mut args = vec!["-e".to_string()]; + let mut args = vec![b"-e".to_vec()]; while i < steps.len() { match &steps[i] { ReplayStep::Expand(base) => { @@ -4351,7 +4355,7 @@ impl Config { } } ReplayStep::Arg(arg) => args.push(arg.clone()), - ReplayStep::DashE => args.push("-e".to_string()), + ReplayStep::DashE => args.push(b"-e".to_vec()), } i += 1; } @@ -4363,8 +4367,8 @@ impl Config { Ok(()) } - fn load_one_arg(&mut self, arg: &str) -> Result<(), ArgError> { - let mut it = cli_args::VecIterator::new(vec![arg.to_string()]); + fn load_one_arg(&mut self, arg: &[u8]) -> Result<(), ArgError> { + let mut it = cli_args::VecIterator::new(vec![arg.to_vec()]); self.load_iter(&mut it) } @@ -4391,7 +4395,7 @@ impl Config { // Load the correct theme depending on the conditional state. // Dark/light themes were programmed prior to conditional configuration // so when we introduce that we probably want to replace this. - let name: &str = match self.conditional_state.theme { + let name = match self.conditional_state.theme { conditional::Theme::Light => &theme.light, conditional::Theme::Dark => &theme.dark, }; @@ -4399,7 +4403,7 @@ impl Config { // Find our theme file and open it. See the open function for details. let resources_dir = ghostty_os::resources_dir().ok(); let Some(themefile) = crate::theme::open( - &EncodedOs::from_encoded(name), + name, resources_dir.as_ref().and_then(|r| r.app()), &mut self.diagnostics, ) else { @@ -4520,7 +4524,7 @@ impl Config { // Prevent setting TERM to an empty string if self.term.is_empty() { // HACK: See comment above at definition - self.term = "xterm-ghostty".to_string(); + self.term = "xterm-ghostty".into(); } // The default for the working directory depends on the system. @@ -4647,7 +4651,7 @@ impl Config { fn platform_defaults(&mut self, wd: &mut WorkingDirectory) -> Result<(), Error> { if self.command.is_none() { log::warn!("no default shell found, will default to using cmd"); - self.command = Some(Command::Shell("cmd.exe".to_owned())); + self.command = Some(Command::Shell("cmd.exe".into())); } if *wd == WorkingDirectory::Home { @@ -4665,8 +4669,8 @@ impl Config { let pw = ghostty_os::passwd::get()?; if self.command.is_none() { if let Some(sh) = pw.shell { - log::info!("default shell src=passwd value={}", sh.as_encoded()); - self.command = Some(Command::Shell(sh.into_encoded())); + log::info!("default shell src=passwd value={}", sh.to_text_lossy()); + self.command = Some(Command::Shell(sh)); } } @@ -4674,7 +4678,7 @@ impl Config { if let Some(home) = pw.home { log::info!( "default working directory src=passwd value={}", - home.as_encoded() + home.to_text_lossy() ); *wd = WorkingDirectory::Path(home); } else { @@ -4809,8 +4813,8 @@ fn probable_cli_environment() -> bool { } impl CliTarget for Config { - fn parse_into_field(&mut self, key: &str, value: Option<&str>) -> Result<(), ArgError> { - self.parse_field(key, value) + fn parse_into_field(&mut self, key: &str, value: Option<&[u8]>) -> Result<(), ArgError> { + self.parse_field_bytes(key, value) } fn valid_values(&self, key: &str) -> Option<&'static [&'static str]> { @@ -4823,24 +4827,28 @@ impl CliTarget for Config { /// Callback for the arg parser to allow us to handle special cases /// like `-e`. Returns `Ok(false)` if the CLI parsing should halt. - fn parse_manually_hook(&mut self, arg: &str, iter: &mut dyn ArgIter) -> Result { - if arg == "-e" { + fn parse_manually_hook( + &mut self, + arg: &[u8], + iter: &mut dyn ArgIter, + ) -> Result { + if arg == b"-e" { // Add the special -e marker. This prevents: // (1) config-file from adding args to the end (see #2908) // (2) dark/light theme from making this conditional self.replay_steps.push(ReplayStep::DashE); // Build up the command. - let mut command: Vec = Vec::new(); + let mut command: Vec = Vec::new(); while let Some(param) = iter.next_arg() { self.replay_steps.push(ReplayStep::Arg(param.clone())); - command.push(param); + command.push(EncodedOs::from_bytes(¶m)); } if command.is_empty() { self.diagnostics.append(Diagnostic { location: iter.location().unwrap_or_default(), - message: format!("missing command after {arg}"), + message: "missing command after -e".to_owned(), ..Diagnostic::default() }); @@ -4861,13 +4869,13 @@ impl CliTarget for Config { } // Keep track of our input args for replay - self.replay_steps.push(ReplayStep::Arg(arg.to_string())); + self.replay_steps.push(ReplayStep::Arg(arg.to_vec())); // If we didn't find a special case, continue parsing normally Ok(true) } - fn compatibility(&mut self, key: &str, value: Option<&str>) -> bool { + fn compatibility(&mut self, key: &str, value: Option<&[u8]>) -> bool { match key { // Ghostty 1.1 introduced background-blur support for Linux which // doesn't support a specific radius value. The renaming is to let @@ -4886,7 +4894,7 @@ impl CliTarget for Config { // Ghostty 1.2 removed the `hidden` value from `gtk-tabs-location` and // moved it to `window-show-tab-bar`. "gtk-tabs-location" => { - if value.unwrap_or("") == "hidden" { + if value.unwrap_or(b"") == b"hidden" { self.window_show_tab_bar = WindowShowTabBar::Never; return true; } @@ -4907,7 +4915,9 @@ impl CliTarget for Config { // Realistically, these fields were mutually exclusive so anyone // relying on that behavior should just upgrade to the new // cursor-color/cursor-text fields. - let Ok(set) = cli_args::parse_bool(value.unwrap_or("t")) else { + let Ok(set) = cli_args::text_value(value) + .and_then(|v| cli_args::parse_bool(v.unwrap_or("t"))) + else { return false; }; if set { @@ -4917,7 +4927,9 @@ impl CliTarget for Config { true } "selection-invert-fg-bg" => { - let Ok(set) = cli_args::parse_bool(value.unwrap_or("t")) else { + let Ok(set) = cli_args::text_value(value) + .and_then(|v| cli_args::parse_bool(v.unwrap_or("t"))) + else { return false; }; if set { @@ -4930,7 +4942,9 @@ impl CliTarget for Config { // Ghostty 1.2 merged `bold-is-bright` into the new `bold-color` // by setting the value to "bright". "bold-is-bright" => { - let Ok(isset) = cli_args::parse_bool(value.unwrap_or("t")) else { + let Ok(isset) = cli_args::text_value(value) + .and_then(|v| cli_args::parse_bool(v.unwrap_or("t"))) + else { return false; }; if isset { @@ -4942,7 +4956,7 @@ impl CliTarget for Config { // Ghostty 1.2 removed the "desktop" option and renamed it to "detect". // The semantics also changed slightly but this is the correct mapping. "gtk-single-instance" => { - if value.unwrap_or("") == "desktop" { + if value.unwrap_or(b"") == b"desktop" { self.gtk_single_instance = GtkSingleInstance::Detect; return true; } @@ -4952,7 +4966,7 @@ impl CliTarget for Config { // Ghostty 1.3 rename the "window" option to "new-window". // See: https://github.com/ghostty-org/ghostty/pull/9764 "macos-dock-drop-behavior" => { - if value.unwrap_or("") == "window" { + if value.unwrap_or(b"") == b"window" { self.macos_dock_drop_behavior = MacOSDockDropBehavior::NewWindow; return true; } @@ -4983,7 +4997,7 @@ mod tests { let Command::Shell(text) = shell_from_env(std::ffi::OsStr::from_bytes(raw)) else { panic!("not a shell command"); }; - assert_eq!(ghostty_os::osbytes::config_str_to_bytes(&text), raw); + assert_eq!(text.to_bytes(), raw); } let Command::Shell(plain) = shell_from_env(std::ffi::OsStr::new("/bin/zsh")) else { panic!() @@ -5063,7 +5077,7 @@ mod tests { fn parse_hook_invalid_command() { let mut cfg = Config::default(); let mut i = it(&["foo"]); - assert!(cfg.parse_manually_hook("--command", &mut i).unwrap()); + assert!(cfg.parse_manually_hook(b"--command", &mut i).unwrap()); assert!(cfg.command.is_none()); } @@ -5071,7 +5085,7 @@ mod tests { fn parse_e_command_only() { let mut cfg = Config::default(); let mut i = it(&["foo"]); - assert!(!cfg.parse_manually_hook("-e", &mut i).unwrap()); + assert!(!cfg.parse_manually_hook(b"-e", &mut i).unwrap()); let Some(Command::Direct(cmd)) = &cfg.initial_command else { panic!("expected direct"); @@ -5083,7 +5097,7 @@ mod tests { fn parse_e_command_and_args() { let mut cfg = Config::default(); let mut i = it(&["echo", "foo", "bar baz"]); - assert!(!cfg.parse_manually_hook("-e", &mut i).unwrap()); + assert!(!cfg.parse_manually_hook(b"-e", &mut i).unwrap()); let Some(Command::Direct(cmd)) = &cfg.initial_command else { panic!("expected direct"); diff --git a/crates/ghostty-config/src/edit.rs b/crates/ghostty-config/src/edit.rs index 090d679..b1f6bee 100644 --- a/crates/ghostty-config/src/edit.rs +++ b/crates/ghostty-config/src/edit.rs @@ -21,7 +21,7 @@ pub fn open_path() -> io::Result { // The config path is an encoded OS string: decode before touching the // filesystem. - let real_path = config_path.to_path(); + let real_path = config_path.to_path()?; // Create config directory recursively. if let Some(config_dir) = real_path.parent() { @@ -55,7 +55,11 @@ fn config_path() -> io::Result { // exists. let mut exists: Option<&EncodedOs> = None; for path in &paths { - let meta = match std::fs::File::open(path.to_path()).and_then(|f| f.metadata()) { + let meta = match path + .to_path() + .and_then(std::fs::File::open) + .and_then(|f| f.metadata()) + { Ok(m) => m, // File doesn't exist, continue. Err(e) if e.kind() == io::ErrorKind::NotFound => continue, @@ -115,7 +119,7 @@ mod tests { let value = std::ffi::OsString::from_vec(xdg.as_os_str().as_bytes().to_vec()); temp_env::with_var("XDG_CONFIG_HOME", Some(&value), || { let p = open_path().unwrap(); - let real = p.to_path(); + let real = p.to_path().unwrap(); assert_eq!(real, xdg.join("ghostty/config.ghostty")); assert!(real.is_file()); // Nothing was created at the escaped (encoded) pathname. diff --git a/crates/ghostty-config/src/file_load.rs b/crates/ghostty-config/src/file_load.rs index 88f7ec0..1d68493 100644 --- a/crates/ghostty-config/src/file_load.rs +++ b/crates/ghostty-config/src/file_load.rs @@ -159,7 +159,10 @@ impl std::error::Error for OpenFileError {} /// if it exists and is non-empty. This also constrains the possible /// errors to a smaller set that we can explicitly handle. pub fn open(path: &EncodedOs) -> Result { - let real = path.to_path(); + let real = path.to_path().map_err(|err| { + log::warn!("invalid config path: {err}"); + OpenFileError::FileOpenFailed + })?; assert!(real.is_absolute()); let file = File::open(&real).map_err(|err| { @@ -221,16 +224,16 @@ mod tests { std::fs::create_dir_all(&dir).unwrap(); let new = EncodedOs::from_path(&dir.join("config.ghostty")); let legacy = EncodedOs::from_path(&dir.join("config")); - let _ = std::fs::remove_file(new.to_path()); - let _ = std::fs::remove_file(legacy.to_path()); + let _ = std::fs::remove_file(new.to_path().unwrap()); + let _ = std::fs::remove_file(legacy.to_path().unwrap()); // Neither exists: the new one. assert_eq!(preferred_of(new.clone(), legacy.clone()), new); // Only legacy exists: the legacy one. - std::fs::write(legacy.to_path(), "a=b").unwrap(); + std::fs::write(legacy.to_path().unwrap(), "a=b").unwrap(); assert_eq!(preferred_of(new.clone(), legacy.clone()), legacy); // Both exist: the new one. - std::fs::write(new.to_path(), "a=b").unwrap(); + std::fs::write(new.to_path().unwrap(), "a=b").unwrap(); assert_eq!(preferred_of(new.clone(), legacy.clone()), new); std::fs::remove_dir_all(&dir).unwrap(); } @@ -241,17 +244,17 @@ mod tests { std::fs::create_dir_all(&dir).unwrap(); let app = EncodedOs::from_path(&dir.join("app")); let xdg_p = EncodedOs::from_path(&dir.join("xdg")); - let _ = std::fs::remove_file(app.to_path()); - let _ = std::fs::remove_file(xdg_p.to_path()); + let _ = std::fs::remove_file(app.to_path().unwrap()); + let _ = std::fs::remove_file(xdg_p.to_path().unwrap()); let xdg_fn = || Ok(xdg_p.clone()); // Neither exists: app support. assert_eq!(preferred_macos(app.clone(), &xdg_fn).unwrap(), app); // XDG exists only: XDG. - std::fs::write(xdg_p.to_path(), "a=b").unwrap(); + std::fs::write(xdg_p.to_path().unwrap(), "a=b").unwrap(); assert_eq!(preferred_macos(app.clone(), &xdg_fn).unwrap(), xdg_p); // App support exists: app support wins. - std::fs::write(app.to_path(), "a=b").unwrap(); + std::fs::write(app.to_path().unwrap(), "a=b").unwrap(); assert_eq!(preferred_macos(app.clone(), &xdg_fn).unwrap(), app); std::fs::remove_dir_all(&dir).unwrap(); } diff --git a/crates/ghostty-config/src/font_types.rs b/crates/ghostty-config/src/font_types.rs index 33a0d0c..083e52d 100644 --- a/crates/ghostty-config/src/font_types.rs +++ b/crates/ghostty-config/src/font_types.rs @@ -40,13 +40,13 @@ pub struct Descriptor { /// /// On systems that use fontconfig (Linux), this can be a full /// fontconfig pattern, such as "Fira Code-14:bold". - pub family: Option, + pub family: Option, /// Specific font style to search for. This will filter the style /// string the font advertises. The "bold/italic" booleans later in this /// struct filter by the style trait the font has, not the string, so /// these can be used in conjunction or not. - pub style: Option, + pub style: Option, /// A codepoint that this font must be able to render. pub codepoint: u32, @@ -184,11 +184,25 @@ mod tests { let mut m = CodepointMap::default(); assert!(m.get(1).is_none()); m.add(e(1, 1, "A")); - assert_eq!(m.get(1).unwrap().family.as_deref(), Some("A")); + assert_eq!( + m.get(1) + .unwrap() + .family + .as_ref() + .map(ghostty_os::osbytes::EncodedOs::as_encoded), + Some("A") + ); // Later entry takes priority m.add(e(1, 2, "B")); - assert_eq!(m.get(1).unwrap().family.as_deref(), Some("B")); + assert_eq!( + m.get(1) + .unwrap() + .family + .as_ref() + .map(ghostty_os::osbytes::EncodedOs::as_encoded), + Some("B") + ); // Non-matching assert!(m.get(0).is_none()); @@ -196,8 +210,29 @@ mod tests { m.add(e(3, 4, "C")); m.add(e(5, 6, "D")); - assert_eq!(m.get(3).unwrap().family.as_deref(), Some("C")); - assert_eq!(m.get(6).unwrap().family.as_deref(), Some("D")); - assert_eq!(m.get(2).unwrap().family.as_deref(), Some("B")); + assert_eq!( + m.get(3) + .unwrap() + .family + .as_ref() + .map(ghostty_os::osbytes::EncodedOs::as_encoded), + Some("C") + ); + assert_eq!( + m.get(6) + .unwrap() + .family + .as_ref() + .map(ghostty_os::osbytes::EncodedOs::as_encoded), + Some("D") + ); + assert_eq!( + m.get(2) + .unwrap() + .family + .as_ref() + .map(ghostty_os::osbytes::EncodedOs::as_encoded), + Some("B") + ); } } diff --git a/crates/ghostty-config/src/input_fields.rs b/crates/ghostty-config/src/input_fields.rs index 89b9284..9938cf6 100644 --- a/crates/ghostty-config/src/input_fields.rs +++ b/crates/ghostty-config/src/input_fields.rs @@ -12,7 +12,8 @@ use crate::{ }; impl ConfigField for KeyRemapSet { - fn parse_cli(&mut self, value: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, value: Option<&[u8]>) -> Result<(), ArgError> { + let value = crate::cli_args::text_value(value)?; self.parse_cli(value).map_err(|_| ArgError::InvalidValue) } @@ -40,7 +41,8 @@ impl ConfigField for OptionAsAlt { }) } - fn parse_cli(&mut self, value: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, value: Option<&[u8]>) -> Result<(), ArgError> { + let value = crate::cli_args::text_value(value)?; let v = value.ok_or(ArgError::ValueRequired)?; *self = match v { "false" => OptionAsAlt::False, diff --git a/crates/ghostty-config/src/io.rs b/crates/ghostty-config/src/io.rs index 97ea38b..9b092ed 100644 --- a/crates/ghostty-config/src/io.rs +++ b/crates/ghostty-config/src/io.rs @@ -5,7 +5,7 @@ use std::fmt; use ghostty_os::osbytes::EncodedOs; use crate::{ - cli_args::{ArgError, ConfigField, split_tagged_union}, + cli_args::{ArgError, ConfigField}, formatter::EntryFormatter, string, }; @@ -18,16 +18,16 @@ use crate::{ /// might be produced in an intermediate state. #[derive(Debug, Clone, PartialEq, Eq)] pub enum ReadableIO { - /// Raw bytes in the reversible OS-byte encoding (decode with - /// `osbytes::config_str_to_bytes` after [`ReadableIO::clone_parsed`]). - Raw(String), + /// Raw bytes, available with `EncodedOs::to_bytes` after + /// [`ReadableIO::clone_parsed`] resolves string escapes. + Raw(EncodedOs), /// A file path (decode with `EncodedOs::to_path` before opening). Path(EncodedOs), } impl Default for ReadableIO { fn default() -> Self { - ReadableIO::Raw(String::new()) + ReadableIO::Raw(EncodedOs::new()) } } @@ -36,18 +36,20 @@ impl ReadableIO { /// the final resulting value all at once. #[must_use] pub fn clone_parsed(&self) -> Self { - // Parsing can't fail because we validate it in parse_cli + // Parsing can't fail because we validate it in parse_bytes match self { - ReadableIO::Raw(v) => ReadableIO::Raw(string::parse(v).unwrap_or_default()), - ReadableIO::Path(v) => ReadableIO::Path(EncodedOs::from_encoded( - string::parse(v.as_encoded()).unwrap_or_default(), + ReadableIO::Raw(v) => ReadableIO::Raw(EncodedOs::from_bytes( + &string::parse_bytes(v.to_bytes()).unwrap_or_default(), + )), + ReadableIO::Path(v) => ReadableIO::Path(EncodedOs::from_bytes( + &string::parse_bytes(v.to_bytes()).unwrap_or_default(), )), } } } impl ConfigField for ReadableIO { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { let input = input.ok_or(ArgError::ValueRequired)?; if input.is_empty() { return Err(ArgError::ValueRequired); @@ -55,22 +57,22 @@ impl ConfigField for ReadableIO { // We only validate that string parsing works. We store the value // as raw so that our formatting can recreate it. - string::parse(input)?; + string::parse_bytes(input)?; // Next, parse the tagged union using normal rules. Invalid values in // the tagged union are interpreted as raw values. - let (tag, rest) = split_tagged_union(input); + let (tag, rest) = crate::cli_args::split_tagged_union_bytes(input); *self = match tag { - "raw" => ReadableIO::Raw(rest.to_string()), - "path" => ReadableIO::Path(EncodedOs::from_encoded(rest)), - _ => ReadableIO::Raw(input.to_string()), + b"raw" => ReadableIO::Raw(EncodedOs::from_bytes(rest)), + b"path" => ReadableIO::Path(EncodedOs::from_bytes(rest)), + _ => ReadableIO::Raw(EncodedOs::from_bytes(input)), }; Ok(()) } fn format_entry(&self, f: &mut EntryFormatter<'_>) -> fmt::Result { let (tag, v) = match self { - ReadableIO::Raw(v) => ("raw", v.as_str()), + ReadableIO::Raw(v) => ("raw", v.as_encoded()), ReadableIO::Path(v) => ("path", v.as_encoded()), }; writeln!(f.writer, "{} = {tag}:{v}", f.name) @@ -93,7 +95,7 @@ impl RepeatableReadableIO { } impl ConfigField for RepeatableReadableIO { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { let value = input.ok_or(ArgError::ValueRequired)?; // Empty value resets the list @@ -103,7 +105,7 @@ impl ConfigField for RepeatableReadableIO { } let mut io = ReadableIO::default(); - io.parse_cli(Some(value))?; + io.parse_bytes(Some(value))?; self.list.push(io); Ok(()) } @@ -139,8 +141,7 @@ mod tests { #[test] fn readable_io_path_decodes_to_the_original_bytes() { let mut io = ReadableIO::default(); - let enc = EncodedOs::from_bytes(b"/tmp/\xffin").into_encoded(); - io.parse_cli(Some(&format!("path:{enc}"))).unwrap(); + io.parse_bytes(Some(b"path:/tmp/\xffin")).unwrap(); let ReadableIO::Path(p) = io.clone_parsed() else { panic!("path expected") }; diff --git a/crates/ghostty-config/src/keybinds.rs b/crates/ghostty-config/src/keybinds.rs index 16170b1..11a8084 100644 --- a/crates/ghostty-config/src/keybinds.rs +++ b/crates/ghostty-config/src/keybinds.rs @@ -24,6 +24,7 @@ use ghostty_input::{ set::{EntryFormatter as BindingEntryFormatter, Value}, }, }; +use ghostty_os::osbytes::EncodedOs; use crate::{ cli_args::{ArgError, ConfigField}, @@ -38,7 +39,7 @@ use crate::{ pub enum ChainTarget { #[default] Root, - Table(String), + Table(EncodedOs), } #[derive(Debug, Clone, Default)] @@ -48,7 +49,7 @@ pub struct Keybinds { /// Defined key tables. The default key table is always the root "set", /// which allows all table names to be available without reservation. /// Insertion ordered. - pub tables: Vec<(String, Set)>, + pub tables: Vec<(EncodedOs, Set)>, /// The most recent binding target for `chain=` additions. pub chain_target: ChainTarget, @@ -445,27 +446,30 @@ impl Keybinds { // others are used to these keys so we set them as defaults. If // people want to get back to the fixterm encoding they can set // the keybinds to `unbind`. - set.put(phys(Key::ArrowRight, sup), Action::Text("\\x05".to_owned())); - set.put(phys(Key::ArrowLeft, sup), Action::Text("\\x01".to_owned())); - set.put(phys(Key::Backspace, sup), Action::Text("\\x15".to_owned())); + set.put(phys(Key::ArrowRight, sup), Action::Text("\\x05".into())); + set.put(phys(Key::ArrowLeft, sup), Action::Text("\\x01".into())); + set.put(phys(Key::Backspace, sup), Action::Text("\\x15".into())); set.put( phys(Key::ArrowLeft, mods(false, false, true, false)), - Action::Esc("b".to_owned()), + Action::Esc("b".into()), ); set.put( phys(Key::ArrowRight, mods(false, false, true, false)), - Action::Esc("f".to_owned()), + Action::Esc("f".into()), ); } - fn table_index(&self, name: &str) -> Option { + fn table_index(&self, name: &EncodedOs) -> Option { self.tables.iter().position(|(n, _)| n == name) } /// Get a table by name. #[must_use] pub fn table(&self, name: &str) -> Option<&Set> { - self.tables.iter().find(|(n, _)| n == name).map(|(_, s)| s) + self.tables + .iter() + .find(|(n, _)| n == &EncodedOs::from_plain_text(name)) + .map(|(_, s)| s) } /// Like format_entry but has an option to include docs. @@ -483,14 +487,18 @@ impl Keybinds { f.writer.write_str("\n")?; } - let mut buf = format!("{}", k.0); + let mut buf = ghostty_os::osbytes::valid_str_to_config_string(&k.0.to_string()); emit(v, &mut buf, f)?; } // Format table bindings for (table_name, table_set) in &self.tables { for (k, v) in &table_set.bindings { - let mut buf = format!("{}/{}", table_name, k.0); + let mut buf = format!( + "{}/{}", + table_name.as_encoded(), + ghostty_os::osbytes::valid_str_to_config_string(&k.0.to_string()) + ); emit(v, &mut buf, f)?; } } @@ -506,7 +514,9 @@ struct Adapter<'a, 'b> { impl BindingEntryFormatter for Adapter<'_, '_> { fn format_entry(&mut self, value: &str) { if self.result.is_ok() { - self.result = self.f.format_entry(&value.to_string()); + self.result = self + .f + .format_entry(&ghostty_os::osbytes::EncodedOs::from_encoded(value)); } } } @@ -562,7 +572,12 @@ impl PartialEq for Keybinds { return false; } for (name, set) in &self.tables { - let Some(other_set) = other.table(name) else { + let Some(other_set) = other + .tables + .iter() + .find(|(n, _)| n == name) + .map(|(_, set)| set) + else { return false; }; if !equal_set(set, other_set) { @@ -574,7 +589,7 @@ impl PartialEq for Keybinds { } impl ConfigField for Keybinds { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { let value = input.ok_or(ArgError::ValueRequired)?; // Check for special values @@ -584,7 +599,7 @@ impl ConfigField for Keybinds { return Ok(()); } - if value == "clear" { + if value == b"clear" { log::info!("config has 'keybind = clear', all keybinds cleared"); self.set = Set::default(); self.tables = Vec::new(); @@ -595,28 +610,29 @@ impl ConfigField for Keybinds { // Check for table syntax: "name/" or "name/binding" // We look for '/' only before the first '=' to avoid matching // action arguments like "foo=text:/hello". - let eq_idx = value.find('=').unwrap_or(value.len()); - if let Some(slash_idx) = value[..eq_idx].find('/') { + let eq_idx = value.iter().position(|b| *b == b'=').unwrap_or(value.len()); + if let Some(slash_idx) = value[..eq_idx].iter().position(|b| *b == b'/') { let table_name = &value[..slash_idx]; // Length zero is valid, so you can set `/=action` for the slash key. // Ignore '+', '>' because they can be part of sequences and // triggers. This lets things like `ctrl+/=action` work. - if !table_name.is_empty() && !table_name.contains(['+', '>']) { + if !table_name.is_empty() && !table_name.iter().any(|b| matches!(b, b'+' | b'>')) { let binding = &value[slash_idx + 1..]; + let table_name = EncodedOs::from_bytes(table_name); // Get or create the table - let idx = if let Some(i) = self.table_index(table_name) { + let idx = if let Some(i) = self.table_index(&table_name) { i } else { - self.tables.push((table_name.to_string(), Set::default())); + self.tables.push((table_name.clone(), Set::default())); self.tables.len() - 1 }; // If there's no binding after the slash, this is a table // definition/clear command if binding.is_empty() { - log::debug!("config has 'keybind = {table_name}/', table cleared"); + log::debug!("config has 'keybind = {table_name:?}/', table cleared"); self.tables[idx].1 = Set::default(); self.chain_target = ChainTarget::Root; return Ok(()); @@ -624,23 +640,23 @@ impl ConfigField for Keybinds { // Chains are only allowed at the root level. Their target is // tracked globally by parse order in `self.chain_target`. - if binding.starts_with("chain=") { + if binding.starts_with(b"chain=") { return Err(ArgError::other("InvalidFormat")); } // Parse and add the binding to the table self.tables[idx] .1 - .parse_and_put(binding) + .parse_and_put_bytes(binding) .map_err(to_arg_error)?; - self.chain_target = ChainTarget::Table(table_name.to_string()); + self.chain_target = ChainTarget::Table(table_name.clone()); return Ok(()); } } - if value.starts_with("chain=") { + if value.starts_with(b"chain=") { match self.chain_target.clone() { - ChainTarget::Root => self.set.parse_and_put(value).map_err(to_arg_error)?, + ChainTarget::Root => self.set.parse_and_put_bytes(value).map_err(to_arg_error)?, ChainTarget::Table(table_name) => { let Some(idx) = self.table_index(&table_name) else { self.chain_target = ChainTarget::Root; @@ -648,7 +664,7 @@ impl ConfigField for Keybinds { }; self.tables[idx] .1 - .parse_and_put(value) + .parse_and_put_bytes(value) .map_err(to_arg_error)?; } } @@ -656,7 +672,7 @@ impl ConfigField for Keybinds { } // Parse into default set - self.set.parse_and_put(value).map_err(to_arg_error)?; + self.set.parse_and_put_bytes(value).map_err(to_arg_error)?; self.chain_target = ChainTarget::Root; Ok(()) } @@ -1037,7 +1053,7 @@ mod tests { let Some(Value::Leaf(l)) = lookup(&kb, phys(Key::ArrowLeft, sup)) else { panic!() }; - assert_eq!(l.action, Action::Text("\\x01".to_owned())); + assert_eq!(l.action, Action::Text("\\x01".into())); // ctrlOrSuper picks super: super+comma opens the config. let Some(Value::Leaf(l)) = lookup(&kb, uni(',', sup)) else { diff --git a/crates/ghostty-config/src/macros.rs b/crates/ghostty-config/src/macros.rs index e2365d5..e3ab37f 100644 --- a/crates/ghostty-config/src/macros.rs +++ b/crates/ghostty-config/src/macros.rs @@ -103,22 +103,28 @@ macro_rules! config_struct { } } + /// Parse a plain UTF-8 value supplied programmatically. + #[allow(dead_code)] + pub fn parse_field(&mut self, key: &str, value: Option<&str>) -> Result<(), $crate::cli_args::ArgError> { + self.parse_field_bytes(key, value.map(str::as_bytes)) + } + #[allow(dead_code)] /// Parse a single key/value into the matching field. Empty /// values reset the field to its default. - pub fn parse_field( + pub fn parse_field_bytes( &mut self, key: &str, - value: ::core::option::Option<&str>, + value: ::core::option::Option<&[u8]>, ) -> ::core::result::Result<(), $crate::cli_args::ArgError> { match key { $( $key => { - if value == ::core::option::Option::Some("") { + if value == ::core::option::Option::Some(b"".as_slice()) { $crate::cli_args::ConfigField::reset(&mut self.$field, $default); return ::core::result::Result::Ok(()); } - $crate::cli_args::ConfigField::parse_cli(&mut self.$field, value) + $crate::cli_args::ConfigField::parse_bytes(&mut self.$field, value) } )* _ => ::core::result::Result::Err($crate::cli_args::ArgError::InvalidField), @@ -215,11 +221,11 @@ macro_rules! config_enum { const VALID_VALUES: ::core::option::Option<&'static [&'static str]> = ::core::option::Option::Some(Self::NAMES); - fn parse_cli( + fn parse_bytes( &mut self, - value: ::core::option::Option<&str>, + value: ::core::option::Option<&[u8]>, ) -> ::core::result::Result<(), $crate::cli_args::ArgError> { - let v = value.ok_or($crate::cli_args::ArgError::ValueRequired)?; + let v = $crate::cli_args::text_value(value)?.ok_or($crate::cli_args::ArgError::ValueRequired)?; *self = Self::from_name(v).ok_or($crate::cli_args::ArgError::InvalidValue)?; ::core::result::Result::Ok(()) } @@ -295,11 +301,11 @@ macro_rules! config_packed { $crate::c_value::CValue::Packed(bits) } - fn parse_cli( + fn parse_bytes( &mut self, - value: ::core::option::Option<&str>, + value: ::core::option::Option<&[u8]>, ) -> ::core::result::Result<(), $crate::cli_args::ArgError> { - let v = value.ok_or($crate::cli_args::ArgError::ValueRequired)?; + let v = $crate::cli_args::text_value(value)?.ok_or($crate::cli_args::ArgError::ValueRequired)?; let mut result = <$name as ::core::default::Default>::default(); $crate::cli_args::parse_packed_struct_with(v, Self::NAMES, |i, b| { result.set_index(i, b); diff --git a/crates/ghostty-config/src/metric_modifier.rs b/crates/ghostty-config/src/metric_modifier.rs index 7e8bc1a..29d3f04 100644 --- a/crates/ghostty-config/src/metric_modifier.rs +++ b/crates/ghostty-config/src/metric_modifier.rs @@ -99,7 +99,8 @@ impl MetricModifier { } impl ConfigField for MetricModifier { - fn parse_cli(&mut self, value: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, value: Option<&[u8]>) -> Result<(), ArgError> { + let value = crate::cli_args::text_value(value)?; *self = Self::parse_cli_value(value)?; Ok(()) } diff --git a/crates/ghostty-config/src/path.rs b/crates/ghostty-config/src/path.rs index 34f5000..94c058d 100644 --- a/crates/ghostty-config/src/path.rs +++ b/crates/ghostty-config/src/path.rs @@ -33,8 +33,8 @@ impl Default for Path { } /// Lexically resolve `base/path` like `std.fs.path.resolve`. -fn resolve_lexical(base: &EncodedOs, path: &EncodedOs) -> EncodedOs { - let joined = base.to_path().join(path.to_path()); +fn resolve_lexical(base: &EncodedOs, path: &EncodedOs) -> std::io::Result { + let joined = base.to_path()?.join(path.to_path()?); let mut out = PathBuf::new(); for c in joined.components() { match c { @@ -45,7 +45,7 @@ fn resolve_lexical(base: &EncodedOs, path: &EncodedOs) -> EncodedOs { other => out.push(other.as_os_str()), } } - EncodedOs::from_path(&out) + Ok(EncodedOs::from_path(&out)) } impl Path { @@ -75,27 +75,31 @@ impl Path { /// /// Returns `Ok(None)` for an empty input. pub fn parse(input: Option<&str>) -> Result, ArgError> { + Self::parse_bytes(input.map(str::as_bytes)) + } + + pub fn parse_bytes(input: Option<&[u8]>) -> Result, ArgError> { let mut value = input.ok_or(ArgError::ValueRequired)?; if value.is_empty() { return Ok(None); } - let optional = if let Some(rest) = value.strip_prefix('?') { + let optional = if let Some(rest) = value.strip_prefix(b"?") { value = rest; true } else { false }; - if value.len() >= 2 && value.starts_with('"') && value.ends_with('"') { + if value.len() >= 2 && value.starts_with(b"\"") && value.ends_with(b"\"") { value = &value[1..value.len() - 1]; } Ok(Some(if optional { - Path::Optional(EncodedOs::from_encoded(value)) + Path::Optional(EncodedOs::from_bytes(value)) } else { - Path::Required(EncodedOs::from_encoded(value)) + Path::Required(EncodedOs::from_bytes(value)) })) } @@ -107,13 +111,13 @@ impl Path { /// only returned if the base directory cannot be opened. pub fn expand(&mut self, base: &EncodedOs, diags: &mut DiagnosticList) -> std::io::Result<()> { // Filesystem calls use the decoded OS paths. - let base_path = base.to_path(); + let base_path = base.to_path()?; assert!(base_path.is_absolute()); let path = self.path().clone(); // If it is already absolute we can ignore it. - if path.is_empty() || path.to_path().is_absolute() { + if path.is_empty() || path.to_path()?.is_absolute() { return Ok(()); } @@ -126,7 +130,7 @@ impl Path { Ok(expanded) => { log::debug!( "expanding file path from home directory: path={}", - expanded.as_encoded() + expanded.to_text_lossy() ); *self.path_mut() = expanded; } @@ -150,13 +154,13 @@ impl Path { // Make sure the base directory can be opened. std::fs::read_dir(&base_path)?; - let full = base_path.join(path.to_path()); + let full = base_path.join(path.to_path()?); let abs = match std::fs::canonicalize(&full) { Ok(p) => EncodedOs::from_path(&p), Err(err) if err.kind() == std::io::ErrorKind::NotFound => { // The file doesn't exist. Try to resolve the relative path // another way. - resolve_lexical(base, &path) + resolve_lexical(base, &path)? } Err(err) => { diags.append(Diagnostic { @@ -172,8 +176,8 @@ impl Path { log::debug!( "expanding file path relative={} abs={}", - path.as_encoded(), - abs.as_encoded() + path.to_text_lossy(), + abs.to_text_lossy() ); *self.path_mut() = abs; Ok(()) @@ -195,8 +199,8 @@ impl ConfigField for Path { } /// Parse CLI option. The value will be overwritten. - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { - let Some(item) = Path::parse(input)? else { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { + let Some(item) = Path::parse_bytes(input)? else { return Ok(()); }; if item.is_empty() { @@ -237,8 +241,8 @@ impl RepeatablePath { impl ConfigField for RepeatablePath { const KIND: crate::cli_args::FieldKind = crate::cli_args::FieldKind::RepeatablePath; - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { - let Some(item) = Path::parse(input)? else { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { + let Some(item) = Path::parse_bytes(input)? else { self.value.clear(); return Ok(()); }; @@ -403,7 +407,7 @@ mod tests { p.expand(&EncodedOs::from_path(&base), &mut diags).unwrap(); assert_eq!(diags.items().len(), 0); // The canonical (non-UTF-8) target is preserved byte for byte. - assert_eq!(p.path().to_path(), target); - assert!(std::fs::metadata(p.path().to_path()).is_ok()); + assert_eq!(p.path().to_path().unwrap(), target); + assert!(std::fs::metadata(p.path().to_path().unwrap()).is_ok()); } } diff --git a/crates/ghostty-config/src/repeatable_string_map.rs b/crates/ghostty-config/src/repeatable_string_map.rs index afb6168..5dc5797 100644 --- a/crates/ghostty-config/src/repeatable_string_map.rs +++ b/crates/ghostty-config/src/repeatable_string_map.rs @@ -4,30 +4,31 @@ use std::fmt; +use ghostty_os::osbytes::EncodedOs; + use crate::{ cli_args::{ArgError, ConfigField}, formatter::EntryFormatter, }; -/// std.ascii.whitespace -fn is_ascii_ws(c: char) -> bool { - matches!(c, ' ' | '\t' | '\n' | '\r' | '\x0b' | '\x0c') -} - /// An insertion-ordered string map. #[derive(Debug, Clone, Default)] pub struct RepeatableStringMap { - map: Vec<(String, String)>, + map: Vec<(EncodedOs, EncodedOs)>, } impl PartialEq for RepeatableStringMap { /// Order-insensitive comparison (Zig: `equal`). fn eq(&self, other: &Self) -> bool { self.map.len() == other.map.len() - && self - .map - .iter() - .all(|(k, v)| other.get(k).is_some_and(|ov| ov == v)) + && self.map.iter().all(|(k, v)| { + other + .map + .iter() + .find(|(key, _)| key == k) + .map(|(_, v)| v) + .is_some_and(|ov| ov == v) + }) } } @@ -39,29 +40,35 @@ impl RepeatableStringMap { } #[must_use] - pub fn get(&self, key: &str) -> Option<&str> { + pub fn get(&self, key: &str) -> Option<&EncodedOs> { self.map .iter() - .find(|(k, _)| k == key) - .map(|(_, v)| v.as_str()) + .find(|(k, _)| k == &EncodedOs::from_plain_text(key)) + .map(|(_, v)| v) } /// Insert or replace an entry (Zig: `map.put`). pub fn put(&mut self, key: &str, value: &str) { - match self.map.iter_mut().find(|(k, _)| k == key) { - Some((_, v)) => value.clone_into(v), - None => self.map.push((key.to_owned(), value.to_owned())), + self.put_bytes(key.as_bytes(), value.as_bytes()); + } + + pub fn put_bytes(&mut self, key: &[u8], value: &[u8]) { + let key = EncodedOs::from_bytes(key); + let value = EncodedOs::from_bytes(value); + match self.map.iter_mut().find(|(k, _)| k == &key) { + Some((_, v)) => *v = value, + None => self.map.push((key, value)), } } /// Iterator over the entries in the map (insertion order). - pub fn iter(&self) -> impl Iterator { - self.map.iter().map(|(k, v)| (k.as_str(), v.as_str())) + pub fn iter(&self) -> impl Iterator { + self.map.iter().map(|(k, v)| (k, v)) } } impl ConfigField for RepeatableStringMap { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { let value = input.ok_or(ArgError::ValueRequired)?; // Empty value resets the list. @@ -70,21 +77,25 @@ impl ConfigField for RepeatableStringMap { return Ok(()); } - let index = value.find('=').ok_or(ArgError::ValueRequired)?; - let key = value[..index].trim_matches(is_ascii_ws); - let val = value[index + 1..].trim_matches(is_ascii_ws); + let index = value + .iter() + .position(|b| *b == b'=') + .ok_or(ArgError::ValueRequired)?; + let key = EncodedOs::from_bytes(crate::cli_args::trim_ascii_whitespace(&value[..index])); + let val = + EncodedOs::from_bytes(crate::cli_args::trim_ascii_whitespace(&value[index + 1..])); // Empty value removes the key from the map. if val.is_empty() { - if let Some(pos) = self.map.iter().position(|(k, _)| k == key) { + if let Some(pos) = self.map.iter().position(|(k, _)| k == &key) { self.map.remove(pos); } return Ok(()); } - match self.map.iter_mut().find(|(k, _)| k == key) { - Some(entry) => entry.1 = val.to_string(), - None => self.map.push((key.to_string(), val.to_string())), + match self.map.iter_mut().find(|(k, _)| k == &key) { + Some(entry) => entry.1 = val, + None => self.map.push((key, val)), } Ok(()) } @@ -95,7 +106,13 @@ impl ConfigField for RepeatableStringMap { return writeln!(f.writer, "{} = ", f.name); } for (k, v) in &self.map { - writeln!(f.writer, "{} = {k}={v}", f.name)?; + writeln!( + f.writer, + "{} = {}={}", + f.name, + k.as_encoded(), + v.as_encoded() + )?; } Ok(()) } @@ -130,7 +147,7 @@ mod tests { assert_eq!(map.count(), 1); map.parse_cli(Some("A=C")).unwrap(); assert_eq!(map.count(), 1); - assert_eq!(map.get("A"), Some("C")); + assert_eq!(map.get("A").map(EncodedOs::as_encoded), Some("C")); // Empty value removes the key map.parse_cli(Some("A=")).unwrap(); diff --git a/crates/ghostty-config/src/string.rs b/crates/ghostty-config/src/string.rs index f746bcc..eb646a6 100644 --- a/crates/ghostty-config/src/string.rs +++ b/crates/ghostty-config/src/string.rs @@ -75,58 +75,43 @@ fn parse_escape_sequence(bytes: &[u8], i: &mut usize) -> Option { Some(cp) } -/// Parse a string literal into the original bytes (Zig `parse`'s output). -/// `input` is a config string in the reversible OS-byte encoding; the result -/// is decoded, so it can be written to the pty / OS directly. -pub fn parse_bytes(input: &str) -> Result, StringError> { - parse(input).map(|s| ghostty_os::osbytes::config_str_to_bytes(&s)) -} - -/// Parse a string literal into a string. The string can contain -/// any valid Zig string literal escape sequences. -/// -/// The input and output are in the reversible OS-byte encoding (see -/// `ghostty_os::osbytes`): bytes are copied through unchanged, and a -/// code point produced by an escape is encoded as plain text, so a -/// literal `U+FDD0` from `\u{fdd0}` stays distinct from the escape -/// character. Use [`parse_bytes`] for the raw bytes. -pub fn parse(bytes: &str) -> Result { - let b = bytes.as_bytes(); - let mut out = String::with_capacity(bytes.len()); +/// Parse literal escapes from the original bytes, copying unescaped bytes verbatim. +/// Like Zig, \xFF denotes U+00FF; only unescaped bytes bypass UTF-8 validation. +pub fn parse_bytes(input: impl AsRef<[u8]>) -> Result, StringError> { + let bytes = input.as_ref(); + let mut out = Vec::with_capacity(bytes.len()); let mut i = 0; - while i < b.len() { - if b[i] != b'\\' { - // Copy a whole UTF-8 character. - let ch = bytes[i..] - .chars() - .next() - .ok_or(StringError::InvalidString)?; - out.push(ch); - i += ch.len_utf8(); + while i < bytes.len() { + if bytes[i] != b'\\' { + out.push(bytes[i]); + i += 1; continue; } - - let cp = parse_escape_sequence(b, &mut i).ok_or(StringError::InvalidString)?; - // Surrogates cannot be encoded. + let cp = parse_escape_sequence(bytes, &mut i).ok_or(StringError::InvalidString)?; let ch = char::from_u32(cp).ok_or(StringError::InvalidString)?; - if ch == '\u{FDD0}' { - out.push_str("\u{FDD0}\u{FDD0}"); - } else { - out.push(ch); - } + out.extend_from_slice(ch.encode_utf8(&mut [0; 4]).as_bytes()); } Ok(out) } +/// Plain-text convenience; byte consumers use [`parse_bytes`]. +pub fn parse(input: &str) -> Result { + String::from_utf8(parse_bytes(input)?).map_err(|_| StringError::InvalidString) +} + /// Creates an iterator that requires no allocation to extract codepoints /// from the string literal, parsing escape sequences as it goes. #[must_use] pub fn codepoint_iterator(bytes: &str) -> CodepointIterator<'_> { + codepoint_iterator_bytes(bytes.as_bytes()) +} + +pub fn codepoint_iterator_bytes(bytes: &[u8]) -> CodepointIterator<'_> { CodepointIterator { bytes, i: 0 } } pub struct CodepointIterator<'a> { - bytes: &'a str, + bytes: &'a [u8], i: usize, } @@ -136,19 +121,31 @@ impl CodepointIterator<'_> { if self.i >= self.bytes.len() { return Ok(None); } - if self.bytes.as_bytes()[self.i] == b'\\' { + if self.bytes[self.i] == b'\\' { // An escape sequence - return parse_escape_sequence(self.bytes.as_bytes(), &mut self.i) + return parse_escape_sequence(self.bytes, &mut self.i) .map(Some) .ok_or(StringError::InvalidString); } // Not an escape, parse as UTF-8 - let ch = self.bytes[self.i..] + let len = match self.bytes[self.i] { + 0..=0x7f => 1, + 0xc2..=0xdf => 2, + 0xe0..=0xef => 3, + 0xf0..=0xf4 => 4, + _ => return Err(StringError::InvalidString), + }; + let raw = self + .bytes + .get(self.i..self.i + len) + .ok_or(StringError::InvalidString)?; + let ch = std::str::from_utf8(raw) + .map_err(|_| StringError::InvalidString)? .chars() .next() .ok_or(StringError::InvalidString)?; - self.i += ch.len_utf8(); + self.i += len; Ok(Some(u32::from(ch))) } } @@ -157,17 +154,23 @@ impl CodepointIterator<'_> { #[allow(clippy::unwrap_used)] mod tests { #[test] - fn parse_bytes_decodes_the_reversible_encoding() { + fn parse_bytes_copies_original_bytes() { use ghostty_os::osbytes::EncodedOs; use super::{parse, parse_bytes}; // Raw non-UTF-8 bytes pass through as the same bytes. let enc = EncodedOs::from_bytes(b"a\xffb\\n").into_encoded(); - assert_eq!(parse_bytes(&enc).unwrap(), b"a\xffb\n"); + assert_eq!( + parse_bytes(EncodedOs::from_encoded(enc).to_bytes()).unwrap(), + b"a\xffb\n" + ); // A literal U+FDD0 (typed or via an escape) is that character. let lit = EncodedOs::from_plain_text("\u{fdd0}").into_encoded(); - assert_eq!(parse_bytes(&lit).unwrap(), "\u{fdd0}".as_bytes()); + assert_eq!( + parse_bytes(EncodedOs::from_encoded(lit).to_bytes()).unwrap(), + "\u{fdd0}".as_bytes() + ); assert_eq!(parse_bytes("\\u{fdd0}").unwrap(), "\u{fdd0}".as_bytes()); // An escape producing U+FF is the UTF-8 encoding, not byte FF. assert_eq!(parse_bytes("\\xff").unwrap(), "\u{ff}".as_bytes()); diff --git a/crates/ghostty-config/src/theme.rs b/crates/ghostty-config/src/theme.rs index 4d02ed1..bc421b4 100644 --- a/crates/ghostty-config/src/theme.rs +++ b/crates/ghostty-config/src/theme.rs @@ -37,7 +37,7 @@ impl Location { .ok(), Location::Resources => { let base = resources_dir?; - Some(EncodedOs::from_path(&base.to_path().join("themes"))) + Some(EncodedOs::from_path(&base.to_path().ok()?.join("themes"))) } } } @@ -157,7 +157,13 @@ pub fn open( resources_dir: Option<&EncodedOs>, diags: &mut DiagnosticList, ) -> Option { - let theme_path = theme.to_path(); + let theme_path = match theme.to_path() { + Ok(path) => path, + Err(error) => { + diag(diags, format!("failed to load theme: {error}")); + return None; + } + }; let theme_text = theme.as_encoded(); // Absolute themes are loaded a different path. @@ -190,8 +196,8 @@ pub fn open( // one that exists. let mut it = LocationIterator::new(resources_dir); while let Some((_, dir)) = it.next_location() { - let path = EncodedOs::from_path(&dir.to_path().join(theme.to_os())); - match File::open(path.to_path()) { + let path = EncodedOs::from_path(&dir.to_path().ok()?.join(&theme_path)); + match path.to_path().and_then(File::open) { Ok(file) => { if !check_regular(&file, theme_text, diags) { return None; @@ -218,7 +224,7 @@ pub fn open( // our iterator, reiterate over in order to build a better error message. it.reset(); while let Some((_, dir)) = it.next_location() { - let path = EncodedOs::from_path(&dir.to_path().join(theme.to_os())); + let path = EncodedOs::from_path(&dir.to_path().ok()?.join(&theme_path)); diag( diags, format!( @@ -235,8 +241,9 @@ 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 { + let path = theme.to_path(); let theme = theme.as_encoded(); - match File::open(EncodedOs::from_encoded(theme).to_path()) { + match path.and_then(File::open) { Ok(f) => Some(f), Err(err) => { if err.kind() == std::io::ErrorKind::NotFound { diff --git a/crates/ghostty-config/src/types_a.rs b/crates/ghostty-config/src/types_a.rs index 28057fc..1555021 100644 --- a/crates/ghostty-config/src/types_a.rs +++ b/crates/ghostty-config/src/types_a.rs @@ -45,7 +45,8 @@ impl ConfigField for CursorStyle { }) } - fn parse_cli(&mut self, value: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, value: Option<&[u8]>) -> Result<(), ArgError> { + let value = crate::cli_args::text_value(value)?; let v = value.ok_or(ArgError::ValueRequired)?; *self = match v { "bar" => CursorStyle::Bar, @@ -151,25 +152,22 @@ pub enum WorkingDirectory { } impl ConfigField for WorkingDirectory { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { let input = input.ok_or(ArgError::ValueRequired)?; - let mut input = input.trim_matches(|c: char| { - // std.ascii.whitespace - matches!(c, ' ' | '\t' | '\n' | '\r' | '\x0b' | '\x0c') - }); + let mut input = crate::cli_args::trim_ascii_whitespace(input); if input.is_empty() { return Err(ArgError::ValueRequired); } // Match path.zig behavior for quoted values. - if input.len() >= 2 && input.starts_with('"') && input.ends_with('"') { + if input.len() >= 2 && input.starts_with(b"\"") && input.ends_with(b"\"") { input = &input[1..input.len() - 1]; } *self = match input { - "home" => WorkingDirectory::Home, - "inherit" => WorkingDirectory::Inherit, - _ => WorkingDirectory::Path(ghostty_os::osbytes::EncodedOs::from_encoded(input)), + b"home" => WorkingDirectory::Home, + b"inherit" => WorkingDirectory::Inherit, + _ => WorkingDirectory::Path(ghostty_os::osbytes::EncodedOs::from_bytes(input)), }; Ok(()) } @@ -199,7 +197,7 @@ impl WorkingDirectory { Err(err) => { log::warn!( "error expanding home directory for working-directory path={}: {err}", - path.as_encoded() + path.to_text_lossy() ); return; } @@ -309,7 +307,8 @@ impl ConfigField for Color { } } - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { + let input = crate::cli_args::text_value(input)?; *self = Color::from_cli(input)?; Ok(()) } @@ -355,7 +354,8 @@ impl TerminalColor { } impl ConfigField for TerminalColor { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { + let input = crate::cli_args::text_value(input)?; *self = TerminalColor::from_cli(input)?; Ok(()) } @@ -394,7 +394,8 @@ impl BoldColor { } impl ConfigField for BoldColor { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { + let input = crate::cli_args::text_value(input)?; *self = BoldColor::from_cli(input)?; Ok(()) } @@ -417,7 +418,8 @@ impl ConfigField for ColorList { CValue::ColorList(self.colors.iter().map(|c| [c.r, c.g, c.b]).collect()) } - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { + let input = crate::cli_args::text_value(input)?; let input = input.ok_or(ArgError::ValueRequired)?; if input.is_empty() { return Err(ArgError::ValueRequired); @@ -485,7 +487,8 @@ impl ConfigField for Palette { CValue::Palette(out) } - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { + let input = crate::cli_args::text_value(input)?; let value = input.ok_or(ArgError::ValueRequired)?; let eql_idx = value.find('=').ok_or(ArgError::InvalidValue)?; @@ -522,7 +525,7 @@ impl ConfigField for Palette { /// comma-separated values. #[derive(Debug, Clone, Default)] pub struct RepeatableString { - pub list: Vec, + pub list: Vec, /// If true, then the next value will clear the list and start over /// rather than append. This is a bit of a hack but is here to make @@ -552,7 +555,7 @@ impl RepeatableString { } impl ConfigField for RepeatableString { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { let value = input.ok_or(ArgError::ValueRequired)?; // Empty value resets the list @@ -567,7 +570,8 @@ impl ConfigField for RepeatableString { self.overwrite_next = false; } - self.list.push(value.to_string()); + self.list + .push(ghostty_os::osbytes::EncodedOs::from_bytes(value)); Ok(()) } @@ -577,7 +581,7 @@ impl ConfigField for RepeatableString { return write_void(f); } for v in &self.list { - write_str(f, v)?; + write_str(f, v.as_encoded())?; } Ok(()) } @@ -631,12 +635,12 @@ impl Default for SelectionWordChars { } impl ConfigField for SelectionWordChars { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { let value = input.ok_or(ArgError::ValueRequired)?; // Always include null as first boundary let mut list = vec![0u32]; - let mut it = crate::string::codepoint_iterator(value); + let mut it = crate::string::codepoint_iterator_bytes(value); while let Some(cp) = it.next().map_err(|_| ArgError::InvalidValue)? { list.push(cp); } @@ -657,7 +661,7 @@ impl ConfigField for SelectionWordChars { } s.push(ch); } - write_str(f, &s) + write_str(f, &ghostty_os::osbytes::valid_str_to_config_string(&s)) } } @@ -682,15 +686,16 @@ pub struct RepeatableFontVariation { } impl ConfigField for RepeatableFontVariation { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { let input = input.ok_or(ArgError::ValueRequired)?; - let eql_idx = input.find('=').ok_or(ArgError::InvalidValue)?; - let key = trim_ws(&input[..eql_idx]); - let value = trim_ws(&input[eql_idx + 1..]); - let key: &[u8; 4] = key - .as_bytes() - .try_into() + let eql_idx = input + .iter() + .position(|b| *b == b'=') + .ok_or(ArgError::InvalidValue)?; + let key = crate::cli_args::trim_bytes(&input[..eql_idx]); + let value = std::str::from_utf8(crate::cli_args::trim_bytes(&input[eql_idx + 1..])) .map_err(|_| ArgError::InvalidValue)?; + let key: &[u8; 4] = key.try_into().map_err(|_| ArgError::InvalidValue)?; self.list.push(Variation { id: VariationId::init(key), value: value.parse::().map_err(|_| ArgError::InvalidValue)?, @@ -704,7 +709,14 @@ impl ConfigField for RepeatableFontVariation { } for v in &self.list { let id = v.id.str(); - write_str(f, &format!("{}={}", String::from_utf8_lossy(&id), v.value))?; + write_str( + f, + &format!( + "{}={}", + ghostty_os::osbytes::bytes_to_config_string(&id), + v.value + ), + )?; } Ok(()) } @@ -860,18 +872,22 @@ pub struct RepeatableCodepointMap { } impl ConfigField for RepeatableCodepointMap { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { let input = input.ok_or(ArgError::ValueRequired)?; - let eql_idx = input.find('=').ok_or(ArgError::InvalidValue)?; - let key = trim_ws(&input[..eql_idx]); - let value = trim_ws(&input[eql_idx + 1..]); + let eql_idx = input + .iter() + .position(|b| *b == b'=') + .ok_or(ArgError::InvalidValue)?; + let key = crate::cli_args::trim_bytes(&input[..eql_idx]); + let value = crate::cli_args::trim_bytes(&input[eql_idx + 1..]); - let mut p = UnicodeRangeParser::new(key); + let mut p = + UnicodeRangeParser::new(std::str::from_utf8(key).map_err(|_| ArgError::InvalidValue)?); while let Some(range) = p.next_range()? { self.map.add(CodepointMapEntry { range, descriptor: Descriptor { - family: Some(value.to_string()), + family: Some(ghostty_os::osbytes::EncodedOs::from_bytes(value)), monospace: false, // we allow any font ..Descriptor::default() }, @@ -886,7 +902,11 @@ impl ConfigField for RepeatableCodepointMap { } for e in &self.map.list { - let family = e.descriptor.family.as_deref().unwrap_or(""); + let family = e + .descriptor + .family + .as_ref() + .map_or("", ghostty_os::osbytes::EncodedOs::as_encoded); if e.range[0] == e.range[1] { write_str(f, &format!("U+{:04X}={}", e.range[0], family))?; } else { @@ -907,7 +927,8 @@ pub struct RepeatableClipboardCodepointMap { } impl ConfigField for RepeatableClipboardCodepointMap { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { + let input = crate::cli_args::text_value(input)?; let input = input.ok_or(ArgError::ValueRequired)?; let eql_idx = input.find('=').ok_or(ArgError::InvalidValue)?; let key = trim_ws(&input[..eql_idx]); @@ -941,7 +962,7 @@ impl ConfigField for RepeatableClipboardCodepointMap { for e in &self.map.list { let value_str = match &e.replacement { Replacement::Codepoint(cp) => format!("U+{cp:04X}"), - Replacement::String(s) => s.clone(), + Replacement::String(s) => ghostty_os::osbytes::valid_str_to_config_string(s), }; if e.range[0] == e.range[1] { @@ -971,14 +992,14 @@ pub enum FontStyle { False, /// A specific named font style to use for this style. - Name(String), + Name(ghostty_os::osbytes::EncodedOs), } impl FontStyle { /// Returns the string name value that can be used with a font /// descriptor. #[must_use] - pub fn name_value(&self) -> Option<&str> { + pub fn name_value(&self) -> Option<&ghostty_os::osbytes::EncodedOs> { match self { FontStyle::Default | FontStyle::False => None, FontStyle::Name(n) => Some(n), @@ -987,12 +1008,12 @@ impl FontStyle { } impl ConfigField for FontStyle { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { let value = input.ok_or(ArgError::ValueRequired)?; *self = match value { - "default" => FontStyle::Default, - "false" => FontStyle::False, - _ => FontStyle::Name(value.to_string()), + b"default" => FontStyle::Default, + b"false" => FontStyle::False, + _ => FontStyle::Name(ghostty_os::osbytes::EncodedOs::from_bytes(value)), }; Ok(()) } @@ -1001,7 +1022,7 @@ impl ConfigField for FontStyle { match self { FontStyle::Default => write_str(f, "default"), FontStyle::False => write_str(f, "false"), - FontStyle::Name(n) => write_str(f, n), + FontStyle::Name(n) => write_str(f, n.as_encoded()), } } } @@ -1032,7 +1053,8 @@ pub struct RepeatableLink { } impl ConfigField for RepeatableLink { - fn parse_cli(&mut self, _input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, _input: Option<&[u8]>) -> Result<(), ArgError> { + let _input = crate::cli_args::text_value(_input)?; Err(ArgError::other("NotImplemented")) } @@ -1477,7 +1499,14 @@ mod tests { fn fam(map: &RepeatableCodepointMap, i: usize) -> (&[u32; 2], &str) { let e = &map.map.list[i]; - (&e.range, e.descriptor.family.as_deref().unwrap()) + ( + &e.range, + e.descriptor + .family + .as_ref() + .map(ghostty_os::osbytes::EncodedOs::as_encoded) + .unwrap(), + ) } #[test] diff --git a/crates/ghostty-config/src/types_b.rs b/crates/ghostty-config/src/types_b.rs index 4252499..2be64ce 100644 --- a/crates/ghostty-config/src/types_b.rs +++ b/crates/ghostty-config/src/types_b.rs @@ -6,6 +6,7 @@ use std::fmt; use ghostty_input::binding::Action; +use ghostty_os::osbytes::EncodedOs; use crate::{ c_value::{CValue, QuickTerminalSizeC}, @@ -92,17 +93,18 @@ use ghostty_input::command::Command; /// Parse a command from `title:...,description:...,action:...` /// (Zig: `parseAutoStruct(inputpkg.Command, alloc, input, null)`). -pub fn parse_command_auto(input: &str) -> Result { +pub fn parse_command_auto(input: &[u8]) -> Result { let mut action: Option = None; - let mut title: Option = None; - let mut description = String::new(); - parse_auto_struct_with(input, |k, v| { + let mut title: Option = None; + let mut description = EncodedOs::new(); + crate::cli_args::parse_auto_struct_bytes_with(input, |k, v| { match k { "action" => { - action = Some(Action::parse(v).map_err(|e| ArgError::Other(format!("{e:?}")))?); + action = + Some(Action::parse_bytes(v).map_err(|e| ArgError::Other(format!("{e:?}")))?); } - "title" => title = Some(v.to_string()), - "description" => description = v.to_string(), + "title" => title = Some(EncodedOs::from_bytes(v)), + "description" => description = EncodedOs::from_bytes(v), _ => return Ok(false), } Ok(true) @@ -121,10 +123,10 @@ fn default_commands() -> Vec { /// Escape a string like Zig's `std.zig.fmtString`: `\n \r \t \\ \"` are /// escaped by name and every other byte outside printable ASCII is written /// as `\xNN` (so multi-byte UTF-8 is escaped per byte). -fn zig_fmt_string(s: &str) -> String { +fn zig_fmt_string(s: &EncodedOs) -> String { use std::fmt::Write; let mut out = String::new(); - for &b in s.as_bytes() { + for b in s.to_bytes() { match b { b'\n' => out.push_str("\\n"), b'\r' => out.push_str("\\r"), @@ -162,9 +164,9 @@ impl ConfigField for RepeatableCommand { ) } - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { // Unset or empty input clears the list - let input = input.unwrap_or(""); + let input = input.unwrap_or(b""); if input.is_empty() { self.value.clear(); return Ok(()); @@ -395,7 +397,8 @@ impl MouseScrollMultiplier { } impl ConfigField for MouseScrollMultiplier { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { + let input = crate::cli_args::text_value(input)?; let input = input.ok_or(ArgError::ValueRequired)?; // Unset fields inherit from the current value. let mut result = *self; @@ -626,7 +629,8 @@ impl ConfigField for QuickTerminalSize { } } - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { + let input = crate::cli_args::text_value(input)?; let input = input.ok_or(ArgError::ValueRequired)?; let mut it = input.split(','); @@ -819,7 +823,8 @@ impl ConfigField for BackgroundBlur { }) } - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { + let input = crate::cli_args::text_value(input)?; let Some(input) = input else { // Emulate behavior for bools *self = BackgroundBlur::True; @@ -919,7 +924,8 @@ impl ConfigField for WindowDecoration { CValue::EnumName(self.as_str()) } - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { + let input = crate::cli_args::text_value(input)?; *self = Self::parse(input)?; Ok(()) } @@ -932,12 +938,12 @@ impl ConfigField for WindowDecoration { /// See theme #[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct Theme { - pub light: String, - pub dark: String, + pub light: EncodedOs, + pub dark: EncodedOs, } impl ConfigField for Theme { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { let input = input.ok_or(ArgError::ValueRequired)?; if input.is_empty() { return Err(ArgError::ValueRequired); @@ -947,13 +953,13 @@ impl ConfigField for Theme { // we're parsing a light/dark mode theme pair. Note that "=" isn't // actually valid for setting a light/dark mode pair but I anticipate // it'll be a common typo. - if input.contains([',', '=', ':']) { - let mut light: Option = None; - let mut dark: Option = None; - parse_auto_struct_with(input, |k, v| { + if input.iter().any(|b| matches!(b, b',' | b'=' | b':')) { + let mut light: Option = None; + let mut dark: Option = None; + crate::cli_args::parse_auto_struct_bytes_with(input, |k, v| { match k { - "light" => light = Some(v.to_string()), - "dark" => dark = Some(v.to_string()), + "light" => light = Some(EncodedOs::from_bytes(v)), + "dark" => dark = Some(EncodedOs::from_bytes(v)), _ => return Ok(false), } Ok(true) @@ -967,19 +973,26 @@ impl ConfigField for Theme { } // Trim our value and set the value to the specified value directly. - let trimmed = trim_ws(input); + let trimmed = crate::cli_args::trim_bytes(input); *self = Theme { - light: trimmed.to_string(), - dark: trimmed.to_string(), + light: EncodedOs::from_bytes(trimmed), + dark: EncodedOs::from_bytes(trimmed), }; Ok(()) } fn format_entry(&self, f: &mut EntryFormatter<'_>) -> fmt::Result { if self.light == self.dark { - return fmt_str(f, &self.light); + return fmt_str(f, self.light.as_encoded()); } - fmt_str(f, &format!("light:{},dark:{}", self.light, self.dark)) + fmt_str( + f, + &format!( + "light:{},dark:{}", + self.light.as_encoded(), + self.dark.as_encoded() + ), + ) } } @@ -1168,7 +1181,8 @@ impl ConfigField for Duration { CValue::Usize(self.as_milliseconds() as usize) } - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { + let input = crate::cli_args::text_value(input)?; *self = Duration::parse(input)?; Ok(()) } @@ -1207,7 +1221,8 @@ impl WindowPadding { } impl ConfigField for WindowPadding { - fn parse_cli(&mut self, input: Option<&str>) -> Result<(), ArgError> { + fn parse_bytes(&mut self, input: Option<&[u8]>) -> Result<(), ArgError> { + let input = crate::cli_args::text_value(input)?; *self = WindowPadding::parse(input)?; Ok(()) } @@ -1602,17 +1617,17 @@ mod tests { // Single let mut v = Theme::default(); v.parse_cli(Some("foo")).unwrap(); - assert_eq!((v.light.as_str(), v.dark.as_str()), ("foo", "foo")); + assert_eq!((v.light.as_encoded(), v.dark.as_encoded()), ("foo", "foo")); // Single whitespace let mut v = Theme::default(); v.parse_cli(Some(" foo ")).unwrap(); - assert_eq!((v.light.as_str(), v.dark.as_str()), ("foo", "foo")); + assert_eq!((v.light.as_encoded(), v.dark.as_encoded()), ("foo", "foo")); // Light/dark let mut v = Theme::default(); v.parse_cli(Some(" light:foo, dark : bar ")).unwrap(); - assert_eq!((v.light.as_str(), v.dark.as_str()), ("foo", "bar")); + assert_eq!((v.light.as_encoded(), v.dark.as_encoded()), ("foo", "bar")); let mut v = Theme::default(); assert_eq!(v.parse_cli(None), Err(ArgError::ValueRequired)); diff --git a/crates/ghostty-config/tests/raw_bytes.rs b/crates/ghostty-config/tests/raw_bytes.rs new file mode 100644 index 0000000..5c275cb --- /dev/null +++ b/crates/ghostty-config/tests/raw_bytes.rs @@ -0,0 +1,186 @@ +#![allow(clippy::unwrap_used, clippy::assert_is_empty)] +use ghostty_config::{ + cli_args::{ArgError, ArgsIterator, LineIterator}, + config::Config, +}; +use ghostty_input::binding::{Action, Trigger}; + +fn load(bytes: &[u8], cli: bool) -> Config { + let mut config = Config::default(); + if cli { + let arg = [b"--".as_slice(), bytes].concat(); + config + .load_iter(&mut ArgsIterator::new([arg].into_iter())) + .unwrap(); + } else { + config.load_iter(&mut LineIterator::new(bytes)).unwrap(); + } + config +} + +#[test] +fn literal_fdd0_is_a_single_unicode_trigger() { + for cli in [false, true] { + let config = load("keybind=\u{fdd0}=text:ok".as_bytes(), cli); + assert!(config.diagnostics.items().is_empty()); + let trigger = Trigger::parse("\u{fdd0}").unwrap(); + assert_eq!( + config + .keybind + .set + .get(trigger) + .unwrap() + .as_leaf() + .unwrap() + .action, + Action::Text("ok".into()) + ); + } +} + +#[test] +fn selection_word_chars_reject_invalid_utf8() { + for cli in [false, true] { + let config = load(b"selection-word-chars=\xff", cli); + assert_eq!(config.diagnostics.items().len(), 1); + assert!( + config.diagnostics.items()[0] + .message + .starts_with("invalid value") + ); + } + let mut config = Config::default(); + assert_eq!( + config.parse_field_bytes("selection-word-chars", Some(b"\xed\xa0\x80")), + Err(ArgError::InvalidValue) + ); + config + .parse_field("selection-word-chars", Some("\u{fdd0}é")) + .unwrap(); + assert_eq!(config.selection_word_chars.codepoints, [0, 0xfdd0, 0xe9]); +} + +#[test] +fn clipboard_replacements_require_utf8_and_keep_literal_fdd0() { + for cli in [false, true] { + let config = load(b"clipboard-codepoint-map=U+0041=\xff", cli); + assert_eq!(config.diagnostics.items().len(), 1); + assert!(config.clipboard_codepoint_map.map.list.is_empty()); + } + let config = load("clipboard-codepoint-map=U+0041=\u{fdd0}é".as_bytes(), false); + assert_eq!( + config.clipboard_codepoint_map.map.list[0].replacement, + ghostty_terminal::formatter::Replacement::String("\u{fdd0}é".into()) + ); +} + +#[test] +fn font_style_and_codepoint_family_keep_original_bytes() { + let config = load( + "font-style=Name\u{fdd0}é\nfont-codepoint-map=U+0041=Family\u{fdd0}é".as_bytes(), + false, + ); + assert_eq!( + config.font_style.name_value().unwrap().to_bytes(), + "Name\u{fdd0}é".as_bytes() + ); + assert_eq!( + config.font_codepoint_map.map.list[0] + .descriptor + .family + .as_ref() + .unwrap() + .to_bytes(), + "Family\u{fdd0}é".as_bytes() + ); + let config = load(b"font-codepoint-map=U+0041=Family\xff", true); + assert!(config.diagnostics.items().is_empty()); + assert_eq!( + config.font_codepoint_map.map.list[0] + .descriptor + .family + .as_ref() + .unwrap() + .to_bytes(), + b"Family\xff" + ); +} + +#[test] +fn byte_fields_and_auto_struct_values_never_parse_escape_encoding() { + let raw = "x\u{fdd0}é".as_bytes(); + for cli in [false, true] { + for key in ["title", "term", "enquiry-response", "language"] { + let config = load(&[key.as_bytes(), b"=", raw, b"\xff"].concat(), cli); + assert!(config.diagnostics.items().is_empty(), "{key}"); + let value = match key { + "title" => config.title.unwrap(), + "term" => config.term, + "enquiry-response" => config.enquiry_response, + _ => config.language.unwrap(), + }; + assert_eq!(value.to_bytes(), [raw, b"\xff"].concat()); + } + } + let config = load(b"env=K\xff=V\xfe\ncommand=direct:exe arg\xff\ninput=raw:in\xff\\n\ncommand-palette-entry=title:\"name\\xff\",action:\"text:payload\\xfe\"", false); + assert!(config.diagnostics.items().is_empty()); + let (key, value) = config.env.iter().next().unwrap(); + assert_eq!(key.to_bytes(), b"K\xff"); + assert_eq!(value.to_bytes(), b"V\xfe"); + assert_eq!( + config + .command + .unwrap() + .arg_iterator() + .last() + .unwrap() + .to_bytes(), + b"arg\xff" + ); + let ghostty_config::io::ReadableIO::Raw(input) = config.input.list[0].clone_parsed() else { + panic!("raw input") + }; + assert_eq!(input.to_bytes(), b"in\xff\n"); + let entry = config.command_palette_entry.value.last().unwrap(); + assert_eq!(entry.title.to_bytes(), b"name\xff"); + assert_eq!( + entry.action, + Action::Text(ghostty_os::osbytes::EncodedOs::from_bytes(b"payload\xfe")) + ); +} + +#[test] +fn byte_fields_trim_zig_ascii_whitespace() { + for cli in [false, true] { + let config = load(b"env=\x0bKEY\x0b=\x0bVALUE\x0b", cli); + assert_eq!(config.env.get("KEY").unwrap().to_bytes(), b"VALUE"); + let config = load(b"working-directory=\x0binherit\x0b", cli); + assert_eq!( + config.working_directory, + Some(ghostty_config::types_a::WorkingDirectory::Inherit) + ); + } +} + +#[test] +fn font_variation_tags_are_four_original_bytes() { + for cli in [false, true] { + for tag in [b"a\xffcd".as_slice(), "\u{fdd0}x".as_bytes()] { + let config = load(&[b"font-variation=", tag, b"=12"].concat(), cli); + assert!(config.diagnostics.items().is_empty()); + assert_eq!(config.font_variation.list[0].id.str(), tag); + assert_eq!(config.font_variation.list[0].value, 12.0); + let mut formatted = String::new(); + ghostty_config::formatter::format_entry( + "font-variation", + &config.font_variation, + &mut formatted, + ) + .unwrap(); + assert_eq!( + ghostty_os::osbytes::config_str_to_bytes(&formatted), + [b"font-variation = ", tag, b"=12\n"].concat() + ); + } + } +} diff --git a/crates/ghostty-core/src/global.rs b/crates/ghostty-core/src/global.rs index 59f3f4f..7f4a845 100644 --- a/crates/ghostty-core/src/global.rs +++ b/crates/ghostty-core/src/global.rs @@ -234,7 +234,10 @@ fn init_with(opts: InitOptions, default_logging: Logging) -> &'static GlobalStat // Setup i18n if let Some(v) = resources_dir.app() - && let Err(err) = ghostty_os::i18n::init(&v.to_path()) + && let Err(err) = v + .to_path() + .map_err(|e| e.to_string()) + .and_then(|path| ghostty_os::i18n::init(&path).map_err(|e| e.to_string())) { log::warn!("failed to init i18n, translations will not be available err={err}"); } diff --git a/crates/ghostty-core/src/surface/binding.rs b/crates/ghostty-core/src/surface/binding.rs index 2c15984..662e8e5 100644 --- a/crates/ghostty-core/src/surface/binding.rs +++ b/crates/ghostty-core/src/surface/binding.rs @@ -720,7 +720,7 @@ impl Surface { }; // The payload is a config string in the reversible OS-byte // encoding: the pty gets the original bytes. - full.extend(ghostty_os::osbytes::config_str_to_bytes(data)); + full.extend(data.to_bytes()); // Zig formats into a fixed 128 byte buffer (`NoSpaceLeft`). if full.len() > 128 { return Err("csi/esc binding sequence too long (max 128 bytes)".into()); @@ -734,10 +734,10 @@ impl Surface { Action::Text(data) => { // Decoded bytes: escapes produce UTF-8, everything else is // copied byte for byte (Zig `config/string.zig`). - let text = match ghostty_config::string::parse_bytes(data) { + let text = match ghostty_config::string::parse_bytes(data.to_bytes()) { Ok(t) => t, Err(err) => { - log::warn!("error parsing text binding text={data} err={err:?}"); + log::warn!("error parsing text binding text={data:?} err={err:?}"); return Ok(true); } }; @@ -824,9 +824,7 @@ impl Surface { s.handle // The payload is a config string in the reversible // encoding: search for the original bytes. - .send(SearchMessage::ChangeNeedle( - ghostty_os::osbytes::config_str_to_bytes(text), - )); + .send(SearchMessage::ChangeNeedle(text.to_bytes())); s.handle.wakeup(); } } @@ -934,15 +932,11 @@ impl Surface { } Action::SetSurfaceTitle(v) => { - return Ok(self.perform(RtAction::SetTitle(SetTitle { - title: ghostty_os::osbytes::config_str_to_text(v), - }))); + return Ok(self.perform(RtAction::SetTitle(SetTitle { title: v.clone() }))); } Action::SetTabTitle(v) => { - return Ok(self.perform(RtAction::SetTabTitle(SetTitle { - title: ghostty_os::osbytes::config_str_to_text(v), - }))); + return Ok(self.perform(RtAction::SetTabTitle(SetTitle { title: v.clone() }))); } Action::ClearScreen => { @@ -1330,7 +1324,7 @@ mod tests { // action's EncodedOs decodes back to the same path. assert_eq!(path_bytes(&path), path.as_os_str().as_bytes()); let url = ghostty_os::osbytes::EncodedOs::from_path(&path); - assert_eq!(url.to_path(), path); + assert_eq!(url.to_path().unwrap(), path); } #[test] diff --git a/crates/ghostty-core/src/surface/clipboard.rs b/crates/ghostty-core/src/surface/clipboard.rs index 6347e42..b6a396d 100644 --- a/crates/ghostty-core/src/surface/clipboard.rs +++ b/crates/ghostty-core/src/surface/clipboard.rs @@ -409,21 +409,9 @@ impl Surface { } } -/// The clipboard codepoint map with string replacements decoded from the -/// config encoding (a literal `U+FDD0` is restored; bytes that are not -/// UTF-8 can't be represented in the formatter's text output). +/// Clipboard replacements are validated UTF-8 by the config field parser. fn decode_codepoint_map(list: &[formatter::CodepointMap]) -> Vec { - list.iter() - .map(|e| formatter::CodepointMap { - range: e.range, - replacement: match &e.replacement { - formatter::Replacement::String(s) => { - formatter::Replacement::String(ghostty_os::osbytes::config_str_to_text(s)) - } - r @ formatter::Replacement::Codepoint(_) => r.clone(), - }, - }) - .collect() + list.to_vec() } #[cfg(test)] @@ -431,9 +419,8 @@ mod decode_map_tests { use super::*; #[test] - fn string_replacements_are_decoded() { - let enc = ghostty_os::osbytes::valid_str_to_config_string("a\u{fdd0}"); - assert_ne!(enc, "a\u{fdd0}"); + fn string_replacements_are_plain_text() { + let enc = "a\u{fdd0}".to_owned(); let list = [formatter::CodepointMap { range: [1, 2], replacement: formatter::Replacement::String(enc), diff --git a/crates/ghostty-core/src/surface/config.rs b/crates/ghostty-core/src/surface/config.rs index 17bd9dc..695998c 100644 --- a/crates/ghostty-core/src/surface/config.rs +++ b/crates/ghostty-core/src/surface/config.rs @@ -55,7 +55,7 @@ impl DerivedConfig { let mut links = Vec::with_capacity(config.link.links.len()); for link in &config.link.links { links.push(Link { - regex: Regex::new(&ghostty_os::osbytes::config_str_to_text(&link.regex))?, + regex: Regex::new(&link.regex)?, action: link.action, highlight: link.highlight, }); diff --git a/crates/ghostty-core/src/surface/keyboard.rs b/crates/ghostty-core/src/surface/keyboard.rs index a8b67bc..74352d0 100644 --- a/crates/ghostty-core/src/surface/keyboard.rs +++ b/crates/ghostty-core/src/surface/keyboard.rs @@ -24,6 +24,7 @@ use ghostty_input::{ key_encode, }; use ghostty_inspector as inspector; +use ghostty_os::osbytes::EncodedOs; use ghostty_terminal::terminal::{MouseEvents, ScrollViewport, Terminal}; use ghostty_termio::message::Message as IoMessage; use ghostty_vt::{modes::Mode, mouse_shape::MouseShape}; @@ -40,7 +41,7 @@ pub enum SetRoot { /// The root keybinding set. Root, /// A named key table. - Table(String), + Table(EncodedOs), } /// The currently active key sequence: the set it started in plus the @@ -54,7 +55,7 @@ pub struct SequencePath { /// An entry of the key table stack. #[derive(Debug, Clone, PartialEq, Eq)] pub struct TableEntry { - pub name: String, + pub name: EncodedOs, pub once: bool, } @@ -796,12 +797,16 @@ impl Surface { /// Activate a key table by name (Zig: `activate_key_table[_once]`). /// Returns whether the table was activated. #[allow(dead_code, clippy::unnecessary_wraps)] // used by the binding-actions module - pub(crate) fn activate_key_table(&self, name: &str, once: bool) -> Result { + pub(crate) fn activate_key_table( + &self, + name: &EncodedOs, + once: bool, + ) -> Result { let cfg = self.cfg(); // Look up the table in our config if root_set(&cfg, &SetRoot::Table(name.to_owned())).is_none() { - log::debug!("key table not found: {name}"); + log::debug!("key table not found: {name:?}"); return Ok(false); } @@ -810,14 +815,14 @@ impl Surface { // If this is the same table as is currently active, then do // nothing. - if kb.table_stack.last().is_some_and(|t| t.name == name) { - log::debug!("ignoring duplicate activate table: {name}"); + if kb.table_stack.last().is_some_and(|t| &t.name == name) { + log::debug!("ignoring duplicate activate table: {name:?}"); return Ok(false); } // If we're already at the max, ignore it. if kb.table_stack.len() >= MAX_ACTIVE_KEY_TABLES { - log::info!("ignoring activate table, max depth reached: {name}"); + log::info!("ignoring activate table, max depth reached: {name:?}"); return Ok(false); } @@ -831,7 +836,7 @@ impl Surface { // Notify the UI. self.perform(RtAction::KeyTable(KeyTable::Activate(name.to_owned()))); - log::debug!("key table activated: {name}"); + log::debug!("key table activated: {name:?}"); Ok(true) } diff --git a/crates/ghostty-core/src/surface/mod.rs b/crates/ghostty-core/src/surface/mod.rs index 37578f3..2b52358 100644 --- a/crates/ghostty-core/src/surface/mod.rs +++ b/crates/ghostty-core/src/surface/mod.rs @@ -459,10 +459,7 @@ impl Surface { // don't leak GHOSTTY_LOG to any subprocesses env.remove("GHOSTTY_LOG"); - let mut ecfg = exec::Config::new( - env, - ghostty_os::osbytes::EncodedOs::from_encoded(config.term.clone()), - ); + let mut ecfg = exec::Config::new(env, config.term.clone()); ecfg.command = command.clone(); ecfg.env_override = config.env.clone(); ecfg.shell_integration = config.shell_integration; @@ -607,7 +604,7 @@ impl Surface { rt_app.perform_action( Target::Surface(id), RtAction::SetTitle(SetTitle { - title: ghostty_os::osbytes::config_str_to_text(title), + title: title.clone(), }), ); } else if config.xdg_terminal_exec @@ -620,7 +617,7 @@ impl Surface { rt_app.perform_action( Target::Surface(id), RtAction::SetTitle(SetTitle { - title: ghostty_os::osbytes::config_str_to_text(&cmd.string()), + title: ghostty_os::osbytes::EncodedOs::from_encoded(cmd.string()), }), ); } else if let Some(cmd) = &command { @@ -636,7 +633,7 @@ impl Surface { rt_app.perform_action( Target::Surface(id), RtAction::SetTitle(SetTitle { - title: ghostty_os::osbytes::config_str_to_text(first), + title: first.clone(), }), ); } @@ -872,8 +869,8 @@ impl Surface { } let end = buf.iter().position(|&b| b == 0).unwrap_or(buf.len()); - let title = String::from_utf8_lossy(&buf[..end]).into_owned(); - log::debug!("changing title \"{title}\""); + let title = ghostty_os::osbytes::EncodedOs::from_bytes(&buf[..end]); + log::debug!("changing title {title:?}"); self.perform(RtAction::SetTitle(SetTitle { title })); } @@ -885,11 +882,14 @@ impl Surface { let title = self.rt_surface.get_title(); let data = match style { - ghostty_apprt::surface::ReportTitleStyle::Csi21T => { - format!("\x1b]l{}\x1b\\", title.unwrap_or_default()) - } + ghostty_apprt::surface::ReportTitleStyle::Csi21T => [ + b"\x1b]l".as_slice(), + &title.unwrap_or_default().to_bytes(), + b"\x1b\\", + ] + .concat(), }; - self.queue_io(IoMessage::write_req(data.as_bytes())); + self.queue_io(IoMessage::write_req(data.as_slice())); } Message::ColorChange(change) => { @@ -1306,7 +1306,7 @@ impl Surface { // configured title. if let Some(title) = &config.title { self.perform(RtAction::SetTitle(SetTitle { - title: ghostty_os::osbytes::config_str_to_text(title), + title: title.clone(), })); } diff --git a/crates/ghostty-core/src/surface/mouse.rs b/crates/ghostty-core/src/surface/mouse.rs index d3d142d..acd2365 100644 --- a/crates/ghostty-core/src/surface/mouse.rs +++ b/crates/ghostty-core/src/surface/mouse.rs @@ -2543,8 +2543,8 @@ mod tests { // The OSC 7 pwd is raw bytes. let pwd = dir.as_os_str().as_bytes().to_vec(); let t = open_link_target(Some(&pwd), "file.txt"); - assert_eq!(t.to_path(), dir.join("file.txt")); - assert!(t.to_path().exists()); + assert_eq!(t.to_path().unwrap(), dir.join("file.txt")); + assert!(t.to_path().unwrap().exists()); } #[test] diff --git a/crates/ghostty-core/tests/common/mod.rs b/crates/ghostty-core/tests/common/mod.rs index 57db6cb..bd6fc1f 100644 --- a/crates/ghostty-core/tests/common/mod.rs +++ b/crates/ghostty-core/tests/common/mod.rs @@ -92,7 +92,7 @@ impl RtApp for FakeRt { let ret = match &action { Action::ShowChildExited(_) => self.show_child_exited.get(), Action::SetTitle(t) => { - *self.title.borrow_mut() = Some(t.title.clone()); + *self.title.borrow_mut() = Some(t.title.to_text_lossy()); true } _ => true, @@ -109,8 +109,8 @@ impl RtSurface for FakeRt { fn cgroup(&self) -> Option { None } - fn get_title(&self) -> Option { - self.title.borrow().clone() + fn get_title(&self) -> Option { + self.title.borrow().clone().map(Into::into) } fn get_content_scale(&self) -> ContentScale { self.scale.get() @@ -262,7 +262,11 @@ impl Harness { config.load_iter(&mut it).expect("config args"); if let Some(script) = &opts.script { let script = script.replace("{out}", out.to_str().unwrap()); - config.command = Some(Command::Direct(vec!["sh".into(), "-c".into(), script])); + config.command = Some(Command::Direct(vec![ + "sh".into(), + "-c".into(), + script.into(), + ])); } config.finalize().expect("finalize"); diff --git a/crates/ghostty-core/tests/e2e_bindings.rs b/crates/ghostty-core/tests/e2e_bindings.rs index b886d4f..6061972 100644 --- a/crates/ghostty-core/tests/e2e_bindings.rs +++ b/crates/ghostty-core/tests/e2e_bindings.rs @@ -369,10 +369,7 @@ fn text_csi_esc_and_ignore_bindings() { #[test] fn config_loaded_text_csi_esc_write_decoded_bytes() { - use ghostty_os::osbytes::EncodedOs; - let h = harness!(Harness::recorder("", &[])); - let enc = |raw: &[u8]| EncodedOs::from_bytes(raw).into_encoded(); // Byte FF and a literal U+FDD0 must reach the pty unchanged, not as // the reversible escape encoding. for (kind, raw, want) in [ @@ -386,7 +383,8 @@ fn config_loaded_text_csi_esc_write_decoded_bytes() { ("esc", "\u{fdd0}".as_bytes(), b"\x1b\xef\xb7\x90".to_vec()), ] { h.mark(); - let a = ghostty_input::binding::Action::parse(&format!("{kind}:{}", enc(raw))).unwrap(); + let a = ghostty_input::binding::Action::parse_bytes(&[kind.as_bytes(), b":", raw].concat()) + .unwrap(); assert!(h.surface.perform_binding_action(&a).unwrap()); h.expect_input(&want); } diff --git a/crates/ghostty-crash/src/dir.rs b/crates/ghostty-crash/src/dir.rs index de5983e..96e2395 100644 --- a/crates/ghostty-crash/src/dir.rs +++ b/crates/ghostty-crash/src/dir.rs @@ -20,7 +20,7 @@ pub fn default_dir() -> io::Result { .map_err(|e| io::Error::other(e.to_string()))?; Ok(Dir { // The XDG helpers return the encoded form of the OS string. - path: path.to_path(), + path: path.to_path()?, }) } diff --git a/crates/ghostty-font/src/codepoint_map.rs b/crates/ghostty-font/src/codepoint_map.rs index 54845cb..48bc108 100644 --- a/crates/ghostty-font/src/codepoint_map.rs +++ b/crates/ghostty-font/src/codepoint_map.rs @@ -16,7 +16,7 @@ mod tests { fn desc(family: &str) -> Descriptor { Descriptor { - family: Some(family.to_string()), + family: Some(family.into()), ..Default::default() } } @@ -35,11 +35,27 @@ mod tests { // Exact range assert!(m.get(1).is_none()); m.add(entry(1, 1, "A")); - assert_eq!("A", m.get(1).unwrap().family.as_deref().unwrap()); + assert_eq!( + "A", + m.get(1) + .unwrap() + .family + .as_ref() + .map(ghostty_os::osbytes::EncodedOs::as_encoded) + .unwrap() + ); // Later entry takes priority m.add(entry(1, 2, "B")); - assert_eq!("B", m.get(1).unwrap().family.as_deref().unwrap()); + assert_eq!( + "B", + m.get(1) + .unwrap() + .family + .as_ref() + .map(ghostty_os::osbytes::EncodedOs::as_encoded) + .unwrap() + ); // Non-matching assert!(m.get(0).is_none()); @@ -47,8 +63,24 @@ mod tests { m.add(entry(3, 4, "C")); m.add(entry(5, 6, "D")); - assert_eq!("C", m.get(3).unwrap().family.as_deref().unwrap()); - assert_eq!("B", m.get(1).unwrap().family.as_deref().unwrap()); + assert_eq!( + "C", + m.get(3) + .unwrap() + .family + .as_ref() + .map(ghostty_os::osbytes::EncodedOs::as_encoded) + .unwrap() + ); + assert_eq!( + "B", + m.get(1) + .unwrap() + .family + .as_ref() + .map(ghostty_os::osbytes::EncodedOs::as_encoded) + .unwrap() + ); } #[test] diff --git a/crates/ghostty-font/src/codepoint_resolver.rs b/crates/ghostty-font/src/codepoint_resolver.rs index cc46cb8..fd7fa8c 100644 --- a/crates/ghostty-font/src/codepoint_resolver.rs +++ b/crates/ghostty-font/src/codepoint_resolver.rs @@ -385,7 +385,9 @@ impl CodepointResolver { { log::debug!( "codepoint override based on config codepoint={cp} family={}", - desc.family.as_deref().unwrap_or("") + desc.family + .as_ref() + .map_or("", ghostty_os::osbytes::EncodedOs::as_encoded) ); return Some(idx); diff --git a/crates/ghostty-font/src/discovery.rs b/crates/ghostty-font/src/discovery.rs index 892195d..c72e4f6 100644 --- a/crates/ghostty-font/src/discovery.rs +++ b/crates/ghostty-font/src/discovery.rs @@ -37,10 +37,10 @@ mod fc_impl { pub fn to_fc_pattern(desc: &Descriptor) -> fc::Pattern { let mut pat = fc::Pattern::create(); if let Some(family) = &desc.family { - assert!(pat.add_string(fc::Property::Family, family)); + assert!(pat.add_string_bytes(fc::Property::Family, &family.to_bytes())); } if let Some(style) = &desc.style { - assert!(pat.add_string(fc::Property::Style, style)); + assert!(pat.add_string_bytes(fc::Property::Style, &style.to_bytes())); } if desc.codepoint > 0 { let mut cs = fc::CharSet::create(); @@ -162,6 +162,43 @@ mod fc_impl { mod tests { use super::*; + #[cfg(font_fontconfig)] + #[test] + fn configured_font_names_reach_fontconfig_as_original_bytes() { + use ghostty_config::config::Config; + use ghostty_os::osbytes::os_bytes; + + use crate::fontconfig::Property; + let mut config = Config::default(); + config + .parse_field("font-family", Some("Family\u{fdd0}é")) + .unwrap(); + config + .parse_field("font-style", Some("Style\u{fdd0}é")) + .unwrap(); + config + .parse_field_bytes("font-codepoint-map", Some(b"U+0041=Family\xff")) + .unwrap(); + let pat = to_fc_pattern(&Descriptor { + family: Some(config.font_family.list[0].clone()), + style: config.font_style.name_value().cloned(), + ..Descriptor::default() + }); + assert_eq!( + os_bytes(&pat.get_os_string(Property::Family, 0).unwrap()), + "Family\u{fdd0}é".as_bytes() + ); + assert_eq!( + os_bytes(&pat.get_os_string(Property::Style, 0).unwrap()), + "Style\u{fdd0}é".as_bytes() + ); + let pat = to_fc_pattern(&config.font_codepoint_map.map.list[0].descriptor); + assert_eq!( + os_bytes(&pat.get_os_string(Property::Family, 0).unwrap()), + b"Family\xff" + ); + } + #[test] fn descriptor_hash() { let d = Descriptor::default(); diff --git a/crates/ghostty-font/src/discovery_coretext.rs b/crates/ghostty-font/src/discovery_coretext.rs index b1cecab..dba18bf 100644 --- a/crates/ghostty-font/src/discovery_coretext.rs +++ b/crates/ghostty-font/src/discovery_coretext.rs @@ -71,7 +71,9 @@ pub fn to_coretext_descriptor( // Family if let Some(family) = &desc.family { - let family = CFString::from_str(family); + let family = CFString::from_str( + std::str::from_utf8(&family.to_bytes()).map_err(|_| DiscoverError::DescriptorFailed)?, + ); // SAFETY: valid static key. keys.push(unsafe { kCTFontFamilyNameAttribute }); values.push(as_type(&*family)); @@ -79,7 +81,9 @@ pub fn to_coretext_descriptor( // Style if let Some(style) = &desc.style { - let style = CFString::from_str(style); + let style = CFString::from_str( + std::str::from_utf8(&style.to_bytes()).map_err(|_| DiscoverError::DescriptorFailed)?, + ); // SAFETY: valid static key. keys.push(unsafe { kCTFontStyleNameAttribute }); values.push(as_type(&*style)); @@ -414,7 +418,11 @@ fn score(desc: &Descriptor, ct_desc: &CTFontDescriptor) -> Score { // The first string in this slice will be used for the exact match, // and for the fuzzy match, all matching substrings will increase // the rank. - let desired_styles = math::desired_styles(desc.style.as_deref(), desc.bold, desc.italic); + let style = desc + .style + .as_ref() + .map(ghostty_os::osbytes::EncodedOs::to_text_lossy); + let desired_styles = math::desired_styles(style.as_deref(), desc.bold, desc.italic); let (exact, fuzzy) = math::style_scores(&style_str, &desired_styles); this.exact_style = exact; this.fuzzy_style = fuzzy; diff --git a/crates/ghostty-font/src/shape.rs b/crates/ghostty-font/src/shape.rs index 89319cb..bbdd588 100644 --- a/crates/ghostty-font/src/shape.rs +++ b/crates/ghostty-font/src/shape.rs @@ -59,5 +59,5 @@ pub struct Options { /// Note: eventually, this will move to font.Face probably as we may /// want to support per-face feature configuration. For now, we only /// support applying features globally. - pub features: Vec, + pub features: Vec, } diff --git a/crates/ghostty-font/src/shaper/coretext.rs b/crates/ghostty-font/src/shaper/coretext.rs index 505aef2..1373685 100644 --- a/crates/ghostty-font/src/shaper/coretext.rs +++ b/crates/ghostty-font/src/shaper/coretext.rs @@ -74,12 +74,17 @@ pub enum ShapeRunError { /// Create a CoreFoundation Dictionary suitable for settings the font /// features of a CoreText font (`makeFeaturesDict`). -fn make_features_dict(feats: &[Feature]) -> CfBox { +fn make_features_dict(feats: &[Feature]) -> Result, ShapeError> { let mut items: Vec>> = Vec::new(); for feat in feats { // Keys can only be ASCII. - let key = CFString::from_str(&String::from_utf8_lossy(&feat.tag)); + if !feat.tag.is_ascii() { + return Err(ShapeError::CoreTextFailed); + } + let key = CFString::from_str( + std::str::from_utf8(&feat.tag).map_err(|_| ShapeError::CoreTextFailed)?, + ); let value = CFNumber::new_i32(feat.value as i32); // SAFETY: valid static keys. @@ -98,7 +103,9 @@ fn make_features_dict(feats: &[Feature]) -> CfBox { let k = unsafe { kCTFontFeatureSettingsAttribute }; let dict = CFDictionary::::from_slices(&[k], &[list.as_ref()]); // SAFETY: the attribute dictionary is used as an untyped CFDictionary. - CfBox::new(unsafe { CFRetained::cast_unchecked::(dict) }) + Ok(CfBox::new(unsafe { + CFRetained::cast_unchecked::(dict) + })) } /// Shaper that uses CoreText. @@ -146,7 +153,7 @@ impl Shaper { pub fn new(opts: &shape::Options) -> Result { let mut feature_list = FeatureList::default(); for feature_str in &opts.features { - feature_list.append_from_string(feature_str); + feature_list.append_from_bytes(&feature_str.to_bytes()); } // We need to construct two attrs dictionaries for font features; @@ -155,8 +162,8 @@ impl Shaper { let mut feats_df: Vec = DEFAULT_FEATURES.to_vec(); feats_df.extend_from_slice(feats); - let features = make_features_dict(&feats_df); - let features_no_default = make_features_dict(feats); + let features = make_features_dict(&feats_df)?; + let features_no_default = make_features_dict(feats)?; // For now we only support LTR text. If we shape RTL text then // rendering will be very wrong so we need to explicitly force diff --git a/crates/ghostty-font/src/shaper/feature.rs b/crates/ghostty-font/src/shaper/feature.rs index f855d87..8e70336 100644 --- a/crates/ghostty-font/src/shaper/feature.rs +++ b/crates/ghostty-font/src/shaper/feature.rs @@ -344,7 +344,11 @@ impl FeatureList { /// Append features to this list from a string with a comma separated list. /// See `Feature::from_reader` for more docs. pub fn append_from_string(&mut self, s: &str) { - let mut reader = Reader::new(s.as_bytes()); + self.append_from_bytes(s.as_bytes()); + } + + pub fn append_from_bytes(&mut self, s: &[u8]) { + let mut reader = Reader::new(s); while !reader.at_end() { let i = reader.pos; if let Some(feature) = Feature::from_reader(&mut reader) { @@ -352,7 +356,7 @@ impl FeatureList { } else { log::warn!( "failed to parse font feature setting: \"{}\"", - String::from_utf8_lossy(&s.as_bytes()[i..reader.pos]) + String::from_utf8_lossy(&s[i..reader.pos]) ); } } diff --git a/crates/ghostty-font/src/shaper/harfbuzz.rs b/crates/ghostty-font/src/shaper/harfbuzz.rs index 8d2ff31..56b088e 100644 --- a/crates/ghostty-font/src/shaper/harfbuzz.rs +++ b/crates/ghostty-font/src/shaper/harfbuzz.rs @@ -76,7 +76,7 @@ impl Shaper { let mut feature_list = FeatureList::default(); feature_list.features.extend_from_slice(&DEFAULT_FEATURES); for feature_str in &opts.features { - feature_list.append_from_string(feature_str); + feature_list.append_from_bytes(&feature_str.to_bytes()); } let hb_feats = feature_list @@ -351,7 +351,7 @@ mod tests { fn shape_ligatures_disabled_by_feature() { let f = face(embedded::INCONSOLATA); let mut s = Shaper::new(&shape::Options { - features: vec!["-liga".to_string()], + features: vec!["-liga".into()], }) .unwrap(); let cells = s.shape_text(Some(&f), &text("===")).unwrap(); diff --git a/crates/ghostty-font/src/shaper/test_util.rs b/crates/ghostty-font/src/shaper/test_util.rs index 9621ef4..12ae50e 100644 --- a/crates/ghostty-font/src/shaper/test_util.rs +++ b/crates/ghostty-font/src/shaper/test_util.rs @@ -93,7 +93,7 @@ pub(crate) fn test_shaper_with_font(font_req: TestFont) -> TestShaper { let grid = SharedGrid::new(CodepointResolver::new(c)).unwrap(); let shaper = Shaper::new(&shape::Options { // Some of our tests rely on dlig being enabled by default - features: vec!["dlig".to_string()], + features: vec!["dlig".into()], }) .unwrap(); @@ -112,7 +112,7 @@ pub(crate) fn test_shaper_with_discovered_font(font_req: &str) -> Option = Vec::new(); for family in &config.font_family.list { descriptors.push(Descriptor { - family: Some(ghostty_os::osbytes::config_str_to_text(family)), - style: config.font_style.name_value().map(str::to_string), + family: Some(family.clone()), + style: config.font_style.name_value().cloned(), size: font_size.points, variations: config.font_variation.list.clone(), ..Descriptor::default() @@ -518,9 +520,9 @@ impl Key { // no results would be found if we restrict to ALSO searching for // italic. for family in &config.font_family_bold.list { - let style = config.font_style_bold.name_value().map(str::to_string); + let style = config.font_style_bold.name_value().cloned(); descriptors.push(Descriptor { - family: Some(ghostty_os::osbytes::config_str_to_text(family)), + family: Some(family.clone()), bold: style.is_none(), style, size: font_size.points, @@ -529,9 +531,9 @@ impl Key { }); } for family in &config.font_family_italic.list { - let style = config.font_style_italic.name_value().map(str::to_string); + let style = config.font_style_italic.name_value().cloned(); descriptors.push(Descriptor { - family: Some(ghostty_os::osbytes::config_str_to_text(family)), + family: Some(family.clone()), italic: style.is_none(), style, size: font_size.points, @@ -540,12 +542,9 @@ impl Key { }); } for family in &config.font_family_bold_italic.list { - let style = config - .font_style_bold_italic - .name_value() - .map(str::to_string); + let style = config.font_style_bold_italic.name_value().cloned(); descriptors.push(Descriptor { - family: Some(ghostty_os::osbytes::config_str_to_text(family)), + family: Some(family.clone()), bold: style.is_none(), italic: style.is_none(), style, diff --git a/crates/ghostty-input/Cargo.toml b/crates/ghostty-input/Cargo.toml index 6f84b97..b836f7a 100644 --- a/crates/ghostty-input/Cargo.toml +++ b/crates/ghostty-input/Cargo.toml @@ -6,6 +6,7 @@ license.workspace = true [dependencies] ghostty-vt.workspace = true +ghostty-os.workspace = true ghostty-unicode.workspace = true indexmap = "2.14.2" thiserror = "2.0.21" diff --git a/crates/ghostty-input/src/binding/action.rs b/crates/ghostty-input/src/binding/action.rs index 02d0c8d..1c8813f 100644 --- a/crates/ghostty-input/src/binding/action.rs +++ b/crates/ghostty-input/src/binding/action.rs @@ -8,6 +8,8 @@ use std::{ hash::{Hash, Hasher}, }; +use ghostty_os::osbytes::EncodedOs; + use super::Error; /// Define a fieldless enum whose variants are named in snake_case in the @@ -314,9 +316,9 @@ macro_rules! num_param { num_param!(f32, usize, isize, i16); /// Escape in the style of Zig's `std.zig.stringEscape`. -pub(super) fn string_escape(s: &str, out: &mut String) { +pub(super) fn string_escape(s: &[u8], out: &mut String) { use std::fmt::Write; - for &b in s.as_bytes() { + for &b in s { match b { b'\n' => out.push_str("\\n"), b'\r' => out.push_str("\\r"), @@ -353,18 +355,37 @@ impl Action { /// action name and value is the action parameter. The parameter /// is optional depending on the action. pub fn parse(input: &str) -> Result { - // Split our action by colon. A colon may not exist for some - // actions so it is optional. The part preceding the colon is the - // action name. - let colon_idx = input.find(':'); - let action = &input[..colon_idx.unwrap_or(input.len())]; + Self::parse_bytes(input.as_bytes()) + } - // An action name is always required - if action.is_empty() { + /// Parse raw action bytes. Only enum/numeric parameters require UTF-8; + /// byte payloads are stored without interpreting the OS escape encoding. + pub fn parse_bytes(input: &[u8]) -> Result { + let idx = input.iter().position(|b| *b == b':'); + let name = std::str::from_utf8(&input[..idx.unwrap_or(input.len())]) + .map_err(|_| Error::InvalidAction)?; + if name.is_empty() { return Err(Error::InvalidFormat); } - - Action::parse_named(action, colon_idx.map(|i| &input[i + 1..])) + let param = idx.map(|i| &input[i + 1..]); + let bytes = || param.map(EncodedOs::from_bytes).ok_or(Error::InvalidFormat); + match name { + "csi" => Ok(Self::Csi(bytes()?)), + "esc" => Ok(Self::Esc(bytes()?)), + "text" => Ok(Self::Text(bytes()?)), + "search" => Ok(Self::Search(bytes()?)), + "set_surface_title" => Ok(Self::SetSurfaceTitle(bytes()?)), + "set_tab_title" => Ok(Self::SetTabTitle(bytes()?)), + "activate_key_table" => Ok(Self::ActivateKeyTable(bytes()?)), + "activate_key_table_once" => Ok(Self::ActivateKeyTableOnce(bytes()?)), + _ => Self::parse_named( + name, + param + .map(std::str::from_utf8) + .transpose() + .map_err(|_| Error::InvalidAction)?, + ), + } } /// The action name (Zig `@tagName`). @@ -423,15 +444,15 @@ pub enum Action { /// `\x1b[`). /// /// For example, `csi:0m` can be sent to reset all styles of the current text. - Csi(String), + Csi(EncodedOs), /// Send an `ESC` sequence. - Esc(String), + Esc(EncodedOs), /// Send the specified text. /// /// Uses Zig string literal syntax. This is currently not validated. /// If the text is invalid (i.e. contains an invalid escape sequence), /// the error will currently only show up in logs. - Text(String), + Text(EncodedOs), /// Send data to the pty depending on whether cursor key mode is enabled /// (`application`) or disabled (`normal`). CursorKey(CursorKey), @@ -478,7 +499,7 @@ pub enum Action { /// should be used. /// /// If a previous search is active, it is replaced. - Search(String), + Search(EncodedOs), /// Start a search for the current text selection. If there is no /// selection, this does nothing. If a search is already active, this /// changes the search terms. @@ -639,11 +660,11 @@ pub enum Action { /// Set the title for the current focused surface. /// /// If the title is empty, the surface title is reset to an empty title. - SetSurfaceTitle(String), + SetSurfaceTitle(EncodedOs), /// Set the title for the current focused tab. /// /// If the title is empty, the tab title override is cleared. - SetTabTitle(String), + SetTabTitle(EncodedOs), /// Create a new split in the specified direction. /// /// Valid arguments: @@ -912,7 +933,7 @@ pub enum Action { /// /// If the named key table is already the currently active key table, /// this action has no effect and performable will report false. - ActivateKeyTable(String), + ActivateKeyTable(EncodedOs), /// Same as activate_key_table, but the key table will only be active /// until the first valid keybinding from that table is used (including /// any defined `catch_all` bindings). @@ -921,7 +942,7 @@ pub enum Action { /// key table. If another key table is activated later, then this /// table will remain active until it pops back out to being the /// active key table. - ActivateKeyTableOnce(String), + ActivateKeyTableOnce(EncodedOs), /// Deactivate the currently active key table, if any. The next most /// recently activated key table (if any) will become active again. /// If no key table is active, this action has no effect. @@ -1353,15 +1374,15 @@ impl Action { } "csi" => { let p = param.ok_or(Error::InvalidFormat)?; - Ok(Action::Csi(p.to_string())) + Ok(Action::Csi(p.into())) } "esc" => { let p = param.ok_or(Error::InvalidFormat)?; - Ok(Action::Esc(p.to_string())) + Ok(Action::Esc(p.into())) } "text" => { let p = param.ok_or(Error::InvalidFormat)?; - Ok(Action::Text(p.to_string())) + Ok(Action::Text(p.into())) } "cursor_key" => Err(Error::InvalidAction), "reset" => { @@ -1426,7 +1447,7 @@ impl Action { } "search" => { let p = param.ok_or(Error::InvalidFormat)?; - Ok(Action::Search(p.to_string())) + Ok(Action::Search(p.into())) } "search_selection" => { if param.is_some() { @@ -1604,11 +1625,11 @@ impl Action { } "set_surface_title" => { let p = param.ok_or(Error::InvalidFormat)?; - Ok(Action::SetSurfaceTitle(p.to_string())) + Ok(Action::SetSurfaceTitle(p.into())) } "set_tab_title" => { let p = param.ok_or(Error::InvalidFormat)?; - Ok(Action::SetTabTitle(p.to_string())) + Ok(Action::SetTabTitle(p.into())) } "new_split" => { let Some(p) = param else { @@ -1798,11 +1819,11 @@ impl Action { } "activate_key_table" => { let p = param.ok_or(Error::InvalidFormat)?; - Ok(Action::ActivateKeyTable(p.to_string())) + Ok(Action::ActivateKeyTable(p.into())) } "activate_key_table_once" => { let p = param.ok_or(Error::InvalidFormat)?; - Ok(Action::ActivateKeyTableOnce(p.to_string())) + Ok(Action::ActivateKeyTableOnce(p.into())) } "deactivate_key_table" => { if param.is_some() { @@ -1892,15 +1913,15 @@ impl Action { | Action::Quit => {} Action::Csi(v) => { out.push(':'); - string_escape(v, out); + string_escape(&v.to_bytes(), out); } Action::Esc(v) => { out.push(':'); - string_escape(v, out); + string_escape(&v.to_bytes(), out); } Action::Text(v) => { out.push(':'); - string_escape(v, out); + string_escape(&v.to_bytes(), out); } Action::CursorKey(_) => panic!("formatting not supported"), Action::CopyToClipboard(v) => { @@ -1921,7 +1942,7 @@ impl Action { } Action::Search(v) => { out.push(':'); - string_escape(v, out); + string_escape(&v.to_bytes(), out); } Action::NavigateSearch(v) => { out.push(':'); @@ -1969,11 +1990,11 @@ impl Action { } Action::SetSurfaceTitle(v) => { out.push(':'); - string_escape(v, out); + string_escape(&v.to_bytes(), out); } Action::SetTabTitle(v) => { out.push(':'); - string_escape(v, out); + string_escape(&v.to_bytes(), out); } Action::NewSplit(v) => { out.push(':'); @@ -2001,11 +2022,11 @@ impl Action { } Action::ActivateKeyTable(v) => { out.push(':'); - string_escape(v, out); + string_escape(&v.to_bytes(), out); } Action::ActivateKeyTableOnce(v) => { out.push(':'); - string_escape(v, out); + string_escape(&v.to_bytes(), out); } Action::Crash(v) => { out.push(':'); diff --git a/crates/ghostty-input/src/binding/mod.rs b/crates/ghostty-input/src/binding/mod.rs index 562d56e..91e78a8 100644 --- a/crates/ghostty-input/src/binding/mod.rs +++ b/crates/ghostty-input/src/binding/mod.rs @@ -129,7 +129,11 @@ pub struct Parser<'a> { impl<'a> Parser<'a> { pub fn new(raw_input: &'a str) -> Result, Error> { - let (flags, start_idx) = parse_flags(raw_input)?; + Self::from_bytes(raw_input.as_bytes()) + } + + pub fn from_bytes(raw_input: &'a [u8]) -> Result, Error> { + let (flags, start_idx) = parse_flags_bytes(raw_input)?; let input = &raw_input[start_idx..]; // Find the equal sign. This is more complicated than it seems on @@ -140,8 +144,8 @@ impl<'a> Parser<'a> { // based parser that parses the trigger fully, then yields the // action after. The loop below is a total mess. let mut offset = 0usize; - let bytes = input.as_bytes(); - while let Some(offset_idx) = input[offset..].find('=') { + let bytes = input; + while let Some(offset_idx) = input[offset..].iter().position(|b| *b == b'=') { // Find: '=+ctrl' or '==action' let idx = offset + offset_idx; if idx + 1 < bytes.len() && (bytes[idx + 1] == b'+' || bytes[idx + 1] == b'=') { @@ -157,7 +161,7 @@ impl<'a> Parser<'a> { }; // Detect chains. Chains must not have flags. - let chain = &input[..eql_idx] == "chain"; + let chain = &input[..eql_idx] == b"chain"; if chain && start_idx > 0 { return Err(Error::InvalidFormat); } @@ -169,10 +173,14 @@ impl<'a> Parser<'a> { // This is kind of hacky but we put a dummy trigger // for chained inputs. The `next` will never yield this // because we have chain set. - input: if chain { "a" } else { &input[..eql_idx] }, + input: if chain { + "a" + } else { + std::str::from_utf8(&input[..eql_idx]).map_err(|_| Error::InvalidFormat)? + }, i: 0, }, - action: Action::parse(&input[eql_idx + 1..])?, + action: Action::parse_bytes(&input[eql_idx + 1..])?, flags, chain, }) @@ -211,36 +219,36 @@ impl<'a> Parser<'a> { } } -fn parse_flags(raw_input: &str) -> Result<(Flags, usize), Error> { +fn parse_flags_bytes(raw_input: &[u8]) -> Result<(Flags, usize), Error> { let mut flags = Flags::default(); let mut start_idx = 0usize; let mut input = raw_input; // Find the next prefix - while let Some(idx) = input.find(':') { + while let Some(idx) = input.iter().position(|b| *b == b':') { let prefix = &input[..idx]; // If the prefix is one of our flags then set it. match prefix { - "all" => { + b"all" => { if flags.all { return Err(Error::InvalidFormat); } flags.all = true; } - "global" => { + b"global" => { if flags.global { return Err(Error::InvalidFormat); } flags.global = true; } - "unconsumed" => { + b"unconsumed" => { if !flags.consumed { return Err(Error::InvalidFormat); } flags.consumed = false; } - "performable" => { + b"performable" => { if flags.performable { return Err(Error::InvalidFormat); } diff --git a/crates/ghostty-input/src/binding/set.rs b/crates/ghostty-input/src/binding/set.rs index 1a9bc77..d808364 100644 --- a/crates/ghostty-input/src/binding/set.rs +++ b/crates/ghostty-input/src/binding/set.rs @@ -162,7 +162,11 @@ impl Value { let pos = buffer.len(); for (k, v) in &set.bindings { buffer.truncate(pos); - let _ = write!(buffer, ">{}", k.0); + let _ = write!( + buffer, + ">{}", + ghostty_os::osbytes::valid_str_to_config_string(&k.0.to_string()) + ); v.format_entries(buffer, formatter); } buffer.truncate(pos); @@ -285,9 +289,13 @@ impl Set { /// /// Any Error returned is validated before any set modifications are made. pub fn parse_and_put(&mut self, input: &str) -> Result<(), Error> { + self.parse_and_put_bytes(input.as_bytes()) + } + + pub fn parse_and_put_bytes(&mut self, input: &[u8]) -> Result<(), Error> { // To make cleanup easier, we ensure that the full sequence is // valid before making any set modifications. - let mut it = Parser::new(input)?; + let mut it = Parser::from_bytes(input)?; while it.next_elem()?.is_some() {} it.reset(); diff --git a/crates/ghostty-input/src/binding/tests.rs b/crates/ghostty-input/src/binding/tests.rs index b808eb1..7fcc3c1 100644 --- a/crates/ghostty-input/src/binding/tests.rs +++ b/crates/ghostty-input/src/binding/tests.rs @@ -824,7 +824,7 @@ fn set_clone_with_leaf_chained_containing_allocated_data() { ) else { panic!("expected text"); }; - assert_ne!(orig.as_ptr(), copy.as_ptr()); + assert_ne!(orig.as_encoded().as_ptr(), copy.as_encoded().as_ptr()); } #[test] diff --git a/crates/ghostty-input/src/command.rs b/crates/ghostty-input/src/command.rs index 26ad62d..01971c1 100644 --- a/crates/ghostty-input/src/command.rs +++ b/crates/ghostty-input/src/command.rs @@ -31,16 +31,16 @@ use crate::binding::{ #[derive(Debug, Clone, PartialEq)] pub struct Command { pub action: Action, - pub title: String, - pub description: String, + pub title: ghostty_os::osbytes::EncodedOs, + pub description: ghostty_os::osbytes::EncodedOs, } impl Command { pub fn new(action: Action, title: &str, description: &str) -> Command { Command { action, - title: title.to_string(), - description: description.to_string(), + title: title.into(), + description: description.into(), } } @@ -75,14 +75,17 @@ impl Command { pub struct CommandC { pub action_key: String, pub action: String, - pub title: String, - pub description: String, + pub title: ghostty_os::osbytes::EncodedOs, + pub description: ghostty_os::osbytes::EncodedOs, } /// `std.ascii.orderIgnoreCase` -fn cmp_ignore_case(a: &str, b: &str) -> std::cmp::Ordering { - let a = a.bytes().map(|c| c.to_ascii_lowercase()); - let b = b.bytes().map(|c| c.to_ascii_lowercase()); +fn cmp_ignore_case( + a: &ghostty_os::osbytes::EncodedOs, + b: &ghostty_os::osbytes::EncodedOs, +) -> std::cmp::Ordering { + let a = a.to_bytes().into_iter().map(|c| c.to_ascii_lowercase()); + let b = b.to_bytes().into_iter().map(|c| c.to_ascii_lowercase()); a.cmp(b) } @@ -562,7 +565,7 @@ fn default_commands() -> Vec { Command::new(Action::Redo, "Redo", "Redo the last undone action."), Command::new(Action::Quit, "Quit", "Quit the application."), Command::new( - Action::Text("👻".to_string()), + Action::Text("👻".into()), "Ghostty", "Put a little Ghostty in your terminal.", ), diff --git a/crates/ghostty-lib/src/action_c.rs b/crates/ghostty-lib/src/action_c.rs index 7caab11..363e293 100644 --- a/crates/ghostty-lib/src/action_c.rs +++ b/crates/ghostty-lib/src/action_c.rs @@ -127,8 +127,8 @@ pub fn action_to_c(action: &Action, keep: &mut Keep) -> ghostty_action_s { v.desktop_notification.title = keep.cstr(&n.title); v.desktop_notification.body = keep.cstr(&n.body); } - Action::SetTitle(t) => v.set_title.title = keep.cstr(&t.title), - Action::SetTabTitle(t) => v.set_tab_title.title = keep.cstr(&t.title), + Action::SetTitle(t) => v.set_title.title = keep.cstr_bytes(&t.title.to_bytes()), + Action::SetTabTitle(t) => v.set_tab_title.title = keep.cstr_bytes(&t.title.to_bytes()), Action::PromptTitle(p) => v.prompt_title = *p as c_int, Action::Pwd(p) => { v.pwd.pwd = keep.cstr_bytes(&p.pwd.to_bytes()); @@ -158,8 +158,9 @@ pub fn action_to_c(action: &Action, keep: &mut Keep) -> ghostty_action_s { } Action::KeyTable(KeyTable::Activate(name)) => { v.key_table.tag = 0; - v.key_table.value.activate.name = keep.cstr(name); - v.key_table.value.activate.len = name.len(); + let bytes = name.to_bytes(); + v.key_table.value.activate.name = keep.cstr_bytes(&bytes); + v.key_table.value.activate.len = bytes.len(); } Action::KeyTable(KeyTable::Deactivate) => v.key_table.tag = 1, Action::KeyTable(KeyTable::DeactivateAll) => v.key_table.tag = 2, @@ -240,6 +241,39 @@ mod tests { use super::*; + #[test] + fn c_title_actions_and_key_table_names_forward_original_bytes() { + use ghostty_os::osbytes::EncodedOs; + let raw = b"name\xff\xef\xb7\x90\xc3\xa9"; + let mut keep = Keep::default(); + for tab in [false, true] { + let title = SetTitle { + title: EncodedOs::from_bytes(raw), + }; + let action = if tab { + Action::SetTabTitle(title) + } else { + Action::SetTitle(title) + }; + let c = action_to_c(&action, &mut keep); + // SAFETY: both title union members have identical layout, pointer owned by keep. + let title = unsafe { std::ffi::CStr::from_ptr(c.action.set_title.title) }; + assert_eq!(title.to_bytes(), raw); + } + let c = action_to_c( + &Action::KeyTable(KeyTable::Activate(EncodedOs::from_bytes(raw))), + &mut keep, + ); + // SAFETY: the tag is key_table.activate and keep owns the buffer. + let name = unsafe { + std::slice::from_raw_parts( + c.action.key_table.value.activate.name.cast::(), + c.action.key_table.value.activate.len, + ) + }; + assert_eq!(name, raw); + } + #[test] fn target_cval() { let t = target_to_c(&Target::App); diff --git a/crates/ghostty-lib/src/config_capi.rs b/crates/ghostty-lib/src/config_capi.rs index b6437de..8303589 100644 --- a/crates/ghostty-lib/src/config_capi.rs +++ b/crates/ghostty-lib/src/config_capi.rs @@ -147,8 +147,8 @@ fn build_backing(value: &CValue) -> Backing { for c in cmds { b.strings.push(cstring_value(&c.action_key)); b.strings.push(cstring_value(&c.action)); - b.strings.push(cstring_value(&c.title)); - b.strings.push(cstring_value(&c.description)); + b.strings.push(cstring_value(c.title.as_encoded())); + b.strings.push(cstring_value(c.description.as_encoded())); } // The CStrings' heap buffers don't move when the Vec grows. b.commands = b @@ -265,7 +265,7 @@ pub unsafe extern "C" fn ghostty_config_load_file(ptr: ghostty_config_t, path: * if let Err(err) = c.config.load_file(&path) { log::error!( "error loading config from file path={} err={err}", - path.as_encoded() + path.to_text_lossy() ); } } @@ -511,11 +511,7 @@ pub unsafe extern "C" fn ghostty_config_trigger( } // SAFETY: `str` points to `len` bytes. let s = unsafe { std::slice::from_raw_parts(str.cast::(), len) }; - let Ok(s) = std::str::from_utf8(s) else { - log::error!("error finding trigger err=InvalidFormat"); - return default_trigger(); - }; - match BindingAction::parse(s) { + match BindingAction::parse_bytes(s) { Ok(action) => { let trigger = c .config diff --git a/crates/ghostty-lib/src/embedded.rs b/crates/ghostty-lib/src/embedded.rs index b3ab654..687dd99 100644 --- a/crates/ghostty-lib/src/embedded.rs +++ b/crates/ghostty-lib/src/embedded.rs @@ -17,7 +17,6 @@ use std::{ cell::{Cell, OnceCell, RefCell}, collections::HashMap, ffi::{CStr, CString, c_char, c_int, c_void}, - fmt::Write as _, rc::{Rc, Weak}, sync::Arc, }; @@ -547,7 +546,7 @@ pub struct SurfaceRt { cursor_pos: Cell, /// The current title of the surface. The embedded apprt saves this so /// that `get_title` works without the implementer needing to save it. - title: RefCell>, + title: RefCell>, } impl SurfaceRt { @@ -570,7 +569,7 @@ impl RtSurface for SurfaceRt { None } - fn get_title(&self) -> Option { + fn get_title(&self) -> Option { self.title.borrow().clone() } @@ -854,36 +853,21 @@ unsafe fn platform_view_from_c( } } -/// Zig's `std.zig.stringEscape` with non-ASCII code points written as -/// `\u{...}` (the config value is parsed as a Zig string; the bytes of -/// valid UTF-8 round trip either way). Bytes that are not valid UTF-8 are -/// emitted in the reversible OS-byte encoding, which the string parser -/// copies through unchanged, so `string::parse_bytes(string_escape(b)) == b`. -fn string_escape(input: &[u8]) -> String { - let mut out = String::with_capacity(input.len()); - for chunk in input.utf8_chunks() { - for c in chunk.valid().chars() { - match c { - '\n' => out.push_str("\\n"), - '\r' => out.push_str("\\r"), - '\t' => out.push_str("\\t"), - '\\' => out.push_str("\\\\"), - '"' => out.push_str("\\\""), - ' ' | '!' | '#'..='&' | '\'' | '('..='[' | ']'..='~' => out.push(c), - c if c.is_ascii() => { - let _ = write!(out, "\\x{:02x}", c as u32); - } - c => { - let _ = write!(out, "\\u{{{:x}}}", c as u32); - } - } - } - for &b in chunk.invalid() { - out.push('\u{FDD0}'); - out.push(char::from(b)); +/// Escape initial input for config/string while retaining arbitrary original bytes. +fn string_escape(input: &[u8]) -> ghostty_os::osbytes::EncodedOs { + let mut out = Vec::with_capacity(input.len()); + for &byte in input { + match byte { + b'\n' => out.extend_from_slice(b"\\n"), + b'\r' => out.extend_from_slice(b"\\r"), + b'\t' => out.extend_from_slice(b"\\t"), + b'\\' => out.extend_from_slice(b"\\\\"), + b'"' => out.extend_from_slice(b"\\\""), + 0..=0x1f | 0x7f => out.extend_from_slice(format!("\\x{byte:02x}").as_bytes()), + _ => out.push(byte), } } - out + ghostty_os::osbytes::EncodedOs::from_bytes(&out) } /// Apply the surface options to the config (the first half of Zig's @@ -921,7 +905,7 @@ unsafe fn apply_surface_options(config: &mut Config, opts: &ghostty_surface_conf if let Some(cmd) = unsafe { cstr(opts.command) }.map(CStr::to_bytes) && !cmd.is_empty() { - config.command = Some(Command::Shell(osbytes::bytes_to_config_string(cmd))); + config.command = Some(Command::Shell(osbytes::EncodedOs::from_bytes(cmd))); config.wait_after_command = true; } @@ -934,10 +918,7 @@ unsafe fn apply_surface_options(config: &mut Config, opts: &ghostty_surface_conf let (Some(k), Some(v)) = (unsafe { cstr(var.key) }, unsafe { cstr(var.value) }) else { continue; }; - config.env.put( - &osbytes::bytes_to_config_string(k.to_bytes()), - &osbytes::bytes_to_config_string(v.to_bytes()), - ); + config.env.put_bytes(k.to_bytes(), v.to_bytes()); } } @@ -1946,11 +1927,10 @@ pub unsafe extern "C" fn ghostty_surface_binding_action( let bytes = unsafe { std::slice::from_raw_parts(action.cast::(), len) }; // The action text is bytes (Zig `[]const u8`); the parser works on the // reversible OS-byte encoding so a `text:` payload keeps invalid UTF-8. - let action_str = ghostty_os::osbytes::bytes_to_config_string(bytes); - let action = match BindingAction::parse(&action_str) { + let action = match BindingAction::parse_bytes(bytes) { Ok(a) => a, Err(err) => { - log::error!("error parsing binding action action={action_str} err={err:?}"); + log::error!("error parsing binding action action={bytes:?} err={err:?}"); return false; } }; @@ -2194,7 +2174,7 @@ mod tests { ] { let esc = string_escape(raw); assert_eq!( - ghostty_config::string::parse_bytes(&esc).unwrap(), + ghostty_config::string::parse_bytes(esc.to_bytes()).unwrap(), raw, "{esc:?}" ); @@ -2241,10 +2221,10 @@ mod tests { let Some(Command::Shell(c)) = &config.command else { panic!("no command"); }; - assert_eq!(osbytes::config_str_to_bytes(c), b"echo \xfe"); + assert_eq!(c.to_bytes(), b"echo \xfe"); let (k, v) = config.env.iter().next().unwrap(); - assert_eq!(osbytes::config_str_to_bytes(k), b"K\xfd"); - assert_eq!(osbytes::config_str_to_bytes(v), b"v\xfc"); + assert_eq!(k.to_bytes(), b"K\xfd"); + assert_eq!(v.to_bytes(), b"v\xfc"); assert!(matches!( config.input.list.first(), Some(ReadableIO::Raw(_)) diff --git a/crates/ghostty-lib/tests/surface.rs b/crates/ghostty-lib/tests/surface.rs index d790cbc..9c0728a 100644 --- a/crates/ghostty-lib/tests/surface.rs +++ b/crates/ghostty-lib/tests/surface.rs @@ -45,6 +45,7 @@ struct Rec { /// (action tag, target tag, surface pointer) actions: Vec<(i32, i32, usize)>, titles: Vec, + raw_titles: Vec>, cell_size: Option<(u32, u32)>, writes: Vec<(i32, String, bool)>, reads: Vec<(i32, usize)>, @@ -86,6 +87,7 @@ unsafe extern "C" fn on_action( // set_title let t = unsafe { CStr::from_ptr(action.action.set_title.title) }; r.titles.push(t.to_string_lossy().into_owned()); + r.raw_titles.push(t.to_bytes().to_vec()); } 25 => { let c = unsafe { action.action.cell_size }; @@ -167,7 +169,7 @@ fn cs(s: &str) -> CString { } impl Host { - fn new(extra_config: &str) -> Option { + fn new(extra_config: impl AsRef<[u8]>) -> Option { rec(|r| *r = Rec::default()); let gl = match Headless::new() { Ok(g) => Rc::new(g), @@ -186,9 +188,11 @@ impl Host { write!( conf, "shell-integration = none\nwindow-padding-x = 0\nwindow-padding-y = 0\n\ - font-size = 12\nscrollback-limit = 100000\n{extra_config}\n" + font-size = 12\nscrollback-limit = 100000\n" ) .unwrap(); + conf.write_all(extra_config.as_ref()).unwrap(); + conf.write_all(b"\n").unwrap(); let cfg = ghostty_config_new(); let path = cs(conf.path().to_str().unwrap()); unsafe { @@ -856,3 +860,26 @@ fn selection_clipboard_support_is_reported() { }); unsafe { ghostty_surface_free(s) }; } + +#[test] +fn configured_and_binding_titles_reach_c_as_original_bytes() { + let Some(h) = Host::new(b"title=config\xff\xef\xb7\x90\xc3\xa9") else { + return; + }; + let surface = h.surface("sleep 30"); + assert!(rec(|r| r + .raw_titles + .iter() + .any(|v| v == b"config\xff\xef\xb7\x90\xc3\xa9"))); + let action = b"set_surface_title:binding\xfe\xef\xb7\x90\xc3\xa9"; + // SAFETY: a live surface and the action slice is valid for the entire call. + assert!(unsafe { + ghostty_surface_binding_action(surface, action.as_ptr().cast(), action.len()) + }); + assert_eq!( + rec(|r| r.raw_titles.last().cloned()).unwrap(), + b"binding\xfe\xef\xb7\x90\xc3\xa9" + ); + // SAFETY: the surface is live and owned by this test. + unsafe { ghostty_surface_free(surface) }; +} diff --git a/crates/ghostty-os/src/args.rs b/crates/ghostty-os/src/args.rs index 11e61ac..41542f3 100644 --- a/crates/ghostty-os/src/args.rs +++ b/crates/ghostty-os/src/args.rs @@ -57,16 +57,6 @@ pub fn collect_bytes() -> Vec> { iterator().map(|a| crate::osbytes::os_bytes(&a)).collect() } -/// Collect the process arguments as `String`s using the reversible -/// byte-escape encoding of [`crate::osbytes`]: no bytes are lost, invalid -/// UTF-8 bytes are carried as private-use escapes that the config layer -/// decodes back to the original bytes. -pub fn collect_config_strings() -> Vec { - iterator() - .map(|a| crate::osbytes::os_to_config_string(&a)) - .collect() -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/ghostty-os/src/flatpak.rs b/crates/ghostty-os/src/flatpak.rs index 49c6a19..39c7b49 100644 --- a/crates/ghostty-os/src/flatpak.rs +++ b/crates/ghostty-os/src/flatpak.rs @@ -1,3 +1,12 @@ +/// GLib's g_variant_new_string substitutes the entire invalid value (Zig's {ss}). +fn glib_env_string(value: &std::ffi::OsStr) -> String { + let bytes = crate::osbytes::os_bytes(value); + let bytes = bytes.split(|b| *b == 0).next().unwrap_or_default(); + std::str::from_utf8(bytes) + .unwrap_or("[Invalid UTF-8]") + .to_owned() +} + use std::{ collections::HashMap, os::fd::{AsFd, OwnedFd}, @@ -147,12 +156,7 @@ impl FlatpakHostCommand { .env .iter() .flat_map(crate::env::EnvMap::iter) - .map(|(k, v)| { - ( - k.to_string_lossy().into_owned(), - v.to_string_lossy().into_owned(), - ) - }) + .map(|(k, v)| (glib_env_string(k), glib_env_string(v))) .collect(), fds: [ self.stdin @@ -491,3 +495,20 @@ mod tests { assert_eq!(((3u32 << 8) >> 8) & 0xff, 3); } } + +#[cfg(test)] +mod byte_string_tests { + #[test] + fn glib_env_replaces_the_whole_invalid_value() { + use crate::osbytes::EncodedOs; + assert_eq!( + super::glib_env_string(&EncodedOs::from_bytes(b"before\xffafter").to_os()), + "[Invalid UTF-8]" + ); + let literal = "x\u{fdd0}é"; + assert_eq!( + super::glib_env_string(std::ffi::OsStr::new(literal)), + literal + ); + } +} diff --git a/crates/ghostty-os/src/homedir.rs b/crates/ghostty-os/src/homedir.rs index 883b695..c40f4a0 100644 --- a/crates/ghostty-os/src/homedir.rs +++ b/crates/ghostty-os/src/homedir.rs @@ -154,6 +154,13 @@ pub fn expand_home_with_capacity( path: &osbytes::EncodedOs, cap: usize, ) -> Result { + #[cfg(windows)] + { + // Zig returns the original byte slice on Windows without converting it. + let _ = cap; + Ok(path.clone()) + } + #[cfg(not(windows))] expand_home_os_with_capacity(&path.to_os(), cap).map(osbytes::EncodedOs::from) } @@ -186,6 +193,14 @@ pub fn expand_home_os_with_capacity(path: &OsStr, cap: usize) -> Result Vec { } /// Build an OS string from raw bytes (WTF-8 on Windows; bytes that are not -/// well-formed WTF-8 become U+FFFD, like Zig's `wtf8ToWtf16Le`/`utf8ToUtf16`). +/// well-formed WTF-8 become U+FFFD). Filesystem and process operations must +/// use their checked conversions instead. #[cfg(unix)] pub fn os_from_bytes(b: Vec) -> OsString { use std::os::unix::ffi::OsStringExt; @@ -182,18 +182,56 @@ pub struct InvalidWtf8; /// Strict [`wtf8_to_wide`]: malformed input is an error instead of U+FFFD. /// Lone surrogates encoded as 3-byte sequences are accepted (WTF-8). This is -/// the conversion used where Zig's is fallible (process launch). +/// the conversion used by Windows filesystem paths; process launch requires UTF-8. pub fn try_wtf8_to_wide(b: &[u8]) -> Result, InvalidWtf8> { - let wide = wtf8_to_wide(b); - // The lossy decoder yields U+FFFD only for malformed bytes; a genuine - // U+FFFD in the input is the 3-byte sequence EF BF BD. Re-encoding is - // the cheapest exact check: it reproduces the input iff it was - // well-formed. - if wide_to_wtf8(&wide) == b { - Ok(wide) - } else { - Err(InvalidWtf8) + let mut result = Vec::with_capacity(b.len()); + let mut rest = b; + while !rest.is_empty() { + match std::str::from_utf8(rest) { + Ok(text) => { + result.extend(text.encode_utf16()); + break; + } + Err(error) => { + let (valid, tail) = rest.split_at(error.valid_up_to()); + // valid_up_to is guaranteed to be a valid UTF-8 prefix. + result.extend( + std::str::from_utf8(valid) + .map_err(|_| InvalidWtf8)? + .encode_utf16(), + ); + // Zig's WTF-8 validation accepts both lone and paired surrogate encodings. + if tail.len() < 3 + || tail[0] != 0xed + || !(0xa0..=0xbf).contains(&tail[1]) + || !(0x80..=0xbf).contains(&tail[2]) + { + return Err(InvalidWtf8); + } + result.push(0xd000 | (u16::from(tail[1] & 0x3f) << 6) | u16::from(tail[2] & 0x3f)); + rest = &tail[3..]; + } + } } + Ok(result) +} + +/// Strict UTF-8 conversion used by Zig's Windows Command (including its environment). +#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] +#[error("invalid utf-8")] +pub struct InvalidUtf8; + +pub fn try_utf8_to_wide(bytes: &[u8]) -> Result, InvalidUtf8> { + Ok(std::str::from_utf8(bytes) + .map_err(|_| InvalidUtf8)? + .encode_utf16() + .collect()) +} + +/// Zig's Windows filesystem conversion has a different error and accepts WTF-8. +pub fn windows_path_wide(bytes: &[u8]) -> std::io::Result> { + try_wtf8_to_wide(bytes) + .map_err(|_| std::io::Error::new(std::io::ErrorKind::InvalidInput, "BadPathName")) } /// OS string to reversible `String` (see the module docs). Lossless on every @@ -258,8 +296,8 @@ impl EncodedOs { EncodedOs(bytes_to_config_string(b)) } - /// Adopt a string that is *already* in the escape encoding (config - /// values, CLI arguments collected with `args::collect_config_strings`). + /// Adopt a string already in the internal escape encoding. Config and CLI + /// input must use `from_bytes`; plain Rust text must use `from_plain_text`. #[must_use] pub fn from_encoded(s: impl Into) -> Self { EncodedOs(s.into()) @@ -272,8 +310,8 @@ impl EncodedOs { EncodedOs(valid_str_to_config_string(s)) } - /// The encoded text (config files, `+show-config`, display of config - /// values). Never pass this to the OS; decode first. + /// Internal encoded text for storage/output formatting. Decode before + /// writing config output or passing it to OS APIs and field parsers. #[must_use] pub fn as_encoded(&self) -> &str { &self.0 @@ -302,6 +340,11 @@ impl EncodedOs { config_str_to_bytes(&self.0) } + /// Render bytes for a UI or log boundary which requires Unicode text. + pub fn to_text_lossy(&self) -> String { + String::from_utf8_lossy(&self.to_bytes()).into_owned() + } + /// Decode to an OS string. #[must_use] pub fn to_os(&self) -> OsString { @@ -316,12 +359,30 @@ impl EncodedOs { wtf8_to_wide(&self.to_bytes()) } - /// Strict [`EncodedOs::to_wide`] for process launch: malformed WTF-8 is + /// Strict [`EncodedOs::to_wide`] for WTF-8 data: malformed WTF-8 is /// an error (Zig's `utf8ToUtf16Le*` fail) instead of becoming U+FFFD. pub fn try_to_wide(&self) -> Result, InvalidWtf8> { try_wtf8_to_wide(&self.to_bytes()) } + /// Windows process inputs use strict UTF-8, unlike filesystem paths. + pub fn try_to_utf16(&self) -> Result, InvalidUtf8> { + try_utf8_to_wide(&self.to_bytes()) + } + + pub fn try_to_utf16_nul(&self) -> Result, InvalidUtf8> { + let mut wide = self.try_to_utf16()?; + wide.push(0); + Ok(wide) + } + + pub fn try_to_utf8_os(&self) -> Result { + let bytes = self.to_bytes(); + Ok(OsString::from( + std::str::from_utf8(&bytes).map_err(|_| InvalidUtf8)?, + )) + } + /// [`EncodedOs::try_to_wide`] plus a trailing NUL. pub fn try_to_wide_nul(&self) -> Result, InvalidWtf8> { let mut w = self.try_to_wide()?; @@ -357,10 +418,17 @@ impl EncodedOs { Self::from_bytes(&wide_to_wtf8(w)) } - /// Decode to a path. - #[must_use] - pub fn to_path(&self) -> std::path::PathBuf { - config_str_to_path(&self.0) + /// Decode to a filesystem path, rejecting malformed Windows WTF-8 as BadPathName. + pub fn to_path(&self) -> std::io::Result { + #[cfg(windows)] + { + use std::os::windows::ffi::OsStringExt; + Ok(OsString::from_wide(&windows_path_wide(&self.to_bytes())?).into()) + } + #[cfg(not(windows))] + { + Ok(self.to_os().into()) + } } /// Whether the decoded value starts with the given plain prefix. @@ -408,6 +476,18 @@ impl PartialEq<&str> for EncodedOs { } } +/// Plain UTF-8 constructors: string literals never carry escape-encoded bytes. +impl From<&str> for EncodedOs { + fn from(s: &str) -> Self { + Self::from_plain_text(s) + } +} +impl From for EncodedOs { + fn from(s: String) -> Self { + Self::from_plain_text(&s) + } +} + impl From<&OsStr> for EncodedOs { fn from(s: &OsStr) -> Self { Self::from_os(s) @@ -438,14 +518,59 @@ pub fn path_to_config_string(p: &std::path::Path) -> String { } /// Inverse of [`path_to_config_string`]. -pub fn config_str_to_path(s: &str) -> std::path::PathBuf { - std::path::PathBuf::from(config_str_to_os(s)) +pub fn config_str_to_path(s: &str) -> std::io::Result { + EncodedOs::from_encoded(s).to_path() } #[cfg(test)] mod tests { use super::*; + #[test] + fn windows_paths_fail_with_bad_path_name_instead_of_replacement() { + let error = windows_path_wide(b"C:\\bad\xff").unwrap_err(); + assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput); + assert_eq!(error.to_string(), "BadPathName"); + assert_eq!( + windows_path_wide("C:\\bad\u{fffd}".as_bytes()) + .unwrap() + .last(), + Some(&0xfffd) + ); + assert_eq!(windows_path_wide(b"\xed\xa0\x80").unwrap(), [0xd800]); + } + + #[test] + fn wtf8_accepts_separately_encoded_surrogate_pairs_like_zig() { + assert_eq!( + try_wtf8_to_wide(b"\xed\xa0\xbd\xed\xb8\x80"), + Ok(vec![0xd83d, 0xde00]) + ); + assert_eq!( + windows_path_wide(b"x\xed\xa0\xbd\xed\xb8\x80y").unwrap(), + [0x78, 0xd83d, 0xde00, 0x79] + ); + assert_eq!(try_wtf8_to_wide(b"\xed\xa0\xbd\xff"), Err(InvalidWtf8)); + } + + #[test] + fn windows_command_inputs_require_strict_utf8() { + for bytes in [ + b"\xff".as_slice(), + b"\xed\xa0\x80", + b"\xed\xa0\xbd\xed\xb8\x80", + ] { + let value = EncodedOs::from_bytes(bytes); + assert_eq!(value.try_to_utf16(), Err(InvalidUtf8)); + assert_eq!(value.try_to_utf16_nul(), Err(InvalidUtf8)); + assert_eq!(value.try_to_utf8_os(), Err(InvalidUtf8)); + } + assert_eq!( + try_utf8_to_wide("😀\u{fdd0}".as_bytes()).unwrap(), + [0xd83d, 0xde00, 0xfdd0] + ); + } + #[test] fn roundtrip_invalid_bytes() { let raw = vec![b'a', 0xff, b'b', 0xc3, 0xa9, 0xc3]; @@ -634,7 +759,7 @@ mod tests { let o = os_from_bytes(vec![b'/', 0xff, b'x']); let e = EncodedOs::from_os(&o); assert_eq!(e.to_os(), o); - assert_eq!(e.to_path(), std::path::PathBuf::from(&o)); + assert_eq!(e.to_path().unwrap(), std::path::PathBuf::from(&o)); assert_eq!(e.join_plain(&["a", "", "b"]).to_bytes(), b"/\xffx/a/b"); } diff --git a/crates/ghostty-os/src/path.rs b/crates/ghostty-os/src/path.rs index dfc3f8a..40ac59f 100644 --- a/crates/ghostty-os/src/path.rs +++ b/crates/ghostty-os/src/path.rs @@ -28,7 +28,7 @@ pub enum Error { /// Search for "cmd" in the PATH and return the absolute path. Non-UTF-8 /// bytes are carried via [`crate::osbytes`]; see [`expand_os`]. pub fn expand(cmd: &crate::osbytes::EncodedOs) -> Result, Error> { - Ok(expand_os(&cmd.to_os())?.map(crate::osbytes::EncodedOs::from)) + Ok(expand_os(cmd.to_path()?.as_os_str())?.map(crate::osbytes::EncodedOs::from)) } /// Byte-preserving [`expand`]: `$PATH` and the command are handled as raw @@ -61,7 +61,7 @@ pub fn expand_os(cmd: &OsStr) -> Result, Error> { let mut full = search_path.to_vec(); full.push(SEP as u8); full.extend_from_slice(&cmd_bytes); - let full_path = PathBuf::from(crate::osbytes::os_from_bytes(full)); + let full_path = crate::osbytes::EncodedOs::from_bytes(&full).to_path()?; // Stat it let f = match std::fs::File::open(&full_path) { @@ -111,6 +111,16 @@ mod tests { } // `uname -n` is the *nix equivalent of `hostname.exe` on Windows + #[cfg(windows)] + #[test] + fn expand_rejects_malformed_windows_path_bytes() { + let path = crate::osbytes::EncodedOs::from_bytes(b"bad\xff"); + let Error::Io(error) = super::expand(&path).unwrap_err() else { + panic!("path conversion error") + }; + assert_eq!(error.to_string(), "BadPathName"); + } + #[test] fn expand_hostname() { let executable = if cfg!(windows) { diff --git a/crates/ghostty-os/src/resourcesdir.rs b/crates/ghostty-os/src/resourcesdir.rs index 479f2b7..6155759 100644 --- a/crates/ghostty-os/src/resourcesdir.rs +++ b/crates/ghostty-os/src/resourcesdir.rs @@ -37,12 +37,12 @@ impl ResourcesDir { /// Byte-preserving [`ResourcesDir::app`]: the string fields carry /// non-UTF-8 bytes via [`crate::osbytes`], this decodes them. pub fn app_os(&self) -> Option { - self.app().map(EncodedOs::to_path) + self.app().and_then(|p| p.to_path().ok()) } /// Byte-preserving [`ResourcesDir::host`]. pub fn host_os(&self) -> Option { - self.host().map(EncodedOs::to_path) + self.host().and_then(|p| p.to_path().ok()) } } diff --git a/crates/ghostty-renderer/src/generic.rs b/crates/ghostty-renderer/src/generic.rs index 242d2ed..45357a2 100644 --- a/crates/ghostty-renderer/src/generic.rs +++ b/crates/ghostty-renderer/src/generic.rs @@ -200,7 +200,7 @@ enum HighlightTag { pub struct DerivedConfig { pub font_thicken: bool, pub font_thicken_strength: u8, - pub font_features: Vec, + pub font_features: Vec, pub font_styles: [bool; 4], pub font_shaping_break: FontShapingBreak, pub cursor_color: Option, @@ -243,10 +243,18 @@ impl DerivedConfig { .custom_shader .value .iter() - .map(|p| match p { - // Decode the config path (encoded OS string) here, once. - ConfigPath::Optional(s) => PathItem::Optional(s.to_path()), - ConfigPath::Required(s) => PathItem::Required(s.to_path()), + .filter_map(|p| { + let path = match p.path().to_path() { + Ok(path) => path, + Err(error) => { + log::warn!("invalid shader path: {error}"); + return None; + } + }; + Some(match p { + ConfigPath::Optional(_) => PathItem::Optional(path), + ConfigPath::Required(_) => PathItem::Required(path), + }) }) .collect(); @@ -859,8 +867,13 @@ impl DrawState { fn prep_background_image(&mut self) { // Then we try to load the background image if we have a path. if let Some(p) = &self.config.bg_image - && let Some(image) = - load_background_image(&p.path().to_path()).map(image::Image::Pending) + && let Some(image) = p + .path() + .to_path() + .map_err(|error| log::warn!("invalid background image path: {error}")) + .ok() + .and_then(|path| load_background_image(&path)) + .map(image::Image::Pending) { // If we have an existing background image, replace it. // Otherwise, set this as our background image directly. diff --git a/crates/ghostty-renderer/src/link.rs b/crates/ghostty-renderer/src/link.rs index c46c115..b9638e1 100644 --- a/crates/ghostty-renderer/src/link.rs +++ b/crates/ghostty-renderer/src/link.rs @@ -43,7 +43,7 @@ impl Set { let mut links = Vec::with_capacity(config.len()); for link in config { links.push(Link { - regex: Regex::new(&ghostty_os::osbytes::config_str_to_text(&link.regex))?, + regex: Regex::new(&link.regex)?, highlight: link.highlight, }); } diff --git a/crates/ghostty-termio/src/command_windows.rs b/crates/ghostty-termio/src/command_windows.rs index 6d96283..332fa04 100644 --- a/crates/ghostty-termio/src/command_windows.rs +++ b/crates/ghostty-termio/src/command_windows.rs @@ -50,8 +50,8 @@ use crate::windows_args::{self, create_windows_env_block, windows_create_command /// Errors from [`Command::start`]. #[derive(Debug, thiserror::Error)] pub enum StartError { - #[error("malformed wtf-8 in a path, argument or environment value")] - InvalidWtf8(#[from] ghostty_os::osbytes::InvalidWtf8), + #[error("invalid utf-8 in a path, argument or environment value")] + InvalidUtf8(#[from] ghostty_os::osbytes::InvalidUtf8), #[error("string or environment is not valid unicode")] Encode(#[from] windows_args::EncodeError), #[error("windows error: {0}")] @@ -132,14 +132,14 @@ impl Command { #[allow(unsafe_code)] pub fn start(&mut self) -> Result<(), StartError> { // `path` and `args` are the config command's argv strings (encoded - // OS strings); decode them to WTF-16 for the Windows API. + // OS strings); validate UTF-8 and convert to UTF-16 for the Windows API. // Like Zig's fallible `utf8ToUtf16Le*`, malformed input fails the // launch instead of selecting a U+FFFD-substituted path or argument. - let application_w = EncodedOs::from_encoded(&self.path).try_to_wide_nul()?; + let application_w = EncodedOs::from_encoded(&self.path).try_to_utf16_nul()?; let cwd_w = self .cwd .as_ref() - .map(EncodedOs::try_to_wide_nul) + .map(EncodedOs::try_to_utf16_nul) .transpose()?; let mut command_line_w = if self.args.is_empty() { None @@ -147,7 +147,7 @@ impl Command { let wide: Vec> = self .args .iter() - .map(|a| EncodedOs::from_encoded(a).try_to_wide()) + .map(|a| EncodedOs::from_encoded(a).try_to_utf16()) .collect::>()?; let mut line = windows_create_command_line_wide(&wide); line.push(0); diff --git a/crates/ghostty-termio/src/exec.rs b/crates/ghostty-termio/src/exec.rs index ddee46e..8166bb4 100644 --- a/crates/ghostty-termio/src/exec.rs +++ b/crates/ghostty-termio/src/exec.rs @@ -790,7 +790,7 @@ impl Subprocess { // If we have a resources dir then set our env var if let Some(dir) = &cfg.resources_dir { - log::info!("found Ghostty resources dir: {}", dir.as_encoded()); + log::info!("found Ghostty resources dir: {}", dir.to_text_lossy()); env.insert("GHOSTTY_RESOURCES_DIR", dir.to_os()); } @@ -900,10 +900,7 @@ impl Subprocess { // Add the environment variables that override any others. for (k, v) in cfg.env_override.iter() { - env.insert( - ghostty_os::osbytes::config_str_to_os(k), - ghostty_os::osbytes::config_str_to_os(v), - ); + env.insert(k.to_os(), v.to_os()); } // Build our args list @@ -1033,10 +1030,10 @@ impl Subprocess { } } Some(proposed) => { - if proposed.to_path().exists() { + if proposed.to_path().is_ok_and(|p| p.exists()) { Some(proposed.clone()) } else { - log::warn!("cannot access cwd, ignoring: {}", proposed.as_encoded()); + log::warn!("cannot access cwd, ignoring: {}", proposed.to_text_lossy()); None } } @@ -1399,7 +1396,7 @@ pub fn exec_command_with( match command { // We need to clone the command since there's no guarantee the config remains valid. - ConfigCommand::Direct(args) => args.clone(), + ConfigCommand::Direct(args) => args.iter().map(|v| v.as_encoded().to_owned()).collect(), ConfigCommand::Shell(v) => { let mut args: Vec = Vec::with_capacity(4); @@ -1414,7 +1411,7 @@ pub fn exec_command_with( args.push("-l".into()); } args.push("-c".into()); - args.push(v.clone()); + args.push(v.as_encoded().to_owned()); args } } @@ -1442,7 +1439,7 @@ fn darwin_login_command( let hush = match &passwd.home { Some(home) => { // The passwd home is an encoded OS string: decode it first. - let home = home.to_path(); + let home = home.to_path().ok()?; if std::fs::read_dir(&home).is_err() { log::warn!("failed to open home dir, not checking for hushlogin"); false @@ -1517,13 +1514,13 @@ fn darwin_login_command( // Direct args can be passed directly to login, since // login uses execvp we don't need to worry about PATH // searching. - ConfigCommand::Direct(v) => args.extend(v.iter().cloned()), + ConfigCommand::Direct(v) => args.extend(v.iter().map(|v| v.as_encoded().to_owned())), ConfigCommand::Shell(v) => { // Use "exec" to replace the bash process with // our intended command so we don't have a parent // process hanging around. - let cmd = format!("exec -l {v}"); + let cmd = format!("exec -l {}", v.as_encoded()); // We execute bash with "--noprofile --norc" so that it doesn't // load startup files so that (1) our shell integration doesn't diff --git a/crates/ghostty-termio/src/exec_windows.rs b/crates/ghostty-termio/src/exec_windows.rs index eca4960..434d1ea 100644 --- a/crates/ghostty-termio/src/exec_windows.rs +++ b/crates/ghostty-termio/src/exec_windows.rs @@ -80,9 +80,9 @@ pub enum Error { #[error(transparent)] ShellIntegration(#[from] shell_integration::Error), /// A configured path, argument or environment value is not well-formed - /// WTF-8 (Zig's process launch conversions fail on such input). + /// UTF-8 (Zig's process launch conversions fail on such input). #[error(transparent)] - InvalidWtf8(#[from] ghostty_os::osbytes::InvalidWtf8), + InvalidUtf8(#[from] ghostty_os::osbytes::InvalidUtf8), #[error("io error: {0}")] Io(#[from] io::Error), #[error("failed to get WINDIR, cannot run shell command")] @@ -588,19 +588,16 @@ fn write_thread_main(handle: &OwnedHandle, rx: &Receiver>) { // Subprocess /// Insert the config `env` overrides, decoding the encoded key/value text -/// to OS strings (WTF-16 on Windows) like the Unix implementation. +/// to OS strings after strict UTF-8 validation, as in Zig. /// -/// Malformed WTF-8 fails (Zig's conversions for the environment block are +/// Invalid UTF-8 fails (Zig's conversions for the environment block are /// fallible) instead of being substituted with U+FFFD. fn apply_env_overrides( env: &mut EnvMap, overrides: &RepeatableStringMap, -) -> Result<(), ghostty_os::osbytes::InvalidWtf8> { +) -> Result<(), ghostty_os::osbytes::InvalidUtf8> { for (k, v) in overrides.iter() { - env.insert( - EncodedOs::from_encoded(k).try_to_os()?, - EncodedOs::from_encoded(v).try_to_os()?, - ); + env.insert(k.try_to_utf8_os()?, v.try_to_utf8_os()?); } Ok(()) } @@ -625,10 +622,10 @@ impl Subprocess { // If we have a resources dir then set our env var if let Some(dir) = &cfg.resources_dir { - log::info!("found Ghostty resources dir: {}", dir.as_encoded()); + log::info!("found Ghostty resources dir: {}", dir.to_text_lossy()); // The resources dir is an encoded OS string: decode it for the // child's environment. - env.insert("GHOSTTY_RESOURCES_DIR", dir.try_to_os()?); + env.insert("GHOSTTY_RESOURCES_DIR", dir.try_to_utf8_os()?); } // Set our TERM var. This is a bit complicated because we want to use @@ -636,13 +633,13 @@ impl Subprocess { // ghostty in the TERMINFO database. if let Some(base) = &cfg.resources_dir { // The configured TERM is an encoded OS string: export its bytes. - env.insert("TERM", cfg.term.try_to_os()?); + env.insert("TERM", cfg.term.try_to_utf8_os()?); env.insert("COLORTERM", "truecolor"); // Assume that the resources directory is adjacent to the terminfo // database - base.try_to_os()?; // malformed WTF-8 fails like Zig - env.insert("TERMINFO", terminfo_dir(base)); + base.try_to_utf8_os()?; // invalid UTF-8 fails like Zig + env.insert("TERMINFO", terminfo_dir(base)?); } else { env.insert("TERM", "xterm-256color"); env.insert("COLORTERM", "truecolor"); @@ -716,7 +713,7 @@ impl Subprocess { // Build our args list. If we fail to find the shell, we'd still like // to try to run something so Ghostty can launch. // %WINDIR% may hold lone surrogates: carry it in the encoded form the - // argv strings use (decoded to WTF-16 when the process is created). + // argv strings use (validated as UTF-8 when the process is created). let windir = std::env::var_os("WINDIR").map(|v| EncodedOs::from_os(&v).into_encoded()); let args = match exec_command(&shell_command, windir.as_deref()) { Ok(args) => args, @@ -733,7 +730,7 @@ impl Subprocess { // Propagate the current working directory (CWD) to the shell. if let Some(pwd) = &cwd { - env.insert("PWD", pwd.try_to_os()?); + env.insert("PWD", pwd.try_to_utf8_os()?); } Ok(Subprocess { @@ -804,7 +801,7 @@ impl Subprocess { // and we don't want to break new windows. let cwd: Option = match &self.cwd { None => None, - Some(proposed) => match std::fs::metadata(proposed.to_path()) { + Some(proposed) => match proposed.to_path().and_then(std::fs::metadata) { Ok(_) => Some(proposed.clone()), Err(err) => { log::warn!("cannot access cwd, ignoring: {err}"); @@ -912,13 +909,13 @@ fn add_ghostty_path_for(env: &mut EnvMap, exe: &std::path::Path) { /// The `TERMINFO` value for a resources dir: `\\terminfo` as /// an OS string (the resources dir is an encoded OS string). -fn terminfo_dir(base: &EncodedOs) -> std::ffi::OsString { - let base = base.to_path(); +fn terminfo_dir(base: &EncodedOs) -> std::io::Result { + let base = base.to_path()?; let mut out = base .parent() .map_or_else(std::ffi::OsString::new, |p| p.as_os_str().to_owned()); out.push("\\terminfo"); - out + Ok(out) } /// Terminate the underlying subprocess and wait for it to exit @@ -937,11 +934,11 @@ fn kill_command(command: &Command) -> Result<(), Error> { pub fn exec_command(command: &ConfigCommand, windir: Option<&str>) -> Result, Error> { match command { // We need to clone the command since there's no guarantee the config remains valid. - ConfigCommand::Direct(args) => Ok(args.clone()), + ConfigCommand::Direct(args) => Ok(args.iter().map(|v| v.as_encoded().to_owned()).collect()), ConfigCommand::Shell(v) => { let windir = windir.ok_or(Error::SystemError)?; - Ok(windows_shell_args(windir, v)) + Ok(windows_shell_args(windir, v.as_encoded())) } } } @@ -949,35 +946,24 @@ pub fn exec_command(command: &ConfigCommand, windir: Option<&str>) -> Result = "C:\\Users\\".encode_utf16().collect(); want.push(0xD800); want.extend("\\share\\terminfo".encode_utf16()); diff --git a/crates/ghostty-termio/src/shell_integration.rs b/crates/ghostty-termio/src/shell_integration.rs index 1c62c37..2b9ab7c 100644 --- a/crates/ghostty-termio/src/shell_integration.rs +++ b/crates/ghostty-termio/src/shell_integration.rs @@ -48,6 +48,9 @@ pub use ghostty_config::{command::Command, types_b::ShellIntegrationFeatures}; pub enum Error { #[error("unable to determine home directory: {0}")] Home(#[from] homedir::Error), + #[cfg(windows)] + #[error(transparent)] + InvalidUtf8(#[from] ghostty_os::osbytes::InvalidUtf8), } /// The result of setting up a shell integration. @@ -65,11 +68,17 @@ pub struct ShellIntegration { /// `/` as an OS string. `resource_dir` carries the /// reversible config-string encoding (it may hold non-UTF-8 bytes), so it is /// decoded before reaching the filesystem or the child environment. -fn resource_path(resource_dir: &EncodedOs, rel: &str) -> std::ffi::OsString { - let mut p = resource_dir.to_os(); - p.push("/"); - p.push(rel); - p +fn resource_path(resource_dir: &EncodedOs, rel: &str) -> Option { + let mut path = resource_dir.to_bytes(); + path.push(b'/'); + path.extend_from_slice(rel.as_bytes()); + match EncodedOs::from_bytes(&path).to_path() { + Ok(path) => Some(path.into_os_string()), + Err(error) => { + log::warn!("invalid shell integration path: {error}"); + None + } + } } /// Set up the command execution environment for automatic @@ -118,7 +127,7 @@ fn detect_shell(command: &Command) -> Option { fn detect_shell_for(command: &Command, darwin: bool) -> Option { let mut it = command.arg_iterator(); let arg0 = it.next()?; - let exe = basename(&arg0); + let exe = basename(arg0.as_encoded()); match exe { "bash" => { @@ -195,7 +204,7 @@ fn setup_bash( // Start accumulating arguments with the executable and initial flags. match iter.next() { - Some(exe) => cmd.append_arg(&exe), + Some(exe) => cmd.append_arg(exe.as_encoded()), None => return Ok(None), } cmd.append_arg("--posix"); @@ -216,6 +225,7 @@ fn setup_bash( // --posix POSIX mode (a la /bin/sh) let mut rcfile: Option = None; while let Some(arg) = iter.next() { + let arg = arg.as_encoded(); if arg == "--posix" { return Ok(None); } else if arg == "--norc" { @@ -223,23 +233,23 @@ fn setup_bash( } else if arg == "--noprofile" { inject.push_str(" --noprofile"); } else if arg == "--rcfile" || arg == "--init-file" { - rcfile = iter.next(); + rcfile = iter.next().map(EncodedOs::into_encoded); } else if arg.len() > 1 && arg.starts_with('-') && !arg[1..].starts_with('-') { // '-c command' is always non-interactive if arg.contains('c') { return Ok(None); } - cmd.append_arg(&arg); + cmd.append_arg(arg); } else if arg == "-" || arg == "--" { // All remaining arguments should be passed directly to the shell // command. We shouldn't perform any further option processing. - cmd.append_arg(&arg); + cmd.append_arg(arg); for remaining in iter.by_ref() { - cmd.append_arg(&remaining); + cmd.append_arg(remaining.as_encoded()); } break; } else { - cmd.append_arg(&arg); + cmd.append_arg(arg); } } @@ -249,7 +259,11 @@ fn setup_bash( } // Set our new ENV to point to our integration script. - let script_path = resource_path(resource_dir, "shell-integration/bash/ghostty.bash"); + let Some(script_path) = resource_path(resource_dir, "shell-integration/bash/ghostty.bash") + else { + env.remove("GHOSTTY_BASH_ENV"); + return Ok(None); + }; match std::fs::File::open(&script_path) { Ok(_) => { env.insert("ENV", script_path); @@ -265,7 +279,12 @@ fn setup_bash( if let Some(v) = rcfile { // The intercepted argument is an encoded OS string; the env var gets // its original bytes (Zig inserts the argument slice unchanged). - env.insert("GHOSTTY_BASH_RCFILE", EncodedOs::from_encoded(v).to_os()); + let value = EncodedOs::from_encoded(v); + #[cfg(windows)] + let value = value.try_to_utf8_os()?; + #[cfg(not(windows))] + let value = value.to_os(); + env.insert("GHOSTTY_BASH_RCFILE", value); } // In POSIX mode, HISTFILE defaults to ~/.sh_history, so unless we're @@ -281,7 +300,7 @@ fn setup_bash( } // Return a copy of our modified command line to use as the shell command. - Ok(Some(Command::Shell(cmd.written().to_owned()))) + Ok(Some(Command::Shell(EncodedOs::from_encoded(cmd.written())))) } /// Setup automatic shell integration for shells that include @@ -293,7 +312,9 @@ fn setup_bash( /// from `XDG_DATA_DIRS` when integration is complete. fn setup_xdg_data_dirs(resource_dir: &EncodedOs, env: &mut EnvMap) -> bool { // Get our path to the shell integration directory. - let integ_path = resource_path(resource_dir, "shell-integration"); + let Some(integ_path) = resource_path(resource_dir, "shell-integration") else { + return false; + }; if let Err(err) = std::fs::read_dir(&integ_path) { log::warn!("unable to open {}: {err}", integ_path.to_string_lossy()); return false; @@ -336,7 +357,7 @@ fn setup_nushell(command: &Command, resource_dir: &EncodedOs, env: &mut EnvMap) let mut iter = command.arg_iterator(); // Start accumulating arguments with the executable and initial flags. - cmd.append_arg(&iter.next()?); + cmd.append_arg(iter.next()?.as_encoded()); // Tell nu to immediately "use" all of the exported functions in our // 'ghostty' module. @@ -355,28 +376,29 @@ fn setup_nushell(command: &Command, resource_dir: &EncodedOs, env: &mut EnvMap) // -c / --command -c is always non-interactive // --lsp --lsp starts the language server while let Some(arg) = iter.next() { + let arg = arg.as_encoded(); if arg == "--command" || arg == "--lsp" { return None; } else if arg.len() > 1 && arg.starts_with('-') && !arg[1..].starts_with('-') { if arg.contains('c') { return None; } - cmd.append_arg(&arg); + cmd.append_arg(arg); } else if arg == "-" || arg == "--" { // All remaining arguments should be passed directly to the shell // command. We shouldn't perform any further option processing. - cmd.append_arg(&arg); + cmd.append_arg(arg); for remaining in iter.by_ref() { - cmd.append_arg(&remaining); + cmd.append_arg(remaining.as_encoded()); } break; } else { - cmd.append_arg(&arg); + cmd.append_arg(arg); } } // Return a copy of our modified command line to use as the shell command. - Some(Command::Shell(cmd.written().to_owned())) + Some(Command::Shell(EncodedOs::from_encoded(cmd.written()))) } /// Setup the zsh automatic shell integration. This works by setting @@ -389,7 +411,7 @@ fn setup_zsh(command: &Command, resource_dir: &EncodedOs, env: &mut EnvMap) -> O } // Set our new ZDOTDIR to point to our shell resource directory. - let integ_path = resource_path(resource_dir, "shell-integration/zsh"); + let integ_path = resource_path(resource_dir, "shell-integration/zsh")?; if !Path::new(&integ_path).is_dir() { // Mirrors openDirAbsolute failing (missing or not a directory). log::warn!("unable to open {}", integ_path.to_string_lossy()); @@ -449,12 +471,12 @@ mod tests { } fn sh(s: &str) -> Command { - Command::Shell(s.to_owned()) + Command::Shell(s.into()) } fn shell_str(c: Option<&Command>) -> &str { match c.unwrap() { - Command::Shell(s) => s, + Command::Shell(s) => s.as_encoded(), Command::Direct(_) => panic!("expected shell"), } } @@ -682,15 +704,35 @@ mod tests { for raw in [&b"/home/x/\xffrc"[..], "/home/x/\u{fdd0}rc".as_bytes()] { let mut env = EnvMap::new(); let command = Command::Direct(vec![ - "bash".to_owned(), - "--rcfile".to_owned(), - EncodedOs::from_bytes(raw).into_encoded(), + "bash".into(), + "--rcfile".into(), + EncodedOs::from_bytes(raw), ]); setup_bash(&command, &res.enc(), &mut env).unwrap(); assert_eq!(env.get("GHOSTTY_BASH_RCFILE").unwrap().as_bytes(), raw); } } + #[cfg(windows)] + #[test] + fn windows_bash_rcfile_rejects_invalid_utf8_before_env_conversion() { + let res = TmpResourcesDir::new(Shell::Bash); + for raw in [b"bad\xff".as_slice(), b"bad\xed\xa0\x80"] { + let command = Command::Direct(vec![ + "bash".into(), + "--rcfile".into(), + EncodedOs::from_bytes(raw), + ]); + let mut env = EnvMap::new(); + assert!(matches!( + setup_bash(&command, &res.enc(), &mut env), + Err(Error::InvalidUtf8(_)) + )); + assert!(!env.contains_key("GHOSTTY_BASH_RCFILE")); + } + assert!(resource_path(&EncodedOs::from_bytes(b"bad\xff"), "shell-integration").is_none()); + } + #[test] fn bash_histfile() { let res = TmpResourcesDir::new(Shell::Bash); diff --git a/crates/ghostty-termio/src/termio.rs b/crates/ghostty-termio/src/termio.rs index 34a48dc..9020410 100644 --- a/crates/ghostty-termio/src/termio.rs +++ b/crates/ghostty-termio/src/termio.rs @@ -143,7 +143,7 @@ impl DerivedConfig { background: config.background, osc_color_report_format: config.osc_color_report_format, clipboard_write: config.clipboard_write, - enquiry_response: ghostty_os::osbytes::config_str_to_bytes(&config.enquiry_response), + enquiry_response: config.enquiry_response.to_bytes(), conditional_state: config.conditional_state, } } @@ -174,13 +174,19 @@ impl ThreadEnterState { let mut out = Vec::with_capacity(self.input.len()); for item in &self.input { match item { - ReadableIO::Raw(v) => out.push(ghostty_os::osbytes::config_str_to_bytes(v)), + ReadableIO::Raw(v) => out.push(v.to_bytes()), ReadableIO::Path(path) => { // The path is an encoded OS string: decode it to open. - let f = std::fs::File::open(path.to_path()).map_err(|err| { - log::warn!("failed to open input file={} err={err}", path.as_encoded()); - Error::InputNotFound - })?; + let f = path + .to_path() + .and_then(std::fs::File::open) + .map_err(|err| { + log::warn!( + "failed to open input file={} err={err}", + path.to_text_lossy() + ); + Error::InputNotFound + })?; let mut buf = Vec::new(); // 10 MiB max let limit = 10 * 1024 * 1024; @@ -803,8 +809,7 @@ mod derived_config_tests { let mut cfg = Config { enquiry_response: ghostty_os::osbytes::EncodedOs::from_bytes( "a\u{fdd0}\u{ff}\u{e9}".as_bytes(), - ) - .into_encoded(), + ), ..Config::default() }; assert_eq!( @@ -812,7 +817,7 @@ mod derived_config_tests { "a\u{fdd0}\u{ff}\u{e9}".as_bytes() ); let raw = b"x\xffy"; - cfg.enquiry_response = ghostty_os::osbytes::EncodedOs::from_bytes(raw).into_encoded(); + cfg.enquiry_response = ghostty_os::osbytes::EncodedOs::from_bytes(raw); assert_eq!(DerivedConfig::new(&cfg).enquiry_response, raw); } } diff --git a/crates/ghostty-termio/src/windows_args.rs b/crates/ghostty-termio/src/windows_args.rs index f715282..7b88d84 100644 --- a/crates/ghostty-termio/src/windows_args.rs +++ b/crates/ghostty-termio/src/windows_args.rs @@ -15,21 +15,11 @@ pub enum EncodeError { InvalidUtf8, } -/// Convert an OS string to UTF-16 code units. On Windows this is lossless -/// (`encode_wide`); elsewhere the string must be valid UTF-8. +/// Zig's Windows Command validates UTF-8, including inherited environment strings. +/// An unpaired UTF-16 surrogate in an OS string is therefore an error. pub fn os_str_to_utf16(s: &OsStr) -> Result, EncodeError> { - #[cfg(windows)] - { - use std::os::windows::ffi::OsStrExt; - Ok(s.encode_wide().collect()) - } - #[cfg(not(windows))] - { - Ok(s.to_str() - .ok_or(EncodeError::InvalidUtf8)? - .encode_utf16() - .collect()) - } + ghostty_os::osbytes::try_utf8_to_wide(&ghostty_os::osbytes::os_bytes(s)) + .map_err(|_| EncodeError::InvalidUtf8) } /// Create a UTF-16 environment block for `CreateProcessW` with @@ -157,6 +147,19 @@ mod tests { windows_create_command_line(args) } + #[test] + fn inherited_environment_rejects_non_utf8() { + let bad = ghostty_os::osbytes::EncodedOs::from_wide(&[0xd800]).to_os(); + assert_eq!( + create_windows_env_block([(std::ffi::OsStr::new("KEY"), bad.as_os_str())]), + Err(EncodeError::InvalidUtf8) + ); + assert_eq!( + create_windows_env_block([(bad.as_os_str(), std::ffi::OsStr::new("value"))]), + Err(EncodeError::InvalidUtf8) + ); + } + #[test] fn wide_command_line_matches_str_version_and_keeps_lone_surrogates() { let w = |s: &str| s.encode_utf16().collect::>(); diff --git a/docs/rewrite/notes/cleanup-V2.md b/docs/rewrite/notes/cleanup-V2.md index 450d526..b6379cb 100644 --- a/docs/rewrite/notes/cleanup-V2.md +++ b/docs/rewrite/notes/cleanup-V2.md @@ -3,7 +3,7 @@ ## Resources (self-contained) - `resources/` mirrors Ghostty's `share/ghostty` plus its sibling `share/terminfo`: `shell-integration/`, `themes/` (463 files copied from the installed Ghostty 1.3.1, - `themes/LICENSE-iTerm2-Color-Schemes` = MIT attribution, written from the upstream license; + `LICENSE-iTerm2-Color-Schemes` = MIT attribution, written from the upstream license; no upstream copy existed on this machine, check the copyright line if it matters) and `terminfo/` (compiled `xterm-ghostty`, `g/ghostty`). - `scripts/gen-terminfo.sh` (`just terminfo`): `cargo run -p ghostty-termio --example terminfo` diff --git a/docs/rewrite/notes/final-bytes-refactor.md b/docs/rewrite/notes/final-bytes-refactor.md new file mode 100644 index 0000000..675c74d --- /dev/null +++ b/docs/rewrite/notes/final-bytes-refactor.md @@ -0,0 +1,147 @@ +# Raw-byte config parsing refactor + +Scope: Ghostty 1.3.1, the nine remaining groups in `review-rv8-codex.md`, and +the config/CLI-to-consumer encoding sweep. No version change or commit. + +## Design + +`ArgIter` yields original byte vectors. `LineIterator` trims and splits bytes; +it never escape-encodes values. Process arguments use `collect_bytes`, including +arguments after `-e`. Replay and conditional replay retain byte vectors. + +`CliTarget::parse_into_field`, generated `parse_field_bytes`, and +`ConfigField::parse_bytes` take `Option<&[u8]>`. Each field chooses its contract: + +- Text/numeric/enum grammars validate UTF-8 at the field boundary and return + `InvalidValue` for invalid input. Selection delimiters decode codepoints from + original bytes and validate each unescaped UTF-8 sequence. Clipboard string + replacements are validated UTF-8, stored as ordinary Rust strings. +- Byte strings, paths, environment entries, commands, font family/style names, + binding payloads, key-table names, themes, readable input, and command-palette + labels retain their bytes in `EncodedOs`. Font variation identifiers retain + their four original bytes. +- Tagged unions, comma splitting, and automatic structs split ASCII syntax + without interpreting payload bytes. Quoted automatic-struct values use the + raw Zig string-literal decoder. `config/string.zig` parsing independently + copies unescaped bytes and UTF-8-encodes escaped codepoints, as upstream does. +- `parse_cli(&str)` and `parse_field(..., &str)` remain convenience APIs for + **plain UTF-8**. They forward `str::as_bytes`; they never decode OS escapes. + +`EncodedOs` remains a distinct storage/OS-plumbing type. `from_bytes` and +`from_plain_text` are input constructors; `from_encoded` is reserved for +already encoded internal data. Runtime font, title, key-table, and command +consumers now carry the typed values to their final boundary. GTK display +conversion is explicit and separate from C callbacks and byte output. + +The existing `fmt::Write` config formatter uses an internal encoded string. +Plain text is encoded exactly once when inserted; `+show-config` decodes once +to bytes at the writer. This preserves normal output and arbitrary byte-valued +fields. Action strings retain Zig's `std.zig.stringEscape` formatting, now +applied to the original bytes. Upstream escaping/reset/append semantics are +preserved; this does not promise that every formatted action or repeatable list +is an identity operation when loaded over existing defaults. + +## RV8 finding → fix → regression coverage + +| # | Fix | Tests | +|---|---|---| +| 1 | `EncodedOs::to_path` is fallible. Windows paths validate WTF-8 and report `BadPathName`; config files, themes, renderer assets and other path consumers propagate/handle the error. | `osbytes::tests::windows_paths_fail_with_bad_path_name_instead_of_replacement` | +| 2 | Windows application, cwd, argv, configured environment, and inherited environment use strict UTF-8 conversion before UTF-16/process creation. Surrogate encodings fail. | `windows_command_inputs_require_strict_utf8`; `windows_args::tests::inherited_environment_rejects_non_utf8`; Windows `env_overrides_keep_literal_fdd0_and_reject_surrogates` | +| 3 | WTF-8 validation accepts separately encoded surrogate pairs without requiring canonical re-encoding equality. The Kitty path conversion uses this corrected decoder. | `wtf8_accepts_separately_encoded_surrogate_pairs_like_zig` | +| 4 | Binding triggers parse original UTF-8, so literal U+FDD0 is one codepoint. Only action payloads and table names become typed byte storage. | `raw_bytes::literal_fdd0_is_a_single_unicode_trigger`, through both file and CLI readers | +| 5 | Selection delimiter codepoint iteration rejects invalid original UTF-8 with `InvalidValue`. | `raw_bytes::selection_word_chars_reject_invalid_utf8` | +| 6 | Clipboard replacements validate UTF-8 before storage; the consumer no longer decodes an escape representation or substitutes U+FFFD. | `raw_bytes::clipboard_replacements_require_utf8_and_keep_literal_fdd0`; clipboard plain-text consumer test | +| 7 | Font styles and codepoint-map families remain typed bytes through descriptors. Fontconfig receives original bytes; CoreText validates UTF-8 when constructing its CF strings. | `raw_bytes::font_style_and_codepoint_family_keep_original_bytes`; `discovery::tests::configured_font_names_reach_fontconfig_as_original_bytes` | +| 8 | Configured, dynamic and binding-action titles remain bytes through runtime actions and C string construction. | `c_title_actions_and_key_table_names_forward_original_bytes`; embedded integration `configured_and_binding_titles_reach_c_as_original_bytes` | +| 9 | New-window preserves valid original argument strings; incoming D-Bus text is plain UTF-8. Invalid UTF-8/interior-NUL arguments abort as Zig/GIO does, without untrusted-GVariant empty-string substitution. Flatpak environment construction reproduces GLib's whole-value `[Invalid UTF-8]` substitution. | `arguments_are_plain_dbus_strings_not_escape_encoded`; subprocess `invalid_dbus_strings_fail_without_substitution`; `glib_env_replaces_the_whole_invalid_value` | + +## Additional sweep + +Reviewed uses of `from_encoded`, `as_encoded`, `config_str_to_bytes`, +`config_str_to_text`, lossy UTF-8 conversion, config field implementations, +formatters, and downstream consumers across the workspace. + +Additional fixes from this sweep: + +- Raw command parsing, environment maps, initial input, and auto-struct quoted + payloads no longer see encoding artifacts. +- Key-table names, command-palette titles/descriptions and C name lengths + retain original bytes; command-palette sorting compares original bytes. +- Font feature tags pass original bytes to their backends. CoreText's ASCII + conversion is checked. Font variation tags accept any four bytes, and their + formatter no longer uses lossy UTF-8. +- Selection, clipboard, trigger and composed keybind formatting distinguish + plain text from internal encoded output, preventing double escaping. +- GTK diagnostics decode the formatted bytes before insertion and mirror + GTK's rejection of invalid UTF-8. The working-directory GObject property + renders plain text from typed byte storage, including in tab tooltips and + window subtitles. Theme preview display/search and theme-name sorting use + decoded text/original bytes. Logs render decoded text; Windows edit-config + output writes the original path bytes. +- Link regex strings are plain text and are no longer mistakenly decoded as + OS escapes. Clipboard replacement consumers likewise use validated text. +- C binding-action input and embedded initial input enter parsing as raw + bytes. Text/CSI/ESC output retains original payload bytes and byte lengths. +- Theme lookup carries typed paths without encode/decode detours or an empty + path fallback. Windows filesystem and process conversion contracts are + deliberately separate. +- Windows home expansion returns the original byte value, as Zig does. PATH + lookup and shell-integration resource paths use checked filesystem conversion. + Bash's extracted rcfile argument validates UTF-8 before entering a Windows + environment string. Windows-specific regressions cover these three paths. +- Byte-based trimming matches each Zig parser: space/tab for config syntax; + all Zig ASCII whitespace, including vertical tab, for working directories, + environment entries, and new-window options. + +Remaining encoding uses are storage/output, diagnostics, existing command +execution plumbing, and explicitly tagged GTK clipboard transport. +They do not feed escape-encoded values to config field parsers. Lossy conversion +is confined to display APIs or legacy OS helpers; Windows path and process +operations use checked conversions. + +A direct GLib/GIO C probe compared trusted and untrusted serialized strings: +untrusted invalid UTF-8 became an empty D-Bus string; Zig's trusted construction +aborted in `g_dbus_message_to_blob`. The Rust IPC path explicitly aborts for +these malformed strings before creating a variant, preserving that failure +without violating the Rust GLib binding's trusted-data safety contract. + +Additional regressions cover raw byte fields/auto structs, whitespace, +four-byte variation tags, `+show-config` output/reload for both byte and text +fields, and action escaping from original bytes. Existing byte-path, shell, +clipboard, key-text and preedit tests remain in the workspace suite. + +## Validation + +All required gates passed on the completed refactor: + +| Gate | Result | +|---|---| +| `timeout 2400 cargo test --workspace --no-fail-fast` | Exit 0; 3,428 passed, 0 failed, 7 ignored across reported test summaries | +| `cargo clippy --workspace --all-targets -- -D warnings` | Exit 0 | +| `cargo +nightly fmt --all` | Exit 0 | +| `just check-macos` | Exit 0 | +| `just check-macos x86_64-apple-darwin` | Exit 0 | +| `just check-windows` | Exit 0 | +| `just check-wasm` | Exit 0, including Node execution | +| `cargo build --bin ghostty` | Exit 0 | +| `git diff --check` | Exit 0 | + +The exact required comparison, +`diff <(target/debug/ghostty +show-config --default --docs) <(ghostty +show-config --default --docs)`, +reports only line 3837: `gtk-opengl-debug = true` in Rust versus `false` in the +installed Ghostty binary. Its exit code is 1 for this single expected difference. + +Eight differential `+show-config` probes against `/usr/bin/ghostty` match byte +for byte, each exiting 0: ordinary config; raw byte strings/font names; +literal U+FDD0 delimiters/replacements/triggers; rejected invalid UTF-8; +raw title/text/search actions; quoted automatic structs; environment ASCII +whitespace; and four-byte font variation tags. These probes explicitly set +`auto-update-channel = stable`: without this setting the pre-existing loaded +config default differs (`stable` in the port versus `tip` in the installed +reference). The required `--default --docs` comparison is unnormalized. + +The macOS and Windows checks compile/lint platform-specific paths; they are +not native hardware execution. The WASM check also executes its Node host +smoke test. Windows conversion regressions execute as platform-independent +unit tests on Linux, with additional Windows-only tests cross-compiled. No new Zig tests were skipped +for these fixes; this refactor adapts existing ports and adds regressions. diff --git a/resources/themes/LICENSE-iTerm2-Color-Schemes b/resources/LICENSE-iTerm2-Color-Schemes similarity index 100% rename from resources/themes/LICENSE-iTerm2-Color-Schemes rename to resources/LICENSE-iTerm2-Color-Schemes