Merge pull request #381 from codegangsta/pluggable-flag-printer

Allow for pluggable flag-level help text formatting

CHANGELOG.md

@@ -3,6 +3,16 @@
 **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.
+- 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

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",
@@ -31,6 +33,8 @@ var HelpFlag = BoolFlag{
 	Usage: "show help",
 }
 
+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
 // this interface be implemented.
@@ -77,19 +81,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 FlagStringer(f)
-	return withEnvHint(f.EnvVar, fmt.Sprintf("%s %v\t%v", prefixedNames(f.Name, placeholder), f.FormatValueHelp(), usage))
-}
-
-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
@@ -146,10 +138,7 @@ type StringSliceFlag struct {
 
 // String returns the usage
 func (f StringSliceFlag) String() string {
-	firstName := strings.Trim(strings.Split(f.Name, ",")[0], " ")
+	return FlagStringer(f)
-	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))
 }
 
 // Apply populates the flag given the flag set and environment
@@ -217,10 +206,7 @@ type IntSliceFlag struct {
 
 // String returns the usage
 func (f IntSliceFlag) String() string {
-	firstName := strings.Trim(strings.Split(f.Name, ",")[0], " ")
+	return FlagStringer(f)
-	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))
 }
 
 // Apply populates the flag given the flag set and environment
@@ -266,8 +252,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 FlagStringer(f)
-	return withEnvHint(f.EnvVar, fmt.Sprintf("%s\t%v", prefixedNames(f.Name, placeholder), usage))
 }
 
 // Apply populates the flag given the flag set and environment
@@ -311,8 +296,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 FlagStringer(f)
-	return withEnvHint(f.EnvVar, fmt.Sprintf("%s\t%v", prefixedNames(f.Name, placeholder), usage))
 }
 
 // Apply populates the flag given the flag set and environment
@@ -356,16 +340,7 @@ type StringFlag struct {
 
 // String returns the usage
 func (f StringFlag) String() string {
-	placeholder, usage := unquoteUsage(f.Usage)
+	return FlagStringer(f)
-	return withEnvHint(f.EnvVar, fmt.Sprintf("%s %v\t%v", prefixedNames(f.Name, placeholder), f.FormatValueHelp(), usage))
-}
-
-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
@@ -406,8 +381,7 @@ type IntFlag struct {
 
 // String returns the usage
 func (f IntFlag) String() string {
-	placeholder, usage := unquoteUsage(f.Usage)
+	return FlagStringer(f)
-	return withEnvHint(f.EnvVar, fmt.Sprintf("%s \"%v\"\t%v", prefixedNames(f.Name, placeholder), f.Value, usage))
 }
 
 // Apply populates the flag given the flag set and environment
@@ -451,8 +425,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 FlagStringer(f)
-	return withEnvHint(f.EnvVar, fmt.Sprintf("%s \"%v\"\t%v", prefixedNames(f.Name, placeholder), f.Value, usage))
 }
 
 // Apply populates the flag given the flag set and environment
@@ -496,8 +469,7 @@ type Float64Flag struct {
 
 // String returns the usage
 func (f Float64Flag) String() string {
-	placeholder, usage := unquoteUsage(f.Usage)
+	return FlagStringer(f)
-	return withEnvHint(f.EnvVar, fmt.Sprintf("%s \"%v\"\t%v", prefixedNames(f.Name, placeholder), f.Value, usage))
 }
 
 // Apply populates the flag given the flag set and environment
@@ -595,3 +567,83 @@ func withEnvHint(envVar, str string) string {
 	}
 	return str + envText
 }
+
+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 := ""
+	val := fv.FieldByName("Value")
+
+	if val.IsValid() {
+		needsPlaceholder = true
+		defaultValueString = fmt.Sprintf(" (default: %v)", val.Interface())
+
+		if val.Kind() == reflect.String && val.String() != "" {
+			defaultValueString = fmt.Sprintf(" (default: %q)", val.String())
+		}
+	}
+
+	if defaultValueString == " (default: )" {
+		defaultValueString = ""
+	}
+
+	if needsPlaceholder && placeholder == "" {
+		placeholder = defaultPlaceholder
+	}
+
+	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))
+}
+
+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
+	}
+
+	defaultVal := ""
+	if len(defaultVals) > 0 {
+		defaultVal = fmt.Sprintf(" (default: %s)", strings.Join(defaultVals, ", "))
+	}
+
+	usageWithDefault := strings.TrimSpace(fmt.Sprintf("%s%s", usage, defaultVal))
+	return fmt.Sprintf("%s\t%s", prefixedNames(name, placeholder), usageWithDefault)
+}

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,21 +35,21 @@ var stringFlagTests = []struct {
 	value    string
 	expected string
 }{
-	{"help", "", "", "--help \t"},
+	{"foo", "", "", "--foo value\t"},
-	{"h", "", "", "-h \t"},
+	{"f", "", "", "-f value\t"},
-	{"h", "", "", "-h \t"},
+	{"f", "The total `foo` desired", "all", "-f foo\tThe total foo desired (default: \"all\")"},
-	{"test", "", "Something", "--test \"Something\"\t"},
+	{"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 `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()
 
 		if output != test.expected {
-			t.Errorf("%s does not match %s", output, test.expected)
+			t.Errorf("%q does not match %q", output, test.expected)
 		}
 	}
 }
