DisableFlagParsing must disable flag completion

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 disables flag name completion when DisableFlagParsing==true
This has for effect that ValidArgsFunction will be called even for flag
completion when DisableFlagParsing==true, which will allow the program
to handle completion for its own flags.

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

bash_completions.go

@@ -157,43 +157,45 @@ __%[1]s_handle_reply()
     local comp
     case $cur in
         -*)
-            if [[ $(type -t compopt) = "builtin" ]]; then
-                compopt -o nospace
-            fi
-            local allflags
-            if [ ${#must_have_one_flag[@]} -ne 0 ]; then
-                allflags=("${must_have_one_flag[@]}")
-            else
-                allflags=("${flags[*]} ${two_word_flags[*]}")
-            fi
-            while IFS='' read -r comp; do
-                COMPREPLY+=("$comp")
-            done < <(compgen -W "${allflags[*]}" -- "$cur")
-            if [[ $(type -t compopt) = "builtin" ]]; then
-                [[ "${COMPREPLY[0]}" == *= ]] || compopt +o nospace
-            fi
-
-            # complete after --flag=abc
-            if [[ $cur == *=* ]]; then
+            if [[ -z "${ignore_flags}" ]]; then
+                if [[ $(type -t compopt) = "builtin" ]]; then
+                    compopt -o nospace
+                fi
+                local allflags
+                if [ ${#must_have_one_flag[@]} -ne 0 ]; then
+                    allflags=("${must_have_one_flag[@]}")
+                else
+                    allflags=("${flags[*]} ${two_word_flags[*]}")
+                fi
+                while IFS='' read -r comp; do
+                    COMPREPLY+=("$comp")
+                done < <(compgen -W "${allflags[*]}" -- "$cur")
                 if [[ $(type -t compopt) = "builtin" ]]; then
-                    compopt +o nospace
+                    [[ "${COMPREPLY[0]}" == *= ]] || compopt +o nospace
                 fi
 
-                local index flag
-                flag="${cur%%=*}"
-                __%[1]s_index_of_word "${flag}" "${flags_with_completion[@]}"
-                COMPREPLY=()
-                if [[ ${index} -ge 0 ]]; then
-                    PREFIX=""
-                    cur="${cur#*=}"
-                    ${flags_completion[${index}]}
-                    if [ -n "${ZSH_VERSION}" ]; then
-                        # zsh completion needs --flag= prefix
-                        eval "COMPREPLY=( \"\${COMPREPLY[@]/#/${flag}=}\" )"
+                # complete after --flag=abc
+                if [[ $cur == *=* ]]; then
+                    if [[ $(type -t compopt) = "builtin" ]]; then
+                        compopt +o nospace
+                    fi
+
+                    local index flag
+                    flag="${cur%%=*}"
+                    __%[1]s_index_of_word "${flag}" "${flags_with_completion[@]}"
+                    COMPREPLY=()
+                    if [[ ${index} -ge 0 ]]; then
+                        PREFIX=""
+                        cur="${cur#*=}"
+                        ${flags_completion[${index}]}
+                        if [ -n "${ZSH_VERSION}" ]; then
+                            # zsh completion needs --flag= prefix
+                            eval "COMPREPLY=( \"\${COMPREPLY[@]/#/${flag}=}\" )"
+                        fi
                     fi
                 fi
+                return 0;
             fi
-            return 0;
             ;;
     esac
 
@@ -394,6 +396,7 @@ func writePostscript(buf *bytes.Buffer, name string) {
     fi
 
     local c=0
+    local ignore_flags
     local flags=()
     local two_word_flags=()
     local local_nonpersistent_flags=()
@@ -526,6 +529,12 @@ func writeFlags(buf *bytes.Buffer, cmd *Command) {
     flags_completion=()
 
 `)
+
+	if cmd.DisableFlagParsing {
+		buf.WriteString("    ignore_flags=1\n")
+		return
+	}
+
 	localNonPersistentFlags := cmd.LocalNonPersistentFlags()
 	cmd.NonInheritedFlags().VisitAll(func(flag *pflag.Flag) {
 		if nonCompletableFlag(flag) {

custom_completions.go

@@ -221,50 +221,40 @@ func (c *Command) getCompletions(args []string) (*Command, []string, ShellCompDi
 		}
 	}
 
-	// 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, "=") {
-		var completions []string
-
-		// First check for required flags
-		completions = completeRequireFlags(finalCmd, toComplete)
-
-		// If we have not found any required flags, only then can we show regular flags
-		if len(completions) == 0 {
-			doCompleteFlags := func(flag *pflag.Flag) {
-				if !flag.Changed ||
-					strings.Contains(flag.Value.Type(), "Slice") ||
-					strings.Contains(flag.Value.Type(), "Array") {
-					// If the flag is not already present, or if it can be specified multiple times (Array or Slice)
-					// we suggest it as a completion
-					completions = append(completions, getFlagNameCompletions(flag, toComplete)...)
-				}
+	// We only handle flag completion if DisableFlagParsing is not set.
+	// This is important for commands which have requested to do their own flag completion.
+	if !finalCmd.DisableFlagParsing {
+		// 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, "=") {
+			var completions []string
+
+			// First check for required flags
+			completions = completeRequireFlags(finalCmd, toComplete)
+
+			// If we have not found any required flags, only then can we show regular flags
+			if len(completions) == 0 {
+				finalCmd.Flags().VisitAll(func(flag *pflag.Flag) {
+					if !flag.Changed ||
+						strings.Contains(flag.Value.Type(), "Slice") ||
+						strings.Contains(flag.Value.Type(), "Array") {
+						// If the flag is not already present, or if it can be specified multiple times (Array or Slice)
+						// we suggest it as a completion
+						completions = append(completions, getFlagNameCompletions(flag, toComplete)...)
+					}
+				})
 			}
 
-			// We cannot use finalCmd.Flags() because we may not have called ParsedFlags() for commands
-			// that have set DisableFlagParsing; it is ParseFlags() that merges the inherited and
-			// non-inherited flags.
-			finalCmd.InheritedFlags().VisitAll(func(flag *pflag.Flag) {
-				doCompleteFlags(flag)
-			})
-			finalCmd.NonInheritedFlags().VisitAll(func(flag *pflag.Flag) {
-				doCompleteFlags(flag)
-			})
-		}
-
-		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
+			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
+			}
+			return finalCmd, completions, directive, nil
 		}
-		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()
 	}
 

custom_completions_test.go

@@ -1898,3 +1898,56 @@ func TestCompleteHelp(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}
+	child1Cmd := &Command{
+		Use:                "child1",
+		Run:                emptyRun,
+		DisableFlagParsing: true,
+		ValidArgsFunction:  flagValidArgs,
+	}
+	child2Cmd := &Command{
+		Use:                "child2",
+		Run:                emptyRun,
+		DisableFlagParsing: false,
+		ValidArgsFunction:  flagValidArgs,
+	}
+	rootCmd.AddCommand(child1Cmd, child2Cmd)
+
+	// Test that when DisableFlagParsing==true, ValidArgsFunction is called to complete flag names
+	output, err := executeCommand(rootCmd, ShellCompNoDescRequestCmd, "child1", "--")
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	expected := strings.Join([]string{
+		"--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 (ValidArgsFunction is not called)
+	output, err = executeCommand(rootCmd, ShellCompNoDescRequestCmd, "child2", "--")
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	// Cobra was not told of any flags, so it returns nothing
+	expected = strings.Join([]string{
+		":4",
+		"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n")
+
+	if output != expected {
+		t.Errorf("expected: %q, got: %q", expected, output)
+	}
+}