Skip to content

Commit

Permalink
format: parse as many characters as possible in numbers
Browse files Browse the repository at this point in the history
Also, make the error messages more compliant with GNU coreutils.
  • Loading branch information
samueltardieu committed Jan 5, 2024
1 parent 5219032 commit 54b87f4
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 29 deletions.
123 changes: 94 additions & 29 deletions src/uucore/src/lib/features/format/argument.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,13 @@
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.

use crate::{
error::set_exit_code,
quoting_style::{escape_name, Quotes, QuotingStyle},
show_error,
};
use os_display::Quotable;

use crate::{error::set_exit_code, show_warning};
use std::{ffi::OsStr, ops::ControlFlow, str::FromStr};

/// An argument for formatting
///
Expand Down Expand Up @@ -40,9 +44,7 @@ impl<'a, T: Iterator<Item = &'a FormatArgument>> ArgumentIter<'a> for T {
};
match next {
FormatArgument::Char(c) => *c,
FormatArgument::Unparsed(s) => {
s.chars().next().unwrap_or('\0')
}
FormatArgument::Unparsed(s) => s.chars().next().unwrap_or('\0'),
_ => '\0',
}
}
Expand All @@ -63,14 +65,7 @@ impl<'a, T: Iterator<Item = &'a FormatArgument>> ArgumentIter<'a> for T {
} else {
s.parse().ok()
};
match opt {
Some(n) => n,
None => {
show_warning!("{}: expected a numeric value", s.quote());
set_exit_code(1);
0
}
}
num_or_partial(opt, s)
}
_ => 0,
}
Expand All @@ -96,14 +91,7 @@ impl<'a, T: Iterator<Item = &'a FormatArgument>> ArgumentIter<'a> for T {
} else {
s.parse().ok()
};
match opt {
Some(n) => n,
None => {
show_warning!("{}: expected a numeric value", s.quote());
set_exit_code(1);
0
}
}
num_or_partial(opt, s)
}
_ => 0,
}
Expand All @@ -123,14 +111,7 @@ impl<'a, T: Iterator<Item = &'a FormatArgument>> ArgumentIter<'a> for T {
} else {
s.parse().ok()
};
match opt {
Some(n) => n,
None => {
show_warning!("{}: expected a numeric value", s.quote());
set_exit_code(1);
0.0
}
}
num_or_partial(opt, s)
}
_ => 0.0,
}
Expand All @@ -143,3 +124,87 @@ impl<'a, T: Iterator<Item = &'a FormatArgument>> ArgumentIter<'a> for T {
}
}
}

/// Return the previously `parsed` argument, or in the case of `None` try to do partial
/// prefix parsing and return the parsed value after showing an error.
fn num_or_partial<T: FromStr + Default>(parsed: Option<T>, raw_arg: &str) -> T {
parsed.unwrap_or_else(|| {
set_exit_code(1);
let escaped = escape_name(
OsStr::new(raw_arg),
&QuotingStyle::C {
quotes: Quotes::None,
},
);
match parse_partial(raw_arg) {
Some(v) => {
show_error!("{}: value not completely converted", escaped.quote());
v
}
None => {
show_error!("{}: expected a numeric value", escaped.quote());
Default::default()
}
}
})
}

/// Return the longest prefix that can be parsed, stopping as soon as this no longer the case.
fn parse_partial<T: FromStr + Default>(raw_arg: &str) -> Option<T> {
let longest_parsed_prefix = raw_arg.char_indices().try_fold(None, |so_far, (i, _)| {
match (so_far, raw_arg[..i].parse()) {
(_, Ok(parsed)) => ControlFlow::Continue(Some(parsed)),
(Some(so_far), Err(_)) => ControlFlow::Break(so_far),
(None, Err(_)) => ControlFlow::Continue(None),
}
});
match longest_parsed_prefix {
ControlFlow::Continue(Some(parsed)) | ControlFlow::Break(parsed) => Some(parsed),
ControlFlow::Continue(None) => None,
}
}

#[cfg(test)]
mod tests {
use super::parse_partial;

#[test]
fn parse_partial_u64() {
assert_eq!(Some(123_u64), parse_partial("123nothing"));
}

#[test]
fn parse_partial_i64() {
assert_eq!(Some(-123_i64), parse_partial("-123nothing"));
}

#[test]
fn parse_partial_u64_no_negative() {
assert_eq!(None, parse_partial::<u64>("-123nothing"));
}

#[test]
fn parse_partial_no_f64() {
assert_eq!(None, parse_partial::<f64>("none"));
}

#[test]
fn parse_partial_f64() {
assert_eq!(Some(42.1_f64), parse_partial("42.1x3"));
}

#[test]
fn parse_partial_f64_inf() {
assert_eq!(Some(f64::INFINITY), parse_partial("inferno"));
}

#[test]
fn parse_partial_f64_minus_inf() {
assert_eq!(Some(f64::NEG_INFINITY), parse_partial("-inferior"));
}

#[test]
fn parse_partial_nan() {
assert!(parse_partial::<f64>("nanotechnology").unwrap().is_nan());
}
}
20 changes: 20 additions & 0 deletions tests/by-util/test_printf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -599,3 +599,23 @@ fn sub_general_round_float_leading_zeroes() {
.succeeds()
.stdout_only("1.00001");
}

#[test]
fn partial_float() {
new_ucmd!()
.args(&["%.2f is %s", "42.03x", "a lot"])
.fails()
.code_is(1)
.stdout_is("42.03 is a lot")
.stderr_is("printf: '42.03x': value not completely converted\n");
}

#[test]
fn partial_integer() {
new_ucmd!()
.args(&["%d is %s", "42x23", "a lot"])
.fails()
.code_is(1)
.stdout_is("42 is a lot")
.stderr_is("printf: '42x23': value not completely converted\n");
}

0 comments on commit 54b87f4

Please sign in to comment.