From 82ddbd9a0748a666ee92c658532fd669aa242498 Mon Sep 17 00:00:00 2001 From: Gregor Noczinski Date: Fri, 22 Jan 2016 15:08:27 +0100 Subject: [PATCH 1/2] * 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 | 38 ++++++++++++++++++++++++++--------- flag_test.go | 57 +++++++++++++++++++++++++++++++++++++++------------- 2 files changed, 72 insertions(+), 23 deletions(-) diff --git a/flag.go b/flag.go index 49f3099..9cf562d 100644 --- a/flag.go +++ b/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 - fmtString = "%s %v\t%v" + return withEnvHint(f.EnvVar, fmt.Sprintf("%s %v\t%v", prefixedNames(f.Name), f.FormatValueHelp(), f.Usage)) +} - if len(f.Value) > 0 { - fmtString = "%s \"%v\"\t%v" +func (f StringFlag) FormatValueHelp() string { + s := f.Value + if len(s) == 0 { + return "" } else { - fmtString = "%s %v\t%v" + return fmt.Sprintf("\"%s\"", s) } - - return withEnvHint(f.EnvVar, fmt.Sprintf(fmtString, prefixedNames(f.Name), f.Value, f.Usage)) } // 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 } diff --git a/flag_test.go b/flag_test.go index 4462d3f..3caa70a 100644 --- a/flag_test.go +++ b/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]") { - t.Errorf("%s does not end with [$APP_FOO]", output) + expectedSuffix := " [$APP_FOO]" + 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]") { - t.Errorf("%q does not end with [$APP_QWWX]", output) + expectedSuffix := " [$APP_QWWX]" + 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]") { - t.Errorf("%s does not end with [$APP_BAR]", output) + expectedSuffix := " [$APP_BAR]" + 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]") { - t.Errorf("%s does not end with [$APP_BAR]", output) + expectedSuffix := " [$APP_BAR]" + 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]") { - t.Errorf("%q does not end with [$APP_SMURF]", output) + expectedSuffix := " [$APP_SMURF]" + 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]") { - t.Errorf("%s does not end with [$APP_BAZ]", output) + expectedSuffix := " [$APP_BAZ]" + 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]") { - t.Errorf("%s does not end with [$APP_ZAP]", output) + expectedSuffix := " [$APP_ZAP]" + if runtime.GOOS == "windows" { + expectedSuffix = " [%APP_ZAP%]" + } + if !strings.HasSuffix(output, expectedSuffix) { + t.Errorf("%s does not end with" + expectedSuffix, output) } } } From 09e2c89597afd31cb88f66c404cd7fc0b783238b Mon Sep 17 00:00:00 2001 From: Gregor Noczinski Date: Sat, 23 Jan 2016 12:01:49 +0100 Subject: [PATCH 2/2] * Changed the way how to return the result. Because of strange ci failure --- flag.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/flag.go b/flag.go index 9cf562d..c8fb68b 100644 --- a/flag.go +++ b/flag.go @@ -350,9 +350,8 @@ func (f StringFlag) FormatValueHelp() string { s := f.Value if len(s) == 0 { return "" - } else { - return fmt.Sprintf("\"%s\"", s) } + return fmt.Sprintf("\"%s\"", s) } // Apply populates the flag given the flag set and environment