From 52f0da632e3718157c5253fdf3d13fa55f0b349d Mon Sep 17 00:00:00 2001 From: Vamsi Avula Date: Sun, 10 Sep 2023 04:14:04 +0530 Subject: [PATCH 1/3] $ go fmt --- flag.go | 19 ++++++++++++++++--- flag_test.go | 2 -- string_slice.go | 28 ++++++++++++++++++++-------- 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/flag.go b/flag.go index 7c058de3..02de50a0 100644 --- a/flag.go +++ b/flag.go @@ -27,23 +27,32 @@ unaffected. Define flags using flag.String(), Bool(), Int(), etc. This declares an integer flag, -flagname, stored in the pointer ip, with type *int. + var ip = flag.Int("flagname", 1234, "help message for flagname") + If you like, you can bind the flag to a variable using the Var() functions. + var flagvar int func init() { flag.IntVar(&flagvar, "flagname", 1234, "help message for flagname") } + Or you can create custom flags that satisfy the Value interface (with pointer receivers) and couple them to flag parsing by + flag.Var(&flagVal, "name", "help message for flagname") + For such flags, the default value is just the initial value of the variable. After all flags are defined, call + flag.Parse() + to parse the command line into the defined flags. Flags may then be used directly. If you're using the flags themselves, they are all pointers; if you bind to variables, they're values. + fmt.Println("ip has value ", *ip) fmt.Println("flagvar has value ", flagvar) @@ -54,22 +63,26 @@ The arguments are indexed from 0 through flag.NArg()-1. The pflag package also defines some new functions that are not in flag, that give one-letter shorthands for flags. You can use these by appending 'P' to the name of any function that defines a flag. + var ip = flag.IntP("flagname", "f", 1234, "help message") var flagvar bool func init() { flag.BoolVarP(&flagvar, "boolname", "b", true, "help message") } flag.VarP(&flagval, "varname", "v", "help message") + Shorthand letters can be used with single dashes on the command line. Boolean shorthand flags can be combined with other shorthand flags. Command line flag syntax: + --flag // boolean flags only --flag=x Unlike the flag package, a single dash before an option means something different than a double dash. Single dashes signify a series of shorthand letters for flags. All but the last shorthand letter must be boolean flags. + // boolean flags -f -abc @@ -934,9 +947,9 @@ func (f *FlagSet) usage() { } } -//--unknown (args will be empty) -//--unknown --next-flag ... (args will be --next-flag ...) -//--unknown arg ... (args will be arg ...) +// --unknown (args will be empty) +// --unknown --next-flag ... (args will be --next-flag ...) +// --unknown arg ... (args will be arg ...) func stripUnknownFlagValue(args []string) []string { if len(args) == 0 { //--unknown diff --git a/flag_test.go b/flag_test.go index 58a5d25a..508550de 100644 --- a/flag_test.go +++ b/flag_test.go @@ -1134,7 +1134,6 @@ func TestMultipleNormalizeFlagNameInvocations(t *testing.T) { } } -// func TestHiddenFlagInUsage(t *testing.T) { f := NewFlagSet("bob", ContinueOnError) f.Bool("secretFlag", true, "shhh") @@ -1149,7 +1148,6 @@ func TestHiddenFlagInUsage(t *testing.T) { } } -// func TestHiddenFlagUsage(t *testing.T) { f := NewFlagSet("bob", ContinueOnError) f.Bool("secretFlag", true, "shhh") diff --git a/string_slice.go b/string_slice.go index 3cb2e69d..d421887e 100644 --- a/string_slice.go +++ b/string_slice.go @@ -98,9 +98,12 @@ func (f *FlagSet) GetStringSlice(name string) ([]string, error) { // The argument p points to a []string variable in which to store the value of the flag. // Compared to StringArray flags, StringSlice flags take comma-separated value as arguments and split them accordingly. // For example: -// --ss="v1,v2" --ss="v3" +// +// --ss="v1,v2" --ss="v3" +// // will result in -// []string{"v1", "v2", "v3"} +// +// []string{"v1", "v2", "v3"} func (f *FlagSet) StringSliceVar(p *[]string, name string, value []string, usage string) { f.VarP(newStringSliceValue(value, p), name, "", usage) } @@ -114,9 +117,12 @@ func (f *FlagSet) StringSliceVarP(p *[]string, name, shorthand string, value []s // The argument p points to a []string variable in which to store the value of the flag. // Compared to StringArray flags, StringSlice flags take comma-separated value as arguments and split them accordingly. // For example: -// --ss="v1,v2" --ss="v3" +// +// --ss="v1,v2" --ss="v3" +// // will result in -// []string{"v1", "v2", "v3"} +// +// []string{"v1", "v2", "v3"} func StringSliceVar(p *[]string, name string, value []string, usage string) { CommandLine.VarP(newStringSliceValue(value, p), name, "", usage) } @@ -130,9 +136,12 @@ func StringSliceVarP(p *[]string, name, shorthand string, value []string, usage // The return value is the address of a []string variable that stores the value of the flag. // Compared to StringArray flags, StringSlice flags take comma-separated value as arguments and split them accordingly. // For example: -// --ss="v1,v2" --ss="v3" +// +// --ss="v1,v2" --ss="v3" +// // will result in -// []string{"v1", "v2", "v3"} +// +// []string{"v1", "v2", "v3"} func (f *FlagSet) StringSlice(name string, value []string, usage string) *[]string { p := []string{} f.StringSliceVarP(&p, name, "", value, usage) @@ -150,9 +159,12 @@ func (f *FlagSet) StringSliceP(name, shorthand string, value []string, usage str // The return value is the address of a []string variable that stores the value of the flag. // Compared to StringArray flags, StringSlice flags take comma-separated value as arguments and split them accordingly. // For example: -// --ss="v1,v2" --ss="v3" +// +// --ss="v1,v2" --ss="v3" +// // will result in -// []string{"v1", "v2", "v3"} +// +// []string{"v1", "v2", "v3"} func StringSlice(name string, value []string, usage string) *[]string { return CommandLine.StringSliceP(name, "", value, usage) } From 8675393fb309b832d40aa74ee96deefb39782f64 Mon Sep 17 00:00:00 2001 From: Vamsi Avula Date: Sun, 10 Sep 2023 04:54:58 +0530 Subject: [PATCH 2/3] fix few lint / static analysis warnings --- duration_slice.go | 5 ++--- flag.go | 8 ++++---- flag_test.go | 2 +- ipnet_slice_test.go | 5 +---- string_array.go | 8 ++------ 5 files changed, 10 insertions(+), 18 deletions(-) diff --git a/duration_slice.go b/duration_slice.go index badadda5..ee987274 100644 --- a/duration_slice.go +++ b/duration_slice.go @@ -1,7 +1,6 @@ package pflag import ( - "fmt" "strings" "time" ) @@ -46,7 +45,7 @@ func (s *durationSliceValue) Type() string { func (s *durationSliceValue) String() string { out := make([]string, len(*s.value)) for i, d := range *s.value { - out[i] = fmt.Sprintf("%s", d) + out[i] = d.String() } return "[" + strings.Join(out, ",") + "]" } @@ -56,7 +55,7 @@ func (s *durationSliceValue) fromString(val string) (time.Duration, error) { } func (s *durationSliceValue) toString(val time.Duration) string { - return fmt.Sprintf("%s", val) + return val.String() } func (s *durationSliceValue) Append(val string) error { diff --git a/flag.go b/flag.go index 02de50a0..1906fda5 100644 --- a/flag.go +++ b/flag.go @@ -378,7 +378,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.Output(), msg) + fmt.Fprint(f.Output(), msg) panic(msg) } c := name[0] @@ -880,7 +880,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.Output(), msg) + fmt.Fprint(f.Output(), msg) panic(msg) } if f.shorthands == nil { @@ -890,7 +890,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.Output(), msg) + fmt.Fprint(f.Output(), msg) panic(msg) } f.shorthands[c] = flag @@ -1148,7 +1148,7 @@ func (f *FlagSet) Parse(arguments []string) error { } f.parsed = true - if len(arguments) < 0 { + if len(arguments) == 0 { return nil } diff --git a/flag_test.go b/flag_test.go index 508550de..aed29585 100644 --- a/flag_test.go +++ b/flag_test.go @@ -571,7 +571,7 @@ func TestShorthandLookup(t *testing.T) { defer func() { recover() }() - flag = f.ShorthandLookup("ab") + f.ShorthandLookup("ab") // should NEVER get here. lookup should panic. defer'd func should recover it. t.Errorf("f.ShorthandLookup(\"ab\") did not panic") } diff --git a/ipnet_slice_test.go b/ipnet_slice_test.go index 11644c58..1161b3e7 100644 --- a/ipnet_slice_test.go +++ b/ipnet_slice_test.go @@ -13,10 +13,7 @@ func getCIDR(ip net.IP, cidr *net.IPNet, err error) net.IPNet { } func equalCIDR(c1 net.IPNet, c2 net.IPNet) bool { - if c1.String() == c2.String() { - return true - } - return false + return c1.String() == c2.String() } func setUpIPNetFlagSet(ipsp *[]net.IPNet) *FlagSet { diff --git a/string_array.go b/string_array.go index d1ff0a96..2404407f 100644 --- a/string_array.go +++ b/string_array.go @@ -30,18 +30,14 @@ func (s *stringArrayValue) Append(val string) error { func (s *stringArrayValue) Replace(val []string) error { out := make([]string, len(val)) - for i, d := range val { - out[i] = d - } + copy(out, val) *s.value = out return nil } func (s *stringArrayValue) GetSlice() []string { out := make([]string, len(*s.value)) - for i, d := range *s.value { - out[i] = d - } + copy(out, *s.value) return out } From ce97b5aaf2d02c56fc27be55b3638ddb50e76d0c Mon Sep 17 00:00:00 2001 From: Vamsi Avula Date: Sat, 9 Sep 2023 23:25:44 +0530 Subject: [PATCH 3/3] usages: align flag types separately from names --- flag.go | 114 ++++++++++++++++++++++++++++++++------------- flag_test.go | 42 ++++++++--------- printusage_test.go | 4 +- 3 files changed, 105 insertions(+), 55 deletions(-) diff --git a/flag.go b/flag.go index 1906fda5..d22a6422 100644 --- a/flag.go +++ b/flag.go @@ -120,6 +120,7 @@ import ( "os" "sort" "strings" + "text/tabwriter" ) // ErrHelp is the error returned if the flag -help is invoked but no such flag is defined. @@ -578,6 +579,39 @@ func (f *Flag) defaultIsZeroValue() bool { } } +// lineWriter is a simple io.Writer implementation backed by a string slice. +// Unsurprisingly, it's most useful when one wants to consume the data written +// as lines -- strings.Join(lw.Lines(), "\n") will return the data written. +type lineWriter struct { + lines []string + buf bytes.Buffer +} + +func (lw *lineWriter) Write(p []byte) (int, error) { + n := len(p) + for i := bytes.IndexByte(p, '\n'); i != -1; { + lw.buf.Write(p[:i]) + lw.lines = append(lw.lines, lw.buf.String()) + lw.buf.Reset() + if i == len(p)-1 { // last character + return n, nil + } + p = p[i+1:] + i = bytes.IndexByte(p, '\n') + } + lw.buf.Write(p) + return n, nil +} + +func (lw *lineWriter) Lines() []string { + if len(lw.lines) == 0 && lw.buf.Len() == 0 { + return nil + } + return append(lw.lines, lw.buf.String()) +} + +var _ io.Writer = (*lineWriter)(nil) + // UnquoteUsage extracts a back-quoted name from the usage // string for a flag and returns it and the un-quoted usage. // Given "a `name` to show" it returns ("name", "a name to show"). @@ -695,74 +729,90 @@ func wrap(i, w int, s string) string { // for all flags in the FlagSet. Wrapped to `cols` columns (0 for no // wrapping) func (f *FlagSet) FlagUsagesWrapped(cols int) string { - buf := new(bytes.Buffer) - - lines := make([]string, 0, len(f.formal)) + var ( + lw = lineWriter{lines: make([]string, 0, len(f.formal))} + // Use tabwriter to align flag names, types and usages. + // Columns are separated by tabs and rows by newlines; tabwriter.Escape + // can be used to escape tabs / newlines within a cell. + tw = tabwriter.NewWriter(&lw, 0, 0, 0, ' ', tabwriter.StripEscape) + ) - maxlen := 0 f.VisitAll(func(flag *Flag) { if flag.Hidden { return } - line := "" if flag.Shorthand != "" && flag.ShorthandDeprecated == "" { - line = fmt.Sprintf(" -%s, --%s", flag.Shorthand, flag.Name) + fmt.Fprintf(tw, " -%s, --%s", flag.Shorthand, flag.Name) } else { - line = fmt.Sprintf(" --%s", flag.Name) + fmt.Fprintf(tw, " --%s", flag.Name) } + fmt.Fprint(tw, "\t") varname, usage := UnquoteUsage(flag) if varname != "" { - line += " " + varname + varname = " " + varname } + fmt.Fprint(tw, varname) + tw.Write([]byte{tabwriter.Escape}) if flag.NoOptDefVal != "" { switch flag.Value.Type() { case "string": - line += fmt.Sprintf("[=\"%s\"]", flag.NoOptDefVal) + fmt.Fprintf(tw, "[=\"%s\"]", flag.NoOptDefVal) case "bool": if flag.NoOptDefVal != "true" { - line += fmt.Sprintf("[=%s]", flag.NoOptDefVal) + fmt.Fprintf(tw, "[=%s]", flag.NoOptDefVal) } case "count": if flag.NoOptDefVal != "+1" { - line += fmt.Sprintf("[=%s]", flag.NoOptDefVal) + fmt.Fprintf(tw, "[=%s]", flag.NoOptDefVal) } default: - line += fmt.Sprintf("[=%s]", flag.NoOptDefVal) + fmt.Fprintf(tw, "[=%s]", flag.NoOptDefVal) } } + tw.Write([]byte{tabwriter.Escape, '\t', tabwriter.Escape}) - // This special character will be replaced with spacing once the - // correct alignment is calculated - line += "\x00" - if len(line) > maxlen { - maxlen = len(line) - } - - line += usage + // '\x00' tracks where usage starts (so we can wrap it as needed). + fmt.Fprintf(tw, " \x00%s", usage) if !flag.defaultIsZeroValue() { if flag.Value.Type() == "string" { - line += fmt.Sprintf(" (default %q)", flag.DefValue) + fmt.Fprintf(tw, " (default %q)", flag.DefValue) } else { - line += fmt.Sprintf(" (default %s)", flag.DefValue) + fmt.Fprintf(tw, " (default %s)", flag.DefValue) } } if len(flag.Deprecated) != 0 { - line += fmt.Sprintf(" (DEPRECATED: %s)", flag.Deprecated) + fmt.Fprintf(tw, " (DEPRECATED: %s)", flag.Deprecated) } - - lines = append(lines, line) + tw.Write([]byte{tabwriter.Escape, '\n'}) }) - for _, line := range lines { - sidx := strings.Index(line, "\x00") - spacing := strings.Repeat(" ", maxlen-sidx) - // maxlen + 2 comes from + 1 for the \x00 and + 1 for the (deliberate) off-by-one in maxlen-sidx - fmt.Fprintln(buf, line[:sidx], spacing, wrap(maxlen+2, cols, line[sidx+1:])) + tw.Flush() + lines := lw.Lines() + if len(lines) == 0 { + return "" } - - return buf.String() + var ( + // Usage should start at the same index for all the lines since + // tabwriter already aligns it for us, so just check the first line. + // Note: there's a small exception, see below. + i = strings.IndexByte(lines[0], '\x00') + b strings.Builder + ) + // Skip the empty line at the end. + for _, line := range lines[:len(lines)-1] { + // Some flags could have multiline usage, which would end up as separate + // lines -- they however wouldn't have '\x00', so we indent them with + // the same number of spaces as the first line and then wrap them. + // wrap indents by i+1 to account for the space introduced by Fprintln. + if strings.IndexByte(line, '\x00') == -1 { + fmt.Fprintln(&b, strings.Repeat(" ", i), wrap(i+1, cols, line)) + } else { + fmt.Fprintln(&b, line[:i], wrap(i+1, cols, line[i+1:])) + } + } + return b.String() } // FlagUsages returns a string containing the usage information for all flags in diff --git a/flag_test.go b/flag_test.go index aed29585..a5d7e4f5 100644 --- a/flag_test.go +++ b/flag_test.go @@ -1164,27 +1164,27 @@ func TestHiddenFlagUsage(t *testing.T) { } } -const defaultOutput = ` --A for bootstrapping, allow 'any' type - --Alongflagname disable bounds checking - -C, --CCC a boolean defaulting to true (default true) - --D path set relative path for local imports - -E, --EEE num[=1234] a num with NoOptDefVal (default 4321) - --F number a non-zero number (default 2.7) - --G float a float that defaults to zero - --IP ip IP address with no default - --IPMask ipMask Netmask address with no default - --IPNet ipNet IP network with no default - --Ints ints int slice with zero default - --N int a non-zero int (default 27) - --ND1 string[="bar"] a string with NoOptDefVal (default "foo") - --ND2 num[=4321] a num with NoOptDefVal (default 1234) - --StringArray stringArray string array with zero default - --StringSlice strings string slice with zero default - --Z int an int that defaults to zero - --custom custom custom Value implementation - --customP custom a VarP with default (default 10) - --maxT timeout set timeout for dial - -v, --verbose count verbosity +const defaultOutput = ` --A for bootstrapping, allow 'any' type + --Alongflagname disable bounds checking + -C, --CCC a boolean defaulting to true (default true) + --D path set relative path for local imports + -E, --EEE num[=1234] a num with NoOptDefVal (default 4321) + --F number a non-zero number (default 2.7) + --G float a float that defaults to zero + --IP ip IP address with no default + --IPMask ipMask Netmask address with no default + --IPNet ipNet IP network with no default + --Ints ints int slice with zero default + --N int a non-zero int (default 27) + --ND1 string[="bar"] a string with NoOptDefVal (default "foo") + --ND2 num[=4321] a num with NoOptDefVal (default 1234) + --StringArray stringArray string array with zero default + --StringSlice strings string slice with zero default + --Z int an int that defaults to zero + --custom custom custom Value implementation + --customP custom a VarP with default (default 10) + --maxT timeout set timeout for dial + -v, --verbose count verbosity ` // Custom value that satisfies the Value interface. diff --git a/printusage_test.go b/printusage_test.go index df982aab..2ee5b4fc 100644 --- a/printusage_test.go +++ b/printusage_test.go @@ -56,7 +56,7 @@ const expectedOutput2 = ` --long-form Some description -o, --other-very-long-arg string Some very long description having break the limit (default "long-default-value") - -l, --some-very-long-arg string Some very long description having + -l, --some-very-long-arg string Some very long description having break the limit (default "test") --some-very-long-arg2 string Some very long description with line break @@ -69,6 +69,6 @@ func TestPrintUsage_2(t *testing.T) { f := setUpPFlagSet2(&buf) res := f.FlagUsagesWrapped(80) if res != expectedOutput2 { - t.Errorf("Expected \n%q \nActual \n%q", expectedOutput2, res) + t.Errorf("Expected \n%s \nActual \n%s", expectedOutput2, res) } }