✨ feat(cli): mark resource flags as required to improve completion#5647
Conversation
| var options *resourceOptions | ||
| if requiresResource { | ||
| options = bindResourceFlags(cmd.Flags()) | ||
| cobra.CheckErr(cmd.MarkFlagRequired("group")) |
There was a problem hiding this comment.
I think there might be an edge case where group can be nil for core types—could we add a check for that?
That said, I really like this idea. It should help us enforce consistency across the whole project. 🎉
There was a problem hiding this comment.
We could add a check IF the validation occurred before flag binding, which is not the case. We mark the flags as required in the initialization hook, and only then the plugin validates the input.
So I don't think it is possible...can't think of a way to make this happen.
But, MarkFlagRequired() does not validate the input, it only check if it is present, so --group "" would still work fine.
|
Unknown CLA label state. Rechecking for CLA labels. Send feedback to sig-contributor-experience at kubernetes/community. /check-cla |
There was a problem hiding this comment.
Pull request overview
This PR aims to improve kubebuilder CLI UX by making the resource identity flags (--group, --version, --kind) show up as required in shell completion for commands that scaffold resources (notably create api and create webhook), preventing default completion from listing files in the current directory.
Changes:
- Marks
group,version, andkindflags as required for commands whose plugins implementplugin.RequiresResource.
| ) ([]cobra.Completion, cobra.ShellCompDirective) { | ||
| completions := []cobra.Completion{} | ||
| if len(args) == 0 && toComplete == "" { | ||
| completions = cobra.AppendActiveHelp(completions, "Type '--' and press TAB to list more flags") |
There was a problem hiding this comment.
@camilamacedo86 I removed the group flag requirement and added ActiveHelp to hint users about other flags. It's going to work like this:
Only version and kind are marked as required, so completion shows them proactively:
$ kubebuilder create api<TAB>
--kind (Resource Kind (e.g., CronJob, Deployment)) --version (Resource Version (e.g., v1, v1beta1))
After the two flags are passed, tab completion shows a hint at other flags (Cobra's ActiveHelp):
$ kubebuilder create api --kind MyKind --version v1alpha
Type '--' and press TAB to list more flags
I'm not 100% happy with the hint message, though. We can change that.
| if err := cmd.MarkFlagRequired("version"); err != nil { | ||
| return nil, fmt.Errorf("failed to mark 'version' flag as required: %w", err) | ||
| } | ||
| if err := cmd.MarkFlagRequired("kind"); err != nil { | ||
| return nil, fmt.Errorf("failed to mark 'kind' flag as required: %w", err) | ||
| } |
There was a problem hiding this comment.
Also added proper err check, as pointed by @copilot.
| */ | ||
|
|
||
| //nolint:dupl | ||
| package cli |
There was a problem hiding this comment.
We need to fix the lint issue here, run make lint-fix to check it out.
The kind and version are now required.This makes completion suggest those flags when completing the create api and create webhooks commands. Just a nice little improvement in UX.
f57f1f3 to
c896d0d
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, vitorfloriano The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This PR mark the
kindandversionas required.This makes tab completion suggest those flags when completing the
create apiandcreate webhookscommands, instead of showing the files in the current directory.👎🏻 Current behavior:
👍🏻 New behavior:
Edit: Only
versionandkindare marked as required, so completion shows them proactively.After the two flags are passed, tab completion shows a hint at other flags (Cobra's ActiveHelp):
Just a nice little polish in UX. 💅🏻
Relates to #5446