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.

Sign up for GitHub

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

Jump to bottom

cmds/core/xargs: minimal xargs, just passing args #2782

Merged
merged 3 commits into from
Oct 21, 2023
Merged

cmds/core/xargs: minimal xargs, just passing args #2782

merged 3 commits into from
Oct 21, 2023

Conversation

binjip978
Copy link
Contributor

Parsing is simplified by strings.Fields, so no quotes support.

@binjip978
cmds/core/xargs: minimal xargs, just passing args
65982a7 
Parsing is simplified by strings.Fields, so no quotes support.

Signed-off-by: Siarhiej Siemianczuk <pdp.eleven11@gmail.com>
@codecov
Copy link

codecov bot commented Oct 14, 2023 •
edited
Loading

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Files Coverage Δ
cmds/core/xargs/xargs.go 80.00% <80.00%> (ø)

... and 5 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@binjip978 binjip978 added the Awaiting reviewer Waiting for a reviewer. label Oct 14, 2023
rminnich
rminnich reviewed Oct 15, 2023
View reviewed changes
Copy link
Member

@rminnich rminnich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interestingly, xargs with no args is cat (from experiment)

might want to make the run function take a variadic?

@rminnich rminnich added Awaiting author Waiting for new changes or feedback for author. and removed Awaiting reviewer Waiting for a reviewer. labels Oct 15, 2023
@binjip978
Copy link
Contributor Author

interestingly, xargs with no args is cat (from experiment)

Interesting because both from Linux man page executes the command (default is /bin/echo) and macOS If utility is omitted, echo(1) is used. In what system it use cat as default?

might want to make the run function take a variadic?

It can be done, but os.Args is kinda variadic already. So maybe I missing how it will improve any scenario?

@binjip978 binjip978 added Awaiting reviewer Waiting for a reviewer. and removed Awaiting author Waiting for new changes or feedback for author. labels Oct 15, 2023
rminnich
rminnich reviewed Oct 20, 2023
View reviewed changes
cmds/core/xargs/xargs.go Outdated Show resolved Hide resolved
@rminnich rminnich added the Awaiting author Waiting for new changes or feedback for author. label Oct 20, 2023
rminnich
rminnich previously approved these changes Oct 20, 2023
View reviewed changes
Copy link
Member

@rminnich rminnich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my comment is really not that important, I leave the decision to you.

@rminnich rminnich removed the Awaiting reviewer Waiting for a reviewer. label Oct 20, 2023
@binjip978
cmds/core/xargs: make run variadic
8ec906f 
Signed-off-by: Siarhiej Siemianczuk <pdp.eleven11@gmail.com>
@binjip978 binjip978 added Awaiting reviewer Waiting for a reviewer. and removed Awaiting author Waiting for new changes or feedback for author. labels Oct 21, 2023
@binjip978 binjip978 merged commit a97b21d into u-root:main Oct 21, 2023
24 checks passed
@binjip978 binjip978 deleted the xargs branch October 21, 2023 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rminnich rminnich rminnich approved these changes

Assignees
No one assigned
Labels
Awaiting reviewer Waiting for a reviewer.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@binjip978 @rminnich