Repository navigation
Conversation
The value of -l, -w, -W, -N and -o was dropped by the legacy operand scan when it looked like one: `pr -l -1 f` read `-1` as a column count and `f` as the page length, and `pr -l +5 f` took `+5` for a page range. A page range that was not one (`--pages=0`, `2:1`) was reported as a bad argument, a `+` operand that was not a range was taken for a file, and a zero or negative length, width or column count got a message of its own. Keep the value of an option out of the operand scan, and let clap take a value that starts with `-`. Report the ranges and numbers the way GNU 9.12 does: `invalid page range '0'`, `--pages argument '...' too large`, `invalid + argument 'x'`, `invalid suffix in + argument '1:3x'`, and for the numbers `'-l PAGE_LENGTH' invalid number of lines: '0': Numerical result out of range`. A `+` operand that GNU takes for a file name, digits followed by other characters or a range that is not one, stays a file name.
Collaborator
|
Is there an existing issue describing the problem being addressed? |
Contributor
Author
|
GNU testsuite comparison: |
sylvestre
reviewed
Oct 8, 2026
sylvestre
reviewed
Oct 8, 2026
sylvestre
reviewed
Oct 8, 2026
sylvestre
reviewed
Oct 8, 2026
| /// Like [`parse_number`], for an option that wants a number from 1 up: | ||
| /// zero and negative numbers are numbers out of range. | ||
| fn parse_positive(raw: &str, option_message: &str) -> Result<usize, PrError> { | ||
| let negative = raw |
Contributor
There was a problem hiding this comment.
the same "is a negative number" check is in the -o parsing too, could be a small helper, no?
Contributor
Author
There was a problem hiding this comment.
Done @sylvestre , is_negative_number() for both.
sylvestre
reviewed
Oct 8, 2026
The list of options whose value is the next argument duplicated what clap knows and missed a cluster (`-tl -1`) and an abbreviation (`--len -1`). Ask the built Command instead: a long option, exact or the one option a prefix fits, or the last short option of a cluster, that takes at least one value and does not require `=`. The scan runs once, on the arguments after `-e` got its attached value, so clap and the operands see the same thing. `-h` and `-D` take a value that starts with `-`, as GNU does. One parse of `FIRST[:LAST]` serves both the `+` operand check and the messages. A number is the digits up to whatever follows them, so `--pages=1x` is an invalid page range and `+1x:3` a file name, and a number too large with characters after it is an invalid suffix, as GNU reports them. The numeric diagnostics go through translate!(). Drop the xstrtol.pl wording patch from build-gnu.sh, since pr now prints what the test expects, and make the file name test independent of the OS error text.
Contributor
Author
|
The xstrtol failure came from util/build-gnu.sh rewriting xstrtol.pl to the old wording; those three seds are gone since pr prints what the test expects now. The Windows test no longer depends on the OS error text. |
This branch has not been deployed
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
3 participants
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.
The value of
-l,-w,-W,-Nand-owas dropped by the legacy operand scan when it looked like one:pr -l -1 fread-1as a column count andfas the page length, andpr -l +5 ftook+5for a page range. A page range that is not one (--pages=0,--pages=2:1) was reported as a bad argument, a+operand that is not a range was taken for a file, and a zero or negative length, width or column count had a wording of its own.The value of an option now stays out of the operand scan, and clap takes a value that starts with
-. The messages follow GNU 9.12:invalid page range '0',--pages argument '…' too large,invalid + argument 'x',invalid suffix in + argument '1:3x', and for the numbers'-l PAGE_LENGTH' invalid number of lines: '0': Numerical result out of range(the errno text, asValue too large for defined data typealready is). A+operand that GNU takes for a file name, digits followed by other characters or numbers that make no page range, stays a file name.Every case in the tests was run against GNU 9.12; the pr suite passes. This covers
pr/options.shfrom #14705.Closes #15179.