From a864a5736ae7c5d17047c32479a72eb510bef2f3 Mon Sep 17 00:00:00 2001 From: Yunus Ozen <94426948+ozenyunus35@users.noreply.github.com> Date: Thu, 22 Oct 2026 20:32:51 +0300 Subject: [PATCH] numfmt: handle --invalid one field at a time With --invalid=fail/warn/ignore, the first field that could not be converted made the whole line be printed unchanged, and only that field was reported. Like GNU, report each invalid field, write it as it is, and still convert the other fields of the line. Fixes #15575 --- src/uu/numfmt/src/format.rs | 66 +++++++++++++++++++++++++++------- src/uu/numfmt/src/numfmt.rs | 68 +++++++++++++++--------------------- tests/by-util/test_numfmt.rs | 25 +++++++++++++ 3 files changed, 106 insertions(+), 53 deletions(-) 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!()