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

etcdctl: add command to generate shell completion #13133

Merged
merged 2 commits into from
Jun 24, 2021

Conversation

avorima
Copy link
Contributor

@avorima avorima commented Jun 22, 2021

To improve the UX of etcdctl. Completion is generated by cobra
according to defined commands and flags.

Fixes #13111

To improve the UX of etcdctl. Completion is generated by cobra
according to defined commands and flags.

Fixes etcd-io#13111
@ptabor
Copy link
Contributor

ptabor commented Jun 23, 2021

Nice. Could you please add an tests/e2e/ctl_v3_*.go test to cover the command is working (even on individual example).

Copy link
Contributor

@lilic lilic left a comment

Choose a reason for hiding this comment

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

Thanks! Works like a charm 🎉 👍

Would you mind adding it for etcdutl in a follow up maybe?

@ptabor should we add this to the release notes as well what do you think?

@ptabor
Copy link
Contributor

ptabor commented Jun 23, 2021

@ptabor +1 for release notes. The notable first entry for 3.6.

@avorima
Copy link
Contributor Author

avorima commented Jun 23, 2021

Nice. Could you please add an tests/e2e/ctl_v3_*.go test to cover the command is working (even on individual example).

I'll look into writing tests. Do you mean one for one shell only?

Would you mind adding it for etcdutl in a follow up maybe?

Sure, I can do that.

@ptabor
Copy link
Contributor

ptabor commented Jun 23, 2021

Nice. Could you please add an tests/e2e/ctl_v3_*.go test to cover the command is working (even on individual example).

I'll look into writing tests. Do you mean one for one shell only?

Yes - mostly to make sure someone didn't turned off the group incidentally.

Would you mind adding it for etcdutl in a follow up maybe?

Sure, I can do that.

@avorima avorima force-pushed the shell-completion branch from a609d35 to 96b8049 Compare June 23, 2021 21:16
@avorima
Copy link
Contributor Author

avorima commented Jun 23, 2021

@ptabor I've added a really basic test that verifies the generated shell code is valid. Testing the completion itself is kind of tricky and probably not feasible in a non-interactive environment. I could add some test cases for the hidden __complete command that the shell code uses though.

The test is for bash, but can easily be extended for zsh and fish. They just have to be installed in CI.

Copy link
Contributor

@ptabor ptabor left a comment

Choose a reason for hiding this comment

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

Great . Thank you.

Copy link
Contributor

@lilic lilic left a comment

Choose a reason for hiding this comment

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

Thank you! 🎉

@ptabor ptabor merged commit 69fadd4 into etcd-io:main Jun 24, 2021
avorima added a commit to avorima/etcd that referenced this pull request Jun 24, 2021
@avorima avorima deleted the shell-completion branch June 24, 2021 14:40
wilsonwang371 pushed a commit to wilsonwang371/etcd that referenced this pull request Oct 29, 2021
wilsonwang371 pushed a commit to wilsonwang371/etcd that referenced this pull request Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

etcdctl: Provide shell completion
3 participants