From 92013600f987104fe17048c649b83de1bc4ac9b5 Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Fri, 15 Feb 2019 17:27:22 +0100 Subject: [PATCH] Refactor plugins' vendor location on --help - The placement of the vendor is now in the end of the line. - A '*' is now added as suffix of plugins' top level commands. Signed-off-by: Ulysses Souza --- cli-plugins/manager/cobra.go | 10 ++++++++-- cli/cobra.go | 32 +++++++++++++++++++------------- cli/cobra_test.go | 26 ++++++++++++++++++-------- e2e/cli-plugins/help_test.go | 2 +- 4 files changed, 46 insertions(+), 24 deletions(-) diff --git a/cli-plugins/manager/cobra.go b/cli-plugins/manager/cobra.go index 692de7fdb1..0fcd73e7b6 100644 --- a/cli-plugins/manager/cobra.go +++ b/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 @@ -37,8 +42,9 @@ func AddPluginCommandStubs(dockerCli command.Cli, cmd *cobra.Command) error { vendor = "unknown" } annotations := map[string]string{ - CommandAnnotationPlugin: "true", - CommandAnnotationPluginVendor: vendor, + CommandAnnotationPlugin: "true", + CommandAnnotationPluginVendor: vendor, + CommandAnnotationPluginVersion: p.Version, } if p.Err != nil { annotations[CommandAnnotationPluginInvalid] = p.Err.Error() diff --git a/cli/cobra.go b/cli/cobra.go index cea1a9be4e..141ca109ae 100644 --- a/cli/cobra.go +++ b/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}} diff --git a/cli/cobra_test.go b/cli/cobra_test.go index a9d943e678..5fe8f5d98c 100644 --- a/cli/cobra_test.go +++ b/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.CommandAnnotationPluginVendor: tc.vendor, + 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*") +} diff --git a/e2e/cli-plugins/help_test.go b/e2e/cli-plugins/help_test.go index 6ecdd4c7f0..f989c78c38 100644 --- a/e2e/cli-plugins/help_test.go +++ b/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{