From bda3448ec3eefd405bcc8b20f815c43a999047c5 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 10 Sep 2026 13:35:05 -0700 Subject: [PATCH] Add split:"false" tag to keep string array values whole []string flags split every value on commas, which is right for lists like --tags a,b but wrong for values that contain commas themselves, such as -e KEY=a,b,c or an HTTP header. There was no way to opt out. StringArrayNoSplitVar registers a string array flag that contributes one element per occurrence, and FromStruct selects it for fields tagged split:"false". A default on such a field is kept whole too. The existing comma-splitting behavior is unchanged for everything else. --- README.md | 3 +- fromstruct.go | 19 ++++++-- mflags.go | 13 +++++ stringarray_nosplit_test.go | 94 +++++++++++++++++++++++++++++++++++++ values.go | 13 ++++- 5 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 stringarray_nosplit_test.go diff --git a/README.md b/README.md index 6980096..72b8a21 100644 --- a/README.md +++ b/README.md @@ -407,7 +407,7 @@ myapp completion zsh > /usr/local/share/zsh/site-functions/_myapp - `bool` - Boolean flags - `string` - String values - `int` - Integer values -- `[]string` - Comma-separated string arrays +- `[]string` - Comma-separated string arrays (add `split:"false"` to keep each occurrence whole) - `time.Duration` - Duration values (parsed by `time.ParseDuration`) ### Struct Tags @@ -421,6 +421,7 @@ myapp completion zsh > /usr/local/share/zsh/site-functions/_myapp | `position` | Positional argument index | `position:"0"` | | `rest` | Capture remaining args | `rest:"true"` | | `unknown` | Capture unknown flags | `unknown:"true"` | +| `split` | For `[]string`: `"false"` stops splitting values on commas, so `-e KEY=a,b` is one element | `split:"false"` | ## Embedded Structs diff --git a/fromstruct.go b/fromstruct.go index 378a4b0..7608da0 100644 --- a/fromstruct.go +++ b/fromstruct.go @@ -69,6 +69,7 @@ var knownTags = map[string]bool{ "rest": true, "unknown": true, "group": true, + "split": true, } // isHiddenTag reports whether a hidden:"..." struct tag value is truthy. @@ -421,11 +422,23 @@ func (f *FlagSet) FromStruct(v any, opts ...FromStructOption) error { case reflect.Slice: switch field.Type.Elem().Kind() { case reflect.String: + // split:"false" keeps each flag occurrence whole instead of + // splitting it on commas; the default is then one element too. var defVal []string - if defaultValue != "" { - defVal = strings.Split(defaultValue, ",") + switch field.Tag.Get("split") { + case "", "true": + if defaultValue != "" { + defVal = strings.Split(defaultValue, ",") + } + f.StringArrayVar(fieldValue.Addr().Interface().(*[]string), longName, short, defVal, usage) + case "false": + if defaultValue != "" { + defVal = []string{defaultValue} + } + f.StringArrayNoSplitVar(fieldValue.Addr().Interface().(*[]string), longName, short, defVal, usage) + default: + return fmt.Errorf("invalid split tag %q on field %s: must be \"true\" or \"false\"", field.Tag.Get("split"), field.Name) } - f.StringArrayVar(fieldValue.Addr().Interface().(*[]string), longName, short, defVal, usage) case reflect.Bool: f.BoolArrayVar(fieldValue.Addr().Interface().(*[]bool), longName, short, usage) case reflect.Int: diff --git a/mflags.go b/mflags.go index 1dcc726..09ec1cc 100644 --- a/mflags.go +++ b/mflags.go @@ -271,6 +271,19 @@ func (f *FlagSet) StringArrayVar(p *[]string, name string, short rune, value []s f.Var(&stringArrayValue{values: p}, name, short, usage) } +// StringArrayNoSplitVar defines a string array flag like StringArrayVar, except +// that each occurrence of the flag contributes exactly one element: the value is +// never split on commas. Use it for flags whose values can contain commas, such +// as KEY=VALUE pairs or HTTP header values. +func (f *FlagSet) StringArrayNoSplitVar(p *[]string, name string, short rune, value []string, usage string) { + if value != nil { + *p = value + } else { + *p = []string{} + } + f.Var(&stringArrayValue{values: p, noSplit: true}, name, short, usage) +} + // StringArray defines a string array flag with the specified name, short form, default value, and usage string. // The return value is the address of a []string variable that stores the value of the flag. // The flag value is expected to be a comma-separated list of strings. diff --git a/stringarray_nosplit_test.go b/stringarray_nosplit_test.go new file mode 100644 index 0000000..e47f92a --- /dev/null +++ b/stringarray_nosplit_test.go @@ -0,0 +1,94 @@ +package mflags + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestStringArrayNoSplitVar(t *testing.T) { + t.Run("keeps commas in a single value", func(t *testing.T) { + fs := NewFlagSet("test") + var env []string + fs.StringArrayNoSplitVar(&env, "env", 'e', nil, "env vars") + + require.NoError(t, fs.Parse([]string{"--env", "KEY=a,b,c"})) + assert.Equal(t, []string{"KEY=a,b,c"}, env) + }) + + t.Run("one element per occurrence", func(t *testing.T) { + fs := NewFlagSet("test") + var env []string + fs.StringArrayNoSplitVar(&env, "env", 'e', nil, "env vars") + + require.NoError(t, fs.Parse([]string{"-e", "A=1", "-eB=2,3", "--env=C=4,5"})) + assert.Equal(t, []string{"A=1", "B=2,3", "C=4,5"}, env) + }) + + t.Run("default is replaced on first set", func(t *testing.T) { + fs := NewFlagSet("test") + var env []string + fs.StringArrayNoSplitVar(&env, "env", 'e', []string{"D=x,y"}, "env vars") + assert.Equal(t, []string{"D=x,y"}, env) + + require.NoError(t, fs.Parse([]string{"-e", "A=1,2"})) + assert.Equal(t, []string{"A=1,2"}, env) + }) + + t.Run("nil default yields an empty slice", func(t *testing.T) { + fs := NewFlagSet("test") + var env []string + fs.StringArrayNoSplitVar(&env, "env", 'e', nil, "env vars") + assert.Equal(t, []string{}, env) + }) +} + +func TestFromStructSplitTag(t *testing.T) { + t.Run("split false keeps values whole", func(t *testing.T) { + type Config struct { + Env []string `long:"env" short:"e" split:"false"` + Tags []string `long:"tags" short:"t"` + } + var cfg Config + fs := NewFlagSet("test") + require.NoError(t, fs.FromStruct(&cfg)) + + require.NoError(t, fs.Parse([]string{"-e", "A=1,2", "-t", "x,y", "-e", "B=3"})) + assert.Equal(t, []string{"A=1,2", "B=3"}, cfg.Env) + assert.Equal(t, []string{"x", "y"}, cfg.Tags, "flags without the tag still split") + }) + + t.Run("split true is the default behavior", func(t *testing.T) { + type Config struct { + Tags []string `long:"tags" split:"true"` + } + var cfg Config + fs := NewFlagSet("test") + require.NoError(t, fs.FromStruct(&cfg)) + + require.NoError(t, fs.Parse([]string{"--tags", "x,y"})) + assert.Equal(t, []string{"x", "y"}, cfg.Tags) + }) + + t.Run("default value is kept whole", func(t *testing.T) { + type Config struct { + Env []string `long:"env" split:"false" default:"A=1,2"` + } + var cfg Config + fs := NewFlagSet("test") + require.NoError(t, fs.FromStruct(&cfg)) + assert.Equal(t, []string{"A=1,2"}, cfg.Env) + }) + + t.Run("other values are rejected", func(t *testing.T) { + type Config struct { + Env []string `long:"env" split:"maybe"` + } + var cfg Config + fs := NewFlagSet("test") + err := fs.FromStruct(&cfg) + require.Error(t, err) + assert.Contains(t, err.Error(), "split") + }) +} diff --git a/values.go b/values.go index d9699a9..27509e6 100644 --- a/values.go +++ b/values.go @@ -342,6 +342,10 @@ func (i *intArrayValue) Type() string { type stringArrayValue struct { values *[]string hasBeenSet bool + // noSplit keeps each flag occurrence as one element instead of splitting + // it on commas, for values that legitimately contain commas (KEY=a,b,c + // environment variables, HTTP header values, and so on). + noSplit bool } func (s *stringArrayValue) Set(val string) error { @@ -350,7 +354,11 @@ func (s *stringArrayValue) Set(val string) error { *s.values = nil s.hasBeenSet = true } - *s.values = append(*s.values, strings.Split(val, ",")...) + if s.noSplit { + *s.values = append(*s.values, val) + } else { + *s.values = append(*s.values, strings.Split(val, ",")...) + } return nil } @@ -366,6 +374,9 @@ func (s *stringArrayValue) IsBool() bool { } func (s *stringArrayValue) Type() string { + if s.noSplit { + return "value" + } return "value,..." }