Repository: spf13/cobra
Pull request #2018 (2023-08-16 16:29:05)

Support usage as plugin for tools like kubectl
In this case the executable is `kubectl-plugin`, but we run it as:

    kubectl plugin

And the help text should reflect the actual usage of the command.

To create a plugin, add the cobra.CommandDisplayNameAnnotation:

    rootCmd := &cobra.Command{
        Use: "plugin",
        Annotations: map[string]string{
            cobra.CommandDisplayNameAnnotation: "kubectl plugin",
        }
    }

Internally this change modifies CommandPath() for the root command to
return the command display name instead of the command name. This is
used for error messages, help text generation, and completions.

CommandPath() is expected to have spaces and code using it already
handle spaces (e.g replacing with _), so hopefully this does not break
anything.

Issues:
- Need documentation

Fixes: #2017

CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/spf13/cobra?pullRequest=2018) <br/>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/spf13/cobra?pullRequest=2018) before we can accept your contribution.<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/spf13/cobra?pullRequest=2018) it.</sub>
nirs: > This seems like a good idea. But I’d like a little more time to dig deeper because now, `cmd.Name()` can return a string with a space. This is new and I’d like to see if it could cause issues.

So far I found that it breaks completion - the generated bash script
contain cmd.Name() as is.

But completions work only when running the the command directly (e.g `./kubectl-foo`)
but not when running the command under kubectl (e.g. `kubectl foo`).
hitzhangjie: I test this feature for 2 following cases:
1. without this PR, we can set Command.Name to `kubectl-plugin`, the help message will be 'help for kubectl-plugin'
2. with this PR, after we set Command.CommandName to `kubectl pluin`, the help message will be 'help for kubectl plugin'

The subcommand 'kubectl plugin xxx yyy' help message, would be the pattern 'help for <subcmd>', here <subcmd> is the name of 'yyy'.

Well, we know the kubectl plugin must be named as `kubectl-<plugin>` and put it into the search path. 
Maybe, using Command.Name="kubectl-plugin" is OK?
marckhouzam: 
> I test this feature for 2 following cases:
> 
> 1. without this PR, we can set Command.Name to `kubectl-plugin`, the help message will be `help for kubectl-plugin`
> 2. with this PR, after we set Command.CommandName to `kubectl plugin`, the help message will be `help for kubectl plugin`
> 
> The subcommand `kubectl plugin xxx yyy` help message, would be the pattern `help for <subcmd>`, here `<subcmd>` is the name of `yyy`.

Just to clarify, the help message you are talking about above, is the description of the `--help` flag.
This PR does affect this message for the root command only as you have tested.  I think that is ok.

> Well, we know the kubectl plugin must be named as `kubectl-<plugin>` and put it into the search path. Maybe, using Command.Name="kubectl-plugin" and display the 'kubectl-plugin' instead of 'kubectl plugin' is OK?

I'm not sure what you mean.  This is the current behaviour, right?  And a plugin that does not want to change this behaviour does not need to use the new `cmd.Root().CommandName` field.

marckhouzam: > So far I found that it breaks completion - the generated bash script contain cmd.Name() as is.

Good find.

> But completions work only when running the the command directly (e.g `./kubectl-foo`) but not when running the command under kubectl (e.g. `kubectl foo`).

Recent versions of kubectl do allow for completion to work when using the form `kubectl foo <TAB>`.  I'll refer you to this project: https://github.com/marckhouzam/kubectl-plugin_completion

Interestingly, this PR does not break shell completion when used as `kubectl foo <TAB>`.
However, the fact that it does break the completion script if the plugin is called directly `kubectl-foo <TAB>` is a problem.

Maybe we can look into an approach that addresses the requirement more directly?
The requirement, IIUC, is to have the help message show `kubectl plugin` instead of `kubectl-plugin`.  Could we provide an easy way for the plugin developer to change just the help message, instead of the `Name()` function?
nirs: > Recent versions of kubectl do allow for completion to work when using the form `kubectl foo <TAB>`. I'll refer you to this project: https://github.com/marckhouzam/kubectl-plugin_completion

Awesome! thanks.
nirs: Testing using https://github.com/nirs/kubectl-cobraplugin/

Root command:

```
% kubectl cobraplugin 
Usage:
  kubectl cobraplugin [command]

Available Commands:
  completion  Generate the autocompletion script for the specified shell
  help        Help about any command
  subcmd      

Flags:
  -h, --help   help for kubectl-cobraplugin

Use "kubectl cobraplugin [command] --help" for more information about a command.
```

Sub command:

