From 6f2f3b4bf6c3fb40a0d2200ce6affee56cf781d8 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Wed, 18 Apr 2018 21:23:08 -0700 Subject: [PATCH 1/4] drop setScalar --- parse.go | 15 +++++---------- parse_test.go | 13 +++++++++++++ 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/parse.go b/parse.go index 10c6841..e272b59 100644 --- a/parse.go +++ b/parse.go @@ -159,7 +159,7 @@ func NewParser(config Config, dests ...interface{}) (*Parser, error) { } // Check whether this field is supported. It's good to do this here rather than - // wait until setScalar because it means that a program with invalid argument + // wait until ParseValue because it means that a program with invalid argument // fields will always fail regardless of whether the arguments it received // exercised those fields. var parseable bool @@ -275,7 +275,7 @@ func process(specs []*spec, args []string) error { } if spec.env != "" { if value, found := os.LookupEnv(spec.env); found { - err := setScalar(spec.dest, value) + err := scalar.ParseValue(spec.dest, value) if err != nil { return fmt.Errorf("error processing environment variable %s: %v", spec.env, err) } @@ -355,7 +355,7 @@ func process(specs []*spec, args []string) error { i++ } - err := setScalar(spec.dest, value) + err := scalar.ParseValue(spec.dest, value) if err != nil { return fmt.Errorf("error processing %s: %v", arg, err) } @@ -374,7 +374,7 @@ func process(specs []*spec, args []string) error { } positionals = nil } else if len(positionals) > 0 { - err := setScalar(spec.dest, positionals[0]) + err := scalar.ParseValue(spec.dest, positionals[0]) if err != nil { return fmt.Errorf("error processing %s: %v", spec.long, err) } @@ -438,7 +438,7 @@ func setSlice(dest reflect.Value, values []string, trunc bool) error { for _, s := range values { v := reflect.New(elem) - if err := setScalar(v.Elem(), s); err != nil { + if err := scalar.ParseValue(v.Elem(), s); err != nil { return err } if !ptr { @@ -500,8 +500,3 @@ func isScalar(t reflect.Type) (parseable, boolean bool) { return parseable, false } } - -// set a value from a string -func setScalar(v reflect.Value, s string) error { - return scalar.ParseValue(v, s) -} diff --git a/parse_test.go b/parse_test.go index 925a23e..ce3ad34 100644 --- a/parse_test.go +++ b/parse_test.go @@ -599,6 +599,19 @@ func TestTextUnmarshaler(t *testing.T) { assert.Equal(t, 3, args.Foo.val) } +func TestRepeatedTextUnmarshaler(t *testing.T) { + // fields that implement TextUnmarshaler should be parsed using that interface + var args struct { + Foo []*textUnmarshaler + } + err := parse("--foo abc d ef", &args) + require.NoError(t, err) + require.Len(t, args.Foo, 3) + assert.Equal(t, 3, args.Foo[0].val) + assert.Equal(t, 1, args.Foo[1].val) + assert.Equal(t, 2, args.Foo[2].val) +} + type boolUnmarshaler bool func (p *boolUnmarshaler) UnmarshalText(b []byte) error { From 74dd5a2c5adaae3e3e12734219ed5e1ab2450447 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Wed, 18 Apr 2018 21:33:46 -0700 Subject: [PATCH 2/4] separate scalar.CanParse from isBoolean --- parse.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/parse.go b/parse.go index e272b59..80f013c 100644 --- a/parse.go +++ b/parse.go @@ -451,7 +451,8 @@ func setSlice(dest reflect.Value, values []string, trunc bool) error { // canParse returns true if the type can be parsed from a string func canParse(t reflect.Type) (parseable, boolean, multiple bool) { - parseable, boolean = isScalar(t) + parseable = scalar.CanParse(t) + boolean = isBoolean(t) if parseable { return } @@ -466,7 +467,8 @@ func canParse(t reflect.Type) (parseable, boolean, multiple bool) { t = t.Elem() } - parseable, boolean = isScalar(t) + parseable = scalar.CanParse(t) + boolean = isBoolean(t) if parseable { return } @@ -476,7 +478,8 @@ func canParse(t reflect.Type) (parseable, boolean, multiple bool) { t = t.Elem() } - parseable, boolean = isScalar(t) + parseable = scalar.CanParse(t) + boolean = isBoolean(t) if parseable { return } @@ -486,17 +489,16 @@ func canParse(t reflect.Type) (parseable, boolean, multiple bool) { var textUnmarshalerType = reflect.TypeOf([]encoding.TextUnmarshaler{}).Elem() -// isScalar returns true if the type can be parsed from a single string -func isScalar(t reflect.Type) (parseable, boolean bool) { - parseable = scalar.CanParse(t) +// isBoolean returns true if the type can be parsed from a single string +func isBoolean(t reflect.Type) bool { switch { case t.Implements(textUnmarshalerType): - return parseable, false + return false case t.Kind() == reflect.Bool: - return parseable, true + return true case t.Kind() == reflect.Ptr && t.Elem().Kind() == reflect.Bool: - return parseable, true + return true default: - return parseable, false + return false } } From b9375a2e66525c12064e8bcd3b2ff68711182ecc Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Wed, 18 Apr 2018 21:51:16 -0700 Subject: [PATCH 3/4] fix repeated text unmarshal bug --- parse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parse.go b/parse.go index 80f013c..1416223 100644 --- a/parse.go +++ b/parse.go @@ -426,7 +426,7 @@ func setSlice(dest reflect.Value, values []string, trunc bool) error { var ptr bool elem := dest.Type().Elem() - if elem.Kind() == reflect.Ptr { + if elem.Kind() == reflect.Ptr && !elem.Implements(textUnmarshalerType) { ptr = true elem = elem.Elem() } From 4d71204936cbe2b4d7ebeb5b8a4d25432599eb17 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Wed, 18 Apr 2018 21:54:27 -0700 Subject: [PATCH 4/4] add positional test --- parse_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/parse_test.go b/parse_test.go index ce3ad34..1461c02 100644 --- a/parse_test.go +++ b/parse_test.go @@ -612,6 +612,19 @@ func TestRepeatedTextUnmarshaler(t *testing.T) { assert.Equal(t, 2, args.Foo[2].val) } +func TestPositionalTextUnmarshaler(t *testing.T) { + // fields that implement TextUnmarshaler should be parsed using that interface + var args struct { + Foo []*textUnmarshaler `arg:"positional"` + } + err := parse("abc d ef", &args) + require.NoError(t, err) + require.Len(t, args.Foo, 3) + assert.Equal(t, 3, args.Foo[0].val) + assert.Equal(t, 1, args.Foo[1].val) + assert.Equal(t, 2, args.Foo[2].val) +} + type boolUnmarshaler bool func (p *boolUnmarshaler) UnmarshalText(b []byte) error {