Make flag usage rendering more consistent; show default values

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() {
+	val := fv.FieldByName("Value")
-		defaultValueString = fmt.Sprintf("%v", val.Interface())
+
+	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 valString != "" {
-	if formatFunc.IsValid() {
+		defaultValueString = valString
-		defaultValueString = formatFunc.Call([]reflect.Value{})[0].String()
 	}
 
-	if len(defaultValueString) > 0 {
+	if needsPlaceholder && placeholder == "" {
-		defaultValueString = " " + defaultValueString
+		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",
+		fmt.Sprintf("%s\t%s", prefixedNames(fv.FieldByName("Name").String(), placeholder), usageWithDefault))
-			prefixedNames(fv.FieldByName("Name").String(), placeholder),
+}
-			defaultValueString, usage))
+
+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)
 }

flag_test.go

@@ -35,15 +35,15 @@ 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 `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"},
+	}(), "--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,8 +166,8 @@ var durationFlagTests = []struct {
 	name     string
 	expected string
 }{
-	{"help", "--help 1s\t"},
+	{"hooting", "--hooting value\t(default: 1s)"},
-	{"h", "-h 1s\t"},
+	{"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"},
+	{"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()
@@ -248,8 +245,8 @@ var float64FlagTests = []struct {
 	name     string
 	expected string
 }{
-	{"help", "--help 0.1\t"},
+	{"hooting", "--hooting value\t(default: 0.1)"},
-	{"h", "-h 0.1\t"},
+	{"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"},
+	{"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) {