From ccb45fbd604ecad8328b875b215a73969a1b330a Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Mon, 19 Sep 2022 08:18:05 -0400 Subject: [PATCH] Include --help and --version flag in completion 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 --- command.go | 4 + completions.go | 24 +++++ completions_test.go | 234 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 257 insertions(+), 5 deletions(-) diff --git a/command.go b/command.go index cdfd360f8..e7e976932 100644 --- a/command.go +++ b/command.go @@ -30,6 +30,8 @@ import ( flag "github.com/spf13/pflag" ) +const FlagSetByCobraAnnotation = "cobra_annotation_flag_set_by_cobra" + // FParseErrWhitelist configures Flag parse errors to be ignored type FParseErrWhitelist flag.ParseErrorsWhitelist @@ -1055,6 +1057,7 @@ func (c *Command) InitDefaultHelpFlag() { usage += c.Name() } c.Flags().BoolP("help", "h", false, usage) + _ = c.Flags().SetAnnotation("help", FlagSetByCobraAnnotation, []string{"true"}) } } @@ -1080,6 +1083,7 @@ func (c *Command) InitDefaultVersionFlag() { } else { c.Flags().Bool("version", false, usage) } + _ = c.Flags().SetAnnotation("version", FlagSetByCobraAnnotation, []string{"true"}) } } diff --git a/completions.go b/completions.go index f8bf4f69d..d8fa1f774 100644 --- a/completions.go +++ b/completions.go @@ -274,6 +274,12 @@ func (c *Command) getCompletions(args []string) (*Command, []string, ShellCompDi } 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. // 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 @@ -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. // This is important for commands which have requested to do their own flag completion. if !finalCmd.DisableFlagParsing { @@ -477,6 +489,18 @@ func (c *Command) getCompletions(args []string) (*Command, []string, ShellCompDi 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 { if nonCompletableFlag(flag) { return []string{} diff --git a/completions_test.go b/completions_test.go index c5f11fa52..abac12e46 100644 --- a/completions_test.go +++ b/completions_test.go @@ -493,8 +493,9 @@ func TestFlagNameCompletionInGo(t *testing.T) { Run: emptyRun, } childCmd := &Command{ - Use: "childCmd", - Run: emptyRun, + Use: "childCmd", + Version: "1.2.3", + Run: emptyRun, } rootCmd.AddCommand(childCmd) @@ -528,6 +529,8 @@ func TestFlagNameCompletionInGo(t *testing.T) { expected = strings.Join([]string{ "--first", "-f", + "--help", + "-h", "--second", "-s", ":4", @@ -561,7 +564,11 @@ func TestFlagNameCompletionInGo(t *testing.T) { expected = strings.Join([]string{ "--second", "-s", + "--help", + "-h", "--subFlag", + "--version", + "-v", ":4", "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") @@ -576,9 +583,10 @@ func TestFlagNameCompletionInGoWithDesc(t *testing.T) { Run: emptyRun, } childCmd := &Command{ - Use: "childCmd", - Short: "first command", - Run: emptyRun, + Use: "childCmd", + Short: "first command", + Version: "1.2.3", + Run: emptyRun, } rootCmd.AddCommand(childCmd) @@ -612,6 +620,8 @@ func TestFlagNameCompletionInGoWithDesc(t *testing.T) { expected = strings.Join([]string{ "--first\tfirst flag", "-f\tfirst flag", + "--help\thelp for root", + "-h\thelp for root", "--second\tsecond flag", "-s\tsecond flag", ":4", @@ -645,7 +655,11 @@ func TestFlagNameCompletionInGoWithDesc(t *testing.T) { expected = strings.Join([]string{ "--second\tsecond flag", "-s\tsecond flag", + "--help\thelp for childCmd", + "-h\thelp for childCmd", "--subFlag\tsub flag", + "--version\tversion for childCmd", + "-v\tversion for childCmd", ":4", "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") @@ -688,6 +702,7 @@ func TestFlagNameCompletionRepeat(t *testing.T) { expected := strings.Join([]string{ "--array", "--bslice", + "--help", "--second", "--slice", ":4", @@ -709,6 +724,7 @@ func TestFlagNameCompletionRepeat(t *testing.T) { expected = strings.Join([]string{ "--array", "--bslice", + "--help", "--slice", ":4", "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") @@ -731,6 +747,7 @@ func TestFlagNameCompletionRepeat(t *testing.T) { "--array", "--bslice", "--first", + "--help", "--second", "--slice", ":4", @@ -756,6 +773,8 @@ func TestFlagNameCompletionRepeat(t *testing.T) { "-b", "--first", "-f", + "--help", + "-h", "--second", "-s", "--slice", @@ -1792,6 +1811,7 @@ func TestFlagCompletionWithNotInterspersedArgs(t *testing.T) { expected := strings.Join([]string{ "--bool\ttest bool flag", + "--help\thelp for child", "--string\ttest string flag", ":4", "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") @@ -2602,6 +2622,8 @@ func TestCompleteWithDisableFlagParsing(t *testing.T) { expected := strings.Join([]string{ "--persistent", "-p", + "--help", + "-h", "--nonPersistent", "-n", "--flag", @@ -2624,6 +2646,8 @@ func TestCompleteWithDisableFlagParsing(t *testing.T) { expected = strings.Join([]string{ "--persistent", "-p", + "--help", + "-h", "--nonPersistent", "-n", ":4", @@ -2753,6 +2777,8 @@ func TestCompletionForGroupedFlags(t *testing.T) { expectedOutput: strings.Join([]string{ "--ingroup1", "--ingroup2", + "--help", + "-h", "--ingroup3", "--nogroup", ":4", @@ -2851,6 +2877,8 @@ func TestCompletionForMutuallyExclusiveFlags(t *testing.T) { expectedOutput: strings.Join([]string{ "--ingroup1", "--ingroup2", + "--help", + "-h", "--ingroup3", "--nogroup", ":4", @@ -2861,6 +2889,8 @@ func TestCompletionForMutuallyExclusiveFlags(t *testing.T) { args: []string{"child", "--ingroup1", "8", "-"}, expectedOutput: strings.Join([]string{ "--ingroup1", // Should be suggested again since it is a slice + "--help", + "-h", "--nogroup", ":4", "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"), @@ -2869,6 +2899,8 @@ func TestCompletionForMutuallyExclusiveFlags(t *testing.T) { desc: "group ignored if some flags not applicable", args: []string{"--ingroup1", "8", "-"}, expectedOutput: strings.Join([]string{ + "--help", + "-h", "--ingroup1", "--ingroup2", ":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) + } + }) + } +}