diff --git a/src/uu/numfmt/src/format.rs b/src/uu/numfmt/src/format.rs index 41bc03f5b6..23f0dc84e9 100644 --- a/src/uu/numfmt/src/format.rs +++ b/src/uu/numfmt/src/format.rs @@ -653,6 +653,30 @@ fn format_gnu_scientific(v: f64) -> String { } } +/// Format `value` with `precision` decimals, exactly as `{:.precision$}` would. +/// +/// Most of what numfmt prints is a whole number — `348M`, `1024` — and the +/// general float formatter reaches for a big-integer expansion of the value to +/// get its last digit right. When the value is an integer the answer needs no +/// expansion, so take the short way and let the rest fall through. +fn format_float(value: f64, precision: usize) -> String { + // Past 2^53 an f64 no longer holds every integer, and the two routes would + // not agree on what to print. + const EXACT_INTEGERS_UP_TO: f64 = 9_007_199_254_740_992.0; + + if precision == 0 && value.is_finite() && value.abs() < EXACT_INTEGERS_UP_TO { + // Both routes round halves to even; they part only on the sign of a + // negative value that rounds to zero, which `{:.0}` keeps. + let rounded = value.round_ties_even(); + if rounded == 0.0 && value.is_sign_negative() { + return "-0".to_string(); + } + return (rounded as i64).to_string(); + } + + format!("{value:.precision$}") +} + fn transform_to( s: ParsedNumber, opts: &TransformOptions, @@ -677,22 +701,24 @@ fn transform_to( } }; Ok(match s { - None if opts.to == Unit::None && precision <= u16::MAX.into() => localize(format!( - "{:.precision$}", + None if opts.to == Unit::None && precision <= u16::MAX.into() => localize(format_float( round_with_precision(i2, round_method, precision), + precision, )), None if is_precision_specified && precision <= u16::MAX.into() => { let i2 = round_with_precision(i2, round_method, 0); - localize(format!("{i2:.precision$}")) + localize(format_float(i2, precision)) } - None => localize(format!("{i2:.0}")), + None => localize(format_float(i2, 0)), Some(s) if precision > 0 && precision <= u16::MAX.into() => localize(format!( "{i2:.precision$}{unit_separator}{}", DisplayableSuffix(s, opts.to), )), - Some(s) if is_precision_specified => { - format!("{i2:.0}{unit_separator}{}", DisplayableSuffix(s, opts.to)) - } + Some(s) if is_precision_specified => format!( + "{}{unit_separator}{}", + format_float(i2, 0), + DisplayableSuffix(s, opts.to) + ), Some(s) if i2.abs() < 10.0 => { // single digit before the decimal, like 1.5K localize(format!( @@ -700,9 +726,11 @@ fn transform_to( DisplayableSuffix(s, opts.to) )) } - Some(s) => { - format!("{i2:.0}{unit_separator}{}", DisplayableSuffix(s, opts.to)) - } + Some(s) => format!( + "{}{unit_separator}{}", + format_float(i2, 0), + DisplayableSuffix(s, opts.to) + ), }) } @@ -932,6 +960,41 @@ pub fn write_formatted_with_whitespace( mod tests { use super::*; + #[test] + fn test_format_float_matches_the_general_formatter() { + // The short route must be indistinguishable from `{:.precision$}`, + // including the halves, the signed zero and the values that leave the + // range where an f64 holds every integer. + for value in [ + 0.0, + -0.0, + -0.4, + 0.5, + 1.5, + 2.5, + -1.5, + -2.5, + -0.5, + 348.123_456, + 1023.999, + 9_007_199_254_740_992.0, + 9_007_199_254_740_994.0, + -9_007_199_254_740_994.0, + 1e300, + f64::INFINITY, + f64::NEG_INFINITY, + f64::NAN, + ] { + for precision in [0, 1, 2, 6] { + assert_eq!( + format_float(value, precision), + format!("{value:.precision$}"), + "value {value}, precision {precision}" + ); + } + } + } + #[test] #[allow(clippy::cognitive_complexity)] fn test_round_with_precision() { diff --git a/src/uu/numfmt/src/numfmt.rs b/src/uu/numfmt/src/numfmt.rs index ac1567c250..8241e13cc9 100644 --- a/src/uu/numfmt/src/numfmt.rs +++ b/src/uu/numfmt/src/numfmt.rs @@ -15,7 +15,7 @@ use crate::options::{ use crate::units::{Result, Unit}; use clap::{Arg, ArgAction, ArgMatches, Command, builder::ValueParser, parser::ValueSource}; use std::ffi::OsString; -use std::io::{BufRead, Write as _, stderr}; +use std::io::{BufRead, BufWriter, IsTerminal, Write, stderr}; use std::str::FromStr; use uucore::display::Quotable; @@ -46,8 +46,8 @@ fn is_scientific(input: &[u8]) -> bool { /// /// Returns `true` if the line contained invalid input (only possible in /// non-abort modes). -fn format_and_write( - writer: &mut W, +fn format_and_write( + writer: &mut dyn Write, input_line: &[u8], options: &NumfmtOptions, eol: Option, @@ -62,7 +62,7 @@ fn format_and_write( // can emit the original line instead. let buffer_output = !matches!(options.invalid, InvalidModes::Abort); let mut buf = Vec::new(); - let dest: &mut dyn std::io::Write = if buffer_output { &mut buf } else { writer }; + let dest: &mut dyn Write = if buffer_output { &mut buf } else { writer }; let result = if options.delimiter.is_some() { write_formatted_with_delimiter(dest, line, options, eol) @@ -112,6 +112,33 @@ fn format_and_write( Ok(false) } +/// Run `body` with stdout buffered the way stdio buffers it: a block at a time +/// into a file or a pipe, where only the total number of writes matters, and a +/// line at a time onto a terminal, where output is read as it is produced. +/// +/// Whatever `body` wrote is flushed before this returns, so an error still +/// leaves the lines that came before it on stdout. +fn with_stdout(body: impl FnOnce(&mut dyn Write) -> UResult) -> UResult { + let stdout = std::io::stdout(); + if stdout.is_terminal() { + let mut writer = stdout.lock(); + finish(body(&mut writer), &mut writer) + } else { + let mut writer = BufWriter::new(stdout.lock()); + finish(body(&mut writer), &mut writer) + } +} + +/// Flush `writer`, keeping the failure `result` already carries, if any. +fn finish(result: UResult, writer: &mut impl Write) -> UResult { + let flushed = writer + .flush() + .map_err(|e| NumfmtError::IoError(e.to_string())); + let value = result?; + flushed?; + Ok(value) +} + /// Process command-line number arguments. /// /// `snapshot` is the command line as typed, for the caret under a number that @@ -123,31 +150,39 @@ fn handle_args<'a>( options: &NumfmtOptions, snapshot: Option<&[OsString]>, ) -> UResult { - let mut stdout = std::io::stdout().lock(); - let terminator = if options.zero_terminated { 0u8 } else { b'\n' }; - let mut saw_invalid = false; - for (n, l) in args.enumerate() { - match format_and_write(&mut stdout, l, options, Some(terminator)) { - Ok(invalid) => saw_invalid |= invalid, - // Only this mode stops on the first bad number; the others carry - // on, where a report per line would bury the output. - Err(error) => { - let reported = snapshot.is_some_and(|args| { - diagnostics::render_input(args, l, n, &error.to_string(), options) - }); - return Err(quiet_if_reported(reported, error)); + with_stdout(|stdout| { + let terminator = if options.zero_terminated { 0u8 } else { b'\n' }; + let mut saw_invalid = false; + for (n, l) in args.enumerate() { + match format_and_write(stdout, l, options, Some(terminator)) { + Ok(invalid) => saw_invalid |= invalid, + // Only this mode stops on the first bad number; the others carry + // on, where a report per line would bury the output. + Err(error) => { + let reported = snapshot.is_some_and(|args| { + diagnostics::render_input(args, l, n, &error.to_string(), options) + }); + return Err(quiet_if_reported(reported, error)); + } } } - } - Ok(saw_invalid) + Ok(saw_invalid) + }) } /// Process lines read from stdin. /// /// Returns `true` if any line contained invalid input. -fn handle_buffer(mut input: R, options: &NumfmtOptions) -> UResult { +fn handle_buffer(input: R, options: &NumfmtOptions) -> UResult { + with_stdout(|stdout| handle_buffer_to(input, options, stdout)) +} + +fn handle_buffer_to( + mut input: R, + options: &NumfmtOptions, + stdout: &mut dyn Write, +) -> UResult { let terminator = if options.zero_terminated { 0u8 } else { b'\n' }; - let mut stdout = std::io::stdout().lock(); let mut buf = Vec::new(); let mut line_idx = 0; let mut saw_invalid = false; @@ -178,7 +213,7 @@ fn handle_buffer(mut input: R, options: &NumfmtOptions) -> UResult