Include --help and --version flag in completion (#1813)

Fixes #1786

The --help, -h, --version and -v flags are normally added when the
`execute()` function is called on a command.  When doing completion
we don't call `execute()` so we need to add these flags explicitly to
the command being completed.

Also, we disable all further completions if the 'help' or 'version'
flags are present on the command-line.

Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>
This commit is contained in:
Marc Khouzam 2022-10-04 18:41:30 -04:00 committed by GitHub
parent d4040ad8db
commit 212ea40783
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 257 additions and 5 deletions

View file

@ -30,6 +30,8 @@ import (
flag "github.com/spf13/pflag" flag "github.com/spf13/pflag"
) )
const FlagSetByCobraAnnotation = "cobra_annotation_flag_set_by_cobra"
// FParseErrWhitelist configures Flag parse errors to be ignored // FParseErrWhitelist configures Flag parse errors to be ignored
type FParseErrWhitelist flag.ParseErrorsWhitelist type FParseErrWhitelist flag.ParseErrorsWhitelist
@ -1064,6 +1066,7 @@ func (c *Command) InitDefaultHelpFlag() {
usage += c.Name() usage += c.Name()
} }
c.Flags().BoolP("help", "h", false, usage) c.Flags().BoolP("help", "h", false, usage)
_ = c.Flags().SetAnnotation("help", FlagSetByCobraAnnotation, []string{"true"})
} }
} }
@ -1089,6 +1092,7 @@ func (c *Command) InitDefaultVersionFlag() {
} else { } else {
c.Flags().Bool("version", false, usage) c.Flags().Bool("version", false, usage)
} }
_ = c.Flags().SetAnnotation("version", FlagSetByCobraAnnotation, []string{"true"})
} }
} }

View file

