v3 (#58)

* Bump to v3, no changes yet

* NoExecError: when terminal command has no func Exec

* WithEnvVarIgnoreCommas -> WithEnvVarSplit

* Priority: env file args -> file env args

* go mod tidy

* Update READMEs with v3

README.md

@@ -1,13 +1,13 @@
-# ff [![go.dev reference](https://img.shields.io/badge/go.dev-reference-007d9c?logo=go&logoColor=white&style=flat-square)](https://pkg.go.dev/github.com/peterbourgon/ff/v2) [![Latest Release](https://img.shields.io/github/release/peterbourgon/ff.svg?style=flat-square)](https://github.com/peterbourgon/ff/releases/latest) [![Build Status](https://img.shields.io/endpoint.svg?url=https%3A%2F%2Factions-badge.atrox.dev%2Fpeterbourgon%2Fff%2Fbadge&style=flat-square&label=build)](https://github.com/peterbourgon/ff/actions?query=workflow%3ATest)
+# ff [![go.dev reference](https://img.shields.io/badge/go.dev-reference-007d9c?logo=go&logoColor=white&style=flat-square)](https://pkg.go.dev/github.com/peterbourgon/ff/v3) [![Latest Release](https://img.shields.io/github/release/peterbourgon/ff.svg?style=flat-square)](https://github.com/peterbourgon/ff/releases/latest) [![Build Status](https://img.shields.io/endpoint.svg?url=https%3A%2F%2Factions-badge.atrox.dev%2Fpeterbourgon%2Fff%2Fbadge&style=flat-square&label=build)](https://github.com/peterbourgon/ff/actions?query=workflow%3ATest)
 
-ff stands for flags-first, and provides an opinionated way to populate
-a [flag.FlagSet](https://golang.org/pkg/flag#FlagSet) with
-configuration data from the environment. By default, it parses only
-from the command line, but you can enable parsing from a configuration
-file (lower priority) and/or environment variables (lowest priority).
+ff stands for flags-first, and provides an opinionated way to populate a
+[flag.FlagSet](https://golang.org/pkg/flag#FlagSet) with configuration data from
+the environment. By default, it parses only from the command line, but you can
+enable parsing from environment variables (lower priority) and/or a
+configuration file (lowest priority).
 
 Building a commandline application in the style of `kubectl` or `docker`?
-Consider [package ffcli](https://pkg.go.dev/github.com/peterbourgon/ff/v2/ffcli),
+Consider [package ffcli](https://pkg.go.dev/github.com/peterbourgon/ff/v3/ffcli),
 a natural companion to, and extension of, package ff.
 
 ## Usage
@@ -20,7 +20,7 @@ import (
 	"os"
 	"time"
 
-	"github.com/peterbourgon/ff/v2"
+	"github.com/peterbourgon/ff/v3"
 )
 
 func main() {
@@ -34,20 +34,27 @@ func main() {
 ```
 
 Then, call ff.Parse instead of fs.Parse.
-[Options](https://pkg.go.dev/github.com/peterbourgon/ff/v2#Option)
+[Options](https://pkg.go.dev/github.com/peterbourgon/ff/v3#Option)
 are available to control parse behavior.
 
 ```go
 	ff.Parse(fs, os.Args[1:],
+		ff.WithEnvVarPrefix("MY_PROGRAM"),
 		ff.WithConfigFileFlag("config"),
 		ff.WithConfigFileParser(ff.PlainParser),
-		ff.WithEnvVarPrefix("MY_PROGRAM"),
 	)
 ```
 
 This example will parse flags from the commandline args, just like regular
-package flag, with the highest priority. If a `-config` file is specified, it
-will try to parse it using the PlainParser, which expects files in this format.
+package flag, with the highest priority.
+
+Additionally, the example will look in the environment for variables with a
+`MY_PROGRAM` prefix. Flag names are capitalized, and separator characters are
+converted to underscores. In this case, for example, `MY_PROGRAM_LISTEN_ADDR`
+would match to `listen-addr`.
+
+Finally, if a `-config` file is specified, the example will try to parse it
+using the PlainParser, which expects files in this format.
 
 ```
 listen-addr localhost:8080
@@ -73,14 +80,6 @@ Or, you could write your own config file parser.
 type ConfigFileParser func(r io.Reader, set func(name, value string) error) error
 ```
 
-Finally, the example will look in the environment for variables with a
-`MY_PROGRAM` prefix. Flag names are capitalized, and separator characters are
-converted to underscores. In this case, for example, `MY_PROGRAM_LISTEN_ADDR`
-would match to `listen-addr`. Parsing of env vars containing commas has special
-behavior, see
-[WithEnvVarIgnoreCommas](https://pkg.go.dev/github.com/peterbourgon/ff/v2?tab=doc#WithEnvVarIgnoreCommas)
-for details.
-
 ## Flags and env vars
 
 One common use case is to allow configuration from both flags and env vars.
@@ -93,7 +92,7 @@ import (
 	"fmt"
 	"os"
 
-	"github.com/peterbourgon/ff/v2"
+	"github.com/peterbourgon/ff/v3"
 )
 
 func main() {

ffcli/README.md

@@ -1,6 +1,6 @@
-# ffcli [![GoDoc](https://godoc.org/github.com/peterbourgon/ff/ffcli?status.svg)](https://godoc.org/github.com/peterbourgon/ff/ffcli)
+# ffcli [![go.dev reference](https://img.shields.io/badge/go.dev-reference-007d9c?logo=go&logoColor=white&style=flat-square)](https://pkg.go.dev/github.com/peterbourgon/ff/v3/ffcli)
 
-ffcli stands for flags-first command line interface, 
+ffcli stands for flags-first command line interface,
 and provides an opinionated way to build CLIs.
 
 ## Rationale
@@ -60,7 +60,7 @@ import (
 	"context"
 	"os"
 
-	"github.com/peterbourgon/ff/v2/ffcli"
+	"github.com/peterbourgon/ff/v3/ffcli"
 )
 
 func main() {

ffcli/command.go

@@ -8,7 +8,7 @@ import (
 	"strings"
 	"text/tabwriter"
 
-	"github.com/peterbourgon/ff/v2"
+	"github.com/peterbourgon/ff/v3"
 )
 
 // Command combines a main function with a flag.FlagSet, and zero or more
@@ -66,18 +66,27 @@ type Command struct {
 	args     []string // args that should be passed to Run, if any
 
 	// Exec is invoked if this command has been determined to be the terminal
-	// command selected by the arguments provided to Run. The args passed to
-	// Exec are the args left over after flags parsing. Optional.
+	// command selected by the arguments provided to Parse or ParseAndRun. The
+	// args passed to Exec are the args left over after flags parsing. Optional.
 	//
-	// If Exec returns flag.ErrHelp, Run will behave as if -h were passed and
-	// emit the complete usage output.
+	// If Exec returns flag.ErrHelp, then Run (or ParseAndRun) will behave as if
+	// -h were passed and emit the complete usage output.
+	//
+	// If Exec is nil, and this command is identified as the terminal command,
+	// then Parse, Run, and ParseAndRun will all return NoExecError. Callers may
+	// check for this error and print e.g. help or usage text to the user, in
+	// effect treating some commands as just collections of subcommands, rather
+	// than being invocable themselves.
 	Exec func(ctx context.Context, args []string) error
 }
 
 // Parse the commandline arguments for this command and all sub-commands
-// recursively, defining flags along the way. If Parse returns without
-// an error, the terminal command has been successfully identified, and may
-// be invoked by calling Run.
+// recursively, defining flags along the way. If Parse returns without an error,
+// the terminal command has been successfully identified, and may be invoked by
+// calling Run.
+//
+// If the terminal command identified by Parse doesn't define an Exec function,
+// then Parse will return NoExecError.
 func (c *Command) Parse(args []string) error {
 	if c.selected != nil {
 		return nil
@@ -110,15 +119,20 @@ func (c *Command) Parse(args []string) error {
 	}
 
 	c.selected = c
+
+	if c.Exec == nil {
+		return NoExecError{Command: c}
+	}
+
 	return nil
 }
 
-// ErrUnparsed is returned by Run if Parse hasn't been called first.
-var ErrUnparsed = errors.New("command tree is unparsed, can't run")
-
 // Run selects the terminal command in a command tree previously identified by a
 // successful call to Parse, and calls that command's Exec function with the
 // appropriate subset of commandline args.
+//
+// If the terminal command previously identified by Parse doesn't define an Exec
+// function, then Run will return NoExecError.
 func (c *Command) Run(ctx context.Context) (err error) {
 	var (
 		unparsed = c.selected == nil
@@ -138,7 +152,7 @@ func (c *Command) Run(ctx context.Context) (err error) {
 	case terminal:
 		return c.Exec(ctx, c.args)
 	case noop:
-		return nil
+		return NoExecError{Command: c}
 	default:
 		return c.selected.Run(ctx)
 	}
@@ -159,6 +173,28 @@ func (c *Command) ParseAndRun(ctx context.Context, args []string) error {
 	return nil
 }
 
+//
+//
+//
+
+// ErrUnparsed is returned by Run if Parse hasn't been called first.
+var ErrUnparsed = errors.New("command tree is unparsed, can't run")
+
+// NoExecError is returned if the terminal command selected during the parse
+// phase doesn't define an Exec function.
+type NoExecError struct {
+	Command *Command
+}
+
+// Error implements the error interface.
+func (e NoExecError) Error() string {
+	return fmt.Sprintf("terminal command (%s) doesn't define an Exec function", e.Command.Name)
+}
+
+//
+//
+//
+
 // DefaultUsageFunc is the default UsageFunc used for all commands
 // if no custom UsageFunc is provided.
 func DefaultUsageFunc(c *Command) string {

ffcli/command_test.go

@@ -6,14 +6,15 @@ import (
 	"errors"
 	"flag"
 	"fmt"
+	"io/ioutil"
 	"log"
 	"reflect"
 	"strings"
 	"testing"
 	"time"
 
-	"github.com/peterbourgon/ff/v2/ffcli"
-	"github.com/peterbourgon/ff/v2/fftest"
+	"github.com/peterbourgon/ff/v3/ffcli"
+	"github.com/peterbourgon/ff/v3/fftest"
 )
 
 func TestCommandRun(t *testing.T) {
@@ -320,6 +321,124 @@ func TestNestedOutput(t *testing.T) {
 	}
 }
 
+func TestIssue57(t *testing.T) {
+	t.Parallel()
+
+	for _, testcase := range []struct {
+		args        []string
+		parseErrAs  error
+		parseErrIs  error
+		parseErrStr string
+		runErrAs    error
+		runErrIs    error
+		runErrStr   string
+	}{
+		{
+			args:       []string{},
+			parseErrAs: &ffcli.NoExecError{},
+			runErrAs:   &ffcli.NoExecError{},
+		},
+		{
+			args:       []string{"-h"},
+			parseErrIs: flag.ErrHelp,
+			runErrIs:   ffcli.ErrUnparsed,
+		},
+		{
+			args:       []string{"bar"},
+			parseErrAs: &ffcli.NoExecError{},
+			runErrAs:   &ffcli.NoExecError{},
+		},
+		{
+			args:       []string{"bar", "-h"},
+			parseErrAs: flag.ErrHelp,
+			runErrAs:   ffcli.ErrUnparsed,
+		},
+		{
+			args:        []string{"bar", "-undefined"},
+			parseErrStr: "error parsing commandline args: flag provided but not defined: -undefined",
+			runErrIs:    ffcli.ErrUnparsed,
+		},
+		{
+			args: []string{"bar", "baz"},
+		},
+		{
+			args:       []string{"bar", "baz", "-h"},
+			parseErrIs: flag.ErrHelp,
+			runErrIs:   ffcli.ErrUnparsed,
+		},
+		{
+			args:        []string{"bar", "baz", "-also.undefined"},
+			parseErrStr: "error parsing commandline args: flag provided but not defined: -also.undefined",
+			runErrIs:    ffcli.ErrUnparsed,
+		},
+	} {
+		t.Run(strings.Join(append([]string{"foo"}, testcase.args...), " "), func(t *testing.T) {
+			fs := flag.NewFlagSet("·", flag.ContinueOnError)
+			fs.SetOutput(ioutil.Discard)
+
+			var (
+				baz = &ffcli.Command{Name: "baz", FlagSet: fs, Exec: func(_ context.Context, args []string) error { return nil }}
+				bar = &ffcli.Command{Name: "bar", FlagSet: fs, Subcommands: []*ffcli.Command{baz}}
+				foo = &ffcli.Command{Name: "foo", FlagSet: fs, Subcommands: []*ffcli.Command{bar}}
+			)
+
+			var (
+				parseErr = foo.Parse(testcase.args)
+				runErr   = foo.Run(context.Background())
+			)
+
+			if testcase.parseErrAs != nil {
+				if want, have := &testcase.parseErrAs, parseErr; !errors.As(have, want) {
+					t.Errorf("Parse: want %v, have %v", want, have)
+				}
+			}
+
+			if testcase.parseErrIs != nil {
+				if want, have := testcase.parseErrIs, parseErr; !errors.Is(have, want) {
+					t.Errorf("Parse: want %v, have %v", want, have)
+				}
+			}
+
+			if testcase.parseErrStr != "" {
+				if want, have := testcase.parseErrStr, parseErr.Error(); want != have {
+					t.Errorf("Parse: want %q, have %q", want, have)
+				}
+			}
+
+			if testcase.runErrAs != nil {
+				if want, have := &testcase.runErrAs, runErr; !errors.As(have, want) {
+					t.Errorf("Run: want %v, have %v", want, have)
+				}
+			}
+
+			if testcase.runErrIs != nil {
+				if want, have := testcase.runErrIs, runErr; !errors.Is(have, want) {
+					t.Errorf("Run: want %v, have %v", want, have)
+				}
+			}
+
+			if testcase.runErrStr != "" {
+				if want, have := testcase.runErrStr, runErr.Error(); want != have {
+					t.Errorf("Run: want %q, have %q", want, have)
+				}
+			}
+
+			var (
+				noParseErr = testcase.parseErrAs == nil && testcase.parseErrIs == nil && testcase.parseErrStr == ""
+				noRunErr   = testcase.runErrAs == nil && testcase.runErrIs == nil && testcase.runErrStr == ""
+			)
+			if noParseErr && noRunErr {
+				if parseErr != nil {
+					t.Errorf("Parse: unexpected error: %v", parseErr)
+				}
+				if runErr != nil {
+					t.Errorf("Run: unexpected error: %v", runErr)
+				}
+			}
+		})
+	}
+}
+
 func ExampleCommand_Parse_then_Run() {
 	// Assume our CLI will use some client that requires a token.
 	type FooClient struct {

ffcli/examples/objectctl/cmd/objectctl/main.go

@@ -5,12 +5,12 @@ import (
 	"fmt"
 	"os"
 
-	"github.com/peterbourgon/ff/v2/ffcli"
-	"github.com/peterbourgon/ff/v2/ffcli/examples/objectctl/pkg/createcmd"
-	"github.com/peterbourgon/ff/v2/ffcli/examples/objectctl/pkg/deletecmd"
-	"github.com/peterbourgon/ff/v2/ffcli/examples/objectctl/pkg/listcmd"
-	"github.com/peterbourgon/ff/v2/ffcli/examples/objectctl/pkg/objectapi"
-	"github.com/peterbourgon/ff/v2/ffcli/examples/objectctl/pkg/rootcmd"
+	"github.com/peterbourgon/ff/v3/ffcli"
+	"github.com/peterbourgon/ff/v3/ffcli/examples/objectctl/pkg/createcmd"
+	"github.com/peterbourgon/ff/v3/ffcli/examples/objectctl/pkg/deletecmd"
+	"github.com/peterbourgon/ff/v3/ffcli/examples/objectctl/pkg/listcmd"
+	"github.com/peterbourgon/ff/v3/ffcli/examples/objectctl/pkg/objectapi"
+	"github.com/peterbourgon/ff/v3/ffcli/examples/objectctl/pkg/rootcmd"
 )
 
 func main() {

ffcli/examples/objectctl/pkg/createcmd/create.go

@@ -8,8 +8,8 @@ import (
 	"io"
 	"strings"
 
-	"github.com/peterbourgon/ff/v2/ffcli"
-	"github.com/peterbourgon/ff/v2/ffcli/examples/objectctl/pkg/rootcmd"
+	"github.com/peterbourgon/ff/v3/ffcli"
+	"github.com/peterbourgon/ff/v3/ffcli/examples/objectctl/pkg/rootcmd"
 )
 
 // Config for the create subcommand, including a reference to the API client.

ffcli/examples/objectctl/pkg/deletecmd/delete.go

@@ -7,8 +7,8 @@ import (
 	"fmt"
 	"io"
 
-	"github.com/peterbourgon/ff/v2/ffcli"
-	"github.com/peterbourgon/ff/v2/ffcli/examples/objectctl/pkg/rootcmd"
+	"github.com/peterbourgon/ff/v3/ffcli"
+	"github.com/peterbourgon/ff/v3/ffcli/examples/objectctl/pkg/rootcmd"
 )
 
 // Config for the delete subcommand, including a reference to the API client.