WIP Modernize documentation and examples

chan.go

@@ -6,6 +6,7 @@ import (
 	"sync"
 )
 
+// TODO: no documentation; [54dbca2b-17f2-42a8-af93-c8d713866138]
 func NewChanResponsePair(req *Request) (ResponseEmitter, Response) {
 	r := &chanResponse{
 		req:     req,

command.go

@@ -173,8 +173,8 @@ func (c *Command) call(req *Request, re ResponseEmitter, env Environment) error
 	return cmd.Run(req, re, env)
 }
 
-// Resolve returns the subcommands at the given path
-// The returned set of subcommands starts with this command and therefore is always at least size 1
+// Resolve returns the subcommands at the given path.
+// The returned set of subcommands starts with this command and therefore is always at least size 1.
 func (c *Command) Resolve(pth []string) ([]*Command, error) {
 	cmds := make([]*Command, len(pth)+1)
 	cmds[0] = c
@@ -233,6 +233,10 @@ func (c *Command) GetOptions(path []string) (map[string]Option, error) {
 // DebugValidate checks if the command tree is well-formed.
 //
 // This operation is slow and should be called from tests only.
+//
+// TODO: review this; I don't see any reason this needs to be attached to all `Command`s
+// rather than being a function which takes in `Command`.
+// ValidateCommand(cmd)[]error|<-chan error; called from tests only.
 func (c *Command) DebugValidate() map[string][]error {
 	errs := make(map[string][]error)
 	var visit func(path string, cm *Command)
@@ -348,6 +352,7 @@ func (c *Command) CheckArguments(req *Request) error {
 	return nil
 }
 
+// TODO: no documentation; [54dbca2b-17f2-42a8-af93-c8d713866138]
 type CommandVisitor func(*Command)
 
 // Walks tree of all subcommands (including this one)
@@ -358,6 +363,7 @@ func (c *Command) Walk(visitor CommandVisitor) {
 	}
 }
 
+// TODO: no documentation; [54dbca2b-17f2-42a8-af93-c8d713866138]
 func (c *Command) ProcessHelp() {
 	c.Walk(func(cm *Command) {
 		ht := &cm.Helptext
@@ -367,6 +373,7 @@ func (c *Command) ProcessHelp() {
 	})
 }
 
+// TODO: no documentation; [54dbca2b-17f2-42a8-af93-c8d713866138]
 func ClientError(msg string) error {
 	return &Error{Code: ErrClient, Message: msg}
 }

command_test.go

@@ -374,9 +374,9 @@ func TestEmitterExpectError(t *testing.T) {
 
 	switch re.errorCount {
 	case 0:
-		t.Errorf("expected SetError to be called")
+		t.Errorf("expected CloseWithError to be called")
 	case 1:
 	default:
-		t.Errorf("expected SetError to be called once, but was called %d times", re.errorCount)
+		t.Errorf("expected CloseWithError to be called once, but was called %d times", re.errorCount)
 	}
 }

doc.go

@@ -26,8 +26,6 @@
 
 	Emitters
 
-
-
 	An emitter has the Emit method, that takes the command's
 	function's output as an argument and passes it to the user.
 
@@ -49,7 +47,6 @@
 
 	Responses
 
-
 	A response is a value that the user can read emitted values
 	from.
 
@@ -60,16 +57,19 @@
 			Next() (interface{}, error)
 		}
 
+	TODO: logic pass; docs are several years out of date.
+	TODO: English pass; <-AME (the whole file needs a pass anyway, so do that).
+
 	Responses have a method Next() that returns the next
 	emitted value and an error value. If the last element has been
 	received, the returned error value is io.EOF. If the
-	application code has sent an error using SetError, the error
-	ErrRcvdError is returned on next, indicating that the caller
-	should call Error(). Depending on the reponse type, other
-	errors may also occur.
+	application's code encounters a fatal error, it will call CloseWithError,
+	and that the error value will be returned via subsiquent calls to Next().
 
 	Pipes
 
+	TODO: ^ was this an actual type/interface or was this always an abstract metaphor?
+
 	Pipes are pairs (emitter, response), such that a value emitted
 	on the emitter can be received in the response value. Most
 	builtin emitters are "pipe" emitters. The most prominent

encoding.go

@@ -49,7 +49,10 @@ var Decoders = map[EncodingType]func(w io.Reader) Decoder{
 	},
 }
 
+// TODO: no documentation; [54dbca2b-17f2-42a8-af93-c8d713866138]
 type EncoderFunc func(req *Request) func(w io.Writer) Encoder
+
+// TODO: no documentation; [54dbca2b-17f2-42a8-af93-c8d713866138]
 type EncoderMap map[EncodingType]EncoderFunc
 
 var Encoders = EncoderMap{
@@ -67,12 +70,14 @@ var Encoders = EncoderMap{
 	},
 }
 
+// TODO: no documentation; [54dbca2b-17f2-42a8-af93-c8d713866138]
 func MakeEncoder(f func(*Request, io.Writer, interface{}) error) func(*Request) func(io.Writer) Encoder {
 	return func(req *Request) func(io.Writer) Encoder {
 		return func(w io.Writer) Encoder { return &genericEncoder{f: f, w: w, req: req} }
 	}
 }
 
+// TODO: no documentation; [54dbca2b-17f2-42a8-af93-c8d713866138]
 func MakeTypedEncoder(f interface{}) func(*Request) func(io.Writer) Encoder {
 	val := reflect.ValueOf(f)
 	t := val.Type()

examples/adder/local/main.go

@@ -2,53 +2,166 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 
 	"github.com/ipfs/go-ipfs-cmds/examples/adder"
 
-	"github.com/ipfs/go-ipfs-cmds"
+	cmds "github.com/ipfs/go-ipfs-cmds"
 	"github.com/ipfs/go-ipfs-cmds/cli"
 )
 
+/* TODO:
+- there's too many words in here
+- text style consistency; sleepy case is currently used with Chicago norm expected as majority
+- We should also move a lot of this into the subcommand definition and refer to that
+instead, of documenting internals in very big comments
+- this would allow us to utilize env inside of `Run` which is important for demonstration
+- Need to trace this when done and make sure the comments are telling the truth about the execution path
+- Should probably throw in a custom emitter example somewhere since we defined one elsewhere
+- and many more
+*/
+
+// `cli.Run` will read a `cmds.Command`'s fields and perform standard behaviour
+// for package defined values.
+// For example, if the `Command` declares it has `cmds.OptLongHelp` or `cmds.OptShortHelp` options,
+// `cli.Run` will check for and handle these flags automatically (but they are not required).
+//
+// If needed, the caller may define an arbitrary "environment"
+// and expect to receive this environment during execution of the `Command`.
+// While not required we'll define one for the sake of example.
+type (
+	envChan chan error
+
+	ourEnviornment struct {
+		whateverWeNeed envChan
+	}
+)
+
+// A `Close` method is also optional; if defined, it's deferred inside of `cli.Run`.
+func (env *ourEnviornment) Close() error {
+	if env.whateverWeNeed != nil {
+		close(env.whateverWeNeed)
+	}
+	return nil
+}
+
+// If desired, the caller may define additional methods that
+// they may need during execution of the `cmds.Command`.
+// Considering the environment constructor returns an untyped interface,
+// it's a good idea to define additional interfaces that can be used for behaviour checking.
+// (Especially if you choose to return different concrete environments for different requests.)
+func (env *ourEnviornment) getChan() envChan {
+	return env.whateverWeNeed
+}
+
+type specificEnvironment interface {
+	getChan() envChan
+}
+
+// While the environment itself is not be required,
+// its constructor and receiver methods are.
+// We'll define them here without any special request parsing, since we don't need it.
+func makeOurEnvironment(ctx context.Context, req *cmds.Request) (cmds.Environment, error) {
+	return &ourEnviornment{
+		whateverWeNeed: make(envChan),
+	}, nil
+}
+func makeOurExecutor(req *cmds.Request, env interface{}) (cmds.Executor, error) {
+	return cmds.NewExecutor(adder.RootCmd), nil
+}
+
 func main() {
+	var (
+		ctx = context.TODO()
+		// If the environment constructor does not return an error
+		// it will pass the environment to the `cmds.Executor` within `cli.Run`;
+		// which passes it to the `Command`'s (optional)`PreRun` and/or `Run` methods.
+		err = cli.Run(ctx, adder.RootCmd, os.Args, // pass in command and args to parse
+			os.Stdin, os.Stdout, os.Stderr, // along with output writers
+			makeOurEnvironment, makeOurExecutor) // and our constructor+receiver pair
+	)
+	cliError := new(cli.ExitError)
+	if errors.As(err, cliError) {
+		os.Exit(int((*cliError)))
+	}
+}
+
+// `cli.Run` is a convenient wrapper and not required.
+// If desired, the caller may define the entire means to process a `cmds.Command` themselves.
+func altMain() {
+	ctx := context.TODO()
+
 	// parse the command path, arguments and options from the command line
-	req, err := cli.Parse(context.TODO(), os.Args[1:], os.Stdin, adder.RootCmd)
+	request, err := cli.Parse(ctx, os.Args[1:], os.Stdin, adder.RootCmd)
 	if err != nil {
 		panic(err)
 	}
+	request.Options[cmds.EncLong] = cmds.Text
 
-	req.Options["encoding"] = cmds.Text
+	// create an environment from the request
+	cmdEnv, err := makeOurEnvironment(ctx, request)
+	if err != nil {
+		panic(err)
+	}
 
-	// create an emitter
-	cliRe, err := cli.NewResponseEmitter(os.Stdout, os.Stderr, req)
+	// get values specific to our request+environment pair
+	wait, err := customPreRun(request, cmdEnv)
 	if err != nil {
 		panic(err)
 	}
 
-	wait := make(chan struct{})
-	var re cmds.ResponseEmitter = cliRe
-	if pr, ok := req.Command.PostRun[cmds.CLI]; ok {
-		var (
-			res   cmds.Response
-			lower = re
-		)
-
-		re, res = cmds.NewChanResponsePair(req)
-
-		go func() {
-			defer close(wait)
-			err := pr(res, lower)
-			if err != nil {
-				fmt.Println("error: ", err)
-			}
-		}()
-	} else {
-		close(wait)
+	// This emitter's `Emit` method will be called from within the `Command`'s `Run` method.
+	// If `Run` encounters a fatal error, the emitter should be closed with `emitter.CloseWithError(err)`
+	// otherwise, it will be closed automatically after `Run` within `Call`.
+	var emitter cmds.ResponseEmitter
+	emitter, err = cli.NewResponseEmitter(os.Stdout, os.Stderr, request)
+	if err != nil {
+		panic(err)
+	}
+
+	// if the command has a `PostRun` method, emit responses to it instead
+	emitter = maybePostRun(request, emitter, wait)
+
+	// call the actual `Run` method on the command
+	adder.RootCmd.Call(request, emitter, cmdEnv)
+	err = <-wait
+
+	cliError := new(cli.ExitError)
+	if errors.As(err, cliError) {
+		os.Exit(int((*cliError)))
 	}
+}
+
+func customPreRun(req *cmds.Request, env cmds.Environment) (envChan, error) {
+	// check that the constructor passed us the environment we expect/need
+	ourEnvIntf, ok := env.(specificEnvironment)
+	if !ok {
+		return nil, fmt.Errorf("environment received does not satisfy expected interface")
+	}
+	return ourEnvIntf.getChan(), nil
+}
 
-	adder.RootCmd.Call(req, re, nil)
-	<-wait
+func maybePostRun(req *cmds.Request, emitter cmds.ResponseEmitter, wait envChan) cmds.ResponseEmitter {
+	postRun, provided := req.Command.PostRun[cmds.CLI]
+	if !provided { // no `PostRun` command was defined
+		close(wait) // don't do anything and unblock instantly
+		return emitter
+	}
+
+	var ( // store the emitter passed to us
+		postRunEmitter = emitter
+		response       cmds.Response
+	)
+	// replace the caller's emitter with one that emits to this `Response` interface
+	emitter, response = cmds.NewChanResponsePair(req)
+
+	go func() { // start listening for emission on the emitter
+		// wait for `PostRun` to return, and send its value to the caller
+		wait <- postRun(response, postRunEmitter)
+		close(wait)
+	}()
 
-	os.Exit(cliRe.Status())
+	return emitter
 }

executor.go

@@ -4,6 +4,7 @@ import (
 	"context"
 )
 
+// TODO: no documentation; [54dbca2b-17f2-42a8-af93-c8d713866138]
 type Executor interface {
 	Execute(req *Request, re ResponseEmitter, env Environment) error
 }
@@ -22,6 +23,7 @@ type MakeEnvironment func(context.Context, *Request) (Environment, error)
 // The user can define a function like this to pass it to cli.Run.
 type MakeExecutor func(*Request, interface{}) (Executor, error)
 
+// TODO: no documentation; [54dbca2b-17f2-42a8-af93-c8d713866138]
 func NewExecutor(root *Command) Executor {
 	return &executor{
 		root: root,

flushfwd.go

@@ -18,6 +18,7 @@ func (ff *flushfwder) Close() error {
 	return ff.ResponseEmitter.Close()
 }
 
+// TODO: no documentation; [54dbca2b-17f2-42a8-af93-c8d713866138]
 func NewFlushForwarder(re ResponseEmitter, f Flusher) ResponseEmitter {
 	return &flushfwder{ResponseEmitter: re, Flusher: f}
 }

responseemitter.go

@@ -41,7 +41,6 @@ type ResponseEmitter interface {
 	CloseWithError(error) error
 
 	// SetLength sets the length of the output
-	// err is an interface{} so we don't have to manually convert to error.
 	SetLength(length uint64)
 
 	// Emit sends a value.
@@ -71,6 +70,7 @@ func Copy(re ResponseEmitter, res Response) error {
 	}
 }
 
+// TODO: no documentation; [54dbca2b-17f2-42a8-af93-c8d713866138]
 func EmitChan(re ResponseEmitter, ch <-chan interface{}) error {
 	for v := range ch {
 		err := re.Emit(v)