Skip to content

numfmt: handle --invalid one field at a time - #15639

Open
ozenyunus35 wants to merge 1 commit into
uutils:mainfrom
ozenyunus35:numfmt-invalid-per-field
Open

ozenyunus35 wants to merge 1 commit into
uutils:mainfrom
ozenyunus35:numfmt-invalid-per-field

Conversation

@ozenyunus35

Copy link
Copy Markdown
Contributor

Fixes #15575

With --invalid=fail, warn or ignore, the field writers stopped at the first selected field that was not a number. format_and_write then printed the original line, so the valid fields on that line were not converted, and only the first invalid field got a diagnostic. GNU converts every valid field, writes each invalid field as it is, and prints one diagnostic per invalid field.

The writers in format.rs now handle an invalid field themselves, through a small report_invalid_field helper:

  • abort returns the error, as before. The line stops there, with the output written so far.
  • fail and warn print the diagnostic, and ignore prints nothing. Then the field is written unchanged, with the blanks before it (or as the raw bytes with -d), and the loop goes on.

The writers return whether a field was invalid, so --debug still prints "failed to convert some of the input numbers". The output is no longer buffered in the non-abort modes, since a line is never replaced after the fact now. A line that is not valid UTF-8 (without -d) is still echoed whole, as before.

printf 'x 1000\n' | numfmt --invalid=ignore --to=si --field=-
# before: x 1000
# after:  x 1.0k        (same as GNU 9.12)

printf 'x 1000 y 2000\n' | numfmt --invalid=fail --to=si --field=-
# before: x 1000 y 2000, one diagnostic ('x')
# after:  x 1.0k y 2.0k, two diagnostics ('x' and 'y'), exit 2    (same as GNU 9.12)

I added test_invalid_field_does_not_stop_the_line, which covers fail, warn with -d:, and ignore with blanks before an invalid field. It fails without the change, and all test_numfmt tests and the uu_numfmt unit tests pass.

I also compared 40 cases with GNU 9.12 in the C locale, checking stdout, stderr and the exit code. They cover all four --invalid modes, -d, --padding, --format, --suffix, --header, --debug, --to-unit, -z, tabs and leading blanks, and invalid fields such as 12abc, 1e3 and 1000Q. On main, most of them differ. With this change, 37 match. The other three differ on main in the same way, and are not about --invalid:

  • 99999999999999999999999999999 --to=si gives 100R, where GNU gives 101R.
  • --from=si 2Q prints the number, where GNU reports "value too large to be printed".
  • A field that is not valid UTF-8 still makes the whole line unconverted.

cargo bench -p uu_numfmt shows no change against the merge base.

I used an AI assistant to help draft parts of the code and tests; I reviewed the changes and ran the tests myself.

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 uutils#15575
@sylvestre
sylvestre force-pushed the numfmt-invalid-per-field branch from 7bbde5b to a864a57 Compare October 25, 2026 13:21

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

numfmt: with --invalid=fail/warn/ignore, one invalid field leaves the whole line unconverted

1 participant