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

Disabling all arguments after foreach #51

Merged
merged 3 commits into from Sep 8, 2021
Merged

Disabling all arguments after foreach #51

merged 3 commits into from Sep 8, 2021

Conversation

sledigabel
Copy link
Contributor

Fixes #50.
Uses DisableFlagParsing as per the documentation to disable the
subsequent flag parsing in the command and leaving it as arguments to
the shell command.

Added TraverseChildren to rootCmd to allow -v and -h to be parsed.

https://pkg.go.dev/github.com/spf13/cobra#Command

Tested on Mac OS.

I'm unable to write tests as we're not testing the flag parsing at the
moment.

Fixes #50.
Uses `DisableFlagParsing` as per the documentation to disable the
subsequent flag parsing in the command and leaving it as arguments to
the shell command.

Added `TraverseChildren` to rootCmd to allow -v and -h to be parsed.

https://pkg.go.dev/github.com/spf13/cobra#Command

Tested on Mac OS.

I'm unable to write tests as we're not testing the flag parsing at the
moment.
@sledigabel
Copy link
Contributor Author

image

Copy link
Contributor

@DiogoMCampos DiogoMCampos left a comment

Choose a reason for hiding this comment

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

LGTM! Nice that you got the TraverseChildren one in too! 👍

@sledigabel
Copy link
Contributor Author

I'm gonna hold off the merge for now, I'd really to figure out how docker did it and make sure we can come back and implement flags for the foreach in the future without too much work.

@rnorth
Copy link
Collaborator

rnorth commented Sep 7, 2021

@sledigabel do you want to discuss this further?

@sledigabel
Copy link
Contributor Author

About to merge this; we'll get particular arguments for foreach as a special case, outside of cobra.

@sledigabel sledigabel merged commit e2183ea into main Sep 8, 2021
sledigabel added a commit that referenced this pull request Oct 7, 2022
the `--` were needed for any command including parameters prior to #51.
After #51, the `--` are no longer needed a cobra isn't trying to parse
the full command. It's also failing the run now if the `--` are
included since they are interpreted as part of the command.

This commit fixes the documentation associated with `foreach`.
sledigabel added a commit that referenced this pull request Oct 12, 2022
the `--` were needed for any command including parameters prior to #51.
After #51, the `--` are no longer needed a cobra isn't trying to parse
the full command. It's also failing the run now if the `--` are
included since they are interpreted as part of the command.

This commit fixes the documentation associated with `foreach`.
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.

foreach sed fails with 'unknown shorthand flag' error
3 participants