Skip to content

Commit

Permalink
Fix handling of -- delimiter
Browse files Browse the repository at this point in the history
If a `--` delimiter is encountered, any remaining _non-option-value_
arguments must be handled as argument.

From the POSIX spec (emphasis mine)
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_0):

> 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.

There was a corner-case scenario, where a `--flag` followed by a `--` did
not use `--` as value for the flag, but instead considered it as delimiter.

As a result;

    foo test arg1 --opt -- arg2

Would be reordered as:

   foo test --opt arg1 arg2

Which caused `arg1` to be used as value for `--opt`, instead of `--`,
which is the actual value.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
  • Loading branch information
thaJeztah committed May 14, 2020
1 parent 464c242 commit 3358e6f
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 16 deletions.
6 changes: 2 additions & 4 deletions app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
30 changes: 18 additions & 12 deletions command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 && !argIsFlag(commandFlags, 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
Expand All @@ -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 -
Expand Down
12 changes: 12 additions & 0 deletions command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,17 @@ func TestParseAndRunHyphenValues(t *testing.T) {
{[]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 {
Expand All @@ -161,6 +172,7 @@ func TestParseAndRunHyphenValues(t *testing.T) {
},
Flags: []Flag{
StringFlag{Name: "opt"},
StringFlag{Name: "opt2"},
},
}

Expand Down

0 comments on commit 3358e6f

Please sign in to comment.