From bafccb0be850cd588ffd5eb4af0d8586d6bfe19c Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Tue, 3 Jan 2023 03:03:15 +0000 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. Merge https://github.com/spf13/cobra/pull/1813 --- command.go | 8 +- completions.go | 24 +++++ completions_test.go | 233 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 258 insertions(+), 7 deletions(-) diff --git a/command.go b/command.go index c97fe37..2a8b540 100644 --- a/command.go +++ b/command.go @@ -32,6 +32,8 @@ import ( "github.com/zulucmd/zulu/internal/util" ) +const FlagSetByCobraAnnotation = "cobra_annotation_flag_set_by_cobra" + //go:embed templates/* var tmplFS embed.FS @@ -1208,7 +1210,7 @@ func (c *Command) InitDefaultHelpFlag() { } else { usage += c.Name() } - c.Flags().Bool("help", false, usage, zflag.OptShorthand('h')) + c.Flags().Bool("help", false, usage, zflag.OptShorthand('h'), zflag.OptAnnotation(FlagSetByCobraAnnotation, []string{"true"})) } } @@ -1230,7 +1232,9 @@ func (c *Command) InitDefaultVersionFlag() { usage += c.Name() } - var opts []zflag.Opt + opts := []zflag.Opt{ + zflag.OptAnnotation(FlagSetByCobraAnnotation, []string{"true"}), + } if c.Flags().ShorthandLookup('v') == nil { opts = append(opts, zflag.OptShorthand('v')) } diff --git a/completions.go b/completions.go index 452ab31..4d74c52 100644 --- a/completions.go +++ b/completions.go @@ -250,6 +250,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 @@ -282,6 +288,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 { @@ -451,6 +463,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 *zflag.Flag, toComplete string) []string { if nonCompletableFlag(flag) { return []string{} diff --git a/completions_test.go b/completions_test.go index efb9c6b..6c9f79e 100644 --- a/completions_test.go +++ b/completions_test.go @@ -482,8 +482,9 @@ func TestFlagNameCompletionInGo(t *testing.T) { RunE: noopRun, } childCmd := &zulu.Command{ - Use: "childCmd", - RunE: noopRun, + Use: "childCmd", + Version: "1.2.3", + RunE: noopRun, } rootCmd.AddCommand(childCmd) @@ -517,6 +518,8 @@ func TestFlagNameCompletionInGo(t *testing.T) { expected = strings.Join([]string{ "--first", "-f", + "--help", + "-h", "--second", "-s", ":4", @@ -550,7 +553,11 @@ func TestFlagNameCompletionInGo(t *testing.T) { expected = strings.Join([]string{ "--second", "-s", + "--help", + "-h", "--subFlag", + "--version", + "-v", ":4", "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") @@ -565,9 +572,10 @@ func TestFlagNameCompletionInGoWithDesc(t *testing.T) { RunE: noopRun, } childCmd := &zulu.Command{ - Use: "childCmd", - Short: "first command", - RunE: noopRun, + Use: "childCmd", + Short: "first command", + Version: "1.2.3", + RunE: noopRun, } rootCmd.AddCommand(childCmd) @@ -601,6 +609,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", @@ -634,7 +644,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") @@ -674,6 +688,7 @@ func TestFlagNameCompletionRepeat(t *testing.T) { expected := strings.Join([]string{ "--bslice", + "--help", "--second", "--slice", ":4", @@ -694,6 +709,7 @@ func TestFlagNameCompletionRepeat(t *testing.T) { expected = strings.Join([]string{ "--bslice", + "--help", "--slice", ":4", "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") @@ -714,6 +730,7 @@ func TestFlagNameCompletionRepeat(t *testing.T) { expected = strings.Join([]string{ "--bslice", "--first", + "--help", "--second", "--slice", ":4", @@ -736,6 +753,8 @@ func TestFlagNameCompletionRepeat(t *testing.T) { "-b", "--first", "-f", + "--help", + "-h", "--second", "-s", "--slice", @@ -1716,6 +1735,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") @@ -2530,6 +2550,8 @@ func TestCompleteWithDisableFlagParsing(t *testing.T) { expected := strings.Join([]string{ "--persistent", "-p", + "--help", + "-h", "--nonPersistent", "-n", "--flag", @@ -2552,6 +2574,8 @@ func TestCompleteWithDisableFlagParsing(t *testing.T) { expected = strings.Join([]string{ "--persistent", "-p", + "--help", + "-h", "--nonPersistent", "-n", ":4", @@ -2681,6 +2705,8 @@ func TestCompletionForGroupedFlags(t *testing.T) { expectedOutput: strings.Join([]string{ "--ingroup1", "--ingroup2", + "--help", + "-h", "--ingroup3", "--nogroup", ":4", @@ -2779,6 +2805,8 @@ func TestCompletionForMutuallyExclusiveFlags(t *testing.T) { expectedOutput: strings.Join([]string{ "--ingroup1", "--ingroup2", + "--help", + "-h", "--ingroup3", "--nogroup", ":4", @@ -2789,6 +2817,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"), @@ -2797,6 +2827,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", @@ -2819,3 +2851,194 @@ func TestCompletionForMutuallyExclusiveFlags(t *testing.T) { }) } } + +func TestCompletionCobraFlags(t *testing.T) { + getCmd := func() *zulu.Command { + rootCmd := &zulu.Command{ + Use: "root", + Version: "1.1.1", + RunE: noopRun, + } + childCmd := &zulu.Command{ + Use: "child", + Version: "1.1.1", + RunE: noopRun, + ValidArgsFunction: func(cmd *zulu.Command, args []string, toComplete string) ([]string, zulu.ShellCompDirective) { + return []string{"extra"}, zulu.ShellCompDirectiveNoFileComp + }, + } + childCmd2 := &zulu.Command{ + Use: "child2", + Version: "1.1.1", + RunE: noopRun, + ValidArgsFunction: func(cmd *zulu.Command, args []string, toComplete string) ([]string, zulu.ShellCompDirective) { + return []string{"extra2"}, zulu.ShellCompDirectiveNoFileComp + }, + } + childCmd3 := &zulu.Command{ + Use: "child3", + Version: "1.1.1", + RunE: noopRun, + ValidArgsFunction: func(cmd *zulu.Command, args []string, toComplete string) ([]string, zulu.ShellCompDirective) { + return []string{"extra3"}, zulu.ShellCompDirectiveNoFileComp + }, + } + + rootCmd.AddCommand(childCmd, childCmd2, childCmd3) + + _ = childCmd.Flags().Bool("bool", false, "A bool flag", zulu.FlagOptRequired()) + + // Have a command that adds its own help and version flag + _ = childCmd2.Flags().Bool("help", false, "My own help", zflag.OptShorthand('h')) + _ = childCmd2.Flags().Bool("version", false, "My own version", zflag.OptShorthand('v')) + + // Have a command that only adds its own -v flag + _ = childCmd3.Flags().Bool("verbose", false, "Not a version flag", zflag.OptShorthand('v')) + + 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{zulu.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) + } + }) + } +}