Merge pull request #1675 from ulyssessouza/format-plugin-vendor-version-help

Reformat plugin's vendor position and add version on --help

cli-plugins/manager/cobra.go

@@ -16,6 +16,11 @@ const (
 	// that plugin.
 	CommandAnnotationPluginVendor = "com.docker.cli.plugin.vendor"
 
+	// CommandAnnotationPluginVersion is added to every stub command
+	// added by AddPluginCommandStubs and contains the version of
+	// that plugin.
+	CommandAnnotationPluginVersion = "com.docker.cli.plugin.version"
+
 	// CommandAnnotationPluginInvalid is added to any stub command
 	// added by AddPluginCommandStubs for an invalid command (that
 	// is, one which failed it's candidate test) and contains the
@@ -39,6 +44,7 @@ func AddPluginCommandStubs(dockerCli command.Cli, cmd *cobra.Command) error {
 		annotations := map[string]string{
 			CommandAnnotationPlugin:        "true",
 			CommandAnnotationPluginVendor:  vendor,
+			CommandAnnotationPluginVersion: p.Version,
 		}
 		if p.Err != nil {
 			annotations[CommandAnnotationPluginInvalid] = p.Err.Error()

cli/cobra.go

@@ -22,6 +22,7 @@ func setupCommonRootCommand(rootCmd *cobra.Command) (*cliflags.ClientOptions, *p
 	flags.StringVar(&opts.ConfigDir, "config", cliconfig.Dir(), "Location of client config files")
 	opts.Common.InstallFlags(flags)
 
+	cobra.AddTemplateFunc("add", func(a, b int) int { return a + b })
 	cobra.AddTemplateFunc("hasSubCommands", hasSubCommands)
 	cobra.AddTemplateFunc("hasManagementSubCommands", hasManagementSubCommands)
 	cobra.AddTemplateFunc("hasInvalidPlugins", hasInvalidPlugins)
@@ -29,9 +30,10 @@ func setupCommonRootCommand(rootCmd *cobra.Command) (*cliflags.ClientOptions, *p
 	cobra.AddTemplateFunc("managementSubCommands", managementSubCommands)
 	cobra.AddTemplateFunc("invalidPlugins", invalidPlugins)
 	cobra.AddTemplateFunc("wrappedFlagUsages", wrappedFlagUsages)
-	cobra.AddTemplateFunc("commandVendor", commandVendor)
-	cobra.AddTemplateFunc("isFirstLevelCommand", isFirstLevelCommand) // is it an immediate sub-command of the root
+	cobra.AddTemplateFunc("vendorAndVersion", vendorAndVersion)
 	cobra.AddTemplateFunc("invalidPluginReason", invalidPluginReason)
+	cobra.AddTemplateFunc("isPlugin", isPlugin)
+	cobra.AddTemplateFunc("decoratedName", decoratedName)
 
 	rootCmd.SetUsageTemplate(usageTemplate)
 	rootCmd.SetHelpTemplate(helpTemplate)
@@ -155,19 +157,23 @@ func wrappedFlagUsages(cmd *cobra.Command) string {
 	return cmd.Flags().FlagUsagesWrapped(width - 1)
 }
 
-func isFirstLevelCommand(cmd *cobra.Command) bool {
-	return cmd.Parent() == cmd.Root()
+func decoratedName(cmd *cobra.Command) string {
+	decoration := " "
+	if isPlugin(cmd) {
+		decoration = "*"
+	}
+	return cmd.Name() + decoration
 }
 
-func commandVendor(cmd *cobra.Command) string {
-	width := 13
-	if v, ok := cmd.Annotations[pluginmanager.CommandAnnotationPluginVendor]; ok {
-		if len(v) > width-2 {
-			v = v[:width-3] + "…"
+func vendorAndVersion(cmd *cobra.Command) string {
+	if vendor, ok := cmd.Annotations[pluginmanager.CommandAnnotationPluginVendor]; ok && isPlugin(cmd) {
+		version := ""
+		if v, ok := cmd.Annotations[pluginmanager.CommandAnnotationPluginVersion]; ok && v != "" {
+			version = ", " + v
 		}
-		return fmt.Sprintf("%-*s", width, "("+v+")")
+		return fmt.Sprintf("(%s%s)", vendor, version)
 	}
-	return strings.Repeat(" ", width)
+	return ""
 }
 
 func managementSubCommands(cmd *cobra.Command) []*cobra.Command {
@@ -230,7 +236,7 @@ Options:
 Management Commands:
 
 {{- range managementSubCommands . }}
-  {{rpad .Name .NamePadding }} {{ if isFirstLevelCommand .}}{{commandVendor .}} {{ end}}{{.Short}}
+  {{rpad (decoratedName .) (add .NamePadding 1)}}{{.Short}}{{ if isPlugin .}} {{vendorAndVersion .}}{{ end}}
 {{- end}}
 
 {{- end}}
@@ -239,7 +245,7 @@ Management Commands:
 Commands:
 
 {{- range operationSubCommands . }}
-  {{rpad .Name .NamePadding }} {{ if isFirstLevelCommand .}}{{commandVendor .}} {{ end}}{{.Short}}
+  {{rpad (decoratedName .) (add .NamePadding 1)}}{{.Short}}{{ if isPlugin .}} {{vendorAndVersion .}}{{ end}}
 {{- end}}
 {{- end}}
 

cli/cobra_test.go

@@ -32,28 +32,29 @@ func TestVisitAll(t *testing.T) {
 	assert.DeepEqual(t, expected, visited)
 }
 
-func TestCommandVendor(t *testing.T) {
+func TestVendorAndVersion(t *testing.T) {
 	// Non plugin.
-	assert.Equal(t, commandVendor(&cobra.Command{Use: "test"}), "             ")
+	assert.Equal(t, vendorAndVersion(&cobra.Command{Use: "test"}), "")
 
 	// Plugins with various lengths of vendor.
 	for _, tc := range []struct {
 		vendor   string
+		version  string
 		expected string
 	}{
-		{vendor: "vendor", expected: "(vendor)     "},
-		{vendor: "vendor12345", expected: "(vendor12345)"},
-		{vendor: "vendor123456", expected: "(vendor1234…)"},
-		{vendor: "vendor1234567", expected: "(vendor1234…)"},
+		{vendor: "vendor", expected: "(vendor)"},
+		{vendor: "vendor", version: "testing", expected: "(vendor, testing)"},
 	} {
 		t.Run(tc.vendor, func(t *testing.T) {
 			cmd := &cobra.Command{
 				Use: "test",
 				Annotations: map[string]string{
+					pluginmanager.CommandAnnotationPlugin:        "true",
 					pluginmanager.CommandAnnotationPluginVendor:  tc.vendor,
+					pluginmanager.CommandAnnotationPluginVersion: tc.version,
 				},
 			}
-			assert.Equal(t, commandVendor(cmd), tc.expected)
+			assert.Equal(t, vendorAndVersion(cmd), tc.expected)
 		})
 	}
 }
@@ -76,3 +77,12 @@ func TestInvalidPlugin(t *testing.T) {
 
 	assert.DeepEqual(t, invalidPlugins(root), []*cobra.Command{sub1}, cmpopts.IgnoreUnexported(cobra.Command{}))
 }
+
+func TestDecoratedName(t *testing.T) {
+	root := &cobra.Command{Use: "root"}
+	topLevelCommand := &cobra.Command{Use: "pluginTopLevelCommand"}
+	root.AddCommand(topLevelCommand)
+	assert.Equal(t, decoratedName(topLevelCommand), "pluginTopLevelCommand ")
+	topLevelCommand.Annotations = map[string]string{pluginmanager.CommandAnnotationPlugin: "true"}
+	assert.Equal(t, decoratedName(topLevelCommand), "pluginTopLevelCommand*")
+}

e2e/cli-plugins/help_test.go

@@ -34,7 +34,7 @@ func TestGlobalHelp(t *testing.T) {
 	//  - The `badmeta` plugin under the "Invalid Plugins" heading.
 	//
 	// Regexps are needed because the width depends on `unix.TIOCGWINSZ` or similar.
-	helloworldre := regexp.MustCompile(`^  helloworld\s+\(Docker Inc\.\)\s+A basic Hello World plugin for tests$`)
+	helloworldre := regexp.MustCompile(`^  helloworld\*\s+A basic Hello World plugin for tests \(Docker Inc\., testing\)$`)
 	badmetare := regexp.MustCompile(`^  badmeta\s+invalid metadata: invalid character 'i' looking for beginning of object key string$`)
 	var helloworldcount, badmetacount int
 	for _, expected := range []*regexp.Regexp{