@@ -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"},
+	}(), "--foo value\t"},
-	{"h", func() *StringSlice {
+	{"f", func() *StringSlice {
 		s := &StringSlice{}
 		s.Set("")
 		return s
-	}(), "-h [-h option -h option]\t"},
+	}(), "-f value\t"},
-	{"h", func() *StringSlice {
+	{"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"},
+	{"hats", "--hats value\t(default: 9)"},
-	{"h", "-h \"0\"\t"},
+	{"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,18 +166,17 @@ var durationFlagTests = []struct {
 	name     string
 	expected string
 }{
-	{"help", "--help \"0\"\t"},
+	{"hooting", "--hooting value\t(default: 1s)"},
-	{"h", "-h \"0\"\t"},
+	{"H", "-H value\t(default: 1s)"},
 }
 
 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)
 		}
 	}
 }
@@ -206,18 +203,17 @@ var intSliceFlagTests = []struct {
 	value    *IntSlice
 	expected string
 }{
-	{"help", &IntSlice{}, "--help [--help option --help option]\t"},
+	{"heads", &IntSlice{}, "--heads value\t"},
-	{"h", &IntSlice{}, "-h [-h option -h option]\t"},
+	{"H", &IntSlice{}, "-H value\t"},
-	{"h", &IntSlice{}, "-h [-h option -h option]\t"},
+	{"H, heads", func() *IntSlice {
-	{"test", 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()
@@ -249,18 +245,17 @@ var float64FlagTests = []struct {
 	name     string
 	expected string
 }{
-	{"help", "--help \"0\"\t"},
+	{"hooting", "--hooting value\t(default: 0.1)"},
-	{"h", "-h \"0\"\t"},
+	{"H", "-H value\t(default: 0.1)"},
 }
 
 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)
 		}
 	}
 }
@@ -287,12 +282,11 @@ var genericFlagTests = []struct {
 	value    Generic
 	expected string
 }{
-	{"test", &Parser{"abc", "def"}, "--test \"abc,def\"\ttest flag"},
+	{"toads", &Parser{"abc", "def"}, "--toads value\ttest flag (default: abc,def)"},
-	{"t", &Parser{"abc", "def"}, "-t \"abc,def\"\ttest flag"},
+	{"t", &Parser{"abc", "def"}, "-t value\ttest flag (default: abc,def)"},
 }
 
 func TestGenericFlagHelpOutput(t *testing.T) {
-
 	for _, test := range genericFlagTests {
 		flag := GenericFlag{Name: test.name, Value: test.value, Usage: "test flag"}
 		output := flag.String()

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