Redirect bash completion v1 to v2 when possible

We are no longer actively maintaining bash completion v1 in favor of its
more rich v2 version.  Previously, using bash completion v2 required
projects to be aware of its existence and to explicitly call
GenBashCompletionV2().

With this commit, any projects calling GenBashCompletion() will
automatically be redirected to using the v2 version.

One exception is if the project uses the legacy custom completion logic
which is not supported in v2.  We can detect that by looking for the
use of the field `BashCompletionFunction` on the root command.

Note that descriptions are kept off when all GenBashCompletion().
This means that to enable completion descriptions for bash, a project
must still explicitly call GenBashCompletionV2().

Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>

bash_completions.go

@@ -684,6 +684,14 @@ func gen(buf io.StringWriter, cmd *Command) {
 
 // GenBashCompletion generates bash completion file and writes to the passed writer.
 func (c *Command) GenBashCompletion(w io.Writer) error {
+	if len(c.BashCompletionFunction) == 0 {
+		// If the program does not define any legacy custom completion (which is not
+		// supported by bash completion V2), use bash completion V2 which is the version
+		// that is maintained.  However, we keep descriptions off to behave as much as v1
+		// as possible to avoid changing things unexpectedly for projects
+		return c.GenBashCompletionV2(w, false)
+	}
+
 	buf := new(bytes.Buffer)
 	writePreamble(buf, c.Name())
 	if len(c.BashCompletionFunction) > 0 {

bash_completions_test.go

@@ -227,39 +227,72 @@ func TestBashCompletions(t *testing.T) {
 }
 
 func TestBashCompletionHiddenFlag(t *testing.T) {
-	c := &Command{Use: "c", Run: emptyRun}
+	c := &Command{
+		Use: "c",
+		// Set the legacy BashCompletionFunction to force the
+		// use of bash completion V1
+		BashCompletionFunction: bashCompletionFunc,
+		Run:                    emptyRun,
+	}
 
-	const flagName = "hiddenFlag"
-	c.Flags().Bool(flagName, false, "")
-	assertNoErr(t, c.Flags().MarkHidden(flagName))
+	const validFlagName = "valid-flag"
+	c.Flags().Bool(validFlagName, false, "")
+
+	const hiddenFlagName = "hiddenFlag"
+	c.Flags().Bool(hiddenFlagName, false, "")
+	assertNoErr(t, c.Flags().MarkHidden(hiddenFlagName))
 
 	buf := new(bytes.Buffer)
 	assertNoErr(t, c.GenBashCompletion(buf))
 	output := buf.String()
 
-	if strings.Contains(output, flagName) {
-		t.Errorf("Expected completion to not include %q flag: Got %v", flagName, output)
+	if !strings.Contains(output, validFlagName) {
+		t.Errorf("Expected completion to include %q flag: Got %v", validFlagName, output)
+	}
+
+	if strings.Contains(output, hiddenFlagName) {
+		t.Errorf("Expected completion to not include %q flag: Got %v", hiddenFlagName, output)
 	}
 }
 
 func TestBashCompletionDeprecatedFlag(t *testing.T) {
-	c := &Command{Use: "c", Run: emptyRun}
+	c := &Command{
+		Use: "c",
+		// Set the legacy BashCompletionFunction to force the
+		// use of bash completion V1
+		BashCompletionFunction: bashCompletionFunc,
+		Run:                    emptyRun,
+	}
+
+	const validFlagName = "valid-flag"
+	c.Flags().Bool(validFlagName, false, "")
 
-	const flagName = "deprecated-flag"
-	c.Flags().Bool(flagName, false, "")
-	assertNoErr(t, c.Flags().MarkDeprecated(flagName, "use --not-deprecated instead"))
+	const deprecatedFlagName = "deprecated-flag"
+	c.Flags().Bool(deprecatedFlagName, false, "")
+	assertNoErr(t, c.Flags().MarkDeprecated(deprecatedFlagName, "use --not-deprecated instead"))
 
 	buf := new(bytes.Buffer)
 	assertNoErr(t, c.GenBashCompletion(buf))
 	output := buf.String()
 
-	if strings.Contains(output, flagName) {
-		t.Errorf("expected completion to not include %q flag: Got %v", flagName, output)
+	if !strings.Contains(output, validFlagName) {
+		t.Errorf("expected completion to include %q flag: Got %v", validFlagName, output)
+	}
+
+	if strings.Contains(output, deprecatedFlagName) {
+		t.Errorf("expected completion to not include %q flag: Got %v", deprecatedFlagName, output)
 	}
 }
 
 func TestBashCompletionTraverseChildren(t *testing.T) {
-	c := &Command{Use: "c", Run: emptyRun, TraverseChildren: true}
+	c := &Command{
+		Use: "c",
+		// Set the legacy BashCompletionFunction to force the
+		// use of bash completion V1
+		BashCompletionFunction: bashCompletionFunc,
+		Run:                    emptyRun,
+		TraverseChildren:       true,
+	}
 
 	c.Flags().StringP("string-flag", "s", "", "string flag")
 	c.Flags().BoolP("bool-flag", "b", false, "bool flag")
@@ -268,6 +301,10 @@ func TestBashCompletionTraverseChildren(t *testing.T) {
 	assertNoErr(t, c.GenBashCompletion(buf))
 	output := buf.String()
 
+	if !strings.Contains(output, "bool-flag") {
+		t.Errorf("Expected completion to include bool-flag flag: Got %v", output)
+	}
+
 	// check that local nonpersistent flag are not set since we have TraverseChildren set to true
 	checkOmit(t, output, `local_nonpersistent_flags+=("--string-flag")`)
 	checkOmit(t, output, `local_nonpersistent_flags+=("--string-flag=")`)
@@ -277,7 +314,13 @@ func TestBashCompletionTraverseChildren(t *testing.T) {
 }
 
 func TestBashCompletionNoActiveHelp(t *testing.T) {
-	c := &Command{Use: "c", Run: emptyRun}
+	c := &Command{
+		Use: "c",
+		// Set the legacy BashCompletionFunction to force the
+		// use of bash completion V1
+		BashCompletionFunction: bashCompletionFunc,
+		Run:                    emptyRun,
+	}
 
 	buf := new(bytes.Buffer)
 	assertNoErr(t, c.GenBashCompletion(buf))
@@ -287,3 +330,37 @@ func TestBashCompletionNoActiveHelp(t *testing.T) {
 	activeHelpVar := activeHelpEnvVar(c.Name())
 	check(t, output, fmt.Sprintf("%s=0", activeHelpVar))
 }
+
+func TestBashCompletionV1WhenBashCompletionFunction(t *testing.T) {
+	c := &Command{
+		Use: "c",
+		// Include the legacy BashCompletionFunction field
+		// and check that we are generating bash completion V1
+		BashCompletionFunction: bashCompletionFunc,
+		Run:                    emptyRun,
+	}
+
+	buf := new(bytes.Buffer)
+	assertNoErr(t, c.GenBashCompletion(buf))
+	output := buf.String()
+
+	// Check the generated script is the V1 version
+	checkOmit(t, output, "# bash completion V2 for")
+	check(t, output, "# bash completion for")
+}
+
+func TestBashCompletionV2WhenNoBashCompletionFunction(t *testing.T) {
+	c := &Command{
+		Use: "c",
+		// Do NOT include the legacy BashCompletionFunction
+		// and check that we are generating bash completion V2
+		Run: emptyRun,
+	}
+
+	buf := new(bytes.Buffer)
+	assertNoErr(t, c.GenBashCompletion(buf))
+	output := buf.String()
+
+	// Check the generated script is the V2 version
+	check(t, output, "# bash completion V2 for")
+}

completions_test.go

@@ -1506,11 +1506,19 @@ func TestValidArgsFuncAliases(t *testing.T) {
 }
 
 func TestValidArgsFuncInBashScript(t *testing.T) {
-	rootCmd := &Command{Use: "root", Args: NoArgs, Run: emptyRun}
+	rootCmd := &Command{
+		Use:  "root",
+		Args: NoArgs,
+		// Set the legacy BashCompletionFunction to force the
+		// use of bash completion V1
+		BashCompletionFunction: bashCompletionFunc,
+		Run:                    emptyRun,
+	}
 	child := &Command{
-		Use:               "child",
-		ValidArgsFunction: validArgsFunc,
-		Run:               emptyRun,
+		Use:                    "child",
+		ValidArgsFunction:      validArgsFunc,
+		BashCompletionFunction: bashCompletionFunc,
+		Run:                    emptyRun,
 	}
 	rootCmd.AddCommand(child)
 
@@ -1522,7 +1530,14 @@ func TestValidArgsFuncInBashScript(t *testing.T) {
 }
 
 func TestNoValidArgsFuncInBashScript(t *testing.T) {
-	rootCmd := &Command{Use: "root", Args: NoArgs, Run: emptyRun}
+	rootCmd := &Command{
+		Use:  "root",
+		Args: NoArgs,
+		// Set the legacy BashCompletionFunction to force the
+		// use of bash completion V1
+		BashCompletionFunction: bashCompletionFunc,
+		Run:                    emptyRun,
+	}
 	child := &Command{
 		Use: "child",
 		Run: emptyRun,

shell_completions.md

@@ -75,7 +75,7 @@ PowerShell:
 	Run: func(cmd *cobra.Command, args []string) {
 		switch args[0] {
 		case "bash":
-			cmd.Root().GenBashCompletion(os.Stdout)
+			cmd.Root().GenBashCompletionV2(os.Stdout, true)
 		case "zsh":
 			cmd.Root().GenZshCompletion(os.Stdout)
 		case "fish":
@@ -412,20 +412,21 @@ Please refer to [Bash Completions](bash_completions.md) for details.
 
 ### Bash completion V2
 
-Cobra provides two versions for bash completion.  The original bash completion (which started it all!) can be used by calling
-`GenBashCompletion()` or `GenBashCompletionFile()`.
+Cobra provides two versions for bash completion.  The original bash completion (which started it all!) that is no longer
+evolving and a new V2 version which is aligned with the completion solution for other shells supported by Cobra.  We
+recommend you use bash completion V2.
 
-A new V2 bash completion version is also available.  This version can be used by calling `GenBashCompletionV2()` or
-`GenBashCompletionFileV2()`.  The V2 version does **not** support the legacy dynamic completion
-(see [Bash Completions](bash_completions.md)) but instead works only with the Go dynamic completion
-solution described in this document.
-Unless your program already uses the legacy dynamic completion solution, it is recommended that you use the bash
-completion V2 solution which provides the following extra features:
+Note that the V2 version does **not** support the legacy dynamic completion solution (see [Bash Completions](bash_completions.md)) 
+but instead works only with the Go dynamic completion solution described in this document.
+Unless your program already uses the legacy dynamic completion solution (meaning that your program defines a
+`BashCompletionFunction` variable), it is recommended that you use the bash completion V2 solution which beyond being
+actively maintained, provides the following extra features:
 - Supports completion descriptions (like the other shells)
-- Small completion script of less than 300 lines (v1 generates scripts of thousands of lines; `kubectl` for example has a bash v1 completion script of over 13K lines)
-- Streamlined user experience thanks to a completion behavior aligned with the other shells 
+- Small completion script of less than 350 lines (v1 generates scripts of thousands of lines; `kubectl` for example has a bash v1 completion script of over 13K lines)
+- Streamlined user experience thanks to a completion behavior aligned with the other shells
 
-`Bash` completion V2 supports descriptions for completions. When calling `GenBashCompletionV2()` or `GenBashCompletionFileV2()`
+`Bash` completion V2 can be used by calling `GenBashCompletionV2()` or `GenBashCompletionFileV2()`. 
+As it supports descriptions for completions, when calling `GenBashCompletionV2()` or `GenBashCompletionFileV2()`
 you must provide these functions with a parameter indicating if the completions should be annotated with a description; Cobra
 will provide the description automatically based on usage information.  You can choose to make this option configurable by
 your users.
@@ -440,7 +441,8 @@ show    (show information of a chart)
 $ helm s[tab][tab]
 search  show  status
 ```
-**Note**: Cobra's default `completion` command uses bash completion V2.  If for some reason you need to use bash completion V1, you will need to implement your own `completion` command. 
+**Note**: Cobra's default `completion` command uses bash completion V2.  If for some reason you need to use bash completion V1, you will need to implement your own `completion` command.
+
 ## Zsh completions
 
 Cobra supports native zsh completion generated from the root `cobra.Command`.