From 3736e5e7b8b78f4408bc35d44065f4da6433d6cc Mon Sep 17 00:00:00 2001 From: Francis Nickels <fnickels@riotgames.com> Date: Mon, 3 Mar 2025 14:18:10 -0800 Subject: [PATCH 1/6] Add AllowCustomFlagCompletions field to Command struct for enhanced flag completion handling --- command.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/command.go b/command.go index 140fbfdf..ec43d7c5 100644 --- a/command.go +++ b/command.go @@ -257,6 +257,11 @@ type Command struct { // SuggestionsMinimumDistance defines minimum levenshtein distance to display suggestions. // Must be > 0. SuggestionsMinimumDistance int + + // When performing commandline completions specific to handling flags only, override + // Cobra's default behavior and allow ValidArgsFunction to be used if it exists, otherwise + // use the default behavior + AllowCustomFlagCompletions bool } // Context returns underlying command context. If command was executed From 39a6bbba47e707c64f67fee5620301716cfb08d0 Mon Sep 17 00:00:00 2001 From: Francis Nickels <fnickels@riotgames.com> Date: Mon, 3 Mar 2025 14:31:20 -0800 Subject: [PATCH 2/6] Add HasCustomFlagCompletion method to Command struct for flag completion validation --- command.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/command.go b/command.go index ec43d7c5..470c7517 100644 --- a/command.go +++ b/command.go @@ -1607,6 +1607,17 @@ func (c *Command) HasSubCommands() bool { return len(c.commands) > 0 } +// IsAvailableCommand determines if a command is available as a non-help command +// (this includes all non deprecated/hidden commands). +func (c *Command) HasCustomFlagCompletion() bool { + + if c.ValidArgsFunction == nil { + return false + } + + return c.AllowCustomFlagCompletions +} + // IsAvailableCommand determines if a command is available as a non-help command // (this includes all non deprecated/hidden commands). func (c *Command) IsAvailableCommand() bool { From f59f7ace1483a2ecaca10a7581c458d614d5dfb6 Mon Sep 17 00:00:00 2001 From: Francis Nickels <fnickels@riotgames.com> Date: Mon, 3 Mar 2025 14:39:19 -0800 Subject: [PATCH 3/6] Refactor HasCustomFlagCompletion method documentation for clarity on custom flag completions --- command.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/command.go b/command.go index 470c7517..45b47c95 100644 --- a/command.go +++ b/command.go @@ -1607,10 +1607,11 @@ func (c *Command) HasSubCommands() bool { return len(c.commands) > 0 } -// IsAvailableCommand determines if a command is available as a non-help command -// (this includes all non deprecated/hidden commands). +// HasCustomFlagCompletion determines if a command has a defined +// ValidArgsFunction, if it does, then it returns the value of +// the AllowCustomFlagCompletions field indicating if the function +// should be used for handling custom flag completions. func (c *Command) HasCustomFlagCompletion() bool { - if c.ValidArgsFunction == nil { return false } From eeca8112eb7b796796a1a59c8e3aad537b1f6812 Mon Sep 17 00:00:00 2001 From: Francis Nickels <fnickels@riotgames.com> Date: Mon, 3 Mar 2025 14:41:38 -0800 Subject: [PATCH 4/6] Update flagCompletion logic to respect HasCustomFlagCompletion method --- completions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/completions.go b/completions.go index a1752f76..b9561f9d 100644 --- a/completions.go +++ b/completions.go @@ -358,7 +358,7 @@ func (c *Command) getCompletions(args []string) (*Command, []Completion, ShellCo // This works by counting the arguments. Normally -- is not counted as arg but // if -- was already set or interspersed is false and there is already one arg then // the extra added -- is counted as arg. - flagCompletion := true + flagCompletion := !finalCmd.HasCustomFlagCompletion() _ = finalCmd.ParseFlags(append(finalArgs, "--")) newArgCount := finalCmd.Flags().NArg() From c0e0fcc8d16dcbbf9670e620cef9668f7354cf3d Mon Sep 17 00:00:00 2001 From: Francis Nickels <fnickels@riotgames.com> Date: Mon, 3 Mar 2025 15:04:04 -0800 Subject: [PATCH 5/6] Add unit tests for HasCustomFlagCompletion method in Command struct --- command_test.go | 80 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/command_test.go b/command_test.go index 156df9eb..dff67b18 100644 --- a/command_test.go +++ b/command_test.go @@ -2952,3 +2952,83 @@ func TestHelpFuncExecuted(t *testing.T) { checkStringContains(t, output, helpText) } + +func TestHasCustomFlagCompletion(t *testing.T) { + + testCases := []struct { + name string + cmd *Command + expectedResult bool + }{ + { + name: "HasCustomFlagCompletion() AllowCustomFlagCompletions not set", + cmd: &Command{ + Use: "c", + Run: emptyRun, + }, + expectedResult: false, + }, + { + name: "HasCustomFlagCompletion() AllowCustomFlagCompletions set true", + cmd: &Command{ + Use: "c", + Run: emptyRun, + AllowCustomFlagCompletions: true, + }, + expectedResult: false, + }, + { + name: "HasCustomFlagCompletion() AllowCustomFlagCompletions set false", + cmd: &Command{ + Use: "c", + Run: emptyRun, + AllowCustomFlagCompletions: false, + }, + expectedResult: false, + }, + { + name: "HasCustomFlagCompletion() AllowCustomFlagCompletions not set with Valid", + cmd: &Command{ + Use: "c", + Run: emptyRun, + ValidArgsFunction: func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) { + return []string{}, ShellCompDirectiveNoFileComp + }, + }, + expectedResult: false, + }, + { + name: "HasCustomFlagCompletion() AllowCustomFlagCompletions set true", + cmd: &Command{ + Use: "c", + Run: emptyRun, + AllowCustomFlagCompletions: true, + ValidArgsFunction: func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) { + return []string{}, ShellCompDirectiveNoFileComp + }, + }, + expectedResult: true, + }, + { + name: "HasCustomFlagCompletion() AllowCustomFlagCompletions set false", + cmd: &Command{ + Use: "c", + Run: emptyRun, + AllowCustomFlagCompletions: false, + ValidArgsFunction: func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) { + return []string{}, ShellCompDirectiveNoFileComp + }, + }, + expectedResult: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + if tc.cmd.HasCustomFlagCompletion() != tc.expectedResult { + t.Errorf("did not return expected results") + } + }) + } + +} From 84f8ee7fefafd73a1ca783967d07fed3c96e6278 Mon Sep 17 00:00:00 2001 From: Francis Nickels <fnickels@riotgames.com> Date: Mon, 3 Mar 2025 16:16:01 -0800 Subject: [PATCH 6/6] Add tests for flag name completion with and without valid functions --- completions_test.go | 291 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 291 insertions(+) diff --git a/completions_test.go b/completions_test.go index 89da3d50..98f0ab8f 100644 --- a/completions_test.go +++ b/completions_test.go @@ -4016,3 +4016,294 @@ func TestInitDefaultCompletionCmd(t *testing.T) { }) } } + +func TestFlagNameCompletionAllowWithoutValidFunc(t *testing.T) { + rootCmd := &Command{ + Use: "root", + Run: emptyRun, + AllowCustomFlagCompletions: true, + } + childCmd := &Command{ + Use: "childCmd", + Short: "first command", + Run: emptyRun, + } + rootCmd.AddCommand(childCmd) + + rootCmd.Flags().IntP("first", "f", -1, "first flag") + firstFlag := rootCmd.Flags().Lookup("first") + + rootCmd.Flags().BoolP("second", "s", false, "second flag") + secondFlag := rootCmd.Flags().Lookup("second") + + rootCmd.Flags().StringArrayP("array", "a", nil, "array flag") + arrayFlag := rootCmd.Flags().Lookup("array") + + rootCmd.Flags().IntSliceP("slice", "l", nil, "slice flag") + sliceFlag := rootCmd.Flags().Lookup("slice") + + rootCmd.Flags().BoolSliceP("bslice", "b", nil, "bool slice flag") + bsliceFlag := rootCmd.Flags().Lookup("bslice") + + rootCmd.Flags().VarP(&customMultiString{}, "multi", "m", "multi string flag") + multiFlag := rootCmd.Flags().Lookup("multi") + + // Test that flag names are not repeated unless they are an array or slice + output, err := executeCommand(rootCmd, ShellCompNoDescRequestCmd, "--first", "1", "--") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + // Reset the flag for the next command + firstFlag.Changed = false + + expected := strings.Join([]string{ + "--array", + "--bslice", + "--help", + "--multi", + "--second", + "--slice", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") + + if output != expected { + t.Errorf("expected: %q, got: %q", expected, output) + } + + // Test that flag names are not repeated unless they are an array or slice + output, err = executeCommand(rootCmd, ShellCompNoDescRequestCmd, "--first", "1", "--second=false", "--") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + // Reset the flag for the next command + firstFlag.Changed = false + secondFlag.Changed = false + + expected = strings.Join([]string{ + "--array", + "--bslice", + "--help", + "--multi", + "--slice", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") + + if output != expected { + t.Errorf("expected: %q, got: %q", expected, output) + } + + // Test that flag names are not repeated unless they are an array or slice + output, err = executeCommand(rootCmd, ShellCompNoDescRequestCmd, "--slice", "1", "--slice=2", "--array", "val", "--bslice", "true", "--multi", "val", "--") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + // Reset the flag for the next command + sliceFlag.Changed = false + arrayFlag.Changed = false + bsliceFlag.Changed = false + multiFlag.Changed = false + + expected = strings.Join([]string{ + "--array", + "--bslice", + "--first", + "--help", + "--multi", + "--second", + "--slice", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") + + if output != expected { + t.Errorf("expected: %q, got: %q", expected, output) + } + + // Test that flag names are not repeated unless they are an array or slice, using shortname + output, err = executeCommand(rootCmd, ShellCompNoDescRequestCmd, "-l", "1", "-l=2", "-a", "val", "-") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + // Reset the flag for the next command + sliceFlag.Changed = false + arrayFlag.Changed = false + multiFlag.Changed = false + + expected = strings.Join([]string{ + "--array", + "-a", + "--bslice", + "-b", + "--first", + "-f", + "--help", + "-h", + "--multi", + "-m", + "--second", + "-s", + "--slice", + "-l", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") + + if output != expected { + t.Errorf("expected: %q, got: %q", expected, output) + } + + // Test that flag names are not repeated unless they are an array or slice, using shortname with prefix + output, err = executeCommand(rootCmd, ShellCompNoDescRequestCmd, "-l", "1", "-l=2", "-a", "val", "-a") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + // Reset the flag for the next command + sliceFlag.Changed = false + arrayFlag.Changed = false + multiFlag.Changed = false + + expected = strings.Join([]string{ + "-a", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") + + if output != expected { + t.Errorf("expected: %q, got: %q", expected, output) + } +} + +func TestFlagNameCompletionAllowWithValidFunc(t *testing.T) { + rootCmd := &Command{ + Use: "root", + Run: emptyRun, + AllowCustomFlagCompletions: true, + ValidArgsFunction: func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) { + return []string{"--bogus-flag", "-z", "non-match-value"}, ShellCompDirectiveNoFileComp + }, + } + childCmd := &Command{ + Use: "childCmd", + Short: "first command", + Run: emptyRun, + } + rootCmd.AddCommand(childCmd) + + rootCmd.Flags().IntP("first", "f", -1, "first flag") + firstFlag := rootCmd.Flags().Lookup("first") + + rootCmd.Flags().BoolP("second", "s", false, "second flag") + secondFlag := rootCmd.Flags().Lookup("second") + + rootCmd.Flags().StringArrayP("array", "a", nil, "array flag") + arrayFlag := rootCmd.Flags().Lookup("array") + + rootCmd.Flags().IntSliceP("slice", "l", nil, "slice flag") + sliceFlag := rootCmd.Flags().Lookup("slice") + + rootCmd.Flags().BoolSliceP("bslice", "b", nil, "bool slice flag") + bsliceFlag := rootCmd.Flags().Lookup("bslice") + + rootCmd.Flags().VarP(&customMultiString{}, "multi", "m", "multi string flag") + multiFlag := rootCmd.Flags().Lookup("multi") + + // Test that flag names are not repeated unless they are an array or slice + output, err := executeCommand(rootCmd, ShellCompNoDescRequestCmd, "--first", "1", "--") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + // Reset the flag for the next command + firstFlag.Changed = false + + expected := strings.Join([]string{ + "--bogus-flag", + "-z", + "non-match-value", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") + + if output != expected { + t.Errorf("expected: %q, got: %q", expected, output) + } + + // Test that flag names are not repeated unless they are an array or slice + output, err = executeCommand(rootCmd, ShellCompNoDescRequestCmd, "--first", "1", "--second=false", "--") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + // Reset the flag for the next command + firstFlag.Changed = false + secondFlag.Changed = false + + expected = strings.Join([]string{ + "--bogus-flag", + "-z", + "non-match-value", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") + + if output != expected { + t.Errorf("expected: %q, got: %q", expected, output) + } + + // Test that flag names are not repeated unless they are an array or slice + output, err = executeCommand(rootCmd, ShellCompNoDescRequestCmd, "--slice", "1", "--slice=2", "--array", "val", "--bslice", "true", "--multi", "val", "--") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + // Reset the flag for the next command + sliceFlag.Changed = false + arrayFlag.Changed = false + bsliceFlag.Changed = false + multiFlag.Changed = false + + expected = strings.Join([]string{ + "--bogus-flag", + "-z", + "non-match-value", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") + + if output != expected { + t.Errorf("expected: %q, got: %q", expected, output) + } + + // Test that flag names are not repeated unless they are an array or slice, using shortname + output, err = executeCommand(rootCmd, ShellCompNoDescRequestCmd, "-l", "1", "-l=2", "-a", "val", "-") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + // Reset the flag for the next command + sliceFlag.Changed = false + arrayFlag.Changed = false + multiFlag.Changed = false + + expected = strings.Join([]string{ + "--bogus-flag", + "-z", + "non-match-value", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") + + if output != expected { + t.Errorf("expected: %q, got: %q", expected, output) + } + + // Test that flag names are not repeated unless they are an array or slice, using shortname with prefix + output, err = executeCommand(rootCmd, ShellCompNoDescRequestCmd, "-l", "1", "-l=2", "-a", "val", "-a") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + // Reset the flag for the next command + sliceFlag.Changed = false + arrayFlag.Changed = false + multiFlag.Changed = false + + expected = strings.Join([]string{ + "--bogus-flag", + "-z", + "non-match-value", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n") + + if output != expected { + t.Errorf("expected: %q, got: %q", expected, output) + } +}