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 ad97c2d..a8edc4f 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 bc9ea35..903fd08 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 22a2e67..0717135 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 91c778c..a152945 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 706ee6c..dfd68fd 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 b0c8ff4..fab517c 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 ac39d4a..7d0a889 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 e099912..401d0b4 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 6c7fd93..15ee9fd 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 4e0afc0..7807e35 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 8070dc4..5d33ed8 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 400bb53..5fdea3a 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 3549703..2fb62aa 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 b1fe70a..8600ba4 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 0382a6b..8980835 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 2e5e76b..127f90b 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 8fc3289..162d447 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 a8edc4f..35ec217 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 903fd08..9462e06 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 0717135..040b5d6 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 a152945..14df60d 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 dfd68fd..0382229 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 fab517c..b92c4de 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 7d0a889..8e0ee59 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 401d0b4..0dda82e 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 15ee9fd..cfb29eb 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 7807e35..f379039 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 5d33ed8..bc8d5a2 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 5fdea3a..5d79ce8 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 2fb62aa..e081c4f 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 8980835..fb174ee 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 127f90b..f64cbe4 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 162d447..a6bf63d 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