feat(completion): static completion for suitable flags#686
feat(completion): static completion for suitable flags#686craicoverflow merged 4 commits intomainfrom
Conversation
craicoverflow
left a comment
craicoverflow
left a comment
There was a problem hiding this comment.
Verified as working but would like to see deduplication. Re-request my review when that is done! :)
| return cmdutil.FilterValidConsumerGroupIDs(f, toComplete) | ||
| }) | ||
|
|
||
| _ = cmd.RegisterFlagCompletionFunc("output", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { |
There was a problem hiding this comment.
Good work but this is being duplicated in every file and they are the same. Could you extract this into the flagutil package?
| cmd.Flags().StringVar(&opts.fileFormat, "file-format", "", opts.localizer.MustLocalize("serviceAccount.common.flag.fileFormat.description")) | ||
| cmd.Flags().BoolVarP(&opts.force, "yes", "y", false, opts.localizer.MustLocalize("serviceAccount.resetCredentials.flag.yes.description")) | ||
|
|
||
| _ = cmd.RegisterFlagCompletionFunc("file-format", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { |
There was a problem hiding this comment.
This is also duplicated. I would say any one with more than one occurrence can be extracted to flagutil.
rkpattnaik780
force-pushed
the
static_flag_comps
branch
from
839ec47 to
68773d7
Compare
craicoverflow
left a comment
craicoverflow
left a comment
There was a problem hiding this comment.
Great work 👍🏻
There may be more flags worth doing this for too.
| return cmdutil.FilterValidConsumerGroupIDs(f, toComplete) | ||
| }) | ||
|
|
||
| flagutil.EnableStaticFlagCompletion(cmd, "output", flagutil.ValidOutputFormats) |
There was a problem hiding this comment.
nitpick: this could be further simplified by having a specific util for "output". There is still repetition here and the values will likely always be the same for the value flag name and the valid values.
Something like flagutil.RegisterOutputFlagCompletion()
There was a problem hiding this comment.
Having a separate method for --output will indeed reduce repetition. I will suggest to have RegisterOutputFlagCompletion for output flag and RegisterOutputFlagCompletion for other flags, else we'll have to define a new method for every flag that will require static completion. What do you think?
There was a problem hiding this comment.
I don't think we should define one for every flag, but for the extremly common ones like this it is good.
There was a problem hiding this comment.
Kept the EnableStaticFlagCompletion with a separate method to enable output flag.
2 participants
Adds completions for flags such as
--outputand--file-format.Closes #682
Verification Steps
json,yamlandymlType of change
Checklist