Merge branch 'master' into check-run-error-in-readme

README.md

@@ -47,6 +47,7 @@ applications in an expressive way.
   * [Version Flag](#version-flag)
     + [Customization](#customization-2)
     + [Full API Example](#full-api-example)
+  * [Combining short Bool options](#combining-short-bool-options)
 - [Contribution Guidelines](#contribution-guidelines)
 
 <!-- tocstop -->
@@ -1500,6 +1501,26 @@ func wopAction(c *cli.Context) error {
 }
 ```
 
+### Combining short Bool options
+
+Traditional use of boolean options using their shortnames look like this:
+```
+# cmd foobar -s -o
+```
+
+Suppose you want users to be able to combine your bool options with their shortname.  This
+can be done using the **UseShortOptionHandling** bool in your commands.  Suppose your program
+has a two bool flags such as *serve* and *option* with the short options of *-o* and
+*-s* respectively. With **UseShortOptionHandling** set to *true*, a user can use a syntax
+like:
+```
+# cmd foobar -so
+```
+
+If you enable the **UseShortOptionHandling*, then you must not use any flags that have a single
+leading *-* or this will result in failures.  For example, **-option** can no longer be used.  Flags
+with two leading dashes (such as **--options**) are still valid.
+
 ## Contribution Guidelines
 
 Feel free to put up a pull request to fix a bug or maybe add a feature. I will

app.go

@@ -453,7 +453,6 @@ func (a *App) hasFlag(flag Flag) bool {
 }
 
 func (a *App) errWriter() io.Writer {
-
 	// When the app ErrWriter is nil use the package level one.
 	if a.ErrWriter == nil {
 		return ErrWriter

app_test.go

@@ -329,6 +329,39 @@ func TestApp_CommandWithArgBeforeFlags(t *testing.T) {
 	expect(t, firstArg, "my-arg")
 }
 
+func TestApp_CommandWithArgBeforeBoolFlags(t *testing.T) {
+	var parsedOption, parsedSecondOption, firstArg string
+	var parsedBool, parsedSecondBool bool
+
+	app := NewApp()
+	command := Command{
+		Name: "cmd",
+		Flags: []Flag{
+			StringFlag{Name: "option", Value: "", Usage: "some option"},
+			StringFlag{Name: "secondOption", Value: "", Usage: "another option"},
+			BoolFlag{Name: "boolflag", Usage: "some bool"},
+			BoolFlag{Name: "b", Usage: "another bool"},
+		},
+		Action: func(c *Context) error {
+			parsedOption = c.String("option")
+			parsedSecondOption = c.String("secondOption")
+			parsedBool = c.Bool("boolflag")
+			parsedSecondBool = c.Bool("b")
+			firstArg = c.Args().First()
+			return nil
+		},
+	}
+	app.Commands = []Command{command}
+
+	app.Run([]string{"", "cmd", "my-arg", "--boolflag", "--option", "my-option", "-b", "--secondOption", "fancy-option"})
+
+	expect(t, parsedOption, "my-option")
+	expect(t, parsedSecondOption, "fancy-option")
+	expect(t, parsedBool, true)
+	expect(t, parsedSecondBool, true)
+	expect(t, firstArg, "my-arg")
+}
+
 func TestApp_RunAsSubcommandParseFlags(t *testing.T) {
 	var context *Context
 

command.go

@@ -1,6 +1,7 @@
 package cli
 
 import (
+	"flag"
 	"fmt"
 	"io/ioutil"
 	"sort"
@@ -55,6 +56,10 @@ type Command struct {
 	HideHelp bool
 	// Boolean to hide this command from help or completion
 	Hidden bool
+	// Boolean to enable short-option handling so user can combine several
+	// single-character bool arguements into one
+	// i.e. foobar -o -v -> foobar -ov
+	UseShortOptionHandling bool
 
 	// Full name of command for help, defaults to full command name, including parent commands.
 	HelpName        string
@@ -106,57 +111,7 @@ func (c Command) Run(ctx *Context) (err error) {
 		)
 	}
 
-	set, err := flagSet(c.Name, c.Flags)
-	if err != nil {
-		return err
-	}
-	set.SetOutput(ioutil.Discard)
-
-	if c.SkipFlagParsing {
-		err = set.Parse(append([]string{"--"}, ctx.Args().Tail()...))
-	} else if !c.SkipArgReorder {
-		firstFlagIndex := -1
-		terminatorIndex := -1
-		for index, arg := range ctx.Args() {
-			if arg == "--" {
-				terminatorIndex = index
-				break
-			} else if arg == "-" {
-				// Do nothing. A dash alone is not really a flag.
-				continue
-			} else if strings.HasPrefix(arg, "-") && firstFlagIndex == -1 {
-				firstFlagIndex = index
-			}
-		}
-
-		if firstFlagIndex > -1 {
-			args := ctx.Args()
-			regularArgs := make([]string, len(args[1:firstFlagIndex]))
-			copy(regularArgs, args[1:firstFlagIndex])
-
-			var flagArgs []string
-			if terminatorIndex > -1 {
-				flagArgs = args[firstFlagIndex:terminatorIndex]
-				regularArgs = append(regularArgs, args[terminatorIndex:]...)
-			} else {
-				flagArgs = args[firstFlagIndex:]
-			}
-
-			err = set.Parse(append(flagArgs, regularArgs...))
-		} else {
-			err = set.Parse(ctx.Args().Tail())
-		}
-	} else {
-		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
-	}
+	set, err := c.parseFlags(ctx.Args().Tail())
 
 	context := NewContext(ctx.App, set, ctx)
 	context.Command = c
@@ -215,6 +170,83 @@ func (c Command) Run(ctx *Context) (err error) {
 	return err
 }
 
+func (c *Command) parseFlags(args Args) (*flag.FlagSet, error) {
+	set, err := flagSet(c.Name, c.Flags)
+	if err != nil {
+		return nil, err
+	}
+	set.SetOutput(ioutil.Discard)
+
+	if c.SkipFlagParsing {
+		return set, set.Parse(append([]string{"--"}, args...))
+	}
+
+	if c.UseShortOptionHandling {
+		args = translateShortOptions(args)
+	}
+
+	if !c.SkipArgReorder {
+		args = reorderArgs(args)
+	}
+
+	err = set.Parse(args)
+	if err != nil {
+		return nil, err
+	}
+
+	err = normalizeFlags(c.Flags, set)
+	if err != nil {
+		return nil, err
+	}
+
+	return set, nil
+}
+
+// reorderArgs moves all flags before arguments as this is what flag expects
+func reorderArgs(args []string) []string {
+	var nonflags, flags []string
+
+	readFlagValue := false
+	for i, arg := range args {
+		if arg == "--" {
+			nonflags = append(nonflags, args[i:]...)
+			break
+		}
+
+		if readFlagValue && !strings.HasPrefix(arg, "-") && !strings.HasPrefix(arg, "--") {
+			readFlagValue = false
+			flags = append(flags, arg)
+			continue
+		}
+		readFlagValue = false
+
+		if arg != "-" && strings.HasPrefix(arg, "-") {
+			flags = append(flags, arg)
+
+			readFlagValue = !strings.Contains(arg, "=")
+		} else {
+			nonflags = append(nonflags, arg)
+		}
+	}
+
+	return append(flags, nonflags...)
+}
+
+func translateShortOptions(flagArgs Args) []string {
+	// separate combined flags
+	var flagArgsSeparated []string
+	for _, flagArg := range flagArgs {
+		if strings.HasPrefix(flagArg, "-") && strings.HasPrefix(flagArg, "--") == false && len(flagArg) > 2 {
+			for _, flagChar := range flagArg[1:] {
+				flagArgsSeparated = append(flagArgsSeparated, "-"+string(flagChar))
+			}
+		} else {
+			flagArgsSeparated = append(flagArgsSeparated, flagArg)
+		}
+	}
+	return flagArgsSeparated
+}
+
 // Names returns the names including short names and aliases.
 func (c Command) Names() []string {
 	names := []string{c.Name}

command_test.go

@@ -11,20 +11,24 @@ import (
 
 func TestCommandFlagParsing(t *testing.T) {
 	cases := []struct {
-		testArgs        []string
-		skipFlagParsing bool
-		skipArgReorder  bool
-		expectedErr     error
+		testArgs               []string
+		skipFlagParsing        bool
+		skipArgReorder         bool
+		expectedErr            error
+		UseShortOptionHandling bool
 	}{
 		// Test normal "not ignoring flags" flow
-		{[]string{"test-cmd", "blah", "blah", "-break"}, false, false, errors.New("flag provided but not defined: -break")},
+		{[]string{"test-cmd", "blah", "blah", "-break"}, false, false, errors.New("flag provided but not defined: -break"), false},
 
 		// Test no arg reorder
-		{[]string{"test-cmd", "blah", "blah", "-break"}, false, true, nil},
+		{[]string{"test-cmd", "blah", "blah", "-break"}, false, true, nil, false},
+		{[]string{"test-cmd", "blah", "blah", "-break", "ls", "-l"}, false, true, nil, true},
+
+		{[]string{"test-cmd", "blah", "blah"}, true, false, nil, false},   // Test SkipFlagParsing without any args that look like flags
+		{[]string{"test-cmd", "blah", "-break"}, true, false, nil, false}, // Test SkipFlagParsing with random flag arg
+		{[]string{"test-cmd", "blah", "-help"}, true, false, nil, false},  // Test SkipFlagParsing with "special" help flag arg
+		{[]string{"test-cmd", "blah"}, false, false, nil, true},           // Test UseShortOptionHandling
 
-		{[]string{"test-cmd", "blah", "blah"}, true, false, nil},   // Test SkipFlagParsing without any args that look like flags
-		{[]string{"test-cmd", "blah", "-break"}, true, false, nil}, // Test SkipFlagParsing with random flag arg
-		{[]string{"test-cmd", "blah", "-help"}, true, false, nil},  // Test SkipFlagParsing with "special" help flag arg
 	}
 
 	for _, c := range cases {
@@ -36,13 +40,14 @@ func TestCommandFlagParsing(t *testing.T) {
 		context := NewContext(app, set, nil)
 
 		command := Command{
-			Name:            "test-cmd",
-			Aliases:         []string{"tc"},
-			Usage:           "this is for testing",
-			Description:     "testing",
-			Action:          func(_ *Context) error { return nil },
-			SkipFlagParsing: c.skipFlagParsing,
-			SkipArgReorder:  c.skipArgReorder,
+			Name:                   "test-cmd",
+			Aliases:                []string{"tc"},
+			Usage:                  "this is for testing",
+			Description:            "testing",
+			Action:                 func(_ *Context) error { return nil },
+			SkipFlagParsing:        c.skipFlagParsing,
+			SkipArgReorder:         c.skipArgReorder,
+			UseShortOptionHandling: c.UseShortOptionHandling,
 		}
 
 		err := command.Run(context)
@@ -238,3 +243,77 @@ func TestCommand_Run_SubcommandsCanUseErrWriter(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
+func TestCommandFlagReordering(t *testing.T) {
+	cases := []struct {
+		testArgs      []string
+		expectedValue string
+		expectedArgs  []string
+		expectedErr   error
+	}{
+		{[]string{"some-exec", "some-command", "some-arg", "--flag", "foo"}, "foo", []string{"some-arg"}, nil},
+		{[]string{"some-exec", "some-command", "some-arg", "--flag=foo"}, "foo", []string{"some-arg"}, nil},
+		{[]string{"some-exec", "some-command", "--flag=foo", "some-arg"}, "foo", []string{"some-arg"}, nil},
+	}
+
+	for _, c := range cases {
+		value := ""
+		args := []string{}
+		app := &App{
+			Commands: []Command{
+				{
+					Name: "some-command",
+					Flags: []Flag{
+						StringFlag{Name: "flag"},
+					},
+					Action: func(c *Context) {
+						fmt.Printf("%+v\n", c.String("flag"))
+						value = c.String("flag")
+						args = c.Args()
+					},
+				},
+			},
+		}
+
+		err := app.Run(c.testArgs)
+		expect(t, err, c.expectedErr)
+		expect(t, value, c.expectedValue)
+		expect(t, args, c.expectedArgs)
+	}
+}
+
+func TestCommandSkipFlagParsing(t *testing.T) {
+	cases := []struct {
+		testArgs     []string
+		expectedArgs []string
+		expectedErr  error
+	}{
+		{[]string{"some-exec", "some-command", "some-arg", "--flag", "foo"}, []string{"some-arg", "--flag", "foo"}, nil},
+		{[]string{"some-exec", "some-command", "some-arg", "--flag=foo"}, []string{"some-arg", "--flag=foo"}, nil},
+	}
+
+	for _, c := range cases {
+		value := ""
+		args := []string{}
+		app := &App{
+			Commands: []Command{
+				{
+					SkipFlagParsing: true,
+					Name:            "some-command",
+					Flags: []Flag{
+						StringFlag{Name: "flag"},
+					},
+					Action: func(c *Context) {
+						fmt.Printf("%+v\n", c.String("flag"))
+						value = c.String("flag")
+						args = c.Args()
+					},
+				},
+			},
+		}
+
+		err := app.Run(c.testArgs)
+		expect(t, err, c.expectedErr)
+		expect(t, args, c.expectedArgs)
+	}
+}

flag.go

@@ -178,7 +178,11 @@ func (f StringSliceFlag) ApplyWithError(set *flag.FlagSet) error {
 				return fmt.Errorf("could not parse %s as string value for flag %s: %s", envVal, f.Name, err)
 			}
 		}
-		f.Value = newVal
+		if f.Value == nil {
+			f.Value = newVal
+		} else {
+			*f.Value = *newVal
+		}
 	}
 
 	eachName(f.Name, func(name string) {
@@ -235,7 +239,11 @@ func (f IntSliceFlag) ApplyWithError(set *flag.FlagSet) error {
 				return fmt.Errorf("could not parse %s as int slice value for flag %s: %s", envVal, f.Name, err)
 			}
 		}
-		f.Value = newVal
+		if f.Value == nil {
+			f.Value = newVal
+		} else {
+			*f.Value = *newVal
+		}
 	}
 
 	eachName(f.Name, func(name string) {
@@ -292,7 +300,11 @@ func (f Int64SliceFlag) ApplyWithError(set *flag.FlagSet) error {
 				return fmt.Errorf("could not parse %s as int64 slice value for flag %s: %s", envVal, f.Name, err)
 			}
 		}
-		f.Value = newVal
+		if f.Value == nil {
+			f.Value = newVal
+		} else {
+			*f.Value = *newVal
+		}
 	}
 
 	eachName(f.Name, func(name string) {
@@ -624,7 +636,7 @@ func withEnvHint(envVar, str string) string {
 			suffix = "%"
 			sep = "%, %"
 		}
-		envText = fmt.Sprintf(" [%s%s%s]", prefix, strings.Join(strings.Split(envVar, ","), sep), suffix)
+		envText = " [" + prefix + strings.Join(strings.Split(envVar, ","), sep) + suffix + "]"
 	}
 	return str + envText
 }
@@ -697,13 +709,13 @@ func stringifyFlag(f Flag) string {
 		placeholder = defaultPlaceholder
 	}
 
-	usageWithDefault := strings.TrimSpace(fmt.Sprintf("%s%s", usage, defaultValueString))
+	usageWithDefault := strings.TrimSpace(usage + defaultValueString)
 
 	return FlagFileHinter(
 		fv.FieldByName("FilePath").String(),
 		FlagEnvHinter(
 			fv.FieldByName("EnvVar").String(),
-			fmt.Sprintf("%s\t%s", FlagNamePrefixer(fv.FieldByName("Name").String(), placeholder), usageWithDefault),
+			FlagNamePrefixer(fv.FieldByName("Name").String(), placeholder)+"\t"+usageWithDefault,
 		),
 	)
 }
@@ -712,7 +724,7 @@ func stringifyIntSliceFlag(f IntSliceFlag) string {
 	defaultVals := []string{}
 	if f.Value != nil && len(f.Value.Value()) > 0 {
 		for _, i := range f.Value.Value() {
-			defaultVals = append(defaultVals, fmt.Sprintf("%d", i))
+			defaultVals = append(defaultVals, strconv.Itoa(i))
 		}
 	}
 
@@ -723,7 +735,7 @@ func stringifyInt64SliceFlag(f Int64SliceFlag) string {
 	defaultVals := []string{}
 	if f.Value != nil && len(f.Value.Value()) > 0 {
 		for _, i := range f.Value.Value() {
-			defaultVals = append(defaultVals, fmt.Sprintf("%d", i))
+			defaultVals = append(defaultVals, strconv.FormatInt(i, 10))
 		}
 	}
 
@@ -735,7 +747,7 @@ func stringifyStringSliceFlag(f StringSliceFlag) string {
 	if f.Value != nil && len(f.Value.Value()) > 0 {
 		for _, s := range f.Value.Value() {
 			if len(s) > 0 {
-				defaultVals = append(defaultVals, fmt.Sprintf("%q", s))
+				defaultVals = append(defaultVals, strconv.Quote(s))
 			}
 		}
 	}
@@ -754,8 +766,8 @@ func stringifySliceFlag(usage, name string, defaultVals []string) string {
 		defaultVal = fmt.Sprintf(" (default: %s)", strings.Join(defaultVals, ", "))
 	}
 
-	usageWithDefault := strings.TrimSpace(fmt.Sprintf("%s%s", usage, defaultVal))
-	return fmt.Sprintf("%s\t%s", FlagNamePrefixer(name, placeholder), usageWithDefault)
+	usageWithDefault := strings.TrimSpace(usage + defaultVal)
+	return FlagNamePrefixer(name, placeholder) + "\t" + usageWithDefault
 }
 
 func flagFromFileEnv(filePath, envName string) (val string, ok bool) {

flag_test.go

@@ -1048,6 +1048,31 @@ func TestParseMultiBool(t *testing.T) {
 	a.Run([]string{"run", "--serve"})
 }
 
+func TestParseBoolShortOptionHandle(t *testing.T) {
+	a := App{
+		Commands: []Command{
+			{
+				Name: "foobar",
+				UseShortOptionHandling: true,
+				Action: func(ctx *Context) error {
+					if ctx.Bool("serve") != true {
+						t.Errorf("main name not set")
+					}
+					if ctx.Bool("option") != true {
+						t.Errorf("short name not set")
+					}
+					return nil
+				},
+				Flags: []Flag{
+					BoolFlag{Name: "serve, s"},
+					BoolFlag{Name: "option, o"},
+				},
+			},
+		},
+	}
+	a.Run([]string{"run", "foobar", "-so"})
+}
+
 func TestParseDestinationBool(t *testing.T) {
 	var dest bool
 	a := App{