From a9d5d68a470daf0253ef50e37c608d6dab7aa2b4 Mon Sep 17 00:00:00 2001
From: Ramana Reddy <90540245+RamanaReddy0M@users.noreply.github.com>
Date: Thu, 2 Nov 2023 15:47:47 +0530
Subject: [PATCH] Fix dynamicvar incorrect parsing on bool values (#151)

* Fix dynamicvar incorrect parsing on bool values

* add tests
---
 dynamic_var.go      |  29 +++++++-----
 dynamic_var_test.go | 107 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+), 12 deletions(-)
 create mode 100644 dynamic_var_test.go

diff --git a/dynamic_var.go b/dynamic_var.go
index ceff9b2..a8c7546 100644
--- a/dynamic_var.go
+++ b/dynamic_var.go
@@ -16,19 +16,24 @@ type dynamicFlag struct {
 
 func (df *dynamicFlag) Set(value string) error {
 	fieldKind := reflect.TypeOf(df.field).Elem().Kind()
-	var isBoolValue bool
-	if _, err := strconv.ParseBool(value); err == nil {
-		isBoolValue = true
-	}
-	if fieldKind == reflect.Bool && isBoolValue {
-		boolField := df.field.(*bool)
-		*boolField = true
-		return nil
+	var (
+		optionWithoutValue bool
+		pv                 bool
+	)
+	if value == "t" || value == "T" || value == "true" || value == "TRUE" {
+		pv = true
+		optionWithoutValue = true
+	} else if value == "f" || value == "F" || value == "false" || value == "FALSE" {
+		pv = false
 	}
+
 	switch fieldKind {
+	case reflect.Bool:
+		boolField := df.field.(*bool)
+		*boolField = pv
 	case reflect.Int:
 		intField := df.field.(*int)
-		if isBoolValue {
+		if optionWithoutValue {
 			*intField = df.defaultValue.(int)
 			return nil
 		}
@@ -39,7 +44,7 @@ func (df *dynamicFlag) Set(value string) error {
 		*intField = newValue
 	case reflect.Float64:
 		floatField := df.field.(*float64)
-		if isBoolValue {
+		if optionWithoutValue {
 			*floatField = df.defaultValue.(float64)
 			return nil
 		}
@@ -50,14 +55,14 @@ func (df *dynamicFlag) Set(value string) error {
 		*floatField = newValue
 	case reflect.String:
 		stringField := df.field.(*string)
-		if isBoolValue {
+		if optionWithoutValue {
 			*stringField = df.defaultValue.(string)
 			return nil
 		}
 		*stringField = value
 	case reflect.Slice:
 		sliceField := df.field.(*[]string)
-		if isBoolValue {
+		if optionWithoutValue {
 			*sliceField = df.defaultValue.([]string)
 			return nil
 		}
diff --git a/dynamic_var_test.go b/dynamic_var_test.go
new file mode 100644
index 0000000..eed51db
--- /dev/null
+++ b/dynamic_var_test.go
@@ -0,0 +1,107 @@
+package goflags
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestDynamicVar(t *testing.T) {
+	t.Run("with bool as type", func(t *testing.T) {
+		var b bool
+		flagSet := NewFlagSet()
+		flagSet.CreateGroup("Option", "option",
+			flagSet.DynamicVar(&b, "kev", false, "kev with or without value"),
+		)
+		os.Args = []string{
+			os.Args[0],
+			"-kev=false",
+		}
+		err := flagSet.Parse()
+		assert.Nil(t, err)
+		assert.Equal(t, false, b)
+		tearDown(t.Name())
+	})
+
+	t.Run("without value for int as type", func(t *testing.T) {
+		var i int
+		flagSet := NewFlagSet()
+		flagSet.CreateGroup("Option", "option",
+			flagSet.DynamicVarP(&i, "concurrency", "c", 25, "concurrency for the process"),
+		)
+		os.Args = []string{
+			os.Args[0],
+			"-c",
+		}
+		err := flagSet.Parse()
+		assert.Nil(t, err)
+		assert.Equal(t, 25, i)
+		tearDown(t.Name())
+	})
+
+	t.Run("with value for int as type", func(t *testing.T) {
+		var i int
+		flagSet := NewFlagSet()
+		flagSet.CreateGroup("Option", "option",
+			flagSet.DynamicVarP(&i, "concurrency", "c", 25, "concurrency for the process"),
+		)
+		os.Args = []string{
+			os.Args[0],
+			"-c=100",
+		}
+		err := flagSet.Parse()
+		assert.Nil(t, err)
+		assert.Equal(t, 100, i)
+		tearDown(t.Name())
+	})
+
+	t.Run("with value for float64 as type", func(t *testing.T) {
+		var f float64
+		flagSet := NewFlagSet()
+		flagSet.CreateGroup("Option", "option",
+			flagSet.DynamicVarP(&f, "percentage", "p", 0.0, "percentage for the process"),
+		)
+		os.Args = []string{
+			os.Args[0],
+			"-p=100.0",
+		}
+		err := flagSet.Parse()
+		assert.Nil(t, err)
+		assert.Equal(t, 100.0, f)
+		tearDown(t.Name())
+	})
+
+	t.Run("with value for string as type", func(t *testing.T) {
+		var s string
+		flagSet := NewFlagSet()
+		flagSet.CreateGroup("Option", "option",
+			flagSet.DynamicVarP(&s, "name", "n", "", "name of the user"),
+		)
+		os.Args = []string{
+			os.Args[0],
+			"-n=test",
+		}
+		err := flagSet.Parse()
+		assert.Nil(t, err)
+		assert.Equal(t, "test", s)
+		tearDown(t.Name())
+	})
+
+	t.Run("with value for string slice as type", func(t *testing.T) {
+		var s []string
+		flagSet := NewFlagSet()
+		flagSet.CreateGroup("Option", "option",
+			flagSet.DynamicVarP(&s, "name", "n", []string{}, "name of the user"),
+		)
+		os.Args = []string{
+			os.Args[0],
+			"-n=test1,test2",
+		}
+		err := flagSet.Parse()
+		assert.Nil(t, err)
+		assert.Equal(t, []string{"test1", "test2"}, s)
+		tearDown(t.Name())
+	})
+
+}