```
% kubectl cobraplugin subcmd -h
Usage:
  kubectl cobraplugin subcmd [flags]

Flags:
  -h, --help   help for subcmd
```

Completions via kubectl:

```
% cat ~/bin/kubectl_complete-cobraplugin 
#!/usr/bin/env bash
kubectl cobraplugin __complete "$@"

% kubectl cobraplugin completion zsh > ~/.zsh/_kubectl-cobraplugin

% kubectl cobraplugin completion [TAB]
bash        -- Generate the autocompletion script for bash
fish        -- Generate the autocompletion script for fish
powershell  -- Generate the autocompletion script for powershell
zsh         -- Generate the autocompletion script for zsh
```

Patch:
diff --git a/active_help.go b/active_help.go
index 2d02394..5f965e0 100644
--- a/active_help.go
+++ b/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
 }
diff --git a/cobra.go b/cobra.go
index f23f509..a6b160c 100644
--- a/cobra.go
+++ b/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 ("").
diff --git a/command.go b/command.go
index 6866f7d..2fbe6c1 100644
--- a/command.go
+++ b/command.go
@@ -30,7 +30,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
@@ -99,7 +102,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
@@ -115,6 +118,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)
@@ -932,15 +937,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 {
@@ -977,10 +998,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
+			}
 		}
 	}
 
@@ -1402,6 +1427,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()
 }
 
@@ -1424,6 +1452,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)
diff --git a/completions.go b/completions.go
index ee38c4d..368b92a 100644
--- a/completions.go
+++ b/completions.go
@@ -145,6 +145,25 @@ func (c *Command) RegisterFlagCompletionFunc(flagName string, f func(cmd *Comman
 	return nil
 }
 
+// GetFlagCompletion returns the completion function for the given flag, if available.
+func GetFlagCompletion(flag *pflag.Flag) (func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective), bool) {
+	flagCompletionMutex.RLock()
+	defer flagCompletionMutex.RUnlock()
+
+	completionFunc, exists := flagCompletionFunctions[flag]
+	return completionFunc, exists
+}
+
+// GetFlagCompletionByName returns the completion function for the given flag in the command by name, if available.
+func (c *Command) GetFlagCompletionByName(flagName string) (func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective), bool) {
+	flag := c.Flags().Lookup(flagName)
+	if flag == nil {
+		return nil, false
+	}
+
+	return GetFlagCompletion(flag)
+}
+
 // Returns a string listing the different directive enabled in the specified parameter
 func (d ShellCompDirective) string() string {
 	var directives []string
@@ -283,9 +302,13 @@ func (c *Command) getCompletions(args []string) (*Command, []string, ShellCompDi
 
 	// These flags are normally added when `execute()` is called on `finalCmd`,
 	// however, when doing completion, we don't call `finalCmd.execute()`.
-	// Let's add the --help and --version flag ourselves.
-	finalCmd.InitDefaultHelpFlag()
-	finalCmd.InitDefaultVersionFlag()
+	// Let's add the --help and --version flag ourselves but only if the finalCmd
+	// has not disabled flag parsing; if flag parsing is disabled, it is up to the
+	// finalCmd itself to handle the completion of *all* flags.
+	if !finalCmd.DisableFlagParsing {
+		finalCmd.InitDefaultHelpFlag()
+		finalCmd.InitDefaultVersionFlag()
+	}
 
 	// Check if we are doing flag value completion before parsing the flags.
 	// This is important because if we are completing a flag value, we need to also
@@ -389,6 +412,11 @@ func (c *Command) getCompletions(args []string) (*Command, []string, ShellCompDi
 			finalCmd.InheritedFlags().VisitAll(func(flag *pflag.Flag) {
 				doCompleteFlags(flag)
 			})
+			// Try to complete non-inherited flags even if DisableFlagParsing==true.
+			// This allows programs to tell Cobra about flags for completion even
+			// if the actual parsing of flags is not done by Cobra.
+			// For instance, Helm uses this to provide flag name completion for
+			// some of its plugins.
 			finalCmd.NonInheritedFlags().VisitAll(func(flag *pflag.Flag) {
 				doCompleteFlags(flag)
 			})
diff --git a/doc/md_docs.go b/doc/md_docs.go
index c4a27c0..f98fe2a 100644
--- a/doc/md_docs.go
+++ b/doc/md_docs.go
@@ -27,6 +27,8 @@ import (
 	"github.com/spf13/cobra"
 )
 
+const markdownExtension = ".md"
+
 func printOptions(buf *bytes.Buffer, cmd *cobra.Command, name string) error {
 	flags := cmd.NonInheritedFlags()
 	flags.SetOutput(buf)
@@ -83,7 +85,7 @@ func GenMarkdownCustom(cmd *cobra.Command, w io.Writer, linkHandler func(string)
 		if cmd.HasParent() {
 			parent := cmd.Parent()
 			pname := parent.CommandPath()
-			link := pname + ".md"
+			link := pname + markdownExtension
 			link = strings.ReplaceAll(link, " ", "_")
 			buf.WriteString(fmt.Sprintf("* [%s](%s)\t - %s\n", pname, linkHandler(link), parent.Short))
 			cmd.VisitParents(func(c *cobra.Command) {
@@ -101,7 +103,7 @@ func GenMarkdownCustom(cmd *cobra.Command, w io.Writer, linkHandler func(string)
 				continue
 			}
 			cname := name + " " + child.Name()
-			link := cname + ".md"
+			link := cname + markdownExtension
 			link = strings.ReplaceAll(link, " ", "_")
 			buf.WriteString(fmt.Sprintf("* [%s](%s)\t - %s\n", cname, linkHandler(link), child.Short))
 		}
@@ -138,7 +140,7 @@ func GenMarkdownTreeCustom(cmd *cobra.Command, dir string, filePrepender, linkHa
 		}
 	}
 
-	basename := strings.ReplaceAll(cmd.CommandPath(), " ", "_") + ".md"
+	basename := strings.ReplaceAll(cmd.CommandPath(), " ", "_") + markdownExtension
 	filename := filepath.Join(dir, basename)
 	f, err := os.Create(filename)
 	if err != nil {
diff --git a/go.mod b/go.mod
index 6361d74..a79e66a 100644
--- a/go.mod
+++ b/go.mod
@@ -3,7 +3,7 @@ module github.com/spf13/cobra
 go 1.15
 
 require (
-	github.com/cpuguy83/go-md2man/v2 v2.0.2
+	github.com/cpuguy83/go-md2man/v2 v2.0.3
 	github.com/inconshreveable/mousetrap v1.1.0
 	github.com/spf13/pflag v1.0.5
 	gopkg.in/yaml.v3 v3.0.1
diff --git a/go.sum b/go.sum
index 5ccb69d..871c3a8 100644
--- a/go.sum
+++ b/go.sum
@@ -1,5 +1,5 @@
-github.com/cpuguy83/go-md2man/v2 v2.0.2 h1:p1EgwI/C7NhT0JmVkwCD2ZBK8j4aeHQX2pMHHBfMQ6w=
-github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
+github.com/cpuguy83/go-md2man/v2 v2.0.3 h1:qMCsGGgs+MAzDFyp9LpAe1Lqy/fY/qCovCm0qnXZOBM=
+github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
 github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
 github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
 github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk=
diff --git a/site/content/active_help.md b/site/content/active_help.md
index 5e7f59a..d72acc7 100644
--- a/site/content/active_help.md
+++ b/site/content/active_help.md
@@ -92,7 +92,7 @@ Allowing to configure Active Help is entirely optional; you can use Active Help
 
 The way to configure Active Help is to use the program's Active Help environment
 variable.  That variable is named `<PROGRAM>_ACTIVE_HELP` where `<PROGRAM>` is the name of your 
-program in uppercase with any `-` replaced by an `_`.  The variable should be set by the user to whatever
+program in uppercase with any non-ASCII-alphanumeric characters replaced by an `_`.  The variable should be set by the user to whatever
 Active Help configuration values are supported by the program.
 
 For example, say `helm` has chosen to support three levels for Active Help: `on`, `off`, `local`.  Then a user
@@ -140,7 +140,7 @@ details for your users.
 
 Debugging your Active Help code is done in the same way as debugging your dynamic completion code, which is with Cobra's hidden `__complete` command.  Please refer to [debugging shell completion](shell_completions.md#debugging) for details.
 
-When debugging with the `__complete` command, if you want to specify different Active Help configurations, you should use the active help environment variable.  That variable is named `<PROGRAM>_ACTIVE_HELP` where any `-` is replaced by an `_`.  For example, we can test deactivating some Active Help as shown below:
+When debugging with the `__complete` command, if you want to specify different Active Help configurations, you should use the active help environment variable.  That variable is named `<PROGRAM>_ACTIVE_HELP` where any non-ASCII-alphanumeric characters are replaced by an `_`.  For example, we can test deactivating some Active Help as shown below:
 ```
 $ HELM_ACTIVE_HELP=1 bin/helm __complete install wordpress bitnami/h<ENTER>
 bitnami/haproxy
diff --git a/site/content/user_guide.md b/site/content/user_guide.md
index 93daadf..4116e8d 100644
--- a/site/content/user_guide.md
+++ b/site/content/user_guide.md
@@ -604,7 +604,7 @@ The Prefix, `Error:` can be customized using the `cmd.SetErrPrefix(s string)` fu
 
 ## PreRun and PostRun Hooks
 
-It is possible to run functions before or after the main `Run` function of your command. The `PersistentPreRun` and `PreRun` functions will be executed before `Run`. `PersistentPostRun` and `PostRun` will be executed after `Run`.  The `Persistent*Run` functions will be inherited by children if they do not declare their own.  These functions are run in the following order:
+It is possible to run functions before or after the main `Run` function of your command. The `PersistentPreRun` and `PreRun` functions will be executed before `Run`. `PersistentPostRun` and `PostRun` will be executed after `Run`.  The `Persistent*Run` functions will be inherited by children if they do not declare their own.  The `*PreRun` and `*PostRun` functions will only be executed if the `Run` function of the current command has been declared.  These functions are run in the following order:
 
 - `PersistentPreRun`
 - `PreRun`
@@ -687,6 +687,10 @@ Inside subCmd PostRun with args: [arg1 arg2]
 Inside subCmd PersistentPostRun with args: [arg1 arg2]
 ```
 
+By default, only the first persistent hook found in the command chain is executed.
+That is why in the above output, the `rootCmd PersistentPostRun` was not called for a child command.
+Set `EnableTraverseRunHooks` global variable to `true` if you want to execute all parents' persistent hooks.
+
 ## Suggestions when "unknown command" happens
 
 Cobra will print automatic suggestions when "unknown command" errors happen. This allows Cobra to behave similarly to the `git` command when a typo happens. For example:


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 6b4c165..a924532 100644
--- a/.github/workflows/test.yml
+++ b/.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/golangci-lint-action@v3.7.0
         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:
diff --git a/command_test.go b/command_test.go
index 4afb7f7..9f686d6 100644
--- a/command_test.go
+++ b/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)
 		}
 	}
 }
diff --git a/completions_test.go b/completions_test.go
index 0172466..7585f88 100644
--- a/completions_test.go
+++ b/completions_test.go
@@ -17,7 +17,9 @@ package cobra
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"strings"
+	"sync"
 	"testing"
 )
 
@@ -2040,6 +2042,114 @@ func TestFlagCompletionWorksRootCommandAddedAfterFlags(t *testing.T) {
 	}
 }
 
+func TestFlagCompletionForPersistentFlagsCalledFromSubCmd(t *testing.T) {
+	rootCmd := &Command{Use: "root", Run: emptyRun}
+	rootCmd.PersistentFlags().String("string", "", "test string flag")
+	_ = rootCmd.RegisterFlagCompletionFunc("string", func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) {
+		return []string{"myval"}, ShellCompDirectiveDefault
+	})
+
+	childCmd := &Command{
+		Use: "child",
+		Run: emptyRun,
+		ValidArgsFunction: func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) {
+			return []string{"--validarg", "test"}, ShellCompDirectiveDefault
+		},
+	}
+	childCmd.Flags().Bool("bool", false, "test bool flag")
+	rootCmd.AddCommand(childCmd)
+
+	// Test that persistent flag completion works for the subcmd
+	output, err := executeCommand(rootCmd, ShellCompRequestCmd, "child", "--string", "")
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	expected := strings.Join([]string{
+		"myval",
+		":0",
+		"Completion ended with directive: ShellCompDirectiveDefault", ""}, "\n")
+
+	if output != expected {
+		t.Errorf("expected: %q, got: %q", expected, output)
+	}
+}
+
+// This test tries to register flag completion concurrently to make sure the
+// code handles concurrency properly.
+// This was reported as a problem when tests are run concurrently:
+// https://github.com/spf13/cobra/issues/1320
+//
+// NOTE: this test can sometimes pass even if the code were to not handle
+// concurrency properly. This is not great but the important part is that
+// it should never fail.  Therefore, if the tests fails sometimes, we will
+// still be able to know there is a problem.
+func TestFlagCompletionConcurrentRegistration(t *testing.T) {
+	rootCmd := &Command{Use: "root", Run: emptyRun}
+	const maxFlags = 50
+	for i := 1; i < maxFlags; i += 2 {
+		flagName := fmt.Sprintf("flag%d", i)
+		rootCmd.Flags().String(flagName, "", fmt.Sprintf("test %s flag on root", flagName))
+	}
+
+	childCmd := &Command{
+		Use: "child",
+		Run: emptyRun,
+	}
+	for i := 2; i <= maxFlags; i += 2 {
+		flagName := fmt.Sprintf("flag%d", i)
+		childCmd.Flags().String(flagName, "", fmt.Sprintf("test %s flag on child", flagName))
+	}
+
+	rootCmd.AddCommand(childCmd)
+
+	// Register completion in different threads to test concurrency.
+	var wg sync.WaitGroup
+	for i := 1; i <= maxFlags; i++ {
+		index := i
+		flagName := fmt.Sprintf("flag%d", i)
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			cmd := rootCmd
+			if index%2 == 0 {
+				cmd = childCmd
+			}
+			_ = cmd.RegisterFlagCompletionFunc(flagName, func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) {
+				return []string{fmt.Sprintf("flag%d", index)}, ShellCompDirectiveDefault
+			})
+		}()
+	}
+
+	wg.Wait()
+
+	// Test that flag completion works for each flag
+	for i := 1; i <= 6; i++ {
+		var output string
+		var err error
+		flagName := fmt.Sprintf("flag%d", i)
+
+		if i%2 == 1 {
+			output, err = executeCommand(rootCmd, ShellCompRequestCmd, "--"+flagName, "")
+		} else {
+			output, err = executeCommand(rootCmd, ShellCompRequestCmd, "child", "--"+flagName, "")
+		}
+
+		if err != nil {
+			t.Errorf("Unexpected error: %v", err)
+		}
+
+		expected := strings.Join([]string{
+			flagName,
+			":0",
+			"Completion ended with directive: ShellCompDirectiveDefault", ""}, "\n")
+
+		if output != expected {
+			t.Errorf("expected: %q, got: %q", expected, output)
+		}
+	}
+}
+
 func TestFlagCompletionInGoWithDesc(t *testing.T) {
 	rootCmd := &Command{
 		Use: "root",
@@ -2622,8 +2732,6 @@ func TestCompleteWithDisableFlagParsing(t *testing.T) {
 	expected := strings.Join([]string{
 		"--persistent",
 		"-p",
-		"--help",
-		"-h",
 		"--nonPersistent",
 		"-n",
 		"--flag",
@@ -3053,8 +3161,26 @@ func TestCompletionCobraFlags(t *testing.T) {
 				return []string{"extra3"}, ShellCompDirectiveNoFileComp
 			},
 		}
+		childCmd4 := &Command{
+			Use:     "child4",
+			Version: "1.1.1",
+			Run:     emptyRun,
+			ValidArgsFunction: func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) {
+				return []string{"extra4"}, ShellCompDirectiveNoFileComp
+			},
+			DisableFlagParsing: true,
+		}
+		childCmd5 := &Command{
+			Use:     "child5",
+			Version: "1.1.1",
+			Run:     emptyRun,
+			ValidArgsFunction: func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) {
+				return []string{"extra5"}, ShellCompDirectiveNoFileComp
+			},
+			DisableFlagParsing: true,
+		}
 
-		rootCmd.AddCommand(childCmd, childCmd2, childCmd3)
+		rootCmd.AddCommand(childCmd, childCmd2, childCmd3, childCmd4, childCmd5)
 
 		_ = childCmd.Flags().Bool("bool", false, "A bool flag")
 		_ = childCmd.MarkFlagRequired("bool")
@@ -3066,6 +3192,10 @@ func TestCompletionCobraFlags(t *testing.T) {
 		// Have a command that only adds its own -v flag
 		_ = childCmd3.Flags().BoolP("verbose", "v", false, "Not a version flag")
 
+		// Have a command that DisablesFlagParsing but that also adds its own help and version flags
+		_ = childCmd5.Flags().BoolP("help", "h", false, "My own help")
+		_ = childCmd5.Flags().BoolP("version", "v", false, "My own version")
+
 		return rootCmd
 	}
 
@@ -3196,6 +3326,26 @@ func TestCompletionCobraFlags(t *testing.T) {
 				":4",
 				"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
 		},
+		{
+			desc: "no completion for --help/-h and --version/-v flags when DisableFlagParsing=true",
+			args: []string{"child4", "-"},
+			expectedOutput: strings.Join([]string{
+				"extra4",
+				":4",
+				"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
+		},
+		{
+			desc: "completions for program-defined --help/-h and --version/-v flags even when DisableFlagParsing=true",
+			args: []string{"child5", "-"},
+			expectedOutput: strings.Join([]string{
+				"--help",
+				"-h",
+				"--version",
+				"-v",
+				"extra5",
+				":4",
+				"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
+		},
 	}
 
 	for _, tc := range testcases {