uniq, split: stop reading an obsolete option after -- - #14255
Open
arbelonson-source wants to merge 1 commit into
Open
uniq, split: stop reading an obsolete option after --#14255arbelonson-source wants to merge 1 commit into
arbelonson-source wants to merge 1 commit into
Conversation
Both walk the arguments before clap to lift out their obsolete numeric
spellings — `uniq -1` for skip-fields and `split -22` for line count.
Neither stopped at `--`, so an operand that merely looked like the
obsolete form was swallowed:
$ uniq -- -1
(reads stdin) # GNU: uniq: -1: No such file or directory
$ split -- -1
(reads stdin) # GNU: split: cannot open '-1' for reading
Track the terminator and pass everything after it through untouched.
Both walkers already tracked whether the previous argument was an
option expecting a value, so this is the same kind of state.
The wording of uniq's own open failure still differs from GNU
("Could not open -1" against "-1"), but that is unrelated: plain
`uniq nosuchfile` differs the same way on an unmodified tree.
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
1 participant
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.
uniqandsplitboth walk the argument list before clap to lift out their obsolete numeric spellings —uniq -1for skip-fields,split -22for line count. Neither stopped at--, so an operand that merely looked like the obsolete form was swallowed instead of being treated as a file name.uniq -- -1uniq: -1: No such file or directorysplit -- -1split: cannot open '-1' for reading: No such file or directoryFound by differential testing against GNU coreutils 9.11.
Both walkers already threaded state for "the previous argument was an option expecting a value", so the terminator is tracked the same way: once
--is seen, everything after it is passed through untouched.This is the same class of bug as #14248 (
fold), but the three implementations are separate, so this is a separate change.Testing
One regression test each. Verified they catch the bug by reverting the two source files alone — both fail, then pass with them restored.
cargo test --features "uniq,split" --no-default-features: 162 passed, 0 failed (160 pre-existing, 2 new)cargo fmt --checkandcargo clippy -p uu_uniq -p uu_split --all-targets: cleansplit -- -1now matches GNU byte for byteNoted but not touched
The wording of uniq's own open failure still differs from GNU —
uniq: Could not open -1: No such file or directoryagainst GNU'suniq: -1: No such file or directory. That is unrelated to argument parsing: plainuniq nosuchfilediffers the same way on an unmodified tree, which I checked before writing the test, so the test asserts only that-1reaches the file layer.fmthas the same--problem (fmt -- -1reads stdin where GNU tries to open-1), but it gets there differently — through a positional withallow_negative_numbers(true)rather than a pre-clap walk — so fixing it belongs in its own change rather than being bolted on here.Disclosure
Prepared with AI assistance (Claude Code), per the AI policy in CONTRIBUTING.md. On the GPL point raised there: expected behavior was established by running the installed GNU binaries as a black box and recording their output. I did not read GNU coreutils source. All testing above was run locally.