Commands Refactor Part 1

commands/argument.go

@@ -0,0 +1,15 @@
+package commands
+
+type ArgumentType int
+
+const (
+	ArgString ArgumentType = iota
+	ArgFile
+)
+
+type Argument struct {
+	Name     string
+	Type     ArgumentType
+	Required bool
+	Variadic bool
+}

commands/cli/parse.go

@@ -1,26 +1,63 @@
 package cli
 
 import (
+	"errors"
 	"fmt"
+	"os"
 	"strings"
 
-	"github.com/jbenet/go-ipfs/commands"
+	cmds "github.com/jbenet/go-ipfs/commands"
 )
 
 // Parse parses the input commandline string (cmd, flags, and args).
 // returns the corresponding command Request object.
-func Parse(input []string, root *commands.Command) (commands.Request, error) {
-	path, input := parsePath(input, root)
-	opts, args, err := parseOptions(input)
+func Parse(input []string, roots ...*cmds.Command) (cmds.Request, *cmds.Command, error) {
+	var root, cmd *cmds.Command
+	var path, stringArgs []string
+	var opts map[string]interface{}
+
+	// use the root that matches the longest path (most accurately matches request)
+	maxLength := 0
+	for _, r := range roots {
+		p, i, c := parsePath(input, r)
+		o, s, err := parseOptions(i)
+		if err != nil {
+			return nil, nil, err
+		}
+
+		length := len(p)
+		if length > maxLength {
+			maxLength = length
+			root = r
+			path = p
+			cmd = c
+			opts = o
+			stringArgs = s
+		}
+	}
+
+	if maxLength == 0 {
+		return nil, nil, errors.New("Not a valid subcommand")
+	}
+
+	args, err := parseArgs(stringArgs, cmd)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	req := cmds.NewRequest(path, opts, args, cmd)
+
+	err = cmd.CheckArguments(req)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
-	return commands.NewRequest(path, opts, args, nil), nil
+	return req, root, nil
 }
 
-// parsePath gets the command path from the command line input
-func parsePath(input []string, root *commands.Command) ([]string, []string) {
+// parsePath separates the command path and the opts and args from a command string
+// returns command path slice, rest slice, and the corresponding *cmd.Command
+func parsePath(input []string, root *cmds.Command) ([]string, []string, *cmds.Command) {
 	cmd := root
 	i := 0
 
@@ -29,15 +66,16 @@ func parsePath(input []string, root *commands.Command) ([]string, []string) {
 			break
 		}
 
-		cmd := cmd.Subcommand(blob)
-		if cmd == nil {
+		sub := cmd.Subcommand(blob)
+		if sub == nil {
 			break
 		}
+		cmd = sub
 
 		i++
 	}
 
-	return input[:i], input[i:]
+	return input[:i], input[i:], cmd
 }
 
 // parseOptions parses the raw string values of the given options
@@ -77,3 +115,27 @@ func parseOptions(input []string) (map[string]interface{}, []string, error) {
 
 	return opts, args, nil
 }
+
+func parseArgs(stringArgs []string, cmd *cmds.Command) ([]interface{}, error) {
+	var argDef cmds.Argument
+	args := make([]interface{}, len(stringArgs))
+
+	for i, arg := range stringArgs {
+		if i < len(cmd.Arguments) {
+			argDef = cmd.Arguments[i]
+		}
+
+		if argDef.Type == cmds.ArgString {
+			args[i] = arg
+
+		} else {
+			in, err := os.Open(arg)
+			if err != nil {
+				return nil, err
+			}
+			args[i] = in
+		}
+	}
+
+	return args, nil
+}

commands/cli/parse_test.go

@@ -8,12 +8,13 @@ import (
 )
 
 func TestOptionParsing(t *testing.T) {
+	subCmd := &commands.Command{}
 	cmd := &commands.Command{
 		Options: []commands.Option{
 			commands.Option{Names: []string{"b"}, Type: commands.String},
 		},
 		Subcommands: map[string]*commands.Command{
-			"test": &commands.Command{},
+			"test": subCmd,
 		},
 	}
 
@@ -37,11 +38,14 @@ func TestOptionParsing(t *testing.T) {
 		t.Error("Should have failed (duplicate option name)")
 	}
 
-	path, args := parsePath([]string{"test", "beep", "boop"}, cmd)
+	path, args, sub := parsePath([]string{"test", "beep", "boop"}, cmd)
 	if len(path) != 1 || path[0] != "test" {
 		t.Errorf("Returned path was defferent than expected: %v", path)
 	}
 	if len(args) != 2 || args[0] != "beep" || args[1] != "boop" {
 		t.Errorf("Returned args were different than expected: %v", args)
 	}
+	if sub != subCmd {
+		t.Errorf("Returned command was different than expected")
+	}
 }

commands/command.go

@@ -3,6 +3,7 @@ package commands
 import (
 	"errors"
 	"fmt"
+	"io"
 	"strings"
 
 	u "github.com/jbenet/go-ipfs/util"
@@ -12,20 +13,35 @@ var log = u.Logger("command")
 
 // Function is the type of function that Commands use.
 // It reads from the Request, and writes results to the Response.
-type Function func(Request, Response)
+type Function func(Response, Request)
+
+// Marshaller is a function that takes in a Response, and returns a marshalled []byte
+// (or an error on failure)
+type Marshaller func(Response) ([]byte, error)
+
+// TODO: check Argument definitions when creating a Command
+//   (might need to use a Command constructor)
+//   * make sure any variadic args are at the end
+//   * make sure there aren't duplicate names
+//   * make sure optional arguments aren't followed by required arguments
 
 // Command is a runnable command, with input arguments and options (flags).
 // It can also have Subcommands, to group units of work into sets.
 type Command struct {
 	Help        string
 	Options     []Option
+	Arguments   []Argument
 	Run         Function
+	Marshallers map[EncodingType]Marshaller
+	Type        interface{}
 	Subcommands map[string]*Command
 }
 
 // ErrNotCallable signals a command that cannot be called.
 var ErrNotCallable = errors.New("This command can't be called directly. Try one of its subcommands.")
 
+var ErrNoFormatter = errors.New("This command cannot be formatted to plain text")
+
 // Call invokes the command for the given Request
 func (c *Command) Call(req Request) Response {
 	res := NewResponse(req)
@@ -42,6 +58,12 @@ func (c *Command) Call(req Request) Response {
 		return res
 	}
 
+	err = cmd.CheckArguments(req)
+	if err != nil {
+		res.SetError(err, ErrClient)
+		return res
+	}
+
 	options, err := c.GetOptions(req.Path())
 	if err != nil {
 		res.SetError(err, ErrClient)
@@ -54,7 +76,7 @@ func (c *Command) Call(req Request) Response {
 		return res
 	}
 
-	cmd.Run(req, res)
+	cmd.Run(res, req)
 
 	return res
 }
@@ -116,7 +138,72 @@ func (c *Command) GetOptions(path []string) (map[string]Option, error) {
 	return optionsMap, nil
 }
 
+func (c *Command) CheckArguments(req Request) error {
+	args := req.Arguments()
+	argDefs := c.Arguments
+
+	// if we have more arg values provided than argument definitions,
+	// and the last arg definition is not variadic (or there are no definitions), return an error
+	notVariadic := len(argDefs) == 0 || !argDefs[len(argDefs)-1].Variadic
+	if notVariadic && len(args) > len(argDefs) {
+		return fmt.Errorf("Expected %v arguments, got %v", len(argDefs), len(args))
+	}
+
+	// iterate over the arg definitions
+	for i, argDef := range c.Arguments {
+
+		// 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]
+		}
+
+		err := checkArgValue(v, argDef)
+		if err != nil {
+			return err
+		}
+
+		// any additional values are for the variadic arg definition
+		if argDef.Variadic && i < len(args)-1 {
+			for _, val := range args[i+1:] {
+				err := checkArgValue(val, argDef)
+				if err != nil {
+					return err
+				}
+			}
+		}
+	}
+
+	return nil
+}
+
 // Subcommand returns the subcommand with the given id
 func (c *Command) Subcommand(id string) *Command {
 	return c.Subcommands[id]
 }
+
+// checkArgValue returns an error if a given arg value is not valid for the given Argument
+func checkArgValue(v interface{}, def Argument) error {
+	if v == nil {
+		if def.Required {
+			return fmt.Errorf("Argument '%s' is required", def.Name)
+		}
+
+		return nil
+	}
+
+	if def.Type == ArgFile {
+		_, ok := v.(io.Reader)
+		if !ok {
+			return fmt.Errorf("Argument '%s' isn't valid", def.Name)
+		}
+
+	} else if def.Type == ArgString {
+		_, ok := v.(string)
+		if !ok {
+			return fmt.Errorf("Argument '%s' must be a string", def.Name)
+		}
+	}
+
+	return nil
+}

commands/command_test.go

@@ -8,20 +8,13 @@ func TestOptionValidation(t *testing.T) {
 			Option{[]string{"b", "beep"}, Int},
 			Option{[]string{"B", "boop"}, String},
 		},
-		Run: func(req Request, res Response) {},
+		Run: func(res Response, req Request) {},
 	}
 
 	req := NewEmptyRequest()
-	req.SetOption("foo", 5)
-	res := cmd.Call(req)
-	if res.Error() == nil {
-		t.Error("Should have failed (unrecognized option)")
-	}
-
-	req = NewEmptyRequest()
 	req.SetOption("beep", 5)
 	req.SetOption("b", 10)
-	res = cmd.Call(req)
+	res := cmd.Call(req)
 	if res.Error() == nil {
 		t.Error("Should have failed (duplicate options)")
 	}
@@ -56,6 +49,13 @@ func TestOptionValidation(t *testing.T) {
 		t.Error("Should have passed")
 	}
 
+	req = NewEmptyRequest()
+	req.SetOption("foo", 5)
+	res = cmd.Call(req)
+	if res.Error() != nil {
+		t.Error("Should have passed")
+	}
+
 	req = NewEmptyRequest()
 	req.SetOption(EncShort, "json")
 	res = cmd.Call(req)
@@ -79,7 +79,7 @@ func TestOptionValidation(t *testing.T) {
 }
 
 func TestRegistration(t *testing.T) {
-	noop := func(req Request, res Response) {}
+	noop := func(res Response, req Request) {}
 
 	cmdA := &Command{
 		Options: []Option{