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

cmd: add zsh as a option for completion #9882

Merged
merged 2 commits into from Jan 21, 2020

Conversation

tonyluj
Copy link

@tonyluj tonyluj commented Jan 16, 2020

This adds zsh as a option for command completion, for zsh is also
popular and cobra natively supports zsh.

This will not change the default behavior of "completion" command.
If no or "bash" argument specified, it generates the completion for
bash.

Signed-off-by: Tony Lu tonylu@linux.alibaba.com


This change is Reviewable

@tonyluj tonyluj requested a review from a team as a code owner January 16, 2020 08:42
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.7.0 Jan 16, 2020
@coveralls
Copy link

coveralls commented Jan 16, 2020

Coverage Status

Coverage decreased (-0.009%) to 45.883% when pulling 5f90e65 on tonyluj:completion-support-zsh into 5fbf127 on cilium:master.

@rolinh rolinh added the area/cli Impacts the command line interface of any command in the repository. label Jan 16, 2020
@maintainer-s-little-helper
Copy link

Release note label not set, please set the appropriate release note.

@rolinh rolinh added the kind/feature This introduces new functionality. label Jan 16, 2020
@rolinh
Copy link
Member

rolinh commented Jan 16, 2020

Thanks for your first contribution! LGTM overall, I'd just change the call as mentioned in the comment above.

@rolinh rolinh added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Jan 16, 2020
Copy link
Member

@aanm aanm left a comment

Choose a reason for hiding this comment

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

LGTM as long @rolinh changes requested are taken into account

@tonyluj
Copy link
Author

tonyluj commented Jan 16, 2020

@rolinh @aanm
Thanks for your reviewing, I have pushed a new commit to solve error issue. I think it could be better to split it into two.

@aanm
Copy link
Member

aanm commented Jan 16, 2020

test-me-please

Copy link
Member

@aanm aanm left a comment

Choose a reason for hiding this comment

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

@tonyluj please run make -C Documentation cmdref and re-submit your changes. Thank you!

@aanm aanm added this to the 1.7 milestone Jan 20, 2020
Tony Lu added 2 commits January 21, 2020 08:09
This adds zsh as a option for command completion, for zsh is also
popular and cobra natively supports zsh.

This will not change the default behavior of "completion" command.
If no or "bash" argument specified, it generates the completion for
bash.

Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
This replaces Command.Run() with Command.RunE(), which exposes the error
to the outer scope, then the rootCmd.Execute() could catch this error
and print the error message with exit code -1.

Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
@tonyluj tonyluj requested a review from a team as a code owner January 21, 2020 08:13
@maintainer-s-little-helper
Copy link

Commit f957815adbc392884e15f26da57c70ed31d951b0 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/contributing/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jan 21, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jan 21, 2020
@tonyluj
Copy link
Author

tonyluj commented Jan 21, 2020

@aanm
I have force-pushed this patch and updated document. Thanks :-)

@aanm
Copy link
Member

aanm commented Jan 21, 2020

test-me-please

@tgraf tgraf requested a review from aanm January 21, 2020 13:31
@tgraf
Copy link
Member

tgraf commented Jan 21, 2020

test-me-please

@aanm aanm merged commit d42f4e9 into cilium:master Jan 21, 2020
1.7.0 automation moved this from In progress to Merged Jan 21, 2020
tklauser added a commit to cilium/hubble that referenced this pull request Mar 1, 2020
Add zsh as an option for hubble command completion. The default behavior
of the `completition` command is still bash completition, i.e. if no
argument or `bash` is specified.

The same was added to the `cilium` command in cilium/cilium#9882

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
tklauser added a commit to cilium/hubble that referenced this pull request Mar 1, 2020
Add zsh as an option for hubble command completion. The default behavior
of the `completition` command is still bash completition, i.e. if no
argument or `bash` is specified.

The same was added to the `cilium` command in cilium/cilium#9882

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
michi-covalent pushed a commit to cilium/hubble that referenced this pull request Mar 2, 2020
Add zsh as an option for hubble command completion. The default behavior
of the `completition` command is still bash completition, i.e. if no
argument or `bash` is specified.

The same was added to the `cilium` command in cilium/cilium#9882

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Impacts the command line interface of any command in the repository. kind/feature This introduces new functionality. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
No open projects
1.7.0
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

7 participants