From 69a8e25f3d7390cf45d81b130200b0a9448d3ca9 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Mon, 2 May 2016 19:42:08 -0400 Subject: [PATCH] Make flag usage rendering more consistent; show default values --- flag.go | 128 ++++++++++++++++++++++++++++++++++----------------- flag_test.go | 57 +++++++++++------------ 2 files changed, 114 insertions(+), 71 deletions(-) diff --git a/flag.go b/flag.go index d00a984..4535b5d 100644 --- a/flag.go +++ b/flag.go @@ -11,6 +11,8 @@ import ( "time" ) +const defaultPlaceholder = "value" + // This flag enables bash-completion for all commands and subcommands var BashCompletionFlag = BoolFlag{ Name: "generate-bash-completion", @@ -82,17 +84,6 @@ func (f GenericFlag) String() string { return FlagStringer(f) } -func (f GenericFlag) FormatValueHelp() string { - if f.Value == nil { - return "" - } - s := f.Value.String() - if len(s) == 0 { - return "" - } - return fmt.Sprintf("\"%s\"", s) -} - // 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) { @@ -179,12 +170,6 @@ func (f StringSliceFlag) GetName() string { return f.Name } -func (f StringSliceFlag) FormatValueHelp() string { - firstName := strings.Trim(strings.Split(f.Name, ",")[0], " ") - pref := prefixFor(firstName) - return fmt.Sprintf("[%s%s option %s%s option]", pref, firstName, pref, firstName) -} - // StringSlice is an opaque type for []int to satisfy flag.Value type IntSlice []int @@ -256,12 +241,6 @@ func (f IntSliceFlag) GetName() string { return f.Name } -func (f IntSliceFlag) FormatValueHelp() string { - firstName := strings.Trim(strings.Split(f.Name, ",")[0], " ") - pref := prefixFor(firstName) - return fmt.Sprintf("[%s%s option %s%s option]", pref, firstName, pref, firstName) -} - // BoolFlag is a switch that defaults to false type BoolFlag struct { Name string @@ -364,14 +343,6 @@ func (f StringFlag) String() string { return FlagStringer(f) } -func (f StringFlag) FormatValueHelp() string { - s := f.Value - if len(s) == 0 { - return "" - } - return fmt.Sprintf("\"%s\"", s) -} - // Apply populates the flag given the flag set and environment func (f StringFlag) Apply(set *flag.FlagSet) { if f.EnvVar != "" { @@ -599,24 +570,99 @@ func withEnvHint(envVar, str string) string { func stringifyFlag(f Flag) string { fv := reflect.ValueOf(f) + + switch f.(type) { + case IntSliceFlag: + return withEnvHint(fv.FieldByName("EnvVar").String(), + stringifyIntSliceFlag(f.(IntSliceFlag))) + case StringSliceFlag: + return withEnvHint(fv.FieldByName("EnvVar").String(), + stringifyStringSliceFlag(f.(StringSliceFlag))) + } + placeholder, usage := unquoteUsage(fv.FieldByName("Usage").String()) + needsPlaceholder := false defaultValueString := "" - if val := fv.FieldByName("Value"); val.IsValid() { - defaultValueString = fmt.Sprintf("%v", val.Interface()) + val := fv.FieldByName("Value") + + valString := "" + if val.IsValid() { + needsPlaceholder = true + if val.Kind() == reflect.String && val.String() != "" { + valString = fmt.Sprintf("%q", val.String()) + } else { + valString = fmt.Sprintf("%v", val.Interface()) + } } - formatFunc := fv.MethodByName("FormatValueHelp") - if formatFunc.IsValid() { - defaultValueString = formatFunc.Call([]reflect.Value{})[0].String() + if valString != "" { + defaultValueString = valString } - if len(defaultValueString) > 0 { - defaultValueString = " " + defaultValueString + if needsPlaceholder && placeholder == "" { + placeholder = defaultPlaceholder } + formattedDefaultValueString := "" + if defaultValueString != "" { + formattedDefaultValueString = fmt.Sprintf(" (default: %v)", defaultValueString) + } + + usageWithDefault := strings.TrimSpace(fmt.Sprintf("%s%s", usage, formattedDefaultValueString)) + return withEnvHint(fv.FieldByName("EnvVar").String(), - fmt.Sprintf("%s%v\t%v", - prefixedNames(fv.FieldByName("Name").String(), placeholder), - defaultValueString, usage)) + fmt.Sprintf("%s\t%s", prefixedNames(fv.FieldByName("Name").String(), placeholder), usageWithDefault)) +} + +func stringifyIntSliceFlag(f IntSliceFlag) string { + defaultVals := []string{} + if f.Value != nil && len(f.Value.Value()) > 0 { + for _, i := range f.Value.Value() { + defaultVals = append(defaultVals, fmt.Sprintf("%d", i)) + } + } + + return stringifySliceFlag(f.Usage, f.Name, defaultVals) +} + +func stringifyStringSliceFlag(f StringSliceFlag) string { + defaultVals := []string{} + if f.Value != nil && len(f.Value.Value()) > 0 { + for _, s := range f.Value.Value() { + if len(s) > 0 { + defaultVals = append(defaultVals, fmt.Sprintf("%q", s)) + } + } + } + + return stringifySliceFlag(f.Usage, f.Name, defaultVals) +} + +func stringifySliceFlag(usage, name string, defaultVals []string) string { + placeholder, usage := unquoteUsage(usage) + if placeholder == "" { + placeholder = defaultPlaceholder + } + + nameParts := []string{} + for _, part := range strings.Split(name, ",") { + nameParts = append(nameParts, strings.TrimSpace(part)) + } + + defaultVal := "" + if len(defaultVals) > 0 { + defaultVal = fmt.Sprintf(" (default: %s)", strings.Join(defaultVals, ", ")) + } + + usageWithDefault := strings.TrimSpace(fmt.Sprintf("%s%s", usage, defaultVal)) + + if len(nameParts) < 2 { + return fmt.Sprintf("%s%s %s\t%s", prefixFor(nameParts[0]), nameParts[0], + placeholder, usageWithDefault) + } + + return fmt.Sprintf("%s%s %s, %s%s %s\t%s", prefixFor(nameParts[0]), nameParts[0], + placeholder, prefixFor(nameParts[1]), nameParts[1], + placeholder, usageWithDefault) } diff --git a/flag_test.go b/flag_test.go index a49cac7..e0df23b 100644 --- a/flag_test.go +++ b/flag_test.go @@ -35,15 +35,15 @@ var stringFlagTests = []struct { value string expected string }{ - {"help", "", "", "--help\t"}, - {"h", "", "", "-h\t"}, - {"h", "", "", "-h\t"}, - {"test", "", "Something", "--test \"Something\"\t"}, + {"foo", "", "", "--foo value\t"}, + {"f", "", "", "-f value\t"}, + {"f", "The total `foo` desired", "all", "-f foo\tThe total foo desired (default: \"all\")"}, + {"test", "", "Something", "--test value\t(default: \"Something\")"}, {"config,c", "Load configuration from `FILE`", "", "--config FILE, -c FILE\tLoad configuration from FILE"}, + {"config,c", "Load configuration from `CONFIG`", "config.json", "--config CONFIG, -c CONFIG\tLoad configuration from CONFIG (default: \"config.json\")"}, } func TestStringFlagHelpOutput(t *testing.T) { - for _, test := range stringFlagTests { flag := StringFlag{Name: test.name, Usage: test.usage, Value: test.value} output := flag.String() @@ -76,30 +76,29 @@ var stringSliceFlagTests = []struct { value *StringSlice expected string }{ - {"help", func() *StringSlice { + {"foo", func() *StringSlice { s := &StringSlice{} s.Set("") return s - }(), "--help [--help option --help option]\t"}, - {"h", func() *StringSlice { + }(), "--foo value\t"}, + {"f", func() *StringSlice { s := &StringSlice{} s.Set("") return s - }(), "-h [-h option -h option]\t"}, - {"h", func() *StringSlice { + }(), "-f value\t"}, + {"f", func() *StringSlice { s := &StringSlice{} - s.Set("") + s.Set("Lipstick") return s - }(), "-h [-h option -h option]\t"}, + }(), "-f value\t(default: \"Lipstick\")"}, {"test", func() *StringSlice { s := &StringSlice{} s.Set("Something") return s - }(), "--test [--test option --test option]\t"}, + }(), "--test value\t(default: \"Something\")"}, } func TestStringSliceFlagHelpOutput(t *testing.T) { - for _, test := range stringSliceFlagTests { flag := StringSliceFlag{Name: test.name, Value: test.value} output := flag.String() @@ -131,14 +130,13 @@ var intFlagTests = []struct { name string expected string }{ - {"help", "--help 0\t"}, - {"h", "-h 0\t"}, + {"hats", "--hats value\t(default: 9)"}, + {"H", "-H value\t(default: 9)"}, } func TestIntFlagHelpOutput(t *testing.T) { - for _, test := range intFlagTests { - flag := IntFlag{Name: test.name} + flag := IntFlag{Name: test.name, Value: 9} output := flag.String() if output != test.expected { @@ -168,8 +166,8 @@ var durationFlagTests = []struct { name string expected string }{ - {"help", "--help 1s\t"}, - {"h", "-h 1s\t"}, + {"hooting", "--hooting value\t(default: 1s)"}, + {"H", "-H value\t(default: 1s)"}, } func TestDurationFlagHelpOutput(t *testing.T) { @@ -205,18 +203,17 @@ var intSliceFlagTests = []struct { value *IntSlice expected string }{ - {"help", &IntSlice{}, "--help [--help option --help option]\t"}, - {"h", &IntSlice{}, "-h [-h option -h option]\t"}, - {"h", &IntSlice{}, "-h [-h option -h option]\t"}, - {"test", func() *IntSlice { + {"heads", &IntSlice{}, "--heads value\t"}, + {"H", &IntSlice{}, "-H value\t"}, + {"H, heads", func() *IntSlice { i := &IntSlice{} i.Set("9") + i.Set("3") return i - }(), "--test [--test option --test option]\t"}, + }(), "-H value, --heads value\t(default: 9, 3)"}, } func TestIntSliceFlagHelpOutput(t *testing.T) { - for _, test := range intSliceFlagTests { flag := IntSliceFlag{Name: test.name, Value: test.value} output := flag.String() @@ -248,8 +245,8 @@ var float64FlagTests = []struct { name string expected string }{ - {"help", "--help 0.1\t"}, - {"h", "-h 0.1\t"}, + {"hooting", "--hooting value\t(default: 0.1)"}, + {"H", "-H value\t(default: 0.1)"}, } func TestFloat64FlagHelpOutput(t *testing.T) { @@ -285,8 +282,8 @@ var genericFlagTests = []struct { value Generic expected string }{ - {"test", &Parser{"abc", "def"}, "--test \"abc,def\"\ttest flag"}, - {"t", &Parser{"abc", "def"}, "-t \"abc,def\"\ttest flag"}, + {"toads", &Parser{"abc", "def"}, "--toads value\ttest flag (default: abc,def)"}, + {"t", &Parser{"abc", "def"}, "-t value\ttest flag (default: abc,def)"}, } func TestGenericFlagHelpOutput(t *testing.T) {