From 116759b688c422e47f5a6ead5d388f31ba4cad39 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sat, 24 Aug 2024 06:00:05 +0300 Subject: [PATCH 1/3] Fix --version help with CommandDisplayNameAnnotation When setting Command.Version, a --version option is added. The help message for the --version command did not consider the command display name: Flags: -h, --help help for kubectl plugin -v, --version version for kubectl-plugin With this change the help test is consistent with other flags: Flags: -h, --help help for kubectl plugin -v, --version version for kubectl plugin --- command.go | 2 +- command_test.go | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/command.go b/command.go index 2df6975f9..a8bda03be 100644 --- a/command.go +++ b/command.go @@ -1216,7 +1216,7 @@ func (c *Command) InitDefaultVersionFlag() { if c.Name() == "" { usage += "this command" } else { - usage += c.Name() + usage += c.displayName() } if c.Flags().ShorthandLookup("v") == nil { c.Flags().BoolP("version", "v", false, usage) diff --git a/command_test.go b/command_test.go index 9ce7a529b..caf8c121a 100644 --- a/command_test.go +++ b/command_test.go @@ -371,8 +371,9 @@ func TestAliasPrefixMatching(t *testing.T) { // text should reflect the way we run the command. func TestPlugin(t *testing.T) { cmd := &Command{ - Use: "kubectl-plugin", - Args: NoArgs, + Use: "kubectl-plugin", + Version: "1.0.0", + Args: NoArgs, Annotations: map[string]string{ CommandDisplayNameAnnotation: "kubectl plugin", }, @@ -386,13 +387,15 @@ func TestPlugin(t *testing.T) { checkStringContains(t, cmdHelp, "kubectl plugin [flags]") checkStringContains(t, cmdHelp, "help for kubectl plugin") + checkStringContains(t, cmdHelp, "version for kubectl plugin") } // TestPlugin checks usage as plugin with sub commands. func TestPluginWithSubCommands(t *testing.T) { rootCmd := &Command{ - Use: "kubectl-plugin", - Args: NoArgs, + Use: "kubectl-plugin", + Version: "1.0.0", + Args: NoArgs, Annotations: map[string]string{ CommandDisplayNameAnnotation: "kubectl plugin", }, @@ -408,6 +411,7 @@ func TestPluginWithSubCommands(t *testing.T) { checkStringContains(t, rootHelp, "kubectl plugin [command]") checkStringContains(t, rootHelp, "help for kubectl plugin") + checkStringContains(t, rootHelp, "version for kubectl plugin") checkStringContains(t, rootHelp, "kubectl plugin [command] --help") childHelp, err := executeCommand(rootCmd, "sub", "-h") From 0eb136226a999add8e8b18e99c0db79b6da727fd Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sat, 24 Aug 2024 06:14:14 +0300 Subject: [PATCH 2/3] Make command DisplayName() public This allows using the display name in templates or other code that want to use the same value. --- command.go | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/command.go b/command.go index a8bda03be..f570d1e56 100644 --- a/command.go +++ b/command.go @@ -1190,7 +1190,7 @@ func (c *Command) InitDefaultHelpFlag() { c.mergePersistentFlags() if c.Flags().Lookup("help") == nil { usage := "help for " - name := c.displayName() + name := c.DisplayName() if name == "" { usage += "this command" } else { @@ -1216,7 +1216,7 @@ func (c *Command) InitDefaultVersionFlag() { if c.Name() == "" { usage += "this command" } else { - usage += c.displayName() + usage += c.DisplayName() } if c.Flags().ShorthandLookup("v") == nil { c.Flags().BoolP("version", "v", false, usage) @@ -1240,7 +1240,7 @@ func (c *Command) InitDefaultHelpCmd() { Use: "help [command]", Short: "Help about any command", Long: `Help provides help for any command in the application. -Simply type ` + c.displayName() + ` help [path to command] for full details.`, +Simply type ` + c.DisplayName() + ` help [path to command] for full details.`, ValidArgsFunction: func(c *Command, args []string, toComplete string) ([]string, ShellCompDirective) { var completions []string cmd, _, e := c.Root().Find(args) @@ -1431,10 +1431,12 @@ func (c *Command) CommandPath() string { if c.HasParent() { return c.Parent().CommandPath() + " " + c.Name() } - return c.displayName() + return c.DisplayName() } -func (c *Command) displayName() string { +// DisplayName returns the name to display in help text. Returns command Name() +// If CommandDisplayNameAnnoation is not set +func (c *Command) DisplayName() string { if displayName, ok := c.Annotations[CommandDisplayNameAnnotation]; ok { return displayName } @@ -1444,7 +1446,7 @@ func (c *Command) displayName() string { // UseLine puts out the full usage for a given command (including parents). func (c *Command) UseLine() string { var useline string - use := strings.Replace(c.Use, c.Name(), c.displayName(), 1) + use := strings.Replace(c.Use, c.Name(), c.DisplayName(), 1) if c.HasParent() { useline = c.parent.CommandPath() + " " + use } else { @@ -1650,7 +1652,7 @@ func (c *Command) GlobalNormalizationFunc() func(f *flag.FlagSet, name string) f // to this command (local and persistent declared here and by all parents). func (c *Command) Flags() *flag.FlagSet { if c.flags == nil { - c.flags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) + c.flags = flag.NewFlagSet(c.DisplayName(), flag.ContinueOnError) if c.flagErrorBuf == nil { c.flagErrorBuf = new(bytes.Buffer) } @@ -1665,7 +1667,7 @@ func (c *Command) Flags() *flag.FlagSet { func (c *Command) LocalNonPersistentFlags() *flag.FlagSet { persistentFlags := c.PersistentFlags() - out := flag.NewFlagSet(c.displayName(), flag.ContinueOnError) + out := flag.NewFlagSet(c.DisplayName(), flag.ContinueOnError) c.LocalFlags().VisitAll(func(f *flag.Flag) { if persistentFlags.Lookup(f.Name) == nil { out.AddFlag(f) @@ -1680,7 +1682,7 @@ func (c *Command) LocalFlags() *flag.FlagSet { c.mergePersistentFlags() if c.lflags == nil { - c.lflags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) + c.lflags = flag.NewFlagSet(c.DisplayName(), flag.ContinueOnError) if c.flagErrorBuf == nil { c.flagErrorBuf = new(bytes.Buffer) } @@ -1708,7 +1710,7 @@ func (c *Command) InheritedFlags() *flag.FlagSet { c.mergePersistentFlags() if c.iflags == nil { - c.iflags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) + c.iflags = flag.NewFlagSet(c.DisplayName(), flag.ContinueOnError) if c.flagErrorBuf == nil { c.flagErrorBuf = new(bytes.Buffer) } @@ -1737,7 +1739,7 @@ func (c *Command) NonInheritedFlags() *flag.FlagSet { // PersistentFlags returns the persistent FlagSet specifically set in the current command. func (c *Command) PersistentFlags() *flag.FlagSet { if c.pflags == nil { - c.pflags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) + c.pflags = flag.NewFlagSet(c.DisplayName(), flag.ContinueOnError) if c.flagErrorBuf == nil { c.flagErrorBuf = new(bytes.Buffer) } @@ -1750,9 +1752,9 @@ func (c *Command) PersistentFlags() *flag.FlagSet { func (c *Command) ResetFlags() { c.flagErrorBuf = new(bytes.Buffer) c.flagErrorBuf.Reset() - c.flags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) + c.flags = flag.NewFlagSet(c.DisplayName(), flag.ContinueOnError) c.flags.SetOutput(c.flagErrorBuf) - c.pflags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) + c.pflags = flag.NewFlagSet(c.DisplayName(), flag.ContinueOnError) c.pflags.SetOutput(c.flagErrorBuf) c.lflags = nil @@ -1869,7 +1871,7 @@ func (c *Command) mergePersistentFlags() { // If c.parentsPflags == nil, it makes new. func (c *Command) updateParentsPflags() { if c.parentsPflags == nil { - c.parentsPflags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) + c.parentsPflags = flag.NewFlagSet(c.DisplayName(), flag.ContinueOnError) c.parentsPflags.SetOutput(c.flagErrorBuf) c.parentsPflags.SortFlags = false } From 555755133d478529df62ba3dd43d231581bd3124 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sat, 24 Aug 2024 06:23:29 +0300 Subject: [PATCH 3/3] Use display name in version template The version template used `{{.Name}}` but for plugins you want to use `{{.DisplayName}}` to be consistent with other help output. With this change will show: $ kubectl plugin --version kubectl plugin version 1.0.0 This may cause issues for programs processing the output if the program assumes that the name of the program never contains spaces, and the version string is the third word. Users can use their own template if they think that this is an issue. If we think that this can break users we can drop this change and let users opt in by setting their own template using `{{.DisplayName}}`. --- command.go | 2 +- command_test.go | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/command.go b/command.go index f570d1e56..4cd712bcb 100644 --- a/command.go +++ b/command.go @@ -607,7 +607,7 @@ func (c *Command) VersionTemplate() string { if c.HasParent() { return c.parent.VersionTemplate() } - return `{{with .Name}}{{printf "%s " .}}{{end}}{{printf "version %s" .Version}} + return `{{with .DisplayName}}{{printf "%s " .}}{{end}}{{printf "version %s" .Version}} ` } diff --git a/command_test.go b/command_test.go index caf8c121a..cd44992bd 100644 --- a/command_test.go +++ b/command_test.go @@ -1094,6 +1094,24 @@ func TestVersionFlagExecuted(t *testing.T) { checkStringContains(t, output, "root version 1.0.0") } +func TestVersionFlagExecutedDiplayName(t *testing.T) { + rootCmd := &Command{ + Use: "kubectl-plugin", + Version: "1.0.0", + Annotations: map[string]string{ + CommandDisplayNameAnnotation: "kubectl plugin", + }, + Run: emptyRun, + } + + output, err := executeCommand(rootCmd, "--version", "arg1") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + checkStringContains(t, output, "kubectl plugin version 1.0.0") +} + func TestVersionFlagExecutedWithNoName(t *testing.T) { rootCmd := &Command{Version: "1.0.0", Run: emptyRun}