Multi-valued values default value not cleared when set by user (#54)

If you define a multi-valued option or arg, e.g. `StringsOpt` , `IntsArg`, etc. with a default value, this default value is not clered even when the value is set by the user. Example: ```go app := App("app", "") app.ErrorHandling = flag.ContinueOnError opt := app.StringsOpt("o option", []string{"a", "b"}, "") ``` If you run the app with: `app -o=c -o=d`, the final value of the `opt` variable will be `["a", "b", "c", "d"]`. The fix: the multi-valued values were only cleared if their value was set from an env var. This check was removed, so that these values are always cleared before being set.
jawher · Aug 20, 2017 · a459d59 · a459d59
1 parent e01aafc
commit a459d59
Show file tree

Hide file tree

Showing 2 changed files with 322 additions and 4 deletions.
diff --git a/cli_test.go b/cli_test.go
@@ -47,6 +47,326 @@ func TestImplicitSpec(t *testing.T) {
  require.True(t, called, "Exec wasn't called")
 }
 
+func TestAppWithBoolOption(t *testing.T) {
+
+ cases := []struct {
+ args []string
+ expectedOptValue bool
+ }{
+ {[]string{"app"}, false},
+ {[]string{"app", "-o"}, true},
+ {[]string{"app", "-o=true"}, true},
+ {[]string{"app", "-o=false"}, false},
+
+ {[]string{"app", "--option"}, true},
+ {[]string{"app", "--option=true"}, true},
+ {[]string{"app", "--option=false"}, false},
+ }
+
+ for _, cas := range cases {
+ t.Logf("Testing %#v", cas.args)
+
+ app := App("app", "")
+ app.ErrorHandling = flag.ContinueOnError
+ opt := app.BoolOpt("o option", false, "")
+
+ ex := false
+ app.Action = func() {
+ ex = true
+ require.Equal(t, cas.expectedOptValue, *opt)
+ }
+ err := app.Run(cas.args)
+
+ require.NoError(t, err)
+ require.True(t, ex, "Exec wasn't called")
+ }
+}
+
+func TestAppWithStringOption(t *testing.T) {
+
+ cases := []struct {
+ args []string
+ expectedOptValue string
+ }{
+ {[]string{"app"}, "default"},
+ {[]string{"app", "-o", "user"}, "user"},
+ {[]string{"app", "-o=user"}, "user"},
+
+ {[]string{"app", "--option", "user"}, "user"},
+ {[]string{"app", "--option=user"}, "user"},
+ }
+
+ for _, cas := range cases {
+ t.Logf("Testing %#v", cas.args)
+
+ app := App("app", "")
+ app.ErrorHandling = flag.ContinueOnError
+ opt := app.StringOpt("o option", "default", "")
+
+ ex := false
+ app.Action = func() {
+ ex = true
+ require.Equal(t, cas.expectedOptValue, *opt)
+ }
+ err := app.Run(cas.args)
+
+ require.NoError(t, err)
+ require.True(t, ex, "Exec wasn't called")
+ }
+}
+
+func TestAppWithIntOption(t *testing.T) {
+
+ cases := []struct {
+ args []string
+ expectedOptValue int
+ }{
+ {[]string{"app"}, 3},
+ {[]string{"app", "-o", "16"}, 16},
+ {[]string{"app", "-o=16"}, 16},
+
+ {[]string{"app", "--option", "16"}, 16},
+ {[]string{"app", "--option=16"}, 16},
+ }
+
+ for _, cas := range cases {
+ t.Logf("Testing %#v", cas.args)
+
+ app := App("app", "")
+ app.ErrorHandling = flag.ContinueOnError
+ opt := app.IntOpt("o option", 3, "")
+
+ ex := false
+ app.Action = func() {
+ ex = true
+ require.Equal(t, cas.expectedOptValue, *opt)
+ }
+ err := app.Run(cas.args)
+
+ require.NoError(t, err)
+ require.True(t, ex, "Exec wasn't called")
+ }
+}
+
+func TestAppWithStringsOption(t *testing.T) {
+
+ cases := []struct {
+ args []string
+ expectedOptValue []string
+ }{
+ {[]string{"app"}, []string{"a", "b"}},
+ {[]string{"app", "-o", "x"}, []string{"x"}},
+ {[]string{"app", "-o", "x", "-o=y"}, []string{"x", "y"}},
+
+ {[]string{"app", "--option", "x"}, []string{"x"}},
+ {[]string{"app", "--option", "x", "--option=y"}, []string{"x", "y"}},
+ }
+
+ for _, cas := range cases {
+ t.Logf("Testing %#v", cas.args)
+
+ app := App("app", "")
+ app.ErrorHandling = flag.ContinueOnError
+ opt := app.StringsOpt("o option", []string{"a", "b"}, "")
+
+ ex := false
+ app.Action = func() {
+ ex = true
+ require.Equal(t, cas.expectedOptValue, *opt)
+ }
+ err := app.Run(cas.args)
+
+ require.NoError(t, err)
+ require.True(t, ex, "Exec wasn't called")
+ }
+}
+
+func TestAppWithIntsOption(t *testing.T) {
+
+ cases := []struct {
+ args []string
+ expectedOptValue []int
+ }{
+ {[]string{"app"}, []int{1, 2}},
+ {[]string{"app", "-o", "10"}, []int{10}},
+ {[]string{"app", "-o", "10", "-o=11"}, []int{10, 11}},
+
+ {[]string{"app", "--option", "10"}, []int{10}},
+ {[]string{"app", "--option", "10", "--option=11"}, []int{10, 11}},
+ }
+
+ for _, cas := range cases {
+ t.Logf("Testing %#v", cas.args)
+
+ app := App("app", "")
+ app.ErrorHandling = flag.ContinueOnError
+ opt := app.IntsOpt("o option", []int{1, 2}, "")
+
+ ex := false
+ app.Action = func() {
+ ex = true
+ require.Equal(t, cas.expectedOptValue, *opt)
+ }
+ err := app.Run(cas.args)
+
+ require.NoError(t, err)
+ require.True(t, ex, "Exec wasn't called")
+ }
+}
+
+func TestAppWithBoolArg(t *testing.T) {
+
+ cases := []struct {
+ args []string
+ expectedOptValue bool
+ }{
+ {[]string{"app"}, false},
+ {[]string{"app", "true"}, true},
+ {[]string{"app", "false"}, false},
+ }
+
+ for _, cas := range cases {
+ t.Logf("Testing %#v", cas.args)
+
+ app := App("app", "")
+ app.Spec = "[ARG]"
+ app.ErrorHandling = flag.ContinueOnError
+ opt := app.BoolArg("ARG", false, "")
+
+ ex := false
+ app.Action = func() {
+ ex = true
+ require.Equal(t, cas.expectedOptValue, *opt)
+ }
+ err := app.Run(cas.args)
+
+ require.NoError(t, err)
+ require.True(t, ex, "Exec wasn't called")
+ }
+}
+
+func TestAppWithStringArg(t *testing.T) {
+
+ cases := []struct {
+ args []string
+ expectedOptValue string
+ }{
+ {[]string{"app"}, "default"},
+ {[]string{"app", "user"}, "user"},
+ }
+
+ for _, cas := range cases {
+ t.Logf("Testing %#v", cas.args)
+
+ app := App("app", "")
+ app.Spec = "[ARG]"
+ app.ErrorHandling = flag.ContinueOnError
+ opt := app.StringArg("ARG", "default", "")
+
+ ex := false
+ app.Action = func() {
+ ex = true
+ require.Equal(t, cas.expectedOptValue, *opt)
+ }
+ err := app.Run(cas.args)
+
+ require.NoError(t, err)
+ require.True(t, ex, "Exec wasn't called")
+ }
+}
+
+func TestAppWithIntArg(t *testing.T) {
+
+ cases := []struct {
+ args []string
+ expectedOptValue int
+ }{
+ {[]string{"app"}, 3},
+ {[]string{"app", "16"}, 16},
+ }
+
+ for _, cas := range cases {
+ t.Logf("Testing %#v", cas.args)
+
+ app := App("app", "")
+ app.Spec = "[ARG]"
+ app.ErrorHandling = flag.ContinueOnError
+ opt := app.IntArg("ARG", 3, "")
+
+ ex := false
+ app.Action = func() {
+ ex = true
+ require.Equal(t, cas.expectedOptValue, *opt)
+ }
+ err := app.Run(cas.args)
+
+ require.NoError(t, err)
+ require.True(t, ex, "Exec wasn't called")
+ }
+}
+
+func TestAppWithStringsArg(t *testing.T) {
+
+ cases := []struct {
+ args []string
+ expectedOptValue []string
+ }{
+ {[]string{"app"}, []string{"a", "b"}},
+ {[]string{"app", "x"}, []string{"x"}},
+ {[]string{"app", "x", "y"}, []string{"x", "y"}},
+ }
+
+ for _, cas := range cases {
+ t.Logf("Testing %#v", cas.args)
+
+ app := App("app", "")
+ app.Spec = "[ARG...]"
+ app.ErrorHandling = flag.ContinueOnError
+ opt := app.StringsArg("ARG", []string{"a", "b"}, "")
+
+ ex := false
+ app.Action = func() {
+ ex = true
+ require.Equal(t, cas.expectedOptValue, *opt)
+ }
+ err := app.Run(cas.args)
+
+ require.NoError(t, err)
+ require.True(t, ex, "Exec wasn't called")
+ }
+}
+
+func TestAppWithIntsArg(t *testing.T) {
+
+ cases := []struct {
+ args []string
+ expectedOptValue []int
+ }{
+ {[]string{"app"}, []int{1, 2}},
+ {[]string{"app", "10"}, []int{10}},
+ {[]string{"app", "10", "11"}, []int{10, 11}},
+ }
+
+ for _, cas := range cases {
+ t.Logf("Testing %#v", cas.args)
+
+ app := App("app", "")
+ app.Spec = "[ARG...]"
+ app.ErrorHandling = flag.ContinueOnError
+ opt := app.IntsArg("ARG", []int{1, 2}, "")
+
+ ex := false
+ app.Action = func() {
+ ex = true
+ require.Equal(t, cas.expectedOptValue, *opt)
+ }
+ err := app.Run(cas.args)
+
+ require.NoError(t, err)
+ require.True(t, ex, "Exec wasn't called")
+ }
+}
+
 func testHelpAndVersionWithOptionsEnd(flag string, t *testing.T) {
  t.Logf("Testing help/version with --: flag=%q", flag)
  defer suppressOutput()()

diff --git a/fsm.go b/fsm.go
@@ -176,8 +176,7 @@ func (s *state) parse(args []string) error {
  }
 
  for opt, vs := range pc.opts {
- multiValued, ok := opt.value.(multiValued)
- if ok && opt.valueSetFromEnv {
+ if multiValued, ok := opt.value.(multiValued); ok {
  multiValued.Clear()
  opt.valueSetFromEnv = false
  }
@@ -193,8 +192,7 @@ func (s *state) parse(args []string) error {
  }
 
  for arg, vs := range pc.args {
- multiValued, ok := arg.value.(multiValued)
- if ok && arg.valueSetFromEnv {
+ if multiValued, ok := arg.value.(multiValued); ok {
  multiValued.Clear()
  arg.valueSetFromEnv = false
  }