* Improve GenericFlag.String() by suppressing empty "" for GenericFlags on nil or empty Generic.String()

* Cleanup StringFlag.String()
* Add os specific envHint handling for Windows (%ENV_VAR% instead of $ENV_VAR on posix systems)

flag.go

@@ -7,6 +7,7 @@ import (
 	"strconv"
 	"strings"
 	"time"
+	"runtime"
 )
 
 // This flag enables bash-completion for all commands and subcommands
@@ -73,7 +74,18 @@ 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 withEnvHint(f.EnvVar, fmt.Sprintf("%s%s \"%v\"\t%v", prefixFor(f.Name), f.Name, f.Value, f.Usage))
+	return withEnvHint(f.EnvVar, fmt.Sprintf("%s %v\t%v", prefixedNames(f.Name), f.FormatValueHelp(), f.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
@@ -331,16 +343,16 @@ type StringFlag struct {
 
 // String returns the usage
 func (f StringFlag) String() string {
-	var fmtString string
+	return withEnvHint(f.EnvVar, fmt.Sprintf("%s %v\t%v", prefixedNames(f.Name), f.FormatValueHelp(), f.Usage))
-	fmtString = "%s %v\t%v"
-
-	if len(f.Value) > 0 {
-		fmtString = "%s \"%v\"\t%v"
-	} else {
-		fmtString = "%s %v\t%v"
 }
 
-	return withEnvHint(f.EnvVar, fmt.Sprintf(fmtString, prefixedNames(f.Name), f.Value, f.Usage))
+func (f StringFlag) FormatValueHelp() string {
+	s := f.Value
+	if len(s) == 0 {
+		return ""
+	} else {
+		return fmt.Sprintf("\"%s\"", s)
+	}
 }
 
 // Apply populates the flag given the flag set and environment
@@ -521,7 +533,15 @@ func prefixedNames(fullName string) (prefixed string) {
 func withEnvHint(envVar, str string) string {
 	envText := ""
 	if envVar != "" {
-		envText = fmt.Sprintf(" [$%s]", strings.Join(strings.Split(envVar, ","), ", $"))
+		prefix := "$"
+		suffix := ""
+		sep := ", $"
+		if runtime.GOOS == "windows" {
+			prefix = "%"
+			suffix = "%"
+			sep = "%, %"
+		}
+		envText = fmt.Sprintf(" [%s%s%s]", prefix, strings.Join(strings.Split(envVar, ","), sep), suffix)
 	}
 	return str + envText
 }

flag_test.go

@@ -6,6 +6,7 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+	"runtime"
 )
 
 var boolFlagTests = []struct {
@@ -58,8 +59,12 @@ func TestStringFlagWithEnvVarHelpOutput(t *testing.T) {
 		flag := StringFlag{Name: test.name, Value: test.value, EnvVar: "APP_FOO"}
 		output := flag.String()
 
-		if !strings.HasSuffix(output, " [$APP_FOO]") {
+		expectedSuffix := " [$APP_FOO]"
-			t.Errorf("%s does not end with [$APP_FOO]", output)
+		if runtime.GOOS == "windows" {
+			expectedSuffix = " [%APP_FOO%]"
+		}
+		if !strings.HasSuffix(output, expectedSuffix) {
+			t.Errorf("%s does not end with" + expectedSuffix, output)
 		}
 	}
 }
@@ -110,8 +115,12 @@ func TestStringSliceFlagWithEnvVarHelpOutput(t *testing.T) {
 		flag := StringSliceFlag{Name: test.name, Value: test.value, EnvVar: "APP_QWWX"}
 		output := flag.String()
 
-		if !strings.HasSuffix(output, " [$APP_QWWX]") {
+		expectedSuffix := " [$APP_QWWX]"
-			t.Errorf("%q does not end with [$APP_QWWX]", output)
+		if runtime.GOOS == "windows" {
+			expectedSuffix = " [%APP_QWWX%]"
+		}
+		if !strings.HasSuffix(output, expectedSuffix) {
+			t.Errorf("%q does not end with" + expectedSuffix, output)
 		}
 	}
 }
@@ -143,8 +152,12 @@ func TestIntFlagWithEnvVarHelpOutput(t *testing.T) {
 		flag := IntFlag{Name: test.name, EnvVar: "APP_BAR"}
 		output := flag.String()
 
-		if !strings.HasSuffix(output, " [$APP_BAR]") {
+		expectedSuffix := " [$APP_BAR]"
-			t.Errorf("%s does not end with [$APP_BAR]", output)
+		if runtime.GOOS == "windows" {
+			expectedSuffix = " [%APP_BAR%]"
+		}
+		if !strings.HasSuffix(output, expectedSuffix) {
+			t.Errorf("%s does not end with" + expectedSuffix, output)
 		}
 	}
 }
@@ -176,8 +189,12 @@ func TestDurationFlagWithEnvVarHelpOutput(t *testing.T) {
 		flag := DurationFlag{Name: test.name, EnvVar: "APP_BAR"}
 		output := flag.String()
 
-		if !strings.HasSuffix(output, " [$APP_BAR]") {
+		expectedSuffix := " [$APP_BAR]"
-			t.Errorf("%s does not end with [$APP_BAR]", output)
+		if runtime.GOOS == "windows" {
+			expectedSuffix = " [%APP_BAR%]"
+		}
+		if !strings.HasSuffix(output, expectedSuffix) {
+			t.Errorf("%s does not end with" + expectedSuffix, output)
 		}
 	}
 }
@@ -216,8 +233,12 @@ func TestIntSliceFlagWithEnvVarHelpOutput(t *testing.T) {
 		flag := IntSliceFlag{Name: test.name, Value: test.value, EnvVar: "APP_SMURF"}
 		output := flag.String()
 
-		if !strings.HasSuffix(output, " [$APP_SMURF]") {
+		expectedSuffix := " [$APP_SMURF]"
-			t.Errorf("%q does not end with [$APP_SMURF]", output)
+		if runtime.GOOS == "windows" {
+			expectedSuffix = " [%APP_SMURF%]"
+		}
+		if !strings.HasSuffix(output, expectedSuffix) {
+			t.Errorf("%q does not end with" + expectedSuffix, output)
 		}
 	}
 }
@@ -249,8 +270,12 @@ func TestFloat64FlagWithEnvVarHelpOutput(t *testing.T) {
 		flag := Float64Flag{Name: test.name, EnvVar: "APP_BAZ"}
 		output := flag.String()
 
-		if !strings.HasSuffix(output, " [$APP_BAZ]") {
+		expectedSuffix := " [$APP_BAZ]"
-			t.Errorf("%s does not end with [$APP_BAZ]", output)
+		if runtime.GOOS == "windows" {
+			expectedSuffix = " [%APP_BAZ%]"
+		}
+		if !strings.HasSuffix(output, expectedSuffix) {
+			t.Errorf("%s does not end with" + expectedSuffix, output)
 		}
 	}
 }
@@ -283,8 +308,12 @@ func TestGenericFlagWithEnvVarHelpOutput(t *testing.T) {
 		flag := GenericFlag{Name: test.name, EnvVar: "APP_ZAP"}
 		output := flag.String()
 
-		if !strings.HasSuffix(output, " [$APP_ZAP]") {
+		expectedSuffix := " [$APP_ZAP]"
-			t.Errorf("%s does not end with [$APP_ZAP]", output)
+		if runtime.GOOS == "windows" {
+			expectedSuffix = " [%APP_ZAP%]"
+		}
+		if !strings.HasSuffix(output, expectedSuffix) {
+			t.Errorf("%s does not end with" + expectedSuffix, output)
 		}
 	}
 }