Refactoring Commands

[mappum]

Now that we have the commands package set up, it's time to move everything over to it. This PR is currently not ready for merge, just setting it up for CR.

The basic scope of this PR is as follows:

• Set up transports based on commands package (HTTP RPC server/client, CLI command handling)
• Refactor existing commands for the new API (won't be a 1:1 mapping, commands will change)
• Make changes to commands package when necessary

[mappum]

At the current HEAD, the following is done:

• HTTP RPC server/client (at commands/http)
• CLI entry point

You can test out the HTTP by doing ipfs daemon, then visiting localhost:8080/api/v0/beep in your browser. While the daemon is running, you can also do ipfs beep and it will send the command to the daemon. For a demonstration of the encoding functionality, visit localhost:8080/api/v0/beep?enc=xml.

[jbenet]

@mappum good progress, this cleans up a lot! 👍

[jbenet]

I guess for now let's just keep outputting CLI-friendly Help-- we can solve that over time once we generate HTTP API docs.

[jbenet]

@mappum heads up -- i rebased over master.

[btc]

@mappum Is it intended for the calling Command to be available to the Function? For example, the root Ipfs command prints out it's help message. Is the intent for this root cli command to have an empty Run Function and print out the help message some other way?

[mappum]

@maybebtc Commands have a Help string field, and it's up to the calling code to decide how/when to display it, not the command itself. For example, the CLI entry point prints the help text if there is an error when trying to call the Run Function, but the HTTP server does not.

[btc]

@mappum cool. as you requested, force-pushed the side-by-side version to the branch under review.

[btc]

@mappum could make it harder to vendor this if we import ipfs-specific packages in the commands lib. Perhaps this root command can be passed in?

func NewHandler(r commands.Command) Handler {
  return Handler{Root: r}
}

[jbenet]

yeah pass it in. will be good to pull this out and give it to the community as the sweetest command package yet.

[btc]

@jbenet
@mappum
@whyrusleeping

@mappum brought me up to speed on the status of the commands refactoring. I'm relaying this information for the group so we may chart a path to the completion of the overhaul.

There are a couple sample commands in place. cat, daemon, and init have been implemented using the new framework! 🎉

And there are just a couple remaining framework concerns. These are briefly outlined below.

To ease integration, may I suggest a course of action?

Merge into master once:

1. @mappum is satisfied with the outstanding framework concerns (outlined below)
2. framework stuff is CR'd
3. existing, newly-implemented commands are CR'd

After the merge, remaining commands can be implemented individually.

🐶 🐕 Dogfooding! Also, I bet it would be really helpful if we'd implement a command (end-to-end) and provide @mappum with feedback. Exercising the various edge-cases will help to iron out the framework and reveal any lingering unknown unknowns.

---

Framework Concerns

• file arguments in CLI (blocks cli/add.go)

... when to read args as files and stream the data, vs sending an arg string. e.g. ipfs add foo.txt - @mappum

• formatting

I just added a Format field to Command, it's a function that takes in a command response and outputs human-readable plaintext. i set it up assuming it would be able to read in the struct response values, which works when running commands locally, but when you run it with unmarshalled output (e.g. from a HTTP response), we have a map[string]interface{} instead of a struct... - @mappum

CLI Concerns

• add.go
• block.go
• bootstrap.go
• cat.go
• commands.go
• config.go
• daemon.go
• diag.go
• init.go
• ipfs.go
• log.go
• ls.go
• mount_unix.go
• mount_windows.go
• name.go
• objects.go
• pin.go
• publish.go
• refs.go
• resolve.go
• run.go
• serve.go
• tour.go
• update.go
• version.go

[mappum]

In the concerns checklist:

• formatting

I took care of that, the beep command is an example of how it works.

[jbenet]

@mappum @maybebtc bravo! this clarity on status makes me feel great. LMK which command you'd like me to tackle.

Thoughts file args in cli concern

I think we could either

1. create an commands.Argument abstraction, like commands.Option that specifies the type. This would help parse things (yay), and cli could make an io.Reader if arg type is "InputFile" or somethig.
2. provide an optional arg parsing function to commands, which can manipulate the args as needed. simpler, but prob less useful.

[mappum]

About the file args in CLI:

Something I've noticed is that args are almost always just strings, which is why I didn't already make static type definitions for them (on the rare occasion that it is an int or something, the command can convert it itself). Also, adding argument specifications can take away some features, such as variadic args (e.g. ipfs cat can take any number of args).

cli could make an io.Reader if arg type is "InputFile"

That sounds pretty clean.

[jbenet]

on the rare occasion that it is an int or something, the command can convert it itself

but might be nice if this was done for you. could get rid of lots of implementation bugs if type checking happens on parse

Also, adding argument specifications can take away some features, such as variadic args

Maybe, not necessarily, you could have something like:

cmd := &Command{
  Args: []Arg{
    Arg{Type: Integer}, // may not be there.
    Arg{Type: String, Required: true}, // must have it or fail
    Arg{Type: InputFile, Required: true, Variadic: true}, // required = 1 or more
  }
  Args: []Arg(String)
}

And Args slice is considered invalid unless there is exactly one Variadic arg, at the end.

And this arg slice descriptor would error out if any of the InputFiles don't exist :)

---

If we want to do anything more complicated, can always just get variadic strings and deal with it yourself. (so have full power, but the lib does some lifting for you).

[jbenet]

@mappum @maybebtc lmk which commands you'd like me to try to implement tonight

[btc]

@jbenet refs is a good place to start.

[btc]

• block get
• block put

Heads up @mappum, I'm porting the above to get acquainted.

[mappum]

@jbenet @maybebtc
Now all the comments have been addressed. Care to give it another look?

[jbenet]

We're splitting this into #262 and #263. Closing this for posterity.