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
 		}