From 35ac968c9e1d4b82f46dbf9b42399f3049a5d8e5 Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Tue, 12 May 2015 17:08:05 -0700 Subject: [PATCH 1/7] upate string slice usage Signed-off-by: Victor Vieux --- flag.go | 2 +- flag_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/flag.go b/flag.go index 2511586..bac044b 100644 --- a/flag.go +++ b/flag.go @@ -124,7 +124,7 @@ type StringSliceFlag struct { func (f StringSliceFlag) String() string { firstName := strings.Trim(strings.Split(f.Name, ",")[0], " ") pref := prefixFor(firstName) - return withEnvHint(f.EnvVar, fmt.Sprintf("%s [%v]\t%v", prefixedNames(f.Name), pref+firstName+" option "+pref+firstName+" option", f.Usage)) + return withEnvHint(f.EnvVar, fmt.Sprintf("%s [%v]\t%v", prefixedNames(f.Name), pref+firstName+" ...", f.Usage)) } func (f StringSliceFlag) Apply(set *flag.FlagSet) { diff --git a/flag_test.go b/flag_test.go index f0f096a..4c7fd5d 100644 --- a/flag_test.go +++ b/flag_test.go @@ -75,22 +75,22 @@ var stringSliceFlagTests = []struct { s := &cli.StringSlice{} s.Set("") return s - }(), "--help [--help option --help option]\t"}, + }(), "--help [--help ...]\t"}, {"h", func() *cli.StringSlice { s := &cli.StringSlice{} s.Set("") return s - }(), "-h [-h option -h option]\t"}, + }(), "-h [-h ...]\t"}, {"h", func() *cli.StringSlice { s := &cli.StringSlice{} s.Set("") return s - }(), "-h [-h option -h option]\t"}, + }(), "-h [-h ...]\t"}, {"test", func() *cli.StringSlice { s := &cli.StringSlice{} s.Set("Something") return s - }(), "--test [--test option --test option]\t"}, + }(), "--test [--test ...]\t"}, } func TestStringSliceFlagHelpOutput(t *testing.T) { From 22773b14c1d7ddda94888c7797405237f547f1db Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Mon, 2 May 2016 13:05:21 -0400 Subject: [PATCH 2/7] Allow for pluggable flag-level help text formatting by defining `cli.DefaultFlagStringFunc` with a default value that uses `withEnvHint`, conditionally running a given flag's `FormatValueHelp` if present. Closes #257 --- CHANGELOG.md | 6 +++++ flag.go | 69 +++++++++++++++++++++++++++++++++++----------------- flag_test.go | 37 +++++++++++++--------------- funcs.go | 4 +++ 4 files changed, 74 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39581e9..efdc548 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,12 @@ **ATTN**: This project uses [semantic versioning](http://semver.org/). ## [Unreleased] +### Added +- Pluggable flag-level help text rendering via `cli.DefaultFlagStringFunc` + +### Changed +- `Float64Flag`, `IntFlag`, and `DurationFlag` default values are no longer +quoted in help text output. ## [1.16.0] - 2016-05-02 ### Added diff --git a/flag.go b/flag.go index 3c5c1b3..d21e018 100644 --- a/flag.go +++ b/flag.go @@ -31,6 +31,8 @@ var HelpFlag = BoolFlag{ Usage: "show help", } +var DefaultFlagStringFunc FlagStringFunc = flagStringer + // Flag is a common interface related to parsing flags in cli. // For more advanced flag parsing techniques, it is recommended that // this interface be implemented. @@ -77,8 +79,7 @@ type GenericFlag struct { // help text to the user (uses the String() method of the generic flag to show // the value) func (f GenericFlag) String() string { - placeholder, usage := unquoteUsage(f.Usage) - return withEnvHint(f.EnvVar, fmt.Sprintf("%s %v\t%v", prefixedNames(f.Name, placeholder), f.FormatValueHelp(), usage)) + return DefaultFlagStringFunc(f) } func (f GenericFlag) FormatValueHelp() string { @@ -146,10 +147,7 @@ type StringSliceFlag struct { // String returns the usage func (f StringSliceFlag) String() string { - firstName := strings.Trim(strings.Split(f.Name, ",")[0], " ") - pref := prefixFor(firstName) - placeholder, usage := unquoteUsage(f.Usage) - return withEnvHint(f.EnvVar, fmt.Sprintf("%s [%v]\t%v", prefixedNames(f.Name, placeholder), pref+firstName+" option "+pref+firstName+" option", usage)) + return DefaultFlagStringFunc(f) } // Apply populates the flag given the flag set and environment @@ -181,6 +179,12 @@ 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 @@ -217,10 +221,7 @@ type IntSliceFlag struct { // String returns the usage func (f IntSliceFlag) String() string { - firstName := strings.Trim(strings.Split(f.Name, ",")[0], " ") - pref := prefixFor(firstName) - placeholder, usage := unquoteUsage(f.Usage) - return withEnvHint(f.EnvVar, fmt.Sprintf("%s [%v]\t%v", prefixedNames(f.Name, placeholder), pref+firstName+" option "+pref+firstName+" option", usage)) + return DefaultFlagStringFunc(f) } // Apply populates the flag given the flag set and environment @@ -255,6 +256,12 @@ 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 @@ -266,8 +273,7 @@ type BoolFlag struct { // String returns a readable representation of this value (for usage defaults) func (f BoolFlag) String() string { - placeholder, usage := unquoteUsage(f.Usage) - return withEnvHint(f.EnvVar, fmt.Sprintf("%s\t%v", prefixedNames(f.Name, placeholder), usage)) + return DefaultFlagStringFunc(f) } // Apply populates the flag given the flag set and environment @@ -311,8 +317,7 @@ type BoolTFlag struct { // String returns a readable representation of this value (for usage defaults) func (f BoolTFlag) String() string { - placeholder, usage := unquoteUsage(f.Usage) - return withEnvHint(f.EnvVar, fmt.Sprintf("%s\t%v", prefixedNames(f.Name, placeholder), usage)) + return DefaultFlagStringFunc(f) } // Apply populates the flag given the flag set and environment @@ -356,8 +361,7 @@ type StringFlag struct { // String returns the usage func (f StringFlag) String() string { - placeholder, usage := unquoteUsage(f.Usage) - return withEnvHint(f.EnvVar, fmt.Sprintf("%s %v\t%v", prefixedNames(f.Name, placeholder), f.FormatValueHelp(), usage)) + return DefaultFlagStringFunc(f) } func (f StringFlag) FormatValueHelp() string { @@ -406,8 +410,7 @@ type IntFlag struct { // String returns the usage func (f IntFlag) String() string { - placeholder, usage := unquoteUsage(f.Usage) - return withEnvHint(f.EnvVar, fmt.Sprintf("%s \"%v\"\t%v", prefixedNames(f.Name, placeholder), f.Value, usage)) + return DefaultFlagStringFunc(f) } // Apply populates the flag given the flag set and environment @@ -451,8 +454,7 @@ type DurationFlag struct { // String returns a readable representation of this value (for usage defaults) func (f DurationFlag) String() string { - placeholder, usage := unquoteUsage(f.Usage) - return withEnvHint(f.EnvVar, fmt.Sprintf("%s \"%v\"\t%v", prefixedNames(f.Name, placeholder), f.Value, usage)) + return DefaultFlagStringFunc(f) } // Apply populates the flag given the flag set and environment @@ -496,8 +498,7 @@ type Float64Flag struct { // String returns the usage func (f Float64Flag) String() string { - placeholder, usage := unquoteUsage(f.Usage) - return withEnvHint(f.EnvVar, fmt.Sprintf("%s \"%v\"\t%v", prefixedNames(f.Name, placeholder), f.Value, usage)) + return DefaultFlagStringFunc(f) } // Apply populates the flag given the flag set and environment @@ -595,3 +596,27 @@ func withEnvHint(envVar, str string) string { } return str + envText } + +func flagStringer(f Flag) string { + fv := reflect.ValueOf(f) + placeholder, usage := unquoteUsage(fv.FieldByName("Usage").String()) + + defaultValueString := "" + if val := fv.FieldByName("Value"); val.IsValid() { + defaultValueString = fmt.Sprintf("%v", val.Interface()) + } + + formatFunc := fv.MethodByName("FormatValueHelp") + if formatFunc.IsValid() { + defaultValueString = formatFunc.Call([]reflect.Value{})[0].String() + } + + if len(defaultValueString) > 0 { + defaultValueString = " " + defaultValueString + } + + return withEnvHint(fv.FieldByName("EnvVar").String(), + fmt.Sprintf("%s%v\t%v", + prefixedNames(fv.FieldByName("Name").String(), placeholder), + defaultValueString, usage)) +} diff --git a/flag_test.go b/flag_test.go index 48d920a..a49cac7 100644 --- a/flag_test.go +++ b/flag_test.go @@ -7,6 +7,7 @@ import ( "runtime" "strings" "testing" + "time" ) var boolFlagTests = []struct { @@ -18,13 +19,12 @@ var boolFlagTests = []struct { } func TestBoolFlagHelpOutput(t *testing.T) { - for _, test := range boolFlagTests { flag := BoolFlag{Name: test.name} output := flag.String() if output != test.expected { - t.Errorf("%s does not match %s", output, test.expected) + t.Errorf("%q does not match %q", output, test.expected) } } } @@ -35,11 +35,11 @@ var stringFlagTests = []struct { value string expected string }{ - {"help", "", "", "--help \t"}, - {"h", "", "", "-h \t"}, - {"h", "", "", "-h \t"}, + {"help", "", "", "--help\t"}, + {"h", "", "", "-h\t"}, + {"h", "", "", "-h\t"}, {"test", "", "Something", "--test \"Something\"\t"}, - {"config,c", "Load configuration from `FILE`", "", "--config FILE, -c FILE \tLoad configuration from FILE"}, + {"config,c", "Load configuration from `FILE`", "", "--config FILE, -c FILE\tLoad configuration from FILE"}, } func TestStringFlagHelpOutput(t *testing.T) { @@ -49,7 +49,7 @@ func TestStringFlagHelpOutput(t *testing.T) { output := flag.String() if output != test.expected { - t.Errorf("%s does not match %s", output, test.expected) + t.Errorf("%q does not match %q", output, test.expected) } } } @@ -131,8 +131,8 @@ var intFlagTests = []struct { name string expected string }{ - {"help", "--help \"0\"\t"}, - {"h", "-h \"0\"\t"}, + {"help", "--help 0\t"}, + {"h", "-h 0\t"}, } func TestIntFlagHelpOutput(t *testing.T) { @@ -168,18 +168,17 @@ var durationFlagTests = []struct { name string expected string }{ - {"help", "--help \"0\"\t"}, - {"h", "-h \"0\"\t"}, + {"help", "--help 1s\t"}, + {"h", "-h 1s\t"}, } func TestDurationFlagHelpOutput(t *testing.T) { - for _, test := range durationFlagTests { - flag := DurationFlag{Name: test.name} + flag := DurationFlag{Name: test.name, Value: 1 * time.Second} output := flag.String() if output != test.expected { - t.Errorf("%s does not match %s", output, test.expected) + t.Errorf("%q does not match %q", output, test.expected) } } } @@ -249,18 +248,17 @@ var float64FlagTests = []struct { name string expected string }{ - {"help", "--help \"0\"\t"}, - {"h", "-h \"0\"\t"}, + {"help", "--help 0.1\t"}, + {"h", "-h 0.1\t"}, } func TestFloat64FlagHelpOutput(t *testing.T) { - for _, test := range float64FlagTests { - flag := Float64Flag{Name: test.name} + flag := Float64Flag{Name: test.name, Value: float64(0.1)} output := flag.String() if output != test.expected { - t.Errorf("%s does not match %s", output, test.expected) + t.Errorf("%q does not match %q", output, test.expected) } } } @@ -292,7 +290,6 @@ var genericFlagTests = []struct { } func TestGenericFlagHelpOutput(t *testing.T) { - for _, test := range genericFlagTests { flag := GenericFlag{Name: test.name, Value: test.value, Usage: "test flag"} output := flag.String() diff --git a/funcs.go b/funcs.go index 94640ea..6b2a846 100644 --- a/funcs.go +++ b/funcs.go @@ -22,3 +22,7 @@ type CommandNotFoundFunc func(*Context, string) // original error messages. If this function is not set, the "Incorrect usage" // is displayed and the execution is interrupted. type OnUsageErrorFunc func(context *Context, err error, isSubcommand bool) error + +// FlagStringFunc is used by the help generation to display a flag, which is +// expected to be a single line. +type FlagStringFunc func(Flag) string From 23af5dd643c494dcc6cf2170734ef40fd87c0674 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Mon, 2 May 2016 13:07:57 -0400 Subject: [PATCH 3/7] Rename flag stringer func bits for clarity, consistency --- flag.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/flag.go b/flag.go index d21e018..d00a984 100644 --- a/flag.go +++ b/flag.go @@ -31,7 +31,7 @@ var HelpFlag = BoolFlag{ Usage: "show help", } -var DefaultFlagStringFunc FlagStringFunc = flagStringer +var FlagStringer FlagStringFunc = stringifyFlag // Flag is a common interface related to parsing flags in cli. // For more advanced flag parsing techniques, it is recommended that @@ -79,7 +79,7 @@ type GenericFlag struct { // help text to the user (uses the String() method of the generic flag to show // the value) func (f GenericFlag) String() string { - return DefaultFlagStringFunc(f) + return FlagStringer(f) } func (f GenericFlag) FormatValueHelp() string { @@ -147,7 +147,7 @@ type StringSliceFlag struct { // String returns the usage func (f StringSliceFlag) String() string { - return DefaultFlagStringFunc(f) + return FlagStringer(f) } // Apply populates the flag given the flag set and environment @@ -221,7 +221,7 @@ type IntSliceFlag struct { // String returns the usage func (f IntSliceFlag) String() string { - return DefaultFlagStringFunc(f) + return FlagStringer(f) } // Apply populates the flag given the flag set and environment @@ -273,7 +273,7 @@ type BoolFlag struct { // String returns a readable representation of this value (for usage defaults) func (f BoolFlag) String() string { - return DefaultFlagStringFunc(f) + return FlagStringer(f) } // Apply populates the flag given the flag set and environment @@ -317,7 +317,7 @@ type BoolTFlag struct { // String returns a readable representation of this value (for usage defaults) func (f BoolTFlag) String() string { - return DefaultFlagStringFunc(f) + return FlagStringer(f) } // Apply populates the flag given the flag set and environment @@ -361,7 +361,7 @@ type StringFlag struct { // String returns the usage func (f StringFlag) String() string { - return DefaultFlagStringFunc(f) + return FlagStringer(f) } func (f StringFlag) FormatValueHelp() string { @@ -410,7 +410,7 @@ type IntFlag struct { // String returns the usage func (f IntFlag) String() string { - return DefaultFlagStringFunc(f) + return FlagStringer(f) } // Apply populates the flag given the flag set and environment @@ -454,7 +454,7 @@ type DurationFlag struct { // String returns a readable representation of this value (for usage defaults) func (f DurationFlag) String() string { - return DefaultFlagStringFunc(f) + return FlagStringer(f) } // Apply populates the flag given the flag set and environment @@ -498,7 +498,7 @@ type Float64Flag struct { // String returns the usage func (f Float64Flag) String() string { - return DefaultFlagStringFunc(f) + return FlagStringer(f) } // Apply populates the flag given the flag set and environment @@ -597,7 +597,7 @@ func withEnvHint(envVar, str string) string { return str + envText } -func flagStringer(f Flag) string { +func stringifyFlag(f Flag) string { fv := reflect.ValueOf(f) placeholder, usage := unquoteUsage(fv.FieldByName("Usage").String()) From 69a8e25f3d7390cf45d81b130200b0a9448d3ca9 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Mon, 2 May 2016 19:42:08 -0400 Subject: [PATCH 4/7] 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) { From 7de151883c333cde1739a25ae112c89e959328d4 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Mon, 2 May 2016 19:48:35 -0400 Subject: [PATCH 5/7] Added more notes about usage formatting changes --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index efdc548..d7ea0a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ ### Changed - `Float64Flag`, `IntFlag`, and `DurationFlag` default values are no longer quoted in help text output. +- All flag types now include `(default: {value})` strings following usage when a +default value can be (reasonably) detected. +- `IntSliceFlag` and `StringSliceFlag` usage strings are now more consistent +with non-slice flag types ## [1.16.0] - 2016-05-02 ### Added From 6089d723a81a9970f806ac7e198c6dcc1210596e Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Mon, 2 May 2016 19:52:39 -0400 Subject: [PATCH 6/7] Use existing `prefixedNames` func --- flag.go | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/flag.go b/flag.go index 4535b5d..8f124d3 100644 --- a/flag.go +++ b/flag.go @@ -645,24 +645,11 @@ func stringifySliceFlag(usage, name string, defaultVals []string) string { 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) + return fmt.Sprintf("%s\t%s", prefixedNames(name, placeholder), usageWithDefault) } From cd92adcb7512ec11789a2b3bed9028367da1b175 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Mon, 2 May 2016 19:58:16 -0400 Subject: [PATCH 7/7] Further simplifying default flag stringer func --- flag.go | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/flag.go b/flag.go index 8f124d3..3b6a2e1 100644 --- a/flag.go +++ b/flag.go @@ -581,35 +581,29 @@ func stringifyFlag(f Flag) string { } placeholder, usage := unquoteUsage(fv.FieldByName("Usage").String()) - needsPlaceholder := false + needsPlaceholder := false defaultValueString := "" val := fv.FieldByName("Value") - valString := "" if val.IsValid() { needsPlaceholder = true + defaultValueString = fmt.Sprintf(" (default: %v)", val.Interface()) + if val.Kind() == reflect.String && val.String() != "" { - valString = fmt.Sprintf("%q", val.String()) - } else { - valString = fmt.Sprintf("%v", val.Interface()) + defaultValueString = fmt.Sprintf(" (default: %q)", val.String()) } } - if valString != "" { - defaultValueString = valString + if defaultValueString == " (default: )" { + defaultValueString = "" } if needsPlaceholder && placeholder == "" { placeholder = defaultPlaceholder } - formattedDefaultValueString := "" - if defaultValueString != "" { - formattedDefaultValueString = fmt.Sprintf(" (default: %v)", defaultValueString) - } - - usageWithDefault := strings.TrimSpace(fmt.Sprintf("%s%s", usage, formattedDefaultValueString)) + usageWithDefault := strings.TrimSpace(fmt.Sprintf("%s%s", usage, defaultValueString)) return withEnvHint(fv.FieldByName("EnvVar").String(), fmt.Sprintf("%s\t%s", prefixedNames(fv.FieldByName("Name").String(), placeholder), usageWithDefault))