From e2dda40825e8f3671cb207f6cc2f6e319404d57f Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Sun, 14 Apr 2019 19:50:17 -0700 Subject: [PATCH 01/28] all tests passing again --- parse.go | 298 ++++++++++++++++++++++++++++++++++++------------------- usage.go | 19 ++-- 2 files changed, 205 insertions(+), 112 deletions(-) diff --git a/parse.go b/parse.go index 32fc619..f4978ec 100644 --- a/parse.go +++ b/parse.go @@ -15,7 +15,9 @@ import ( // spec represents a command line option type spec struct { - dest reflect.Value + root int + path []string // sequence of field names + typ reflect.Type long string short string multiple bool @@ -27,6 +29,13 @@ type spec struct { boolean bool } +// command represents a named subcommand, or the top-level command +type command struct { + name string + specs []*spec + subcommands []*command +} + // ErrHelp indicates that -h or --help were provided var ErrHelp = errors.New("help requested by user") @@ -79,7 +88,8 @@ type Config struct { // Parser represents a set of command line options with destination values type Parser struct { - specs []*spec + cmd *command + roots []reflect.Value config Config version string description string @@ -102,134 +112,176 @@ type Described interface { } // walkFields calls a function for each field of a struct, recursively expanding struct fields. -func walkFields(v reflect.Value, visit func(field reflect.StructField, val reflect.Value, owner reflect.Type) bool) { - t := v.Type() +func walkFields(t reflect.Type, visit func(field reflect.StructField, owner reflect.Type) bool) { for i := 0; i < t.NumField(); i++ { field := t.Field(i) - val := v.Field(i) - expand := visit(field, val, t) + expand := visit(field, t) if expand && field.Type.Kind() == reflect.Struct { - walkFields(val, visit) + walkFields(field.Type, visit) } } } // NewParser constructs a parser from a list of destination structs func NewParser(config Config, dests ...interface{}) (*Parser, error) { + // first pick a name for the command for use in the usage text + var name string + switch { + case config.Program != "": + name = config.Program + case len(os.Args) > 0: + name = filepath.Base(os.Args[0]) + default: + name = "program" + } + + // construct a parser p := Parser{ + cmd: &command{name: name}, config: config, } + + // make a list of roots for _, dest := range dests { + p.roots = append(p.roots, reflect.ValueOf(dest)) + } + + // process each of the destination values + for i, dest := range dests { + t := reflect.TypeOf(dest) + if t.Kind() != reflect.Ptr { + panic(fmt.Sprintf("%s is not a pointer (did you forget an ampersand?)", t)) + } + t = t.Elem() + + cmd, err := cmdFromStruct(name, t, nil, i) + if err != nil { + return nil, err + } + p.cmd.specs = append(p.cmd.specs, cmd.specs...) + if dest, ok := dest.(Versioned); ok { p.version = dest.Version() } if dest, ok := dest.(Described); ok { p.description = dest.Description() } - v := reflect.ValueOf(dest) - if v.Kind() != reflect.Ptr { - panic(fmt.Sprintf("%s is not a pointer (did you forget an ampersand?)", v.Type())) - } - v = v.Elem() - if v.Kind() != reflect.Struct { - panic(fmt.Sprintf("%T is not a struct pointer", dest)) + } + + return &p, nil +} + +func cmdFromStruct(name string, t reflect.Type, path []string, root int) (*command, error) { + if t.Kind() != reflect.Struct { + panic(fmt.Sprintf("%v is not a struct pointer", t)) + } + + var cmd command + var errs []string + walkFields(t, func(field reflect.StructField, t reflect.Type) bool { + // Check for the ignore switch in the tag + tag := field.Tag.Get("arg") + if tag == "-" { + return false } - var errs []string - walkFields(v, func(field reflect.StructField, val reflect.Value, t reflect.Type) bool { - // Check for the ignore switch in the tag - tag := field.Tag.Get("arg") - if tag == "-" { - return false - } + // If this is an embedded struct then recurse into its fields + if field.Anonymous && field.Type.Kind() == reflect.Struct { + return true + } - // If this is an embedded struct then recurse into its fields - if field.Anonymous && field.Type.Kind() == reflect.Struct { - return true - } + spec := spec{ + root: root, + path: append(path, field.Name), + long: strings.ToLower(field.Name), + typ: field.Type, + } - spec := spec{ - long: strings.ToLower(field.Name), - dest: val, - } + help, exists := field.Tag.Lookup("help") + if exists { + spec.help = help + } - help, exists := field.Tag.Lookup("help") - if exists { - spec.help = help - } + // Check whether this field is supported. It's good to do this here rather than + // wait until ParseValue because it means that a program with invalid argument + // fields will always fail regardless of whether the arguments it received + // exercised those fields. + var parseable bool + parseable, spec.boolean, spec.multiple = canParse(field.Type) + if !parseable { + errs = append(errs, fmt.Sprintf("%s.%s: %s fields are not supported", + t.Name(), field.Name, field.Type.String())) + return false + } - // Check whether this field is supported. It's good to do this here rather than - // wait until ParseValue because it means that a program with invalid argument - // fields will always fail regardless of whether the arguments it received - // exercised those fields. - var parseable bool - parseable, spec.boolean, spec.multiple = canParse(field.Type) - if !parseable { - errs = append(errs, fmt.Sprintf("%s.%s: %s fields are not supported", - t.Name(), field.Name, field.Type.String())) - return false - } + // Look at the tag + if tag != "" { + for _, key := range strings.Split(tag, ",") { + key = strings.TrimLeft(key, " ") + var value string + if pos := strings.Index(key, ":"); pos != -1 { + value = key[pos+1:] + key = key[:pos] + } - // Look at the tag - if tag != "" { - for _, key := range strings.Split(tag, ",") { - key = strings.TrimLeft(key, " ") - var value string - if pos := strings.Index(key, ":"); pos != -1 { - value = key[pos+1:] - key = key[:pos] + switch { + case strings.HasPrefix(key, "---"): + errs = append(errs, fmt.Sprintf("%s.%s: too many hyphens", t.Name(), field.Name)) + case strings.HasPrefix(key, "--"): + spec.long = key[2:] + case strings.HasPrefix(key, "-"): + if len(key) != 2 { + errs = append(errs, fmt.Sprintf("%s.%s: short arguments must be one character only", + t.Name(), field.Name)) + return false + } + spec.short = key[1:] + case key == "required": + spec.required = true + case key == "positional": + spec.positional = true + case key == "separate": + spec.separate = true + case key == "help": // deprecated + spec.help = value + case key == "env": + // Use override name if provided + if value != "" { + spec.env = value + } else { + spec.env = strings.ToUpper(field.Name) + } + case key == "subcommand": + // decide on a name for the subcommand + cmdname := value + if cmdname == "" { + cmdname = strings.ToLower(field.Name) } - switch { - case strings.HasPrefix(key, "---"): - errs = append(errs, fmt.Sprintf("%s.%s: too many hyphens", t.Name(), field.Name)) - case strings.HasPrefix(key, "--"): - spec.long = key[2:] - case strings.HasPrefix(key, "-"): - if len(key) != 2 { - errs = append(errs, fmt.Sprintf("%s.%s: short arguments must be one character only", - t.Name(), field.Name)) - return false - } - spec.short = key[1:] - case key == "required": - spec.required = true - case key == "positional": - spec.positional = true - case key == "separate": - spec.separate = true - case key == "help": // deprecated - spec.help = value - case key == "env": - // Use override name if provided - if value != "" { - spec.env = value - } else { - spec.env = strings.ToUpper(field.Name) - } - default: - errs = append(errs, fmt.Sprintf("unrecognized tag '%s' on field %s", key, tag)) + subcmd, err := cmdFromStruct(cmdname, field.Type, append(path, field.Name), root) + if err != nil { + errs = append(errs, err.Error()) return false } + + cmd.subcommands = append(cmd.subcommands, subcmd) + default: + errs = append(errs, fmt.Sprintf("unrecognized tag '%s' on field %s", key, tag)) + return false } } - p.specs = append(p.specs, &spec) + } + cmd.specs = append(cmd.specs, &spec) - // if this was an embedded field then we already returned true up above - return false - }) + // if this was an embedded field then we already returned true up above + return false + }) - if len(errs) > 0 { - return nil, errors.New(strings.Join(errs, "\n")) - } + if len(errs) > 0 { + return nil, errors.New(strings.Join(errs, "\n")) } - if p.config.Program == "" { - p.config.Program = "program" - if len(os.Args) > 0 { - p.config.Program = filepath.Base(os.Args[0]) - } - } - return &p, nil + + return &cmd, nil } // Parse processes the given command line option, storing the results in the field @@ -249,12 +301,12 @@ func (p *Parser) Parse(args []string) error { } // Process all command line arguments - return process(p.specs, args) + return p.process(p.cmd.specs, args) } // process goes through arguments one-by-one, parses them, and assigns the result to // the underlying struct field -func process(specs []*spec, args []string) error { +func (p *Parser) process(specs []*spec, args []string) error { // track the options we have seen wasPresent := make(map[*spec]bool) @@ -294,7 +346,7 @@ func process(specs []*spec, args []string) error { err, ) } - if err = setSlice(spec.dest, values, !spec.separate); err != nil { + if err = setSlice(p.settable(spec), values, !spec.separate); err != nil { return fmt.Errorf( "error processing environment variable %s with multiple values: %v", spec.env, @@ -302,7 +354,7 @@ func process(specs []*spec, args []string) error { ) } } else { - if err := scalar.ParseValue(spec.dest, value); err != nil { + if err := scalar.ParseValue(p.settable(spec), value); err != nil { return fmt.Errorf("error processing environment variable %s: %v", spec.env, err) } } @@ -355,7 +407,7 @@ func process(specs []*spec, args []string) error { } else { values = append(values, value) } - err := setSlice(spec.dest, values, !spec.separate) + err := setSlice(p.settable(spec), values, !spec.separate) if err != nil { return fmt.Errorf("error processing %s: %v", arg, err) } @@ -373,14 +425,14 @@ func process(specs []*spec, args []string) error { if i+1 == len(args) { return fmt.Errorf("missing value for %s", arg) } - if !nextIsNumeric(spec.dest.Type(), args[i+1]) && isFlag(args[i+1]) { + if !nextIsNumeric(spec.typ, args[i+1]) && isFlag(args[i+1]) { return fmt.Errorf("missing value for %s", arg) } value = args[i+1] i++ } - err := scalar.ParseValue(spec.dest, value) + err := scalar.ParseValue(p.settable(spec), value) if err != nil { return fmt.Errorf("error processing %s: %v", arg, err) } @@ -396,13 +448,13 @@ func process(specs []*spec, args []string) error { } wasPresent[spec] = true if spec.multiple { - err := setSlice(spec.dest, positionals, true) + err := setSlice(p.settable(spec), positionals, true) if err != nil { return fmt.Errorf("error processing %s: %v", spec.long, err) } positionals = nil } else { - err := scalar.ParseValue(spec.dest, positionals[0]) + err := scalar.ParseValue(p.settable(spec), positionals[0]) if err != nil { return fmt.Errorf("error processing %s: %v", spec.long, err) } @@ -445,6 +497,44 @@ func isFlag(s string) bool { return strings.HasPrefix(s, "-") && strings.TrimLeft(s, "-") != "" } +func (p *Parser) get(spec *spec) reflect.Value { + v := p.roots[spec.root] + for _, field := range spec.path { + if v.Kind() == reflect.Ptr { + if v.IsNil() { + return reflect.Value{} + } + v = v.Elem() + } + + v = v.FieldByName(field) + if !v.IsValid() { + panic(fmt.Errorf("error resolving path %v: %v has no field named %v", + spec.path, v.Type(), field)) + } + } + return v +} + +func (p *Parser) settable(spec *spec) reflect.Value { + v := p.roots[spec.root] + for _, field := range spec.path { + if v.Kind() == reflect.Ptr { + if v.IsNil() { + v.Set(reflect.New(v.Type().Elem())) + } + v = v.Elem() + } + + v = v.FieldByName(field) + if !v.IsValid() { + panic(fmt.Errorf("error resolving path %v: %v has no field named %v", + spec.path, v.Type(), field)) + } + } + return v +} + // parse a value as the appropriate type and store it in the struct func setSlice(dest reflect.Value, values []string, trunc bool) error { if !dest.CanSet() { diff --git a/usage.go b/usage.go index cfac563..f9c1a76 100644 --- a/usage.go +++ b/usage.go @@ -22,7 +22,7 @@ func (p *Parser) Fail(msg string) { // WriteUsage writes usage information to the given writer func (p *Parser) WriteUsage(w io.Writer) { var positionals, options []*spec - for _, spec := range p.specs { + for _, spec := range p.cmd.specs { if spec.positional { positionals = append(positionals, spec) } else { @@ -34,7 +34,7 @@ func (p *Parser) WriteUsage(w io.Writer) { fmt.Fprintln(w, p.version) } - fmt.Fprintf(w, "Usage: %s", p.config.Program) + fmt.Fprintf(w, "Usage: %s", p.cmd.name) // write the option component of the usage message for _, spec := range options { @@ -72,7 +72,7 @@ func (p *Parser) WriteUsage(w io.Writer) { // WriteHelp writes the usage string followed by the full help string for each option func (p *Parser) WriteHelp(w io.Writer) { var positionals, options []*spec - for _, spec := range p.specs { + for _, spec := range p.cmd.specs { if spec.positional { positionals = append(positionals, spec) } else { @@ -106,17 +106,17 @@ func (p *Parser) WriteHelp(w io.Writer) { // write the list of options fmt.Fprint(w, "\nOptions:\n") for _, spec := range options { - printOption(w, spec) + p.printOption(w, spec) } // write the list of built in options - printOption(w, &spec{boolean: true, long: "help", short: "h", help: "display this help and exit"}) + p.printOption(w, &spec{boolean: true, long: "help", short: "h", help: "display this help and exit", root: -1}) if p.version != "" { - printOption(w, &spec{boolean: true, long: "version", help: "display version and exit"}) + p.printOption(w, &spec{boolean: true, long: "version", help: "display version and exit", root: -1}) } } -func printOption(w io.Writer, spec *spec) { +func (p *Parser) printOption(w io.Writer, spec *spec) { left := " " + synopsis(spec, "--"+spec.long) if spec.short != "" { left += ", " + synopsis(spec, "-"+spec.short) @@ -131,7 +131,10 @@ func printOption(w io.Writer, spec *spec) { fmt.Fprint(w, spec.help) } // If spec.dest is not the zero value then a default value has been added. - v := spec.dest + var v reflect.Value + if spec.root >= 0 { + v = p.get(spec) + } if v.IsValid() { z := reflect.Zero(v.Type()) if (v.Type().Comparable() && z.Type().Comparable() && v.Interface() != z.Interface()) || v.Kind() == reflect.Slice && !v.IsNil() { From f282f71f2696dde5ad388183e755d030f9cbe38c Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Tue, 30 Apr 2019 11:16:01 -0700 Subject: [PATCH 02/28] minor reformat --- usage.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/usage.go b/usage.go index f9c1a76..b40619f 100644 --- a/usage.go +++ b/usage.go @@ -110,9 +110,20 @@ func (p *Parser) WriteHelp(w io.Writer) { } // write the list of built in options - p.printOption(w, &spec{boolean: true, long: "help", short: "h", help: "display this help and exit", root: -1}) + p.printOption(w, &spec{ + boolean: true, + long: "help", + short: "h", + help: "display this help and exit", + root: -1, + }) if p.version != "" { - p.printOption(w, &spec{boolean: true, long: "version", help: "display version and exit", root: -1}) + p.printOption(w, &spec{ + boolean: true, + long: "version", + help: "display version and exit", + root: -1, + }) } } From 7df132abe85e0f04971c9b271bc9cd62d750997c Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Tue, 30 Apr 2019 11:16:10 -0700 Subject: [PATCH 03/28] check error in test --- parse_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/parse_test.go b/parse_test.go index 81cd2c3..9aad2e3 100644 --- a/parse_test.go +++ b/parse_test.go @@ -979,6 +979,7 @@ func TestReuseParser(t *testing.T) { require.NoError(t, err) err = p.Parse([]string{"--foo=abc"}) + require.NoError(t, err) assert.Equal(t, args.Foo, "abc") err = p.Parse([]string{}) From ddec9e9e4febd4f367f31297ad2744d683f474b4 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Tue, 30 Apr 2019 11:40:11 -0700 Subject: [PATCH 04/28] rename get/settable to readable/writable --- parse.go | 27 +++++++++++++++++++-------- usage.go | 2 +- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/parse.go b/parse.go index f4978ec..b5b76b8 100644 --- a/parse.go +++ b/parse.go @@ -346,7 +346,7 @@ func (p *Parser) process(specs []*spec, args []string) error { err, ) } - if err = setSlice(p.settable(spec), values, !spec.separate); err != nil { + if err = setSlice(p.writable(spec), values, !spec.separate); err != nil { return fmt.Errorf( "error processing environment variable %s with multiple values: %v", spec.env, @@ -354,7 +354,7 @@ func (p *Parser) process(specs []*spec, args []string) error { ) } } else { - if err := scalar.ParseValue(p.settable(spec), value); err != nil { + if err := scalar.ParseValue(p.writable(spec), value); err != nil { return fmt.Errorf("error processing environment variable %s: %v", spec.env, err) } } @@ -407,7 +407,7 @@ func (p *Parser) process(specs []*spec, args []string) error { } else { values = append(values, value) } - err := setSlice(p.settable(spec), values, !spec.separate) + err := setSlice(p.writable(spec), values, !spec.separate) if err != nil { return fmt.Errorf("error processing %s: %v", arg, err) } @@ -432,7 +432,7 @@ func (p *Parser) process(specs []*spec, args []string) error { i++ } - err := scalar.ParseValue(p.settable(spec), value) + err := scalar.ParseValue(p.writable(spec), value) if err != nil { return fmt.Errorf("error processing %s: %v", arg, err) } @@ -448,13 +448,13 @@ func (p *Parser) process(specs []*spec, args []string) error { } wasPresent[spec] = true if spec.multiple { - err := setSlice(p.settable(spec), positionals, true) + err := setSlice(p.writable(spec), positionals, true) if err != nil { return fmt.Errorf("error processing %s: %v", spec.long, err) } positionals = nil } else { - err := scalar.ParseValue(p.settable(spec), positionals[0]) + err := scalar.ParseValue(p.writable(spec), positionals[0]) if err != nil { return fmt.Errorf("error processing %s: %v", spec.long, err) } @@ -497,7 +497,9 @@ func isFlag(s string) bool { return strings.HasPrefix(s, "-") && strings.TrimLeft(s, "-") != "" } -func (p *Parser) get(spec *spec) reflect.Value { +// readable returns a reflect.Value corresponding to the current value for the +// given +func (p *Parser) readable(spec *spec) reflect.Value { v := p.roots[spec.root] for _, field := range spec.path { if v.Kind() == reflect.Ptr { @@ -509,6 +511,9 @@ func (p *Parser) get(spec *spec) reflect.Value { v = v.FieldByName(field) if !v.IsValid() { + // it is appropriate to panic here because this can only happen due to + // an internal bug in this library (since we construct spec.path ourselves + // by reflecting on the same struct) panic(fmt.Errorf("error resolving path %v: %v has no field named %v", spec.path, v.Type(), field)) } @@ -516,7 +521,10 @@ func (p *Parser) get(spec *spec) reflect.Value { return v } -func (p *Parser) settable(spec *spec) reflect.Value { +// writable traverses the destination struct to find the destination to +// which the value of the given spec should be written. It fills in null +// structs with pointers to the zero value for that struct. +func (p *Parser) writable(spec *spec) reflect.Value { v := p.roots[spec.root] for _, field := range spec.path { if v.Kind() == reflect.Ptr { @@ -528,6 +536,9 @@ func (p *Parser) settable(spec *spec) reflect.Value { v = v.FieldByName(field) if !v.IsValid() { + // it is appropriate to panic here because this can only happen due to + // an internal bug in this library (since we construct spec.path ourselves + // by reflecting on the same struct) panic(fmt.Errorf("error resolving path %v: %v has no field named %v", spec.path, v.Type(), field)) } diff --git a/usage.go b/usage.go index b40619f..833046f 100644 --- a/usage.go +++ b/usage.go @@ -144,7 +144,7 @@ func (p *Parser) printOption(w io.Writer, spec *spec) { // If spec.dest is not the zero value then a default value has been added. var v reflect.Value if spec.root >= 0 { - v = p.get(spec) + v = p.readable(spec) } if v.IsValid() { z := reflect.Zero(v.Type()) From 4e977796af5ef0863a674ef468c5036dcca20623 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Tue, 30 Apr 2019 12:54:28 -0700 Subject: [PATCH 05/28] add recursive expansion of subcommands --- parse.go | 130 ++++++++++++++++++++++++++++++++++++++------------ parse_test.go | 7 ++- 2 files changed, 103 insertions(+), 34 deletions(-) diff --git a/parse.go b/parse.go index b5b76b8..353b365 100644 --- a/parse.go +++ b/parse.go @@ -152,7 +152,6 @@ func NewParser(config Config, dests ...interface{}) (*Parser, error) { if t.Kind() != reflect.Ptr { panic(fmt.Sprintf("%s is not a pointer (did you forget an ampersand?)", t)) } - t = t.Elem() cmd, err := cmdFromStruct(name, t, nil, i) if err != nil { @@ -172,8 +171,16 @@ func NewParser(config Config, dests ...interface{}) (*Parser, error) { } func cmdFromStruct(name string, t reflect.Type, path []string, root int) (*command, error) { + // commands can only be created from pointers to structs + if t.Kind() != reflect.Ptr { + return nil, fmt.Errorf("subcommands must be pointers to structs but args.%s is a %s", + strings.Join(path, "."), t.Kind()) + } + + t = t.Elem() if t.Kind() != reflect.Struct { - panic(fmt.Sprintf("%v is not a struct pointer", t)) + return nil, fmt.Errorf("subcommands must be pointers to structs but args.%s is a pointer to %s", + strings.Join(path, "."), t.Kind()) } var cmd command @@ -190,9 +197,13 @@ func cmdFromStruct(name string, t reflect.Type, path []string, root int) (*comma return true } + // duplicate the entire path to avoid slice overwrites + subpath := make([]string, len(path)+1) + copy(subpath, append(path, field.Name)) + spec := spec{ root: root, - path: append(path, field.Name), + path: subpath, long: strings.ToLower(field.Name), typ: field.Type, } @@ -258,7 +269,7 @@ func cmdFromStruct(name string, t reflect.Type, path []string, root int) (*comma cmdname = strings.ToLower(field.Name) } - subcmd, err := cmdFromStruct(cmdname, field.Type, append(path, field.Name), root) + subcmd, err := cmdFromStruct(cmdname, field.Type, subpath, root) if err != nil { errs = append(errs, err.Error()) return false @@ -281,6 +292,17 @@ func cmdFromStruct(name string, t reflect.Type, path []string, root int) (*comma return nil, errors.New(strings.Join(errs, "\n")) } + // check that we don't have both positionals and subcommands + var hasPositional bool + for _, spec := range cmd.specs { + if spec.positional { + hasPositional = true + } + } + if hasPositional && len(cmd.subcommands) > 0 { + return nil, fmt.Errorf("%T cannot have both subcommands and positional arguments", t) + } + return &cmd, nil } @@ -301,30 +323,11 @@ func (p *Parser) Parse(args []string) error { } // Process all command line arguments - return p.process(p.cmd.specs, args) + return p.process(args) } -// process goes through arguments one-by-one, parses them, and assigns the result to -// the underlying struct field -func (p *Parser) process(specs []*spec, args []string) error { - // track the options we have seen - wasPresent := make(map[*spec]bool) - - // construct a map from --option to spec - optionMap := make(map[string]*spec) - for _, spec := range specs { - if spec.positional { - continue - } - if spec.long != "" { - optionMap[spec.long] = spec - } - if spec.short != "" { - optionMap[spec.short] = spec - } - } - - // deal with environment vars +// process environment vars for the given arguments +func (p *Parser) captureEnvVars(specs []*spec, wasPresent map[*spec]bool) error { for _, spec := range specs { if spec.env == "" { continue @@ -361,6 +364,28 @@ func (p *Parser) process(specs []*spec, args []string) error { wasPresent[spec] = true } + return nil +} + +// process goes through arguments one-by-one, parses them, and assigns the result to +// the underlying struct field +func (p *Parser) process(args []string) error { + // track the options we have seen + wasPresent := make(map[*spec]bool) + + // union of specs for the chain of subcommands encountered so far + curCmd := p.cmd + + // make a copy of the specs because we will add to this list each time we expand a subcommand + specs := make([]*spec, len(curCmd.specs)) + copy(specs, curCmd.specs) + + // deal with environment vars + err := p.captureEnvVars(specs, wasPresent) + if err != nil { + return err + } + // process each string from the command line var allpositional bool var positionals []string @@ -374,7 +399,28 @@ func (p *Parser) process(specs []*spec, args []string) error { } if !isFlag(arg) || allpositional { - positionals = append(positionals, arg) + // each subcommand can have either subcommands or positionals, but not both + if len(curCmd.subcommands) == 0 { + positionals = append(positionals, arg) + continue + } + + // if we have a subcommand then make sure it is valid for the current context + subcmd := findSubcommand(curCmd.subcommands, arg) + if subcmd == nil { + return fmt.Errorf("invalid subcommand: %s", arg) + } + + // add the new options to the set of allowed options + specs = append(specs, subcmd.specs...) + + // capture environment vars for these new options + err := p.captureEnvVars(subcmd.specs, wasPresent) + if err != nil { + return err + } + + curCmd = subcmd continue } @@ -386,9 +432,10 @@ func (p *Parser) process(specs []*spec, args []string) error { opt = opt[:pos] } - // lookup the spec for this option - spec, ok := optionMap[opt] - if !ok { + // lookup the spec for this option (note that the "specs" slice changes as + // we expand subcommands so it is better not to use a map) + spec := findOption(specs, opt) + if spec == nil { return fmt.Errorf("unknown argument %s", arg) } wasPresent[spec] = true @@ -630,3 +677,26 @@ func isBoolean(t reflect.Type) bool { return false } } + +// findOption finds an option from its name, or returns null if no spec is found +func findOption(specs []*spec, name string) *spec { + for _, spec := range specs { + if spec.positional { + continue + } + if spec.long == name || spec.short == name { + return spec + } + } + return nil +} + +// findSubcommand finds a subcommand using its name, or returns null if no subcommand is found +func findSubcommand(cmds []*command, name string) *command { + for _, cmd := range cmds { + if cmd.name == name { + return cmd + } + } + return nil +} diff --git a/parse_test.go b/parse_test.go index 9aad2e3..94cf21a 100644 --- a/parse_test.go +++ b/parse_test.go @@ -462,11 +462,10 @@ func TestPanicOnNonPointer(t *testing.T) { }) } -func TestPanicOnNonStruct(t *testing.T) { +func TestErrorOnNonStruct(t *testing.T) { var args string - assert.Panics(t, func() { - _ = parse("", &args) - }) + err := parse("", &args) + assert.Error(t, err) } func TestUnsupportedType(t *testing.T) { From 6a796e2c4131f734028186c023c32e08b5ef7758 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Tue, 30 Apr 2019 12:54:39 -0700 Subject: [PATCH 06/28] add first two subcommand tests --- subcommand_test.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 subcommand_test.go diff --git a/subcommand_test.go b/subcommand_test.go new file mode 100644 index 0000000..d17c604 --- /dev/null +++ b/subcommand_test.go @@ -0,0 +1,27 @@ +package arg + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// This file contains tests for parse.go but I decided to put them here +// since that file is getting large + +func TestSubcommandNotAStruct(t *testing.T) { + var args struct { + A string `arg:"subcommand"` + } + _, err := NewParser(Config{}, &args) + assert.Error(t, err) +} + +func TestPositionalAndSubcommandNotAllowed(t *testing.T) { + var args struct { + A string `arg:"positional"` + B struct{} `arg:"subcommand"` + } + _, err := NewParser(Config{}, &args) + assert.Error(t, err) +} From af12b7cfc22b056f6232bb25f5f4b7d35ca37e7e Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Tue, 30 Apr 2019 13:30:23 -0700 Subject: [PATCH 07/28] introduced path struct --- parse.go | 124 +++++++++++++++++++++++++++++---------------- subcommand_test.go | 22 +++++++- usage.go | 6 +-- 3 files changed, 102 insertions(+), 50 deletions(-) diff --git a/parse.go b/parse.go index 353b365..cb8be6d 100644 --- a/parse.go +++ b/parse.go @@ -13,10 +13,32 @@ import ( scalar "github.com/alexflint/go-scalar" ) +// path represents a sequence of steps to find the output location for an +// argument or subcommand in the final destination struct +type path struct { + root int // index of the destination struct + fields []string // sequence of struct field names to traverse +} + +// String gets a string representation of the given path +func (p path) String() string { + return "args." + strings.Join(p.fields, ".") +} + +// Child gets a new path representing a child of this path. +func (p path) Child(child string) path { + // copy the entire slice of fields to avoid possible slice overwrite + subfields := make([]string, len(p.fields)+1) + copy(subfields, append(p.fields, child)) + return path{ + root: p.root, + fields: subfields, + } +} + // spec represents a command line option type spec struct { - root int - path []string // sequence of field names + dest path typ reflect.Type long string short string @@ -32,6 +54,7 @@ type spec struct { // command represents a named subcommand, or the top-level command type command struct { name string + dest path specs []*spec subcommands []*command } @@ -153,11 +176,12 @@ func NewParser(config Config, dests ...interface{}) (*Parser, error) { panic(fmt.Sprintf("%s is not a pointer (did you forget an ampersand?)", t)) } - cmd, err := cmdFromStruct(name, t, nil, i) + cmd, err := cmdFromStruct(name, path{root: i}, t) if err != nil { return nil, err } p.cmd.specs = append(p.cmd.specs, cmd.specs...) + p.cmd.subcommands = append(p.cmd.subcommands, cmd.subcommands...) if dest, ok := dest.(Versioned); ok { p.version = dest.Version() @@ -170,20 +194,24 @@ func NewParser(config Config, dests ...interface{}) (*Parser, error) { return &p, nil } -func cmdFromStruct(name string, t reflect.Type, path []string, root int) (*command, error) { +func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { // commands can only be created from pointers to structs if t.Kind() != reflect.Ptr { - return nil, fmt.Errorf("subcommands must be pointers to structs but args.%s is a %s", - strings.Join(path, "."), t.Kind()) + return nil, fmt.Errorf("subcommands must be pointers to structs but %s is a %s", + dest, t.Kind()) } t = t.Elem() if t.Kind() != reflect.Struct { - return nil, fmt.Errorf("subcommands must be pointers to structs but args.%s is a pointer to %s", - strings.Join(path, "."), t.Kind()) + return nil, fmt.Errorf("subcommands must be pointers to structs but %s is a pointer to %s", + dest, t.Kind()) + } + + cmd := command{ + name: name, + dest: dest, } - var cmd command var errs []string walkFields(t, func(field reflect.StructField, t reflect.Type) bool { // Check for the ignore switch in the tag @@ -198,12 +226,9 @@ func cmdFromStruct(name string, t reflect.Type, path []string, root int) (*comma } // duplicate the entire path to avoid slice overwrites - subpath := make([]string, len(path)+1) - copy(subpath, append(path, field.Name)) - + subdest := dest.Child(field.Name) spec := spec{ - root: root, - path: subpath, + dest: subdest, long: strings.ToLower(field.Name), typ: field.Type, } @@ -213,19 +238,8 @@ func cmdFromStruct(name string, t reflect.Type, path []string, root int) (*comma spec.help = help } - // Check whether this field is supported. It's good to do this here rather than - // wait until ParseValue because it means that a program with invalid argument - // fields will always fail regardless of whether the arguments it received - // exercised those fields. - var parseable bool - parseable, spec.boolean, spec.multiple = canParse(field.Type) - if !parseable { - errs = append(errs, fmt.Sprintf("%s.%s: %s fields are not supported", - t.Name(), field.Name, field.Type.String())) - return false - } - // Look at the tag + var isSubcommand bool // tracks whether this field is a subcommand if tag != "" { for _, key := range strings.Split(tag, ",") { key = strings.TrimLeft(key, " ") @@ -269,20 +283,37 @@ func cmdFromStruct(name string, t reflect.Type, path []string, root int) (*comma cmdname = strings.ToLower(field.Name) } - subcmd, err := cmdFromStruct(cmdname, field.Type, subpath, root) + subcmd, err := cmdFromStruct(cmdname, subdest, field.Type) if err != nil { errs = append(errs, err.Error()) return false } cmd.subcommands = append(cmd.subcommands, subcmd) + isSubcommand = true + fmt.Println("found a subcommand") default: errs = append(errs, fmt.Sprintf("unrecognized tag '%s' on field %s", key, tag)) return false } } } - cmd.specs = append(cmd.specs, &spec) + + // Check whether this field is supported. It's good to do this here rather than + // wait until ParseValue because it means that a program with invalid argument + // fields will always fail regardless of whether the arguments it received + // exercised those fields. + if !isSubcommand { + cmd.specs = append(cmd.specs, &spec) + + var parseable bool + parseable, spec.boolean, spec.multiple = canParse(field.Type) + if !parseable { + errs = append(errs, fmt.Sprintf("%s.%s: %s fields are not supported", + t.Name(), field.Name, field.Type.String())) + return false + } + } // if this was an embedded field then we already returned true up above return false @@ -303,6 +334,8 @@ func cmdFromStruct(name string, t reflect.Type, path []string, root int) (*comma return nil, fmt.Errorf("%T cannot have both subcommands and positional arguments", t) } + fmt.Printf("parsed a command with %d subcommands\n", len(cmd.subcommands)) + return &cmd, nil } @@ -349,7 +382,7 @@ func (p *Parser) captureEnvVars(specs []*spec, wasPresent map[*spec]bool) error err, ) } - if err = setSlice(p.writable(spec), values, !spec.separate); err != nil { + if err = setSlice(p.writable(spec.dest), values, !spec.separate); err != nil { return fmt.Errorf( "error processing environment variable %s with multiple values: %v", spec.env, @@ -357,7 +390,7 @@ func (p *Parser) captureEnvVars(specs []*spec, wasPresent map[*spec]bool) error ) } } else { - if err := scalar.ParseValue(p.writable(spec), value); err != nil { + if err := scalar.ParseValue(p.writable(spec.dest), value); err != nil { return fmt.Errorf("error processing environment variable %s: %v", spec.env, err) } } @@ -400,6 +433,7 @@ func (p *Parser) process(args []string) error { if !isFlag(arg) || allpositional { // each subcommand can have either subcommands or positionals, but not both + fmt.Printf("processing %q, with %d subcommands", arg, len(curCmd.subcommands)) if len(curCmd.subcommands) == 0 { positionals = append(positionals, arg) continue @@ -454,7 +488,7 @@ func (p *Parser) process(args []string) error { } else { values = append(values, value) } - err := setSlice(p.writable(spec), values, !spec.separate) + err := setSlice(p.writable(spec.dest), values, !spec.separate) if err != nil { return fmt.Errorf("error processing %s: %v", arg, err) } @@ -479,7 +513,7 @@ func (p *Parser) process(args []string) error { i++ } - err := scalar.ParseValue(p.writable(spec), value) + err := scalar.ParseValue(p.writable(spec.dest), value) if err != nil { return fmt.Errorf("error processing %s: %v", arg, err) } @@ -495,13 +529,13 @@ func (p *Parser) process(args []string) error { } wasPresent[spec] = true if spec.multiple { - err := setSlice(p.writable(spec), positionals, true) + err := setSlice(p.writable(spec.dest), positionals, true) if err != nil { return fmt.Errorf("error processing %s: %v", spec.long, err) } positionals = nil } else { - err := scalar.ParseValue(p.writable(spec), positionals[0]) + err := scalar.ParseValue(p.writable(spec.dest), positionals[0]) if err != nil { return fmt.Errorf("error processing %s: %v", spec.long, err) } @@ -546,9 +580,9 @@ func isFlag(s string) bool { // readable returns a reflect.Value corresponding to the current value for the // given -func (p *Parser) readable(spec *spec) reflect.Value { - v := p.roots[spec.root] - for _, field := range spec.path { +func (p *Parser) readable(dest path) reflect.Value { + v := p.roots[dest.root] + for _, field := range dest.fields { if v.Kind() == reflect.Ptr { if v.IsNil() { return reflect.Value{} @@ -559,21 +593,21 @@ func (p *Parser) readable(spec *spec) reflect.Value { v = v.FieldByName(field) if !v.IsValid() { // it is appropriate to panic here because this can only happen due to - // an internal bug in this library (since we construct spec.path ourselves + // an internal bug in this library (since we construct the path ourselves // by reflecting on the same struct) panic(fmt.Errorf("error resolving path %v: %v has no field named %v", - spec.path, v.Type(), field)) + dest.fields, v.Type(), field)) } } return v } -// writable traverses the destination struct to find the destination to +// writable trav.patherses the destination struct to find the destination to // which the value of the given spec should be written. It fills in null // structs with pointers to the zero value for that struct. -func (p *Parser) writable(spec *spec) reflect.Value { - v := p.roots[spec.root] - for _, field := range spec.path { +func (p *Parser) writable(dest path) reflect.Value { + v := p.roots[dest.root] + for _, field := range dest.fields { if v.Kind() == reflect.Ptr { if v.IsNil() { v.Set(reflect.New(v.Type().Elem())) @@ -584,10 +618,10 @@ func (p *Parser) writable(spec *spec) reflect.Value { v = v.FieldByName(field) if !v.IsValid() { // it is appropriate to panic here because this can only happen due to - // an internal bug in this library (since we construct spec.path ourselves + // an internal bug in this library (since we construct the path ourselves // by reflecting on the same struct) panic(fmt.Errorf("error resolving path %v: %v has no field named %v", - spec.path, v.Type(), field)) + dest.fields, v.Type(), field)) } } return v diff --git a/subcommand_test.go b/subcommand_test.go index d17c604..02c7b54 100644 --- a/subcommand_test.go +++ b/subcommand_test.go @@ -4,12 +4,13 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // This file contains tests for parse.go but I decided to put them here // since that file is getting large -func TestSubcommandNotAStruct(t *testing.T) { +func TestSubcommandNotAPointer(t *testing.T) { var args struct { A string `arg:"subcommand"` } @@ -17,6 +18,14 @@ func TestSubcommandNotAStruct(t *testing.T) { assert.Error(t, err) } +func TestSubcommandNotAPointerToStruct(t *testing.T) { + var args struct { + A struct{} `arg:"subcommand"` + } + _, err := NewParser(Config{}, &args) + assert.Error(t, err) +} + func TestPositionalAndSubcommandNotAllowed(t *testing.T) { var args struct { A string `arg:"positional"` @@ -25,3 +34,14 @@ func TestPositionalAndSubcommandNotAllowed(t *testing.T) { _, err := NewParser(Config{}, &args) assert.Error(t, err) } + +func TestMinimalSubcommand(t *testing.T) { + type listCmd struct { + } + var args struct { + List *listCmd `arg:"subcommand"` + } + err := parse("list", &args) + require.NoError(t, err) + assert.NotNil(t, args.List) +} diff --git a/usage.go b/usage.go index 833046f..d73da71 100644 --- a/usage.go +++ b/usage.go @@ -115,14 +115,12 @@ func (p *Parser) WriteHelp(w io.Writer) { long: "help", short: "h", help: "display this help and exit", - root: -1, }) if p.version != "" { p.printOption(w, &spec{ boolean: true, long: "version", help: "display version and exit", - root: -1, }) } } @@ -143,8 +141,8 @@ func (p *Parser) printOption(w io.Writer, spec *spec) { } // If spec.dest is not the zero value then a default value has been added. var v reflect.Value - if spec.root >= 0 { - v = p.readable(spec) + if len(spec.dest.fields) > 0 { + v = p.readable(spec.dest) } if v.IsValid() { z := reflect.Zero(v.Type()) From a78c6ded26bda9fdce7f4f3bef07ae2e9acad1cf Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Tue, 30 Apr 2019 13:40:45 -0700 Subject: [PATCH 08/28] set subcommand structs to be struct pointers --- parse.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/parse.go b/parse.go index cb8be6d..63059fc 100644 --- a/parse.go +++ b/parse.go @@ -283,6 +283,7 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { cmdname = strings.ToLower(field.Name) } + // parse the subcommand recursively subcmd, err := cmdFromStruct(cmdname, subdest, field.Type) if err != nil { errs = append(errs, err.Error()) @@ -445,6 +446,10 @@ func (p *Parser) process(args []string) error { return fmt.Errorf("invalid subcommand: %s", arg) } + // instantiate the field to point to a new struct + v := p.writable(subcmd.dest) + v.Set(reflect.New(v.Type().Elem())) // we already checked that all subcommands are struct pointers + // add the new options to the set of allowed options specs = append(specs, subcmd.specs...) From 39decf197fbf257081eee533974412140d7364e2 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Tue, 30 Apr 2019 13:49:55 -0700 Subject: [PATCH 09/28] add several subcommand unittests --- parse.go | 4 -- subcommand_test.go | 115 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 4 deletions(-) diff --git a/parse.go b/parse.go index 63059fc..2a3442a 100644 --- a/parse.go +++ b/parse.go @@ -292,7 +292,6 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { cmd.subcommands = append(cmd.subcommands, subcmd) isSubcommand = true - fmt.Println("found a subcommand") default: errs = append(errs, fmt.Sprintf("unrecognized tag '%s' on field %s", key, tag)) return false @@ -335,8 +334,6 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { return nil, fmt.Errorf("%T cannot have both subcommands and positional arguments", t) } - fmt.Printf("parsed a command with %d subcommands\n", len(cmd.subcommands)) - return &cmd, nil } @@ -434,7 +431,6 @@ func (p *Parser) process(args []string) error { if !isFlag(arg) || allpositional { // each subcommand can have either subcommands or positionals, but not both - fmt.Printf("processing %q, with %d subcommands", arg, len(curCmd.subcommands)) if len(curCmd.subcommands) == 0 { positionals = append(positionals, arg) continue diff --git a/subcommand_test.go b/subcommand_test.go index 02c7b54..6df3147 100644 --- a/subcommand_test.go +++ b/subcommand_test.go @@ -45,3 +45,118 @@ func TestMinimalSubcommand(t *testing.T) { require.NoError(t, err) assert.NotNil(t, args.List) } + +func TestNamedSubcommand(t *testing.T) { + type listCmd struct { + } + var args struct { + List *listCmd `arg:"subcommand:ls"` + } + err := parse("ls", &args) + require.NoError(t, err) + assert.NotNil(t, args.List) +} + +func TestEmptySubcommand(t *testing.T) { + type listCmd struct { + } + var args struct { + List *listCmd `arg:"subcommand"` + } + err := parse("", &args) + require.NoError(t, err) + assert.Nil(t, args.List) +} + +func TestTwoSubcommands(t *testing.T) { + type getCmd struct { + } + type listCmd struct { + } + var args struct { + Get *getCmd `arg:"subcommand"` + List *listCmd `arg:"subcommand"` + } + err := parse("list", &args) + require.NoError(t, err) + assert.Nil(t, args.Get) + assert.NotNil(t, args.List) +} + +func TestSubcommandsWithOptions(t *testing.T) { + type getCmd struct { + Name string + } + type listCmd struct { + Limit int + } + type cmd struct { + Verbose bool + Get *getCmd `arg:"subcommand"` + List *listCmd `arg:"subcommand"` + } + + { + var args cmd + err := parse("list", &args) + require.NoError(t, err) + assert.Nil(t, args.Get) + assert.NotNil(t, args.List) + } + + { + var args cmd + err := parse("list --limit 3", &args) + require.NoError(t, err) + assert.Nil(t, args.Get) + assert.NotNil(t, args.List) + assert.Equal(t, args.List.Limit, 3) + } + + { + var args cmd + err := parse("list --limit 3 --verbose", &args) + require.NoError(t, err) + assert.Nil(t, args.Get) + assert.NotNil(t, args.List) + assert.Equal(t, args.List.Limit, 3) + assert.True(t, args.Verbose) + } + + { + var args cmd + err := parse("list --verbose --limit 3", &args) + require.NoError(t, err) + assert.Nil(t, args.Get) + assert.NotNil(t, args.List) + assert.Equal(t, args.List.Limit, 3) + assert.True(t, args.Verbose) + } + + { + var args cmd + err := parse("--verbose list --limit 3", &args) + require.NoError(t, err) + assert.Nil(t, args.Get) + assert.NotNil(t, args.List) + assert.Equal(t, args.List.Limit, 3) + assert.True(t, args.Verbose) + } + + { + var args cmd + err := parse("get", &args) + require.NoError(t, err) + assert.NotNil(t, args.Get) + assert.Nil(t, args.List) + } + + { + var args cmd + err := parse("get --name test", &args) + require.NoError(t, err) + assert.NotNil(t, args.Get) + assert.Nil(t, args.List) + assert.Equal(t, args.Get.Name, "test") + } +} From 15b9bcfbb4af9e491c1aeab36d71f60f60c6e7ea Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Tue, 30 Apr 2019 13:53:14 -0700 Subject: [PATCH 10/28] add several subcommand unittests --- subcommand_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/subcommand_test.go b/subcommand_test.go index 6df3147..25689a4 100644 --- a/subcommand_test.go +++ b/subcommand_test.go @@ -160,3 +160,11 @@ func TestSubcommandsWithOptions(t *testing.T) { assert.Equal(t, args.Get.Name, "test") } } + +func TestNestedSubcommands(t *testing.T) { + // tree of subcommands +} + +func TestSubcommandsWithPositionals(t *testing.T) { + // subcommands with positional arguments +} From 5b649de04338e2ce398b9b1de3dc5f16144bdcd4 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Thu, 2 May 2019 09:16:33 -0700 Subject: [PATCH 11/28] test no such subcommand --- subcommand_test.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/subcommand_test.go b/subcommand_test.go index 25689a4..783ff9c 100644 --- a/subcommand_test.go +++ b/subcommand_test.go @@ -28,8 +28,8 @@ func TestSubcommandNotAPointerToStruct(t *testing.T) { func TestPositionalAndSubcommandNotAllowed(t *testing.T) { var args struct { - A string `arg:"positional"` - B struct{} `arg:"subcommand"` + A string `arg:"positional"` + B *struct{} `arg:"subcommand"` } _, err := NewParser(Config{}, &args) assert.Error(t, err) @@ -46,6 +46,16 @@ func TestMinimalSubcommand(t *testing.T) { assert.NotNil(t, args.List) } +func TestNoSuchSubcommand(t *testing.T) { + type listCmd struct { + } + var args struct { + List *listCmd `arg:"subcommand"` + } + err := parse("invalid", &args) + assert.Error(t, err) +} + func TestNamedSubcommand(t *testing.T) { type listCmd struct { } From 87be2d97907952e87203bd3a4b09cfbe49ce028d Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Thu, 2 May 2019 09:28:17 -0700 Subject: [PATCH 12/28] add unittests for canParse --- parse.go | 55 ------------------------------------------ reflect.go | 64 +++++++++++++++++++++++++++++++++++++++++++++++++ reflect_test.go | 40 +++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 55 deletions(-) create mode 100644 reflect.go create mode 100644 reflect_test.go diff --git a/parse.go b/parse.go index 2a3442a..0633bb6 100644 --- a/parse.go +++ b/parse.go @@ -1,7 +1,6 @@ package arg import ( - "encoding" "encoding/csv" "errors" "fmt" @@ -659,60 +658,6 @@ func setSlice(dest reflect.Value, values []string, trunc bool) error { return nil } -// canParse returns true if the type can be parsed from a string -func canParse(t reflect.Type) (parseable, boolean, multiple bool) { - parseable = scalar.CanParse(t) - boolean = isBoolean(t) - if parseable { - return - } - - // Look inside pointer types - if t.Kind() == reflect.Ptr { - t = t.Elem() - } - // Look inside slice types - if t.Kind() == reflect.Slice { - multiple = true - t = t.Elem() - } - - parseable = scalar.CanParse(t) - boolean = isBoolean(t) - if parseable { - return - } - - // Look inside pointer types (again, in case of []*Type) - if t.Kind() == reflect.Ptr { - t = t.Elem() - } - - parseable = scalar.CanParse(t) - boolean = isBoolean(t) - if parseable { - return - } - - return false, false, false -} - -var textUnmarshalerType = reflect.TypeOf([]encoding.TextUnmarshaler{}).Elem() - -// isBoolean returns true if the type can be parsed from a single string -func isBoolean(t reflect.Type) bool { - switch { - case t.Implements(textUnmarshalerType): - return false - case t.Kind() == reflect.Bool: - return true - case t.Kind() == reflect.Ptr && t.Elem().Kind() == reflect.Bool: - return true - default: - return false - } -} - // findOption finds an option from its name, or returns null if no spec is found func findOption(specs []*spec, name string) *spec { for _, spec := range specs { diff --git a/reflect.go b/reflect.go new file mode 100644 index 0000000..32efe04 --- /dev/null +++ b/reflect.go @@ -0,0 +1,64 @@ +package arg + +import ( + "encoding" + "reflect" + + scalar "github.com/alexflint/go-scalar" +) + +var textUnmarshalerType = reflect.TypeOf([]encoding.TextUnmarshaler{}).Elem() + +// This file contains miscellaneous reflection utilities + +// canParse returns true if the type can be parsed from a string +func canParse(t reflect.Type) (parseable, boolean, multiple bool) { + parseable = scalar.CanParse(t) + boolean = isBoolean(t) + if parseable { + return + } + + // Look inside pointer types + if t.Kind() == reflect.Ptr { + t = t.Elem() + } + // Look inside slice types + if t.Kind() == reflect.Slice { + multiple = true + t = t.Elem() + } + + parseable = scalar.CanParse(t) + boolean = isBoolean(t) + if parseable { + return + } + + // Look inside pointer types (again, in case of []*Type) + if t.Kind() == reflect.Ptr { + t = t.Elem() + } + + parseable = scalar.CanParse(t) + boolean = isBoolean(t) + if parseable { + return + } + + return false, false, false +} + +// isBoolean returns true if the type can be parsed from a single string +func isBoolean(t reflect.Type) bool { + switch { + case t.Implements(textUnmarshalerType): + return false + case t.Kind() == reflect.Bool: + return true + case t.Kind() == reflect.Ptr && t.Elem().Kind() == reflect.Bool: + return true + default: + return false + } +} diff --git a/reflect_test.go b/reflect_test.go new file mode 100644 index 0000000..0f285f7 --- /dev/null +++ b/reflect_test.go @@ -0,0 +1,40 @@ +package arg + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/assert" +) + +func assertCanParse(t *testing.T, typ reflect.Type, parseable, boolean, multiple bool) { + p, b, m := canParse(typ) + assert.Equal(t, parseable, p, "expected %v to have parseable=%v but was %v", typ, parseable, p) + assert.Equal(t, boolean, b, "expected %v to have boolean=%v but was %v", typ, boolean, b) + assert.Equal(t, multiple, m, "expected %v to have multiple=%v but was %v", typ, multiple, m) +} + +func TestCanParse(t *testing.T) { + var b bool + var i int + var s string + var f float64 + var bs []bool + var is []int + + assertCanParse(t, reflect.TypeOf(b), true, true, false) + assertCanParse(t, reflect.TypeOf(i), true, false, false) + assertCanParse(t, reflect.TypeOf(s), true, false, false) + assertCanParse(t, reflect.TypeOf(f), true, false, false) + + assertCanParse(t, reflect.TypeOf(&b), true, true, false) + assertCanParse(t, reflect.TypeOf(&s), true, false, false) + assertCanParse(t, reflect.TypeOf(&i), true, false, false) + assertCanParse(t, reflect.TypeOf(&f), true, false, false) + + assertCanParse(t, reflect.TypeOf(bs), true, true, true) + assertCanParse(t, reflect.TypeOf(&bs), true, true, true) + + assertCanParse(t, reflect.TypeOf(is), true, false, true) + assertCanParse(t, reflect.TypeOf(&is), true, false, true) +} From a15b6ad67063562c974b346ca71dfceea045b0e4 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Thu, 2 May 2019 09:32:23 -0700 Subject: [PATCH 13/28] add test for canParse with TextUnmarshaler --- reflect.go | 2 -- reflect_test.go | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/reflect.go b/reflect.go index 32efe04..e113583 100644 --- a/reflect.go +++ b/reflect.go @@ -9,8 +9,6 @@ import ( var textUnmarshalerType = reflect.TypeOf([]encoding.TextUnmarshaler{}).Elem() -// This file contains miscellaneous reflection utilities - // canParse returns true if the type can be parsed from a string func canParse(t reflect.Type) (parseable, boolean, multiple bool) { parseable = scalar.CanParse(t) diff --git a/reflect_test.go b/reflect_test.go index 0f285f7..47e68b5 100644 --- a/reflect_test.go +++ b/reflect_test.go @@ -38,3 +38,18 @@ func TestCanParse(t *testing.T) { assertCanParse(t, reflect.TypeOf(is), true, false, true) assertCanParse(t, reflect.TypeOf(&is), true, false, true) } + +type implementsTextUnmarshaler struct{} + +func (*implementsTextUnmarshaler) UnmarshalText(text []byte) error { + return nil +} + +func TestCanParseTextUnmarshaler(t *testing.T) { + var u implementsTextUnmarshaler + var su []implementsTextUnmarshaler + assertCanParse(t, reflect.TypeOf(u), true, false, false) + assertCanParse(t, reflect.TypeOf(&u), true, false, false) + assertCanParse(t, reflect.TypeOf(su), true, false, true) + assertCanParse(t, reflect.TypeOf(&su), true, false, true) +} From f2f7bdbbd7f2dba359ce7e8bd8cd60cd54ce20e6 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Thu, 2 May 2019 09:39:12 -0700 Subject: [PATCH 14/28] add test case for missing value in middle of argument string --- parse_test.go | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/parse_test.go b/parse_test.go index 94cf21a..de36d67 100644 --- a/parse_test.go +++ b/parse_test.go @@ -371,7 +371,7 @@ func TestNonsenseKey(t *testing.T) { assert.Error(t, err) } -func TestMissingValue(t *testing.T) { +func TestMissingValueAtEnd(t *testing.T) { var args struct { Foo string } @@ -379,6 +379,24 @@ func TestMissingValue(t *testing.T) { assert.Error(t, err) } +func TestMissingValueInMIddle(t *testing.T) { + var args struct { + Foo string + Bar string + } + err := parse("--foo --bar=abc", &args) + assert.Error(t, err) +} + +func TestNegativeValue(t *testing.T) { + var args struct { + Foo int + } + err := parse("--foo -123", &args) + require.NoError(t, err) + assert.Equal(t, -123, args.Foo) +} + func TestInvalidInt(t *testing.T) { var args struct { Foo int From c8c61cf8bbc4849c252c52b61381c072fcde66a6 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Thu, 2 May 2019 09:44:48 -0700 Subject: [PATCH 15/28] add test for case where environment var is not present --- parse_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/parse_test.go b/parse_test.go index de36d67..c44fe00 100644 --- a/parse_test.go +++ b/parse_test.go @@ -557,6 +557,15 @@ func TestEnvironmentVariable(t *testing.T) { assert.Equal(t, "bar", args.Foo) } +func TestEnvironmentVariableNotPresent(t *testing.T) { + var args struct { + NotPresent string `arg:"env"` + } + os.Args = []string{"example"} + MustParse(&args) + assert.Equal(t, "", args.NotPresent) +} + func TestEnvironmentVariableOverrideName(t *testing.T) { var args struct { Foo string `arg:"env:BAZ"` From 93fcb0e87d06c7080c6542b408dcbad0ce113427 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Thu, 2 May 2019 09:46:11 -0700 Subject: [PATCH 16/28] use backticks rather than backslashes in string literal --- parse_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parse_test.go b/parse_test.go index c44fe00..3482e6e 100644 --- a/parse_test.go +++ b/parse_test.go @@ -610,7 +610,7 @@ func TestEnvironmentVariableSliceArgumentString(t *testing.T) { var args struct { Foo []string `arg:"env"` } - setenv(t, "FOO", "bar,\"baz, qux\"") + setenv(t, "FOO", `bar,"baz, qux"`) MustParse(&args) assert.Equal(t, []string{"bar", "baz, qux"}, args.Foo) } From a68d6000b69b0f4769ee1d72ff61a15dd1589383 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Thu, 2 May 2019 09:47:39 -0700 Subject: [PATCH 17/28] test use of --version --- parse_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/parse_test.go b/parse_test.go index 3482e6e..00b0875 100644 --- a/parse_test.go +++ b/parse_test.go @@ -1011,3 +1011,10 @@ func TestReuseParser(t *testing.T) { err = p.Parse([]string{}) assert.Error(t, err) } + +func TestVersion(t *testing.T) { + var args struct{} + err := parse("--version", &args) + assert.Equal(t, ErrVersion, err) + +} From e55b361498fd8d1c697984e095d017fdb679b60d Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Thu, 2 May 2019 09:50:44 -0700 Subject: [PATCH 18/28] fix error message --- parse.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/parse.go b/parse.go index 0633bb6..d4a6334 100644 --- a/parse.go +++ b/parse.go @@ -21,6 +21,9 @@ type path struct { // String gets a string representation of the given path func (p path) String() string { + if len(p.fields) == 0 { + return "args" + } return "args." + strings.Join(p.fields, ".") } @@ -330,7 +333,7 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { } } if hasPositional && len(cmd.subcommands) > 0 { - return nil, fmt.Errorf("%T cannot have both subcommands and positional arguments", t) + return nil, fmt.Errorf("%s cannot have both subcommands and positional arguments", dest) } return &cmd, nil From c6473c4586765df1407072d27a0db2f95e35d7ce Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Fri, 3 May 2019 11:21:34 -0700 Subject: [PATCH 19/28] add tests for nested subcommands and subcommands with positionals --- subcommand_test.go | 163 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 161 insertions(+), 2 deletions(-) diff --git a/subcommand_test.go b/subcommand_test.go index 783ff9c..fdc3ff4 100644 --- a/subcommand_test.go +++ b/subcommand_test.go @@ -172,9 +172,168 @@ func TestSubcommandsWithOptions(t *testing.T) { } func TestNestedSubcommands(t *testing.T) { - // tree of subcommands + type child struct{} + type parent struct { + Child *child `arg:"subcommand"` + } + type grandparent struct { + Parent *parent `arg:"subcommand"` + } + type root struct { + Grandparent *grandparent `arg:"subcommand"` + } + + { + var args root + err := parse("grandparent parent child", &args) + require.NoError(t, err) + require.NotNil(t, args.Grandparent) + require.NotNil(t, args.Grandparent.Parent) + require.NotNil(t, args.Grandparent.Parent.Child) + } + + { + var args root + err := parse("grandparent parent", &args) + require.NoError(t, err) + require.NotNil(t, args.Grandparent) + require.NotNil(t, args.Grandparent.Parent) + require.Nil(t, args.Grandparent.Parent.Child) + } + + { + var args root + err := parse("grandparent", &args) + require.NoError(t, err) + require.NotNil(t, args.Grandparent) + require.Nil(t, args.Grandparent.Parent) + } + + { + var args root + err := parse("", &args) + require.NoError(t, err) + require.Nil(t, args.Grandparent) + } } func TestSubcommandsWithPositionals(t *testing.T) { - // subcommands with positional arguments + type listCmd struct { + Pattern string `arg:"positional"` + } + type cmd struct { + Format string + List *listCmd `arg:"subcommand"` + } + + { + var args cmd + err := parse("list", &args) + require.NoError(t, err) + assert.NotNil(t, args.List) + assert.Equal(t, "", args.List.Pattern) + } + + { + var args cmd + err := parse("list --format json", &args) + require.NoError(t, err) + assert.NotNil(t, args.List) + assert.Equal(t, "", args.List.Pattern) + assert.Equal(t, "json", args.Format) + } + + { + var args cmd + err := parse("list somepattern", &args) + require.NoError(t, err) + assert.NotNil(t, args.List) + assert.Equal(t, "somepattern", args.List.Pattern) + } + + { + var args cmd + err := parse("list somepattern --format json", &args) + require.NoError(t, err) + assert.NotNil(t, args.List) + assert.Equal(t, "somepattern", args.List.Pattern) + assert.Equal(t, "json", args.Format) + } + + { + var args cmd + err := parse("list --format json somepattern", &args) + require.NoError(t, err) + assert.NotNil(t, args.List) + assert.Equal(t, "somepattern", args.List.Pattern) + assert.Equal(t, "json", args.Format) + } + + { + var args cmd + err := parse("--format json list somepattern", &args) + require.NoError(t, err) + assert.NotNil(t, args.List) + assert.Equal(t, "somepattern", args.List.Pattern) + assert.Equal(t, "json", args.Format) + } + + { + var args cmd + err := parse("--format json", &args) + require.NoError(t, err) + assert.Nil(t, args.List) + assert.Equal(t, "json", args.Format) + } +} +func TestSubcommandsWithMultiplePositionals(t *testing.T) { + type getCmd struct { + Items []string `arg:"positional"` + } + type cmd struct { + Limit int + Get *getCmd `arg:"subcommand"` + } + + { + var args cmd + err := parse("get", &args) + require.NoError(t, err) + assert.NotNil(t, args.Get) + assert.Empty(t, args.Get.Items) + } + + { + var args cmd + err := parse("get --limit 5", &args) + require.NoError(t, err) + assert.NotNil(t, args.Get) + assert.Empty(t, args.Get.Items) + assert.Equal(t, 5, args.Limit) + } + + { + var args cmd + err := parse("get item1", &args) + require.NoError(t, err) + assert.NotNil(t, args.Get) + assert.Equal(t, []string{"item1"}, args.Get.Items) + } + + { + var args cmd + err := parse("get item1 item2 item3", &args) + require.NoError(t, err) + assert.NotNil(t, args.Get) + assert.Equal(t, []string{"item1", "item2", "item3"}, args.Get.Items) + } + + { + var args cmd + err := parse("get item1 --limit 5 item2", &args) + require.NoError(t, err) + assert.NotNil(t, args.Get) + assert.Equal(t, []string{"item1", "item2"}, args.Get.Items) + assert.Equal(t, 5, args.Limit) + } } From 15bf383f1d5db9bf362029529f3c83f092e2d00f Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Fri, 3 May 2019 15:02:10 -0700 Subject: [PATCH 20/28] add subcommands to usage string --- example_test.go | 37 +++++++++++++++++++++++++++++ parse.go | 3 +++ usage.go | 63 +++++++++++++++++++++++++++++-------------------- 3 files changed, 78 insertions(+), 25 deletions(-) diff --git a/example_test.go b/example_test.go index 72807a7..eb8d315 100644 --- a/example_test.go +++ b/example_test.go @@ -135,3 +135,40 @@ func Example_usageString() { // optimization level // --help, -h display this help and exit } + +// This example shows the usage string generated by go-arg +func Example_usageStringWithSubcommand() { + // These are the args you would pass in on the command line + os.Args = split("./example --help") + + type getCmd struct { + Item string `arg:"positional" help:"item to fetch"` + } + + type listCmd struct { + Format string `help:"output format"` + Limit int + } + + var args struct { + Verbose bool + Get *getCmd `arg:"subcommand" help:"fetch an item and print it"` + List *listCmd `arg:"subcommand" help:"list available items"` + } + + // This is only necessary when running inside golang's runnable example harness + osExit = func(int) {} + + MustParse(&args) + + // output: + // Usage: example [--verbose] + // + // Options: + // --verbose + // --help, -h display this help and exit + // + // Commands: + // get fetch an item and print it + // list list available items +} diff --git a/parse.go b/parse.go index d06b299..3a48880 100644 --- a/parse.go +++ b/parse.go @@ -59,6 +59,7 @@ type spec struct { // command represents a named subcommand, or the top-level command type command struct { name string + help string dest path specs []*spec subcommands []*command @@ -296,6 +297,8 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { return false } + subcmd.help = field.Tag.Get("help") + cmd.subcommands = append(cmd.subcommands, subcmd) isSubcommand = true default: diff --git a/usage.go b/usage.go index d73da71..42c564b 100644 --- a/usage.go +++ b/usage.go @@ -69,6 +69,23 @@ func (p *Parser) WriteUsage(w io.Writer) { fmt.Fprint(w, "\n") } +func printTwoCols(w io.Writer, left, help string, defaultVal *string) { + lhs := " " + left + fmt.Fprint(w, lhs) + if help != "" { + if len(lhs)+2 < colWidth { + fmt.Fprint(w, strings.Repeat(" ", colWidth-len(lhs))) + } else { + fmt.Fprint(w, "\n"+strings.Repeat(" ", colWidth)) + } + fmt.Fprint(w, help) + } + if defaultVal != nil { + fmt.Fprintf(w, " [default: %s]", *defaultVal) + } + fmt.Fprint(w, "\n") +} + // WriteHelp writes the usage string followed by the full help string for each option func (p *Parser) WriteHelp(w io.Writer) { var positionals, options []*spec @@ -89,17 +106,7 @@ func (p *Parser) WriteHelp(w io.Writer) { if len(positionals) > 0 { fmt.Fprint(w, "\nPositional arguments:\n") for _, spec := range positionals { - left := " " + strings.ToUpper(spec.long) - fmt.Fprint(w, left) - if spec.help != "" { - if len(left)+2 < colWidth { - fmt.Fprint(w, strings.Repeat(" ", colWidth-len(left))) - } else { - fmt.Fprint(w, "\n"+strings.Repeat(" ", colWidth)) - } - fmt.Fprint(w, spec.help) - } - fmt.Fprint(w, "\n") + printTwoCols(w, strings.ToUpper(spec.long), spec.help, nil) } } @@ -123,42 +130,44 @@ func (p *Parser) WriteHelp(w io.Writer) { help: "display version and exit", }) } + + // write the list of subcommands + if len(p.cmd.subcommands) > 0 { + fmt.Fprint(w, "\nCommands:\n") + for _, subcmd := range p.cmd.subcommands { + printTwoCols(w, subcmd.name, subcmd.help, nil) + } + } } func (p *Parser) printOption(w io.Writer, spec *spec) { - left := " " + synopsis(spec, "--"+spec.long) + left := synopsis(spec, "--"+spec.long) if spec.short != "" { left += ", " + synopsis(spec, "-"+spec.short) } - fmt.Fprint(w, left) - if spec.help != "" { - if len(left)+2 < colWidth { - fmt.Fprint(w, strings.Repeat(" ", colWidth-len(left))) - } else { - fmt.Fprint(w, "\n"+strings.Repeat(" ", colWidth)) - } - fmt.Fprint(w, spec.help) - } + // If spec.dest is not the zero value then a default value has been added. var v reflect.Value if len(spec.dest.fields) > 0 { v = p.readable(spec.dest) } + + var defaultVal *string if v.IsValid() { z := reflect.Zero(v.Type()) if (v.Type().Comparable() && z.Type().Comparable() && v.Interface() != z.Interface()) || v.Kind() == reflect.Slice && !v.IsNil() { if scalar, ok := v.Interface().(encoding.TextMarshaler); ok { if value, err := scalar.MarshalText(); err != nil { - fmt.Fprintf(w, " [default: error: %v]", err) + defaultVal = ptrTo(fmt.Sprintf("error: %v", err)) } else { - fmt.Fprintf(w, " [default: %v]", string(value)) + defaultVal = ptrTo(fmt.Sprintf("%v", string(value))) } } else { - fmt.Fprintf(w, " [default: %v]", v) + defaultVal = ptrTo(fmt.Sprintf("%v", v)) } } } - fmt.Fprint(w, "\n") + printTwoCols(w, left, spec.help, defaultVal) } func synopsis(spec *spec, form string) string { @@ -167,3 +176,7 @@ func synopsis(spec *spec, form string) string { } return form + " " + strings.ToUpper(spec.long) } + +func ptrTo(s string) *string { + return &s +} From b83047068d06bf682cdcaad3a090ff289df827b1 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Fri, 3 May 2019 15:49:44 -0700 Subject: [PATCH 21/28] print help and usage at subcommand level if necessary --- example_test.go | 89 +++++++++++++++++++++++++++++++++++++++++++++++-- parse.go | 44 +++++++++++++++--------- usage.go | 48 +++++++++++++++++++++----- 3 files changed, 154 insertions(+), 27 deletions(-) diff --git a/example_test.go b/example_test.go index eb8d315..b58085f 100644 --- a/example_test.go +++ b/example_test.go @@ -104,7 +104,7 @@ func Example_multipleMixed() { } // This example shows the usage string generated by go-arg -func Example_usageString() { +func Example_helpText() { // These are the args you would pass in on the command line os.Args = split("./example --help") @@ -136,8 +136,8 @@ func Example_usageString() { // --help, -h display this help and exit } -// This example shows the usage string generated by go-arg -func Example_usageStringWithSubcommand() { +// This example shows the usage string generated by go-arg when using subcommands +func Example_helpTextWithSubcommand() { // These are the args you would pass in on the command line os.Args = split("./example --help") @@ -172,3 +172,86 @@ func Example_usageStringWithSubcommand() { // get fetch an item and print it // list list available items } + +// This example shows the usage string generated by go-arg when using subcommands +func Example_helpTextForSubcommand() { + // These are the args you would pass in on the command line + os.Args = split("./example get --help") + + type getCmd struct { + Item string `arg:"positional" help:"item to fetch"` + } + + type listCmd struct { + Format string `help:"output format"` + Limit int + } + + var args struct { + Verbose bool + Get *getCmd `arg:"subcommand" help:"fetch an item and print it"` + List *listCmd `arg:"subcommand" help:"list available items"` + } + + // This is only necessary when running inside golang's runnable example harness + osExit = func(int) {} + + MustParse(&args) + + // output: + // Usage: example get ITEM + // + // Positional arguments: + // ITEM item to fetch + // + // Options: + // --help, -h display this help and exit +} + +// This example shows the error string generated by go-arg when an invalid option is provided +func Example_errorText() { + // These are the args you would pass in on the command line + os.Args = split("./example --optimize INVALID") + + var args struct { + Input string `arg:"positional"` + Output []string `arg:"positional"` + Verbose bool `arg:"-v" help:"verbosity level"` + Dataset string `help:"dataset to use"` + Optimize int `arg:"-O,help:optimization level"` + } + + // This is only necessary when running inside golang's runnable example harness + osExit = func(int) {} + stderr = os.Stdout + + MustParse(&args) + + // output: + // Usage: example [--verbose] [--dataset DATASET] [--optimize OPTIMIZE] INPUT [OUTPUT [OUTPUT ...]] + // error: error processing --optimize: strconv.ParseInt: parsing "INVALID": invalid syntax +} + +// This example shows the error string generated by go-arg when an invalid option is provided +func Example_errorTextForSubcommand() { + // These are the args you would pass in on the command line + os.Args = split("./example get --count INVALID") + + type getCmd struct { + Count int + } + + var args struct { + Get *getCmd `arg:"subcommand"` + } + + // This is only necessary when running inside golang's runnable example harness + osExit = func(int) {} + stderr = os.Stdout + + MustParse(&args) + + // output: + // Usage: example get [--count COUNT] + // error: error processing --count: strconv.ParseInt: parsing "INVALID": invalid syntax +} diff --git a/parse.go b/parse.go index 3a48880..e0de705 100644 --- a/parse.go +++ b/parse.go @@ -63,6 +63,7 @@ type command struct { dest path specs []*spec subcommands []*command + parent *command } // ErrHelp indicates that -h or --help were provided @@ -77,18 +78,19 @@ func MustParse(dest ...interface{}) *Parser { if err != nil { fmt.Println(err) osExit(-1) + return nil // just in case osExit was monkey-patched } err = p.Parse(flags()) switch { case err == ErrHelp: - p.WriteHelp(os.Stdout) + p.writeHelpForCommand(os.Stdout, p.lastCmd) osExit(0) case err == ErrVersion: fmt.Println(p.version) osExit(0) case err != nil: - p.Fail(err.Error()) + p.failWithCommand(err.Error(), p.lastCmd) } return p @@ -123,6 +125,9 @@ type Parser struct { config Config version string description string + + // the following fields change curing processing of command line arguments + lastCmd *command } // Versioned is the interface that the destination struct should implement to @@ -297,6 +302,7 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { return false } + subcmd.parent = &cmd subcmd.help = field.Tag.Get("help") cmd.subcommands = append(cmd.subcommands, subcmd) @@ -349,21 +355,19 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { // Parse processes the given command line option, storing the results in the field // of the structs from which NewParser was constructed func (p *Parser) Parse(args []string) error { - // If -h or --help were specified then print usage - for _, arg := range args { - if arg == "-h" || arg == "--help" { - return ErrHelp - } - if arg == "--version" { - return ErrVersion - } - if arg == "--" { - break + err := p.process(args) + if err != nil { + // If -h or --help were specified then make sure help text supercedes other errors + for _, arg := range args { + if arg == "-h" || arg == "--help" { + return ErrHelp + } + if arg == "--" { + break + } } } - - // Process all command line arguments - return p.process(args) + return err } // process environment vars for the given arguments @@ -415,6 +419,7 @@ func (p *Parser) process(args []string) error { // union of specs for the chain of subcommands encountered so far curCmd := p.cmd + p.lastCmd = curCmd // make a copy of the specs because we will add to this list each time we expand a subcommand specs := make([]*spec, len(curCmd.specs)) @@ -465,9 +470,18 @@ func (p *Parser) process(args []string) error { } curCmd = subcmd + p.lastCmd = curCmd continue } + // check for special --help and --version flags + switch arg { + case "-h", "--help": + return ErrHelp + case "--version": + return ErrVersion + } + // check for an equals sign, as in "--foo=bar" var value string opt := strings.TrimLeft(arg, "-") diff --git a/usage.go b/usage.go index 42c564b..69e4e62 100644 --- a/usage.go +++ b/usage.go @@ -12,17 +12,30 @@ import ( // the width of the left column const colWidth = 25 +// to allow monkey patching in tests +var stderr = os.Stderr + // Fail prints usage information to stderr and exits with non-zero status func (p *Parser) Fail(msg string) { - p.WriteUsage(os.Stderr) - fmt.Fprintln(os.Stderr, "error:", msg) - os.Exit(-1) + p.failWithCommand(msg, p.cmd) +} + +// failWithCommand prints usage information for the given subcommand to stderr and exits with non-zero status +func (p *Parser) failWithCommand(msg string, cmd *command) { + p.writeUsageForCommand(stderr, cmd) + fmt.Fprintln(stderr, "error:", msg) + osExit(-1) } // WriteUsage writes usage information to the given writer func (p *Parser) WriteUsage(w io.Writer) { + p.writeUsageForCommand(w, p.cmd) +} + +// writeUsageForCommand writes usage information for the given subcommand +func (p *Parser) writeUsageForCommand(w io.Writer, cmd *command) { var positionals, options []*spec - for _, spec := range p.cmd.specs { + for _, spec := range cmd.specs { if spec.positional { positionals = append(positionals, spec) } else { @@ -34,7 +47,19 @@ func (p *Parser) WriteUsage(w io.Writer) { fmt.Fprintln(w, p.version) } - fmt.Fprintf(w, "Usage: %s", p.cmd.name) + // make a list of ancestor commands so that we print with full context + var ancestors []string + ancestor := cmd + for ancestor != nil { + ancestors = append(ancestors, ancestor.name) + ancestor = ancestor.parent + } + + // print the beginning of the usage string + fmt.Fprintf(w, "Usage:") + for i := len(ancestors) - 1; i >= 0; i-- { + fmt.Fprint(w, " "+ancestors[i]) + } // write the option component of the usage message for _, spec := range options { @@ -88,8 +113,13 @@ func printTwoCols(w io.Writer, left, help string, defaultVal *string) { // WriteHelp writes the usage string followed by the full help string for each option func (p *Parser) WriteHelp(w io.Writer) { + p.writeHelpForCommand(w, p.cmd) +} + +// writeHelp writes the usage string for the given subcommand +func (p *Parser) writeHelpForCommand(w io.Writer, cmd *command) { var positionals, options []*spec - for _, spec := range p.cmd.specs { + for _, spec := range cmd.specs { if spec.positional { positionals = append(positionals, spec) } else { @@ -100,7 +130,7 @@ func (p *Parser) WriteHelp(w io.Writer) { if p.description != "" { fmt.Fprintln(w, p.description) } - p.WriteUsage(w) + p.writeUsageForCommand(w, cmd) // write the list of positionals if len(positionals) > 0 { @@ -132,9 +162,9 @@ func (p *Parser) WriteHelp(w io.Writer) { } // write the list of subcommands - if len(p.cmd.subcommands) > 0 { + if len(cmd.subcommands) > 0 { fmt.Fprint(w, "\nCommands:\n") - for _, subcmd := range p.cmd.subcommands { + for _, subcmd := range cmd.subcommands { printTwoCols(w, subcmd.name, subcmd.help, nil) } } From 3c5e61a2927728226af0abafab402c511c4e27ac Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Fri, 3 May 2019 15:50:41 -0700 Subject: [PATCH 22/28] simplify Fprint call --- usage.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usage.go b/usage.go index 69e4e62..1f6b569 100644 --- a/usage.go +++ b/usage.go @@ -56,7 +56,7 @@ func (p *Parser) writeUsageForCommand(w io.Writer, cmd *command) { } // print the beginning of the usage string - fmt.Fprintf(w, "Usage:") + fmt.Fprint(w, "Usage:") for i := len(ancestors) - 1; i >= 0; i-- { fmt.Fprint(w, " "+ancestors[i]) } From bd97edec87a0541321c6e2529150e315ee11cd8b Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Fri, 3 May 2019 16:08:29 -0700 Subject: [PATCH 23/28] add Parser.Subcommand and Parser.SubcommandNames --- parse_test.go | 9 +++++++-- subcommand.go | 37 +++++++++++++++++++++++++++++++++++++ subcommand_test.go | 34 +++++++++++++++++++++++++--------- 3 files changed, 69 insertions(+), 11 deletions(-) create mode 100644 subcommand.go diff --git a/parse_test.go b/parse_test.go index 00b0875..c2544fd 100644 --- a/parse_test.go +++ b/parse_test.go @@ -19,15 +19,20 @@ func setenv(t *testing.T, name, val string) { } func parse(cmdline string, dest interface{}) error { + _, err := pparse(cmdline, dest) + return err +} + +func pparse(cmdline string, dest interface{}) (*Parser, error) { p, err := NewParser(Config{}, dest) if err != nil { - return err + return nil, err } var parts []string if len(cmdline) > 0 { parts = strings.Split(cmdline, " ") } - return p.Parse(parts) + return p, p.Parse(parts) } func TestString(t *testing.T) { diff --git a/subcommand.go b/subcommand.go new file mode 100644 index 0000000..b73e933 --- /dev/null +++ b/subcommand.go @@ -0,0 +1,37 @@ +package arg + +// Subcommand returns the user struct for the subcommand selected by +// the command line arguments most recently processed by the parser. +// The return value is always a pointer to a struct. If no subcommand +// was specified then it returns the top-level arguments struct. If +// no command line arguments have been processed by this parser then it +// returns nil. +func (p *Parser) Subcommand() interface{} { + if p.lastCmd == nil || p.lastCmd.parent == nil { + return nil + } + return p.readable(p.lastCmd.dest).Interface() +} + +// SubcommandNames returns the sequence of subcommands specified by the +// user. If no subcommands were given then it returns an empty slice. +func (p *Parser) SubcommandNames() []string { + if p.lastCmd == nil { + return nil + } + + // make a list of ancestor commands + var ancestors []string + cur := p.lastCmd + for cur.parent != nil { // we want to exclude the root + ancestors = append(ancestors, cur.name) + cur = cur.parent + } + + // reverse the list + out := make([]string, len(ancestors)) + for i := 0; i < len(ancestors); i++ { + out[i] = ancestors[len(ancestors)-i-1] + } + return out +} diff --git a/subcommand_test.go b/subcommand_test.go index fdc3ff4..c34ab01 100644 --- a/subcommand_test.go +++ b/subcommand_test.go @@ -41,9 +41,11 @@ func TestMinimalSubcommand(t *testing.T) { var args struct { List *listCmd `arg:"subcommand"` } - err := parse("list", &args) + p, err := pparse("list", &args) require.NoError(t, err) assert.NotNil(t, args.List) + assert.Equal(t, args.List, p.Subcommand()) + assert.Equal(t, []string{"list"}, p.SubcommandNames()) } func TestNoSuchSubcommand(t *testing.T) { @@ -52,7 +54,7 @@ func TestNoSuchSubcommand(t *testing.T) { var args struct { List *listCmd `arg:"subcommand"` } - err := parse("invalid", &args) + _, err := pparse("invalid", &args) assert.Error(t, err) } @@ -62,9 +64,11 @@ func TestNamedSubcommand(t *testing.T) { var args struct { List *listCmd `arg:"subcommand:ls"` } - err := parse("ls", &args) + p, err := pparse("ls", &args) require.NoError(t, err) assert.NotNil(t, args.List) + assert.Equal(t, args.List, p.Subcommand()) + assert.Equal(t, []string{"ls"}, p.SubcommandNames()) } func TestEmptySubcommand(t *testing.T) { @@ -73,9 +77,11 @@ func TestEmptySubcommand(t *testing.T) { var args struct { List *listCmd `arg:"subcommand"` } - err := parse("", &args) + p, err := pparse("", &args) require.NoError(t, err) assert.Nil(t, args.List) + assert.Nil(t, p.Subcommand()) + assert.Empty(t, p.SubcommandNames()) } func TestTwoSubcommands(t *testing.T) { @@ -87,10 +93,12 @@ func TestTwoSubcommands(t *testing.T) { Get *getCmd `arg:"subcommand"` List *listCmd `arg:"subcommand"` } - err := parse("list", &args) + p, err := pparse("list", &args) require.NoError(t, err) assert.Nil(t, args.Get) assert.NotNil(t, args.List) + assert.Equal(t, args.List, p.Subcommand()) + assert.Equal(t, []string{"list"}, p.SubcommandNames()) } func TestSubcommandsWithOptions(t *testing.T) { @@ -185,35 +193,43 @@ func TestNestedSubcommands(t *testing.T) { { var args root - err := parse("grandparent parent child", &args) + p, err := pparse("grandparent parent child", &args) require.NoError(t, err) require.NotNil(t, args.Grandparent) require.NotNil(t, args.Grandparent.Parent) require.NotNil(t, args.Grandparent.Parent.Child) + assert.Equal(t, args.Grandparent.Parent.Child, p.Subcommand()) + assert.Equal(t, []string{"grandparent", "parent", "child"}, p.SubcommandNames()) } { var args root - err := parse("grandparent parent", &args) + p, err := pparse("grandparent parent", &args) require.NoError(t, err) require.NotNil(t, args.Grandparent) require.NotNil(t, args.Grandparent.Parent) require.Nil(t, args.Grandparent.Parent.Child) + assert.Equal(t, args.Grandparent.Parent, p.Subcommand()) + assert.Equal(t, []string{"grandparent", "parent"}, p.SubcommandNames()) } { var args root - err := parse("grandparent", &args) + p, err := pparse("grandparent", &args) require.NoError(t, err) require.NotNil(t, args.Grandparent) require.Nil(t, args.Grandparent.Parent) + assert.Equal(t, args.Grandparent, p.Subcommand()) + assert.Equal(t, []string{"grandparent"}, p.SubcommandNames()) } { var args root - err := parse("", &args) + p, err := pparse("", &args) require.NoError(t, err) require.Nil(t, args.Grandparent) + assert.Nil(t, p.Subcommand()) + assert.Empty(t, p.SubcommandNames()) } } From 990e87d80d9989dd2fbac4db21c8527e1f17cea3 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Fri, 3 May 2019 16:32:16 -0700 Subject: [PATCH 24/28] no need to initialize nil structs during path traversal --- parse.go | 47 +++++++++++------------------------------------ parse_test.go | 20 ++++++++++++++++++++ subcommand.go | 2 +- usage.go | 2 +- 4 files changed, 33 insertions(+), 38 deletions(-) diff --git a/parse.go b/parse.go index e0de705..2126ee5 100644 --- a/parse.go +++ b/parse.go @@ -393,7 +393,7 @@ func (p *Parser) captureEnvVars(specs []*spec, wasPresent map[*spec]bool) error err, ) } - if err = setSlice(p.writable(spec.dest), values, !spec.separate); err != nil { + if err = setSlice(p.val(spec.dest), values, !spec.separate); err != nil { return fmt.Errorf( "error processing environment variable %s with multiple values: %v", spec.env, @@ -401,7 +401,7 @@ func (p *Parser) captureEnvVars(specs []*spec, wasPresent map[*spec]bool) error ) } } else { - if err := scalar.ParseValue(p.writable(spec.dest), value); err != nil { + if err := scalar.ParseValue(p.val(spec.dest), value); err != nil { return fmt.Errorf("error processing environment variable %s: %v", spec.env, err) } } @@ -457,7 +457,7 @@ func (p *Parser) process(args []string) error { } // instantiate the field to point to a new struct - v := p.writable(subcmd.dest) + v := p.val(subcmd.dest) v.Set(reflect.New(v.Type().Elem())) // we already checked that all subcommands are struct pointers // add the new options to the set of allowed options @@ -512,7 +512,7 @@ func (p *Parser) process(args []string) error { } else { values = append(values, value) } - err := setSlice(p.writable(spec.dest), values, !spec.separate) + err := setSlice(p.val(spec.dest), values, !spec.separate) if err != nil { return fmt.Errorf("error processing %s: %v", arg, err) } @@ -537,7 +537,7 @@ func (p *Parser) process(args []string) error { i++ } - err := scalar.ParseValue(p.writable(spec.dest), value) + err := scalar.ParseValue(p.val(spec.dest), value) if err != nil { return fmt.Errorf("error processing %s: %v", arg, err) } @@ -553,13 +553,13 @@ func (p *Parser) process(args []string) error { } wasPresent[spec] = true if spec.multiple { - err := setSlice(p.writable(spec.dest), positionals, true) + err := setSlice(p.val(spec.dest), positionals, true) if err != nil { return fmt.Errorf("error processing %s: %v", spec.long, err) } positionals = nil } else { - err := scalar.ParseValue(p.writable(spec.dest), positionals[0]) + err := scalar.ParseValue(p.val(spec.dest), positionals[0]) if err != nil { return fmt.Errorf("error processing %s: %v", spec.long, err) } @@ -602,9 +602,9 @@ func isFlag(s string) bool { return strings.HasPrefix(s, "-") && strings.TrimLeft(s, "-") != "" } -// readable returns a reflect.Value corresponding to the current value for the -// given -func (p *Parser) readable(dest path) reflect.Value { +// val returns a reflect.Value corresponding to the current value for the +// given path +func (p *Parser) val(dest path) reflect.Value { v := p.roots[dest.root] for _, field := range dest.fields { if v.Kind() == reflect.Ptr { @@ -626,35 +626,10 @@ func (p *Parser) readable(dest path) reflect.Value { return v } -// writable trav.patherses the destination struct to find the destination to -// which the value of the given spec should be written. It fills in null -// structs with pointers to the zero value for that struct. -func (p *Parser) writable(dest path) reflect.Value { - v := p.roots[dest.root] - for _, field := range dest.fields { - if v.Kind() == reflect.Ptr { - if v.IsNil() { - v.Set(reflect.New(v.Type().Elem())) - } - v = v.Elem() - } - - v = v.FieldByName(field) - if !v.IsValid() { - // it is appropriate to panic here because this can only happen due to - // an internal bug in this library (since we construct the path ourselves - // by reflecting on the same struct) - panic(fmt.Errorf("error resolving path %v: %v has no field named %v", - dest.fields, v.Type(), field)) - } - } - return v -} - // parse a value as the appropriate type and store it in the struct func setSlice(dest reflect.Value, values []string, trunc bool) error { if !dest.CanSet() { - return fmt.Errorf("field is not writable") + return fmt.Errorf("field is not val") } var ptr bool diff --git a/parse_test.go b/parse_test.go index c2544fd..c18dc16 100644 --- a/parse_test.go +++ b/parse_test.go @@ -877,6 +877,26 @@ func TestEmbedded(t *testing.T) { assert.Equal(t, true, args.Z) } +func TestEmbeddedPtr(t *testing.T) { + // embedded pointer fields are not supported so this should return an error + var args struct { + *A + } + err := parse("--x=hello", &args) + require.Error(t, err) +} + +func TestEmbeddedPtrIgnored(t *testing.T) { + // embedded pointer fields are not supported but here we + var args struct { + *A `arg:"-"` + B + } + err := parse("--y=321", &args) + require.NoError(t, err) + assert.Equal(t, 321, args.Y) +} + func TestEmptyArgs(t *testing.T) { origArgs := os.Args diff --git a/subcommand.go b/subcommand.go index b73e933..dff732c 100644 --- a/subcommand.go +++ b/subcommand.go @@ -10,7 +10,7 @@ func (p *Parser) Subcommand() interface{} { if p.lastCmd == nil || p.lastCmd.parent == nil { return nil } - return p.readable(p.lastCmd.dest).Interface() + return p.val(p.lastCmd.dest).Interface() } // SubcommandNames returns the sequence of subcommands specified by the diff --git a/usage.go b/usage.go index 1f6b569..33b6184 100644 --- a/usage.go +++ b/usage.go @@ -179,7 +179,7 @@ func (p *Parser) printOption(w io.Writer, spec *spec) { // If spec.dest is not the zero value then a default value has been added. var v reflect.Value if len(spec.dest.fields) > 0 { - v = p.readable(spec.dest) + v = p.val(spec.dest) } var defaultVal *string From fcdfbc090b3262d6d078337b5de78fe1038a41ba Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Tue, 6 Aug 2019 16:00:13 -0700 Subject: [PATCH 25/28] fix comment --- parse_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/parse_test.go b/parse_test.go index c18dc16..882564e 100644 --- a/parse_test.go +++ b/parse_test.go @@ -887,7 +887,9 @@ func TestEmbeddedPtr(t *testing.T) { } func TestEmbeddedPtrIgnored(t *testing.T) { - // embedded pointer fields are not supported but here we + // embedded pointer fields are not normally supported but here + // we explicitly exclude it so the non-nil embedded structs + // should work as expected var args struct { *A `arg:"-"` B From 9f37d5f6001b9379af81defbddd8ae63a0744af0 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Tue, 6 Aug 2019 16:38:11 -0700 Subject: [PATCH 26/28] fix typo --- parse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parse.go b/parse.go index 2126ee5..2a6b7c7 100644 --- a/parse.go +++ b/parse.go @@ -629,7 +629,7 @@ func (p *Parser) val(dest path) reflect.Value { // parse a value as the appropriate type and store it in the struct func setSlice(dest reflect.Value, values []string, trunc bool) error { if !dest.CanSet() { - return fmt.Errorf("field is not val") + return fmt.Errorf("field is not writable") } var ptr bool From e6003d3b6a3cca65443440e6be8267b6c5c4ea33 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Tue, 6 Aug 2019 16:41:50 -0700 Subject: [PATCH 27/28] add subcommands to readme --- README.md | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ example_test.go | 44 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/README.md b/README.md index c772994..c516c51 100644 --- a/README.md +++ b/README.md @@ -353,6 +353,59 @@ Options: --help, -h display this help and exit ``` +### Subcommands + +*Introduced in `v1.1.0`* + +Subcommands are commonly used in tools that wish to group multiple functions into a single program. An example is the `git` tool: +```shell +$ git checkout [arguments specific to checking out code] +$ git commit [arguments specific to committing] +$ git push [arguments specific to pushing] +``` + +The strings "checkout", "commit", and "push" are different from simple positional arguments because the options available to the user change depending on which subcommand they choose. + +This can be implemented with `go-arg` as follows: + +```go +type CheckoutCmd struct { + Branch string `arg:"positional"` + Track bool `arg:"-t"` +} +type CommitCmd struct { + All bool `arg:"-a"` + Message string `arg:"-m"` +} +type PushCmd struct { + Remote string `arg:"positional"` + Branch string `arg:"positional"` + SetUpstream bool `arg:"-u"` +} +var args struct { + Checkout *CheckoutCmd `arg:"subcommand:checkout"` + Commit *CommitCmd `arg:"subcommand:commit"` + Push *PushCmd `arg:"subcommand:push"` + Quiet bool `arg:"-q"` // this flag is global to all subcommands +} + +arg.MustParse(&args) + +switch { +case args.Checkout != nil: + fmt.Printf("checkout requested for branch %s\n", args.Checkout.Branch) +case args.Commit != nil: + fmt.Printf("commit requested with message \"%s\"\n", args.Commit.Message) +case args.Push != nil: + fmt.Printf("push requested from %s to %s\n", args.Push.Branch, args.Push.Remote) +} +``` + +Some additional rules apply when working with subcommands: +* The `subcommand` tag can only be used with fields that are pointers to structs +* Any struct that contains a subcommand must not contain any positionals + + ### API Documentation https://godoc.org/github.com/alexflint/go-arg diff --git a/example_test.go b/example_test.go index b58085f..2188253 100644 --- a/example_test.go +++ b/example_test.go @@ -255,3 +255,47 @@ func Example_errorTextForSubcommand() { // Usage: example get [--count COUNT] // error: error processing --count: strconv.ParseInt: parsing "INVALID": invalid syntax } + +// This example demonstrates use of subcommands +func Example_subcommand() { + // These are the args you would pass in on the command line + os.Args = split("./example commit -a -m what-this-commit-is-about") + + type CheckoutCmd struct { + Branch string `arg:"positional"` + Track bool `arg:"-t"` + } + type CommitCmd struct { + All bool `arg:"-a"` + Message string `arg:"-m"` + } + type PushCmd struct { + Remote string `arg:"positional"` + Branch string `arg:"positional"` + SetUpstream bool `arg:"-u"` + } + var args struct { + Checkout *CheckoutCmd `arg:"subcommand:checkout"` + Commit *CommitCmd `arg:"subcommand:commit"` + Push *PushCmd `arg:"subcommand:push"` + Quiet bool `arg:"-q"` // this flag is global to all subcommands + } + + // This is only necessary when running inside golang's runnable example harness + osExit = func(int) {} + stderr = os.Stdout + + MustParse(&args) + + switch { + case args.Checkout != nil: + fmt.Printf("checkout requested for branch %s\n", args.Checkout.Branch) + case args.Commit != nil: + fmt.Printf("commit requested with message \"%s\"\n", args.Commit.Message) + case args.Push != nil: + fmt.Printf("push requested from %s to %s\n", args.Push.Branch, args.Push.Remote) + } + + // output: + // commit requested with message "what-this-commit-is-about" +} From 11a27074fcbbcce6d5b2129c9d33328152cd56be Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Tue, 6 Aug 2019 16:49:02 -0700 Subject: [PATCH 28/28] test with go 1.12 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index f953225..d2a00fd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ language: go go: - "1.10" - - "1.11" + - "1.12" - tip env: - GO111MODULE=on # will only be used in go 1.11