Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xargs: do not merge extra args before replacing #363

Merged
merged 4 commits into from
Jun 23, 2024

Conversation

YDX-2147483647
Copy link
Contributor

@YDX-2147483647 YDX-2147483647 commented Apr 28, 2024

Resolves #362

  • If -I/-i/--replace is given, now we process extra args (read from stdin) one by one, and replace all matches in initial args (given in argv) with the same extra arg in each time. (i.e. Cartesian product)
# Before
$ printf 'a    bc\ndef\ng' | xargs -I _ echo '[_]'
[a bc def g]

# After
$ printf 'a    bc\ndef\ng' | cargo run --bin xargs -- -I _ echo '[_]'
[a    bc]
[def]
[g]
  • -L (--max-lines), -n (--max-args) and -I/-i are mutually exclusive. Now we warn if more than one are given.

@YDX-2147483647 YDX-2147483647 marked this pull request as draft April 28, 2024 13:22
Copy link

codecov bot commented Apr 28, 2024

Codecov Report

Attention: Patch coverage is 91.78082% with 6 lines in your changes missing coverage. Please review.

Project coverage is 60.48%. Comparing base (a2e350c) to head (9bc80cd).

Files Patch % Lines
src/xargs/mod.rs 86.66% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #363      +/-   ##
==========================================
+ Coverage   60.03%   60.48%   +0.44%     
==========================================
  Files          32       32              
  Lines        4021     4069      +48     
  Branches      891      895       +4     
==========================================
+ Hits         2414     2461      +47     
  Misses       1254     1254              
- Partials      353      354       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@YDX-2147483647 YDX-2147483647 force-pushed the xargs-replace branch 7 times, most recently from 458fab4 to 31a9c11 Compare April 28, 2024 16:34
@YDX-2147483647

This comment was marked as resolved.

@YDX-2147483647 YDX-2147483647 marked this pull request as ready for review April 29, 2024 01:46
@YDX-2147483647

This comment was marked as resolved.

@sylvestre
Copy link
Sponsor Contributor

@YDX-2147483647 sorry, i missed your question :(
you should do the same as the GNU implementation

@YDX-2147483647
Copy link
Contributor Author

YDX-2147483647 commented May 16, 2024

Never mind and thanks for your answer.

So those 100+ lines is worth changing. I'll update the branch and add more tests when I have time.

@YDX-2147483647
Copy link
Contributor Author

Well, I've updated the branch. Could someone approve the CI? I think this PR is ready now.

@sylvestre
Copy link
Sponsor Contributor

it it ready for review? :)
thanks

@YDX-2147483647
Copy link
Contributor Author

Yes! It is ready.

@sylvestre
Copy link
Sponsor Contributor

nice:
Warning: Changes from main: PASS +4 / FAIL -4 / ERROR +0 / SKIP +0

@sylvestre sylvestre merged commit 84e4be8 into uutils:main Jun 23, 2024
18 checks passed
@YDX-2147483647 YDX-2147483647 deleted the xargs-replace branch June 23, 2024 18:06
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.

xargs: stdin should not be merged before replacing
2 participants