grep: expand bundled numeric context options - #76
Conversation
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
|
||
| let mut out: Vec<OsString> = args.collect(); | ||
| let mut i = 1; // argv[0] is the executable name | ||
| let mut args = args; |
| // A digit embedded in a short-option cluster acts as a single `-NUM` | ||
| // option at that point in the cluster. Emitting each option separately | ||
| // preserves left-to-right override semantics. | ||
| let mut i = 0; |
There was a problem hiding this comment.
can it be moved into a function?
lhecker
left a comment
lhecker
left a comment
There was a problem hiding this comment.
I'm not entirely sure if I agree with the way this PR is written. It removes the Vec collect() optimization (os_args() returns a Vec IntoIterator), adds full UTF8 validation (is that necessary?), and is overall a lot more verbose than the old code.
I haven't thought it through, and so I may be mistaken, but I intuitively feel like the issue should be moderately easy to solve within the old code's structure.
| // Only the first value option in a cluster takes a value, so the trailing | ||
| // `e` is `-e`'s value here and the following `-2` is still a shorthand. | ||
| ok(&["-ee", "-2"], "a\nbee\nc\nd\n", "a\nbee\nc\nd\n"); |
There was a problem hiding this comment.
This new test case is what causes most of the changes. It requires us to forward-scan the -shortoption string and scan for those that take an argument.
3 participants
Summary
--arguments, and non-UTF-8 arguments unchangedThe implementation is based only on the behavior specified in issue #68 and the original uutils grep code. No GNU source code was consulted or used.
Verification
cargo clippy --all-targets --all-features -- -D warningscargo fmt --all -- --checkgit diff --checkCloses #68