From faebd3e0f24333a5777102db081a844fdaf7b07b Mon Sep 17 00:00:00 2001 From: Andrew Morozko Date: Sun, 20 Dec 2020 02:54:03 +0300 Subject: [PATCH 1/6] Optional long arguments --- parse.go | 16 +++++++++++----- parse_test.go | 12 ++++++++++++ usage.go | 35 +++++++++++++++++++++++++++-------- usage_test.go | 19 +++++++++++++++++++ 4 files changed, 69 insertions(+), 13 deletions(-) diff --git a/parse.go b/parse.go index 8fdbd9d..3b4ecd3 100644 --- a/parse.go +++ b/parse.go @@ -48,6 +48,7 @@ func (p path) Child(f reflect.StructField) path { type spec struct { dest path typ reflect.Type + name string // canonical name for the option long string short string multiple bool @@ -280,6 +281,8 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { typ: field.Type, } + spec.name = spec.long + help, exists := field.Tag.Lookup("help") if exists { spec.help = help @@ -308,6 +311,9 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { errs = append(errs, fmt.Sprintf("%s.%s: too many hyphens", t.Name(), field.Name)) case strings.HasPrefix(key, "--"): spec.long = key[2:] + if spec.long != "" { + spec.name = spec.long + } case strings.HasPrefix(key, "-"): if len(key) != 2 { errs = append(errs, fmt.Sprintf("%s.%s: short arguments must be one character only", @@ -364,7 +370,7 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { if hasPlaceholder { spec.placeholder = placeholder } else { - spec.placeholder = strings.ToUpper(spec.long) + spec.placeholder = strings.ToUpper(spec.name) } // Check whether this field is supported. It's good to do this here rather than @@ -617,13 +623,13 @@ func (p *Parser) process(args []string) error { if spec.multiple { err := setSlice(p.val(spec.dest), positionals, true) if err != nil { - return fmt.Errorf("error processing %s: %v", spec.long, err) + return fmt.Errorf("error processing %s: %v", spec.name, err) } positionals = nil } else { err := scalar.ParseValue(p.val(spec.dest), positionals[0]) if err != nil { - return fmt.Errorf("error processing %s: %v", spec.long, err) + return fmt.Errorf("error processing %s: %v", spec.name, err) } positionals = positionals[1:] } @@ -638,8 +644,8 @@ func (p *Parser) process(args []string) error { continue } - name := spec.long - if !spec.positional { + name := spec.name + if spec.long != "" && !spec.positional { name = "--" + spec.long } diff --git a/parse_test.go b/parse_test.go index ad668a9..91d8f54 100644 --- a/parse_test.go +++ b/parse_test.go @@ -231,6 +231,18 @@ func TestPlaceholder(t *testing.T) { assert.NoError(t, err) } +func TestNoLongName(t *testing.T) { + var args struct { + ShortOnly string `arg:"-s,--"` + EnvOnly string `arg:"--,env"` + } + setenv(t, "ENVONLY", "TestVal") + err := parse("-s TestVal2", &args) + assert.NoError(t, err) + assert.Equal(t, "TestVal", args.EnvOnly) + assert.Equal(t, "TestVal2", args.ShortOnly) +} + func TestCaseSensitive(t *testing.T) { var args struct { Lower bool `arg:"-v"` diff --git a/usage.go b/usage.go index a741570..fc8b09a 100644 --- a/usage.go +++ b/usage.go @@ -36,12 +36,15 @@ func (p *Parser) WriteUsage(w io.Writer) { // writeUsageForCommand writes usage information for the given subcommand func (p *Parser) writeUsageForCommand(w io.Writer, cmd *command) { - var positionals, options []*spec + var positionals, longOptions, shortOptions []*spec for _, spec := range cmd.specs { - if spec.positional { + switch { + case spec.positional: positionals = append(positionals, spec) - } else { - options = append(options, spec) + case spec.long != "": + longOptions = append(longOptions, spec) + case spec.short != "": + shortOptions = append(shortOptions, spec) } } @@ -64,7 +67,19 @@ func (p *Parser) writeUsageForCommand(w io.Writer, cmd *command) { } // write the option component of the usage message - for _, spec := range options { + for _, spec := range shortOptions { + // prefix with a space + fmt.Fprint(w, " ") + if !spec.required { + fmt.Fprint(w, "[") + } + fmt.Fprint(w, synopsis(spec, "-"+spec.short)) + if !spec.required { + fmt.Fprint(w, "]") + } + } + + for _, spec := range longOptions { // prefix with a space fmt.Fprint(w, " ") if !spec.required { @@ -215,10 +230,14 @@ func (p *Parser) writeHelpForCommand(w io.Writer, cmd *command) { } func (p *Parser) printOption(w io.Writer, spec *spec) { - left := synopsis(spec, "--"+spec.long) - if spec.short != "" { - left += ", " + synopsis(spec, "-"+spec.short) + text := make([]string, 0, 2) + if spec.long != "" { + text = append(text, synopsis(spec, "--"+spec.long)) } + if spec.short != "" { + text = append(text, synopsis(spec, "-"+spec.short)) + } + left := strings.Join(text, ", ") printTwoCols(w, left, spec.help, spec.defaultVal, spec.env) } diff --git a/usage_test.go b/usage_test.go index 5d379a1..de40ebd 100644 --- a/usage_test.go +++ b/usage_test.go @@ -309,3 +309,22 @@ Global options: p.WriteHelp(&help) assert.Equal(t, expectedHelp, help.String()) } + +func TestUsageWithOptionalLongNames(t *testing.T) { + expectedHelp := `Usage: example [-a PLACEHOLDER] -b SHORTONLY2 + +Options: + -a PLACEHOLDER some help [default: some val] + -b SHORTONLY2 some help2 + --help, -h display this help and exit +` + var args struct { + ShortOnly string `arg:"-a,--" help:"some help" default:"some val" placeholder:"PLACEHOLDER"` + ShortOnly2 string `arg:"-b,--,required" help:"some help2"` + } + p, err := NewParser(Config{Program: "example"}, &args) + assert.NoError(t, err) + var help bytes.Buffer + p.WriteHelp(&help) + assert.Equal(t, expectedHelp, help.String()) +} From 04c3fdbd8019af78797e714ac5070d7056e71f57 Mon Sep 17 00:00:00 2001 From: Andrew Morozko Date: Sun, 20 Dec 2020 03:07:18 +0300 Subject: [PATCH 2/6] Updated readme --- README.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index a9555b9..da69469 100644 --- a/README.md +++ b/README.md @@ -244,21 +244,23 @@ someprogram 4.3.0 ```go var args struct { - Short string `arg:"-s"` - Long string `arg:"--custom-long-option"` - ShortAndLong string `arg:"-x,--my-option"` + Short string `arg:"-s"` + Long string `arg:"--custom-long-option"` + ShortAndLong string `arg:"-x,--my-option"` + OnlyShort string `arg:"-o,--"` } arg.MustParse(&args) ``` ```shell $ ./example --help -Usage: [--short SHORT] [--custom-long-option CUSTOM-LONG-OPTION] [--my-option MY-OPTION] +Usage: example [-o ONLYSHORT] [--short SHORT] [--custom-long-option CUSTOM-LONG-OPTION] [--my-option MY-OPTION] Options: --short SHORT, -s SHORT --custom-long-option CUSTOM-LONG-OPTION --my-option MY-OPTION, -x MY-OPTION + -o ONLYSHORT --help, -h display this help and exit ``` From 438a91dba1e3f7a9263f5408899eeeb12d8453ed Mon Sep 17 00:00:00 2001 From: Andrew Morozko Date: Sun, 20 Dec 2020 03:51:33 +0300 Subject: [PATCH 3/6] Skip right column if the left is empty --- usage.go | 3 +++ usage_test.go | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/usage.go b/usage.go index fc8b09a..4704183 100644 --- a/usage.go +++ b/usage.go @@ -117,6 +117,9 @@ func (p *Parser) writeUsageForCommand(w io.Writer, cmd *command) { } func printTwoCols(w io.Writer, left, help string, defaultVal string, envVal string) { + if left == "" { + return + } lhs := " " + left fmt.Fprint(w, lhs) if help != "" { diff --git a/usage_test.go b/usage_test.go index de40ebd..8e28dc5 100644 --- a/usage_test.go +++ b/usage_test.go @@ -328,3 +328,23 @@ Options: p.WriteHelp(&help) assert.Equal(t, expectedHelp, help.String()) } + +func TestUsageWithEnvOptions(t *testing.T) { + expectedHelp := `Usage: example [-s SHORT] + +Options: + -s SHORT [env: SHORT] + --help, -h display this help and exit +` + var args struct { + Short string `arg:"--,-s,env"` + EnvOnly string `arg:"--,env"` + EnvOnlyOverriden string `arg:"--,env:CUSTOM"` + } + + p, err := NewParser(Config{Program: "example"}, &args) + assert.NoError(t, err) + var help bytes.Buffer + p.WriteHelp(&help) + assert.Equal(t, expectedHelp, help.String()) +} From 788c166025870bc4653adb5986d03f99c1d14af2 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Sun, 31 Jan 2021 19:15:49 -0800 Subject: [PATCH 4/6] test that short-only options are printed first in the help message --- usage.go | 34 +++++++++++++++++++--------------- usage_test.go | 21 ++++++++++++++++++++- 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/usage.go b/usage.go index 4704183..f3f9fe6 100644 --- a/usage.go +++ b/usage.go @@ -117,9 +117,6 @@ func (p *Parser) writeUsageForCommand(w io.Writer, cmd *command) { } func printTwoCols(w io.Writer, left, help string, defaultVal string, envVal string) { - if left == "" { - return - } lhs := " " + left fmt.Fprint(w, lhs) if help != "" { @@ -162,12 +159,15 @@ func (p *Parser) WriteHelp(w io.Writer) { // writeHelp writes the usage string for the given subcommand func (p *Parser) writeHelpForCommand(w io.Writer, cmd *command) { - var positionals, options []*spec + var positionals, longOptions, shortOptions []*spec for _, spec := range cmd.specs { - if spec.positional { + switch { + case spec.positional: positionals = append(positionals, spec) - } else { - options = append(options, spec) + case spec.long != "": + longOptions = append(longOptions, spec) + case spec.short != "": + shortOptions = append(shortOptions, spec) } } @@ -184,10 +184,13 @@ func (p *Parser) writeHelpForCommand(w io.Writer, cmd *command) { } } - // write the list of options - if len(options) > 0 || cmd.parent == nil { + // write the list of options with the short-only ones first to match the usage string + if len(shortOptions)+len(longOptions) > 0 || cmd.parent == nil { fmt.Fprint(w, "\nOptions:\n") - for _, spec := range options { + for _, spec := range shortOptions { + p.printOption(w, spec) + } + for _, spec := range longOptions { p.printOption(w, spec) } } @@ -233,15 +236,16 @@ func (p *Parser) writeHelpForCommand(w io.Writer, cmd *command) { } func (p *Parser) printOption(w io.Writer, spec *spec) { - text := make([]string, 0, 2) + ways := make([]string, 0, 2) if spec.long != "" { - text = append(text, synopsis(spec, "--"+spec.long)) + ways = append(ways, synopsis(spec, "--"+spec.long)) } if spec.short != "" { - text = append(text, synopsis(spec, "-"+spec.short)) + ways = append(ways, synopsis(spec, "-"+spec.short)) + } + if len(ways) > 0 { + printTwoCols(w, strings.Join(ways, ", "), spec.help, spec.defaultVal, spec.env) } - left := strings.Join(text, ", ") - printTwoCols(w, left, spec.help, spec.defaultVal, spec.env) } func synopsis(spec *spec, form string) string { diff --git a/usage_test.go b/usage_test.go index 8e28dc5..6dee402 100644 --- a/usage_test.go +++ b/usage_test.go @@ -310,7 +310,7 @@ Global options: assert.Equal(t, expectedHelp, help.String()) } -func TestUsageWithOptionalLongNames(t *testing.T) { +func TestUsageWithoutLongNames(t *testing.T) { expectedHelp := `Usage: example [-a PLACEHOLDER] -b SHORTONLY2 Options: @@ -329,6 +329,25 @@ Options: assert.Equal(t, expectedHelp, help.String()) } +func TestUsageWithShortFirst(t *testing.T) { + expectedHelp := `Usage: example [-c CAT] [--dog DOG] + +Options: + -c CAT + --dog DOG + --help, -h display this help and exit +` + var args struct { + Dog string + Cat string `arg:"-c,--"` + } + p, err := NewParser(Config{Program: "example"}, &args) + assert.NoError(t, err) + var help bytes.Buffer + p.WriteHelp(&help) + assert.Equal(t, expectedHelp, help.String()) +} + func TestUsageWithEnvOptions(t *testing.T) { expectedHelp := `Usage: example [-s SHORT] From efe5cdf4da7e0590ee3293247a3990ce609a8fdf Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Sun, 31 Jan 2021 19:40:38 -0800 Subject: [PATCH 5/6] replace "name" and "typ" by storing the original StructField --- parse.go | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/parse.go b/parse.go index 5308d70..22729e6 100644 --- a/parse.go +++ b/parse.go @@ -47,10 +47,9 @@ func (p path) Child(f reflect.StructField) path { // spec represents a command line option type spec struct { dest path - typ reflect.Type - name string // canonical name for the option - long string - short string + field reflect.StructField // name of struct field from this this option was created + long string // the --long form for this option, or empty if none + short string // the -s short form for this option, or empty if none multiple bool required bool positional bool @@ -276,13 +275,11 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { // duplicate the entire path to avoid slice overwrites subdest := dest.Child(field) spec := spec{ - dest: subdest, - long: strings.ToLower(field.Name), - typ: field.Type, + dest: subdest, + field: field, + long: strings.ToLower(field.Name), } - spec.name = spec.long - help, exists := field.Tag.Lookup("help") if exists { spec.help = help @@ -311,9 +308,6 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { errs = append(errs, fmt.Sprintf("%s.%s: too many hyphens", t.Name(), field.Name)) case strings.HasPrefix(key, "--"): spec.long = key[2:] - if spec.long != "" { - spec.name = spec.long - } case strings.HasPrefix(key, "-"): if len(key) != 2 { errs = append(errs, fmt.Sprintf("%s.%s: short arguments must be one character only", @@ -369,8 +363,10 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { placeholder, hasPlaceholder := field.Tag.Lookup("placeholder") if hasPlaceholder { spec.placeholder = placeholder + } else if spec.long != "" { + spec.placeholder = strings.ToUpper(spec.long) } else { - spec.placeholder = strings.ToUpper(spec.name) + spec.placeholder = strings.ToUpper(spec.field.Name) } // Check whether this field is supported. It's good to do this here rather than @@ -598,7 +594,7 @@ func (p *Parser) process(args []string) error { if i+1 == len(args) { return fmt.Errorf("missing value for %s", arg) } - if !nextIsNumeric(spec.typ, args[i+1]) && isFlag(args[i+1]) { + if !nextIsNumeric(spec.field.Type, args[i+1]) && isFlag(args[i+1]) { return fmt.Errorf("missing value for %s", arg) } value = args[i+1] @@ -623,13 +619,13 @@ func (p *Parser) process(args []string) error { if spec.multiple { err := setSlice(p.val(spec.dest), positionals, true) if err != nil { - return fmt.Errorf("error processing %s: %v", spec.name, err) + return fmt.Errorf("error processing %s: %v", spec.field.Name, err) } positionals = nil } else { err := scalar.ParseValue(p.val(spec.dest), positionals[0]) if err != nil { - return fmt.Errorf("error processing %s: %v", spec.name, err) + return fmt.Errorf("error processing %s: %v", spec.field.Name, err) } positionals = positionals[1:] } @@ -644,7 +640,7 @@ func (p *Parser) process(args []string) error { continue } - name := spec.name + name := strings.ToLower(spec.field.Name) if spec.long != "" && !spec.positional { name = "--" + spec.long } From 172800ff9a2765185520c06dcf969fde9f5eb5e3 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Fri, 16 Apr 2021 20:44:18 -0700 Subject: [PATCH 6/6] fix a comment --- parse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parse.go b/parse.go index 22729e6..b7d159d 100644 --- a/parse.go +++ b/parse.go @@ -47,7 +47,7 @@ func (p path) Child(f reflect.StructField) path { // spec represents a command line option type spec struct { dest path - field reflect.StructField // name of struct field from this this option was created + field reflect.StructField // the struct field from which this option was created long string // the --long form for this option, or empty if none short string // the -s short form for this option, or empty if none multiple bool