Ignore -E/-e flag when a delimiter is specified#603
Conversation
(This fixes an incompatibility found by the Gnu findutils test suite)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #603 +/- ##
==========================================
+ Coverage 91.71% 91.74% +0.03%
==========================================
Files 31 31
Lines 6194 6205 +11
Branches 327 328 +1
==========================================
+ Hits 5681 5693 +12
+ Misses 392 391 -1
Partials 121 121 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| .stdout(predicate::str::diff("ab\n")); | ||
| } | ||
|
|
||
| cargo_bin_cmd!("xargs") |
There was a problem hiding this comment.
please create a new test for this
| } | ||
| } | ||
|
|
||
| #[allow(clippy::type_complexity)] |
There was a problem hiding this comment.
can we avoid this please? thanks
There was a problem hiding this comment.
The alternatives that come to mind are:
- Instead of returning a tuple from that function, return a new
Optionsobject. - Or pass in the
Optionsobject as amutreference and modify it.
Would either of those be a good stylistic fit for the rest of the program?
There was a problem hiding this comment.
yeah, a new data structure sounds appropriate!
There was a problem hiding this comment.
4 was probably already too much :)
There was a problem hiding this comment.
it can be removed now, no ?
| verbose: options.verbose, | ||
| eof_delimiter, | ||
| } | ||
| // (max_args, max_lines, replace, delimiter, eof_delimiter) |
There was a problem hiding this comment.
please remove this comment
| // It is possible to have multiple matches and multiple extra args, and the Cartesian product is desired. | ||
| // To be specific, we process extra args one by one, and replace all matches with the same extra arg in each time. | ||
| (Some(1), None, &options.replace) | ||
| (Some(1), None, options.replace.clone()) |
There was a problem hiding this comment.
is the clone really necessary ?
There was a problem hiding this comment.
In the update I just pushed, I changed the function so that option is moved rather than being passed by reference, which eliminates the need for the cloning of fields.
2 participants
(This fixes an incompatibility found by the Gnu findutils test suite)