Refactor flag handling logic

Refactor logic introduced by #686

command.go

@@ -1,6 +1,7 @@
 package cli
 
 import (
+	"flag"
 	"fmt"
 	"io/ioutil"
 	"sort"
@@ -110,43 +111,7 @@ func (c Command) Run(ctx *Context) (err error) {
 		)
 	}
 
-	set, err := flagSet(c.Name, c.Flags)
+	set, err := c.parseFlags(ctx.Args().Tail())
-	if err != nil {
-		return err
-	}
-	set.SetOutput(ioutil.Discard)
-	firstFlagIndex, terminatorIndex := getIndexes(ctx)
-	flagArgs, regularArgs := getAllArgs(ctx.Args(), firstFlagIndex, terminatorIndex)
-	if c.UseShortOptionHandling {
-		flagArgs = translateShortOptions(flagArgs)
-	}
-	if c.SkipFlagParsing {
-		err = set.Parse(append([]string{"--"}, ctx.Args().Tail()...))
-	} else if !c.SkipArgReorder {
-		if firstFlagIndex > -1 {
-			err = set.Parse(append(flagArgs, regularArgs...))
-		} else {
-			err = set.Parse(ctx.Args().Tail())
-		}
-	} else if c.UseShortOptionHandling {
-		if terminatorIndex == -1 && firstFlagIndex > -1 {
-			// Handle shortname AND no options
-			err = set.Parse(append(regularArgs, flagArgs...))
-		} else {
-			// Handle shortname and options
-			err = set.Parse(flagArgs)
-		}
-	} else {
-		err = set.Parse(append(regularArgs, flagArgs...))
-	}
-
-	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)
 	context.Command = c
@@ -205,60 +170,65 @@ func (c Command) Run(ctx *Context) (err error) {
 	return err
 }
 
-func getIndexes(ctx *Context) (int, int) {
+func (c *Command) parseFlags(args Args) (*flag.FlagSet, error) {
-	firstFlagIndex := -1
+	set, err := flagSet(c.Name, c.Flags)
-	terminatorIndex := -1
+	if err != nil {
-	for index, arg := range ctx.Args() {
+		return nil, err
-		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 len(ctx.Args()) > 0 && !strings.HasPrefix(ctx.Args()[0], "-") && firstFlagIndex == -1 {
+	set.SetOutput(ioutil.Discard)
-		return -1, -1
+
+	if c.SkipFlagParsing {
+		return set, set.Parse(append([]string{c.Name, "--"}, args...))
 	}
 
-	return firstFlagIndex, terminatorIndex
+	if c.UseShortOptionHandling {
+		args = translateShortOptions(args)
+	}
 
-}
+	if !c.SkipArgReorder {
+		args = reorderArgs(args)
+	}
 
-// copyStringslice takes a string slice and copies it
+	err = set.Parse(args)
-func copyStringSlice(slice []string, start, end int) []string {
+	if err != nil {
-	newSlice := make([]string, end-start)
+		return nil, err
-	copy(newSlice, slice[start:end])
+	}
-	return newSlice
-}
 
-// getAllArgs extracts and returns two string slices representing
+	err = normalizeFlags(c.Flags, set)
-// regularArgs and flagArgs
+	if err != nil {
-func getAllArgs(args []string, firstFlagIndex, terminatorIndex int) ([]string, []string) {
+		return nil, err
-	var regularArgs []string
-	// if there are no options, the we set the index to 1 manually
-	if firstFlagIndex == -1 {
-		firstFlagIndex = 1
-		regularArgs = copyStringSlice(args, 0, len(args))
-	} else {
-		regularArgs = copyStringSlice(args, 1, firstFlagIndex)
 	}
-	var flagArgs []string
+
-	// a flag terminatorIndex was found in the input. we need to collect
+	return set, nil
-	// flagArgs based on it.
+}
-	if terminatorIndex > -1 {
+
-		flagArgs = copyStringSlice(args, firstFlagIndex, terminatorIndex)
+// reorderArgs moves all flags before arguments as this is what flag expects
-		additionalRegularArgs := copyStringSlice(args, terminatorIndex, len(args))
+func reorderArgs(args []string) []string {
-		regularArgs = append(regularArgs, additionalRegularArgs...)
+	var nonflags, flags []string
-		for _, i := range additionalRegularArgs {
+
-			regularArgs = append(regularArgs, i)
+	readFlagValue := false
+	for i, arg := range args {
+		if arg == "--" {
+			nonflags = append(nonflags, args[i:]...)
+			break
+		}
+
+		if readFlagValue {
+			readFlagValue = false
+			flags = append(flags, arg)
+			continue
+		}
+
+		if arg != "-" && strings.HasPrefix(arg, "-") {
+			flags = append(flags, arg)
+
+			readFlagValue = !strings.Contains(arg, "=")
+		} else {
+			nonflags = append(nonflags, arg)
 		}
-	} else {
-		flagArgs = args[firstFlagIndex:]
 	}
-	return flagArgs, regularArgs
+
+	return append(flags, nonflags...)
 }
 
 func translateShortOptions(flagArgs Args) []string {