Merge pull request #1528 from dearchap/issue_1206

Fix:(issue_1206) Default value shouldnt depend on env variable or val…
urfave · Oct 15, 2022 · 5ac0710 · 5ac0710
2 parents 39b1245 + e8b5e4a
commit 5ac0710
Show file tree

Hide file tree

Showing 16 changed files with 385 additions and 88 deletions.
diff --git a/app_test.go b/app_test.go
@@ -2625,19 +2625,6 @@ func TestSetupInitializesOnlyNilWriters(t *testing.T) {
 	}
 }
 
-type stringGeneric struct {
-	value string
-}
-
-func (s *stringGeneric) Set(value string) error {
-	s.value = value
-	return nil
-}
-
-func (s *stringGeneric) String() string {
-	return s.value
-}
-
 func TestFlagAction(t *testing.T) {
 	stringFlag := &StringFlag{
 		Name: "f_string",

diff --git a/cmd/urfave-cli-genflags/generated.gotmpl b/cmd/urfave-cli-genflags/generated.gotmpl
@@ -22,6 +22,8 @@ type {{.TypeName}} struct {
   Aliases []string
   EnvVars []string
 
+  defaultValue {{if .ValuePointer}}*{{end}}{{.GoType}}
+
   {{range .StructFields}}
   {{.Name}} {{if .Pointer}}*{{end}}{{.Type}}
   {{end}}

diff --git a/flag_bool.go b/flag_bool.go
@@ -84,7 +84,7 @@ func (f *BoolFlag) GetDefaultText() string {
 	if f.DefaultText != "" {
 		return f.DefaultText
 	}
-	return fmt.Sprintf("%v", f.Value)
+	return fmt.Sprintf("%v", f.defaultValue)
 }
 
 // GetEnvVars returns the env vars for this flag
@@ -103,6 +103,9 @@ func (f *BoolFlag) RunAction(c *Context) error {
 
 // Apply populates the flag given the flag set and environment
 func (f *BoolFlag) Apply(set *flag.FlagSet) error {
+	// set default value so that environment wont be able to overwrite it
+	f.defaultValue = f.Value
+
 	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		if val != "" {
 			valBool, err := strconv.ParseBool(val)

diff --git a/flag_duration.go b/flag_duration.go
@@ -32,7 +32,7 @@ func (f *DurationFlag) GetDefaultText() string {
 	if f.DefaultText != "" {
 		return f.DefaultText
 	}
-	return f.GetValue()
+	return f.defaultValue.String()
 }
 
 // GetEnvVars returns the env vars for this flag
@@ -42,6 +42,9 @@ func (f *DurationFlag) GetEnvVars() []string {
 
 // Apply populates the flag given the flag set and environment
 func (f *DurationFlag) Apply(set *flag.FlagSet) error {
+	// set default value so that environment wont be able to overwrite it
+	f.defaultValue = f.Value
+
 	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		if val != "" {
 			valDuration, err := time.ParseDuration(val)

diff --git a/flag_generic.go b/flag_generic.go
@@ -11,6 +11,19 @@ type Generic interface {
 	String() string
 }
 
+type stringGeneric struct {
+	value string
+}
+
+func (s *stringGeneric) Set(value string) error {
+	s.value = value
+	return nil
+}
+
+func (s *stringGeneric) String() string {
+	return s.value
+}
+
 // TakesValue returns true of the flag takes a value, otherwise false
 func (f *GenericFlag) TakesValue() bool {
 	return true
@@ -40,7 +53,10 @@ func (f *GenericFlag) GetDefaultText() string {
 	if f.DefaultText != "" {
 		return f.DefaultText
 	}
-	return f.GetValue()
+	if f.defaultValue != nil {
+		return f.defaultValue.String()
+	}
+	return ""
 }
 
 // GetEnvVars returns the env vars for this flag
@@ -51,6 +67,11 @@ func (f *GenericFlag) GetEnvVars() []string {
 // Apply takes the flagset and calls Set on the generic flag with the value
 // provided by the user for parsing by the flag
 func (f *GenericFlag) Apply(set *flag.FlagSet) error {
+	// set default value so that environment wont be able to overwrite it
+	if f.Value != nil {
+		f.defaultValue = &stringGeneric{value: f.Value.String()}
+	}
+
 	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		if val != "" {
 			if err := f.Value.Set(val); err != nil {

diff --git a/flag_int.go b/flag_int.go
@@ -32,7 +32,7 @@ func (f *IntFlag) GetDefaultText() string {
 	if f.DefaultText != "" {
 		return f.DefaultText
 	}
-	return f.GetValue()
+	return fmt.Sprintf("%d", f.defaultValue)
 }
 
 // GetEnvVars returns the env vars for this flag
@@ -42,6 +42,9 @@ func (f *IntFlag) GetEnvVars() []string {
 
 // Apply populates the flag given the flag set and environment
 func (f *IntFlag) Apply(set *flag.FlagSet) error {
+	// set default value so that environment wont be able to overwrite it
+	f.defaultValue = f.Value
+
 	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		if val != "" {
 			valInt, err := strconv.ParseInt(val, f.Base, 64)

diff --git a/flag_int64.go b/flag_int64.go
@@ -32,7 +32,7 @@ func (f *Int64Flag) GetDefaultText() string {
 	if f.DefaultText != "" {
 		return f.DefaultText
 	}
-	return f.GetValue()
+	return fmt.Sprintf("%d", f.defaultValue)
 }
 
 // GetEnvVars returns the env vars for this flag
@@ -42,6 +42,9 @@ func (f *Int64Flag) GetEnvVars() []string {
 
 // Apply populates the flag given the flag set and environment
 func (f *Int64Flag) Apply(set *flag.FlagSet) error {
+	// set default value so that environment wont be able to overwrite it
+	f.defaultValue = f.Value
+
 	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		if val != "" {
 			valInt, err := strconv.ParseInt(val, f.Base, 64)

diff --git a/flag_path.go b/flag_path.go
@@ -33,10 +33,10 @@ func (f *PathFlag) GetDefaultText() string {
 	if f.DefaultText != "" {
 		return f.DefaultText
 	}
-	if f.Value == "" {
-		return f.Value
+	if f.defaultValue == "" {
+		return f.defaultValue
 	}
-	return fmt.Sprintf("%q", f.Value)
+	return fmt.Sprintf("%q", f.defaultValue)
 }
 
 // GetEnvVars returns the env vars for this flag
@@ -46,6 +46,9 @@ func (f *PathFlag) GetEnvVars() []string {
 
 // Apply populates the flag given the flag set and environment
 func (f *PathFlag) Apply(set *flag.FlagSet) error {
+	// set default value so that environment wont be able to overwrite it
+	f.defaultValue = f.Value
+
 	if val, _, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		f.Value = val
 		f.HasBeenSet = true

diff --git a/flag_string.go b/flag_string.go
@@ -31,10 +31,10 @@ func (f *StringFlag) GetDefaultText() string {
 	if f.DefaultText != "" {
 		return f.DefaultText
 	}
-	if f.Value == "" {
-		return f.Value
+	if f.defaultValue == "" {
+		return f.defaultValue
 	}
-	return fmt.Sprintf("%q", f.Value)
+	return fmt.Sprintf("%q", f.defaultValue)
 }
 
 // GetEnvVars returns the env vars for this flag
@@ -44,6 +44,9 @@ func (f *StringFlag) GetEnvVars() []string {
 
 // Apply populates the flag given the flag set and environment
 func (f *StringFlag) Apply(set *flag.FlagSet) error {
+	// set default value so that environment wont be able to overwrite it
+	f.defaultValue = f.Value
+
 	if val, _, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		f.Value = val
 		f.HasBeenSet = true