diff --git a/src/uu/numfmt/locales/en-US.ftl b/src/uu/numfmt/locales/en-US.ftl index 4e5faf56e..c8b51dbe2 100644 --- a/src/uu/numfmt/locales/en-US.ftl +++ b/src/uu/numfmt/locales/en-US.ftl @@ -42,6 +42,7 @@ numfmt-help-field = replace the numbers in these input fields; see FIELDS below numfmt-help-format = use printf style floating-point FORMAT; see FORMAT below for details numfmt-help-from = auto-scale input numbers to UNITs; see UNIT below numfmt-help-from-unit = specify the input unit size +numfmt-help-grouping = use locale-defined grouping of digits, for example 1,000,000 (which means it has no effect in the C/POSIX locale) numfmt-help-to = auto-scale output numbers to UNITs; see UNIT below numfmt-help-to-unit = the output unit size numfmt-help-padding = pad the output to N characters; positive N will right-align; negative N will left-align; padding is ignored if the output is wider than N; the default is to automatically pad if a whitespace is found @@ -57,6 +58,7 @@ numfmt-error-unsupported-unit = Unsupported unit is specified numfmt-error-invalid-unit-size = invalid unit size: { $size } numfmt-error-invalid-padding = invalid padding value { $value } numfmt-error-invalid-header = invalid header value { $value } +numfmt-error-grouping-cannot-be-combined-with-format = --grouping cannot be combined with --format numfmt-error-grouping-cannot-be-combined-with-to = grouping cannot be combined with --to numfmt-error-delimiter-must-be-single-character = the delimiter must be a single character numfmt-error-invalid-number-empty = invalid number: '' @@ -75,7 +77,3 @@ numfmt-error-invalid-format-width-overflow = invalid format '{ $format }' (width numfmt-error-invalid-precision = invalid precision in format '{ $format }' numfmt-error-format-too-many-percent = format '{ $format }' has too many % directives numfmt-error-unknown-invalid-mode = Unknown invalid mode: { $mode } - -# Debug messages -numfmt-debug-no-conversion = no conversion option specified -numfmt-debug-header-ignored = --header ignored with command-line input diff --git a/src/uu/numfmt/locales/fr-FR.ftl b/src/uu/numfmt/locales/fr-FR.ftl index 20bd91db9..3a6094eeb 100644 --- a/src/uu/numfmt/locales/fr-FR.ftl +++ b/src/uu/numfmt/locales/fr-FR.ftl @@ -41,6 +41,7 @@ numfmt-help-field = remplacer les nombres dans ces champs d'entrée ; voir FIELD numfmt-help-format = utiliser le FORMAT à virgule flottante de style printf ; voir FORMAT ci-dessous pour les détails numfmt-help-from = mettre automatiquement à l'échelle les nombres d'entrée vers les UNITÉs ; voir UNIT ci-dessous numfmt-help-from-unit = spécifier la taille de l'unité d'entrée +numfmt-help-grouping = utiliser le groupement des chiffres défini par la locale, par exemple 1 000 000 (ce qui n'a aucun effet dans la locale C/POSIX) numfmt-help-to = mettre automatiquement à l'échelle les nombres de sortie vers les UNITÉs ; voir UNIT ci-dessous numfmt-help-to-unit = la taille de l'unité de sortie numfmt-help-padding = remplir la sortie à N caractères ; N positif alignera à droite ; N négatif alignera à gauche ; le remplissage est ignoré si la sortie est plus large que N ; la valeur par défaut est de remplir automatiquement si un espace est trouvé @@ -55,6 +56,7 @@ numfmt-error-unsupported-unit = Une unité non supportée est spécifiée numfmt-error-invalid-unit-size = taille d'unité invalide : { $size } numfmt-error-invalid-padding = valeur de remplissage invalide { $value } numfmt-error-invalid-header = valeur d'en-tête invalide { $value } +numfmt-error-grouping-cannot-be-combined-with-format = --grouping ne peut pas être combiné avec --format numfmt-error-grouping-cannot-be-combined-with-to = le groupement ne peut pas être combiné avec --to numfmt-error-delimiter-must-be-single-character = le délimiteur doit être un seul caractère numfmt-error-invalid-number-empty = nombre invalide : '' diff --git a/src/uu/numfmt/src/format.rs b/src/uu/numfmt/src/format.rs index d8247bb23..12f6e9ac5 100644 --- a/src/uu/numfmt/src/format.rs +++ b/src/uu/numfmt/src/format.rs @@ -6,6 +6,7 @@ // spell-checker:ignore powf use uucore::display::Quotable; +use uucore::i18n::decimal::locale_grouping_separator; use uucore::translate; use crate::options::{NumfmtOptions, RoundMethod, TransformOptions}; @@ -75,6 +76,14 @@ fn detailed_error_message(s: &str, unit: Unit) -> Option { .ok_or(translate!("numfmt-error-invalid-number", "input" => s.quote())) .ok()?; + if number_prefix == "." { + return Some(translate!("numfmt-error-invalid-suffix", "input" => s.quote())); + } + + if number_prefix.ends_with('.') { + return Some(translate!("numfmt-error-invalid-number", "input" => s.quote())); + } + if valid_part != s && valid_part.parse::().is_ok() { return match s.chars().nth(valid_part.len()) { Some('+' | '-') => { @@ -96,6 +105,15 @@ fn detailed_error_message(s: &str, unit: Unit) -> Option { None } +fn parse_number_part(s: &str, input: &str) -> Result { + if s.ends_with('.') { + return Err(translate!("numfmt-error-invalid-number", "input" => input.quote())); + } + + s.parse::() + .map_err(|_| translate!("numfmt-error-invalid-number", "input" => input.quote())) +} + fn parse_suffix( s: &str, unit: Unit, @@ -159,20 +177,48 @@ fn parse_suffix( whitespace }; - let number = number_part[..number_part.len() - separator_len] - .parse::() - .map_err(|_| translate!("numfmt-error-invalid-number", "input" => s.quote()))?; + let number = parse_number_part(&number_part[..number_part.len() - separator_len], s)?; return Ok((number, suffix)); } - let number = number_part - .parse::() - .map_err(|_| translate!("numfmt-error-invalid-number", "input" => s.quote()))?; + let number = parse_number_part(number_part, s)?; Ok((number, suffix)) } +fn apply_grouping(s: &str) -> String { + let grouping_separator = locale_grouping_separator(); + if grouping_separator.is_empty() { + return s.to_string(); + } + + let (sign, rest) = if let Some(rest) = s.strip_prefix('-') { + ("-", rest) + } else { + ("", s) + }; + let (integer, fraction) = rest.split_once('.').map_or((rest, ""), |(i, f)| (i, f)); + if integer.len() < 4 { + return s.to_string(); + } + + let mut grouped_rev = String::with_capacity(s.len() + (integer.len() / 3)); + for (idx, ch) in integer.chars().rev().enumerate() { + if idx > 0 && idx % 3 == 0 { + grouped_rev.push_str(grouping_separator); + } + grouped_rev.push(ch); + } + let grouped_integer: String = grouped_rev.chars().rev().collect(); + + if fraction.is_empty() { + format!("{sign}{grouped_integer}") + } else { + format!("{sign}{grouped_integer}.{fraction}") + } +} + fn next_field_index(s: &str) -> usize { s.find(char::is_whitespace).unwrap_or(s.len()) } @@ -210,10 +256,16 @@ fn split_mergeable_suffix<'a>(s: &'a str, options: &NumfmtOptions) -> Option<(&' match field.len() { 1 => { - let _ = field.chars().next().filter(|c| RawSuffix::try_from(c).is_ok())?; + let _ = field + .chars() + .next() + .filter(|c| RawSuffix::try_from(c).is_ok())?; } 2 if field.ends_with('i') => { - let _ = field.chars().next().filter(|c| RawSuffix::try_from(c).is_ok())?; + let _ = field + .chars() + .next() + .filter(|c| RawSuffix::try_from(c).is_ok())?; } _ => return None, } @@ -307,7 +359,7 @@ fn transform_from(s: &str, opts: &TransformOptions, options: &NumfmtOptions) -> &options.unit_separator, options.explicit_unit_separator, ) - .map_err(|original| detailed_error_message(s, opts.from).unwrap_or(original))?; + .map_err(|original| detailed_error_message(s, opts.from).unwrap_or(original))?; let i = i * (opts.from_unit as f64); remove_suffix(i, suffix, opts.from).map(|n| { @@ -462,11 +514,7 @@ fn format_string( }; let number = transform_to( - transform_from( - source_without_suffix, - &options.transform, - options, - )?, + transform_from(source_without_suffix, &options.transform, options)?, &options.transform, options.round, precision, @@ -474,9 +522,15 @@ fn format_string( )?; // bring back the suffix before applying padding + let grouped_number = if options.grouping { + apply_grouping(&number) + } else { + number + }; + let number_with_suffix = match &options.suffix { - Some(suffix) => format!("{number}{suffix}"), - None => number, + Some(suffix) => format!("{grouped_number}{suffix}"), + None => grouped_number, }; let padding = options @@ -669,7 +723,7 @@ mod tests { #[test] fn test_parse_suffix_q_r_k() { - let result = parse_suffix("1Q", Unit::Auto, 1); + let result = parse_suffix("1Q", Unit::Auto, "", false); assert!(result.is_ok()); let (number, suffix) = result.unwrap(); assert_eq!(number, 1.0); @@ -678,7 +732,7 @@ mod tests { assert_eq!(raw_suffix as i32, RawSuffix::Q as i32); assert!(!with_i); - let result = parse_suffix("2R", Unit::Auto, 1); + let result = parse_suffix("2R", Unit::Auto, "", false); assert!(result.is_ok()); let (number, suffix) = result.unwrap(); assert_eq!(number, 2.0); @@ -687,7 +741,7 @@ mod tests { assert_eq!(raw_suffix as i32, RawSuffix::R as i32); assert!(!with_i); - let result = parse_suffix("3k", Unit::Auto, 1); + let result = parse_suffix("3k", Unit::Auto, "", false); assert!(result.is_ok()); let (number, suffix) = result.unwrap(); assert_eq!(number, 3.0); @@ -696,7 +750,7 @@ mod tests { assert_eq!(raw_suffix as i32, RawSuffix::K as i32); assert!(!with_i); - let result = parse_suffix("4Qi", Unit::Auto, 1); + let result = parse_suffix("4Qi", Unit::Auto, "", false); assert!(result.is_ok()); let (number, suffix) = result.unwrap(); assert_eq!(number, 4.0); @@ -705,7 +759,7 @@ mod tests { assert_eq!(raw_suffix as i32, RawSuffix::Q as i32); assert!(with_i); - let result = parse_suffix("5Ri", Unit::Auto, 1); + let result = parse_suffix("5Ri", Unit::Auto, "", false); assert!(result.is_ok()); let (number, suffix) = result.unwrap(); assert_eq!(number, 5.0); @@ -717,13 +771,13 @@ mod tests { #[test] fn test_parse_suffix_error_messages() { - let result = parse_suffix("foo", Unit::Auto, 1); + let result = parse_suffix("foo", Unit::Auto, "", false); assert!(result.is_err()); let error = result.unwrap_err(); assert!(error.contains("numfmt-error-invalid-number") || error.contains("invalid number")); assert!(!error.contains("invalid suffix")); - let result = parse_suffix("World", Unit::Auto, 1); + let result = parse_suffix("World", Unit::Auto, "", false); assert!(result.is_err()); let error = result.unwrap_err(); assert!(error.contains("numfmt-error-invalid-number") || error.contains("invalid number")); diff --git a/src/uu/numfmt/src/numfmt.rs b/src/uu/numfmt/src/numfmt.rs index a779a7cd5..c0726ac67 100644 --- a/src/uu/numfmt/src/numfmt.rs +++ b/src/uu/numfmt/src/numfmt.rs @@ -7,9 +7,9 @@ use crate::errors::NumfmtError; use crate::format::{escape_line, write_formatted_with_delimiter, write_formatted_with_whitespace}; use crate::options::{ DEBUG, DELIMITER, FIELD, FIELD_DEFAULT, FORMAT, FROM, FROM_DEFAULT, FROM_UNIT, - FROM_UNIT_DEFAULT, FormatOptions, HEADER, HEADER_DEFAULT, INVALID, InvalidModes, NUMBER, - NumfmtOptions, PADDING, ROUND, RoundMethod, SUFFIX, TO, TO_DEFAULT, TO_UNIT, TO_UNIT_DEFAULT, - TransformOptions, UNIT_SEPARATOR, ZERO_TERMINATED, + FROM_UNIT_DEFAULT, FormatOptions, GROUPING, HEADER, HEADER_DEFAULT, INVALID, InvalidModes, + NUMBER, NumfmtOptions, PADDING, ROUND, RoundMethod, SUFFIX, TO, TO_DEFAULT, TO_UNIT, + TO_UNIT_DEFAULT, TransformOptions, UNIT_SEPARATOR, ZERO_TERMINATED, }; use crate::units::{Result, Unit}; use clap::{Arg, ArgAction, ArgMatches, Command, builder::ValueParser, parser::ValueSource}; @@ -21,6 +21,7 @@ use units::{IEC_BASES, SI_BASES}; use uucore::display::Quotable; use uucore::error::UResult; +use uucore::i18n::decimal::locale_grouping_separator; use uucore::parser::shortcut_value_parser::ShortcutValueParser; use uucore::ranges::Range; use uucore::{format_usage, os_str_as_bytes, show, translate}; @@ -30,73 +31,28 @@ pub mod format; pub mod options; mod units; -fn handle_args<'a>(args: impl Iterator, options: &NumfmtOptions) -> UResult<()> { - let mut stdout = std::io::stdout().lock(); - let terminator = if options.zero_terminated { 0u8 } else { b'\n' }; - for l in args { - write_line(&mut stdout, l, options, Some(terminator))?; - } - Ok(()) -} - -fn handle_buffer(mut input: R, options: &NumfmtOptions) -> 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 idx = 0; - - loop { - buf.clear(); - let n = input - .read_until(terminator, &mut buf) - .map_err(|e| NumfmtError::IoError(e.to_string()))?; - if n == 0 { - break; - } - - let has_terminator = buf.last() == Some(&terminator); - let line = if has_terminator { - &buf[..buf.len() - 1] - } else { - &buf[..] - }; - - // Emit the terminator only if the input line had one. - // i.e. if the last line of the input does not end with a newline, we should not add one. - let eol = has_terminator.then_some(terminator); - - if idx < options.header { - stdout.write_all(line)?; - if let Some(t) = eol { - stdout.write_all(&[t])?; - } - } else { - write_line(&mut stdout, line, options, eol)?; - } - - idx += 1; - } - - Ok(()) -} - -fn write_line( +/// Format a single line and write it, handling `--invalid` error modes. +/// +/// Returns `true` if the line contained invalid input (only possible in +/// non-abort modes). +fn format_and_write( writer: &mut W, input_line: &[u8], options: &NumfmtOptions, eol: Option, -) -> UResult<()> { - // Read lines only up to null byte (as GNU does) +) -> UResult { + // GNU truncates at the first embedded null byte. let line = match memchr::memchr(b'\0', input_line) { Some(i) => &input_line[..i], None => input_line, }; + let mut formatted_line = Vec::new(); let handled_line = if options.delimiter.is_some() { - write_formatted_with_delimiter(writer, line, options, eol) + write_formatted_with_delimiter(&mut formatted_line, line, options, eol) } else { // Whitespace mode requires valid UTF-8 match std::str::from_utf8(line) { - Ok(s) => write_formatted_with_whitespace(writer, s, options, eol), + Ok(s) => write_formatted_with_whitespace(&mut formatted_line, s, options, eol), Err(_) => { Err(translate!("numfmt-error-invalid-number", "input" => escape_line(line).quote())) } @@ -121,9 +77,11 @@ fn write_line( if let Some(eol) = eol { writer.write_all(&[eol])?; } + return Ok(true); } - Ok(()) + writer.write_all(&formatted_line)?; + Ok(false) } fn parse_unit(s: &str) -> Result { @@ -252,12 +210,21 @@ fn parse_options(args: &ArgMatches) -> Result { Range::from_list(fields)? }; + let grouping = args.get_flag(GROUPING); let format = match args.get_one::(FORMAT) { Some(s) => s.parse()?, None => FormatOptions::default(), }; - if format.grouping && to != Unit::None { + if grouping && args.contains_id(FORMAT) { + return Err(translate!( + "numfmt-error-grouping-cannot-be-combined-with-format" + )); + } + + let grouping = grouping || format.grouping; + + if grouping && to != Unit::None { return Err(translate!( "numfmt-error-grouping-cannot-be-combined-with-to" )); @@ -303,6 +270,7 @@ fn parse_options(args: &ArgMatches) -> Result { round, suffix, unit_separator, + grouping, explicit_unit_separator, format, invalid, @@ -314,15 +282,14 @@ fn parse_options(args: &ArgMatches) -> Result { fn print_debug_warnings(options: &NumfmtOptions, matches: &ArgMatches) { fn print_warning(msg_key: &str) { let _ = writeln!(stderr(), "numfmt: {}", translate!(msg_key)); - } - - // Warn if no conversion option is specified - // 2>/dev/full does not abort - if options.transform.from == Unit::None - && options.transform.to == Unit::None - && options.padding == 0 - { - print_warning("numfmt-debug-no-conversion"); + let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; + if options.grouping && locale_grouping_separator().is_empty() { + let _ = writeln!( + stderr(), + "{}: {}", + util_name(), + translate!("numfmt-debug-grouping-no-effect") + ); } // Warn if --header is used with command-line input @@ -333,7 +300,17 @@ fn print_debug_warnings(options: &NumfmtOptions, matches: &ArgMatches) { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; + // GNU numfmt accepts both --debug and ---debug (triple dash); normalize for clap. + let normalized_args: Vec = args + .map(|arg| { + if arg == "---debug" { + OsString::from("--debug") + } else { + arg + } + }) + .collect(); + let matches = uucore::clap_localization::handle_clap_result(uu_app(), normalized_args)?; let options = parse_options(&matches).map_err(NumfmtError::IllegalArgument)?; @@ -346,19 +323,23 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .map(|s| os_str_as_bytes(s).map_err(|e| e.to_string())) .collect::, _>>() .map_err(NumfmtError::IllegalArgument)?; - handle_args(byte_args.into_iter(), &options) - } else { - let stdin = std::io::stdin(); - let mut locked_stdin = stdin.lock(); - handle_buffer(&mut locked_stdin, &options) - }; match result { Err(e) => { std::io::stdout().flush().expect("error flushing stdout"); Err(e) } - _ => Ok(()), + Ok(saw_invalid) => { + if options.debug && saw_invalid { + let _ = writeln!( + stderr(), + "{}: {}", + util_name(), + translate!("numfmt-debug-failed-to-convert") + ); + } + Ok(()) + } } } @@ -377,6 +358,12 @@ pub fn uu_app() -> Command { .help(translate!("numfmt-help-debug")) .action(ArgAction::SetTrue), ) + .arg( + Arg::new(GROUPING) + .long(GROUPING) + .help(translate!("numfmt-help-grouping")) + .action(ArgAction::SetTrue), + ) .arg( Arg::new(DELIMITER) .short('d') @@ -524,6 +511,7 @@ mod tests { round: RoundMethod::Nearest, suffix: None, unit_separator: String::new(), + grouping: false, explicit_unit_separator: false, format: FormatOptions::default(), invalid: InvalidModes::Abort, diff --git a/src/uu/numfmt/src/options.rs b/src/uu/numfmt/src/options.rs index 21d6e7ff9..ced2d4b64 100644 --- a/src/uu/numfmt/src/options.rs +++ b/src/uu/numfmt/src/options.rs @@ -17,6 +17,7 @@ pub const FROM: &str = "from"; pub const FROM_DEFAULT: &str = "none"; pub const FROM_UNIT: &str = "from-unit"; pub const FROM_UNIT_DEFAULT: &str = "1"; +pub const GROUPING: &str = "grouping"; pub const HEADER: &str = "header"; pub const HEADER_DEFAULT: &str = "1"; pub const INVALID: &str = "invalid"; @@ -55,6 +56,7 @@ pub struct NumfmtOptions { pub round: RoundMethod, pub suffix: Option, pub unit_separator: String, + pub grouping: bool, pub explicit_unit_separator: bool, pub format: FormatOptions, pub invalid: InvalidModes, diff --git a/tests/by-util/test_numfmt.rs b/tests/by-util/test_numfmt.rs index e9c9994a5..f4af8f4f1 100644 --- a/tests/by-util/test_numfmt.rs +++ b/tests/by-util/test_numfmt.rs @@ -1113,6 +1113,14 @@ fn test_format_grouping_conflicts_with_to_option() { .stderr_contains("grouping cannot be combined with --to"); } +#[test] +fn test_grouping_conflicts_with_format_option() { + new_ucmd!() + .args(&["--format=%f", "--grouping"]) + .fails_with_code(1) + .stderr_contains("--grouping cannot be combined with --format"); +} + #[test] fn test_zero_terminated_command_line_args() { new_ucmd!() @@ -1208,6 +1216,59 @@ fn test_debug_warnings() { .succeeds() .stdout_is("4.0K\n") .stderr_is("numfmt: --header ignored with command-line input\n"); + + new_ucmd!() + .env("LC_ALL", "C") + .args(&["--debug", "--grouping", "--from=si", "4.0K"]) + .succeeds() + .stdout_is("4000\n") + .stderr_is("numfmt: grouping has no effect in this locale\n"); +} + +#[test] +fn test_debug_reports_failed_conversions_summary() { + new_ucmd!() + .args(&[ + "--invalid=fail", + "--debug", + "--to=si", + "1000", + "Foo", + "3000", + ]) + .fails_with_code(2) + .stdout_is("1.0k\nFoo\n3.0k\n") + .stderr_is( + "numfmt: invalid number: 'Foo'\nnumfmt: failed to convert some of the input numbers\n", + ); +} + +#[test] +fn test_invalid_fail_with_fields_does_not_duplicate_output() { + new_ucmd!() + .args(&["--invalid=fail", "--field=2", "--from=si", "--to=iec"]) + .pipe_in("A 1K x\nB Foo y\nC 3G z\n") + .fails_with_code(2) + .stdout_is("A 1000 x\nB Foo y\nC 2.8G z\n") + .stderr_is("numfmt: invalid number: 'Foo'\n"); +} + +#[test] +fn test_rejects_malformed_number_forms() { + new_ucmd!() + .args(&["--from=si", "12.K"]) + .fails_with_code(2) + .stderr_contains("invalid number: '12.K'"); + + new_ucmd!() + .args(&["--from=si", "--delimiter=,", "12. 2"]) + .fails_with_code(2) + .stderr_contains("invalid number: '12. 2'"); + + new_ucmd!() + .arg("..1") + .fails_with_code(2) + .stderr_contains("invalid suffix in input: '..1'"); } #[test]