From 850b5956df0d61188fad7ede776f3b84ecae0c13 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Sun, 2 Jun 2024 14:11:46 -0400 Subject: [PATCH 1/9] Fix:(issue_1916) Add additional check for -- --- command.go | 4 ++++ completion_test.go | 8 ++++++++ parse.go | 2 +- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/command.go b/command.go index 9fa8f9dcb0..d5891475fc 100644 --- a/command.go +++ b/command.go @@ -151,6 +151,8 @@ type Command struct { didSetupDefaults bool // whether in shell completion mode shellCompletion bool + // run args + runArgs []string } // FullName returns the full name of the command. @@ -184,6 +186,8 @@ func (cmd *Command) setupDefaults(osArgs []string) { cmd.didSetupDefaults = true + cmd.runArgs = osArgs + isRoot := cmd.parent == nil tracef("isRoot? %[1]v (cmd=%[2]q)", isRoot, cmd.Name) diff --git a/completion_test.go b/completion_test.go index c204ae2521..fb98c7144d 100644 --- a/completion_test.go +++ b/completion_test.go @@ -97,6 +97,14 @@ func TestCompletionSubcommand(t *testing.T) { out.String(), "-g", "Expected output to contain flag %[1]q", "-g", ) + + out.Reset() + + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", "bar", "xyz", "--", "--generate-shell-completion"})) + r.Containsf( + out.String(), "-g", + "Expected output to contain flag %[1]q", "-g", + ) } type mockWriter struct { diff --git a/parse.go b/parse.go index 8ec5d4c631..dde0e3d099 100644 --- a/parse.go +++ b/parse.go @@ -22,7 +22,7 @@ func parseIter(set *flag.FlagSet, ip iterativeParser, args []string, shellComple err := set.Parse(args) if !ip.useShortOptionHandling() || err == nil { if shellComplete { - tracef("returning nil due to shellComplete=true") + tracef("returning nil due to shellComplete=true err=%v", err) return nil } From 7e4fa0b2b85b18cfa828fb032a9b1592dd7a3a71 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Sun, 25 Aug 2024 20:16:00 -0400 Subject: [PATCH 2/9] Change test --- completion_test.go | 4 ++-- help.go | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/completion_test.go b/completion_test.go index fb98c7144d..60d7cf8378 100644 --- a/completion_test.go +++ b/completion_test.go @@ -101,9 +101,9 @@ func TestCompletionSubcommand(t *testing.T) { out.Reset() r.NoError(cmd.Run(buildTestContext(t), []string{"foo", "bar", "xyz", "--", "--generate-shell-completion"})) - r.Containsf( + r.NotContainsf( out.String(), "-g", - "Expected output to contain flag %[1]q", "-g", + "Expected output to not contain flag %[1]q", "-g", ) } diff --git a/help.go b/help.go index 20c7489709..7014b63540 100644 --- a/help.go +++ b/help.go @@ -467,6 +467,9 @@ func checkShellCompleteFlag(c *Command, arguments []string) (bool, []string) { // because after "--" only positional arguments are accepted. // https://unix.stackexchange.com/a/11382 if arg == "--" { + if lastArg == "--generate-shell-completion" { + return false, arguments[:pos-2] + } return false, arguments } } From 64dd81469a31301958efe8d5ddcf66e598441c2d Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Sun, 25 Aug 2024 20:17:10 -0400 Subject: [PATCH 3/9] Revert paarse.go changes --- parse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parse.go b/parse.go index dde0e3d099..8ec5d4c631 100644 --- a/parse.go +++ b/parse.go @@ -22,7 +22,7 @@ func parseIter(set *flag.FlagSet, ip iterativeParser, args []string, shellComple err := set.Parse(args) if !ip.useShortOptionHandling() || err == nil { if shellComplete { - tracef("returning nil due to shellComplete=true err=%v", err) + tracef("returning nil due to shellComplete=true") return nil } From 69562610b75d68984dbf69453fd8d6888efd1e6c Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Mon, 30 Sep 2024 19:36:32 -0400 Subject: [PATCH 4/9] stash --- help.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/help.go b/help.go index 7014b63540..20c7489709 100644 --- a/help.go +++ b/help.go @@ -467,9 +467,6 @@ func checkShellCompleteFlag(c *Command, arguments []string) (bool, []string) { // because after "--" only positional arguments are accepted. // https://unix.stackexchange.com/a/11382 if arg == "--" { - if lastArg == "--generate-shell-completion" { - return false, arguments[:pos-2] - } return false, arguments } } From c1d8833a2919075aadfbb880ae85743a09fa7edc Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Thu, 24 Oct 2024 15:45:27 -0400 Subject: [PATCH 5/9] Add check for -- --- help.go | 8 +++++++- help_test.go | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/help.go b/help.go index 20c7489709..76111ebb9f 100644 --- a/help.go +++ b/help.go @@ -89,6 +89,9 @@ func helpCommandAction(ctx context.Context, cmd *Command) error { // Case 4. $ app help foo // foo is the command for which help needs to be shown if firstArg != "" { + if firstArg == "--" { + return nil + } tracef("returning ShowCommandHelp with %[1]q", firstArg) return ShowCommandHelp(ctx, cmd, firstArg) } @@ -467,7 +470,7 @@ func checkShellCompleteFlag(c *Command, arguments []string) (bool, []string) { // because after "--" only positional arguments are accepted. // https://unix.stackexchange.com/a/11382 if arg == "--" { - return false, arguments + return false, arguments[:pos] } } @@ -478,6 +481,7 @@ func checkCompletions(ctx context.Context, cmd *Command) bool { tracef("checking completions on command %[1]q", cmd.Name) if !cmd.Root().shellCompletion { + tracef("completion not enabled skipping %[1]q", cmd.Name) return false } @@ -489,6 +493,8 @@ func checkCompletions(ctx context.Context, cmd *Command) bool { } } + tracef("no subcommand found for completiot %[1]q", cmd.Name) + if cmd.ShellComplete != nil { tracef("running shell completion func for command %[1]q", cmd.Name) cmd.ShellComplete(ctx, cmd) diff --git a/help_test.go b/help_test.go index c39e9447d1..f9c9347763 100644 --- a/help_test.go +++ b/help_test.go @@ -1794,7 +1794,7 @@ func Test_checkShellCompleteFlag(t *testing.T) { EnableShellCompletion: true, }, wantShellCompletion: false, - wantArgs: []string{"--", "foo", "--generate-shell-completion"}, + wantArgs: []string{"--", "foo"}, }, { name: "shell completion", From d0bd5105ca31ead479ab910901b344b61afe2155 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Thu, 24 Oct 2024 15:47:12 -0400 Subject: [PATCH 6/9] Revert command --- command.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/command.go b/command.go index d5891475fc..9fa8f9dcb0 100644 --- a/command.go +++ b/command.go @@ -151,8 +151,6 @@ type Command struct { didSetupDefaults bool // whether in shell completion mode shellCompletion bool - // run args - runArgs []string } // FullName returns the full name of the command. @@ -186,8 +184,6 @@ func (cmd *Command) setupDefaults(osArgs []string) { cmd.didSetupDefaults = true - cmd.runArgs = osArgs - isRoot := cmd.parent == nil tracef("isRoot? %[1]v (cmd=%[2]q)", isRoot, cmd.Name) From 23727361fceea01e003c546312379f0a9ce21be1 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Thu, 24 Oct 2024 21:27:45 -0400 Subject: [PATCH 7/9] Add more tests --- command_test.go | 8 +-- completion.go | 2 + completion_test.go | 133 +++++++++++++++++++++++++++++++++------------ flag.go | 2 +- help.go | 2 +- help_test.go | 26 ++++----- 6 files changed, 118 insertions(+), 55 deletions(-) diff --git a/command_test.go b/command_test.go index 544a21e64e..c64b624132 100644 --- a/command_test.go +++ b/command_test.go @@ -454,7 +454,7 @@ func TestCommand_Run_CustomShellCompleteAcceptsMalformedFlags(t *testing.T) { osArgs := &stringSliceArgs{v: []string{"bar"}} osArgs.v = append(osArgs.v, c.testArgs.Slice()...) - osArgs.v = append(osArgs.v, "--generate-shell-completion") + osArgs.v = append(osArgs.v, completionFlag) r := require.New(t) @@ -1446,7 +1446,7 @@ func TestCommand_BeforeAfterFuncShellCompletion(t *testing.T) { []string{ "command", "--opt", "succeed", - "sub", "--generate-shell-completion", + "sub", completionFlag, }, ), ) @@ -1810,7 +1810,7 @@ func TestCommand_OrderOfOperations(t *testing.T) { cmd, counts := buildCmdCounts() r := require.New(t) - _ = cmd.Run(buildTestContext(t), []string{"command", "--" + "generate-shell-completion"}) + _ = cmd.Run(buildTestContext(t), []string{"command", completionFlag}) r.Equal(1, counts.ShellComplete) r.Equal(1, counts.Total) }) @@ -2410,7 +2410,7 @@ func TestShellCompletionForIncompleteFlags(t *testing.T) { Writer: io.Discard, } - err := cmd.Run(buildTestContext(t), []string{"", "--test-completion", "--" + "generate-shell-completion"}) + err := cmd.Run(buildTestContext(t), []string{"", "--test-completion", completionFlag}) assert.NoError(t, err, "app should not return an error") } diff --git a/completion.go b/completion.go index 2319a815c9..d344fc4d5c 100644 --- a/completion.go +++ b/completion.go @@ -9,6 +9,8 @@ import ( const ( completionCommandName = "generate-completion" + completionFlagName = "generate-shell-completion" + completionFlag = "--" + completionFlagName ) var ( diff --git a/completion_test.go b/completion_test.go index 60d7cf8378..a8c91c3049 100644 --- a/completion_test.go +++ b/completion_test.go @@ -57,54 +57,115 @@ func TestCompletionShell(t *testing.T) { } func TestCompletionSubcommand(t *testing.T) { - out := &bytes.Buffer{} + tests := []struct { + name string + args []string + contains string + msg string + msgArgs []interface{} + notContains bool + }{ + { + name: "subcommand general completion", + args: []string{"foo", "bar", completionFlag}, + contains: "xyz", + msg: "Expected output to contain shell name %[1]q", + msgArgs: []interface{}{ + "xyz", + }, + }, + { + name: "subcommand flag completion", + args: []string{"foo", "bar", "-", completionFlag}, + contains: "l1", + msg: "Expected output to contain shell name %[1]q", + msgArgs: []interface{}{ + "l1", + }, + }, + { + name: "subcommand flag no completion", + args: []string{"foo", "bar", "--", completionFlag}, + contains: "l1", + msg: "Expected output to contain shell name %[1]q", + msgArgs: []interface{}{ + "l1", + }, + notContains: true, + }, + { + name: "sub sub command general completion", + args: []string{"foo", "bar", "xyz", completionFlag}, + contains: "-g", + msg: "Expected output to contain flag %[1]q", + msgArgs: []interface{}{ + "-g", + }, + notContains: true, + }, + { + name: "sub sub command flag completion", + args: []string{"foo", "bar", "xyz", "-", completionFlag}, + contains: "-g", + msg: "Expected output to contain flag %[1]q", + msgArgs: []interface{}{ + "-g", + }, + }, + { + name: "sub sub command no completion", + args: []string{"foo", "bar", "xyz", "--", completionFlag}, + contains: "-g", + msg: "Expected output to contain flag %[1]q", + msgArgs: []interface{}{ + "-g", + }, + notContains: true, + }, + } - cmd := &Command{ - EnableShellCompletion: true, - Writer: out, - Commands: []*Command{ - { - Name: "bar", + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + out := &bytes.Buffer{} + + cmd := &Command{ + EnableShellCompletion: true, + Writer: out, Commands: []*Command{ { - Name: "xyz", + Name: "bar", Flags: []Flag{ &StringFlag{ - Name: "g", - Aliases: []string{ - "t", + Name: "l1", + }, + }, + Commands: []*Command{ + { + Name: "xyz", + Flags: []Flag{ + &StringFlag{ + Name: "g", + Aliases: []string{ + "t", + }, + }, }, }, }, }, }, - }, - }, - } - - r := require.New(t) - - r.NoError(cmd.Run(buildTestContext(t), []string{"foo", "bar", "--generate-shell-completion"})) - r.Containsf( - out.String(), "xyz", - "Expected output to contain shell name %[1]q", "xyz", - ) - - out.Reset() - - r.NoError(cmd.Run(buildTestContext(t), []string{"foo", "bar", "xyz", "-", "--generate-shell-completion"})) - r.Containsf( - out.String(), "-g", - "Expected output to contain flag %[1]q", "-g", - ) + } - out.Reset() + r := require.New(t) - r.NoError(cmd.Run(buildTestContext(t), []string{"foo", "bar", "xyz", "--", "--generate-shell-completion"})) - r.NotContainsf( - out.String(), "-g", - "Expected output to not contain flag %[1]q", "-g", - ) + r.NoError(cmd.Run(buildTestContext(t), test.args)) + if test.notContains { + r.NotContainsf(out.String(), test.contains, test.msg, test.msgArgs...) + } else { + r.Containsf(out.String(), test.contains, test.msg, test.msgArgs...) + } + }) + } } type mockWriter struct { diff --git a/flag.go b/flag.go index 11b13662c5..67b34b71b8 100644 --- a/flag.go +++ b/flag.go @@ -28,7 +28,7 @@ var ( // GenerateShellCompletionFlag enables shell completion var GenerateShellCompletionFlag Flag = &BoolFlag{ - Name: "generate-shell-completion", + Name: completionFlagName, Hidden: true, } diff --git a/help.go b/help.go index 76111ebb9f..626968a8f7 100644 --- a/help.go +++ b/help.go @@ -461,7 +461,7 @@ func checkShellCompleteFlag(c *Command, arguments []string) (bool, []string) { pos := len(arguments) - 1 lastArg := arguments[pos] - if lastArg != "--generate-shell-completion" { + if lastArg != completionFlag { return false, arguments } diff --git a/help_test.go b/help_test.go index f9c9347763..e9db4383df 100644 --- a/help_test.go +++ b/help_test.go @@ -1188,7 +1188,7 @@ func TestDefaultCompleteWithFlags(t *testing.T) { }, }, }, - argv: []string{"cmd", "--e", "--generate-shell-completion"}, + argv: []string{"cmd", "--e", completionFlag}, env: map[string]string{"SHELL": "bash"}, expected: "--excitement\n", }, @@ -1210,7 +1210,7 @@ func TestDefaultCompleteWithFlags(t *testing.T) { }, }, }, - argv: []string{"cmd", "--e", "--generate-shell-completion"}, + argv: []string{"cmd", "--e", completionFlag}, env: map[string]string{"SHELL": "bash"}, expected: "", }, @@ -1232,7 +1232,7 @@ func TestDefaultCompleteWithFlags(t *testing.T) { }, }, }, - argv: []string{"cmd", "--e", "--", "--generate-shell-completion"}, + argv: []string{"cmd", "--e", "--", completionFlag}, env: map[string]string{"SHELL": "bash"}, expected: "", }, @@ -1255,7 +1255,7 @@ func TestDefaultCompleteWithFlags(t *testing.T) { }, }, }, - argv: []string{"cmd", "--generate-shell-completion"}, + argv: []string{"cmd", completionFlag}, env: map[string]string{"SHELL": "bash"}, expected: "futz\n", }, @@ -1278,7 +1278,7 @@ func TestDefaultCompleteWithFlags(t *testing.T) { }, }, }, - argv: []string{"cmd", "--url", "http://localhost:8000", "h", "--generate-shell-completion"}, + argv: []string{"cmd", "--url", "http://localhost:8000", "h", completionFlag}, env: map[string]string{"SHELL": "bash"}, expected: "help\n", }, @@ -1298,7 +1298,7 @@ func TestDefaultCompleteWithFlags(t *testing.T) { }, }, }, - argv: []string{"cmd", "putz", "-e", "--generate-shell-completion"}, + argv: []string{"cmd", "putz", "-e", completionFlag}, env: map[string]string{"SHELL": "zsh"}, expected: "--excitement:an exciting flag\n", }, @@ -1318,7 +1318,7 @@ func TestDefaultCompleteWithFlags(t *testing.T) { }, }, }, - argv: []string{"cmd", "putz", "-e", "--generate-shell-completion"}, + argv: []string{"cmd", "putz", "-e", completionFlag}, env: map[string]string{"SHELL": "zsh"}, expected: "--excitement\n", }, @@ -1764,19 +1764,19 @@ func Test_checkShellCompleteFlag(t *testing.T) { }{ { name: "disable-shell-completion", - arguments: []string{"--generate-shell-completion"}, + arguments: []string{completionFlag}, cmd: &Command{}, wantShellCompletion: false, - wantArgs: []string{"--generate-shell-completion"}, + wantArgs: []string{completionFlag}, }, { name: "child-disable-shell-completion", - arguments: []string{"--generate-shell-completion"}, + arguments: []string{completionFlag}, cmd: &Command{ parent: &Command{}, }, wantShellCompletion: false, - wantArgs: []string{"--generate-shell-completion"}, + wantArgs: []string{completionFlag}, }, { name: "last argument isn't --generate-shell-completion", @@ -1789,7 +1789,7 @@ func Test_checkShellCompleteFlag(t *testing.T) { }, { name: "arguments include double dash", - arguments: []string{"--", "foo", "--generate-shell-completion"}, + arguments: []string{"--", "foo", completionFlag}, cmd: &Command{ EnableShellCompletion: true, }, @@ -1798,7 +1798,7 @@ func Test_checkShellCompleteFlag(t *testing.T) { }, { name: "shell completion", - arguments: []string{"foo", "--generate-shell-completion"}, + arguments: []string{"foo", completionFlag}, cmd: &Command{ EnableShellCompletion: true, }, From 3421f0fc97e7ec381fb94444b2620172ca0c791b Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Fri, 25 Oct 2024 20:46:27 -0400 Subject: [PATCH 8/9] Add more tests --- completion_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/completion_test.go b/completion_test.go index a8c91c3049..e3c951b1b7 100644 --- a/completion_test.go +++ b/completion_test.go @@ -122,6 +122,16 @@ func TestCompletionSubcommand(t *testing.T) { }, notContains: true, }, + { + name: "sub sub command no completion extra args", + args: []string{"foo", "bar", "xyz", "--", "sargs", completionFlag}, + contains: "-g", + msg: "Expected output to contain flag %[1]q", + msgArgs: []interface{}{ + "-g", + }, + notContains: true, + }, } for _, test := range tests { @@ -159,6 +169,7 @@ func TestCompletionSubcommand(t *testing.T) { r := require.New(t) r.NoError(cmd.Run(buildTestContext(t), test.args)) + t.Log(out.String()) if test.notContains { r.NotContainsf(out.String(), test.contains, test.msg, test.msgArgs...) } else { From 021f97a62a96aca79227373c7703a61e96f740f0 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Fri, 25 Oct 2024 20:50:42 -0400 Subject: [PATCH 9/9] Revert name --- flag.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flag.go b/flag.go index 67b34b71b8..11b13662c5 100644 --- a/flag.go +++ b/flag.go @@ -28,7 +28,7 @@ var ( // GenerateShellCompletionFlag enables shell completion var GenerateShellCompletionFlag Flag = &BoolFlag{ - Name: completionFlagName, + Name: "generate-shell-completion", Hidden: true, }