From df547f5fc6ee86071f73c36c16afd885bb4e3f28 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sun, 12 Nov 2023 17:20:47 +0200 Subject: [PATCH] Fix help text for plugins When using `CommandDisplayNameAnnotation` we want to use it instead of the command name in `--help` message or in the default help command. With current code we get the wrong text in the --help usage text: Flags: -h, --help help for kubectl-plugin And in the long description of the default help command: $ kubectl cobraplugin help -h Help provides help for any command in the application. Simply type kubectl-plugin help [path to command] for full details. The issue was hidden since the test checked only the Usage line. Fixed by extracting a displayName() function and use it when creating FlagSet and when formatting the default help flag usage and the help command long description. Enhance the TestPlugin to check all the lines including the command name. --- command.go | 27 ++++++++++++++++----------- command_test.go | 13 ++++++++++++- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/command.go b/command.go index 11a3e9c..5c08c00 100644 --- a/command.go +++ b/command.go @@ -1187,10 +1187,11 @@ func (c *Command) InitDefaultHelpFlag() { c.mergePersistentFlags() if c.Flags().Lookup("help") == nil { usage := "help for " - if c.Name() == "" { + name := c.displayName() + if name == "" { usage += "this command" } else { - usage += c.Name() + usage += name } c.Flags().BoolP("help", "h", false, usage) _ = c.Flags().SetAnnotation("help", FlagSetByCobraAnnotation, []string{"true"}) @@ -1236,7 +1237,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.Name() + ` 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) @@ -1427,6 +1428,10 @@ func (c *Command) CommandPath() string { if c.HasParent() { return c.Parent().CommandPath() + " " + c.Name() } + return c.displayName() +} + +func (c *Command) displayName() string { if displayName, ok := c.Annotations[CommandDisplayNameAnnotation]; ok { return displayName } @@ -1642,7 +1647,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.Name(), flag.ContinueOnError) + c.flags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) if c.flagErrorBuf == nil { c.flagErrorBuf = new(bytes.Buffer) } @@ -1656,7 +1661,7 @@ func (c *Command) Flags() *flag.FlagSet { func (c *Command) LocalNonPersistentFlags() *flag.FlagSet { persistentFlags := c.PersistentFlags() - out := flag.NewFlagSet(c.Name(), 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) @@ -1670,7 +1675,7 @@ func (c *Command) LocalFlags() *flag.FlagSet { c.mergePersistentFlags() if c.lflags == nil { - c.lflags = flag.NewFlagSet(c.Name(), flag.ContinueOnError) + c.lflags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) if c.flagErrorBuf == nil { c.flagErrorBuf = new(bytes.Buffer) } @@ -1697,7 +1702,7 @@ func (c *Command) InheritedFlags() *flag.FlagSet { c.mergePersistentFlags() if c.iflags == nil { - c.iflags = flag.NewFlagSet(c.Name(), flag.ContinueOnError) + c.iflags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) if c.flagErrorBuf == nil { c.flagErrorBuf = new(bytes.Buffer) } @@ -1725,7 +1730,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.Name(), flag.ContinueOnError) + c.pflags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) if c.flagErrorBuf == nil { c.flagErrorBuf = new(bytes.Buffer) } @@ -1738,9 +1743,9 @@ func (c *Command) PersistentFlags() *flag.FlagSet { func (c *Command) ResetFlags() { c.flagErrorBuf = new(bytes.Buffer) c.flagErrorBuf.Reset() - c.flags = flag.NewFlagSet(c.Name(), flag.ContinueOnError) + c.flags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) c.flags.SetOutput(c.flagErrorBuf) - c.pflags = flag.NewFlagSet(c.Name(), flag.ContinueOnError) + c.pflags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) c.pflags.SetOutput(c.flagErrorBuf) c.lflags = nil @@ -1857,7 +1862,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.Name(), flag.ContinueOnError) + c.parentsPflags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) c.parentsPflags.SetOutput(c.flagErrorBuf) c.parentsPflags.SortFlags = false } diff --git a/command_test.go b/command_test.go index 9f686d6..a30c836 100644 --- a/command_test.go +++ b/command_test.go @@ -371,7 +371,7 @@ func TestAliasPrefixMatching(t *testing.T) { // text should reflect the way we run the command. func TestPlugin(t *testing.T) { rootCmd := &Command{ - Use: "plugin", + Use: "kubectl-plugin", Args: NoArgs, Annotations: map[string]string{ CommandDisplayNameAnnotation: "kubectl plugin", @@ -387,6 +387,8 @@ func TestPlugin(t *testing.T) { } checkStringContains(t, rootHelp, "kubectl plugin [command]") + checkStringContains(t, rootHelp, "help for kubectl plugin") + checkStringContains(t, rootHelp, "kubectl plugin [command] --help") childHelp, err := executeCommand(rootCmd, "sub", "-h") if err != nil { @@ -394,6 +396,15 @@ func TestPlugin(t *testing.T) { } checkStringContains(t, childHelp, "kubectl plugin sub [flags]") + checkStringContains(t, childHelp, "help for sub") + + helpHelp, err := executeCommand(rootCmd, "help", "-h") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + checkStringContains(t, helpHelp, "kubectl plugin help [path to command]") + checkStringContains(t, helpHelp, "kubectl plugin help [command]") } // TestChildSameName checks the correct behaviour of cobra in cases,