ptx: match GNU's error for a bad --gap-size/--width value - #14315
Closed
arbelonson-source wants to merge 1 commit into
Closed
ptx: match GNU's error for a bad --gap-size/--width value#14315arbelonson-source wants to merge 1 commit into
arbelonson-source wants to merge 1 commit into
Conversation
--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
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
Author
|
GNU testsuite comparison: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
--gap-size/-gand--width/-wvalidate their values with clap'sown
value_parser!(u64).range(1..), which causes the same twoproblems already fixed for
sort --parallel(#14303) andnl'snumeric options (#14314):
Fix
Resolve the value by hand: parse as
u64, reject anything thatdoesn'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 99999999999999999999999999gets the exact same"invalid gap width: '...'" message as
--gap-size bogus-- so thisone 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/ fulltests/by-util/test_ptx.rssuite: 50 passed, 0 failed.test_invalid_argtest (which only checked the exit code, with a comment noting the message was clap-provided) to assert the exact GNU wording.-gand--width.LC_ALL=C.cargo clippy -p uu_ptx --all-targets -- -D warningsandcargo 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.