✨ feat(CLI): Add completion for the plugins flag#5596
✨ feat(CLI): Add completion for the plugins flag#5596vitorfloriano wants to merge 1 commit intokubernetes-sigs:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: vitorfloriano The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
c6c1805 to
d9b9d71
Compare
| > [!TIP] | ||
| > To take advantage of tab completion, pass one plugin key at a time to multiple instances of the plugins flag: | ||
| > ``` | ||
| > kubebuilder init --plugins pluginA --plugins pluginB --plugins pluginC |
There was a problem hiding this comment.
The completion should work as it works the plugins
So, it should work well with --plugins=pluginA,pluginB,pluginB
Could we ensure that?
Also, do we really need this note here?
If yes, could we add the default style note instead [!TIP]?
There was a problem hiding this comment.
Could we ensure that?
I couldn't. 😅
I even opened a ticket with the Cobra folx to see if I could get some help, but their guidance was not enough (at least for me) to get this to work. See: spf13/cobra#2385
Other related issues: spf13/cobra#1963 and spf13/cobra#1981
Also, the solution the maintainer provided has been tried by other well-maintained CLIs, like gh (GitHub's CLI) and they also couldn't achieve the flag completion for comma-separated values. See: cli/cli#3628
So I believe it is indeed a shell/Cobra limitation.
If you find any Cobra-based CLIs that support completion after comma, please let me know and I'll take a look at their completion function.
Also, do we really need this note here?
If yes, could we add the default style note instead [!TIP]?
Yeah, I don't think that's really necessary. I'll remove that.
The --plugins flag now shows completion for shows all the available plugins (external ones too!).
d9b9d71 to
9eaff61
Compare
There was a problem hiding this comment.
Pull request overview
Adds dynamic shell completion for the --plugins flag in the Kubebuilder CLI to improve plugin discoverability (including external/out-of-tree plugins) while omitting deprecated plugins.
Changes:
- Registers a Cobra flag-completion function for the root
--pluginspersistent flag. - Implements
completionPluginsFlagto suggest plugin keys with descriptions, filtering already-entered and deprecated plugins. - Adds a unit test covering suggested plugins and filtering behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/cli/root.go | Registers and implements --plugins dynamic completion logic. |
| pkg/cli/cli_test.go | Adds a test for completionPluginsFlag behavior. |
| // in case the user chains the --plugins flag multiple times, | ||
| alreadyEntered, err := cmd.Flags().GetStringSlice(pluginsFlag) | ||
| if err != nil { | ||
| cobra.CheckErr(err) |
There was a problem hiding this comment.
In a completion callback, calling cobra.CheckErr on GetStringSlice error can terminate the completion run (os.Exit), which can break shell completion unexpectedly. Prefer returning (nil, cobra.ShellCompDirectiveError) or treating the error as “no already-entered values” instead of exiting.
| cobra.CheckErr(err) | |
| return nil, cobra.ShellCompDirectiveError |
|
|
||
| for pluginKey, p := range c.plugins { |
There was a problem hiding this comment.
The completion list is built by ranging over a map, so the suggestion order will be non-deterministic between runs. For a stable UX (and deterministic tests if added later), collect keys and sort them before appending to comps.
| for pluginKey, p := range c.plugins { | |
| pluginKeys := make([]string, 0, len(c.plugins)) | |
| for pluginKey := range c.plugins { | |
| pluginKeys = append(pluginKeys, pluginKey) | |
| } | |
| slices.Sort(pluginKeys) | |
| for _, pluginKey := range pluginKeys { | |
| p := c.plugins[pluginKey] |
| k2 := plugin.KeyFor(p2) | ||
|
|
||
| c.cmd = c.newRootCmd() | ||
| c.cmd.Flags().StringSlice(pluginsFlag, []string{}, "test usage") |
There was a problem hiding this comment.
newRootCmd() already defines --plugins as a PersistentFlag; re-declaring the same flag via c.cmd.Flags().StringSlice risks a duplicate-flag panic and also means the test may not be exercising the real persistent-flag behavior. Remove the extra StringSlice() call and just Set() the existing flag (or set it on PersistentFlags()).
| c.cmd.Flags().StringSlice(pluginsFlag, []string{}, "test usage") |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@vitorfloriano: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This PR adds dynamic flag completion for the
--pluginsflag.The
--pluginsflag now shows completion for all the available plugins (external ones too, including out-of-tree! 🎉), while hiding deprecated ones.This will improve plugin discoverability and usage, especially for the autoupdate plugin.
Relates to #5446 and #5291
Note
Completion does not work for comma-chained values, so users will either use another instance of
--pluginsto get completion or type everything (or copy-paste) the keys after a comma:Works (multiple instances of the flag):
Does not work (trailing comma):
This is a known limitation in Cobra. There are workarounds but they look a bit of a kludge.