commands: Fixed argument value/definition mapping

commands/cli/parse.go

@@ -117,25 +117,58 @@ func parseOptions(input []string) (map[string]interface{}, []string, error) {
 }
 
 func parseArgs(stringArgs []string, cmd *cmds.Command) ([]interface{}, error) {
-	var argDef cmds.Argument
-	args := make([]interface{}, len(stringArgs))
+	args := make([]interface{}, 0)
 
-	for i, arg := range stringArgs {
-		if i < len(cmd.Arguments) {
-			argDef = cmd.Arguments[i]
+	// count required argument definitions
+	lenRequired := 0
+	for _, argDef := range cmd.Arguments {
+		if argDef.Required {
+			lenRequired++
 		}
+	}
 
-		if argDef.Type == cmds.ArgString {
-			args[i] = arg
+	j := 0
+	for _, argDef := range cmd.Arguments {
+		// skip optional argument definitions if there aren't sufficient remaining values
+		if len(stringArgs)-j <= lenRequired && !argDef.Required {
+			continue
+		}
 
+		if j >= len(stringArgs) {
+			break
+		}
+
+		if argDef.Variadic {
+			for _, arg := range stringArgs[j:] {
+				var err error
+				args, err = appendArg(args, argDef, arg)
+				if err != nil {
+					return nil, err
+				}
+			}
 		} else {
-			in, err := os.Open(arg)
+			var err error
+			args, err = appendArg(args, argDef, stringArgs[j])
 			if err != nil {
 				return nil, err
 			}
-			args[i] = in
 		}
+
+		j++
 	}
 
 	return args, nil
 }
+
+func appendArg(args []interface{}, argDef cmds.Argument, value string) ([]interface{}, error) {
+	if argDef.Type == cmds.ArgString {
+		return append(args, value), nil
+
+	} else {
+		in, err := os.Open(value)
+		if err != nil {
+			return nil, err
+		}
+		return append(args, in), nil
+	}
+}

commands/command.go

@@ -149,13 +149,27 @@ func (c *Command) CheckArguments(req Request) error {
 		return fmt.Errorf("Expected %v arguments, got %v", len(argDefs), len(args))
 	}
 
+	// count required argument definitions
+	lenRequired := 0
+	for _, argDef := range c.Arguments {
+		if argDef.Required {
+			lenRequired++
+		}
+	}
+
 	// iterate over the arg definitions
-	for i, argDef := range c.Arguments {
+	j := 0
+	for _, argDef := range c.Arguments {
+		// skip optional argument definitions if there aren't sufficient remaining values
+		if len(args)-j <= lenRequired && !argDef.Required {
+			continue
+		}
 
 		// the value for this argument definition. can be nil if it wasn't provided by the caller
 		var v interface{}
-		if i < len(args) {
-			v = args[i]
+		if j < len(args) {
+			v = args[j]
+			j++
 		}
 
 		err := checkArgValue(v, argDef)
@@ -164,8 +178,8 @@ func (c *Command) CheckArguments(req Request) error {
 		}
 
 		// any additional values are for the variadic arg definition
-		if argDef.Variadic && i < len(args)-1 {
-			for _, val := range args[i+1:] {
+		if argDef.Variadic && j < len(args)-1 {
+			for _, val := range args[j+1:] {
 				err := checkArgValue(val, argDef)
 				if err != nil {
 					return err

commands/http/parse.go

@@ -39,13 +39,32 @@ func Parse(r *http.Request, root *cmds.Command) (cmds.Request, error) {
 	opts, stringArgs2 := parseOptions(r)
 	stringArgs = append(stringArgs, stringArgs2...)
 
-	// Note that the argument handling here is dumb, it does not do any error-checking.
-	// (Arguments are further processed when the request is passed to the command to run)
 	args := make([]interface{}, 0)
 
-	for _, arg := range cmd.Arguments {
-		if arg.Type == cmds.ArgString {
-			if arg.Variadic {
+	// count required argument definitions
+	lenRequired := 0
+	for _, argDef := range cmd.Arguments {
+		if argDef.Required {
+			lenRequired++
+		}
+	}
+
+	// count the number of provided argument values
+	valCount := len(stringArgs)
+	// TODO: add total number of parts in request body (instead of just 1 if body is present)
+	if r.Body != nil {
+		valCount += 1
+	}
+
+	for _, argDef := range cmd.Arguments {
+		// skip optional argument definitions if there aren't sufficient remaining values
+		if valCount <= lenRequired && !argDef.Required {
+			continue
+		}
+		valCount--
+
+		if argDef.Type == cmds.ArgString {
+			if argDef.Variadic {
 				for _, s := range stringArgs {
 					args = append(args, s)
 				}