diff --git a/src/uu/numfmt/src/format.rs b/src/uu/numfmt/src/format.rs index a6416595b1e..1a6120215f6 100644 --- a/src/uu/numfmt/src/format.rs +++ b/src/uu/numfmt/src/format.rs @@ -6,13 +6,15 @@ // spell-checker:ignore powf seps replacen use std::fmt::Write as _; +use std::io::Write as _; use uucore::display::Quotable; use uucore::i18n::decimal::{locale_decimal_separator, locale_grouping_separator}; -use uucore::translate; +use uucore::{show, translate}; +use crate::errors::NumfmtError; use crate::numeric::ParsedNumber; -use crate::options::{NumfmtOptions, RoundMethod, TransformOptions}; +use crate::options::{InvalidModes, NumfmtOptions, RoundMethod, TransformOptions}; use crate::units::{ DisplayableSuffix, RawSuffix, Result, Suffix, Unit, iec_bases_f64, si_bases_f64, }; @@ -935,13 +937,34 @@ fn split_bytes<'a>(input: &'a [u8], delim: &'a [u8]) -> impl Iterator std::result::Result<(), WriteError> { + match options.invalid { + InvalidModes::Abort => return Err(WriteError::Invalid(message)), + InvalidModes::Fail => show!(NumfmtError::FormattingError(message)), + InvalidModes::Warn => { + let _ = writeln!(std::io::stderr(), "numfmt: {message}"); + } + InvalidModes::Ignore => {} + } + Ok(()) +} + +/// Writes `input` with each selected field converted, and returns whether a +/// field could not be converted. pub fn write_formatted_with_delimiter( writer: &mut W, input: &[u8], options: &NumfmtOptions, eol: Option, -) -> std::result::Result<(), WriteError> { +) -> std::result::Result { let delimiter = options.delimiter.as_deref().unwrap(); + let mut saw_invalid = false; for (n, field) in (1..).zip(split_bytes(input, delimiter)) { let field_selected = uucore::ranges::contain(&options.fields, n); @@ -953,11 +976,17 @@ pub fn write_formatted_with_delimiter( if field_selected { // Field must be valid UTF-8 for numeric conversion - let field_str = std::str::from_utf8(field) - .map_err(|_| translate!("numfmt-error-invalid-number", "input" => escape_line(field).quote()))? - .trim_start(); - let formatted = format_string(field_str, options, None)?; - writer.write_all(formatted.as_bytes())?; + let formatted = std::str::from_utf8(field) + .map_err(|_| translate!("numfmt-error-invalid-number", "input" => escape_line(field).quote())) + .and_then(|field_str| format_string(field_str.trim_start(), options, None)); + match formatted { + Ok(formatted) => writer.write_all(formatted.as_bytes())?, + Err(message) => { + report_invalid_field(options, message)?; + saw_invalid = true; + writer.write_all(field)?; + } + } } else { // add unselected field without conversion writer.write_all(field)?; @@ -968,7 +997,7 @@ pub fn write_formatted_with_delimiter( writer.write_all(&[eol])?; } - Ok(()) + Ok(saw_invalid) } /// Writes the space that stands for the first of the `blanks` between two @@ -981,12 +1010,15 @@ fn write_separator<'a, W: std::io::Write + ?Sized>( Ok(&blanks[blanks.chars().next().map_or(0, char::len_utf8)..]) } +/// Writes `s` with each selected field converted, and returns whether a field +/// could not be converted. pub fn write_formatted_with_whitespace( writer: &mut W, s: &str, options: &NumfmtOptions, eol: Option, -) -> std::result::Result<(), WriteError> { +) -> std::result::Result { + let mut saw_invalid = false; for (n, (prefix, field)) in (1..).zip(WhitespaceSplitter { s: Some(s), options, @@ -1009,8 +1041,16 @@ pub fn write_formatted_with_whitespace( None }; - let formatted = format_string(field, options, implicit_padding)?; - writer.write_all(formatted.as_bytes())?; + match format_string(field, options, implicit_padding) { + Ok(formatted) => writer.write_all(formatted.as_bytes())?, + Err(message) => { + // An invalid field is written as it is, like an unselected one. + report_invalid_field(options, message)?; + saw_invalid = true; + writer.write_all(prefix.as_bytes())?; + writer.write_all(field.as_bytes())?; + } + } } else { // the first blank between fields is written as a space let prefix = if n > 1 { @@ -1028,7 +1068,7 @@ pub fn write_formatted_with_whitespace( writer.write_all(&[eol])?; } - Ok(()) + Ok(saw_invalid) } /// Writes a line that could not be converted, with the blanks between its diff --git a/src/uu/numfmt/src/numfmt.rs b/src/uu/numfmt/src/numfmt.rs index 969217c52bb..3a156888f7f 100644 --- a/src/uu/numfmt/src/numfmt.rs +++ b/src/uu/numfmt/src/numfmt.rs @@ -54,19 +54,15 @@ fn format_and_write( None => input_line, }; - // In non-abort modes we buffer the formatted output so that on error we - // can emit the original line instead. - let buffer_output = !matches!(options.invalid, InvalidModes::Abort); - let mut buf = Vec::new(); - let dest: &mut dyn Write = if buffer_output { &mut buf } else { writer }; - + // A field that cannot be converted is reported and written unchanged by + // the writers below, unless --invalid=abort, where it fails the line. let result = if options.delimiter.is_some() { - write_formatted_with_delimiter(dest, line, options, eol) + write_formatted_with_delimiter(writer, line, options, eol) } else { match std::str::from_utf8(line) { // Scientific notation is rejected per field when the number is // parsed, so a token in a field that is not converted is left alone. - Ok(s) => write_formatted_with_whitespace(dest, s, options, eol), + Ok(s) => write_formatted_with_whitespace(writer, s, options, eol), Err(_) => Err(translate!( "numfmt-error-invalid-number", "input" => escape_line(line).quote() @@ -76,50 +72,42 @@ fn format_and_write( }; // A failure to write is fatal, whatever --invalid says. - let result = match result { - Ok(()) => Ok(()), + let msg = match result { + Ok(saw_invalid) => return Ok(saw_invalid), Err(WriteError::Io(error)) => { return Err::(error).map_err_context(|| translate!("numfmt-error-write")); } - Err(WriteError::Invalid(message)) => Err(message), + Err(WriteError::Invalid(message)) => message, }; - if let Err(msg) = result { - match options.invalid { - InvalidModes::Abort => { - return Err(Box::new(NumfmtError::FormattingError(msg))); - } - InvalidModes::Fail => { - show!(NumfmtError::FormattingError(msg)); - } - InvalidModes::Warn => { - let _ = writeln!(stderr(), "numfmt: {msg}"); - } - InvalidModes::Ignore => {} + // Only --invalid=abort, or a line that is not valid UTF-8, gets here. + match options.invalid { + InvalidModes::Abort => { + return Err(Box::new(NumfmtError::FormattingError(msg))); } - // On error, echo the original line, with the blanks between fields - // written as usual unless an explicit delimiter is set. - match std::str::from_utf8(input_line) { - Ok(s) if options.delimiter.is_none() => { - write_unconverted_with_whitespace(writer, s, options) - } - _ => writer.write_all(input_line), + InvalidModes::Fail => { + show!(NumfmtError::FormattingError(msg)); } - .map_err_context(|| translate!("numfmt-error-write"))?; - if let Some(eol) = eol { - writer - .write_all(&[eol]) - .map_err_context(|| translate!("numfmt-error-write"))?; + InvalidModes::Warn => { + let _ = writeln!(stderr(), "numfmt: {msg}"); } - return Ok(true); + InvalidModes::Ignore => {} } - - if buffer_output { + // Echo the original line, with the blanks between fields written as + // usual unless an explicit delimiter is set. + match std::str::from_utf8(input_line) { + Ok(s) if options.delimiter.is_none() => { + write_unconverted_with_whitespace(writer, s, options) + } + _ => writer.write_all(input_line), + } + .map_err_context(|| translate!("numfmt-error-write"))?; + if let Some(eol) = eol { writer - .write_all(&buf) + .write_all(&[eol]) .map_err_context(|| translate!("numfmt-error-write"))?; } - Ok(false) + Ok(true) } /// Run `body` with stdout buffered the way stdio buffers it: a block at a time diff --git a/tests/by-util/test_numfmt.rs b/tests/by-util/test_numfmt.rs index 036da1c09cc..3c553ed4093 100644 --- a/tests/by-util/test_numfmt.rs +++ b/tests/by-util/test_numfmt.rs @@ -1156,6 +1156,31 @@ fn test_invalid_fail_with_fields_does_not_duplicate_output() { .stderr_is("numfmt: invalid number: 'Foo'\n"); } +#[test] +fn test_invalid_field_does_not_stop_the_line() { + // Each invalid field is reported and written as it is, and the fields + // around it are still converted. + new_ucmd!() + .args(&["--invalid=fail", "--to=si", "--field=-"]) + .pipe_in("x 1000 y 2000\n") + .fails_with_code(2) + .stdout_is("x 1.0k y 2.0k\n") + .stderr_is("numfmt: invalid number: 'x'\nnumfmt: invalid number: 'y'\n"); + + new_ucmd!() + .args(&["--invalid=warn", "--to=si", "--field=-", "-d:"]) + .pipe_in("x:1000:y\n") + .succeeds() + .stdout_is("x:1.0k:y\n") + .stderr_is("numfmt: invalid number: 'x'\nnumfmt: invalid number: 'y'\n"); + + new_ucmd!() + .args(&["--invalid=ignore", "--to=si", "--field=-"]) + .pipe_in(" x 1000\n") + .succeeds() + .stdout_only(" x 1.0k\n"); +} + #[test] fn test_abort_with_fields_preserves_partial_output() { new_ucmd!()