From c538c376c9859540836cbbf2096fa90efa9a602f Mon Sep 17 00:00:00 2001 From: Nathan LeClaire Date: Fri, 25 Sep 2015 14:13:36 -0700 Subject: [PATCH 1/5] Do not return error when flag parsing should be skipped Signed-off-by: Nathan LeClaire --- command.go | 6 ++++++ command_test.go | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/command.go b/command.go index fac754d..7a1dcab 100644 --- a/command.go +++ b/command.go @@ -102,6 +102,12 @@ func (c Command) Run(ctx *Context) error { err = set.Parse(append(flagArgs, regularArgs...)) } else { err = set.Parse(ctx.Args().Tail()) + + // Work around issue where if the first arg in ctx.Args.Tail() + // is a flag, set.Parse returns an error + if c.SkipFlagParsing { + err = nil + } } if err != nil { diff --git a/command_test.go b/command_test.go index fd39548..bf7cc99 100644 --- a/command_test.go +++ b/command_test.go @@ -45,3 +45,25 @@ func TestCommandIgnoreFlags(t *testing.T) { expect(t, err, nil) } + +// Fix bug with ignoring flag parsing that would still parse the first flag +func TestCommandIgnoreFlagsIncludingFirstArgument(t *testing.T) { + app := NewApp() + set := flag.NewFlagSet("test", 0) + test := []string{"blah", "-break"} + set.Parse(test) + + c := NewContext(app, set, nil) + + command := Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(_ *Context) {}, + SkipFlagParsing: true, + } + err := command.Run(c) + + expect(t, err, nil) +} From bc3cb33cef749380a3e1ae4c1a04312521c5f0be Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sat, 17 Oct 2015 11:36:09 -0700 Subject: [PATCH 2/5] Actually skip parsing of flags if SkipFlagParsing is set Previous just skipped if firstFlagIndex was > -1 --- command.go | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/command.go b/command.go index 7a1dcab..c63b049 100644 --- a/command.go +++ b/command.go @@ -86,27 +86,23 @@ func (c Command) Run(ctx *Context) error { } var err error - if firstFlagIndex > -1 && !c.SkipFlagParsing { - args := ctx.Args() - regularArgs := make([]string, len(args[1:firstFlagIndex])) - copy(regularArgs, args[1:firstFlagIndex]) + if !c.SkipFlagParsing { + 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:]...) + 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 { - flagArgs = args[firstFlagIndex:] - } - - err = set.Parse(append(flagArgs, regularArgs...)) - } else { - err = set.Parse(ctx.Args().Tail()) - - // Work around issue where if the first arg in ctx.Args.Tail() - // is a flag, set.Parse returns an error - if c.SkipFlagParsing { - err = nil + err = set.Parse(ctx.Args().Tail()) } } From 6191d931b7fc228924bd8dd6a3952301b14078af Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Mon, 26 Oct 2015 21:40:03 -0700 Subject: [PATCH 3/5] When skipping flag parsing, still parse into arguments Fool the FlagSet into thinking that all arguments are actually arguments rather than attempting to parse them as flags. --- command.go | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/command.go b/command.go index c63b049..824e77b 100644 --- a/command.go +++ b/command.go @@ -74,19 +74,19 @@ func (c Command) Run(ctx *Context) error { set := flagSet(c.Name, c.Flags) set.SetOutput(ioutil.Discard) - firstFlagIndex := -1 - terminatorIndex := -1 - for index, arg := range ctx.Args() { - if arg == "--" { - terminatorIndex = index - break - } else if strings.HasPrefix(arg, "-") && firstFlagIndex == -1 { - firstFlagIndex = index - } - } - var err error if !c.SkipFlagParsing { + firstFlagIndex := -1 + terminatorIndex := -1 + for index, arg := range ctx.Args() { + if arg == "--" { + terminatorIndex = index + break + } else if strings.HasPrefix(arg, "-") && firstFlagIndex == -1 { + firstFlagIndex = index + } + } + if firstFlagIndex > -1 { args := ctx.Args() regularArgs := make([]string, len(args[1:firstFlagIndex])) @@ -104,6 +104,10 @@ func (c Command) Run(ctx *Context) error { } else { err = set.Parse(ctx.Args().Tail()) } + } else { + if c.SkipFlagParsing { + err = set.Parse(append([]string{"--"}, ctx.Args().Tail()...)) + } } if err != nil { From 8cd49b108c0b76b4de71d89a106c7f6e59e536fe Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Mon, 26 Oct 2015 21:45:28 -0700 Subject: [PATCH 4/5] Update TestCommandIgnoreFlagsIncludingFirstArgument to test for arguments --- command_test.go | 3 ++- helpers_test.go | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/command_test.go b/command_test.go index bf7cc99..1e9e7fa 100644 --- a/command_test.go +++ b/command_test.go @@ -64,6 +64,7 @@ func TestCommandIgnoreFlagsIncludingFirstArgument(t *testing.T) { SkipFlagParsing: true, } err := command.Run(c) - expect(t, err, nil) + + expect(t, []string(c.Args()), []string{"blah", "-break"}) } diff --git a/helpers_test.go b/helpers_test.go index 3ce8e93..b1b7339 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -7,13 +7,13 @@ import ( /* Test Helpers */ func expect(t *testing.T, a interface{}, b interface{}) { - if a != b { + if !reflect.DeepEqual(a, b) { t.Errorf("Expected %v (type %v) - Got %v (type %v)", b, reflect.TypeOf(b), a, reflect.TypeOf(a)) } } func refute(t *testing.T, a interface{}, b interface{}) { - if a == b { + if reflect.DeepEqual(a, b) { t.Errorf("Did not expect %v (type %v) - Got %v (type %v)", b, reflect.TypeOf(b), a, reflect.TypeOf(a)) } } From 3323ab44608a53d356d25fa462a114b111950f2f Mon Sep 17 00:00:00 2001 From: Nathan LeClaire Date: Tue, 27 Oct 2015 13:33:55 -0700 Subject: [PATCH 5/5] Use a test table and add --help test Signed-off-by: Nathan LeClaire --- command_test.go | 85 +++++++++++++++++-------------------------------- 1 file changed, 29 insertions(+), 56 deletions(-) diff --git a/command_test.go b/command_test.go index 1e9e7fa..dd9fc87 100644 --- a/command_test.go +++ b/command_test.go @@ -1,70 +1,43 @@ package cli import ( + "errors" "flag" "testing" ) -func TestCommandDoNotIgnoreFlags(t *testing.T) { - app := NewApp() - set := flag.NewFlagSet("test", 0) - test := []string{"blah", "blah", "-break"} - set.Parse(test) - - c := NewContext(app, set, nil) - - command := Command{ - Name: "test-cmd", - Aliases: []string{"tc"}, - Usage: "this is for testing", - Description: "testing", - Action: func(_ *Context) {}, +func TestCommandFlagParsing(t *testing.T) { + cases := []struct { + testArgs []string + skipFlagParsing 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 } - err := command.Run(c) - expect(t, err.Error(), "flag provided but not defined: -break") -} + for _, c := range cases { + app := NewApp() + set := flag.NewFlagSet("test", 0) + set.Parse(c.testArgs) -func TestCommandIgnoreFlags(t *testing.T) { - app := NewApp() - set := flag.NewFlagSet("test", 0) - test := []string{"blah", "blah", "-break"} - set.Parse(test) + context := NewContext(app, set, nil) - c := NewContext(app, set, nil) + command := Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(_ *Context) {}, + } - command := Command{ - Name: "test-cmd", - Aliases: []string{"tc"}, - Usage: "this is for testing", - Description: "testing", - Action: func(_ *Context) {}, - SkipFlagParsing: true, + command.SkipFlagParsing = c.skipFlagParsing + + err := command.Run(context) + + expect(t, err, c.expectedErr) + expect(t, []string(context.Args()), c.testArgs) } - err := command.Run(c) - - expect(t, err, nil) -} - -// Fix bug with ignoring flag parsing that would still parse the first flag -func TestCommandIgnoreFlagsIncludingFirstArgument(t *testing.T) { - app := NewApp() - set := flag.NewFlagSet("test", 0) - test := []string{"blah", "-break"} - set.Parse(test) - - c := NewContext(app, set, nil) - - command := Command{ - Name: "test-cmd", - Aliases: []string{"tc"}, - Usage: "this is for testing", - Description: "testing", - Action: func(_ *Context) {}, - SkipFlagParsing: true, - } - err := command.Run(c) - expect(t, err, nil) - - expect(t, []string(c.Args()), []string{"blah", "-break"}) }