Skip to content

numfmt: accept hyphen-leading -d/--suffix/--padding values - #14322

Open
arbelonson-source wants to merge 1 commit into
uutils:mainfrom
arbelonson-source:fix/numfmt-hyphen-values
Open

numfmt: accept hyphen-leading -d/--suffix/--padding values#14322
arbelonson-source wants to merge 1 commit into
uutils:mainfrom
arbelonson-source:fix/numfmt-hyphen-values

Conversation

@arbelonson-source

Copy link
Copy Markdown
Contributor

What

-d/--delimiter, --suffix, and --padding all had the same gap
already fixed this session for several other options: their value was
rejected as an unrecognized flag when given as its own argument, since
the Args were missing allow_hyphen_values:

$ echo 5 | numfmt --padding -8
# GNU: left-justifies (negative padding is meaningful)
5       ...
# uutils, before this PR:
error: unexpected argument '-8' found

$ echo 5 | numfmt --suffix -x
# GNU: 5-x
# uutils, before this PR: error: unexpected argument '-x' found

$ echo "5:6" | numfmt -d -x --field=1
# GNU: numfmt: the delimiter must be a single character
# uutils, before this PR: error: unexpected argument '-x' found

The attached form (--padding=-8, etc.) already worked; only the
separate-argument form broke -- the same gap already fixed this
session for sort --parallel, nl's numeric options, ptx --gap-size/--width, join -t, csplit --suffix-format, and paste -d.

--padding already parsed its value as a signed isize, so no change
was needed there beyond letting clap route the value to it in the
first place.

Testing

  • cargo test -p uu_numfmt / full tests/by-util/test_numfmt.rs suite: 188 passed, 0 failed.
  • Added 3 regression tests, one per option, for the separate-argument hyphen-leading case.
  • Manually diffed --suffix -x/x, --padding -5/5/-0, and -d -x/: against GNU numfmt 9.11 under LC_ALL=C.
  • cargo clippy -p uu_numfmt --all-targets -- -D warnings and cargo fmt --check: clean.

This PR was written with AI assistance (Claude Opus 5, via Claude Code). I've tested the changes but please review the code carefully.

-d/--delimiter, --suffix, and --padding all had the same gap already
fixed this session for several other options: their value was rejected
as an unrecognized flag when given as its own argument (numfmt
--padding -8, not the attached --padding=-8), since the Args were
missing allow_hyphen_values. GNU's own numfmt accepts any argument as
these options' values regardless of the first character -- including
a negative --padding, which is meaningful (left-justifies instead of
right-justifying).

--padding already parsed as a signed isize, so no change was needed
beyond letting clap route the value to it in the first place.

AI-assisted-by: Claude Opus 5, via Claude Code
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/date/resolution (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/expand/bounded-memory is now being skipped but was previously passing.
Note: The gnu test tests/rm/many-dir-entries-vs-OOM is now being skipped but was previously passing.
Skip an intermittent issue tests/pr/bounded-memory (was skipped on 'main', now failing)

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.

1 participant