From 50a0e2b3ed1fb823869c67f35ff18f0bb30e8801 Mon Sep 17 00:00:00 2001 From: David Finkel Date: Fri, 7 Jun 2024 15:12:28 -0400 Subject: [PATCH] flags/pflags: slice/map flags concat/merge Instead of always using the last flag value referenced, merge all values present. --- README.md | 19 +++++++ sources/flag/flag_test.go | 81 ++++++++++++++++++++++++++++++ sources/flag/flaghelper/ints.go | 12 +++-- sources/flag/flaghelper/strings.go | 55 +++++++++++++++----- sources/flag/flaghelper/uints.go | 12 +++-- sources/pflag/pflag_test.go | 55 +++++++++++++++++++- 6 files changed, 215 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 7344cc9..c02d4d5 100644 --- a/README.md +++ b/README.md @@ -277,6 +277,25 @@ If you wish to watch the config file and make updates to your configuration, use }(ctx) ``` +### Flags +When setting commandline flags using either the pflag or flag sources, additional flag-types become available for simple slices and maps. + +#### Slices + +Slices of integer-types get parsed as comma-separated values using Go's parsing rules (with whitespace stripped off each component) +e.g. `--a=1,2,3` parses as `[]int{1,2,3}` + +Slices of strings get parsed as comma-separated values if the individual values are alphanumeric, and must be quoted in conformance with Go's [`strconv.Unquote`](https://pkg.go.dev/strconv#Unquote) for more complicated values +e.g. `--a=abc` parses as `[]string{"abc"}`, `--a=a,b,c` parses as `[]string{"a", "b", "c"}`, while `--a="bbbb,ffff"` has additional quoting (ignoring any shell), so it becomes `[]string{"bbbb,ffff"}` + +Slice-typed flags may be specified multiple times, and the values will be concatenated. +As a result, a commandline with `"--a=b", "--a=c"` may be parsed as `[]string{b,c}`. + +#### Maps +Maps are parsed like Slices, with the addition of `:` separators between keys and values. ([`strconv.Unquote`](https://pkg.go.dev/strconv#Unquote)-compatible quoting is mandatory for more complicated strins as well) + +e.g. `--a=b:c` parses as `map[string]string{"b": "c"}` + ### Source The Source interface is implemented by different configuration sources that populate the configuration struct. Dials currently supports environment variables, command line flags, and config file sources. When the `dials.Config` method is going through the different `Source`s to extract the values, it calls the `Value` method on each of these sources. This allows for the logic of the Source to be encapsulated while giving the application access to the values populated by each Source. Please note that the Value method on the Source interface and the Watcher interface are likely to change in the near future. diff --git a/sources/flag/flag_test.go b/sources/flag/flag_test.go index 578784e..82f0ff0 100644 --- a/sources/flag/flag_test.go +++ b/sources/flag/flag_test.go @@ -265,6 +265,15 @@ func TestTable(t *testing.T) { args: []string{"--a=128,32"}, expected: &struct{ A []int16 }{A: []int16{128, 32}}, }, + { + name: "basic_int16_slice_set_nooverflow_multiple_flag", + tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { + cfg := struct{ A []int16 }{A: []int16{10}} + return &cfg, testWrapDials(&cfg) + }, + args: []string{"--a=128", "--a=32"}, + expected: &struct{ A []int16 }{A: []int16{128, 32}}, + }, { name: "basic_int16_slice_set_overflow", tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { @@ -405,6 +414,15 @@ func TestTable(t *testing.T) { args: []string{"--a=128,32"}, expected: &struct{ A []int64 }{A: []int64{128, 32}}, }, + { + name: "basic_int64_slice_set_nooverflow_multiple_flag", + tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { + cfg := struct{ A []int64 }{A: []int64{10}} + return &cfg, testWrapDials(&cfg) + }, + args: []string{"--a=128", "--a=32"}, + expected: &struct{ A []int64 }{A: []int64{128, 32}}, + }, { name: "basic_uint_defaulted", @@ -535,6 +553,15 @@ func TestTable(t *testing.T) { args: []string{"--a=128,32"}, expected: &struct{ A []uint32 }{A: []uint32{128, 32}}, }, + { + name: "basic_uint32_slice_set_nooverflow_multiple_flag", + tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { + cfg := struct{ A []uint32 }{A: []uint32{10}} + return &cfg, testWrapDials(&cfg) + }, + args: []string{"--a=128", "--a=32"}, + expected: &struct{ A []uint32 }{A: []uint32{128, 32}}, + }, { name: "basic_uint8_default", tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { @@ -628,6 +655,15 @@ func TestTable(t *testing.T) { args: []string{"--a=128,32"}, expected: &struct{ A []uint64 }{A: []uint64{128, 32}}, }, + { + name: "basic_uint64_slice_set_nooverflow_multiple_flag", + tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { + cfg := struct{ A []uint64 }{A: []uint64{10}} + return &cfg, testWrapDials(&cfg) + }, + args: []string{"--a=128", "--a=32"}, + expected: &struct{ A []uint64 }{A: []uint64{128, 32}}, + }, { name: "basic_uintptr_set_nooverflow", tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { @@ -665,6 +701,15 @@ func TestTable(t *testing.T) { args: []string{"--a=128,32"}, expected: &struct{ A []uintptr }{A: []uintptr{128, 32}}, }, + { + name: "basic_uintptr_slice_set_nooverflow_multiple_flag", + tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { + cfg := struct{ A []uintptr }{A: []uintptr{10}} + return &cfg, testWrapDials(&cfg) + }, + args: []string{"--a=128", "--a=32"}, + expected: &struct{ A []uintptr }{A: []uintptr{128, 32}}, + }, { name: "map_string_string_set", tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { @@ -674,6 +719,15 @@ func TestTable(t *testing.T) { args: []string{"--a=l:v"}, expected: &struct{ A map[string]string }{A: map[string]string{"l": "v"}}, }, + { + name: "map_string_string_set_multiple_flags", + tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { + cfg := struct{ A map[string]string }{A: map[string]string{"z": "i"}} + return &cfg, testWrapDials(&cfg) + }, + args: []string{"--a=l:v", "--a=z:sss"}, + expected: &struct{ A map[string]string }{A: map[string]string{"l": "v", "z": "sss"}}, + }, { name: "map_string_string_default", tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { @@ -692,6 +746,15 @@ func TestTable(t *testing.T) { args: []string{"--a=l:v,l:z"}, expected: &struct{ A map[string][]string }{A: map[string][]string{"l": {"v", "z"}}}, }, + { + name: "map_string_string_slice_set_multiple_flag", + tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { + cfg := struct{ A map[string][]string }{A: map[string][]string{"z": {"i"}}} + return &cfg, testWrapDials(&cfg) + }, + args: []string{"--a=l:v", "--a=l:z"}, + expected: &struct{ A map[string][]string }{A: map[string][]string{"l": {"v", "z"}}}, + }, { name: "map_string_string_slice_default", tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { @@ -710,6 +773,15 @@ func TestTable(t *testing.T) { args: []string{"--a=v"}, expected: &struct{ A []string }{A: []string{"v"}}, }, + { + name: "string_slice_set_multiple_flag", + tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { + cfg := struct{ A []string }{A: []string{"i"}} + return &cfg, testWrapDials(&cfg) + }, + args: []string{"--a=v", "--a=zzz"}, + expected: &struct{ A []string }{A: []string{"v", "zzz"}}, + }, { name: "string_slice_default", tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { @@ -728,6 +800,15 @@ func TestTable(t *testing.T) { args: []string{"--a=v"}, expected: &struct{ A map[string]struct{} }{A: map[string]struct{}{"v": {}}}, }, + { + name: "string_set_set_multiple_flag", + tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { + cfg := struct{ A map[string]struct{} }{A: map[string]struct{}{"i": {}}} + return &cfg, testWrapDials(&cfg) + }, + args: []string{"--a=v", "--a=a"}, + expected: &struct{ A map[string]struct{} }{A: map[string]struct{}{"v": {}, "a": {}}}, + }, { name: "string_set_default", tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { diff --git a/sources/flag/flaghelper/ints.go b/sources/flag/flaghelper/ints.go index 5a8cbf9..e07a575 100644 --- a/sources/flag/flaghelper/ints.go +++ b/sources/flag/flaghelper/ints.go @@ -15,12 +15,13 @@ type SignedInt interface { // SignedIntegralSliceFlag is a wrapper around an integral-typed slice type SignedIntegralSliceFlag[I SignedInt] struct { - s *[]I + s *[]I + defaulted bool } // NewSignedIntegralSlice is a constructor for NewSignedIntegralSliceFlag func NewSignedIntegralSlice[I SignedInt](s *[]I) *SignedIntegralSliceFlag[I] { - return &SignedIntegralSliceFlag[I]{s: s} + return &SignedIntegralSliceFlag[I]{s: s, defaulted: true} } // Set implements pflag.Value and flag.Value @@ -29,7 +30,12 @@ func (v *SignedIntegralSliceFlag[I]) Set(s string) error { if err != nil { return err } - *v.s = parsed + if v.defaulted { + *v.s = parsed + v.defaulted = false + return nil + } + *v.s = append(*v.s, parsed...) return nil } diff --git a/sources/flag/flaghelper/strings.go b/sources/flag/flaghelper/strings.go index f70d472..c821979 100644 --- a/sources/flag/flaghelper/strings.go +++ b/sources/flag/flaghelper/strings.go @@ -12,12 +12,13 @@ import ( // StringSliceFlag is a wrapper around a string slice type StringSliceFlag struct { - s *[]string + s *[]string + defaulted bool } // NewStringSliceFlag is a constructor for StringSliceFlag func NewStringSliceFlag(s *[]string) *StringSliceFlag { - return &StringSliceFlag{s: s} + return &StringSliceFlag{s: s, defaulted: true} } // Set implement pflag.Value and flag.Value @@ -26,7 +27,13 @@ func (v *StringSliceFlag) Set(s string) error { if err != nil { return err } - v.s = &parsed + if v.defaulted { + v.s = &parsed + v.defaulted = false + return nil + } + newSlice := append(*v.s, parsed...) + v.s = &newSlice return nil } @@ -55,12 +62,13 @@ func (v *StringSliceFlag) String() string { // StringSetFlag is a wrapper around map[string]struct used for implementing sets type StringSetFlag struct { - s *map[string]struct{} + s *map[string]struct{} + defaulted bool } // NewStringSetFlag is the constructor for StringSetFlags func NewStringSetFlag(m *map[string]struct{}) *StringSetFlag { - return &StringSetFlag{s: m} + return &StringSetFlag{s: m, defaulted: true} } // Set implement pflag.Value and flag.Value @@ -69,7 +77,14 @@ func (v *StringSetFlag) Set(s string) error { if err != nil { return err } - *v.s = parsed + if *v.s == nil || v.defaulted { + *v.s = parsed + v.defaulted = false + return nil + } + for s := range parsed { + (*v.s)[s] = struct{}{} + } return nil } @@ -111,12 +126,13 @@ func (v *StringSetFlag) Type() string { // MapStringStringSliceFlag is a wrapper around map[string][]string type MapStringStringSliceFlag struct { - s *map[string][]string + s *map[string][]string + defaulted bool } // NewMapStringStringSliceFlag is the constructor for MapStringStringSliceFlag func NewMapStringStringSliceFlag(m *map[string][]string) *MapStringStringSliceFlag { - return &MapStringStringSliceFlag{s: m} + return &MapStringStringSliceFlag{s: m, defaulted: true} } // Set implement pflag.Value and flag.Value @@ -125,7 +141,14 @@ func (v *MapStringStringSliceFlag) Set(s string) error { if err != nil { return err } - *v.s = parsed + if *v.s == nil || v.defaulted { + *v.s = parsed + v.defaulted = false + return nil + } + for k, innerSlice := range parsed { + (*v.s)[k] = append((*v.s)[k], innerSlice...) + } return nil } @@ -171,12 +194,13 @@ func (v *MapStringStringSliceFlag) Type() string { // MapStringStringFlag is a wrapper around *map[string]string type MapStringStringFlag struct { - s *map[string]string + s *map[string]string + defaulted bool } // NewMapStringStringFlag is the constructor for MapStringStringFlag func NewMapStringStringFlag(m *map[string]string) *MapStringStringFlag { - return &MapStringStringFlag{s: m} + return &MapStringStringFlag{s: m, defaulted: true} } // Set implement pflag.Value and flag.Value @@ -186,7 +210,14 @@ func (v *MapStringStringFlag) Set(s string) error { return err } castParsed := parsed.Interface().(map[string]string) - *v.s = castParsed + if v.s == nil || v.defaulted { + *v.s = castParsed + v.defaulted = false + return nil + } + for k, val := range castParsed { + (*v.s)[k] = val + } return nil } diff --git a/sources/flag/flaghelper/uints.go b/sources/flag/flaghelper/uints.go index 42c07d1..4e8c3a7 100644 --- a/sources/flag/flaghelper/uints.go +++ b/sources/flag/flaghelper/uints.go @@ -15,12 +15,13 @@ type UnsignedInt interface { // UnsignedIntegralSliceFlag is a wrapper around an unsigned integral-typed slice type UnsignedIntegralSliceFlag[I UnsignedInt] struct { - s *[]I + s *[]I + defaulted bool } // NewUnsignedIntegralSlice is a constructor for StringSliceFlag func NewUnsignedIntegralSlice[I UnsignedInt](s *[]I) *UnsignedIntegralSliceFlag[I] { - return &UnsignedIntegralSliceFlag[I]{s: s} + return &UnsignedIntegralSliceFlag[I]{s: s, defaulted: true} } // Set implements pflag.Value and flag.Value @@ -29,7 +30,12 @@ func (v *UnsignedIntegralSliceFlag[I]) Set(s string) error { if err != nil { return err } - *v.s = parsed + if v.defaulted { + *v.s = parsed + v.defaulted = false + return nil + } + *v.s = append(*v.s, parsed...) return nil } diff --git a/sources/pflag/pflag_test.go b/sources/pflag/pflag_test.go index 1a37c54..9e5d22b 100644 --- a/sources/pflag/pflag_test.go +++ b/sources/pflag/pflag_test.go @@ -333,6 +333,15 @@ func TestPFlags(t *testing.T) { args: []string{"--a=128,32"}, expected: &struct{ A []uint16 }{A: []uint16{128, 32}}, }, + { + name: "basic_uint16_slice_set_nooverflow_multiple_flag", + tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { + cfg := struct{ A []uint16 }{A: []uint16{10}} + return &cfg, testWrapDials(&cfg) + }, + args: []string{"--a=128", "--a=32"}, + expected: &struct{ A []uint16 }{A: []uint16{128, 32}}, + }, { name: "basic_uint16_slice_set_overflow", tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { @@ -510,7 +519,15 @@ func TestPFlags(t *testing.T) { args: []string{"--a=128,32"}, expected: &struct{ A []uintptr }{A: []uintptr{128, 32}}, }, - + { + name: "basic_uintptr_slice_set_nooverflow_multiple_flag", + tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { + cfg := struct{ A []uintptr }{A: []uintptr{10}} + return &cfg, testWrapDials(&cfg) + }, + args: []string{"--a=128", "--a=32"}, + expected: &struct{ A []uintptr }{A: []uintptr{128, 32}}, + }, { name: "map_string_string_set", tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { @@ -520,6 +537,15 @@ func TestPFlags(t *testing.T) { args: []string{"--a=l:v"}, expected: &struct{ A map[string]string }{A: map[string]string{"l": "v"}}, }, + { + name: "map_string_string_set_multiple_flags", + tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { + cfg := struct{ A map[string]string }{A: map[string]string{"z": "i"}} + return &cfg, testWrapDials(&cfg) + }, + args: []string{"--a=l:v", "--a=z:sss"}, + expected: &struct{ A map[string]string }{A: map[string]string{"l": "v", "z": "sss"}}, + }, { name: "map_string_string_default", tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { @@ -538,6 +564,15 @@ func TestPFlags(t *testing.T) { args: []string{"--a=l:v,l:z"}, expected: &struct{ A map[string][]string }{A: map[string][]string{"l": {"v", "z"}}}, }, + { + name: "map_string_string_slice_set_multiple_flag", + tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { + cfg := struct{ A map[string][]string }{A: map[string][]string{"z": {"i"}}} + return &cfg, testWrapDials(&cfg) + }, + args: []string{"--a=l:v", "--a=l:z"}, + expected: &struct{ A map[string][]string }{A: map[string][]string{"l": {"v", "z"}}}, + }, { name: "map_string_string_slice_default", tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { @@ -556,6 +591,15 @@ func TestPFlags(t *testing.T) { args: []string{"--a=v"}, expected: &struct{ A map[string]struct{} }{A: map[string]struct{}{"v": {}}}, }, + { + name: "string_slice_set_multiple_flag", + tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { + cfg := struct{ A []string }{A: []string{"i"}} + return &cfg, testWrapDials(&cfg) + }, + args: []string{"--a=v", "--a=zzz"}, + expected: &struct{ A []string }{A: []string{"v", "zzz"}}, + }, { name: "string_set_default", tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { @@ -565,6 +609,15 @@ func TestPFlags(t *testing.T) { args: []string{}, expected: &struct{ A map[string]struct{} }{A: map[string]struct{}{"i": {}}}, }, + { + name: "string_set_set_multiple_flag", + tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) { + cfg := struct{ A map[string]struct{} }{A: map[string]struct{}{"i": {}}} + return &cfg, testWrapDials(&cfg) + }, + args: []string{"--a=v", "--a=a"}, + expected: &struct{ A map[string]struct{} }{A: map[string]struct{}{"v": {}, "a": {}}}, + }, { name: "int_slice_default_val", tmplCB: func() (any, func(ctx context.Context, src *Set) (any, error)) {