From 500d6b04e6dc2b9882c0e555a80df7523a97e05f Mon Sep 17 00:00:00 2001 From: Mostyn Bramley-Moore Date: Thu, 30 Jul 2020 12:05:11 +0200 Subject: [PATCH 1/2] Report the source of a value when we cannot parse it If you allow a flag to be set from environment variables or files and a parse error occurs from one of them, it is very useful for the error message to mention where the value came from. Without this, it can be difficult to notice an error caused by an unexpected environment variable being set. Implements #1167. --- flag.go | 8 ++++---- flag_bool.go | 4 ++-- flag_duration.go | 4 ++-- flag_float64.go | 4 ++-- flag_float64_slice.go | 4 ++-- flag_generic.go | 4 ++-- flag_int.go | 4 ++-- flag_int64.go | 4 ++-- flag_int64_slice.go | 4 ++-- flag_int_slice.go | 4 ++-- flag_path.go | 3 ++- flag_string.go | 3 ++- flag_string_slice.go | 4 ++-- flag_test.go | 32 ++++++++++++++++---------------- flag_timestamp.go | 4 ++-- flag_uint.go | 4 ++-- flag_uint64.go | 4 ++-- 17 files changed, 50 insertions(+), 48 deletions(-) diff --git a/flag.go b/flag.go index ad97c2d058..a8edc4fc26 100644 --- a/flag.go +++ b/flag.go @@ -372,17 +372,17 @@ func hasFlag(flags []Flag, fl Flag) bool { return false } -func flagFromEnvOrFile(envVars []string, filePath string) (val string, ok bool) { +func flagFromEnvOrFile(envVars []string, filePath string) (val string, ok bool, source string) { for _, envVar := range envVars { envVar = strings.TrimSpace(envVar) if val, ok := syscall.Getenv(envVar); ok { - return val, true + return val, true, fmt.Sprintf("from environment variable %q", envVar) } } for _, fileVar := range strings.Split(filePath, ",") { if data, err := ioutil.ReadFile(fileVar); err == nil { - return string(data), true + return string(data), true, fmt.Sprintf("from file %q", filePath) } } - return "", false + return "", false, "" } diff --git a/flag_bool.go b/flag_bool.go index bc9ea35d08..903fd08492 100644 --- a/flag_bool.go +++ b/flag_bool.go @@ -60,12 +60,12 @@ func (f *BoolFlag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *BoolFlag) Apply(set *flag.FlagSet) error { - if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { if val != "" { valBool, err := strconv.ParseBool(val) if err != nil { - return fmt.Errorf("could not parse %q as bool value for flag %s: %s", val, f.Name, err) + return fmt.Errorf("could not parse %q as bool value %s for flag %s: %s", val, source, f.Name, err) } f.Value = valBool diff --git a/flag_duration.go b/flag_duration.go index 22a2e67201..0717135d8e 100644 --- a/flag_duration.go +++ b/flag_duration.go @@ -60,12 +60,12 @@ func (f *DurationFlag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *DurationFlag) Apply(set *flag.FlagSet) error { - if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { if val != "" { valDuration, err := time.ParseDuration(val) if err != nil { - return fmt.Errorf("could not parse %q as duration value for flag %s: %s", val, f.Name, err) + return fmt.Errorf("could not parse %q as duration value %s for flag %s: %s", val, source, f.Name, err) } f.Value = valDuration diff --git a/flag_float64.go b/flag_float64.go index 91c778c873..a152945752 100644 --- a/flag_float64.go +++ b/flag_float64.go @@ -60,12 +60,12 @@ func (f *Float64Flag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *Float64Flag) Apply(set *flag.FlagSet) error { - if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { if val != "" { valFloat, err := strconv.ParseFloat(val, 10) if err != nil { - return fmt.Errorf("could not parse %q as float64 value for flag %s: %s", val, f.Name, err) + return fmt.Errorf("could not parse %q as float64 value %s for flag %s: %s", val, source, f.Name, err) } f.Value = valFloat diff --git a/flag_float64_slice.go b/flag_float64_slice.go index 706ee6cd4b..dfd68fdc65 100644 --- a/flag_float64_slice.go +++ b/flag_float64_slice.go @@ -119,13 +119,13 @@ func (f *Float64SliceFlag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *Float64SliceFlag) Apply(set *flag.FlagSet) error { - if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { if val != "" { f.Value = &Float64Slice{} for _, s := range strings.Split(val, ",") { if err := f.Value.Set(strings.TrimSpace(s)); err != nil { - return fmt.Errorf("could not parse %q as float64 slice value for flag %s: %s", f.Value, f.Name, err) + return fmt.Errorf("could not parse %q as float64 slice value %s for flag %s: %s", f.Value, source, f.Name, err) } } diff --git a/flag_generic.go b/flag_generic.go index b0c8ff44d2..fab517c430 100644 --- a/flag_generic.go +++ b/flag_generic.go @@ -69,10 +69,10 @@ func (f *GenericFlag) GetValue() 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 { - if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { if val != "" { if err := f.Value.Set(val); err != nil { - return fmt.Errorf("could not parse %q as value for flag %s: %s", val, f.Name, err) + return fmt.Errorf("could not parse %q %s as value for flag %s: %s", val, source, f.Name, err) } f.HasBeenSet = true diff --git a/flag_int.go b/flag_int.go index ac39d4a9e4..7d0a889985 100644 --- a/flag_int.go +++ b/flag_int.go @@ -60,12 +60,12 @@ func (f *IntFlag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *IntFlag) Apply(set *flag.FlagSet) error { - if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { if val != "" { valInt, err := strconv.ParseInt(val, 0, 64) if err != nil { - return fmt.Errorf("could not parse %q as int value for flag %s: %s", val, f.Name, err) + return fmt.Errorf("could not parse %q as int value %s for flag %s: %s", val, source, f.Name, err) } f.Value = int(valInt) diff --git a/flag_int64.go b/flag_int64.go index e09991269b..401d0b435c 100644 --- a/flag_int64.go +++ b/flag_int64.go @@ -60,12 +60,12 @@ func (f *Int64Flag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *Int64Flag) Apply(set *flag.FlagSet) error { - if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { if val != "" { valInt, err := strconv.ParseInt(val, 0, 64) if err != nil { - return fmt.Errorf("could not parse %q as int value for flag %s: %s", val, f.Name, err) + return fmt.Errorf("could not parse %q as int value %s for flag %s: %s", val, source, f.Name, err) } f.Value = valInt diff --git a/flag_int64_slice.go b/flag_int64_slice.go index 6c7fd9376d..15ee9fd060 100644 --- a/flag_int64_slice.go +++ b/flag_int64_slice.go @@ -120,12 +120,12 @@ func (f *Int64SliceFlag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *Int64SliceFlag) Apply(set *flag.FlagSet) error { - if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { f.Value = &Int64Slice{} for _, s := range strings.Split(val, ",") { if err := f.Value.Set(strings.TrimSpace(s)); err != nil { - return fmt.Errorf("could not parse %q as int64 slice value for flag %s: %s", val, f.Name, err) + return fmt.Errorf("could not parse %q as int64 slice value %s for flag %s: %s", val, source, f.Name, err) } } diff --git a/flag_int_slice.go b/flag_int_slice.go index 4e0afc0210..7807e354fe 100644 --- a/flag_int_slice.go +++ b/flag_int_slice.go @@ -131,12 +131,12 @@ func (f *IntSliceFlag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *IntSliceFlag) Apply(set *flag.FlagSet) error { - if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { f.Value = &IntSlice{} for _, s := range strings.Split(val, ",") { if err := f.Value.Set(strings.TrimSpace(s)); err != nil { - return fmt.Errorf("could not parse %q as int slice value for flag %s: %s", val, f.Name, err) + return fmt.Errorf("could not parse %q as int slice value %s for flag %s: %s", val, source, f.Name, err) } } diff --git a/flag_path.go b/flag_path.go index 8070dc4b0b..5d33ed8d99 100644 --- a/flag_path.go +++ b/flag_path.go @@ -56,7 +56,8 @@ func (f *PathFlag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *PathFlag) Apply(set *flag.FlagSet) error { - if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + // TODO: how to report the source of parse errors? + if val, ok, _ := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { f.Value = val f.HasBeenSet = true } diff --git a/flag_string.go b/flag_string.go index 400bb532e7..5fdea3a6a0 100644 --- a/flag_string.go +++ b/flag_string.go @@ -57,7 +57,8 @@ func (f *StringFlag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *StringFlag) Apply(set *flag.FlagSet) error { - if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + // TODO: how to report source? + if val, ok, _ := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { f.Value = val f.HasBeenSet = true } diff --git a/flag_string_slice.go b/flag_string_slice.go index 35497032cb..2fb62aa0f7 100644 --- a/flag_string_slice.go +++ b/flag_string_slice.go @@ -123,7 +123,7 @@ func (f *StringSliceFlag) Apply(set *flag.FlagSet) error { } - if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { if f.Value == nil { f.Value = &StringSlice{} } @@ -134,7 +134,7 @@ func (f *StringSliceFlag) Apply(set *flag.FlagSet) error { for _, s := range strings.Split(val, ",") { if err := destination.Set(strings.TrimSpace(s)); err != nil { - return fmt.Errorf("could not parse %q as string value for flag %s: %s", val, f.Name, err) + return fmt.Errorf("could not parse %q as string value %s for flag %s: %s", val, source, f.Name, err) } } diff --git a/flag_test.go b/flag_test.go index b1fe70a4fd..8600ba439b 100644 --- a/flag_test.go +++ b/flag_test.go @@ -79,30 +79,30 @@ func TestFlagsFromEnv(t *testing.T) { {"", false, &BoolFlag{Name: "debug", EnvVars: []string{"DEBUG"}}, ""}, {"1", true, &BoolFlag{Name: "debug", EnvVars: []string{"DEBUG"}}, ""}, {"false", false, &BoolFlag{Name: "debug", EnvVars: []string{"DEBUG"}}, ""}, - {"foobar", true, &BoolFlag{Name: "debug", EnvVars: []string{"DEBUG"}}, `could not parse "foobar" as bool value for flag debug: .*`}, + {"foobar", true, &BoolFlag{Name: "debug", EnvVars: []string{"DEBUG"}}, `could not parse "foobar" as bool value from environment variable "DEBUG" for flag debug: .*`}, {"1s", 1 * time.Second, &DurationFlag{Name: "time", EnvVars: []string{"TIME"}}, ""}, - {"foobar", false, &DurationFlag{Name: "time", EnvVars: []string{"TIME"}}, `could not parse "foobar" as duration value for flag time: .*`}, + {"foobar", false, &DurationFlag{Name: "time", EnvVars: []string{"TIME"}}, `could not parse "foobar" as duration value from environment variable "TIME" for flag time: .*`}, {"1.2", 1.2, &Float64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""}, {"1", 1.0, &Float64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""}, - {"foobar", 0, &Float64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as float64 value for flag seconds: .*`}, + {"foobar", 0, &Float64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as float64 value from environment variable "SECONDS" for flag seconds: .*`}, {"1", int64(1), &Int64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""}, - {"1.2", 0, &Int64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as int value for flag seconds: .*`}, - {"foobar", 0, &Int64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int value for flag seconds: .*`}, + {"1.2", 0, &Int64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as int value from environment variable "SECONDS" for flag seconds: .*`}, + {"foobar", 0, &Int64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int value from environment variable "SECONDS" for flag seconds: .*`}, {"1", 1, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""}, - {"1.2", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as int value for flag seconds: .*`}, - {"foobar", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int value for flag seconds: .*`}, + {"1.2", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as int value from environment variable "SECONDS" for flag seconds: .*`}, + {"foobar", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int value from environment variable "SECONDS" for flag seconds: .*`}, {"1,2", newSetIntSlice(1, 2), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""}, - {"1.2,2", newSetIntSlice(), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2,2" as int slice value for flag seconds: .*`}, - {"foobar", newSetIntSlice(), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int slice value for flag seconds: .*`}, + {"1.2,2", newSetIntSlice(), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2,2" as int slice value from environment variable "SECONDS" for flag seconds: .*`}, + {"foobar", newSetIntSlice(), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int slice value from environment variable "SECONDS" for flag seconds: .*`}, {"1,2", newSetInt64Slice(1, 2), &Int64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""}, - {"1.2,2", newSetInt64Slice(), &Int64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2,2" as int64 slice value for flag seconds: .*`}, - {"foobar", newSetInt64Slice(), &Int64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int64 slice value for flag seconds: .*`}, + {"1.2,2", newSetInt64Slice(), &Int64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2,2" as int64 slice value from environment variable "SECONDS" for flag seconds: .*`}, + {"foobar", newSetInt64Slice(), &Int64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int64 slice value from environment variable "SECONDS" for flag seconds: .*`}, {"foo", "foo", &StringFlag{Name: "name", EnvVars: []string{"NAME"}}, ""}, {"path", "path", &PathFlag{Name: "path", EnvVars: []string{"PATH"}}, ""}, @@ -110,12 +110,12 @@ func TestFlagsFromEnv(t *testing.T) { {"foo,bar", newSetStringSlice("foo", "bar"), &StringSliceFlag{Name: "names", EnvVars: []string{"NAMES"}}, ""}, {"1", uint(1), &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""}, - {"1.2", 0, &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as uint value for flag seconds: .*`}, - {"foobar", 0, &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as uint value for flag seconds: .*`}, + {"1.2", 0, &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as uint value from environment variable "SECONDS" for flag seconds: .*`}, + {"foobar", 0, &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as uint value from environment variable "SECONDS" for flag seconds: .*`}, {"1", uint64(1), &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""}, - {"1.2", 0, &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as uint64 value for flag seconds: .*`}, - {"foobar", 0, &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as uint64 value for flag seconds: .*`}, + {"1.2", 0, &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as uint64 value from environment variable "SECONDS" for flag seconds: .*`}, + {"foobar", 0, &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as uint64 value from environment variable "SECONDS" for flag seconds: .*`}, {"foo,bar", &Parser{"foo", "bar"}, &GenericFlag{Name: "names", Value: &Parser{}, EnvVars: []string{"NAMES"}}, ""}, } @@ -1758,7 +1758,7 @@ func TestFlagFromFile(t *testing.T) { } for _, filePathTest := range filePathTests { - got, _ := flagFromEnvOrFile(filePathTest.name, filePathTest.path) + got, _, _ := flagFromEnvOrFile(filePathTest.name, filePathTest.path) if want := filePathTest.expected; got != want { t.Errorf("Did not expect %v - Want %v", got, want) } diff --git a/flag_timestamp.go b/flag_timestamp.go index 0382a6b9dc..898083513b 100644 --- a/flag_timestamp.go +++ b/flag_timestamp.go @@ -123,9 +123,9 @@ func (f *TimestampFlag) Apply(set *flag.FlagSet) error { } f.Value.SetLayout(f.Layout) - if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { if err := f.Value.Set(val); err != nil { - return fmt.Errorf("could not parse %q as timestamp value for flag %s: %s", val, f.Name, err) + return fmt.Errorf("could not parse %q as timestamp value %s for flag %s: %s", val, source, f.Name, err) } f.HasBeenSet = true } diff --git a/flag_uint.go b/flag_uint.go index 2e5e76b0ea..127f90b50d 100644 --- a/flag_uint.go +++ b/flag_uint.go @@ -54,11 +54,11 @@ func (f *UintFlag) GetUsage() string { // Apply populates the flag given the flag set and environment func (f *UintFlag) Apply(set *flag.FlagSet) error { - if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { if val != "" { valInt, err := strconv.ParseUint(val, 0, 64) if err != nil { - return fmt.Errorf("could not parse %q as uint value for flag %s: %s", val, f.Name, err) + return fmt.Errorf("could not parse %q as uint value %s for flag %s: %s", val, source, f.Name, err) } f.Value = uint(valInt) diff --git a/flag_uint64.go b/flag_uint64.go index 8fc3289d82..162d44798f 100644 --- a/flag_uint64.go +++ b/flag_uint64.go @@ -54,11 +54,11 @@ func (f *Uint64Flag) GetUsage() string { // Apply populates the flag given the flag set and environment func (f *Uint64Flag) Apply(set *flag.FlagSet) error { - if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { if val != "" { valInt, err := strconv.ParseUint(val, 0, 64) if err != nil { - return fmt.Errorf("could not parse %q as uint64 value for flag %s: %s", val, f.Name, err) + return fmt.Errorf("could not parse %q as uint64 value %s for flag %s: %s", val, source, f.Name, err) } f.Value = valInt From cdc1f6e07c17b57ef6e907bc505fddcae87abebe Mon Sep 17 00:00:00 2001 From: Mostyn Bramley-Moore Date: Sat, 7 Nov 2020 14:05:37 +0100 Subject: [PATCH 2/2] fixup! Report the source of a value when we cannot parse it move bool to the end of the return arguments remove "from " prefix in the source/fromWhere description remove TODO notes from functions that don't currently perform error checking --- flag.go | 13 ++++++++----- flag_bool.go | 4 ++-- flag_duration.go | 4 ++-- flag_float64.go | 4 ++-- flag_float64_slice.go | 4 ++-- flag_generic.go | 4 ++-- flag_int.go | 4 ++-- flag_int64.go | 4 ++-- flag_int64_slice.go | 4 ++-- flag_int_slice.go | 4 ++-- flag_path.go | 3 +-- flag_string.go | 3 +-- flag_string_slice.go | 4 ++-- flag_timestamp.go | 4 ++-- flag_uint.go | 4 ++-- flag_uint64.go | 4 ++-- 16 files changed, 36 insertions(+), 35 deletions(-) diff --git a/flag.go b/flag.go index a8edc4fc26..35ec2172fb 100644 --- a/flag.go +++ b/flag.go @@ -372,17 +372,20 @@ func hasFlag(flags []Flag, fl Flag) bool { return false } -func flagFromEnvOrFile(envVars []string, filePath string) (val string, ok bool, source string) { +// Return the first value from a list of environment variables and files +// (which may or may not exist), a description of where the value was found, +// and a boolean which is true if a value was found. +func flagFromEnvOrFile(envVars []string, filePath string) (value string, fromWhere string, found bool) { for _, envVar := range envVars { envVar = strings.TrimSpace(envVar) - if val, ok := syscall.Getenv(envVar); ok { - return val, true, fmt.Sprintf("from environment variable %q", envVar) + if value, found := syscall.Getenv(envVar); found { + return value, fmt.Sprintf("environment variable %q", envVar), true } } for _, fileVar := range strings.Split(filePath, ",") { if data, err := ioutil.ReadFile(fileVar); err == nil { - return string(data), true, fmt.Sprintf("from file %q", filePath) + return string(data), fmt.Sprintf("file %q", filePath), true } } - return "", false, "" + return "", "", false } diff --git a/flag_bool.go b/flag_bool.go index 903fd08492..9462e0670e 100644 --- a/flag_bool.go +++ b/flag_bool.go @@ -60,12 +60,12 @@ func (f *BoolFlag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *BoolFlag) Apply(set *flag.FlagSet) error { - if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found { if val != "" { valBool, err := strconv.ParseBool(val) if err != nil { - return fmt.Errorf("could not parse %q as bool value %s for flag %s: %s", val, source, f.Name, err) + return fmt.Errorf("could not parse %q as bool value from %s for flag %s: %s", val, source, f.Name, err) } f.Value = valBool diff --git a/flag_duration.go b/flag_duration.go index 0717135d8e..040b5d602b 100644 --- a/flag_duration.go +++ b/flag_duration.go @@ -60,12 +60,12 @@ func (f *DurationFlag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *DurationFlag) Apply(set *flag.FlagSet) error { - if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found { if val != "" { valDuration, err := time.ParseDuration(val) if err != nil { - return fmt.Errorf("could not parse %q as duration value %s for flag %s: %s", val, source, f.Name, err) + return fmt.Errorf("could not parse %q as duration value from %s for flag %s: %s", val, source, f.Name, err) } f.Value = valDuration diff --git a/flag_float64.go b/flag_float64.go index a152945752..14df60d4c3 100644 --- a/flag_float64.go +++ b/flag_float64.go @@ -60,12 +60,12 @@ func (f *Float64Flag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *Float64Flag) Apply(set *flag.FlagSet) error { - if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found { if val != "" { valFloat, err := strconv.ParseFloat(val, 10) if err != nil { - return fmt.Errorf("could not parse %q as float64 value %s for flag %s: %s", val, source, f.Name, err) + return fmt.Errorf("could not parse %q as float64 value from %s for flag %s: %s", val, source, f.Name, err) } f.Value = valFloat diff --git a/flag_float64_slice.go b/flag_float64_slice.go index dfd68fdc65..03822292b6 100644 --- a/flag_float64_slice.go +++ b/flag_float64_slice.go @@ -119,13 +119,13 @@ func (f *Float64SliceFlag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *Float64SliceFlag) Apply(set *flag.FlagSet) error { - if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found { if val != "" { f.Value = &Float64Slice{} for _, s := range strings.Split(val, ",") { if err := f.Value.Set(strings.TrimSpace(s)); err != nil { - return fmt.Errorf("could not parse %q as float64 slice value %s for flag %s: %s", f.Value, source, f.Name, err) + return fmt.Errorf("could not parse %q as float64 slice value from %s for flag %s: %s", f.Value, source, f.Name, err) } } diff --git a/flag_generic.go b/flag_generic.go index fab517c430..b92c4de584 100644 --- a/flag_generic.go +++ b/flag_generic.go @@ -69,10 +69,10 @@ func (f *GenericFlag) GetValue() 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 { - if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found { if val != "" { if err := f.Value.Set(val); err != nil { - return fmt.Errorf("could not parse %q %s as value for flag %s: %s", val, source, f.Name, err) + return fmt.Errorf("could not parse %q from %s as value for flag %s: %s", val, source, f.Name, err) } f.HasBeenSet = true diff --git a/flag_int.go b/flag_int.go index 7d0a889985..8e0ee599ad 100644 --- a/flag_int.go +++ b/flag_int.go @@ -60,12 +60,12 @@ func (f *IntFlag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *IntFlag) Apply(set *flag.FlagSet) error { - if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found { if val != "" { valInt, err := strconv.ParseInt(val, 0, 64) if err != nil { - return fmt.Errorf("could not parse %q as int value %s for flag %s: %s", val, source, f.Name, err) + return fmt.Errorf("could not parse %q as int value from %s for flag %s: %s", val, source, f.Name, err) } f.Value = int(valInt) diff --git a/flag_int64.go b/flag_int64.go index 401d0b435c..0dda82e9b5 100644 --- a/flag_int64.go +++ b/flag_int64.go @@ -60,12 +60,12 @@ func (f *Int64Flag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *Int64Flag) Apply(set *flag.FlagSet) error { - if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found { if val != "" { valInt, err := strconv.ParseInt(val, 0, 64) if err != nil { - return fmt.Errorf("could not parse %q as int value %s for flag %s: %s", val, source, f.Name, err) + return fmt.Errorf("could not parse %q as int value from %s for flag %s: %s", val, source, f.Name, err) } f.Value = valInt diff --git a/flag_int64_slice.go b/flag_int64_slice.go index 15ee9fd060..cfb29eb298 100644 --- a/flag_int64_slice.go +++ b/flag_int64_slice.go @@ -120,12 +120,12 @@ func (f *Int64SliceFlag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *Int64SliceFlag) Apply(set *flag.FlagSet) error { - if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found { f.Value = &Int64Slice{} for _, s := range strings.Split(val, ",") { if err := f.Value.Set(strings.TrimSpace(s)); err != nil { - return fmt.Errorf("could not parse %q as int64 slice value %s for flag %s: %s", val, source, f.Name, err) + return fmt.Errorf("could not parse %q as int64 slice value from %s for flag %s: %s", val, source, f.Name, err) } } diff --git a/flag_int_slice.go b/flag_int_slice.go index 7807e354fe..f379039811 100644 --- a/flag_int_slice.go +++ b/flag_int_slice.go @@ -131,12 +131,12 @@ func (f *IntSliceFlag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *IntSliceFlag) Apply(set *flag.FlagSet) error { - if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found { f.Value = &IntSlice{} for _, s := range strings.Split(val, ",") { if err := f.Value.Set(strings.TrimSpace(s)); err != nil { - return fmt.Errorf("could not parse %q as int slice value %s for flag %s: %s", val, source, f.Name, err) + return fmt.Errorf("could not parse %q as int slice value from %s for flag %s: %s", val, source, f.Name, err) } } diff --git a/flag_path.go b/flag_path.go index 5d33ed8d99..bc8d5a2821 100644 --- a/flag_path.go +++ b/flag_path.go @@ -56,8 +56,7 @@ func (f *PathFlag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *PathFlag) Apply(set *flag.FlagSet) error { - // TODO: how to report the source of parse errors? - if val, ok, _ := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + 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 index 5fdea3a6a0..5d79ce8e38 100644 --- a/flag_string.go +++ b/flag_string.go @@ -57,8 +57,7 @@ func (f *StringFlag) GetValue() string { // Apply populates the flag given the flag set and environment func (f *StringFlag) Apply(set *flag.FlagSet) error { - // TODO: how to report source? - if val, ok, _ := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, _, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found { f.Value = val f.HasBeenSet = true } diff --git a/flag_string_slice.go b/flag_string_slice.go index 2fb62aa0f7..e081c4f5f0 100644 --- a/flag_string_slice.go +++ b/flag_string_slice.go @@ -123,7 +123,7 @@ func (f *StringSliceFlag) Apply(set *flag.FlagSet) error { } - if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found { if f.Value == nil { f.Value = &StringSlice{} } @@ -134,7 +134,7 @@ func (f *StringSliceFlag) Apply(set *flag.FlagSet) error { for _, s := range strings.Split(val, ",") { if err := destination.Set(strings.TrimSpace(s)); err != nil { - return fmt.Errorf("could not parse %q as string value %s for flag %s: %s", val, source, f.Name, err) + return fmt.Errorf("could not parse %q as string value from %s for flag %s: %s", val, source, f.Name, err) } } diff --git a/flag_timestamp.go b/flag_timestamp.go index 898083513b..fb174eee97 100644 --- a/flag_timestamp.go +++ b/flag_timestamp.go @@ -123,9 +123,9 @@ func (f *TimestampFlag) Apply(set *flag.FlagSet) error { } f.Value.SetLayout(f.Layout) - if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found { if err := f.Value.Set(val); err != nil { - return fmt.Errorf("could not parse %q as timestamp value %s for flag %s: %s", val, source, f.Name, err) + return fmt.Errorf("could not parse %q as timestamp value from %s for flag %s: %s", val, source, f.Name, err) } f.HasBeenSet = true } diff --git a/flag_uint.go b/flag_uint.go index 127f90b50d..f64cbe4119 100644 --- a/flag_uint.go +++ b/flag_uint.go @@ -54,11 +54,11 @@ func (f *UintFlag) GetUsage() string { // Apply populates the flag given the flag set and environment func (f *UintFlag) Apply(set *flag.FlagSet) error { - if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found { if val != "" { valInt, err := strconv.ParseUint(val, 0, 64) if err != nil { - return fmt.Errorf("could not parse %q as uint value %s for flag %s: %s", val, source, f.Name, err) + return fmt.Errorf("could not parse %q as uint value from %s for flag %s: %s", val, source, f.Name, err) } f.Value = uint(valInt) diff --git a/flag_uint64.go b/flag_uint64.go index 162d44798f..a6bf63d123 100644 --- a/flag_uint64.go +++ b/flag_uint64.go @@ -54,11 +54,11 @@ func (f *Uint64Flag) GetUsage() string { // Apply populates the flag given the flag set and environment func (f *Uint64Flag) Apply(set *flag.FlagSet) error { - if val, ok, source := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok { + if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found { if val != "" { valInt, err := strconv.ParseUint(val, 0, 64) if err != nil { - return fmt.Errorf("could not parse %q as uint64 value %s for flag %s: %s", val, source, f.Name, err) + return fmt.Errorf("could not parse %q as uint64 value from %s for flag %s: %s", val, source, f.Name, err) } f.Value = valInt