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

Use Cobra to provide all shell completions #199

Closed
wants to merge 1 commit into from

Conversation

marckhouzam
Copy link
Member

@marckhouzam marckhouzam commented Feb 2, 2022

This comes a little late since this project is pretty stable but I have been working on making this happen for 18 months (spf13/cobra#1161), so here it is anyway.

Cobra automatically provides support for flag and command completion. By using helm's dynamic completion for plugins, we can use Cobra's support in a plugin.complete file instead of coding things ourselves in completion.yaml.

This removes the need to keep the now removed completion.yaml up to date with any new command or flag.

This change requires the use of Helm 3.8 that uses Cobra 1.3.
So, this change will cause a regression to shell completion for older versions of helm, so I'm not sure if we want the change or not, but I wanted to at least suggest it.

Furthermore, with this change, adding shell completions for release names and such in helm-2to3 would then automatically work when used as a plugin.

Cobra provides support for flag and command completion.  By using
helm's dynamic completion for plugins, we can use Cobra's support
instead of coding things ourselves.

Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>
@helm-bot helm-bot added the size/M label Feb 2, 2022
Copy link
Collaborator

@hickeyma hickeyma left a comment

Choose a reason for hiding this comment

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

This change requires the use of Helm 3.8 that uses Cobra 1.3.

Hmm, that might be an issue alright. Or is it better that the plugin is using the latest Helm anyways. I suppose we could add a note to the README.

What do you think @marckhouzam

@marckhouzam
Copy link
Member Author

Bah, we should probably not merge this since the value-add is smaller than the regression for older helm version.

I wanted to at least post it as our could be a reference for other plugins.

@hickeyma
Copy link
Collaborator

Ok, thanks for the feedback @marckhouzam. I will close it out as not adding this feature.

@hickeyma hickeyma closed this Aug 18, 2022
@hickeyma hickeyma added the invalid This doesn't seem right label Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants