Merge pull request #559 from joshuarubin/completion_fix

make shell autocomplete more robust

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
+	shellComplete, arguments := checkShellCompleteFlag(a, arguments)
+
 	// parse flags
 	set, err := flagSet(a.Name, a.Flags)
 	if err != nil {
@@ -188,6 +192,7 @@ func (a *App) Run(arguments []string) (err error) {
 		ShowAppHelp(context)
 		return nerr
 	}
+	context.shellComplete = shellComplete
 
 	if checkCompletions(context) {
 		return nil
@@ -291,11 +296,6 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) {
 	}
 	a.Commands = newCmds
 
-	// append flags
-	if a.EnableBashCompletion {
-		a.appendFlag(BashCompletionFlag)
-	}
-
 	// parse flags
 	set, err := flagSet(a.Name, a.Flags)
 	if err != nil {

app_test.go

@@ -1620,3 +1620,47 @@ func TestHandleAction_WithUnknownPanic(t *testing.T) {
 	}
 	HandleAction(app.Action, NewContext(app, fs, 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)
+	}
+}

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, err := flagSet(c.Name, c.Flags)
 	if err != nil {
 		return err
@@ -135,6 +131,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)
@@ -147,20 +156,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
 	}

context.go

@@ -15,6 +15,7 @@ import (
 type Context struct {
 	App           *App
 	Command       Command
+	shellComplete 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.shellComplete = parentCtx.shellComplete
+	}
+
+	return c
 }
 
 // NumFlags returns the number of flags set

help.go

@@ -252,20 +252,43 @@ func checkSubcommandHelp(c *Context) bool {
 	return false
 }
 
-func checkCompletions(c *Context) bool {
+func checkShellCompleteFlag(a *App, arguments []string) (bool, []string) {
-	if (c.GlobalBool(BashCompletionFlag.Name) || c.Bool(BashCompletionFlag.Name)) && c.App.EnableBashCompletion {
+	if !a.EnableBashCompletion {
-		ShowCompletions(c)
+		return false, arguments
-		return true
 	}
 
-	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.shellComplete {
+		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 {
+	if !c.shellComplete {
-		ShowCommandCompletions(c, name)
+		return false
-		return true
 	}
 
-	return false
+	ShowCommandCompletions(c, name)
+	return true
 }