From 4727d4f50526f4dd181bacf4bd716b9442074508 Mon Sep 17 00:00:00 2001 From: Ryan McKern Date: Mon, 22 Jul 2019 23:56:40 -0700 Subject: [PATCH 1/3] Rename out() to Output() This brings behavior inline with go's flag library, and allows for printing output directly to whatever the current FlagSet is using for output. This change will make it easier to correctly emit output to stdout or stderr (e.g. a user has requested a help screen, which should emit to stdout since it's the desired outcome). --- flag.go | 24 +++++++++++++----------- flag_test.go | 11 +++++++++++ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/flag.go b/flag.go index 24a5036e..c14f393e 100644 --- a/flag.go +++ b/flag.go @@ -160,7 +160,7 @@ type FlagSet struct { args []string // arguments after flags argsLenAtDash int // len(args) when a '--' was located when parsing, or -1 if no -- errorHandling ErrorHandling - output io.Writer // nil means stderr; use out() accessor + output io.Writer // nil means stderr; use Output() accessor interspersed bool // allow interspersed option/non-option args normalizeNameFunc func(f *FlagSet, name string) NormalizedName @@ -255,7 +255,9 @@ func (f *FlagSet) normalizeFlagName(name string) NormalizedName { return n(f, name) } -func (f *FlagSet) out() io.Writer { +// Output returns the destination for usage and error messages. os.Stderr is returned if +// output was not set or was set to nil. +func (f *FlagSet) Output() io.Writer { if f.output == nil { return os.Stderr } @@ -358,7 +360,7 @@ func (f *FlagSet) ShorthandLookup(name string) *Flag { } if len(name) > 1 { msg := fmt.Sprintf("can not look up shorthand which is more than one ASCII character: %q", name) - fmt.Fprintf(f.out(), msg) + fmt.Fprintf(f.Output(), msg) panic(msg) } c := name[0] @@ -482,7 +484,7 @@ func (f *FlagSet) Set(name, value string) error { } if flag.Deprecated != "" { - fmt.Fprintf(f.out(), "Flag --%s has been deprecated, %s\n", flag.Name, flag.Deprecated) + fmt.Fprintf(f.Output(), "Flag --%s has been deprecated, %s\n", flag.Name, flag.Deprecated) } return nil } @@ -523,7 +525,7 @@ func Set(name, value string) error { // otherwise, the default values of all defined flags in the set. func (f *FlagSet) PrintDefaults() { usages := f.FlagUsages() - fmt.Fprint(f.out(), usages) + fmt.Fprint(f.Output(), usages) } // defaultIsZeroValue returns true if the default value for this flag represents @@ -758,7 +760,7 @@ func PrintDefaults() { // defaultUsage is the default function to print a usage message. func defaultUsage(f *FlagSet) { - fmt.Fprintf(f.out(), "Usage of %s:\n", f.name) + fmt.Fprintf(f.Output(), "Usage of %s:\n", f.name) f.PrintDefaults() } @@ -844,7 +846,7 @@ func (f *FlagSet) AddFlag(flag *Flag) { _, alreadyThere := f.formal[normalizedFlagName] if alreadyThere { msg := fmt.Sprintf("%s flag redefined: %s", f.name, flag.Name) - fmt.Fprintln(f.out(), msg) + fmt.Fprintln(f.Output(), msg) panic(msg) // Happens only if flags are declared with identical names } if f.formal == nil { @@ -860,7 +862,7 @@ func (f *FlagSet) AddFlag(flag *Flag) { } if len(flag.Shorthand) > 1 { msg := fmt.Sprintf("%q shorthand is more than one ASCII character", flag.Shorthand) - fmt.Fprintf(f.out(), msg) + fmt.Fprintf(f.Output(), msg) panic(msg) } if f.shorthands == nil { @@ -870,7 +872,7 @@ func (f *FlagSet) AddFlag(flag *Flag) { used, alreadyThere := f.shorthands[c] if alreadyThere { msg := fmt.Sprintf("unable to redefine %q shorthand in %q flagset: it's already used for %q flag", c, f.name, used.Name) - fmt.Fprintf(f.out(), msg) + fmt.Fprintf(f.Output(), msg) panic(msg) } f.shorthands[c] = flag @@ -909,7 +911,7 @@ func VarP(value Value, name, shorthand, usage string) { func (f *FlagSet) failf(format string, a ...interface{}) error { err := fmt.Errorf(format, a...) if f.errorHandling != ContinueOnError { - fmt.Fprintln(f.out(), err) + fmt.Fprintln(f.Output(), err) f.usage() } return err @@ -1060,7 +1062,7 @@ func (f *FlagSet) parseSingleShortArg(shorthands string, args []string, fn parse } if flag.ShorthandDeprecated != "" { - fmt.Fprintf(f.out(), "Flag shorthand -%s has been deprecated, %s\n", flag.Shorthand, flag.ShorthandDeprecated) + fmt.Fprintf(f.Output(), "Flag shorthand -%s has been deprecated, %s\n", flag.Shorthand, flag.ShorthandDeprecated) } err = fn(flag, value) diff --git a/flag_test.go b/flag_test.go index 7d02dbc8..dc24f8fb 100644 --- a/flag_test.go +++ b/flag_test.go @@ -854,6 +854,17 @@ func TestSetOutput(t *testing.T) { } } +func TestOutput(t *testing.T) { + var flags FlagSet + var buf bytes.Buffer + expect := "an example string" + flags.SetOutput(&buf) + fmt.Fprint(flags.Output(), expect) + if out := buf.String(); !strings.Contains(out, expect) { + t.Errorf("expected output %q; got %q", expect, out) + } +} + // This tests that one can reset the flags. This still works but not well, and is // superseded by FlagSet. func TestChangingArgs(t *testing.T) { From 3f7a00764bded088941a43f0dcf10a1ec242219e Mon Sep 17 00:00:00 2001 From: Chloe Kudryavtsev Date: Wed, 29 Jan 2020 21:53:02 -0500 Subject: [PATCH 2/3] improve compat. with pkg/flag by adding Name() pkg/flag has a public `Name()` function, which returns the name of the flag set when called. This commit adds that function, as well as a test for it. --- flag.go | 5 +++++ flag_test.go | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/flag.go b/flag.go index c14f393e..7c058de3 100644 --- a/flag.go +++ b/flag.go @@ -264,6 +264,11 @@ func (f *FlagSet) Output() io.Writer { return f.output } +// Name returns the name of the flag set. +func (f *FlagSet) Name() string { + return f.name +} + // SetOutput sets the destination for usage and error messages. // If output is nil, os.Stderr is used. func (f *FlagSet) SetOutput(output io.Writer) { diff --git a/flag_test.go b/flag_test.go index dc24f8fb..c999849a 100644 --- a/flag_test.go +++ b/flag_test.go @@ -115,6 +115,13 @@ func TestAddFlagSet(t *testing.T) { oldSet := NewFlagSet("old", ContinueOnError) newSet := NewFlagSet("new", ContinueOnError) + if oldSet.Name() != "old" { + t.Errorf("When reading a flagset's name, expected %s, but found %s", "old", oldSet.Name()) + } + if newSet.Name() != "new" { + t.Errorf("When reading a flagset's name, expected %s, but found %s", "new", newSet.Name()) + } + oldSet.String("flag1", "flag1", "flag1") oldSet.String("flag2", "flag2", "flag2") From 9335dd82dd35dc18f370c44fb16abd2b19a701d9 Mon Sep 17 00:00:00 2001 From: Ryan McKern Date: Thu, 20 Feb 2020 01:06:58 -0800 Subject: [PATCH 3/3] Streamline testing Name() Testing `Name()` will move into its own explicit test, instead of running inline during `TestAddFlagSet()`. Co-authored-by: Chloe Kudryavtsev --- flag_test.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/flag_test.go b/flag_test.go index c999849a..58a5d25a 100644 --- a/flag_test.go +++ b/flag_test.go @@ -115,13 +115,6 @@ func TestAddFlagSet(t *testing.T) { oldSet := NewFlagSet("old", ContinueOnError) newSet := NewFlagSet("new", ContinueOnError) - if oldSet.Name() != "old" { - t.Errorf("When reading a flagset's name, expected %s, but found %s", "old", oldSet.Name()) - } - if newSet.Name() != "new" { - t.Errorf("When reading a flagset's name, expected %s, but found %s", "new", newSet.Name()) - } - oldSet.String("flag1", "flag1", "flag1") oldSet.String("flag2", "flag2", "flag2") @@ -166,6 +159,16 @@ func TestAnnotation(t *testing.T) { } } +func TestName(t *testing.T) { + flagSetName := "bob" + f := NewFlagSet(flagSetName, ContinueOnError) + + givenName := f.Name() + if givenName != flagSetName { + t.Errorf("Unexpected result when retrieving a FlagSet's name: expected %s, but found %s", flagSetName, givenName) + } +} + func testParse(f *FlagSet, t *testing.T) { if f.Parsed() { t.Error("f.Parse() = true before Parse")