DisableFlagParsing must disable flag completion (#1161)

When a command has set DisableFlagParsing=true, it means Cobra should
not be handling flags for that command but should let the command
handle them itself.  In fact, Cobra normally won't have been told at all
about flags for this command.

Not knowing about the flags of the command also implies that Cobra
cannot perform flag completion as it does not have the necessary info.

This commit still tries to do flag name completion, but when
DisableFlagParsing==true, it continues on so that ValidArgsFunction will
be called; this allows the program to handle completion for its own
flags when DisableFlagParsing==true.

Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>

bash_completions.go

@@ -193,7 +193,13 @@ __%[1]s_handle_reply()
                     fi
                 fi
             fi
+
+            if [[ -z "${flag_parsing_disabled}" ]]; then
+                # If flag parsing is enabled, we have completed the flags and can return.
+                # If flag parsing is disabled, we may not know all (or any) of the flags, so we fallthrough
+                # to possibly call handle_go_custom_completion.
                 return 0;
+            fi
             ;;
     esac
 
@@ -394,6 +400,7 @@ func writePostscript(buf io.StringWriter, name string) {
     fi
 
     local c=0
+    local flag_parsing_disabled=
     local flags=()
     local two_word_flags=()
     local local_nonpersistent_flags=()
@@ -535,6 +542,11 @@ func writeFlags(buf io.StringWriter, cmd *Command) {
     flags_completion=()
 
 `)
+
+	if cmd.DisableFlagParsing {
+		WriteStringAndCheck(buf, "    flag_parsing_disabled=1\n")
+	}
+
 	localNonPersistentFlags := cmd.LocalNonPersistentFlags()
 	cmd.NonInheritedFlags().VisitAll(func(flag *pflag.Flag) {
 		if nonCompletableFlag(flag) {

completions.go

@@ -266,6 +266,12 @@ func (c *Command) getCompletions(args []string) (*Command, []string, ShellCompDi
 		}
 	}
 
+	// We only remove the flags from the arguments if DisableFlagParsing is not set.
+	// This is important for commands which have requested to do their own flag completion.
+	if !finalCmd.DisableFlagParsing {
+		finalArgs = finalCmd.Flags().Args()
+	}
+
 	if flag != nil && flagCompletion {
 		// Check if we are completing a flag value subject to annotations
 		if validExts, present := flag.Annotations[BashCompFilenameExt]; present {
@@ -290,12 +296,16 @@ func (c *Command) getCompletions(args []string) (*Command, []string, ShellCompDi
 		}
 	}
 
+	var completions []string
+	var directive ShellCompDirective
+
+	// Note that we want to perform flagname completion even if finalCmd.DisableFlagParsing==true;
+	// doing this allows for completion of persistant flag names even for commands that disable flag parsing.
+	//
 	// When doing completion of a flag name, as soon as an argument starts with
 	// a '-' we know it is a flag.  We cannot use isFlagArg() here as it requires
 	// the flag name to be complete
 	if flag == nil && len(toComplete) > 0 && toComplete[0] == '-' && !strings.Contains(toComplete, "=") && flagCompletion {
-		var completions []string
-
 		// First check for required flags
 		completions = completeRequireFlags(finalCmd, toComplete)
 
@@ -322,23 +332,22 @@ func (c *Command) getCompletions(args []string) (*Command, []string, ShellCompDi
 			})
 		}
 
-		directive := ShellCompDirectiveNoFileComp
+		directive = ShellCompDirectiveNoFileComp
 		if len(completions) == 1 && strings.HasSuffix(completions[0], "=") {
 			// If there is a single completion, the shell usually adds a space
 			// after the completion.  We don't want that if the flag ends with an =
 			directive = ShellCompDirectiveNoSpace
 		}
+
+		if !finalCmd.DisableFlagParsing {
+			// If DisableFlagParsing==false, we have completed the flags as known by Cobra;
+			// we can return what we found.
+			// If DisableFlagParsing==true, Cobra may not be aware of all flags, so we
+			// let the logic continue to see if ValidArgsFunction needs to be called.
 			return finalCmd, completions, directive, nil
 		}
-
-	// We only remove the flags from the arguments if DisableFlagParsing is not set.
-	// This is important for commands which have requested to do their own flag completion.
-	if !finalCmd.DisableFlagParsing {
-		finalArgs = finalCmd.Flags().Args()
-	}
-
-	var completions []string
-	directive := ShellCompDirectiveDefault
+	} else {
+		directive = ShellCompDirectiveDefault
 		if flag == nil {
 			foundLocalNonPersistentFlag := false
 			// If TraverseChildren is true on the root command we don't check for
@@ -403,6 +412,7 @@ func (c *Command) getCompletions(args []string) (*Command, []string, ShellCompDi
 			// even if we already found sub-commands.
 			// This is for commands that have subcommands but also specify a ValidArgsFunction.
 		}
+	}
 
 	// Find the completion function for the flag or command
 	var completionFn func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective)

completions_test.go

@@ -2543,3 +2543,64 @@ func TestMultipleShorthandFlagCompletion(t *testing.T) {
 		t.Errorf("expected: %q, got: %q", expected, output)
 	}
 }
+
+func TestCompleteWithDisableFlagParsing(t *testing.T) {
+
+	flagValidArgs := func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) {
+		return []string{"--flag", "-f"}, ShellCompDirectiveNoFileComp
+	}
+
+	rootCmd := &Command{Use: "root", Args: NoArgs, Run: emptyRun}
+	childCmd := &Command{
+		Use:                "child",
+		Run:                emptyRun,
+		DisableFlagParsing: true,
+		ValidArgsFunction:  flagValidArgs,
+	}
+	rootCmd.AddCommand(childCmd)
+
+	rootCmd.PersistentFlags().StringP("persistent", "p", "", "persistent flag")
+	childCmd.Flags().StringP("nonPersistent", "n", "", "non-persistent flag")
+
+	// Test that when DisableFlagParsing==true, ValidArgsFunction is called to complete flag names,
+	// after Cobra tried to complete the flags it knows about.
+	childCmd.DisableFlagParsing = true
+	output, err := executeCommand(rootCmd, ShellCompNoDescRequestCmd, "child", "-")
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	expected := strings.Join([]string{
+		"--persistent",
+		"-p",
+		"--nonPersistent",
+		"-n",
+		"--flag",
+		"-f",
+		":4",
+		"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n")
+
+	if output != expected {
+		t.Errorf("expected: %q, got: %q", expected, output)
+	}
+
+	// Test that when DisableFlagParsing==false, Cobra completes the flags itself and ValidArgsFunction is not called
+	childCmd.DisableFlagParsing = false
+	output, err = executeCommand(rootCmd, ShellCompNoDescRequestCmd, "child", "-")
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	// Cobra was not told of any flags, so it returns nothing
+	expected = strings.Join([]string{
+		"--persistent",
+		"-p",
+		"--nonPersistent",
+		"-n",
+		":4",
+		"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n")
+
+	if output != expected {
+		t.Errorf("expected: %q, got: %q", expected, output)
+	}
+}