From 3df4b718425bc9a59c5bed1b54854bd0bc185af6 Mon Sep 17 00:00:00 2001 From: Frances Bar Date: Sun, 11 Sep 2016 12:04:35 -0700 Subject: [PATCH] Backport no argument reordering as an option In the unreleased version 2, the argument reordering has been removed (in f585ec7cb8b0d9189ac9016fe83f911f3a360f2e) since it only worked if all of the arguments appeared before all of the flags, but not if they were intermixed which was of limited utility and caused some confusion. This commit allows enabling of this future behavior via SkipArgReorder. Ideally we'd support complete intermingling of flags and arguments, but this is unlikely to happen until we switch flag parsers. Fixes #515 --- command.go | 13 ++++++++----- command_test.go | 28 +++++++++++++++++----------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/command.go b/command.go index 8950cca..3137649 100644 --- a/command.go +++ b/command.go @@ -46,6 +46,11 @@ type Command struct { Flags []Flag // Treat all flags as normal arguments if true SkipFlagParsing bool + // Skip argument reordering which attempts to move flags before arguments, + // but only works if all flags appear after all arguments. This behavior was + // removed n version 2 since it only works under specific conditions so we + // backport here by exposing it as an option for compatibility. + SkipArgReorder bool // Boolean to hide built-in help command HideHelp bool // Boolean to hide this command from help or completion @@ -89,7 +94,9 @@ func (c Command) Run(ctx *Context) (err error) { set := flagSet(c.Name, c.Flags) set.SetOutput(ioutil.Discard) - if !c.SkipFlagParsing { + 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() { @@ -121,10 +128,6 @@ func (c Command) Run(ctx *Context) (err error) { } else { err = set.Parse(ctx.Args().Tail()) } - } else { - if c.SkipFlagParsing { - err = set.Parse(append([]string{"--"}, ctx.Args().Tail()...)) - } } if err != nil { diff --git a/command_test.go b/command_test.go index 1d5a711..5e0e8de 100644 --- a/command_test.go +++ b/command_test.go @@ -13,12 +13,18 @@ func TestCommandFlagParsing(t *testing.T) { cases := []struct { testArgs []string skipFlagParsing bool + skipArgReorder bool expectedErr error }{ - {[]string{"blah", "blah", "-break"}, false, errors.New("flag provided but not defined: -break")}, // Test normal "not ignoring flags" flow - {[]string{"blah", "blah"}, true, nil}, // Test SkipFlagParsing without any args that look like flags - {[]string{"blah", "-break"}, true, nil}, // Test SkipFlagParsing with random flag arg - {[]string{"blah", "-help"}, true, nil}, // Test SkipFlagParsing with "special" help flag arg + // Test normal "not ignoring flags" flow + {[]string{"test-cmd", "blah", "blah", "-break"}, false, false, errors.New("flag provided but not defined: -break")}, + + // Test no arg reorder + {[]string{"test-cmd", "blah", "blah", "-break"}, false, true, nil}, + + {[]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 { @@ -30,15 +36,15 @@ 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 }, + 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, } - command.SkipFlagParsing = c.skipFlagParsing - err := command.Run(context) expect(t, err, c.expectedErr)