From 974ad882fadbe72959f04a31ebf485b4d13e003a Mon Sep 17 00:00:00 2001 From: Peter Ammon Date: Thu, 19 Sep 2024 15:34:40 -0700 Subject: [PATCH] Clean up fish-printf in preparation for publishing Make fish-printf no longer depend on the widestring crate, as other clients won't use it; instead this is an optional feature. Make format strings a generic type, so that both narrow and wide strings can serve. This removes a lot of the complexity around converting from narrow to wide. Add a README.md to this crate. --- Cargo.lock | 2 +- Cargo.toml | 6 +- printf/Cargo.toml | 4 +- printf/README.md | 43 ++++++ printf/src/arg.rs | 14 +- printf/src/lib.rs | 79 ++++++----- printf/src/printf_impl.rs | 263 +++++++++++++++++++++-------------- printf/src/tests.rs | 116 +++++++++++---- src/builtins/jobs.rs | 3 +- src/builtins/printf.rs | 2 +- src/builtins/string/match.rs | 1 - src/common.rs | 41 ------ src/complete.rs | 2 +- src/lib.rs | 4 +- src/parser.rs | 1 - src/proc.rs | 3 +- src/wchar.rs | 2 +- src/wutil/gettext.rs | 4 +- src/wutil/mod.rs | 4 +- src/wutil/printf.rs | 52 ++++++- 20 files changed, 419 insertions(+), 227 deletions(-) create mode 100644 printf/README.md diff --git a/Cargo.lock b/Cargo.lock index 5ffe3aac0..a8ef97b40 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -104,7 +104,7 @@ dependencies = [ [[package]] name = "fish-printf" -version = "0.1.0" +version = "0.2.0" dependencies = [ "libc", "widestring", diff --git a/Cargo.toml b/Cargo.toml index bcdcbd417..5f31c3d31 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,14 +44,16 @@ nix = { version = "0.29.0", default-features = false, features = [ ] } num-traits = "0.2.19" once_cell = "1.19.0" -fish-printf = { path = "./printf" } +fish-printf = { path = "./printf", features = ["widestring"] } rand = { version = "0.8.5", features = ["small_rng"] } widestring = "1.1.0" # We need 0.9.0 specifically for some crash fixes. terminfo = "0.9.0" [target.'cfg(not(target_has_atomic = "64"))'.dependencies] -portable-atomic = { version = "1", default-features = false, features = ["fallback"] } +portable-atomic = { version = "1", default-features = false, features = [ + "fallback", +] } [dev-dependencies] rand_pcg = "0.3.1" diff --git a/printf/Cargo.toml b/printf/Cargo.toml index e2476dfbe..cd77d585e 100644 --- a/printf/Cargo.toml +++ b/printf/Cargo.toml @@ -1,10 +1,10 @@ [package] name = "fish-printf" edition = "2021" -version = "0.1.0" +version = "0.2.0" description = "printf implementation, based on musl" license = "MIT" [dependencies] libc = "0.2.155" -widestring = "1.0.2" +widestring = { version = "1.0.2", optional = true } diff --git a/printf/README.md b/printf/README.md new file mode 100644 index 000000000..4ff662905 --- /dev/null +++ b/printf/README.md @@ -0,0 +1,43 @@ +# fish-printf + +The printf implementation used in [fish-shell](https://fishshell.com), based on musl printf. + +[![crates.io](https://img.shields.io/crates/v/fish-printf.svg)](https://crates.io/crates/fish-printf) + +Licensed under the MIT license. + +### Usage + +Run `cargo add fish-printf` to add this crate to your `Cargo.toml` file. + +Also run `cargo add widestring` to add the widestring crate. + +### Notes + +fish-printf attempts to match the C standard for printf. It supports the following features: + +- Locale-specific formatting (decimal point, thousands separator, etc.) +- Honors the current rounding mode. +- Supports the `%n` modifier for counting characters written. + +fish-printf does not support positional arguments, such as `printf("%2$d", 1, 2)`. + +Prefixes like `l` or `ll` are recognized, but only used for validating the format string. +The size of integer values is taken from the argument type. + +fish-printf can output to an `std::fmt::Write` object, or return a string. + +For reasons related to fish-shell, fish-printf has a feature "widestring" which uses the [widestring](https://crates.io/crates/widestring) crate. This is off by default. + +### Examples + +```rust +use fish_printf::sprintf; + +// Create a `String` from a format string. +let s = sprintf!("%0.5g", 123456.0) // 1.2346e+05 + +// Append to an existing string. +let s = String::new(); +sprintf!(=> &mut s, "%0.5g", 123456.0) // 1.2346e+05 +``` diff --git a/printf/src/arg.rs b/printf/src/arg.rs index dcce21521..2abc98158 100644 --- a/printf/src/arg.rs +++ b/printf/src/arg.rs @@ -1,15 +1,18 @@ use super::printf_impl::Error; use std::result::Result; +#[cfg(feature = "widestring")] use widestring::{Utf32Str as wstr, Utf32String as WString}; /// Printf argument types. -/// Note no implementation of ToArg constructs the owned variants (String and WString); +/// Note no implementation of `ToArg` constructs the owned variants (String and WString); /// callers can do so explicitly. #[derive(Debug, PartialEq)] pub enum Arg<'a> { Str(&'a str), + #[cfg(feature = "widestring")] WStr(&'a wstr), String(String), + #[cfg(feature = "widestring")] WString(WString), UInt(u64), SInt(i64, u8), // signed integers track their width as the number of bits @@ -27,6 +30,8 @@ impl<'a> Arg<'a> { } // Convert this to a narrow string, using the provided storage if necessary. + // In practice 'storage' is only used if the widestring feature is enabled. + #[allow(unused_variables, clippy::ptr_arg)] pub fn as_str<'s>(&'s self, storage: &'s mut String) -> Result<&'s str, Error> where 'a: 's, @@ -34,11 +39,13 @@ impl<'a> Arg<'a> { match self { Arg::Str(s) => Ok(s), Arg::String(s) => Ok(s), + #[cfg(feature = "widestring")] Arg::WStr(s) => { storage.clear(); storage.extend(s.chars()); Ok(storage) } + #[cfg(feature = "widestring")] Arg::WString(s) => { storage.clear(); storage.extend(s.chars()); @@ -118,12 +125,14 @@ impl<'a> ToArg<'a> for &'a String { } } +#[cfg(feature = "widestring")] impl<'a> ToArg<'a> for &'a wstr { fn to_arg(self) -> Arg<'a> { Arg::WStr(self) } } +#[cfg(feature = "widestring")] impl<'a> ToArg<'a> for &'a WString { fn to_arg(self) -> Arg<'a> { Arg::WStr(self) @@ -191,6 +200,7 @@ impl_to_arg_u!(u8, u16, u32, u64, usize); #[cfg(test)] mod tests { use super::*; + #[cfg(feature = "widestring")] use widestring::utf32str; #[test] @@ -199,7 +209,9 @@ mod tests { assert!(matches!("test".to_arg(), Arg::Str("test"))); assert!(matches!(String::from("test").to_arg(), Arg::Str(_))); + #[cfg(feature = "widestring")] assert!(matches!(utf32str!("test").to_arg(), Arg::WStr(_))); + #[cfg(feature = "widestring")] assert!(matches!(WString::from("test").to_arg(), Arg::WStr(_))); assert!(matches!(42f32.to_arg(), Arg::Float(_))); assert!(matches!(42f64.to_arg(), Arg::Float(_))); diff --git a/printf/src/lib.rs b/printf/src/lib.rs index 19c26ec8f..6e9b7b581 100644 --- a/printf/src/lib.rs +++ b/printf/src/lib.rs @@ -4,41 +4,50 @@ pub use arg::{Arg, ToArg}; mod fmt_fp; mod printf_impl; -pub use printf_impl::{sprintf_locale, Error}; +pub use printf_impl::{sprintf_locale, Error, FormatString}; pub mod locale; pub use locale::{Locale, C_LOCALE, EN_US_LOCALE}; #[cfg(test)] mod tests; +/// A macro to format a string using `fish_printf` with C-locale formatting rules. +/// +/// # Examples +/// +/// ``` +/// use fish_printf::sprintf; +/// +/// // Create a `String` from a format string. +/// let s = sprintf!("%0.5g", 123456.0); +/// assert_eq!(s, "1.2346e+05"); +/// +/// // Append to an existing string. +/// let mut s = String::new(); +/// sprintf!(=> &mut s, "%0.5g", 123456.0); +/// assert_eq!(s, "1.2346e+05"); +/// ``` #[macro_export] macro_rules! sprintf { - // Variant which allows a string literal and returns a `Utf32String`. - ($fmt:literal, $($arg:expr),* $(,)?) => { - { - let mut target = widestring::Utf32String::new(); - $crate::sprintf!(=> &mut target, widestring::utf32str!($fmt), $($arg),*); - target - } - }; - - // Variant which allows a string literal and writes to a target. - // The target should implement std::fmt::Write. + // Write to a newly allocated String, and return it. + // This panics if the format string or arguments are invalid. ( - => $target:expr, // target string - $fmt:literal, // format string + $fmt:expr, // Format string, which should implement FormatString. $($arg:expr),* // arguments $(,)? // optional trailing comma ) => { { - $crate::sprintf!(=> $target, widestring::utf32str!($fmt), $($arg),*); + let mut target = String::new(); + $crate::sprintf!(=> &mut target, $fmt, $($arg),*); + target } }; - // Variant which allows a `Utf32String` as a format, and writes to a target. + // Variant which writes to a target. + // The target should implement std::fmt::Write. ( => $target:expr, // target string - $fmt:expr, // format string as UTF32String + $fmt:expr, // format string $($arg:expr),* // arguments $(,)? // optional trailing comma ) => { @@ -46,22 +55,13 @@ macro_rules! sprintf { // May be no args! #[allow(unused_imports)] use $crate::ToArg; - $crate::sprintf_c_locale( + $crate::printf_c_locale( $target, - $fmt.as_char_slice(), + $fmt, &mut [$($arg.to_arg()),*], ).unwrap() } }; - - // Variant which allows a `Utf32String` as a format, and returns a `Utf32String`. - ($fmt:expr, $($arg:expr),* $(,)?) => { - { - let mut target = widestring::Utf32String::new(); - $crate::sprintf!(=> &mut target, $fmt, $($arg),*); - target - } - }; } /// Formats a string using the provided format specifiers and arguments, using the C locale, @@ -70,14 +70,29 @@ macro_rules! sprintf { /// # Parameters /// - `f`: The receiver of formatted output. /// - `fmt`: The format string being parsed. -/// - `locale`: The locale to use for number formatting. /// - `args`: Iterator over the arguments to format. /// /// # Returns -/// A `Result` which is `Ok` containing the number of bytes written on success, or an `Error`. -pub fn sprintf_c_locale( +/// A `Result` which is `Ok` containing the number of characters written on success, or an `Error`. +/// +/// # Example +/// +/// ``` +/// use fish_printf::{printf_c_locale, ToArg, FormatString}; +/// use std::fmt::Write; +/// +/// let mut output = String::new(); +/// let fmt: &str = "%0.5g"; // Example format string +/// let mut args = [123456.0.to_arg()]; +/// +/// let result = printf_c_locale(&mut output, fmt, &mut args); +/// +/// assert!(result == Ok(10)); +/// assert_eq!(output, "1.2346e+05"); +/// ``` +pub fn printf_c_locale( f: &mut impl std::fmt::Write, - fmt: &[char], + fmt: impl FormatString, args: &mut [Arg], ) -> Result { sprintf_locale(f, fmt, &locale::C_LOCALE, args) diff --git a/printf/src/printf_impl.rs b/printf/src/printf_impl.rs index b658072d7..8a09137ac 100644 --- a/printf/src/printf_impl.rs +++ b/printf/src/printf_impl.rs @@ -4,9 +4,11 @@ use super::fmt_fp::format_float; use super::locale::Locale; use std::fmt::{self, Write}; use std::mem; -use std::ops::{AddAssign, Index}; use std::result::Result; +#[cfg(feature = "widestring")] +use widestring::Utf32Str as wstr; + /// Possible errors from printf. #[derive(Debug, PartialEq, Eq)] pub enum Error { @@ -151,107 +153,143 @@ impl ConversionSpec { } } -// A helper type that holds a format string slice and points into it. -// As a convenience, this returns '\0' for one-past-the-end. -#[derive(Debug)] -struct FormatString<'a>(&'a [char]); - -impl<'a> FormatString<'a> { - // Return the underlying slice. - fn as_slice(&self) -> &'a [char] { - self.0 - } - +// A helper type with convenience functions for format strings. +pub trait FormatString { // Return true if we are empty. - fn is_empty(&self) -> bool { - self.0.is_empty() - } + fn is_empty(&self) -> bool; - // Read an int from our cursor, stopping at the first non-digit. - // Negative values are not supported. - // If there are no digits, return 0. - // Adjust the cursor to point to the char after the int. - fn get_int(&mut self) -> Result { - use Error::Overflow; - let mut i: usize = 0; - while let Some(digit) = self[0].to_digit(10) { - i = i.checked_mul(10).ok_or(Overflow)?; - i = i.checked_add(digit as usize).ok_or(Overflow)?; - *self += 1; - } - Ok(i) - } + // Return the character at a given index, or None if out of bounds. + // Note the index is a count of characters, not bytes. + fn at(&self, index: usize) -> Option; - // Read a conversion prefix from our cursor, advancing it. - fn get_prefix(&mut self) -> ConversionPrefix { - use ConversionPrefix as CP; - let prefix = match self[0] { - 'h' if self[1] == 'h' => CP::hh, - 'h' => CP::h, - 'l' if self[1] == 'l' => CP::ll, - 'l' => CP::l, - 'j' => CP::j, - 't' => CP::t, - 'z' => CP::z, - 'L' => CP::L, - _ => CP::Empty, - }; - *self += match prefix { - CP::Empty => 0, - CP::hh | CP::ll => 2, - _ => 1, - }; - prefix - } - - // Read an (optionally prefixed) format specifier, such as d, Lf, etc. - // Adjust the cursor to point to the char after the specifier. - fn get_specifier(&mut self) -> Result { - let prefix = self.get_prefix(); - // Awkwardly placed hack to disallow %lC and %lS, since we otherwise treat - // them as the same. - if prefix != ConversionPrefix::Empty && matches!(self[0], 'C' | 'S') { - return Err(Error::BadFormatString); - } - let spec = ConversionSpec::from_char(self[0]).ok_or(Error::BadFormatString)?; - if !spec.supports_prefix(prefix) { - return Err(Error::BadFormatString); - } - *self += 1; - Ok(spec) - } + // Advance by the given number of characters. + fn advance_by(&mut self, n: usize); // Read a sequence of characters to be output literally, advancing the cursor. + // The characters may optionally be stored in the given buffer. // This handles a tail of %%. - fn get_lit(&mut self) -> &'a [char] { - let s = self.0; + fn take_literal<'a: 'b, 'b>(&'a mut self, buffer: &'b mut String) -> &'b str; +} + +impl FormatString for &str { + fn is_empty(&self) -> bool { + (*self).is_empty() + } + + fn at(&self, index: usize) -> Option { + self.chars().nth(index) + } + + fn advance_by(&mut self, n: usize) { + let mut chars = self.chars(); + for _ in 0..n { + let c = chars.next(); + assert!(c.is_some(), "FormatString::advance(): index out of bounds"); + } + *self = chars.as_str(); + } + + fn take_literal<'a: 'b, 'b>(&'a mut self, _buffer: &'b mut String) -> &'b str { + // Count length of non-percent characters. + let non_percents: usize = self + .chars() + .take_while(|&c| c != '%') + .map(|c| c.len_utf8()) + .sum(); + // Take only an even number of percents. Note we know these have byte length 1. + let percent_pairs = self[non_percents..] + .chars() + .take_while(|&c| c == '%') + .count() + / 2; + let (prefix, rest) = self.split_at(non_percents + percent_pairs * 2); + *self = rest; + // Trim half of the trailing percent characters from the prefix. + &prefix[..prefix.len() - percent_pairs] + } +} + +#[cfg(feature = "widestring")] +impl FormatString for &wstr { + fn is_empty(&self) -> bool { + (*self).is_empty() + } + + fn at(&self, index: usize) -> Option { + self.as_char_slice().get(index).copied() + } + + fn advance_by(&mut self, n: usize) { + *self = &self[n..]; + } + + fn take_literal<'a: 'b, 'b>(&'a mut self, buffer: &'b mut String) -> &'b str { + let s = self.as_char_slice(); let non_percents = s.iter().take_while(|&&c| c != '%').count(); // Take only an even number of percents. let percent_pairs: usize = s[non_percents..].iter().take_while(|&&c| c == '%').count() / 2; - *self += non_percents + percent_pairs * 2; - &s[..non_percents + percent_pairs] + *self = &self[non_percents + percent_pairs * 2..]; + buffer.clear(); + buffer.extend(s[..non_percents + percent_pairs].iter()); + buffer.as_str() } } -// Advance this format string by a number of chars. -impl AddAssign for FormatString<'_> { - fn add_assign(&mut self, rhs: usize) { - self.0 = &self.0[rhs..]; +// Read an int from a format string, stopping at the first non-digit. +// Negative values are not supported. +// If there are no digits, return 0. +// Adjust the format string to point to the char after the int. +fn get_int(fmt: &mut impl FormatString) -> Result { + use Error::Overflow; + let mut i: usize = 0; + while let Some(digit) = fmt.at(0).and_then(|c| c.to_digit(10)) { + i = i.checked_mul(10).ok_or(Overflow)?; + i = i.checked_add(digit as usize).ok_or(Overflow)?; + fmt.advance_by(1); } + Ok(i) } -// Index into FormatString, returning \0 for one-past-the-end. -impl Index for FormatString<'_> { - type Output = char; +// Read a conversion prefix from a format string, advancing it. +fn get_prefix(fmt: &mut impl FormatString) -> ConversionPrefix { + use ConversionPrefix as CP; + let prefix = match fmt.at(0).unwrap_or('\0') { + 'h' if fmt.at(1) == Some('h') => CP::hh, + 'h' => CP::h, + 'l' if fmt.at(1) == Some('l') => CP::ll, + 'l' => CP::l, + 'j' => CP::j, + 't' => CP::t, + 'z' => CP::z, + 'L' => CP::L, + _ => CP::Empty, + }; + fmt.advance_by(match prefix { + CP::Empty => 0, + CP::hh | CP::ll => 2, + _ => 1, + }); + prefix +} - fn index(&self, idx: usize) -> &char { - let s = self.as_slice(); - if idx == s.len() { - &'\0' - } else { - &s[idx] - } +// Read an (optionally prefixed) format specifier, such as d, Lf, etc. +// Adjust the cursor to point to the char after the specifier. +fn get_specifier(fmt: &mut impl FormatString) -> Result { + let prefix = get_prefix(fmt); + // Awkwardly placed hack to disallow %lC and %lS, since we otherwise treat + // them as the same. + if prefix != ConversionPrefix::Empty && matches!(fmt.at(0), Some('C' | 'S')) { + return Err(Error::BadFormatString); } + let spec = fmt + .at(0) + .and_then(ConversionSpec::from_char) + .ok_or(Error::BadFormatString)?; + if !spec.supports_prefix(prefix) { + return Err(Error::BadFormatString); + } + fmt.advance_by(1); + Ok(spec) } // Pad output by emitting `c` until `min_width` is reached. @@ -288,14 +326,30 @@ pub(super) fn pad( /// /// # Returns /// A `Result` which is `Ok` containing the number of bytes written on success, or an `Error`. +/// +/// # Example +/// +/// ``` +/// use fish_printf::{sprintf_locale, ToArg, FormatString, locale}; +/// use std::fmt::Write; +/// +/// let mut output = String::new(); +/// let fmt: &str = "%'0.2f"; +/// let mut args = [1234567.89.to_arg()]; +/// +/// let result = sprintf_locale(&mut output, fmt, &locale::EN_US_LOCALE, &mut args); +/// +/// assert!(result == Ok(12)); +/// assert_eq!(output, "1,234,567.89"); +/// ``` pub fn sprintf_locale( f: &mut impl Write, - fmt: &[char], + fmt: impl FormatString, locale: &Locale, args: &mut [Arg], ) -> Result { use ConversionSpec as CS; - let mut s = FormatString(fmt); + let mut s = fmt; let mut args = args.iter_mut(); let mut out_len: usize = 0; @@ -305,31 +359,32 @@ pub fn sprintf_locale( buf.clear(); // Handle literal text and %% format specifiers. - let lit = s.get_lit(); + let lit = s.take_literal(buf); if !lit.is_empty() { - buf.extend(lit.iter()); - f.write_str(buf)?; - out_len = out_len.checked_add(lit.len()).ok_or(Error::Overflow)?; + f.write_str(lit)?; + out_len = out_len + .checked_add(lit.chars().count()) + .ok_or(Error::Overflow)?; continue 'main; } // Consume the % at the start of the format specifier. - debug_assert!(s[0] == '%'); - s += 1; + debug_assert!(s.at(0) == Some('%')); + s.advance_by(1); // Read modifier flags. '-' and '0' flags are mutually exclusive. let mut flags = ModifierFlags::default(); - while flags.try_set(s[0]) { - s += 1; + while flags.try_set(s.at(0).unwrap_or('\0')) { + s.advance_by(1); } if flags.left_adj { flags.zero_pad = false; } // Read field width. We do not support $. - let width = if s[0] == '*' { + let width = if s.at(0) == Some('*') { let arg_width = args.next().ok_or(Error::MissingArg)?.as_sint()?; - s += 1; + s.advance_by(1); if arg_width < 0 { flags.left_adj = true; } @@ -338,19 +393,19 @@ pub fn sprintf_locale( .try_into() .map_err(|_| Error::Overflow)? } else { - s.get_int()? + get_int(&mut s)? }; // Optionally read precision. We do not support $. - let mut prec: Option = if s[0] == '.' && s[1] == '*' { + let mut prec: Option = if s.at(0) == Some('.') && s.at(1) == Some('*') { // "A negative precision is treated as though it were missing." // Here we assume the precision is always signed. - s += 2; + s.advance_by(2); let p = args.next().ok_or(Error::MissingArg)?.as_sint()?; p.try_into().ok() - } else if s[0] == '.' { - s += 1; - Some(s.get_int()?) + } else if s.at(0) == Some('.') { + s.advance_by(1); + Some(get_int(&mut s)?) } else { None }; @@ -360,7 +415,7 @@ pub fn sprintf_locale( } // Read out the format specifier and arg. - let conv_spec = s.get_specifier()?; + let conv_spec = get_specifier(&mut s)?; let arg = args.next().ok_or(Error::MissingArg)?; let mut prefix = ""; diff --git a/printf/src/tests.rs b/printf/src/tests.rs index 7968c2dbe..e87e41b96 100644 --- a/printf/src/tests.rs +++ b/printf/src/tests.rs @@ -1,10 +1,9 @@ use crate::arg::ToArg; use crate::locale::{Locale, C_LOCALE, EN_US_LOCALE}; -use crate::{sprintf_locale, Error}; +use crate::{sprintf_locale, Error, FormatString}; use libc::c_char; use std::f64::consts::{E, PI, TAU}; use std::fmt; -use widestring::{utf32str, Utf32Str}; // sprintf, checking length macro_rules! sprintf_check { @@ -15,11 +14,11 @@ macro_rules! sprintf_check { ) => { { let mut target = String::new(); - let chars: Vec = $fmt.chars().collect(); - let len = $crate::sprintf_c_locale( + let mut args = [$($arg.to_arg()),*]; + let len = $crate::printf_c_locale( &mut target, - &chars, - &mut [$($arg.to_arg()),*] + $fmt.as_ref() as &str, + &mut args, ).expect("printf failed"); assert!(len == target.len(), "Wrong length returned: {} vs {}", len, target.len()); target @@ -43,10 +42,9 @@ macro_rules! assert_fmt1 { macro_rules! sprintf_err { ($fmt:expr, $($arg:expr),* => $expected:expr) => { { - let chars: Vec = $fmt.chars().collect(); - let err = $crate::sprintf_c_locale( + let err = $crate::printf_c_locale( &mut NullOutput, - &chars, + $fmt.as_ref() as &str, &mut [$($arg.to_arg()),*], ).unwrap_err(); assert_eq!(err, $expected, "Wrong error returned: {:?}", err); @@ -58,10 +56,9 @@ macro_rules! sprintf_err { macro_rules! sprintf_count { ($fmt:expr $(, $arg:expr)*) => { { - let chars: Vec = $fmt.chars().collect(); - $crate::sprintf_c_locale( + $crate::printf_c_locale( &mut NullOutput, - &chars, + $fmt, &mut [$($arg.to_arg()),*], ).expect("printf failed") } @@ -84,6 +81,69 @@ fn smoke() { assert_fmt!("" => ""); } +#[test] +fn test_format_string_str() { + let mut s: &str = "hello%world%%%%%"; + assert_eq!(s.is_empty(), false); + for (idx, c) in s.char_indices() { + assert_eq!(s.at(idx), Some(c)); + } + assert_eq!(s.at(s.chars().count()), None); + + let mut buffer = String::new(); + assert_eq!(s.take_literal(&mut buffer), "hello"); + + s.advance_by(1); // skip '%' + assert_eq!(s.at(0), Some('w')); + assert_eq!(s.take_literal(&mut buffer), "world%%"); + + s.advance_by(1); // advancing over one more % + assert_eq!(s.is_empty(), true); // remaining content is empty +} + +#[cfg(feature = "widestring")] +#[test] +fn test_format_string_wstr() { + use widestring::Utf32String; + + let utf32: Utf32String = Utf32String::from_str("hello%world%%%%%"); + let mut s = utf32.as_utfstr(); + + for (idx, c) in s.char_indices() { + assert_eq!(s.at(idx), Some(c)); + } + assert_eq!(s.at(s.chars().count()), None); + + let mut buffer = String::new(); + assert_eq!(s.take_literal(&mut buffer), "hello"); + + s.advance_by(1); // skip '%' + assert_eq!(s.at(0), Some('w')); + assert_eq!(s.take_literal(&mut buffer), "world%%"); + + s.advance_by(1); // advancing over one more % + assert_eq!(s.is_empty(), true); // remaining content is empty +} + +#[test] +fn test_char_counts() { + // printf returns the number of characters, not the number of bytes. + assert_eq!(sprintf_count!("%d", 123), 3); + assert_eq!(sprintf_count!("%d", -123), 4); + assert_eq!(sprintf_count!("\u{1F680}"), 1); +} + +#[cfg(feature = "widestring")] +#[test] +fn test_wide_char_counts() { + use widestring::utf32str; + + // printf returns the number of characters, not the number of bytes. + assert_eq!(sprintf_count!(utf32str!("%d"), 123), 3); + assert_eq!(sprintf_count!(utf32str!("%d"), -123), 4); + assert_eq!(sprintf_count!(utf32str!("\u{1F680}")), 1); +} + #[test] fn test1() { // A convenient place to isolate a single test, e.g. cargo test -- test1 @@ -591,6 +651,16 @@ fn test_prefixes() { assert_eq!(sprintf_check!("%ls", "cs"), "cs"); } +#[test] +fn test_crate_macros() { + let mut target = String::new(); + crate::sprintf!(=> &mut target, "%d ok %d", 1, 2); + assert_eq!(target, "1 ok 2"); + + target = crate::sprintf!("%d ok %d", 3, 4); + assert_eq!(target, "3 ok 4"); +} + #[test] #[cfg_attr( all(target_arch = "x86", not(target_feature = "sse2")), @@ -711,8 +781,7 @@ fn test_errors() { fn test_locale() { fn test_printf_loc<'a>(expected: &str, locale: &Locale, format: &str, arg: impl ToArg<'a>) { let mut target = String::new(); - let format_chars: Vec = format.chars().collect(); - let len = sprintf_locale(&mut target, &format_chars, locale, &mut [arg.to_arg()]) + let len = sprintf_locale(&mut target, format, locale, &mut [arg.to_arg()]) .expect("printf failed"); assert_eq!(len, target.len()); assert_eq!(target, expected); @@ -769,7 +838,7 @@ fn test_float_hex_prec() { v *= sign; for preci in 1..=200_i32 { rust_str.clear(); - crate::sprintf!(=> &mut rust_str, utf32str!("%.*a"), preci, v); + crate::sprintf!(=> &mut rust_str, "%.*a", preci, v); let printf_str = unsafe { let len = libc::snprintf(c_storage_ptr, c_storage.len(), c_fmt, preci, v); @@ -792,7 +861,7 @@ fn test_float_hex_prec() { assert!(!failed); } -fn test_exhaustive(rust_fmt: &Utf32Str, c_fmt: *const c_char) { +fn test_exhaustive(rust_fmt: &str, c_fmt: *const c_char) { // "There's only 4 billion floats so test them all." // This tests a format string expected to be of the form "%.*g" or "%.*e". // That is, it takes a precision and a double. @@ -835,28 +904,19 @@ fn test_exhaustive(rust_fmt: &Utf32Str, c_fmt: *const c_char) { #[ignore] fn test_float_g_exhaustive() { // To run: cargo test test_float_g_exhaustive --release -- --ignored --nocapture - test_exhaustive( - widestring::utf32str!("%.*g"), - b"%.*g\0".as_ptr() as *const c_char, - ); + test_exhaustive("%.*g", b"%.*g\0".as_ptr() as *const c_char); } #[test] #[ignore] fn test_float_e_exhaustive() { // To run: cargo test test_float_e_exhaustive --release -- --ignored --nocapture - test_exhaustive( - widestring::utf32str!("%.*e"), - b"%.*e\0".as_ptr() as *const c_char, - ); + test_exhaustive("%.*e", b"%.*e\0".as_ptr() as *const c_char); } #[test] #[ignore] fn test_float_f_exhaustive() { // To run: cargo test test_float_f_exhaustive --release -- --ignored --nocapture - test_exhaustive( - widestring::utf32str!("%.*f"), - b"%.*f\0".as_ptr() as *const c_char, - ); + test_exhaustive("%.*f", b"%.*f\0".as_ptr() as *const c_char); } diff --git a/src/builtins/jobs.rs b/src/builtins/jobs.rs index 5b633c401..04835939b 100644 --- a/src/builtins/jobs.rs +++ b/src/builtins/jobs.rs @@ -15,9 +15,8 @@ use crate::wutil::wgettext; use crate::{ builtins::shared::STATUS_CMD_OK, wchar::{wstr, WString, L}, - wutil::{fish_wcstoi, wgettext_fmt}, + wutil::{fish_wcstoi, sprintf, wgettext_fmt}, }; -use fish_printf::sprintf; use libc::c_int; use std::num::NonZeroU32; use std::sync::atomic::Ordering; diff --git a/src/builtins/printf.rs b/src/builtins/printf.rs index a67e418fd..01631ab0c 100644 --- a/src/builtins/printf.rs +++ b/src/builtins/printf.rs @@ -265,7 +265,7 @@ impl<'a, 'b> builtin_printf_state_t<'a, 'b> { if !self.early_exit { sprintf_locale( &mut self.buff, - $fmt.as_char_slice(), + $fmt, &self.locale, &mut [$($arg.to_arg()),*] ).expect("sprintf failed"); diff --git a/src/builtins/string/match.rs b/src/builtins/string/match.rs index 037f5f50a..2a2db9597 100644 --- a/src/builtins/string/match.rs +++ b/src/builtins/string/match.rs @@ -1,4 +1,3 @@ -use fish_printf::sprintf; use pcre2::utf32::{Captures, Regex, RegexBuilder}; use std::collections::HashMap; use std::num::NonZeroUsize; diff --git a/src/common.rs b/src/common.rs index 3500bf0e9..b2a7834ba 100644 --- a/src/common.rs +++ b/src/common.rs @@ -2006,44 +2006,3 @@ impl ToCString for &[u8] { CString::new(self).unwrap() } } - -#[allow(unused_macros)] -#[deprecated = "use printf!, eprintf! or fprintf"] -macro_rules! fwprintf { - ($args:tt) => { - panic!() - }; -} - -// test-only -#[allow(unused_macros)] -#[deprecated = "use printf!"] -macro_rules! err { - ($format:expr $(, $args:expr)* $(,)? ) => { - printf!($format $(, $args )*); - } -} - -#[macro_export] -macro_rules! fprintf { - ($fd:expr, $format:expr $(, $arg:expr)* $(,)?) => { - { - let wide = $crate::wutil::sprintf!($format, $( $arg ),*); - $crate::wutil::wwrite_to_fd(&wide, $fd); - } - } -} - -#[macro_export] -macro_rules! printf { - ($format:expr $(, $arg:expr)* $(,)?) => { - fprintf!(libc::STDOUT_FILENO, $format $(, $arg)*) - } -} - -#[macro_export] -macro_rules! eprintf { - ($format:expr $(, $arg:expr)* $(,)?) => { - fprintf!(libc::STDERR_FILENO, $format $(, $arg)*) - } -} diff --git a/src/complete.rs b/src/complete.rs index 99833adbe..f2b2d6e78 100644 --- a/src/complete.rs +++ b/src/complete.rs @@ -13,9 +13,9 @@ use crate::{ common::{charptr2wcstring, escape_string, EscapeFlags, EscapeStringStyle}, reader::{get_quote, is_backslashed}, util::wcsfilecmp, + wutil::sprintf, }; use bitflags::bitflags; -use fish_printf::sprintf; use once_cell::sync::Lazy; use crate::{ diff --git a/src/lib.rs b/src/lib.rs index c3eb7819e..bbf688c32 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,6 +25,9 @@ pub const BUILD_VERSION: &str = env!("FISH_BUILD_VERSION"); #[macro_use] pub mod common; +#[macro_use] +pub mod wutil; + pub mod abbrs; pub mod ast; pub mod autoload; @@ -97,7 +100,6 @@ pub mod wcstringutil; pub mod wgetopt; pub mod widecharwidth; pub mod wildcard; -pub mod wutil; #[cfg(test)] mod tests; diff --git a/src/parser.rs b/src/parser.rs index fbb02805b..ded236947 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -31,7 +31,6 @@ use crate::wait_handle::WaitHandleStore; use crate::wchar::{wstr, WString, L}; use crate::wutil::{perror, wgettext, wgettext_fmt}; use crate::{function, FLOG}; -use fish_printf::sprintf; use libc::c_int; #[cfg(not(target_has_atomic = "64"))] use portable_atomic::AtomicU64; diff --git a/src/proc.rs b/src/proc.rs index e5fd00686..3c11b6382 100644 --- a/src/proc.rs +++ b/src/proc.rs @@ -24,8 +24,7 @@ use crate::topic_monitor::{topic_monitor_principal, GenerationsList, Topic}; use crate::wait_handle::{InternalJobId, WaitHandle, WaitHandleRef, WaitHandleStore}; use crate::wchar::{wstr, WString, L}; use crate::wchar_ext::ToWString; -use crate::wutil::{perror, wbasename, wgettext, wperror}; -use fish_printf::sprintf; +use crate::wutil::{perror, sprintf, wbasename, wgettext, wperror}; use libc::{ EBADF, EINVAL, ENOTTY, EPERM, EXIT_SUCCESS, SIGABRT, SIGBUS, SIGCONT, SIGFPE, SIGHUP, SIGILL, SIGINT, SIGKILL, SIGPIPE, SIGQUIT, SIGSEGV, SIGSYS, SIGTTOU, SIG_DFL, SIG_IGN, STDIN_FILENO, diff --git a/src/wchar.rs b/src/wchar.rs index c40bfe2de..2a2f7f4ac 100644 --- a/src/wchar.rs +++ b/src/wchar.rs @@ -14,7 +14,7 @@ pub mod prelude { pub use crate::{ wchar::{wstr, IntoCharIter, WString, L}, wchar_ext::{ToWString, WExt}, - wutil::{sprintf, wgettext, wgettext_fmt, wgettext_maybe_fmt, wgettext_str}, + wutil::{eprintf, sprintf, wgettext, wgettext_fmt, wgettext_maybe_fmt, wgettext_str}, }; } diff --git a/src/wutil/gettext.rs b/src/wutil/gettext.rs index 19620031f..ab968ed34 100644 --- a/src/wutil/gettext.rs +++ b/src/wutil/gettext.rs @@ -146,7 +146,7 @@ macro_rules! wgettext_fmt { $($args:expr),+ // list of expressions $(,)? // optional trailing comma ) => { - $crate::wutil::sprintf!(&$crate::wutil::wgettext!($string), $($args),+) + $crate::wutil::sprintf!($crate::wutil::wgettext!($string), $($args),+) }; } pub use wgettext_fmt; @@ -160,7 +160,7 @@ macro_rules! wgettext_maybe_fmt { $(, $args:expr)* // list of expressions $(,)? // optional trailing comma ) => { - $crate::wutil::sprintf!(&$crate::wutil::wgettext!($string), $($args),*) + $crate::wutil::sprintf!($crate::wutil::wgettext!($string), $($args),*) }; } pub use wgettext_maybe_fmt; diff --git a/src/wutil/mod.rs b/src/wutil/mod.rs index 8f52ddf24..33bed8e19 100644 --- a/src/wutil/mod.rs +++ b/src/wutil/mod.rs @@ -4,6 +4,7 @@ pub mod errors; pub mod fileid; pub mod gettext; mod hex_float; +#[macro_use] pub mod printf; #[cfg(test)] mod tests; @@ -25,8 +26,7 @@ use std::fs::{self, canonicalize}; use std::io::{self, Write}; use std::os::unix::prelude::*; -extern crate fish_printf; -pub use fish_printf::sprintf; +pub use crate::wutil::printf::{eprintf, fprintf, printf, sprintf}; pub use fileid::{ file_id_for_fd, file_id_for_path, file_id_for_path_narrow, DevInode, FileId, INVALID_FILE_ID, diff --git a/src/wutil/printf.rs b/src/wutil/printf.rs index 066e28b91..7e0adafe7 100644 --- a/src/wutil/printf.rs +++ b/src/wutil/printf.rs @@ -1,5 +1,53 @@ -// Re-export sprintf macro. -pub use fish_printf::sprintf; +// Support for printf-style formatting. +#[macro_export] +macro_rules! sprintf { + // Allow a `&str` or `&Utf32Str` as a format, and return a `Utf32String`. + ($fmt:expr $(, $arg:expr)* $(,)?) => { + { + let mut target = widestring::Utf32String::new(); + $crate::sprintf!(=> &mut target, $fmt, $($arg),*); + target + } + }; + + // Allow a `&str` or `&Utf32Str` as a format, and write to a target, + // which should be a `&mut String` or `&mut Utf32String`. + // + (=> $target:expr, $fmt:expr $(, $arg:expr)* $(,)?) => { + { + let _ = fish_printf::sprintf!(=> $target, $fmt, $($arg),*); + } + }; +} + +#[macro_export] +macro_rules! fprintf { + // Allow a `&str` or `&Utf32Str` as a format, and write to an fd. + ($fd:expr, $fmt:expr $(, $arg:expr)* $(,)?) => { + { + let wide = $crate::wutil::sprintf!($fmt, $( $arg ),*); + $crate::wutil::wwrite_to_fd(&wide, $fd); + } + } +} + +#[macro_export] +macro_rules! printf { + // Allow a `&str` or `&Utf32Str` as a format, and write to stdout. + ($fmt:expr $(, $arg:expr)* $(,)?) => { + $crate::fprintf!(libc::STDOUT_FILENO, $fmt $(, $arg)*) + } +} + +#[macro_export] +macro_rules! eprintf { + // Allow a `&str` or `&Utf32Str` as a format, and write to stderr. + ($fmt:expr $(, $arg:expr)* $(,)?) => { + fprintf!(libc::STDERR_FILENO, $fmt $(, $arg)*) + } +} + +pub use {eprintf, fprintf, printf, sprintf}; #[cfg(test)] mod tests {