From 4c5326ffa34b2026d5437b8b5b118f5f67467f0a Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Fri, 5 Jan 2024 14:40:45 +0100 Subject: [PATCH 1/3] uucore/num_format: the default precision for %g is 6 --- src/uucore/src/lib/features/format/num_format.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/uucore/src/lib/features/format/num_format.rs b/src/uucore/src/lib/features/format/num_format.rs index 51f3336cf..4e60015f6 100644 --- a/src/uucore/src/lib/features/format/num_format.rs +++ b/src/uucore/src/lib/features/format/num_format.rs @@ -286,7 +286,13 @@ impl Formatter for Float { let precision = match precision { Some(CanAsterisk::Fixed(x)) => x, - None => 0, + None => { + if matches!(variant, FloatVariant::Shortest) { + 6 + } else { + 0 + } + } Some(CanAsterisk::Asterisk) => return Err(FormatError::WrongSpecType), }; From f5179290a6d571229c977b6b7eede92b1492f1aa Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Fri, 5 Jan 2024 14:44:09 +0100 Subject: [PATCH 2/3] uucore/num_format: replace saturating_sub by regular subtraction Using `saturating_sub()` before converting to `usize` gives a wrong feeling of security as it looks like it ensures that the value will never go negative. However, since it is applied to `i32`, it can, and converting it to `usize` would go horribly wrong anyway. By following the code flow, `exponent` cannot be greater than `precision`, or the `else` block would not have been taken. A plain subtraction will give the same result and will at least panic in debug mode. --- src/uucore/src/lib/features/format/num_format.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uucore/src/lib/features/format/num_format.rs b/src/uucore/src/lib/features/format/num_format.rs index 4e60015f6..dce39641d 100644 --- a/src/uucore/src/lib/features/format/num_format.rs +++ b/src/uucore/src/lib/features/format/num_format.rs @@ -425,7 +425,7 @@ fn format_float_shortest( // - The precision works differently and specifies the total number // of digits instead of the digits in the fractional part. // - If we don't force the decimal, '0' and `.` are trimmed. - let decimal_places = (precision as i32).saturating_sub(exponent) as usize; + let decimal_places = (precision as i32 - exponent) as usize; let mut formatted = if decimal_places == 0 && force_decimal == ForceDecimal::Yes { format!("{f:.0}.") } else { From 32f0256d7d974ba8b845d0dae3bc82e3437767a9 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Fri, 5 Jan 2024 14:49:09 +0100 Subject: [PATCH 3/3] =?UTF-8?q?uucore/num=5Fformat:=20properly=20display?= =?UTF-8?q?=2010=E1=B5=96=20where=20p=20is=20the=20precision?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `seq --format %.2g 10 10` would display `1` because the precision would not allow room for the decimal point, and the `0` in `10` would be trimmed as an insignificant trailing `0`. This has been fixed by only trimming trailing `0` in the presence of a decimal point. --- .../src/lib/features/format/num_format.rs | 37 ++++++++++++++----- tests/by-util/test_seq.rs | 20 ++++++++++ 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/src/uucore/src/lib/features/format/num_format.rs b/src/uucore/src/lib/features/format/num_format.rs index dce39641d..60a3a404a 100644 --- a/src/uucore/src/lib/features/format/num_format.rs +++ b/src/uucore/src/lib/features/format/num_format.rs @@ -411,7 +411,7 @@ fn format_float_shortest( let mut normalized = format!("{normalized:.*}", precision); if force_decimal == ForceDecimal::No { - strip_zeros_and_dot(&mut normalized); + strip_fractional_zeroes_and_dot(&mut normalized); } let exp_char = match case { @@ -424,7 +424,8 @@ fn format_float_shortest( // Decimal-ish notation with a few differences: // - The precision works differently and specifies the total number // of digits instead of the digits in the fractional part. - // - If we don't force the decimal, '0' and `.` are trimmed. + // - If we don't force the decimal, `.` and trailing `0` in the fractional part + // are trimmed. let decimal_places = (precision as i32 - exponent) as usize; let mut formatted = if decimal_places == 0 && force_decimal == ForceDecimal::Yes { format!("{f:.0}.") @@ -433,7 +434,7 @@ fn format_float_shortest( }; if force_decimal == ForceDecimal::No { - strip_zeros_and_dot(&mut formatted); + strip_fractional_zeroes_and_dot(&mut formatted); } formatted @@ -469,12 +470,16 @@ fn format_float_hexadecimal( s } -fn strip_zeros_and_dot(s: &mut String) { - while s.ends_with('0') { - s.pop(); - } - if s.ends_with('.') { - s.pop(); +fn strip_fractional_zeroes_and_dot(s: &mut String) { + let mut trim_to = s.len(); + for (pos, c) in s.char_indices().rev() { + if pos + c.len_utf8() == trim_to && (c == '0' || c == '.') { + trim_to = pos; + } + if c == '.' { + s.truncate(trim_to); + break; + } } } @@ -580,4 +585,18 @@ mod test { assert_eq!(f(1000000.0), "1.e+06"); assert_eq!(f(99999999.0), "1.e+08"); } + + #[test] + fn strip_insignificant_end() { + use super::strip_fractional_zeroes_and_dot; + let f = |s| { + let mut s = String::from(s); + strip_fractional_zeroes_and_dot(&mut s); + s + }; + assert_eq!(&f("1000"), "1000"); + assert_eq!(&f("1000."), "1000"); + assert_eq!(&f("1000.02030"), "1000.0203"); + assert_eq!(&f("1000.00000"), "1000"); + } } diff --git a/tests/by-util/test_seq.rs b/tests/by-util/test_seq.rs index da28181eb..4a3286960 100644 --- a/tests/by-util/test_seq.rs +++ b/tests/by-util/test_seq.rs @@ -766,3 +766,23 @@ fn test_invalid_zero_increment_value() { .no_stdout() .usage_error("invalid Zero increment value: '0'"); } + +#[test] +fn test_power_of_ten_display() { + new_ucmd!() + .args(&["-f", "%.2g", "10", "10"]) + .succeeds() + .stdout_only("10\n"); +} + +#[test] +fn test_default_g_precision() { + new_ucmd!() + .args(&["-f", "%010g", "1e5", "1e5"]) + .succeeds() + .stdout_only("0000100000\n"); + new_ucmd!() + .args(&["-f", "%010g", "1e6", "1e6"]) + .succeeds() + .stdout_only("000001e+06\n"); +}