From 50c71ed517bd28c3ead9b70ad8f13e668957f7b6 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Sat, 27 Mar 2021 13:07:49 -0400 Subject: [PATCH 1/3] Remove reflect calls for doc generation --- flag.go | 58 ++++++++++++++----------------------------- flag_bool.go | 13 ++++++++++ flag_duration.go | 13 ++++++++++ flag_float64.go | 15 ++++++++++- flag_float64_slice.go | 15 ++++++++++- flag_generic.go | 13 ++++++++++ flag_int.go | 13 ++++++++++ flag_int64.go | 13 ++++++++++ flag_int64_slice.go | 15 ++++++++++- flag_int_slice.go | 15 ++++++++++- flag_path.go | 21 +++++++++++++++- flag_string.go | 21 +++++++++++++++- flag_string_slice.go | 15 ++++++++++- flag_timestamp.go | 13 ++++++++++ flag_uint.go | 13 ++++++++++ flag_uint64.go | 13 ++++++++++ 16 files changed, 233 insertions(+), 46 deletions(-) diff --git a/flag.go b/flag.go index 125c6cecce..0d7266214f 100644 --- a/flag.go +++ b/flag.go @@ -117,6 +117,12 @@ type DocGenerationFlag interface { // GetValue returns the flags value as string representation and an empty // string if the flag takes no value at all. GetValue() string + + // GetDefaultText returns the default text for this flag + GetDefaultText() string + + // GetEnvVars returns the env vars for this flag + GetEnvVars() []string } // VisibleFlag is an interface that allows to check if a flag is visible @@ -299,55 +305,29 @@ func formatDefault(format string) string { } func stringifyFlag(f Flag) string { - fv := flagValue(f) - - switch f := f.(type) { - case *IntSliceFlag: - return withEnvHint(flagStringSliceField(f, "EnvVars"), - stringifyIntSliceFlag(f)) - case *Int64SliceFlag: - return withEnvHint(flagStringSliceField(f, "EnvVars"), - stringifyInt64SliceFlag(f)) - case *Float64SliceFlag: - return withEnvHint(flagStringSliceField(f, "EnvVars"), - stringifyFloat64SliceFlag(f)) - case *StringSliceFlag: - return withEnvHint(flagStringSliceField(f, "EnvVars"), - stringifyStringSliceFlag(f)) + // enforce DocGeneration interface on flags to avoid reflection + df, ok := f.(DocGenerationFlag) + if !ok { + return "" } - placeholder, usage := unquoteUsage(fv.FieldByName("Usage").String()) + placeholder, usage := unquoteUsage(df.GetUsage()) + needsPlaceholder := df.TakesValue() - needsPlaceholder := false - defaultValueString := "" - val := fv.FieldByName("Value") - if val.IsValid() { - needsPlaceholder = val.Kind() != reflect.Bool - defaultValueString = fmt.Sprintf(formatDefault("%v"), val.Interface()) - - if val.Kind() == reflect.String && val.String() != "" { - defaultValueString = fmt.Sprintf(formatDefault("%q"), val.String()) - } - } - - helpText := fv.FieldByName("DefaultText") - if helpText.IsValid() && helpText.String() != "" { - needsPlaceholder = val.Kind() != reflect.Bool - defaultValueString = fmt.Sprintf(formatDefault("%s"), helpText.String()) + if needsPlaceholder && placeholder == "" { + placeholder = defaultPlaceholder } - if defaultValueString == formatDefault("") { - defaultValueString = "" - } + defaultValueString := "" - if needsPlaceholder && placeholder == "" { - placeholder = defaultPlaceholder + if s := df.GetDefaultText(); s != "" { + defaultValueString = fmt.Sprintf(formatDefault("%s"), s) } usageWithDefault := strings.TrimSpace(usage + defaultValueString) - return withEnvHint(flagStringSliceField(f, "EnvVars"), - fmt.Sprintf("%s\t%s", prefixedNames(f.Names(), placeholder), usageWithDefault)) + return withEnvHint(df.GetEnvVars(), + fmt.Sprintf("%s\t%s", prefixedNames(df.Names(), placeholder), usageWithDefault)) } func stringifyIntSliceFlag(f *IntSliceFlag) string { diff --git a/flag_bool.go b/flag_bool.go index 8bd582094f..b8e625a1d1 100644 --- a/flag_bool.go +++ b/flag_bool.go @@ -63,6 +63,19 @@ func (f *BoolFlag) IsVisible() bool { return !f.Hidden } +// GetDefaultText returns the default text for this flag +func (f *BoolFlag) GetDefaultText() string { + if f.DefaultText != "" { + return f.DefaultText + } + return fmt.Sprintf("%v", f.Value) +} + +// GetEnvVars returns the env vars for this flag +func (f *BoolFlag) GetEnvVars() []string { + return f.EnvVars +} + // 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 { diff --git a/flag_duration.go b/flag_duration.go index 28f3978051..e8ca15ec02 100644 --- a/flag_duration.go +++ b/flag_duration.go @@ -63,6 +63,19 @@ func (f *DurationFlag) IsVisible() bool { return !f.Hidden } +// GetDefaultText returns the default text for this flag +func (f *DurationFlag) GetDefaultText() string { + if f.DefaultText != "" { + return f.DefaultText + } + return f.GetValue() +} + +// GetEnvVars returns the env vars for this flag +func (f *DurationFlag) GetEnvVars() []string { + return f.EnvVars +} + // 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 { diff --git a/flag_float64.go b/flag_float64.go index 10fb8dfc1c..e08e97f66b 100644 --- a/flag_float64.go +++ b/flag_float64.go @@ -55,7 +55,20 @@ func (f *Float64Flag) GetUsage() string { // GetValue returns the flags value as string representation and an empty // string if the flag takes no value at all. func (f *Float64Flag) GetValue() string { - return fmt.Sprintf("%f", f.Value) + return fmt.Sprintf("%v", f.Value) +} + +// GetDefaultText returns the default text for this flag +func (f *Float64Flag) GetDefaultText() string { + if f.DefaultText != "" { + return f.DefaultText + } + return f.GetValue() +} + +// GetEnvVars returns the env vars for this flag +func (f *Float64Flag) GetEnvVars() []string { + return f.EnvVars } // IsVisible returns true if the flag is not hidden, otherwise false diff --git a/flag_float64_slice.go b/flag_float64_slice.go index f752ad7534..ecef6eecfc 100644 --- a/flag_float64_slice.go +++ b/flag_float64_slice.go @@ -95,7 +95,7 @@ func (f *Float64SliceFlag) IsSet() bool { // String returns a readable representation of this value // (for usage defaults) func (f *Float64SliceFlag) String() string { - return FlagStringer(f) + return withEnvHint(f.GetEnvVars(), stringifyFloat64SliceFlag(f)) } // Names returns the names of the flag @@ -132,6 +132,19 @@ func (f *Float64SliceFlag) IsVisible() bool { return !f.Hidden } +// GetDefaultText returns the default text for this flag +func (f *Float64SliceFlag) GetDefaultText() string { + if f.DefaultText != "" { + return f.DefaultText + } + return f.GetValue() +} + +// GetEnvVars returns the env vars for this flag +func (f *Float64SliceFlag) GetEnvVars() []string { + return f.EnvVars +} + // 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 { diff --git a/flag_generic.go b/flag_generic.go index fdf586d127..d159507147 100644 --- a/flag_generic.go +++ b/flag_generic.go @@ -71,6 +71,19 @@ func (f *GenericFlag) IsVisible() bool { return !f.Hidden } +// GetDefaultText returns the default text for this flag +func (f *GenericFlag) GetDefaultText() string { + if f.DefaultText != "" { + return f.DefaultText + } + return f.GetValue() +} + +// GetEnvVars returns the env vars for this flag +func (f *GenericFlag) GetEnvVars() []string { + return f.EnvVars +} + // 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 { diff --git a/flag_int.go b/flag_int.go index 1ebe3569c1..62c0848688 100644 --- a/flag_int.go +++ b/flag_int.go @@ -63,6 +63,19 @@ func (f *IntFlag) IsVisible() bool { return !f.Hidden } +// GetDefaultText returns the default text for this flag +func (f *IntFlag) GetDefaultText() string { + if f.DefaultText != "" { + return f.DefaultText + } + return f.GetValue() +} + +// GetEnvVars returns the env vars for this flag +func (f *IntFlag) GetEnvVars() []string { + return f.EnvVars +} + // 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 { diff --git a/flag_int64.go b/flag_int64.go index ecf0e9ef6f..2f0be7aa06 100644 --- a/flag_int64.go +++ b/flag_int64.go @@ -63,6 +63,19 @@ func (f *Int64Flag) IsVisible() bool { return !f.Hidden } +// GetDefaultText returns the default text for this flag +func (f *Int64Flag) GetDefaultText() string { + if f.DefaultText != "" { + return f.DefaultText + } + return f.GetValue() +} + +// GetEnvVars returns the env vars for this flag +func (f *Int64Flag) GetEnvVars() []string { + return f.EnvVars +} + // 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 { diff --git a/flag_int64_slice.go b/flag_int64_slice.go index b4c8bc1496..7995f44ea2 100644 --- a/flag_int64_slice.go +++ b/flag_int64_slice.go @@ -96,7 +96,7 @@ func (f *Int64SliceFlag) IsSet() bool { // String returns a readable representation of this value // (for usage defaults) func (f *Int64SliceFlag) String() string { - return FlagStringer(f) + return withEnvHint(f.GetEnvVars(), stringifyInt64SliceFlag(f)) } // Names returns the names of the flag @@ -133,6 +133,19 @@ func (f *Int64SliceFlag) IsVisible() bool { return !f.Hidden } +// GetDefaultText returns the default text for this flag +func (f *Int64SliceFlag) GetDefaultText() string { + if f.DefaultText != "" { + return f.DefaultText + } + return f.GetValue() +} + +// GetEnvVars returns the env vars for this flag +func (f *Int64SliceFlag) GetEnvVars() []string { + return f.EnvVars +} + // 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 { diff --git a/flag_int_slice.go b/flag_int_slice.go index d4889b382c..8f5f1d57c2 100644 --- a/flag_int_slice.go +++ b/flag_int_slice.go @@ -107,7 +107,7 @@ func (f *IntSliceFlag) IsSet() bool { // String returns a readable representation of this value // (for usage defaults) func (f *IntSliceFlag) String() string { - return FlagStringer(f) + return withEnvHint(f.GetEnvVars(), stringifyIntSliceFlag(f)) } // Names returns the names of the flag @@ -144,6 +144,19 @@ func (f *IntSliceFlag) IsVisible() bool { return !f.Hidden } +// GetDefaultText returns the default text for this flag +func (f *IntSliceFlag) GetDefaultText() string { + if f.DefaultText != "" { + return f.DefaultText + } + return f.GetValue() +} + +// GetEnvVars returns the env vars for this flag +func (f *IntSliceFlag) GetEnvVars() []string { + return f.EnvVars +} + // 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 { diff --git a/flag_path.go b/flag_path.go index b3aa9191d3..4010e84c62 100644 --- a/flag_path.go +++ b/flag_path.go @@ -1,6 +1,9 @@ package cli -import "flag" +import ( + "flag" + "fmt" +) type PathFlag struct { Name string @@ -59,6 +62,22 @@ func (f *PathFlag) IsVisible() bool { return !f.Hidden } +// GetDefaultText returns the default text for this flag +func (f *PathFlag) GetDefaultText() string { + if f.DefaultText != "" { + return f.DefaultText + } + if f.Value == "" { + return f.Value + } + return fmt.Sprintf("%q", f.Value) +} + +// GetEnvVars returns the env vars for this flag +func (f *PathFlag) GetEnvVars() []string { + return f.EnvVars +} + // 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 { diff --git a/flag_string.go b/flag_string.go index e7bd412299..7e81499a8b 100644 --- a/flag_string.go +++ b/flag_string.go @@ -1,6 +1,9 @@ package cli -import "flag" +import ( + "flag" + "fmt" +) // StringFlag is a flag with type string type StringFlag struct { @@ -60,6 +63,22 @@ func (f *StringFlag) IsVisible() bool { return !f.Hidden } +// GetDefaultText returns the default text for this flag +func (f *StringFlag) GetDefaultText() string { + if f.DefaultText != "" { + return f.DefaultText + } + if f.Value == "" { + return f.Value + } + return fmt.Sprintf("%q", f.Value) +} + +// GetEnvVars returns the env vars for this flag +func (f *StringFlag) GetEnvVars() []string { + return f.EnvVars +} + // 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 { diff --git a/flag_string_slice.go b/flag_string_slice.go index 5269643105..166424775f 100644 --- a/flag_string_slice.go +++ b/flag_string_slice.go @@ -92,7 +92,7 @@ func (f *StringSliceFlag) IsSet() bool { // String returns a readable representation of this value // (for usage defaults) func (f *StringSliceFlag) String() string { - return FlagStringer(f) + return withEnvHint(f.GetEnvVars(), stringifyStringSliceFlag(f)) } // Names returns the names of the flag @@ -129,6 +129,19 @@ func (f *StringSliceFlag) IsVisible() bool { return !f.Hidden } +// GetDefaultText returns the default text for this flag +func (f *StringSliceFlag) GetDefaultText() string { + if f.DefaultText != "" { + return f.DefaultText + } + return f.GetValue() +} + +// GetEnvVars returns the env vars for this flag +func (f *StringSliceFlag) GetEnvVars() []string { + return f.EnvVars +} + // Apply populates the flag given the flag set and environment func (f *StringSliceFlag) Apply(set *flag.FlagSet) error { diff --git a/flag_timestamp.go b/flag_timestamp.go index 7458a79b6a..ed06418284 100644 --- a/flag_timestamp.go +++ b/flag_timestamp.go @@ -119,6 +119,19 @@ func (f *TimestampFlag) IsVisible() bool { return !f.Hidden } +// GetDefaultText returns the default text for this flag +func (f *TimestampFlag) GetDefaultText() string { + if f.DefaultText != "" { + return f.DefaultText + } + return f.GetValue() +} + +// GetEnvVars returns the env vars for this flag +func (f *TimestampFlag) GetEnvVars() []string { + return f.EnvVars +} + // Apply populates the flag given the flag set and environment func (f *TimestampFlag) Apply(set *flag.FlagSet) error { if f.Layout == "" { diff --git a/flag_uint.go b/flag_uint.go index 23a70a7f90..dd10e1c01d 100644 --- a/flag_uint.go +++ b/flag_uint.go @@ -88,6 +88,19 @@ func (f *UintFlag) GetValue() string { return fmt.Sprintf("%d", f.Value) } +// GetDefaultText returns the default text for this flag +func (f *UintFlag) GetDefaultText() string { + if f.DefaultText != "" { + return f.DefaultText + } + return f.GetValue() +} + +// GetEnvVars returns the env vars for this flag +func (f *UintFlag) GetEnvVars() []string { + return f.EnvVars +} + // Uint looks up the value of a local UintFlag, returns // 0 if not found func (c *Context) Uint(name string) uint { diff --git a/flag_uint64.go b/flag_uint64.go index a2df024e18..017db53c1d 100644 --- a/flag_uint64.go +++ b/flag_uint64.go @@ -88,6 +88,19 @@ func (f *Uint64Flag) GetValue() string { return fmt.Sprintf("%d", f.Value) } +// GetDefaultText returns the default text for this flag +func (f *Uint64Flag) GetDefaultText() string { + if f.DefaultText != "" { + return f.DefaultText + } + return f.GetValue() +} + +// GetEnvVars returns the env vars for this flag +func (f *Uint64Flag) GetEnvVars() []string { + return f.EnvVars +} + // Uint64 looks up the value of a local Uint64Flag, returns // 0 if not found func (c *Context) Uint64(name string) uint64 { From 7cd7ff7dd55db47593c1a5e45a4c8b8457851b87 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Sun, 28 Mar 2021 18:34:30 -0400 Subject: [PATCH 2/3] Remove reflect from flag_test --- flag_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/flag_test.go b/flag_test.go index e46270d034..d44214c9d9 100644 --- a/flag_test.go +++ b/flag_test.go @@ -123,8 +123,13 @@ func TestFlagsFromEnv(t *testing.T) { for i, test := range flagTests { defer resetEnv(os.Environ()) os.Clearenv() - envVarSlice := reflect.Indirect(reflect.ValueOf(test.flag)).FieldByName("EnvVars").Slice(0, 1) - _ = os.Setenv(envVarSlice.Index(0).String(), test.input) + + f, ok := test.flag.(DocGenerationFlag) + if !ok { + t.Errorf("flag %v needs to implement DocGenerationFlag to retrieve env vars", test.flag) + } + envVarSlice := f.GetEnvVars() + _ = os.Setenv(envVarSlice[0], test.input) a := App{ Flags: []Flag{test.flag}, From 69366976476213a8da7b774588fe78167b75e15c Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Wed, 28 Apr 2021 21:00:51 -0400 Subject: [PATCH 3/3] Change min binary size --- internal/build/build.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/build/build.go b/internal/build/build.go index d0be855ebc..a7bf0ddec7 100644 --- a/internal/build/build.go +++ b/internal/build/build.go @@ -193,7 +193,7 @@ func checkBinarySizeActionFunc(c *cli.Context) (err error) { cliBuiltFilePath = "./internal/example-cli/built-example" helloSourceFilePath = "./internal/example-hello-world/example-hello-world.go" helloBuiltFilePath = "./internal/example-hello-world/built-example" - desiredMinBinarySize = 1.9 + desiredMinBinarySize = 1.8 desiredMaxBinarySize = 2.1 badNewsEmoji = "🚨" goodNewsEmoji = "✨"