Skip to content

ptx: match GNU's error for a bad --gap-size/--width value - #14315

Closed
arbelonson-source wants to merge 1 commit into
uutils:mainfrom
arbelonson-source:fix/ptx-gap-width-invalid-value
Closed

ptx: match GNU's error for a bad --gap-size/--width value#14315
arbelonson-source wants to merge 1 commit into
uutils:mainfrom
arbelonson-source:fix/ptx-gap-width-invalid-value

Conversation

@arbelonson-source

Copy link
Copy Markdown
Contributor

What

--gap-size/-g and --width/-w validate their values with clap's
own value_parser!(u64).range(1..), which causes the same two
problems already fixed for sort --parallel (#14303) and nl's
numeric options (#14314):

$ ptx -g -5 f

# GNU: still rejects the value, but with its own wording
ptx: invalid gap width: '-5'

# uutils, before this PR: treated as an unrecognized flag instead
error: unexpected argument '-5' found
  tip: to pass '-5' as a value, use '-- -5'
$ ptx -g bogus f

# GNU
ptx: invalid gap width: 'bogus'

# uutils, before this PR
error: invalid value 'bogus' for '--gap-size <NUMBER>': invalid digit found in string

Fix

Resolve the value by hand: parse as u64, reject anything that
doesn't parse as a positive integer (unparseable, zero, negative, or
overflowing) with GNU's wording. Unlike nl --number-width (#14314),
GNU does not distinguish an overflow from any other invalid value
here -- --gap-size 99999999999999999999999999 gets the exact same
"invalid gap width: '...'" message as --gap-size bogus -- so this
one only needed a single uniform check, confirmed by diffing both
cases against real GNU rather than assuming the nl-style distinction
would apply here too.

Testing

  • cargo test -p uu_ptx / full tests/by-util/test_ptx.rs suite: 50 passed, 0 failed.
  • Strengthened the existing test_invalid_arg test (which only checked the exit code, with a comment noting the message was clap-provided) to assert the exact GNU wording.
  • Added a regression test for the separate-argument negative-value case (the actual reported bug) on both -g and --width.
  • Manually diffed 12 cases (invalid, negative attached/separate/short-flag, zero, overflow, empty, valid) against GNU ptx 9.11 under LC_ALL=C.
  • cargo clippy -p uu_ptx --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.

--gap-size and --width validated their values with clap's own
value_parser!(u64).range(1..), so:

- a negative value passed as its own argument (`-g -5`, not the
  attached `-g-5`) was rejected as an unrecognized flag instead of
  being read as the option's value (still ultimately invalid, since
  GNU rejects negative values here too, but with the wrong message);
- an invalid value -- unparseable, zero, negative, or overflowing --
  produced clap's own generic wording instead of GNU's uniform
  'invalid gap width: ...'/'invalid line width: ...', which (unlike
  nl's --number-width fixed in uutils#14314) does not distinguish an
  overflow from any other kind of invalid value.

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

This comment was marked as outdated.

@arbelonson-source

Copy link
Copy Markdown
Contributor Author

Closing proactively, same reasoning as #14314 just now — same template, same maintainer guidance from #14308/#14312.

@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Note: The gnu test tests/cp/sparse-2 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.

2 participants