Merge branch 'main' into completion

.github/workflows/test.yml

@@ -18,7 +18,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
 
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - run: >-
           docker run
@@ -39,15 +39,15 @@ jobs:
     runs-on: ubuntu-latest
     steps:
 
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
-      - uses: actions/setup-go@v3
+      - uses: actions/setup-go@v4
         with:
           go-version: '^1.21'
           check-latest: true
           cache: true
 
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - uses: golangci/[email protected]
         with:
@@ -72,9 +72,9 @@ jobs:
     runs-on: ${{ matrix.platform }}-latest
     steps:
 
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4
 
-    - uses: actions/setup-go@v3
+    - uses: actions/setup-go@v4
       with:
         go-version: 1.${{ matrix.go }}.x
         cache: true
@@ -108,7 +108,7 @@ jobs:
           unzip
           mingw-w64-x86_64-go
 
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4
 
     - uses: actions/cache@v3
       with:

active_help.go

@@ -17,6 +17,7 @@ package cobra
 import (
 	"fmt"
 	"os"
+	"regexp"
 	"strings"
 )
 
@@ -29,6 +30,8 @@ const (
 	activeHelpGlobalDisable = "0"
 )
 
+var activeHelpEnvVarPrefixSubstRegexp = regexp.MustCompile(`[^A-Z0-9_]`)
+
 // AppendActiveHelp adds the specified string to the specified array to be used as ActiveHelp.
 // Such strings will be processed by the completion script and will be shown as ActiveHelp
 // to the user.
