numfmt: add --grouping flag, debug diagnostics, and stricter number validation

Fixes GNU numfmt.pl tests: grp-1, grp-2, debug-2, fmt-err-9,
fmt-err-11.
This commit is contained in:
Sylvestre Ledru
2026-04-04 23:24:46 +02:00
parent a39921e206
commit d68ff519b5
6 changed files with 209 additions and 104 deletions
+2 -4
View File
@@ -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
+2
View File
@@ -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 : ''
+77 -23
View File
@@ -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<String> {
.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::<f64>().is_ok() {
return match s.chars().nth(valid_part.len()) {
Some('+' | '-') => {
@@ -96,6 +105,15 @@ fn detailed_error_message(s: &str, unit: Unit) -> Option<String> {
None
}
fn parse_number_part(s: &str, input: &str) -> Result<f64> {
if s.ends_with('.') {
return Err(translate!("numfmt-error-invalid-number", "input" => input.quote()));
}
s.parse::<f64>()
.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::<f64>()
.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::<f64>()
.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"));
+65 -77
View File
@@ -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<Item = &'a [u8]>, 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<R: BufRead>(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<W: std::io::Write>(
/// 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<W: std::io::Write>(
writer: &mut W,
input_line: &[u8],
options: &NumfmtOptions,
eol: Option<u8>,
) -> UResult<()> {
// Read lines only up to null byte (as GNU does)
) -> UResult<bool> {
// 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<W: std::io::Write>(
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<Unit> {
@@ -252,12 +210,21 @@ fn parse_options(args: &ArgMatches) -> Result<NumfmtOptions> {
Range::from_list(fields)?
};
let grouping = args.get_flag(GROUPING);
let format = match args.get_one::<String>(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<NumfmtOptions> {
round,
suffix,
unit_separator,
grouping,
explicit_unit_separator,
format,
invalid,
@@ -314,15 +282,14 @@ fn parse_options(args: &ArgMatches) -> Result<NumfmtOptions> {
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<OsString> = 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::<std::result::Result<Vec<_>, _>>()
.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,
+2
View File
@@ -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<String>,
pub unit_separator: String,
pub grouping: bool,
pub explicit_unit_separator: bool,
pub format: FormatOptions,
pub invalid: InvalidModes,
+61
View File
@@ -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]