From 3c0b5a69cda1bca403bd88a123ae8134ffbbe34c Mon Sep 17 00:00:00 2001 From: Hugo Chargois Date: Sat, 17 Dec 2016 04:29:52 +0100 Subject: [PATCH 1/2] Fix PassAfterNonOption not working with positional args --- options_test.go | 98 +++++++++++++++++++++++++++++++++++++++++++++++++ parser.go | 30 ++++++--------- 2 files changed, 109 insertions(+), 19 deletions(-) diff --git a/options_test.go b/options_test.go index b0fe9f4..110fe2f 100644 --- a/options_test.go +++ b/options_test.go @@ -1,6 +1,7 @@ package flags import ( + "strings" "testing" ) @@ -43,3 +44,100 @@ func TestPassAfterNonOption(t *testing.T) { assertStringArray(t, ret, []string{"arg", "-v", "-g"}) } + +func TestPassAfterNonOptionWithPositional(t *testing.T) { + var opts = struct { + Value bool `short:"v"` + + Positional struct { + Rest []string `required:"yes"` + } `positional-args:"yes"` + }{} + + p := NewParser(&opts, PassAfterNonOption) + ret, err := p.ParseArgs([]string{"-v", "arg", "-v", "-g"}) + + if err != nil { + t.Fatalf("Unexpected error: %v", err) + return + } + + if !opts.Value { + t.Errorf("Expected Value to be true") + } + + assertStringArray(t, ret, []string{}) + assertStringArray(t, opts.Positional.Rest, []string{"arg", "-v", "-g"}) +} + +func TestPassAfterNonOptionWithPositionalIntPass(t *testing.T) { + var opts = struct { + Value bool `short:"v"` + + Positional struct { + Rest []int `required:"yes"` + } `positional-args:"yes"` + }{} + + p := NewParser(&opts, PassAfterNonOption) + ret, err := p.ParseArgs([]string{"-v", "1", "2", "3"}) + + if err != nil { + t.Fatalf("Unexpected error: %v", err) + return + } + + if !opts.Value { + t.Errorf("Expected Value to be true") + } + + assertStringArray(t, ret, []string{}) + for i, rest := range opts.Positional.Rest { + if rest != i+1 { + assertErrorf(t, "Expected %v got %v", i+1, rest) + } + } +} + +func TestPassAfterNonOptionWithPositionalIntFail(t *testing.T) { + var opts = struct { + Value bool `short:"v"` + + Positional struct { + Rest []int `required:"yes"` + } `positional-args:"yes"` + }{} + + tests := []struct { + opts []string + errContains string + ret []string + }{ + { + []string{"-v", "notint1", "notint2", "notint3"}, + "notint1", + []string{"notint1", "notint2", "notint3"}, + }, + { + []string{"-v", "1", "notint2", "notint3"}, + "notint2", + []string{"1", "notint2", "notint3"}, + }, + } + + for _, test := range tests { + p := NewParser(&opts, PassAfterNonOption) + ret, err := p.ParseArgs(test.opts) + + if err == nil { + assertErrorf(t, "Expected error") + return + } + + if !strings.Contains(err.Error(), test.errContains) { + assertErrorf(t, "Expected the first illegal argument in the error") + } + + assertStringArray(t, ret, test.ret) + } +} diff --git a/parser.go b/parser.go index fd2fd5f..ab21158 100644 --- a/parser.go +++ b/parser.go @@ -237,6 +237,7 @@ func (p *Parser) ParseArgs(args []string) ([]string, error) { p.fillParseState(s) for !s.eof() { + var err error arg := s.pop() // When PassDoubleDash is set and we encounter a --, then @@ -247,6 +248,15 @@ func (p *Parser) ParseArgs(args []string) ([]string, error) { } if !argumentIsOption(arg) { + if (p.Options & PassAfterNonOption) != None { + // If PassAfterNonOption is set then all remaining arguments + // are considered positional + if err = s.addArgs(s.arg); err != nil { + break + } + s.addArgs(s.args...) + break + } // Note: this also sets s.err, so we can just check for // nil here and use s.err later if p.parseNonOption(s) != nil { @@ -256,8 +266,6 @@ func (p *Parser) ParseArgs(args []string) ([]string, error) { continue } - var err error - prefix, optname, islong := stripOptionPrefix(arg) optname, _, argument := splitOption(prefix, optname, islong) @@ -649,23 +657,7 @@ func (p *Parser) parseNonOption(s *parseState) error { } } - if (p.Options & PassAfterNonOption) != None { - // If PassAfterNonOption is set then all remaining arguments - // are considered positional - if err := s.addArgs(s.arg); err != nil { - return err - } - - if err := s.addArgs(s.args...); err != nil { - return err - } - - s.args = []string{} - } else { - return s.addArgs(s.arg) - } - - return nil + return s.addArgs(s.arg) } func (p *Parser) showBuiltinHelp() error { From 5e95b3a0401b96a18250d374726bc654214ece8f Mon Sep 17 00:00:00 2001 From: Jesse van den Kieboom Date: Sun, 23 Sep 2018 09:30:17 +0200 Subject: [PATCH 2/2] Catch error on s.addArgs --- parser.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/parser.go b/parser.go index 8c8216a..d75a17c 100644 --- a/parser.go +++ b/parser.go @@ -258,9 +258,14 @@ func (p *Parser) ParseArgs(args []string) ([]string, error) { if err = s.addArgs(s.arg); err != nil { break } - s.addArgs(s.args...) + + if err = s.addArgs(s.args...); err != nil { + break + } + break } + // Note: this also sets s.err, so we can just check for // nil here and use s.err later if p.parseNonOption(s) != nil {