Merging 1135 fix flag parsing

app_test.go

@@ -546,8 +546,7 @@ func TestApp_CommandWithFlagBeforeTerminator(t *testing.T) {
 
 	expect(t, parsedOption, "my-option")
 	expect(t, args[0], "my-arg")
-	expect(t, args[1], "--")
-	expect(t, args[2], "--notARealFlag")
+	expect(t, args[1], "--notARealFlag")
 }
 
 func TestApp_CommandWithDash(t *testing.T) {
@@ -585,8 +584,7 @@ func TestApp_CommandWithNoFlagBeforeTerminator(t *testing.T) {
 	_ = app.Run([]string{"", "cmd", "my-arg", "--", "notAFlagAtAll"})
 
 	expect(t, args[0], "my-arg")
-	expect(t, args[1], "--")
-	expect(t, args[2], "notAFlagAtAll")
+	expect(t, args[1], "notAFlagAtAll")
 }
 
 func TestApp_VisibleCommands(t *testing.T) {

command.go

@@ -226,18 +226,23 @@ func reorderArgs(commandFlags []Flag, args []string) []string {
 	nextIndexMayContainValue := false
 	for i, arg := range args {
 
-		// dont reorder any args after a --
-		// read about -- here:
-		// https://unix.stackexchange.com/questions/11376/what-does-double-dash-mean-also-known-as-bare-double-dash
-		if arg == "--" {
-			remainingArgs = append(remainingArgs, args[i:]...)
-			break
-
-			// checks if this arg is a value that should be re-ordered next to its associated flag
-		} else if nextIndexMayContainValue && !strings.HasPrefix(arg, "-") {
+		// if we're expecting an option-value, check if this arg is a value, in
+		// which case it should be re-ordered next to its associated flag
+		if nextIndexMayContainValue && !argIsFlag(commandFlags, arg) {
 			nextIndexMayContainValue = false
 			reorderedArgs = append(reorderedArgs, arg)
-
+		} else if arg == "--" {
+			// don't reorder any args after the -- delimiter As described in the POSIX spec:
+			// https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_02
+			// > Guideline 10:
+			// >   The first -- argument that is not an option-argument should be accepted
+			// >   as a delimiter indicating the end of options. Any following arguments
+			// >   should be treated as operands, even if they begin with the '-' character.
+
+			// make sure the "--" delimiter itself is at the start
+			remainingArgs = append([]string{"--"}, remainingArgs...)
+			remainingArgs = append(remainingArgs, args[i+1:]...)
+			break
 			// checks if this is an arg that should be re-ordered
 		} else if argIsFlag(commandFlags, arg) {
 			// we have determined that this is a flag that we should re-order
@@ -256,8 +261,9 @@ func reorderArgs(commandFlags []Flag, args []string) []string {
 
 // argIsFlag checks if an arg is one of our command flags
 func argIsFlag(commandFlags []Flag, arg string) bool {
-	// checks if this is just a `-`, and so definitely not a flag
-	if arg == "-" {
+	if arg == "-" || arg == "--"{
+		// `-` is never a flag
+		// `--` is an option-value when following a flag, and a delimiter indicating the end of options in other cases.
 		return false
 	}
 	// flags always start with a -

command_test.go

@@ -114,6 +114,83 @@ func TestParseAndRunShortOpts(t *testing.T) {
 	}
 }
 
+// test-case for https://github.com/urfave/cli/issues/1092
+func TestParseAndRunHyphenValues(t *testing.T) {
+	cases := []struct {
+		testArgs     []string
+		expectedArgs []string
+		expectedOpt string
+	}{
+		{[]string{"foo", "test", "argz"}, []string{"argz"}, ""},
+		{[]string{"foo", "test", "argz", "arga"}, []string{"argz", "arga"}, ""},
+		{[]string{"foo", "test", "--opt", "opt-value"}, []string{}, "opt-value"},
+		{[]string{"foo", "test", "--opt", "opt-value", "argz"}, []string{"argz"}, "opt-value"},
+		{[]string{"foo", "test", "--opt", "opt-value", "argz", "arga"}, []string{"argz", "arga"}, "opt-value"},
+		{[]string{"foo", "test", "argz", "--opt", "opt-value"}, []string{"argz"}, "opt-value"},
+		{[]string{"foo", "test", "argz", "--opt", "opt-value", "arga"}, []string{"argz", "arga"}, "opt-value"},
+
+		{[]string{"foo", "test", "--opt", "-opt-value"}, []string{}, "-opt-value"},
+		{[]string{"foo", "test", "--opt", "-opt-value", "argz"}, []string{"argz"}, "-opt-value"},
+		{[]string{"foo", "test", "--opt", "-opt-value", "argz", "arga"}, []string{"argz", "arga"}, "-opt-value"},
+		{[]string{"foo", "test", "argz", "--opt", "-opt-value"}, []string{"argz"}, "-opt-value"},
+		{[]string{"foo", "test", "argz", "--opt", "-opt-value", "arga"}, []string{"argz", "arga"}, "-opt-value"},
+
+		{[]string{"foo", "test", "--opt", "--opt-value"}, []string{}, "--opt-value"},
+		{[]string{"foo", "test", "--opt", "--opt-value", "argz"}, []string{"argz"}, "--opt-value"},
+		{[]string{"foo", "test", "--opt", "--opt-value", "argz", "arga"}, []string{"argz", "arga"}, "--opt-value"},
+		{[]string{"foo", "test", "argz", "--opt", "--opt-value"}, []string{"argz"}, "--opt-value"},
+		{[]string{"foo", "test", "argz", "--opt", "--opt-value", "arga"}, []string{"argz", "arga"}, "--opt-value"},
+
+		// Tests below test handling of the "--" delimiter
+		{[]string{"foo", "test", "--", "--opt", "--opt-value", "argz"}, []string{"--opt", "--opt-value", "argz"}, ""},
+		{[]string{"foo", "test", "--", "argz", "--opt", "--opt-value", "arga"}, []string{"argz", "--opt", "--opt-value", "arga"}, ""},
+		{[]string{"foo", "test", "argz", "--", "--opt", "--opt-value", "arga"}, []string{"argz", "--opt", "--opt-value", "arga"}, ""},
+		{[]string{"foo", "test", "argz", "--opt", "--", "--opt-value", "arga"}, []string{"argz", "--opt-value", "arga"}, "--"},
+
+		// first "--" is an option-value for "--opt", second "--" is the delimiter indicating the end of options.
+		{[]string{"foo", "test", "argz", "--opt", "--", "--", "--opt2", "--opt-value", "arga"}, []string{"argz", "--opt2", "--opt-value", "arga"}, "--"},
+		{[]string{"foo", "test", "argz", "--opt", "--opt-value", "--", "arga"}, []string{"argz", "arga"}, "--opt-value"},
+		{[]string{"foo", "test", "argz", "--opt", "--opt-value", "arga", "--"}, []string{"argz", "arga"}, "--opt-value"},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+		t.Run(strings.Join(tc.testArgs, "_"), func(t *testing.T){
+			var (
+				args []string
+				opt string
+			)
+
+			cmd := Command{
+				Name:        "test",
+				Usage:       "this is for testing",
+				Description: "testing",
+				Action: func(c *Context) error {
+					args = c.Args()
+					opt = c.String("opt")
+					return nil
+				},
+				Flags: []Flag{
+					StringFlag{Name:  "opt"},
+					StringFlag{Name:  "opt2"},
+				},
+			}
+
+			app := NewApp()
+			app.Writer = ioutil.Discard
+			app.ErrWriter = ioutil.Discard
+			app.Commands = []Command{cmd}
+
+			err := app.Run(tc.testArgs)
+
+			expect(t, err, nil)
+			expect(t, args, tc.expectedArgs)
+			expect(t, opt, tc.expectedOpt)
+		})
+
+	}
+}
+
 func TestCommand_Run_DoesNotOverwriteErrorFromBefore(t *testing.T) {
 	app := NewApp()
 	app.Commands = []Command{