Skip to content

Commit

Permalink
Merge pull request #1919 from dearchap/issue_1916_fix1
Browse files Browse the repository at this point in the history
Fix:(issue_1916) Add additional check for --
Signed-off-by: Naveen Gogineni <dear.chap@gmail.com>
  • Loading branch information
dearchap committed Oct 26, 2024
2 parents fe2c626 + 021f97a commit 94107c2
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 48 deletions.
8 changes: 4 additions & 4 deletions command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -1446,7 +1446,7 @@ func TestCommand_BeforeAfterFuncShellCompletion(t *testing.T) {
[]string{
"command",
"--opt", "succeed",
"sub", "--generate-shell-completion",
"sub", completionFlag,
},
),
)
Expand Down Expand Up @@ -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)
})
Expand Down Expand Up @@ -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")
}

Expand Down
2 changes: 2 additions & 0 deletions completion.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (

const (
completionCommandName = "generate-completion"
completionFlagName = "generate-shell-completion"
completionFlag = "--" + completionFlagName
)

var (
Expand Down
136 changes: 108 additions & 28 deletions completion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,46 +57,126 @@ 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,
},
{
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,
},
}

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 := require.New(t)

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",
)
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 {
r.Containsf(out.String(), test.contains, test.msg, test.msgArgs...)
}
})
}
}

type mockWriter struct {
Expand Down
10 changes: 8 additions & 2 deletions help.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -458,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
}

Expand All @@ -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]
}
}

Expand All @@ -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
}

Expand All @@ -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)
Expand Down
28 changes: 14 additions & 14 deletions help_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand All @@ -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: "",
},
Expand All @@ -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: "",
},
Expand All @@ -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",
},
Expand All @@ -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",
},
Expand All @@ -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",
},
Expand All @@ -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",
},
Expand Down Expand Up @@ -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",
Expand All @@ -1789,16 +1789,16 @@ 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,
},
wantShellCompletion: false,
wantArgs: []string{"--", "foo", "--generate-shell-completion"},
wantArgs: []string{"--", "foo"},
},
{
name: "shell completion",
arguments: []string{"foo", "--generate-shell-completion"},
arguments: []string{"foo", completionFlag},
cmd: &Command{
EnableShellCompletion: true,
},
Expand Down

0 comments on commit 94107c2

Please sign in to comment.