From bb21f1eb44c435821611fab154f53d5d19b61e5a Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Sun, 8 Mar 2020 16:16:29 +0100 Subject: [PATCH 1/6] Bump to v3, no changes yet --- ffcli/command.go | 2 +- ffcli/command_test.go | 4 ++-- ffcli/examples/objectctl/cmd/objectctl/main.go | 12 ++++++------ ffcli/examples/objectctl/pkg/createcmd/create.go | 4 ++-- ffcli/examples/objectctl/pkg/deletecmd/delete.go | 4 ++-- ffcli/examples/objectctl/pkg/listcmd/list.go | 4 ++-- ffcli/examples/objectctl/pkg/rootcmd/root.go | 4 ++-- ffcli/examples/textctl/textctl.go | 2 +- fftoml/fftoml.go | 2 +- fftoml/fftoml_test.go | 6 +++--- ffyaml/ffyaml.go | 2 +- ffyaml/ffyaml_test.go | 6 +++--- go.mod | 4 ++-- json_test.go | 4 ++-- parse_test.go | 4 ++-- 15 files changed, 32 insertions(+), 32 deletions(-) diff --git a/ffcli/command.go b/ffcli/command.go index 0e7fd91..de613c5 100644 --- a/ffcli/command.go +++ b/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 diff --git a/ffcli/command_test.go b/ffcli/command_test.go index 2e67366..6d64370 100644 --- a/ffcli/command_test.go +++ b/ffcli/command_test.go @@ -12,8 +12,8 @@ import ( "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) { diff --git a/ffcli/examples/objectctl/cmd/objectctl/main.go b/ffcli/examples/objectctl/cmd/objectctl/main.go index a202a2a..ec47ba1 100644 --- a/ffcli/examples/objectctl/cmd/objectctl/main.go +++ b/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() { diff --git a/ffcli/examples/objectctl/pkg/createcmd/create.go b/ffcli/examples/objectctl/pkg/createcmd/create.go index 71faf28..d3702fc 100644 --- a/ffcli/examples/objectctl/pkg/createcmd/create.go +++ b/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. diff --git a/ffcli/examples/objectctl/pkg/deletecmd/delete.go b/ffcli/examples/objectctl/pkg/deletecmd/delete.go index d77e15e..94743d5 100644 --- a/ffcli/examples/objectctl/pkg/deletecmd/delete.go +++ b/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. diff --git a/ffcli/examples/objectctl/pkg/listcmd/list.go b/ffcli/examples/objectctl/pkg/listcmd/list.go index b120a0a..5d4b243 100644 --- a/ffcli/examples/objectctl/pkg/listcmd/list.go +++ b/ffcli/examples/objectctl/pkg/listcmd/list.go @@ -8,8 +8,8 @@ import ( "text/tabwriter" "time" - "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 list subcommand, including a reference diff --git a/ffcli/examples/objectctl/pkg/rootcmd/root.go b/ffcli/examples/objectctl/pkg/rootcmd/root.go index db259fc..329b7b3 100644 --- a/ffcli/examples/objectctl/pkg/rootcmd/root.go +++ b/ffcli/examples/objectctl/pkg/rootcmd/root.go @@ -4,8 +4,8 @@ import ( "context" "flag" - "github.com/peterbourgon/ff/v2/ffcli" - "github.com/peterbourgon/ff/v2/ffcli/examples/objectctl/pkg/objectapi" + "github.com/peterbourgon/ff/v3/ffcli" + "github.com/peterbourgon/ff/v3/ffcli/examples/objectctl/pkg/objectapi" ) // Config for the root command, including flags and types that should be diff --git a/ffcli/examples/textctl/textctl.go b/ffcli/examples/textctl/textctl.go index 446cf60..201daa9 100644 --- a/ffcli/examples/textctl/textctl.go +++ b/ffcli/examples/textctl/textctl.go @@ -6,7 +6,7 @@ import ( "fmt" "os" - "github.com/peterbourgon/ff/v2/ffcli" + "github.com/peterbourgon/ff/v3/ffcli" ) // textctl is a simple applications in which all commands are built up in func diff --git a/fftoml/fftoml.go b/fftoml/fftoml.go index d132dba..d57451d 100644 --- a/fftoml/fftoml.go +++ b/fftoml/fftoml.go @@ -7,7 +7,7 @@ import ( "strconv" "github.com/pelletier/go-toml" - "github.com/peterbourgon/ff/v2" + "github.com/peterbourgon/ff/v3" ) // Parser is a parser for TOML file format. Flags and their values are read diff --git a/fftoml/fftoml_test.go b/fftoml/fftoml_test.go index 4dca378..c2b035b 100644 --- a/fftoml/fftoml_test.go +++ b/fftoml/fftoml_test.go @@ -6,9 +6,9 @@ import ( "testing" "time" - "github.com/peterbourgon/ff/v2" - "github.com/peterbourgon/ff/v2/fftest" - "github.com/peterbourgon/ff/v2/fftoml" + "github.com/peterbourgon/ff/v3" + "github.com/peterbourgon/ff/v3/fftest" + "github.com/peterbourgon/ff/v3/fftoml" ) func TestParser(t *testing.T) { diff --git a/ffyaml/ffyaml.go b/ffyaml/ffyaml.go index e2a607d..9421998 100644 --- a/ffyaml/ffyaml.go +++ b/ffyaml/ffyaml.go @@ -6,7 +6,7 @@ import ( "io" "strconv" - "github.com/peterbourgon/ff/v2" + "github.com/peterbourgon/ff/v3" "gopkg.in/yaml.v2" ) diff --git a/ffyaml/ffyaml_test.go b/ffyaml/ffyaml_test.go index 423b2b2..8cc42e1 100644 --- a/ffyaml/ffyaml_test.go +++ b/ffyaml/ffyaml_test.go @@ -4,9 +4,9 @@ import ( "testing" "time" - "github.com/peterbourgon/ff/v2" - "github.com/peterbourgon/ff/v2/fftest" - "github.com/peterbourgon/ff/v2/ffyaml" + "github.com/peterbourgon/ff/v3" + "github.com/peterbourgon/ff/v3/fftest" + "github.com/peterbourgon/ff/v3/ffyaml" ) func TestParser(t *testing.T) { diff --git a/go.mod b/go.mod index b3f3d11..5be894a 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ -module github.com/peterbourgon/ff/v2 +module github.com/peterbourgon/ff/v3 -go 1.13 +go 1.14 require ( github.com/pelletier/go-toml v1.6.0 diff --git a/json_test.go b/json_test.go index c72b40a..7929da6 100644 --- a/json_test.go +++ b/json_test.go @@ -5,8 +5,8 @@ import ( "testing" "time" - "github.com/peterbourgon/ff/v2" - "github.com/peterbourgon/ff/v2/fftest" + "github.com/peterbourgon/ff/v3" + "github.com/peterbourgon/ff/v3/fftest" ) func TestJSONParser(t *testing.T) { diff --git a/parse_test.go b/parse_test.go index db9d016..a3c00eb 100644 --- a/parse_test.go +++ b/parse_test.go @@ -5,8 +5,8 @@ import ( "testing" "time" - "github.com/peterbourgon/ff/v2" - "github.com/peterbourgon/ff/v2/fftest" + "github.com/peterbourgon/ff/v3" + "github.com/peterbourgon/ff/v3/fftest" ) func TestParseBasics(t *testing.T) { From 168dabc6dbd2df48b8c644c03f899270897fea0b Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Sun, 8 Mar 2020 23:05:28 +0100 Subject: [PATCH 2/6] NoExecError: when terminal command has no func Exec --- ffcli/command.go | 58 ++++++++++++++++---- ffcli/command_test.go | 119 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+), 11 deletions(-) diff --git a/ffcli/command.go b/ffcli/command.go index de613c5..3fa335f 100644 --- a/ffcli/command.go +++ b/ffcli/command.go @@ -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 { diff --git a/ffcli/command_test.go b/ffcli/command_test.go index 6d64370..2c31c63 100644 --- a/ffcli/command_test.go +++ b/ffcli/command_test.go @@ -6,6 +6,7 @@ import ( "errors" "flag" "fmt" + "io/ioutil" "log" "reflect" "strings" @@ -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 { From 14458fed8df389a5dfbf7b780bb286611e43b48b Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Mon, 9 Mar 2020 21:29:50 +0100 Subject: [PATCH 3/6] WithEnvVarIgnoreCommas -> WithEnvVarSplit --- parse.go | 150 ++++++++++++++++++++++++++++++-------------------- parse_test.go | 26 ++++++--- 2 files changed, 108 insertions(+), 68 deletions(-) diff --git a/parse.go b/parse.go index 873b6a2..6473776 100644 --- a/parse.go +++ b/parse.go @@ -18,6 +18,7 @@ func Parse(fs *flag.FlagSet, args []string, options ...Option) error { option(&c) } + // First priority: commandline flags (explicit user preference). if err := fs.Parse(args); err != nil { return fmt.Errorf("error parsing commandline args: %w", err) } @@ -27,38 +28,44 @@ func Parse(fs *flag.FlagSet, args []string, options ...Option) error { provided[f.Name] = true }) + // TEMPORARY second priority: config file (host). if c.configFile == "" && c.configFileFlagName != "" { if f := fs.Lookup(c.configFileFlagName); f != nil { c.configFile = f.Value.String() } } - if c.configFile != "" && c.configFileParser != nil { + if parseConfig := c.configFile != "" && c.configFileParser != nil; parseConfig { f, err := os.Open(c.configFile) - if err == nil { + switch { + case err == nil: defer f.Close() - err = c.configFileParser(f, func(name, value string) error { - if fs.Lookup(name) == nil { - if c.ignoreUndefined { - return nil - } - return fmt.Errorf("config file flag %q not defined in flag set", name) + if err := c.configFileParser(f, func(name, value string) error { + if provided[name] { + return nil } - if provided[name] { - return nil // commandline args take precedence + defined := fs.Lookup(name) != nil + switch { + case !defined && c.ignoreUndefined: + return nil + case !defined && !c.ignoreUndefined: + return fmt.Errorf("config file flag %q not defined in flag set", name) } if err := fs.Set(name, value); err != nil { - return fmt.Errorf("error setting flag %q from config file: %v", name, err) + return fmt.Errorf("error setting flag %q from config file: %w", name, err) } return nil - }) - if err != nil { + }); err != nil { return err } - } else if err != nil && !c.allowMissingConfigFile { + + case os.IsNotExist(err) && c.allowMissingConfigFile: + // no problem + + default: return err } } @@ -67,40 +74,44 @@ func Parse(fs *flag.FlagSet, args []string, options ...Option) error { provided[f.Name] = true }) - if c.envVarPrefix != "" || c.envVarNoPrefix { - var errs []string + // TEMPORARY third priority: environment variables (session). + if parseEnv := c.envVarPrefix != "" || c.envVarNoPrefix; parseEnv { + var visitErr error fs.VisitAll(func(f *flag.Flag) { + if visitErr != nil { + return + } + if provided[f.Name] { - return // commandline args and config file take precedence + return } var key string - { - key = strings.ToUpper(f.Name) - key = envVarReplacer.Replace(key) - if !c.envVarNoPrefix { - key = strings.ToUpper(c.envVarPrefix) + "_" + key - } + key = strings.ToUpper(f.Name) + key = envVarReplacer.Replace(key) + key = maybePrefix(key, c.envVarNoPrefix, c.envVarPrefix) + + value := os.Getenv(key) + if value == "" { + return } - if value := os.Getenv(key); value != "" { - var values []string - if c.envVarIgnoreCommas { - values = []string{value} - } else { - values = strings.Split(value, ",") - } - for _, v := range values { - if err := fs.Set(f.Name, v); err != nil { - errs = append(errs, fmt.Sprintf("error setting flag %q from env var %q: %v", f.Name, key, err)) - } + + for _, v := range maybeSplit(value, c.envVarSplit) { + if err := fs.Set(f.Name, v); err != nil { + visitErr = fmt.Errorf("error setting flag %q from env var %q: %w", f.Name, key, err) + return } } }) - if len(errs) > 0 { - return fmt.Errorf("error parsing env vars: %s", strings.Join(errs, "; ")) + if visitErr != nil { + return fmt.Errorf("error parsing env vars: %w", visitErr) } } + fs.Visit(func(f *flag.Flag) { + provided[f.Name] = true + }) + return nil } @@ -112,14 +123,14 @@ type Context struct { allowMissingConfigFile bool envVarPrefix string envVarNoPrefix bool - envVarIgnoreCommas bool + envVarSplit string ignoreUndefined bool } -// Option controls some aspect of parse behavior. +// Option controls some aspect of Parse behavior. type Option func(*Context) -// WithConfigFile tells parse to read the provided filename as a config file. +// WithConfigFile tells Parse to read the provided filename as a config file. // Requires WithConfigFileParser, and overrides WithConfigFileFlag. func WithConfigFile(filename string) Option { return func(c *Context) { @@ -127,62 +138,65 @@ func WithConfigFile(filename string) Option { } } -// WithConfigFileFlag tells parse to treat the flag with the given name as a -// config file. Requires WithConfigFileParser, and is overridden by WithConfigFile. +// WithConfigFileFlag tells Parse to treat the flag with the given name as a +// config file. Requires WithConfigFileParser, and is overridden by +// WithConfigFile. func WithConfigFileFlag(flagname string) Option { return func(c *Context) { c.configFileFlagName = flagname } } -// WithConfigFileParser tells parse how to interpret the config file provided via -// WithConfigFile or WithConfigFileFlag. +// WithConfigFileParser tells Parse how to interpret the config file provided +// via WithConfigFile or WithConfigFileFlag. func WithConfigFileParser(p ConfigFileParser) Option { return func(c *Context) { c.configFileParser = p } } -// WithAllowMissingConfigFile will permit parse to succeed, even if a provided -// config file doesn't exist. +// WithAllowMissingConfigFile tells Parse to permit the case where a config file +// is specified but doesn't exist. By default, missing config files result in an +// error. func WithAllowMissingConfigFile(allow bool) Option { return func(c *Context) { c.allowMissingConfigFile = allow } } -// WithEnvVarPrefix tells parse to look in the environment for variables with -// the given prefix. Flag names are converted to environment variables by -// capitalizing them, and replacing separator characters like periods or hyphens -// with underscores. +// WithEnvVarPrefix tells Parse to try to set flags from environment variables +// with the given prefix. Flag names are matched to environment variables with +// the given prefix, followed by an underscore, followed by the capitalized flag +// names, with separator characters like periods or hyphens replaced with +// underscores. By default, flags are not set from environment variables at all. func WithEnvVarPrefix(prefix string) Option { return func(c *Context) { c.envVarPrefix = prefix } } -// WithEnvVarNoPrefix tells parse to look in the environment for variables with -// no prefix. See WithEnvVarPrefix for an explanation of how flag names are -// converted to environment variables names. +// WithEnvVarNoPrefix tells Parse to try to set flags from environment variables +// without any specific prefix. Flag names are matched to environment variables +// by capitalizing the flag name, and replacing separator characters like +// periods or hyphens with underscores. By default, flags are not set from +// environment variables at all. func WithEnvVarNoPrefix() Option { return func(c *Context) { c.envVarNoPrefix = true } } -// WithEnvVarIgnoreCommas tells parse to ignore commas in environment variable -// values, treating the complete value as a single string passed to the -// associated flag. By default, if an environment variable's value contains -// commas, each comma-delimited token is treated as a separate instance of the -// associated flag. -func WithEnvVarIgnoreCommas(ignore bool) Option { +// WithEnvVarSplit tells Parse to split environment variables on the given +// delimiter, and to make a call to Set on the corresponding flag with each +// split token. +func WithEnvVarSplit(delimiter string) Option { return func(c *Context) { - c.envVarIgnoreCommas = ignore + c.envVarSplit = delimiter } } -// WithIgnoreUndefined tells parse to ignore undefined flags that it encounters, -// which would normally throw an error. +// WithIgnoreUndefined tells Parse to ignore undefined flags that it encounters. +// By default, undefined flags result in an error. func WithIgnoreUndefined(ignore bool) Option { return func(c *Context) { c.ignoreUndefined = ignore @@ -237,3 +251,17 @@ var envVarReplacer = strings.NewReplacer( ".", "_", "/", "_", ) + +func maybePrefix(key string, noPrefix bool, prefix string) string { + if noPrefix { + return key + } + return strings.ToUpper(prefix) + "_" + key +} + +func maybeSplit(value, split string) []string { + if split == "" { + return []string{value} + } + return strings.Split(value, split) +} diff --git a/parse_test.go b/parse_test.go index a3c00eb..f70cb01 100644 --- a/parse_test.go +++ b/parse_test.go @@ -38,6 +38,7 @@ func TestParseBasics(t *testing.T) { { name: "env only", env: map[string]string{"TEST_PARSE_S": "baz", "TEST_PARSE_F": "0.99", "TEST_PARSE_D": "100s"}, + opts: []ff.Option{ff.WithEnvVarPrefix("TEST_PARSE")}, want: fftest.Vars{S: "baz", F: 0.99, D: 100 * time.Second}, }, { @@ -50,12 +51,14 @@ func TestParseBasics(t *testing.T) { name: "env and args", env: map[string]string{"TEST_PARSE_S": "should be overridden", "TEST_PARSE_B": "true"}, args: []string{"-s", "explicit wins", "-i", "7"}, + opts: []ff.Option{ff.WithEnvVarPrefix("TEST_PARSE")}, want: fftest.Vars{S: "explicit wins", I: 7, B: true}, }, { name: "env and file", env: map[string]string{"TEST_PARSE_S": "should be overridden", "TEST_PARSE_B": "true"}, file: "testdata/3.conf", + opts: []ff.Option{ff.WithEnvVarPrefix("TEST_PARSE")}, want: fftest.Vars{S: "bar", I: 99, B: true, D: 34 * time.Second}, }, { @@ -63,6 +66,7 @@ func TestParseBasics(t *testing.T) { env: map[string]string{"TEST_PARSE_S": "from env", "TEST_PARSE_I": "300", "TEST_PARSE_F": "0.15", "TEST_PARSE_B": "true", "TEST_PARSE_D": "1h"}, file: "testdata/4.conf", args: []string{"-s", "from arg", "-i", "100"}, + opts: []ff.Option{ff.WithEnvVarPrefix("TEST_PARSE")}, want: fftest.Vars{S: "from arg", I: 100, F: 2.3, B: true, D: time.Minute}, }, { @@ -75,6 +79,7 @@ func TestParseBasics(t *testing.T) { env: map[string]string{"TEST_PARSE_S": "s.env", "TEST_PARSE_X": "x.env.1"}, file: "testdata/5.conf", args: []string{"-s", "s.arg.1", "-s", "s.arg.2", "-x", "x.arg.1", "-x", "x.arg.2"}, + opts: []ff.Option{ff.WithEnvVarPrefix("TEST_PARSE")}, want: fftest.Vars{S: "s.arg.2", X: []string{"x.arg.1", "x.arg.2"}}, // highest prio wins and no others are called }, { @@ -90,18 +95,25 @@ func TestParseBasics(t *testing.T) { { name: "default comma behavior", env: map[string]string{"TEST_PARSE_S": "one,two,three", "TEST_PARSE_X": "one,two,three"}, - want: fftest.Vars{S: "three", X: []string{"one", "two", "three"}}, + opts: []ff.Option{ff.WithEnvVarPrefix("TEST_PARSE")}, + want: fftest.Vars{S: "one,two,three", X: []string{"one,two,three"}}, }, { - name: "WithEnvVarIgnoreCommas", + name: "WithEnvVarSplit", env: map[string]string{"TEST_PARSE_S": "one,two,three", "TEST_PARSE_X": "one,two,three"}, - opts: []ff.Option{ff.WithEnvVarIgnoreCommas(true)}, - want: fftest.Vars{S: "one,two,three", X: []string{"one,two,three"}}, + opts: []ff.Option{ff.WithEnvVarPrefix("TEST_PARSE"), ff.WithEnvVarSplit(",")}, + want: fftest.Vars{S: "three", X: []string{"one", "two", "three"}}, + }, + { + name: "WithEnvVarNoPrefix", + env: map[string]string{"TEST_PARSE_S": "foo", "S": "bar"}, + opts: []ff.Option{ff.WithEnvVarNoPrefix()}, + want: fftest.Vars{S: "bar"}, }, { name: "WithIgnoreUndefined env", env: map[string]string{"TEST_PARSE_UNDEFINED": "one", "TEST_PARSE_S": "one"}, - opts: []ff.Option{ff.WithIgnoreUndefined(true)}, + opts: []ff.Option{ff.WithEnvVarPrefix("TEST_PARSE"), ff.WithIgnoreUndefined(true)}, want: fftest.Vars{S: "one"}, }, { @@ -117,8 +129,9 @@ func TestParseBasics(t *testing.T) { want: fftest.Vars{WantParseErrorString: "config file flag"}, }, { - name: "env var comma whitespace", + name: "env var split comma whitespace", env: map[string]string{"TEST_PARSE_S": "one, two, three ", "TEST_PARSE_X": "one, two, three "}, + opts: []ff.Option{ff.WithEnvVarPrefix("TEST_PARSE"), ff.WithEnvVarSplit(",")}, want: fftest.Vars{S: " three ", X: []string{"one", " two", " three "}}, }, } { @@ -132,7 +145,6 @@ func TestParseBasics(t *testing.T) { defer os.Setenv(k, os.Getenv(k)) os.Setenv(k, v) } - testcase.opts = append(testcase.opts, ff.WithEnvVarPrefix("TEST_PARSE")) } fs, vars := fftest.Pair() From 1a5e5099434edbe354e045313e2c6529af5dc593 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Mon, 9 Mar 2020 21:37:07 +0100 Subject: [PATCH 4/6] Priority: env file args -> file env args --- parse.go | 78 ++++++++++++++++++++++++------------------------- parse_test.go | 16 +++++----- testdata/3.conf | 2 +- 3 files changed, 48 insertions(+), 48 deletions(-) diff --git a/parse.go b/parse.go index 6473776..0e9173e 100644 --- a/parse.go +++ b/parse.go @@ -28,7 +28,45 @@ func Parse(fs *flag.FlagSet, args []string, options ...Option) error { provided[f.Name] = true }) - // TEMPORARY second priority: config file (host). + // Second priority: environment variables (session). + if parseEnv := c.envVarPrefix != "" || c.envVarNoPrefix; parseEnv { + var visitErr error + fs.VisitAll(func(f *flag.Flag) { + if visitErr != nil { + return + } + + if provided[f.Name] { + return + } + + var key string + key = strings.ToUpper(f.Name) + key = envVarReplacer.Replace(key) + key = maybePrefix(key, c.envVarNoPrefix, c.envVarPrefix) + + value := os.Getenv(key) + if value == "" { + return + } + + for _, v := range maybeSplit(value, c.envVarSplit) { + if err := fs.Set(f.Name, v); err != nil { + visitErr = fmt.Errorf("error setting flag %q from env var %q: %w", f.Name, key, err) + return + } + } + }) + if visitErr != nil { + return fmt.Errorf("error parsing env vars: %w", visitErr) + } + } + + fs.Visit(func(f *flag.Flag) { + provided[f.Name] = true + }) + + // Third priority: config file (host). if c.configFile == "" && c.configFileFlagName != "" { if f := fs.Lookup(c.configFileFlagName); f != nil { c.configFile = f.Value.String() @@ -74,44 +112,6 @@ func Parse(fs *flag.FlagSet, args []string, options ...Option) error { provided[f.Name] = true }) - // TEMPORARY third priority: environment variables (session). - if parseEnv := c.envVarPrefix != "" || c.envVarNoPrefix; parseEnv { - var visitErr error - fs.VisitAll(func(f *flag.Flag) { - if visitErr != nil { - return - } - - if provided[f.Name] { - return - } - - var key string - key = strings.ToUpper(f.Name) - key = envVarReplacer.Replace(key) - key = maybePrefix(key, c.envVarNoPrefix, c.envVarPrefix) - - value := os.Getenv(key) - if value == "" { - return - } - - for _, v := range maybeSplit(value, c.envVarSplit) { - if err := fs.Set(f.Name, v); err != nil { - visitErr = fmt.Errorf("error setting flag %q from env var %q: %w", f.Name, key, err) - return - } - } - }) - if visitErr != nil { - return fmt.Errorf("error parsing env vars: %w", visitErr) - } - } - - fs.Visit(func(f *flag.Flag) { - provided[f.Name] = true - }) - return nil } diff --git a/parse_test.go b/parse_test.go index f70cb01..3001f16 100644 --- a/parse_test.go +++ b/parse_test.go @@ -42,32 +42,32 @@ func TestParseBasics(t *testing.T) { want: fftest.Vars{S: "baz", F: 0.99, D: 100 * time.Second}, }, { - name: "file and args", + name: "file args", file: "testdata/2.conf", args: []string{"-s", "foo", "-i", "1234"}, want: fftest.Vars{S: "foo", I: 1234, D: 3 * time.Second}, }, { - name: "env and args", + name: "env args", env: map[string]string{"TEST_PARSE_S": "should be overridden", "TEST_PARSE_B": "true"}, args: []string{"-s", "explicit wins", "-i", "7"}, opts: []ff.Option{ff.WithEnvVarPrefix("TEST_PARSE")}, want: fftest.Vars{S: "explicit wins", I: 7, B: true}, }, { - name: "env and file", - env: map[string]string{"TEST_PARSE_S": "should be overridden", "TEST_PARSE_B": "true"}, + name: "file env", + env: map[string]string{"TEST_PARSE_S": "env takes priority", "TEST_PARSE_B": "true"}, file: "testdata/3.conf", opts: []ff.Option{ff.WithEnvVarPrefix("TEST_PARSE")}, - want: fftest.Vars{S: "bar", I: 99, B: true, D: 34 * time.Second}, + want: fftest.Vars{S: "env takes priority", I: 99, B: true, D: 34 * time.Second}, }, { - name: "env file args", - env: map[string]string{"TEST_PARSE_S": "from env", "TEST_PARSE_I": "300", "TEST_PARSE_F": "0.15", "TEST_PARSE_B": "true", "TEST_PARSE_D": "1h"}, + name: "file env args", file: "testdata/4.conf", + env: map[string]string{"TEST_PARSE_S": "from env", "TEST_PARSE_I": "300", "TEST_PARSE_F": "0.15", "TEST_PARSE_B": "true"}, args: []string{"-s", "from arg", "-i", "100"}, opts: []ff.Option{ff.WithEnvVarPrefix("TEST_PARSE")}, - want: fftest.Vars{S: "from arg", I: 100, F: 2.3, B: true, D: time.Minute}, + want: fftest.Vars{S: "from arg", I: 100, F: 0.15, B: true, D: time.Minute}, }, { name: "repeated args", diff --git a/testdata/3.conf b/testdata/3.conf index 9571775..0366bdd 100644 --- a/testdata/3.conf +++ b/testdata/3.conf @@ -1,4 +1,4 @@ s bar i 99 d 34s -# comment line \ No newline at end of file +# comment line From a1f2a82fb177ee86ffb927eebb510cbdbf8c9d89 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Mon, 9 Mar 2020 21:37:46 +0100 Subject: [PATCH 5/6] go mod tidy --- go.sum | 5 ----- 1 file changed, 5 deletions(-) diff --git a/go.sum b/go.sum index 1adb6a1..97c2cc2 100644 --- a/go.sum +++ b/go.sum @@ -2,13 +2,8 @@ github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/mitchellh/go-wordwrap v1.0.0/go.mod h1:ZXFpozHsX6DPmq2I0TCekCxypsnAUbP2oI0UX1GXzOo= github.com/pelletier/go-toml v1.6.0 h1:aetoXYr0Tv7xRU/V4B4IZJ2QcbtMUFoNb3ORp7TzIK4= github.com/pelletier/go-toml v1.6.0/go.mod h1:5N711Q9dKgbdkxHL+MEfF31hpT7l0S0s/t2kKREewys= -github.com/peterbourgon/ff/v2 v1.7.0 h1:hknvTgsh90jNBIjPq7xeq32Y9AmSbpXvjrFW4sJwW+A= -github.com/peterbourgon/ff/v2 v1.7.0/go.mod h1:/KKxnU5cBj4w21jEMj4Rway/kslRP6XAOHh7CH8AyAM= -golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc= -golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I= From 0ae86028f6e7436186ea2a3e13a282d9df1d5c4e Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Mon, 9 Mar 2020 21:48:39 +0100 Subject: [PATCH 6/6] Update READMEs with v3 --- README.md | 41 ++++++++++++++++++++--------------------- ffcli/README.md | 6 +++--- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index b31877c..9db2099 100644 --- a/README.md +++ b/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() { diff --git a/ffcli/README.md b/ffcli/README.md index 2809fd0..aee199e 100644 --- a/ffcli/README.md +++ b/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() {