@ -274,6 +274,12 @@ func (c *Command) getCompletions(args []string) (*Command, []string, ShellCompDi
} }
finalCmd.ctx = c.ctx finalCmd.ctx = c.ctx
// 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()
// Check if we are doing flag value completion before parsing the flags. // 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 // This is important because if we are completing a flag value, we need to also
// remove the flag name argument from the list of finalArgs or else the parsing // remove the flag name argument from the list of finalArgs or else the parsing
@ -306,6 +312,12 @@ func (c *Command) getCompletions(args []string) (*Command, []string, ShellCompDi
} }
} }
// Look for the --help or --version flags. If they are present,
// there should be no further completions.
if helpOrVersionFlagPresent(finalCmd) {
return finalCmd, []string{}, ShellCompDirectiveNoFileComp, nil
}
// We only remove the flags from the arguments if DisableFlagParsing is not set. // 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. // This is important for commands which have requested to do their own flag completion.
if !finalCmd.DisableFlagParsing { if !finalCmd.DisableFlagParsing {
@ -477,6 +489,18 @@ func (c *Command) getCompletions(args []string) (*Command, []string, ShellCompDi
return finalCmd, completions, directive, nil return finalCmd, completions, directive, nil
} }
func helpOrVersionFlagPresent(cmd *Command) bool {
if versionFlag := cmd.Flags().Lookup("version"); versionFlag != nil &&
len(versionFlag.Annotations[FlagSetByCobraAnnotation]) > 0 && versionFlag.Changed {
return true
}
if helpFlag := cmd.Flags().Lookup("help"); helpFlag != nil &&
len(helpFlag.Annotations[FlagSetByCobraAnnotation]) > 0 && helpFlag.Changed {
return true
}
return false
}
func getFlagNameCompletions(flag *pflag.Flag, toComplete string) []string { func getFlagNameCompletions(flag *pflag.Flag, toComplete string) []string {
if nonCompletableFlag(flag) { if nonCompletableFlag(flag) {
return []string{} return []string{}

View file

@ -494,6 +494,7 @@ func TestFlagNameCompletionInGo(t *testing.T) {
} }
childCmd := &Command{ childCmd := &Command{
Use: "childCmd", Use: "childCmd",
Version: "1.2.3",
Run: emptyRun, Run: emptyRun,
} }
rootCmd.AddCommand(childCmd) rootCmd.AddCommand(childCmd)
@ -528,6 +529,8 @@ func TestFlagNameCompletionInGo(t *testing.T) {
expected = strings.Join([]string{ expected = strings.Join([]string{
"--first", "--first",
"-f", "-f",
"--help",
"-h",
"--second", "--second",
"-s", "-s",
":4", ":4",
@ -561,7 +564,11 @@ func TestFlagNameCompletionInGo(t *testing.T) {
expected = strings.Join([]string{ expected = strings.Join([]string{
"--second", "--second",
"-s", "-s",
"--help",
"-h",
"--subFlag", "--subFlag",
"--version",
"-v",
":4", ":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n")
@ -578,6 +585,7 @@ func TestFlagNameCompletionInGoWithDesc(t *testing.T) {
childCmd := &Command{ childCmd := &Command{
Use: "childCmd", Use: "childCmd",
Short: "first command", Short: "first command",
Version: "1.2.3",
Run: emptyRun, Run: emptyRun,
} }
rootCmd.AddCommand(childCmd) rootCmd.AddCommand(childCmd)
@ -612,6 +620,8 @@ func TestFlagNameCompletionInGoWithDesc(t *testing.T) {
expected = strings.Join([]string{ expected = strings.Join([]string{
"--first\tfirst flag", "--first\tfirst flag",
"-f\tfirst flag", "-f\tfirst flag",
"--help\thelp for root",
"-h\thelp for root",
"--second\tsecond flag", "--second\tsecond flag",
"-s\tsecond flag", "-s\tsecond flag",
":4", ":4",
@ -645,7 +655,11 @@ func TestFlagNameCompletionInGoWithDesc(t *testing.T) {
expected = strings.Join([]string{ expected = strings.Join([]string{
"--second\tsecond flag", "--second\tsecond flag",
"-s\tsecond flag", "-s\tsecond flag",
"--help\thelp for childCmd",
"-h\thelp for childCmd",
"--subFlag\tsub flag", "--subFlag\tsub flag",
"--version\tversion for childCmd",
"-v\tversion for childCmd",
":4", ":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n")
@ -688,6 +702,7 @@ func TestFlagNameCompletionRepeat(t *testing.T) {
expected := strings.Join([]string{ expected := strings.Join([]string{
"--array", "--array",
"--bslice", "--bslice",
"--help",
"--second", "--second",
"--slice", "--slice",
":4", ":4",
@ -709,6 +724,7 @@ func TestFlagNameCompletionRepeat(t *testing.T) {
expected = strings.Join([]string{ expected = strings.Join([]string{
"--array", "--array",
"--bslice", "--bslice",
"--help",
"--slice", "--slice",
":4", ":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n")
@ -731,6 +747,7 @@ func TestFlagNameCompletionRepeat(t *testing.T) {
"--array", "--array",
"--bslice", "--bslice",
"--first", "--first",
"--help",
"--second", "--second",
"--slice", "--slice",
":4", ":4",
@ -756,6 +773,8 @@ func TestFlagNameCompletionRepeat(t *testing.T) {
"-b", "-b",
"--first", "--first",
"-f", "-f",
"--help",
"-h",
"--second", "--second",
"-s", "-s",
"--slice", "--slice",
@ -1792,6 +1811,7 @@ func TestFlagCompletionWithNotInterspersedArgs(t *testing.T) {
expected := strings.Join([]string{ expected := strings.Join([]string{
"--bool\ttest bool flag", "--bool\ttest bool flag",
"--help\thelp for child",
"--string\ttest string flag", "--string\ttest string flag",
":4", ":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n")
@ -2602,6 +2622,8 @@ func TestCompleteWithDisableFlagParsing(t *testing.T) {
expected := strings.Join([]string{ expected := strings.Join([]string{
"--persistent", "--persistent",
"-p", "-p",
"--help",
"-h",
"--nonPersistent", "--nonPersistent",
"-n", "-n",
"--flag", "--flag",
@ -2624,6 +2646,8 @@ func TestCompleteWithDisableFlagParsing(t *testing.T) {
expected = strings.Join([]string{ expected = strings.Join([]string{
"--persistent", "--persistent",
"-p", "-p",
"--help",
"-h",
"--nonPersistent", "--nonPersistent",
"-n", "-n",
":4", ":4",
@ -2753,6 +2777,8 @@ func TestCompletionForGroupedFlags(t *testing.T) {
expectedOutput: strings.Join([]string{ expectedOutput: strings.Join([]string{
"--ingroup1", "--ingroup1",
"--ingroup2", "--ingroup2",
"--help",
"-h",
"--ingroup3", "--ingroup3",
"--nogroup", "--nogroup",
":4", ":4",
@ -2851,6 +2877,8 @@ func TestCompletionForMutuallyExclusiveFlags(t *testing.T) {
expectedOutput: strings.Join([]string{ expectedOutput: strings.Join([]string{
"--ingroup1", "--ingroup1",
"--ingroup2", "--ingroup2",
"--help",
"-h",
"--ingroup3", "--ingroup3",
"--nogroup", "--nogroup",
":4", ":4",
@ -2861,6 +2889,8 @@ func TestCompletionForMutuallyExclusiveFlags(t *testing.T) {
args: []string{"child", "--ingroup1", "8", "-"}, args: []string{"child", "--ingroup1", "8", "-"},
expectedOutput: strings.Join([]string{ expectedOutput: strings.Join([]string{
"--ingroup1", // Should be suggested again since it is a slice "--ingroup1", // Should be suggested again since it is a slice
"--help",
"-h",
"--nogroup", "--nogroup",
":4", ":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"), "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
@ -2869,6 +2899,8 @@ func TestCompletionForMutuallyExclusiveFlags(t *testing.T) {
desc: "group ignored if some flags not applicable", desc: "group ignored if some flags not applicable",
args: []string{"--ingroup1", "8", "-"}, args: []string{"--ingroup1", "8", "-"},
expectedOutput: strings.Join([]string{ expectedOutput: strings.Join([]string{
"--help",
"-h",
"--ingroup1", "--ingroup1",
"--ingroup2", "--ingroup2",
":4", ":4",
@ -2891,3 +2923,195 @@ func TestCompletionForMutuallyExclusiveFlags(t *testing.T) {
}) })
} }
} }
func TestCompletionCobraFlags(t *testing.T) {
getCmd := func() *Command {
rootCmd := &Command{
Use: "root",
Version: "1.1.1",
Run: emptyRun,
}
childCmd := &Command{
Use: "child",
Version: "1.1.1",
Run: emptyRun,
ValidArgsFunction: func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) {
return []string{"extra"}, ShellCompDirectiveNoFileComp
},
}
childCmd2 := &Command{
Use: "child2",
Version: "1.1.1",
Run: emptyRun,
ValidArgsFunction: func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) {
return []string{"extra2"}, ShellCompDirectiveNoFileComp
},
}
childCmd3 := &Command{
Use: "child3",
Version: "1.1.1",
Run: emptyRun,
ValidArgsFunction: func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) {
return []string{"extra3"}, ShellCompDirectiveNoFileComp
},
}
rootCmd.AddCommand(childCmd, childCmd2, childCmd3)
_ = childCmd.Flags().Bool("bool", false, "A bool flag")
_ = childCmd.MarkFlagRequired("bool")
// Have a command that adds its own help and version flag
_ = childCmd2.Flags().BoolP("help", "h", false, "My own help")
_ = childCmd2.Flags().BoolP("version", "v", false, "My own version")
// Have a command that only adds its own -v flag
_ = childCmd3.Flags().BoolP("verbose", "v", false, "Not a version flag")
return rootCmd
}
// Each test case uses a unique command from the function above.
testcases := []struct {
desc string
args []string
expectedOutput string
}{
{
desc: "completion of help and version flags",
args: []string{"-"},
expectedOutput: strings.Join([]string{
"--help",
"-h",
"--version",
"-v",
":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
},
{
desc: "no completion after --help flag",
args: []string{"--help", ""},
expectedOutput: strings.Join([]string{
":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
},
{
desc: "no completion after -h flag",
args: []string{"-h", ""},
expectedOutput: strings.Join([]string{
":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
},
{
desc: "no completion after --version flag",
args: []string{"--version", ""},
expectedOutput: strings.Join([]string{
":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
},
{
desc: "no completion after -v flag",
args: []string{"-v", ""},
expectedOutput: strings.Join([]string{
":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
},
{
desc: "no completion after --help flag even with other completions",
args: []string{"child", "--help", ""},
expectedOutput: strings.Join([]string{
":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
},
{
desc: "no completion after -h flag even with other completions",
args: []string{"child", "-h", ""},
expectedOutput: strings.Join([]string{
":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
},
{
desc: "no completion after --version flag even with other completions",
args: []string{"child", "--version", ""},
expectedOutput: strings.Join([]string{
":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
},
{
desc: "no completion after -v flag even with other completions",
args: []string{"child", "-v", ""},
expectedOutput: strings.Join([]string{
":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
},
{
desc: "no completion after -v flag even with other flag completions",
args: []string{"child", "-v", "-"},
expectedOutput: strings.Join([]string{
":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
},
{
desc: "completion after --help flag when created by program",
args: []string{"child2", "--help", ""},
expectedOutput: strings.Join([]string{
"extra2",
":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
},
{
desc: "completion after -h flag when created by program",
args: []string{"child2", "-h", ""},
expectedOutput: strings.Join([]string{
"extra2",
":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
},
{
desc: "completion after --version flag when created by program",
args: []string{"child2", "--version", ""},
expectedOutput: strings.Join([]string{
"extra2",
":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
},
{
desc: "completion after -v flag when created by program",
args: []string{"child2", "-v", ""},
expectedOutput: strings.Join([]string{
"extra2",
":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
},
{
desc: "completion after --version when only -v flag was created by program",
args: []string{"child3", "--version", ""},
expectedOutput: strings.Join([]string{
":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
},
{
desc: "completion after -v flag when only -v flag was created by program",
args: []string{"child3", "-v", ""},
expectedOutput: strings.Join([]string{
"extra3",
":4",
"Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"),
},
}
for _, tc := range testcases {
t.Run(tc.desc, func(t *testing.T) {
c := getCmd()
args := []string{ShellCompNoDescRequestCmd}
args = append(args, tc.args...)
output, err := executeCommand(c, args...)
switch {
case err == nil && output != tc.expectedOutput:
t.Errorf("expected: %q, got: %q", tc.expectedOutput, output)
case err != nil:
t.Errorf("Unexpected error %q", err)
}
})
}
}