From ea3df26e645524f2b56f2c6c4d81826ab43a3f45 Mon Sep 17 00:00:00 2001 From: Joshua Rubin Date: Fri, 4 Nov 2016 14:56:28 -0600 Subject: [PATCH 1/3] make shell autocomplete more robust --- app.go | 18 +++++++++--------- command.go | 31 +++++++++++++------------------ context.go | 9 ++++++++- help.go | 41 ++++++++++++++++++++++++++++++++--------- 4 files changed, 62 insertions(+), 37 deletions(-) diff --git a/app.go b/app.go index 26cf09a..50edf9a 100644 --- a/app.go +++ b/app.go @@ -145,10 +145,6 @@ func (a *App) Setup() { } } - if a.EnableBashCompletion { - a.appendFlag(BashCompletionFlag) - } - if !a.HideVersion { a.appendFlag(VersionFlag) } @@ -173,6 +169,14 @@ func (a *App) Setup() { func (a *App) Run(arguments []string) (err error) { a.Setup() + // handle the completion flag separately from the flagset since + // completion could be attempted after a flag, but before its value was put + // on the command line. this causes the flagset to interpret the completion + // flag name as the value of the flag before it which is undesirable + // note that we can only do this because the shell autocomplete function + // always appends the completion flag at the end of the command + complete, arguments := checkCompleteFlag(a, arguments) + // parse flags set := flagSet(a.Name, a.Flags) set.SetOutput(ioutil.Discard) @@ -184,6 +188,7 @@ func (a *App) Run(arguments []string) (err error) { ShowAppHelp(context) return nerr } + context.complete = complete if checkCompletions(context) { return nil @@ -283,11 +288,6 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { } a.Commands = newCmds - // append flags - if a.EnableBashCompletion { - a.appendFlag(BashCompletionFlag) - } - // parse flags set := flagSet(a.Name, a.Flags) set.SetOutput(ioutil.Discard) diff --git a/command.go b/command.go index d955249..04eb0b8 100644 --- a/command.go +++ b/command.go @@ -87,10 +87,6 @@ func (c Command) Run(ctx *Context) (err error) { ) } - if ctx.App.EnableBashCompletion { - c.Flags = append(c.Flags, BashCompletionFlag) - } - set := flagSet(c.Name, c.Flags) set.SetOutput(ioutil.Discard) @@ -132,6 +128,19 @@ func (c Command) Run(ctx *Context) (err error) { err = set.Parse(ctx.Args().Tail()) } + nerr := normalizeFlags(c.Flags, set) + if nerr != nil { + fmt.Fprintln(ctx.App.Writer, nerr) + fmt.Fprintln(ctx.App.Writer) + ShowCommandHelp(ctx, c.Name) + return nerr + } + + context := NewContext(ctx.App, set, ctx) + if checkCommandCompletions(context, c.Name) { + return nil + } + if err != nil { if c.OnUsageError != nil { err := c.OnUsageError(ctx, err, false) @@ -144,20 +153,6 @@ func (c Command) Run(ctx *Context) (err error) { return err } - nerr := normalizeFlags(c.Flags, set) - if nerr != nil { - fmt.Fprintln(ctx.App.Writer, nerr) - fmt.Fprintln(ctx.App.Writer) - ShowCommandHelp(ctx, c.Name) - return nerr - } - - context := NewContext(ctx.App, set, ctx) - - if checkCommandCompletions(context, c.Name) { - return nil - } - if checkCommandHelp(context, c.Name) { return nil } diff --git a/context.go b/context.go index 492a742..81404c4 100644 --- a/context.go +++ b/context.go @@ -15,6 +15,7 @@ import ( type Context struct { App *App Command Command + complete bool flagSet *flag.FlagSet setFlags map[string]bool parentContext *Context @@ -22,7 +23,13 @@ type Context struct { // NewContext creates a new context. For use in when invoking an App or Command action. func NewContext(app *App, set *flag.FlagSet, parentCtx *Context) *Context { - return &Context{App: app, flagSet: set, parentContext: parentCtx} + c := &Context{App: app, flagSet: set, parentContext: parentCtx} + + if parentCtx != nil { + c.complete = parentCtx.complete + } + + return c } // NumFlags returns the number of flags set diff --git a/help.go b/help.go index 515f744..e453e72 100644 --- a/help.go +++ b/help.go @@ -252,20 +252,43 @@ func checkSubcommandHelp(c *Context) bool { return false } -func checkCompletions(c *Context) bool { - if (c.GlobalBool(BashCompletionFlag.Name) || c.Bool(BashCompletionFlag.Name)) && c.App.EnableBashCompletion { - ShowCompletions(c) - return true +func checkCompleteFlag(a *App, arguments []string) (bool, []string) { + if !a.EnableBashCompletion { + return false, arguments } - return false + pos := len(arguments) - 1 + lastArg := arguments[pos] + + if lastArg != "--"+BashCompletionFlag.Name { + return false, arguments + } + + return true, arguments[:pos] +} + +func checkCompletions(c *Context) bool { + if !c.complete { + return false + } + + if args := c.Args(); args.Present() { + name := args.First() + if cmd := c.App.Command(name); cmd != nil { + // let the command handle the completion + return false + } + } + + ShowCompletions(c) + return true } func checkCommandCompletions(c *Context, name string) bool { - if c.Bool(BashCompletionFlag.Name) && c.App.EnableBashCompletion { - ShowCommandCompletions(c, name) - return true + if !c.complete { + return false } - return false + ShowCommandCompletions(c, name) + return true } From 8dd1962f7b0654225424d5d1bc0189b0113fcafc Mon Sep 17 00:00:00 2001 From: Joshua Rubin Date: Mon, 14 Nov 2016 09:35:22 -0700 Subject: [PATCH 2/3] change "complete" to "shellComplete" --- app.go | 4 ++-- context.go | 4 ++-- help.go | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app.go b/app.go index 50edf9a..9836d31 100644 --- a/app.go +++ b/app.go @@ -175,7 +175,7 @@ func (a *App) Run(arguments []string) (err error) { // flag name as the value of the flag before it which is undesirable // note that we can only do this because the shell autocomplete function // always appends the completion flag at the end of the command - complete, arguments := checkCompleteFlag(a, arguments) + shellComplete, arguments := checkShellCompleteFlag(a, arguments) // parse flags set := flagSet(a.Name, a.Flags) @@ -188,7 +188,7 @@ func (a *App) Run(arguments []string) (err error) { ShowAppHelp(context) return nerr } - context.complete = complete + context.shellComplete = shellComplete if checkCompletions(context) { return nil diff --git a/context.go b/context.go index 81404c4..4f440ec 100644 --- a/context.go +++ b/context.go @@ -15,7 +15,7 @@ import ( type Context struct { App *App Command Command - complete bool + shellComplete bool flagSet *flag.FlagSet setFlags map[string]bool parentContext *Context @@ -26,7 +26,7 @@ func NewContext(app *App, set *flag.FlagSet, parentCtx *Context) *Context { c := &Context{App: app, flagSet: set, parentContext: parentCtx} if parentCtx != nil { - c.complete = parentCtx.complete + c.shellComplete = parentCtx.shellComplete } return c diff --git a/help.go b/help.go index e453e72..e261bb6 100644 --- a/help.go +++ b/help.go @@ -252,7 +252,7 @@ func checkSubcommandHelp(c *Context) bool { return false } -func checkCompleteFlag(a *App, arguments []string) (bool, []string) { +func checkShellCompleteFlag(a *App, arguments []string) (bool, []string) { if !a.EnableBashCompletion { return false, arguments } @@ -268,7 +268,7 @@ func checkCompleteFlag(a *App, arguments []string) (bool, []string) { } func checkCompletions(c *Context) bool { - if !c.complete { + if !c.shellComplete { return false } @@ -285,7 +285,7 @@ func checkCompletions(c *Context) bool { } func checkCommandCompletions(c *Context, name string) bool { - if !c.complete { + if !c.shellComplete { return false } From 3272baf434b471d349ddd0f51d07ab11a8cb9228 Mon Sep 17 00:00:00 2001 From: Joshua Rubin Date: Mon, 14 Nov 2016 10:10:51 -0700 Subject: [PATCH 3/3] add a test for shell completion using incomplete flags --- app_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/app_test.go b/app_test.go index 40a598d..ae87237 100644 --- a/app_test.go +++ b/app_test.go @@ -1561,3 +1561,47 @@ func TestHandleAction_WithUnknownPanic(t *testing.T) { } HandleAction(app.Action, NewContext(app, flagSet(app.Name, app.Flags), nil)) } + +func TestShellCompletionForIncompleteFlags(t *testing.T) { + app := NewApp() + app.Flags = []Flag{ + IntFlag{ + Name: "test-completion", + }, + } + app.EnableBashCompletion = true + app.BashComplete = func(ctx *Context) { + for _, command := range ctx.App.Commands { + if command.Hidden { + continue + } + + for _, name := range command.Names() { + fmt.Fprintln(ctx.App.Writer, name) + } + } + + for _, flag := range ctx.App.Flags { + for _, name := range strings.Split(flag.GetName(), ",") { + if name == BashCompletionFlag.Name { + continue + } + + switch name = strings.TrimSpace(name); len(name) { + case 0: + case 1: + fmt.Fprintln(ctx.App.Writer, "-"+name) + default: + fmt.Fprintln(ctx.App.Writer, "--"+name) + } + } + } + } + app.Action = func(ctx *Context) error { + return fmt.Errorf("should not get here") + } + err := app.Run([]string{"", "--test-completion", "--" + BashCompletionFlag.Name}) + if err != nil { + t.Errorf("app should not return an error: %s", err) + } +}