Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: disable bash completion if double dash is included in arguments (v2) #1938

Merged
merged 1 commit into from
Jun 29, 2024
Merged

fix: disable bash completion if double dash is included in arguments (v2) #1938

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions help.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,6 @@ func ShowCommandHelpAndExit(c *Context, command string, code int) {

// ShowCommandHelp prints help for the given command
func ShowCommandHelp(ctx *Context, command string) error {

commands := ctx.App.Commands
if ctx.Command.Subcommands != nil {
commands = ctx.Command.Subcommands
Expand Down Expand Up @@ -337,15 +336,13 @@ func ShowCommandCompletions(ctx *Context, command string) {
DefaultCompleteWithFlags(c)(ctx)
}
}

}

// printHelpCustom is the default implementation of HelpPrinterCustom.
//
// The customFuncs map will be combined with a default template.FuncMap to
// allow using arbitrary functions in template rendering.
func printHelpCustom(out io.Writer, templ string, data interface{}, customFuncs map[string]interface{}) {

const maxLineLength = 10000

funcMap := template.FuncMap{
Expand Down Expand Up @@ -450,6 +447,15 @@ func checkShellCompleteFlag(a *App, arguments []string) (bool, []string) {
return false, arguments
}

for _, arg := range arguments {
// If arguments include "--", shell completion is disabled
// because after "--" only positional arguments are accepted.
// https://unix.stackexchange.com/a/11382
if arg == "--" {
return false, arguments
}
}

return true, arguments[:pos]
}

Expand Down Expand Up @@ -499,7 +505,6 @@ func wrap(input string, offset int, wrapAt int) string {
ss = append(ss, wrapped)
} else {
ss = append(ss, padding+wrapped)

}

}
Expand Down
70 changes: 65 additions & 5 deletions help_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io"
"os"
"reflect"
"runtime"
"strings"
"testing"
Expand Down Expand Up @@ -1349,7 +1350,6 @@ func TestWrap(t *testing.T) {
}

func TestWrappedHelp(t *testing.T) {

// Reset HelpPrinter after this test.
defer func(old helpPrinter) {
HelpPrinter = old
Expand All @@ -1359,7 +1359,8 @@ func TestWrappedHelp(t *testing.T) {
app := &App{
Writer: output,
Flags: []Flag{
&BoolFlag{Name: "foo",
&BoolFlag{
Name: "foo",
Aliases: []string{"h"},
Usage: "here's a really long help text line, let's see where it wraps. blah blah blah and so on.",
},
Expand Down Expand Up @@ -1443,7 +1444,6 @@ COPYRIGHT:
}

func TestWrappedCommandHelp(t *testing.T) {

// Reset HelpPrinter after this test.
defer func(old helpPrinter) {
HelpPrinter = old
Expand Down Expand Up @@ -1504,7 +1504,6 @@ OPTIONS:
}

func TestWrappedSubcommandHelp(t *testing.T) {

// Reset HelpPrinter after this test.
defer func(old helpPrinter) {
HelpPrinter = old
Expand Down Expand Up @@ -1573,7 +1572,6 @@ OPTIONS:
}

func TestWrappedHelpSubcommand(t *testing.T) {

// Reset HelpPrinter after this test.
defer func(old helpPrinter) {
HelpPrinter = old
Expand Down Expand Up @@ -1714,3 +1712,65 @@ GLOBAL OPTIONS:
output.String(), expected)
}
}

func Test_checkShellCompleteFlag(t *testing.T) {
t.Parallel()
tests := []struct {
name string
app *App
arguments []string
wantShellCompletion bool
wantArgs []string
}{
{
name: "disable bash completion",
arguments: []string{"--generate-bash-completion"},
app: &App{},
wantShellCompletion: false,
wantArgs: []string{"--generate-bash-completion"},
},
{
name: "--generate-bash-completion isn't used",
arguments: []string{"foo"},
app: &App{
EnableBashCompletion: true,
},
wantShellCompletion: false,
wantArgs: []string{"foo"},
},
{
name: "arguments include double dash",
arguments: []string{"--", "foo", "--generate-bash-completion"},
app: &App{
EnableBashCompletion: true,
},
wantShellCompletion: false,
wantArgs: []string{"--", "foo", "--generate-bash-completion"},
},
{
name: "--generate-bash-completion",
arguments: []string{"foo", "--generate-bash-completion"},
app: &App{
EnableBashCompletion: true,
},
wantShellCompletion: true,
wantArgs: []string{"foo"},
},
}

for _, tt := range tests {
tt := tt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this needed ?

Copy link
Contributor Author

@suzuki-shunsuke suzuki-shunsuke Jun 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we remove this, go vet fails.

$ go vet ./...
# github.com/urfave/cli/v2
# [github.com/urfave/cli/v2]
./help_test.go:1765:52: loop variable tt captured by func literal
./help_test.go:1765:60: loop variable tt captured by func literal
./help_test.go:1766:7: loop variable tt captured by func literal
./help_test.go:1768:23: loop variable tt captured by func literal
./help_test.go:1770:26: loop variable tt captured by func literal
./help_test.go:1772:12: loop variable tt captured by func literal

$ echo $?
1

If we update go directive to 1.22 or later, we can remove this because the treatment of loop variable was changed.

https://tip.golang.org/doc/go1.22

cli/go.mod

Line 3 in 8e2384c

go 1.18

But I don't think this change is the scope of this pull request, so I didn't do that.

t.Run(tt.name, func(t *testing.T) {
t.Parallel()
shellCompletion, args := checkShellCompleteFlag(tt.app, tt.arguments)
if tt.wantShellCompletion != shellCompletion {
t.Errorf("Unexpected shell completion, got:\n%v\nexpected: %v",
shellCompletion, tt.wantShellCompletion)
}
if !reflect.DeepEqual(tt.wantArgs, args) {
t.Errorf("Unexpected arguments, got:\n%v\nexpected: %v",
args, tt.wantArgs)
}
})
}
}