Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Hide the default completion command of plugins #3888

Merged

Conversation

marckhouzam
Copy link
Contributor

What this PR does / why we need it

Plugins automatically get a default completion command from Cobra. For example:

tanzu cluster completion
tanzu package completion

This commit hides this default command for any plugin through the plugin runtime library.

Notice that users enable shell completion using tanzu completion, and that this allows completion to work for plugins as well. Therefore plugins do not need to also have their own completion command. Having such a command for a plugin (e.g., tanzu cluster completion) is just confusing for users.

Which issue(s) this PR fixes

Fixes #3160

Describe testing done for PR

I built the CLI and the plugins and then ran

$ tanzu cluster -h
$ tanzu management-cluster -h
$ tanzu package -h

and confirmed there was no completion command listed.

I then ran

$ tanzu cluster completion
$ tanzu management-cluster completion
$ tanzu package completion

and confirmed the command still existed but is simply hidden.
The reasons for keeping it hidden instead of removing it is explained in the section below.

Release note

The completion command of any plugin is removed since it is not needed.

Additional information

We don't completely disable a plugin's default completion command for two reasons:

  1. backwards-compatibility, as the command used to be available for some plugins
  2. to allow to use shell completion when using the plugin as a native program (mostly for testing)

Note that a plugin can completely disable this command itself using:

plugin.Cmd.CompletionOptions.DisableDefaultCmd = true

Special notes for your reviewer

This change impacts all plugins that will be compiled with this new version of the runtime library.

Fixes vmware-tanzu#3160

To enable shell completion for all tanzu commands, including plugins, a
user makes use of `tanzu completion`.  Consequently, plugins do not need
to also have their own `completion` command.  Having such a command
for a plugin (e.g., tanzu cluster completion) is just confusing for
users.

Plugins get a `completion` command automatically from Cobra.  This
commit hides this default command for any plugin.
We don't completely disable a plugin's default completion command for
two reasons:
 1. backwards-compatibility, as the command used to be available for
    some plugins
 2. to allow to use shell completion when using the plugin as a native
    program (mostly for testing)

Note that a plugin can completely disable this command itself using:
plugin.Cmd.CompletionOptions.DisableDefaultCmd = true

Signed-off-by: Marc Khouzam <kmarc@vmware.com>
@marckhouzam
Copy link
Contributor Author

Although the code is ready I've left this PR as a draft because is like to discuss if plugin owners need to be consulted

@codecov
Copy link

codecov bot commented Nov 10, 2022

Codecov Report

Merging #3888 (43c8e8c) into main (2f9044a) will decrease coverage by 0.89%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #3888      +/-   ##
==========================================
- Coverage   46.93%   46.03%   -0.90%     
==========================================
  Files         402      427      +25     
  Lines       40203    41765    +1562     
==========================================
+ Hits        18869    19227     +358     
- Misses      19582    20768    +1186     
- Partials     1752     1770      +18     
Impacted Files Coverage Δ
cli/runtime/plugin/root.go 100.00% <100.00%> (ø)
...ons/controllers/packageinstallstatus_controller.go 77.99% <0.00%> (-1.16%) ⬇️
cmd/cli/plugin/cluster/main.go 0.00% <0.00%> (ø)
cmd/cli/plugin/cluster/kubeconfig_get.go 8.82% <0.00%> (ø)
...cluster/delete_machinehealthcheck_control_plane.go 16.66% <0.00%> (ø)
cmd/cli/plugin/cluster/delete.go 12.50% <0.00%> (ø)
cmd/cli/plugin/cluster/set_machinehealthcheck.go 23.33% <0.00%> (ø)
cmd/cli/plugin/cluster/set_node_pool.go 14.63% <0.00%> (ø)
cmd/cli/plugin/cluster/list.go 11.36% <0.00%> (ø)
cmd/cli/plugin/cluster/get_machinehealthcheck.go 11.42% <0.00%> (ø)
... and 17 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@marckhouzam marckhouzam marked this pull request as ready for review November 16, 2022 19:12
@marckhouzam marckhouzam requested a review from a team as a code owner November 16, 2022 19:12
@marckhouzam
Copy link
Contributor Author

This is ready for review

Copy link
Contributor

@anujc25 anujc25 left a comment

Choose a reason for hiding this comment

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

This looks good.

@anujc25 anujc25 added the ok-to-merge PRs should be labelled with this before merging label Nov 17, 2022
@marckhouzam marckhouzam merged commit 65e163c into vmware-tanzu:main Nov 17, 2022
@marckhouzam marckhouzam deleted the fix/hideCompletionCmdForPlugins branch November 17, 2022 14:12
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugins should hide or disable their default 'completion' command
4 participants