@@ -42,7 +45,7 @@ func AppendActiveHelp(compArray []string, activeHelpStr string) []string {
 
 // GetActiveHelpConfig returns the value of the ActiveHelp environment variable
 // <PROGRAM>_ACTIVE_HELP where <PROGRAM> is the name of the root command in upper
-// case, with all - replaced by _.
+// case, with all non-ASCII-alphanumeric characters replaced by `_`.
 // It will always return "0" if the global environment variable COBRA_ACTIVE_HELP
 // is set to "0".
 func GetActiveHelpConfig(cmd *Command) string {
@@ -55,9 +58,10 @@ func GetActiveHelpConfig(cmd *Command) string {
 
 // activeHelpEnvVar returns the name of the program-specific ActiveHelp environment
 // variable.  It has the format <PROGRAM>_ACTIVE_HELP where <PROGRAM> is the name of the
-// root command in upper case, with all - replaced by _.
+// root command in upper case, with all non-ASCII-alphanumeric characters replaced by `_`.
 func activeHelpEnvVar(name string) string {
 	// This format should not be changed: users will be using it explicitly.
 	activeHelpEnvVar := strings.ToUpper(fmt.Sprintf("%s%s", name, activeHelpEnvVarSuffix))
-	return strings.ReplaceAll(activeHelpEnvVar, "-", "_")
+	activeHelpEnvVar = activeHelpEnvVarPrefixSubstRegexp.ReplaceAllString(activeHelpEnvVar, "_")
+	return activeHelpEnvVar
 }

cobra.go

@@ -43,9 +43,10 @@ var initializers []func()
 var finalizers []func()
 
 const (
-	defaultPrefixMatching  = false
-	defaultCommandSorting  = true
-	defaultCaseInsensitive = false
+	defaultPrefixMatching   = false
+	defaultCommandSorting   = true
+	defaultCaseInsensitive  = false
+	defaultTraverseRunHooks = false
 )
 
 // EnablePrefixMatching allows setting automatic prefix matching. Automatic prefix matching can be a dangerous thing
@@ -60,6 +61,10 @@ var EnableCommandSorting = defaultCommandSorting
 // EnableCaseInsensitive allows case-insensitive commands names. (case sensitive by default)
 var EnableCaseInsensitive = defaultCaseInsensitive
 
+// EnableTraverseRunHooks executes persistent pre-run and post-run hooks from all parents.
+// By default this is disabled, which means only the first run hook to be found is executed.
+var EnableTraverseRunHooks = defaultTraverseRunHooks
+
 // MousetrapHelpText enables an information splash screen on Windows
 // if the CLI is started from explorer.exe.
 // To disable the mousetrap, just set this variable to blank string ("").

command.go

@@ -31,7 +31,10 @@ import (
 	flag "github.com/spf13/pflag"
 )
 
-const FlagSetByCobraAnnotation = "cobra_annotation_flag_set_by_cobra"
+const (
+	FlagSetByCobraAnnotation     = "cobra_annotation_flag_set_by_cobra"
+	CommandDisplayNameAnnotation = "cobra_annotation_command_display_name"
+)
 
 // FParseErrWhitelist configures Flag parse errors to be ignored
 type FParseErrWhitelist flag.ParseErrorsWhitelist
@@ -100,7 +103,7 @@ type Command struct {
 	Deprecated string
 
 	// Annotations are key/value pairs that can be used by applications to identify or
-	// group commands.
+	// group commands or set special options.
 	Annotations map[string]string
 
 	// Version defines the version for this command. If this value is non-empty and the command does not
@@ -116,6 +119,8 @@ type Command struct {
 	//   * PostRun()
 	//   * PersistentPostRun()
 	// All functions get the same args, the arguments after the command name.
+	// The *PreRun and *PostRun functions will only be executed if the Run function of the current
+	// command has been declared.
 	//
 	// PersistentPreRun: children of this command will inherit and execute.
 	PersistentPreRun func(cmd *Command, args []string)
@@ -940,15 +945,31 @@ func (c *Command) execute(a []string) (err error) {
 		return err
 	}
 
+	parents := make([]*Command, 0, 5)
 	for p := c; p != nil; p = p.Parent() {
+		if EnableTraverseRunHooks {
+			// When EnableTraverseRunHooks is set:
+			// - Execute all persistent pre-runs from the root parent till this command.
+			// - Execute all persistent post-runs from this command till the root parent.
+			parents = append([]*Command{p}, parents...)
+		} else {
+			// Otherwise, execute only the first found persistent hook.
+			parents = append(parents, p)
+		}
+	}
+	for _, p := range parents {
 		if p.PersistentPreRunE != nil {
 			if err := p.PersistentPreRunE(c, argWoFlags); err != nil {
 				return err
 			}
-			break
+			if !EnableTraverseRunHooks {
+				break
+			}
 		} else if p.PersistentPreRun != nil {
 			p.PersistentPreRun(c, argWoFlags)
-			break
+			if !EnableTraverseRunHooks {
+				break
+			}
 		}
 	}
 	if c.PreRunE != nil {
@@ -985,10 +1006,14 @@ func (c *Command) execute(a []string) (err error) {
 			if err := p.PersistentPostRunE(c, argWoFlags); err != nil {
 				return err
 			}
-			break
+			if !EnableTraverseRunHooks {
+				break
+			}
 		} else if p.PersistentPostRun != nil {
 			p.PersistentPostRun(c, argWoFlags)
-			break
+			if !EnableTraverseRunHooks {
+				break
+			}
 		}
 	}
 
@@ -1410,6 +1435,9 @@ func (c *Command) CommandPath() string {
 	if c.HasParent() {
 		return c.Parent().CommandPath() + " " + c.Name()
 	}
+	if displayName, ok := c.Annotations[CommandDisplayNameAnnotation]; ok {
+		return displayName
+	}
 	return c.Name()
 }
 
@@ -1432,6 +1460,7 @@ func (c *Command) UseLine() string {
 
 // DebugFlags used to determine which flags have been assigned to which commands
 // and which persist.
+// nolint:goconst
 func (c *Command) DebugFlags() {
 	c.Println("DebugFlags called on", c.Name())
 	var debugflags func(*Command)

command_test.go

@@ -366,6 +366,36 @@ func TestAliasPrefixMatching(t *testing.T) {
 	EnablePrefixMatching = defaultPrefixMatching
 }
 
+// TestPlugin checks usage as plugin for another command such as kubectl.  The
+// executable is `kubectl-plugin`, but we run it as `kubectl plugin`. The help
+// text should reflect the way we run the command.
+func TestPlugin(t *testing.T) {
+	rootCmd := &Command{
+		Use:  "plugin",
+		Args: NoArgs,
+		Annotations: map[string]string{
+			CommandDisplayNameAnnotation: "kubectl plugin",
+		},
+	}
+
+	subCmd := &Command{Use: "sub [flags]", Args: NoArgs, Run: emptyRun}
+	rootCmd.AddCommand(subCmd)
+
+	rootHelp, err := executeCommand(rootCmd, "-h")
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	checkStringContains(t, rootHelp, "kubectl plugin [command]")
+
+	childHelp, err := executeCommand(rootCmd, "sub", "-h")
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	checkStringContains(t, childHelp, "kubectl plugin sub [flags]")
+}
+
 // TestChildSameName checks the correct behaviour of cobra in cases,
 // when an application with name "foo" and with subcommand "foo"
 // is executed with args "foo foo".
@@ -1530,57 +1560,73 @@ func TestHooks(t *testing.T) {
 }
 
 func TestPersistentHooks(t *testing.T) {
-	var (
-		parentPersPreArgs  string
-		parentPreArgs      string
-		parentRunArgs      string
-		parentPostArgs     string
-		parentPersPostArgs string
-	)
+	EnableTraverseRunHooks = true
+	testPersistentHooks(t, []string{
+		"parent PersistentPreRun",
+		"child PersistentPreRun",
+		"child PreRun",
+		"child Run",
+		"child PostRun",
+		"child PersistentPostRun",
+		"parent PersistentPostRun",
+	})
 
-	var (
-		childPersPreArgs  string
-		childPreArgs      string
-		childRunArgs      string
-		childPostArgs     string
-		childPersPostArgs string
-	)
+	EnableTraverseRunHooks = false
+	testPersistentHooks(t, []string{
+		"child PersistentPreRun",
+		"child PreRun",
+		"child Run",
+		"child PostRun",
+		"child PersistentPostRun",
+	})
+}
+
+func testPersistentHooks(t *testing.T, expectedHookRunOrder []string) {
+	var hookRunOrder []string
+
+	validateHook := func(args []string, hookName string) {
+		hookRunOrder = append(hookRunOrder, hookName)
+		got := strings.Join(args, " ")
+		if onetwo != got {
+			t.Errorf("Expected %s %q, got %q", hookName, onetwo, got)
+		}
+	}
 
 	parentCmd := &Command{
 		Use: "parent",
 		PersistentPreRun: func(_ *Command, args []string) {
-			parentPersPreArgs = strings.Join(args, " ")
+			validateHook(args, "parent PersistentPreRun")
 		},
 		PreRun: func(_ *Command, args []string) {
-			parentPreArgs = strings.Join(args, " ")
+			validateHook(args, "parent PreRun")
 		},
 		Run: func(_ *Command, args []string) {
-			parentRunArgs = strings.Join(args, " ")
+			validateHook(args, "parent Run")
 		},
 		PostRun: func(_ *Command, args []string) {
-			parentPostArgs = strings.Join(args, " ")
+			validateHook(args, "parent PostRun")
 		},
 		PersistentPostRun: func(_ *Command, args []string) {
-			parentPersPostArgs = strings.Join(args, " ")
+			validateHook(args, "parent PersistentPostRun")
 		},
 	}
 
 	childCmd := &Command{
 		Use: "child",
 		PersistentPreRun: func(_ *Command, args []string) {
-			childPersPreArgs = strings.Join(args, " ")
+			validateHook(args, "child PersistentPreRun")
 		},
 		PreRun: func(_ *Command, args []string) {
-			childPreArgs = strings.Join(args, " ")
+			validateHook(args, "child PreRun")
 		},
 		Run: func(_ *Command, args []string) {
-			childRunArgs = strings.Join(args, " ")
+			validateHook(args, "child Run")
 		},
 		PostRun: func(_ *Command, args []string) {
-			childPostArgs = strings.Join(args, " ")
+			validateHook(args, "child PostRun")
 		},
 		PersistentPostRun: func(_ *Command, args []string) {
-			childPersPostArgs = strings.Join(args, " ")
+			validateHook(args, "child PersistentPostRun")
 		},
 	}
 	parentCmd.AddCommand(childCmd)
@@ -1593,41 +1639,13 @@ func TestPersistentHooks(t *testing.T) {
 		t.Errorf("Unexpected error: %v", err)
 	}
 
-	for _, v := range []struct {
-		name string
-		got  string
-	}{
-		// TODO: currently PersistentPreRun* defined in parent does not
-		// run if the matching child subcommand has PersistentPreRun.
-		// If the behavior changes (https://github.com/spf13/cobra/issues/252)
-		// this test must be fixed.
-		{"parentPersPreArgs", parentPersPreArgs},
-		{"parentPreArgs", parentPreArgs},
-		{"parentRunArgs", parentRunArgs},
-		{"parentPostArgs", parentPostArgs},
-		// TODO: currently PersistentPostRun* defined in parent does not
-		// run if the matching child subcommand has PersistentPostRun.
-		// If the behavior changes (https://github.com/spf13/cobra/issues/252)
-		// this test must be fixed.
-		{"parentPersPostArgs", parentPersPostArgs},
-	} {
-		if v.got != "" {
-			t.Errorf("Expected blank %s, got %q", v.name, v.got)
-		}
-	}
-
-	for _, v := range []struct {
-		name string
-		got  string
-	}{
-		{"childPersPreArgs", childPersPreArgs},
-		{"childPreArgs", childPreArgs},
-		{"childRunArgs", childRunArgs},
-		{"childPostArgs", childPostArgs},
-		{"childPersPostArgs", childPersPostArgs},
-	} {
-		if v.got != onetwo {
-			t.Errorf("Expected %s %q, got %q", v.name, onetwo, v.got)
+	for idx, exp := range expectedHookRunOrder {
+		if len(hookRunOrder) > idx {
+			if act := hookRunOrder[idx]; act != exp {
+				t.Errorf("Expected %q at %d, got %q", exp, idx, act)
+			}
+		} else {
+			t.Errorf("Expected %q at %d, got nothing", exp, idx)
 		}
 	}
 }