actually allow repeated options

internal/constants/help.md

@@ -64,7 +64,7 @@ description: {{ .Command.Short }}
 ## Options
 
 {{ range $name, $opt := .Spec.Options -}}
-- `--{{ $name }}` (_{{$opt.Type}}_): {{ trimSuffix $opt.Description "."}}.{{ if $opt.Default }} Default: _{{ $opt.Default }}_.{{ end }}
+- `--{{ $name }}` (_{{if $opt.Repeated}}[]{{end}}{{$opt.Type}}_): {{ trimSuffix $opt.Description "."}}.{{if $opt.Repeated}} May be specified more than once. {{end}}{{ if $opt.Default }} Default: _{{ $opt.Default }}_.{{ end }}
 {{ end -}}
 {{- end -}}
 

pkg/command/command.go

@@ -98,11 +98,31 @@ func (cmd *Command) FlagSet() *pflag.FlagSet {
 				fs.IntP(name, opt.ShortName, def, opt.Description)
 			case ValueTypeDefault, ValueTypeString:
 				opt.Type = ValueTypeString
-				def := ""
+				if opt.Repeated {
-				if opt.Default != nil {
+					def := []string{}
-					def = fmt.Sprintf("%s", opt.Default)
+					if opt.Default != nil {
+						switch defV := opt.Default.(type) {
+						case []any:
+							for _, v := range defV {
+								def = append(def, fmt.Sprintf("%s", v))
+							}
+						case []string:
+							def = defV
+						case string:
+							def = []string{defV}
+						default:
+							logger.Errorf("Invalid default for repeated option %s configuration: %+v", name, defV)
+						}
+					}
+
+					fs.StringArrayP(name, opt.ShortName, def, opt.Description)
+				} else {
+					def := ""
+					if opt.Default != nil {
+						def = fmt.Sprintf("%s", opt.Default)
+					}
+					fs.StringP(name, opt.ShortName, def, opt.Description)
 				}
-				fs.StringP(name, opt.ShortName, def, opt.Description)
 			default:
 				// ignore flag
 				log.Warnf("Ignoring unknown option type <%s> for option <%s>", opt.Type, name)

pkg/command/options.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"git.rob.mx/nidito/chinampa/pkg/errors"
+	"git.rob.mx/nidito/chinampa/pkg/logger"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
@@ -15,6 +16,7 @@ import (
 // Options is a map of name to Option.
 type Options map[string]*Option
 
+// AllKnown returns a map of option names to their resolved values
 func (opts *Options) AllKnown() map[string]any {
 	col := map[string]any{}
 	for name, opt := range *opts {
@@ -23,6 +25,7 @@ func (opts *Options) AllKnown() map[string]any {
 	return col
 }
 
+// AllKnownStr returns a map of option names to their stringified values
 func (opts *Options) AllKnownStr() map[string]string {
 	col := map[string]string{}
 	for name, opt := range *opts {
@@ -31,6 +34,7 @@ func (opts *Options) AllKnownStr() map[string]string {
 	return col
 }
 
+// Parse populates values with those supplied in the provided pflag.Flagset
 func (opts *Options) Parse(supplied *pflag.FlagSet) {
 	// log.Debugf("Parsing supplied flags, %v", supplied)
 	for name, opt := range *opts {
@@ -47,14 +51,24 @@ func (opts *Options) Parse(supplied *pflag.FlagSet) {
 			}
 		default:
 			opt.Type = ValueTypeString
-			if val, err := supplied.GetString(name); err == nil {
+			if opt.Repeated {
-				opt.provided = val
+				if val, err := supplied.GetStringArray(name); err == nil {
-				continue
+					opt.provided = val
+					continue
+				} else {
+					logger.Errorf("Invalid option configuration: %s", err)
+				}
+			} else {
+				if val, err := supplied.GetString(name); err == nil {
+					opt.provided = val
+					continue
+				}
 			}
 		}
 	}
 }
 
+// AreValid tells if these options are all valid
 func (opts *Options) AreValid() error {
 	for name, opt := range *opts {
 		if err := opt.Validate(name); err != nil {
@@ -67,20 +81,29 @@ func (opts *Options) AreValid() error {
 
 // Option represents a command line flag.
 type Option struct {
-	ShortName   string       `json:"short-name,omitempty" yaml:"short-name,omitempty"` // nolint:tagliatelle
+	// Type represents the type of value expected to be provided for this option
-	Type        ValueType    `json:"type" yaml:"type" validate:"omitempty,oneof=string bool int"`
+	Type ValueType `json:"type" yaml:"type" validate:"omitempty,oneof=string bool int"`
-	Description string       `json:"description" yaml:"description" validate:"required"`
+	// Description is a required field that show up during completions and help
-	Default     any          `json:"default,omitempty" yaml:"default,omitempty"`
+	Description string `json:"description" yaml:"description" validate:"required"`
-	Values      *ValueSource `json:"values,omitempty" yaml:"values,omitempty" validate:"omitempty"`
+	// Default value for this option, if none provided
-	Repeated    bool         `json:"repeated" yaml:"repeated" validate:"omitempty"`
+	Default any `json:"default,omitempty" yaml:"default,omitempty"`
-	Command     *Command     `json:"-" yaml:"-" validate:"-"`
+	// ShortName When set, enables representing this Option as a short flag (-x)
-	provided    any
+	ShortName string `json:"short-name,omitempty" yaml:"short-name,omitempty"` // nolint:tagliatelle
+	// Values denote the source for completion/validation values of this option
+	Values *ValueSource `json:"values,omitempty" yaml:"values,omitempty" validate:"omitempty"`
+	// Repeated options may be specified more than once
+	Repeated bool `json:"repeated" yaml:"repeated" validate:"omitempty"`
+	// Command references the Command this Option is defined for
+	Command  *Command `json:"-" yaml:"-" validate:"-"`
+	provided any
 }
 
+// IsKnown tells if the option was provided by the user
 func (opt *Option) IsKnown() bool {
 	return opt.provided != nil
 }
 
+// Returns the resolved value for an option
 func (opt *Option) ToValue() any {
 	if opt.IsKnown() {
 		return opt.provided
@@ -88,6 +111,7 @@ func (opt *Option) ToValue() any {
 	return opt.Default
 }
 
+// Returns a string representation of this Option's resolved value
 func (opt *Option) ToString() string {
 	value := opt.ToValue()
 	stringValue := ""
@@ -113,13 +137,7 @@ func (opt *Option) ToString() string {
 	return stringValue
 }
 
-func (opt *Option) Validate(name string) error {
+func (opt *Option) internalValidate(name, current string) error {
-	if !opt.Validates() {
-		return nil
-	}
-
-	current := opt.ToString() // nolint:ifshort
-
 	if current == "" {
 		return nil
 	}
@@ -136,6 +154,24 @@ func (opt *Option) Validate(name string) error {
 	return nil
 }
 
+// Validate validates the provided value if a value source
+func (opt *Option) Validate(name string) error {
+	if !opt.Validates() {
+		return nil
+	}
+
+	if opt.Repeated {
+		values := opt.ToValue().([]string)
+		for _, current := range values {
+			opt.internalValidate(name, current)
+		}
+	} else {
+		opt.internalValidate(name, opt.ToString()) // nolint:ifshort
+	}
+
+	return nil
+}
+
 // Validates tells if the user-supplied value needs validation.
 func (opt *Option) Validates() bool {
 	return opt.Values != nil && opt.Values.Validates()
@@ -161,12 +197,14 @@ func (opt *Option) Resolve(currentValue string) (values []string, flag cobra.She
 // CompletionFunction is called by cobra when asked to complete an option.
 func (opt *Option) CompletionFunction(cmd *cobra.Command, args []string, toComplete string) (values []string, flag cobra.ShellCompDirective) {
 	if !opt.providesAutocomplete() {
+		logger.Tracef("Option does not provide autocomplete %+v", opt)
 		flag = cobra.ShellCompDirectiveNoFileComp
 		return
 	}
 
 	if err := opt.Command.Arguments.Parse(args); err != nil {
-		return []string{err.Error()}, cobra.ShellCompDirectiveDefault
+		logger.Errorf("Could not parse command arguments %s", err)
+		return []string{}, cobra.ShellCompDirectiveDefault
 	}
 	opt.Command.Options.Parse(cmd.Flags())