From 906c1f36decb2fc79f0c26ce12218ac1ea7d34f0 Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Sat, 11 Jul 2020 07:06:59 -0400 Subject: [PATCH] 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 --- bash_completions.go | 71 +++++++++++++++++++++----------------- custom_completions.go | 70 ++++++++++++++++--------------------- custom_completions_test.go | 53 ++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 71 deletions(-) diff --git a/bash_completions.go b/bash_completions.go index ab428ccb8c..1c8bc31bfc 100644 --- a/bash_completions.go +++ b/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) { diff --git a/custom_completions.go b/custom_completions.go index c25c03e407..edffb63344 100644 --- a/custom_completions.go +++ b/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() } diff --git a/custom_completions_test.go b/custom_completions_test.go index 276b8a77ba..1c3ddc035a 100644 --- a/custom_completions_test.go +++ b/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) + } +}