Skip to content

shuf: report a bad -i range the way GNU does - #14402

Open
Socialpranker wants to merge 1 commit into
uutils:mainfrom
Socialpranker:shuf-invalid-input-range
Open

shuf: report a bad -i range the way GNU does#14402
Socialpranker wants to merge 1 commit into
uutils:mainfrom
Socialpranker:shuf-invalid-input-range

Conversation

@Socialpranker

Copy link
Copy Markdown
Contributor

A LO-HI that would not parse was rejected by clap's value parser, which
names the option and the internal reason:

$ shuf -i 5-1
shuf: invalid input range: '5-1'                                             # GNU
error: invalid value '5-1' for '--input-range <LO-HI>': start exceeds end    # uutils, before

$ shuf -i 1-
shuf: invalid input range: '1-'                                              # GNU
error: invalid value '1-' for '--input-range <LO-HI>': cannot parse integer  # uutils, before
from empty string

$ shuf -i 99999999999999999999999-1
shuf: invalid input range: '99999999999999999999999-1': Value too large to be stored in data type

GNU names the range as a whole whatever is wrong with it — a missing -,
a non-numeric bound, an extra -, LO above HI — and adds a detail in
exactly one case, a bound that does not fit.

The fix

parse_range moves out of .value_parser() and into uumain, returning
a UResult with USimpleError::new(1, ...). IntErrorKind::PosOverflow
selects the "Value too large" wording; everything else gets the plain one.
The two message keys replace shuf-error-start-exceeds-end and
shuf-error-missing-dash, which had no other user.

Five existing assertions pinned the clap wording; they are updated.

How the GNU behavior was established

By running the installed GNU coreutils 9.11 binary (Homebrew, gshuf) as
a black box over eight -i values — 5-1, abc, 1-, 1-2-3, -3,
the empty string, an overflowing bound, and the valid 3-3 — diffing
stderr and exit status against uutils. Seven of the eight now match. I did
not read GNU coreutils source.

Testing

  • New test_shuf_invalid_input_range_message in
    tests/by-util/test_shuf.rs: five malformed ranges with the exact
    stderr and exit status 1, plus the overflow case. Mutation-checked:
    reverting shuf.rs and the .ftl makes six tests fail.
  • cargo test --features shuf --test tests test_shuf: 75 passed, 0 failed.
  • cargo clippy -p uu_shuf --all-targets -- -D warnings: clean.
  • cargo fmt --check: clean.
  • Differential A/B against GNU coreutils 9.11 over the 15 shuf
    invocations in my harness: mismatches 1 -> 0.

Not in scope

shuf -i -3 still stops at clap with unexpected argument '-3' found
where GNU takes -3 as the value of -i. That is about hyphen handling
in the parser, not about the message, and is left alone.

Disclosure

Prepared with AI assistance (Claude Opus 5, via Claude Code), per the AI
policy in CONTRIBUTING.md. Every GNU behavior quoted above came from
running the installed binary, not from reading GPL source. All testing was
run locally.

A LO-HI that would not parse was rejected by clap, which names the option
and the reason it could not use the value:

    $ shuf -i 5-1
    error: invalid value '5-1' for '--input-range <LO-HI>': start exceeds end   # was
    shuf: invalid input range: '5-1'                                            # GNU, and now

GNU names the range as a whole whatever is wrong with it, and adds a
detail only when a bound does not fit: an overflowing bound gets
': Value too large to be stored in data type'.

parse_range moved out of the clap value parser and into uumain, returning
a UResult.
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

GNU testsuite comparison:

GNU test failed: tests/dd/misc. tests/dd/misc is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/df/over-mount-device. tests/df/over-mount-device is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test tests/cat/splice is no longer failing!
Congrats! The gnu test tests/cp/cp-a-selinux is no longer failing!
Congrats! The gnu test tests/cut/cut is no longer failing!
Congrats! The gnu test tests/dd/partial-write is no longer failing!
Congrats! The gnu test tests/ls/stat-free-symlinks is no longer failing!
Congrats! The gnu test tests/misc/close-stdout is no longer failing!
Congrats! The gnu test tests/mktemp/write-error is no longer failing!
Congrats! The gnu test tests/mv/dir2dir is no longer failing!
Congrats! The gnu test tests/nl/multibyte is no longer failing!
Congrats! The gnu test tests/od/od-float is no longer failing!
Congrats! The gnu test tests/ptx/ptx-overrun is no longer failing!
Congrats! The gnu test tests/sort/sort-merge-fdlimit is no longer failing!
Note: The gnu test tests/expand/bounded-memory is now being skipped but was previously passing.
Congrats! The gnu test tests/rm/many-dir-entries-vs-OOM is now passing!
Note: The gnu test tests/dd/fail-ftruncate-fstat was skipped on 'main' but is 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