From 3fc3cd64854e64d2b772631a938083e41269c859 Mon Sep 17 00:00:00 2001 From: Yasuhiro KANDA Date: Fri, 22 Jul 2016 22:45:05 +0900 Subject: [PATCH 01/11] Add TOML config file loader --- altsrc/toml_command_test.go | 310 ++++++++++++++++++++++++++++++++++++ altsrc/toml_file_loader.go | 113 +++++++++++++ 2 files changed, 423 insertions(+) create mode 100644 altsrc/toml_command_test.go create mode 100644 altsrc/toml_file_loader.go diff --git a/altsrc/toml_command_test.go b/altsrc/toml_command_test.go new file mode 100644 index 0000000..c887ab2 --- /dev/null +++ b/altsrc/toml_command_test.go @@ -0,0 +1,310 @@ +// Disabling building of tom support in cases where golang is 1.0 or 1.1 +// as the encoding library is not implemented or supported. + +// +build go1.2 + +package altsrc + +import ( + "flag" + "io/ioutil" + "os" + "testing" + + "github.com/urfave/cli" +) + +func TestCommandTomFileTest(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte("test = 15"), 0666) + defer os.Remove("current.toml") + test := []string{"test-cmd", "--load", "current.toml"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("test") + expect(t, val, 15) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "test"}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + err := command.Run(c) + + expect(t, err, nil) +} + +func TestCommandTomlFileTestGlobalEnvVarWins(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte("test = 15"), 0666) + defer os.Remove("current.toml") + + os.Setenv("THE_TEST", "10") + defer os.Setenv("THE_TEST", "") + test := []string{"test-cmd", "--load", "current.toml"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("test") + expect(t, val, 10) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "test", EnvVar: "THE_TEST"}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + + err := command.Run(c) + + expect(t, err, nil) +} + +func TestCommandTomlFileTestGlobalEnvVarWinsNested(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte("[top]\ntest = 15"), 0666) + defer os.Remove("current.toml") + + os.Setenv("THE_TEST", "10") + defer os.Setenv("THE_TEST", "") + test := []string{"test-cmd", "--load", "current.toml"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("top.test") + expect(t, val, 10) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "top.test", EnvVar: "THE_TEST"}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + + err := command.Run(c) + + expect(t, err, nil) +} + +func TestCommandTomlFileTestSpecifiedFlagWins(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte("test = 15"), 0666) + defer os.Remove("current.toml") + + test := []string{"test-cmd", "--load", "current.toml", "--test", "7"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("test") + expect(t, val, 7) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "test"}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + + err := command.Run(c) + + expect(t, err, nil) +} + +func TestCommandTomlFileTestSpecifiedFlagWinsNested(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte(`[top] + test = 15`), 0666) + defer os.Remove("current.toml") + + test := []string{"test-cmd", "--load", "current.toml", "--top.test", "7"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("top.test") + expect(t, val, 7) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "top.test"}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + + err := command.Run(c) + + expect(t, err, nil) +} + +func TestCommandTomlFileTestDefaultValueFileWins(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte("test = 15"), 0666) + defer os.Remove("current.toml") + + test := []string{"test-cmd", "--load", "current.toml"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("test") + expect(t, val, 15) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "test", Value: 7}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + + err := command.Run(c) + + expect(t, err, nil) +} + +func TestCommandTomlFileTestDefaultValueFileWinsNested(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte("[top]\ntest = 15"), 0666) + defer os.Remove("current.toml") + + test := []string{"test-cmd", "--load", "current.toml"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("top.test") + expect(t, val, 15) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "top.test", Value: 7}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + + err := command.Run(c) + + expect(t, err, nil) +} + +func TestCommandTomlFileFlagHasDefaultGlobalEnvTomlSetGlobalEnvWins(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte("test = 15"), 0666) + defer os.Remove("current.toml") + + os.Setenv("THE_TEST", "11") + defer os.Setenv("THE_TEST", "") + + test := []string{"test-cmd", "--load", "current.toml"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("test") + expect(t, val, 11) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "test", Value: 7, EnvVar: "THE_TEST"}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + err := command.Run(c) + + expect(t, err, nil) +} + +func TestCommandTomlFileFlagHasDefaultGlobalEnvTomlSetGlobalEnvWinsNested(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte("[top]\ntest = 15"), 0666) + defer os.Remove("current.toml") + + os.Setenv("THE_TEST", "11") + defer os.Setenv("THE_TEST", "") + + test := []string{"test-cmd", "--load", "current.toml"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("top.test") + expect(t, val, 11) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "top.test", Value: 7, EnvVar: "THE_TEST"}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + err := command.Run(c) + + expect(t, err, nil) +} diff --git a/altsrc/toml_file_loader.go b/altsrc/toml_file_loader.go new file mode 100644 index 0000000..64250a3 --- /dev/null +++ b/altsrc/toml_file_loader.go @@ -0,0 +1,113 @@ +// Disabling building of toml support in cases where golang is 1.0 or 1.1 +// as the encoding library is not implemented or supported. + +// +build go1.2 + +package altsrc + +import ( + "fmt" + "reflect" + + "github.com/BurntSushi/toml" + "github.com/urfave/cli" +) + +type TomlMap struct { + Map map[interface{}]interface{} +} + +func unmarshalMap(i interface{}) (ret map[interface{}]interface{}, err error) { + ret = make(map[interface{}]interface{}) + m := i.(map[string]interface{}) + for key, val := range m { + v := reflect.ValueOf(val) + switch v.Kind() { + case reflect.Bool: + ret[key] = val.(bool) + case reflect.String: + ret[key] = val.(string) + case reflect.Int: + ret[key] = int(val.(int)) + case reflect.Int8: + ret[key] = int(val.(int8)) + case reflect.Int16: + ret[key] = int(val.(int16)) + case reflect.Int32: + ret[key] = int(val.(int32)) + case reflect.Int64: + ret[key] = int(val.(int64)) + case reflect.Uint: + ret[key] = int(val.(uint)) + case reflect.Uint8: + ret[key] = int(val.(uint8)) + case reflect.Uint16: + ret[key] = int(val.(uint16)) + case reflect.Uint32: + ret[key] = int(val.(uint32)) + case reflect.Uint64: + ret[key] = int(val.(uint64)) + case reflect.Float32: + ret[key] = float64(val.(float32)) + case reflect.Float64: + ret[key] = float64(val.(float64)) + case reflect.Map: + if tmp, err := unmarshalMap(val); err == nil { + ret[key] = tmp + } else { + return nil, err + } + case reflect.Array: + fallthrough // [todo] - Support array type + default: + return nil, fmt.Errorf("Unsupported: type = %#v", v.Kind()) + } + } + return ret, nil +} + +func (self *TomlMap) UnmarshalTOML(i interface{}) error { + if tmp, err := unmarshalMap(i); err == nil { + self.Map = tmp + } else { + return err + } + return nil +} + +type tomlSourceContext struct { + FilePath string +} + +// NewTomlSourceFromFile creates a new TOML InputSourceContext from a filepath. +func NewTomlSourceFromFile(file string) (InputSourceContext, error) { + tsc := &tomlSourceContext{FilePath: file} + var results TomlMap = TomlMap{} + if err := readCommandToml(tsc.FilePath, &results); err != nil { + return nil, fmt.Errorf("Unable to load TOML file '%s': inner error: \n'%v'", tsc.FilePath, err.Error()) + } + return &MapInputSource{valueMap: results.Map}, nil +} + +// NewTomlSourceFromFlagFunc creates a new TOML InputSourceContext from a provided flag name and source context. +func NewTomlSourceFromFlagFunc(flagFileName string) func(context *cli.Context) (InputSourceContext, error) { + return func(context *cli.Context) (InputSourceContext, error) { + filePath := context.String(flagFileName) + return NewTomlSourceFromFile(filePath) + } +} + +func readCommandToml(filePath string, container interface{}) (err error) { + b, err := loadDataFrom(filePath) + if err != nil { + return err + } + + err = toml.Unmarshal(b, container) + if err != nil { + return err + } + + err = nil + return +} From 71a99921b4ed75328f4f3eb887d4718161ad3017 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Sat, 23 Jul 2016 17:52:19 -0400 Subject: [PATCH 02/11] Bump tested go versions and (maybe?) take advantage of support for N.x version syntax --- .travis.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index 47f25dc..93d3a2e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,18 +7,18 @@ cache: - node_modules go: -- 1.2.2 -- 1.3.3 -- 1.4 -- 1.5.4 -- 1.6.2 +- 1.2.x +- 1.3.x +- 1.4.x +- 1.5.x +- 1.6.x - master matrix: allow_failures: - go: master include: - - go: 1.6.2 + - go: 1.6.x os: osx before_script: From 76fb6d2ab73b2919dd9714784b44389508da96f0 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Sat, 23 Jul 2016 18:16:45 -0400 Subject: [PATCH 03/11] Pin testing on go 1.4.2 since 1.4.3 is lacking `vet` --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 93d3a2e..d1d820d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,7 +9,7 @@ cache: go: - 1.2.x - 1.3.x -- 1.4.x +- 1.4.2 - 1.5.x - 1.6.x - master @@ -23,7 +23,7 @@ matrix: before_script: - go get github.com/urfave/gfmrun/... -- go get golang.org/x/tools/cmd/goimports || true +- go get golang.org/x/tools/... || true - if [ ! -f node_modules/.bin/markdown-toc ] ; then npm install markdown-toc ; fi From dd253d122c836210efb731095e85876228f41708 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Sun, 24 Jul 2016 11:57:51 -0400 Subject: [PATCH 04/11] Write err to stderr, exit 1 if err != "" Closes #475 --- app_test.go | 13 ++++++++++++ errors.go | 6 ++++++ errors_test.go | 54 ++++++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/app_test.go b/app_test.go index b0b02e6..23c8aa6 100644 --- a/app_test.go +++ b/app_test.go @@ -13,6 +13,19 @@ import ( "testing" ) +var ( + lastExitCode = 0 + fakeOsExiter = func(rc int) { + lastExitCode = rc + } + fakeErrWriter = &bytes.Buffer{} +) + +func init() { + OsExiter = fakeOsExiter + ErrWriter = fakeErrWriter +} + type opCounts struct { Total, BashComplete, OnUsageError, Before, CommandNotFound, Action, After, SubCommand int } diff --git a/errors.go b/errors.go index ea551be..15ac790 100644 --- a/errors.go +++ b/errors.go @@ -88,5 +88,11 @@ func HandleExitCoder(err error) { for _, merr := range multiErr.Errors { HandleExitCoder(merr) } + return + } + + if err.Error() != "" { + fmt.Fprintln(ErrWriter, err) + OsExiter(1) } } diff --git a/errors_test.go b/errors_test.go index 8f5f284..e357dc4 100644 --- a/errors_test.go +++ b/errors_test.go @@ -1,8 +1,8 @@ package cli import ( + "bytes" "errors" - "os" "testing" ) @@ -15,7 +15,7 @@ func TestHandleExitCoder_nil(t *testing.T) { called = true } - defer func() { OsExiter = os.Exit }() + defer func() { OsExiter = fakeOsExiter }() HandleExitCoder(nil) @@ -32,7 +32,7 @@ func TestHandleExitCoder_ExitCoder(t *testing.T) { called = true } - defer func() { OsExiter = os.Exit }() + defer func() { OsExiter = fakeOsExiter }() HandleExitCoder(NewExitError("galactic perimeter breach", 9)) @@ -49,7 +49,7 @@ func TestHandleExitCoder_MultiErrorWithExitCoder(t *testing.T) { called = true } - defer func() { OsExiter = os.Exit }() + defer func() { OsExiter = fakeOsExiter }() exitErr := NewExitError("galactic perimeter breach", 9) err := NewMultiError(errors.New("wowsa"), errors.New("egad"), exitErr) @@ -58,3 +58,49 @@ func TestHandleExitCoder_MultiErrorWithExitCoder(t *testing.T) { expect(t, exitCode, 9) expect(t, called, true) } + +func TestHandleExitCoder_ErrorWithMessage(t *testing.T) { + exitCode := 0 + called := false + + OsExiter = func(rc int) { + exitCode = rc + called = true + } + ErrWriter = &bytes.Buffer{} + + defer func() { + OsExiter = fakeOsExiter + ErrWriter = fakeErrWriter + }() + + err := errors.New("gourd havens") + HandleExitCoder(err) + + expect(t, exitCode, 1) + expect(t, called, true) + expect(t, ErrWriter.(*bytes.Buffer).String(), "gourd havens\n") +} + +func TestHandleExitCoder_ErrorWithoutMessage(t *testing.T) { + exitCode := 0 + called := false + + OsExiter = func(rc int) { + exitCode = rc + called = true + } + ErrWriter = &bytes.Buffer{} + + defer func() { + OsExiter = fakeOsExiter + ErrWriter = fakeErrWriter + }() + + err := errors.New("") + HandleExitCoder(err) + + expect(t, exitCode, 0) + expect(t, called, false) + expect(t, ErrWriter.(*bytes.Buffer).String(), "") +} From e9688813e4ffad908f2e8a3218d852dc293d8529 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Sun, 24 Jul 2016 12:09:05 -0400 Subject: [PATCH 05/11] Refine error handling behavior so that exit 1 happens as long as error is non-nil --- CHANGELOG.md | 1 + errors.go | 2 +- errors_test.go | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4976635..26dd564 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ## [Unreleased] ### Added - Flag type code generation via `go generate` +- Write to stderr and exit 1 if action returns non-nil error ### Changed - Raise minimum tested/supported Go version to 1.2+ diff --git a/errors.go b/errors.go index 15ac790..c7d8c2f 100644 --- a/errors.go +++ b/errors.go @@ -93,6 +93,6 @@ func HandleExitCoder(err error) { if err.Error() != "" { fmt.Fprintln(ErrWriter, err) - OsExiter(1) } + OsExiter(1) } diff --git a/errors_test.go b/errors_test.go index e357dc4..04df031 100644 --- a/errors_test.go +++ b/errors_test.go @@ -100,7 +100,7 @@ func TestHandleExitCoder_ErrorWithoutMessage(t *testing.T) { err := errors.New("") HandleExitCoder(err) - expect(t, exitCode, 0) - expect(t, called, false) + expect(t, exitCode, 1) + expect(t, called, true) expect(t, ErrWriter.(*bytes.Buffer).String(), "") } From c59ec842c11a6ceb3dd44aa4e6def4320a9b79c2 Mon Sep 17 00:00:00 2001 From: Yi EungJun Date: Mon, 25 Jul 2016 20:09:41 +0900 Subject: [PATCH 06/11] README: Remove unnecessary 'v' from version numbers 'partay version v19.99.0' sounds 'partay version version 19.99.0' because 'v' stands for 'version'. --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 0ffa92f..615e95d 100644 --- a/README.md +++ b/README.md @@ -953,7 +953,7 @@ setting `cli.VersionFlag`, e.g.: ``` go package main @@ -972,7 +972,7 @@ func main() { app := cli.NewApp() app.Name = "partay" - app.Version = "v19.99.0" + app.Version = "19.99.0" app.Run(os.Args) } ``` @@ -981,7 +981,7 @@ Alternatively, the version printer at `cli.VersionPrinter` may be overridden, e. ``` go package main @@ -1004,7 +1004,7 @@ func main() { app := cli.NewApp() app.Name = "partay" - app.Version = "v19.99.0" + app.Version = "19.99.0" app.Run(os.Args) } ``` @@ -1083,7 +1083,7 @@ func (g *genericType) String() string { func main() { app := cli.NewApp() app.Name = "kənˈtrīv" - app.Version = "v19.99.0" + app.Version = "19.99.0" app.Compiled = time.Now() app.Authors = []cli.Author{ cli.Author{ From 812de9e2507c3ec9ce2750b67e5efe8cc643c0f4 Mon Sep 17 00:00:00 2001 From: kandayasu Date: Tue, 26 Jul 2016 01:37:03 +0900 Subject: [PATCH 07/11] type "TomlMap" to private (rename TomlMap -> tomlMap) --- altsrc/toml_file_loader.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/altsrc/toml_file_loader.go b/altsrc/toml_file_loader.go index 64250a3..bc2c11d 100644 --- a/altsrc/toml_file_loader.go +++ b/altsrc/toml_file_loader.go @@ -13,7 +13,7 @@ import ( "github.com/urfave/cli" ) -type TomlMap struct { +type tomlMap struct { Map map[interface{}]interface{} } @@ -66,7 +66,7 @@ func unmarshalMap(i interface{}) (ret map[interface{}]interface{}, err error) { return ret, nil } -func (self *TomlMap) UnmarshalTOML(i interface{}) error { +func (self *tomlMap) UnmarshalTOML(i interface{}) error { if tmp, err := unmarshalMap(i); err == nil { self.Map = tmp } else { @@ -82,7 +82,7 @@ type tomlSourceContext struct { // NewTomlSourceFromFile creates a new TOML InputSourceContext from a filepath. func NewTomlSourceFromFile(file string) (InputSourceContext, error) { tsc := &tomlSourceContext{FilePath: file} - var results TomlMap = TomlMap{} + var results tomlMap = tomlMap{} if err := readCommandToml(tsc.FilePath, &results); err != nil { return nil, fmt.Errorf("Unable to load TOML file '%s': inner error: \n'%v'", tsc.FilePath, err.Error()) } From 36b7d89bedc1626a65c03df4f9fad9d4a96a4354 Mon Sep 17 00:00:00 2001 From: kandayasu Date: Tue, 26 Jul 2016 01:37:18 +0900 Subject: [PATCH 08/11] Fix typo --- altsrc/toml_command_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/altsrc/toml_command_test.go b/altsrc/toml_command_test.go index c887ab2..1d91d56 100644 --- a/altsrc/toml_command_test.go +++ b/altsrc/toml_command_test.go @@ -1,4 +1,4 @@ -// Disabling building of tom support in cases where golang is 1.0 or 1.1 +// Disabling building of toml support in cases where golang is 1.0 or 1.1 // as the encoding library is not implemented or supported. // +build go1.2 From b616f6088660d2eaa33739718f0583f8d467a178 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 31 Jul 2016 12:11:52 -0700 Subject: [PATCH 09/11] Note TOML support in README and CHANGELOG --- CHANGELOG.md | 1 + README.md | 16 ++++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26dd564..8b0d0ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ ### Added - Flag type code generation via `go generate` - Write to stderr and exit 1 if action returns non-nil error +- Added support for TOML to the `altsrc` loader ### Changed - Raise minimum tested/supported Go version to 1.2+ diff --git a/README.md b/README.md index 615e95d..31b25b8 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ applications in an expressive way. + [Placeholder Values](#placeholder-values) + [Alternate Names](#alternate-names) + [Values from the Environment](#values-from-the-environment) - + [Values from alternate input sources (YAML and others)](#values-from-alternate-input-sources-yaml-and-others) + + [Values from alternate input sources (YAML, TOML, and others)](#values-from-alternate-input-sources-yaml-toml-and-others) * [Subcommands](#subcommands) * [Subcommands categories](#subcommands-categories) * [Exit code](#exit-code) @@ -513,10 +513,14 @@ func main() { } ``` -#### Values from alternate input sources (YAML and others) +#### Values from alternate input sources (YAML, TOML, and others) There is a separate package altsrc that adds support for getting flag values -from other input sources like YAML. +from other file input sources. + +Currently supported input source formats: +* YAML +* TOML In order to get values for a flag from an alternate input source the following code would be added to wrap an existing cli.Flag like below: @@ -538,9 +542,9 @@ the yaml input source for any flags that are defined on that command. As a note the "load" flag used would also have to be defined on the command flags in order for this code snipped to work. -Currently only YAML files are supported but developers can add support for other -input sources by implementing the altsrc.InputSourceContext for their given -sources. +Currently only the aboved specified formats are supported but developers can +add support for other input sources by implementing the +altsrc.InputSourceContext for their given sources. Here is a more complete sample of a command using YAML support: From 6c1f51aa95e7966c49a5211ffabe337bf4d2f3fb Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 31 Jul 2016 14:14:46 -0700 Subject: [PATCH 10/11] Fix context.(Global)IsSet to respect environment variables This appeared to be the least messy approach to hack in support for IsSet also checking environment variables to see if a particular cli.Flag was set without making backwards incompatible changes to the interface. I intend to fix this more properly in v2, probably by adding another method to the cli.Flag interface to push the responsibility down as it occurred to me that it was really the `Flag`s themselves that offer support for configuration via the environment as opposed to the `context` or other supporting structures. This opens the door for the anything implementing the `Flag` interface to have additional sources of input while still supporting `context.IsSet`. --- context.go | 79 ++++++++++++++++++++++++++++++++++++++----------- context_test.go | 60 +++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 18 deletions(-) diff --git a/context.go b/context.go index 14ad3f7..5ea4f8f 100644 --- a/context.go +++ b/context.go @@ -3,6 +3,8 @@ package cli import ( "errors" "flag" + "os" + "reflect" "strings" ) @@ -11,12 +13,11 @@ import ( // can be used to retrieve context-specific Args and // parsed command-line options. type Context struct { - App *App - Command Command - flagSet *flag.FlagSet - setFlags map[string]bool - globalSetFlags map[string]bool - parentContext *Context + App *App + Command Command + flagSet *flag.FlagSet + setFlags map[string]bool + parentContext *Context } // NewContext creates a new context. For use in when invoking an App or Command action. @@ -43,28 +44,70 @@ func (c *Context) GlobalSet(name, value string) error { func (c *Context) IsSet(name string) bool { if c.setFlags == nil { c.setFlags = make(map[string]bool) + c.flagSet.Visit(func(f *flag.Flag) { c.setFlags[f.Name] = true }) + + c.flagSet.VisitAll(func(f *flag.Flag) { + if _, ok := c.setFlags[f.Name]; ok { + return + } + c.setFlags[f.Name] = false + }) + + // XXX hack to support IsSet for flags with EnvVar + // + // There isn't an easy way to do this with the current implementation since + // whether a flag was set via an environment variable is very difficult to + // determine here. Instead, we intend to introduce a backwards incompatible + // change in version 2 to add `IsSet` to the Flag interface to push the + // responsibility closer to where the information required to determine + // whether a flag is set by non-standard means such as environment + // variables is avaliable. + // + // See https://github.com/urfave/cli/issues/294 for additional discussion + flags := c.Command.Flags + if c.Command.Name == "" { // cannot == Command{} since it contains slice types + if c.App != nil { + flags = c.App.Flags + } + } + for _, f := range flags { + eachName(f.GetName(), func(name string) { + if isSet, ok := c.setFlags[name]; isSet || !ok { + return + } + + envVars := reflect.ValueOf(f).FieldByName("EnvVar").String() + + eachName(envVars, func(envVar string) { + envVar = strings.TrimSpace(envVar) + if envVal := os.Getenv(envVar); envVal != "" { + c.setFlags[name] = true + return + } + }) + }) + } } - return c.setFlags[name] == true + + return c.setFlags[name] } // GlobalIsSet determines if the global flag was actually set func (c *Context) GlobalIsSet(name string) bool { - if c.globalSetFlags == nil { - c.globalSetFlags = make(map[string]bool) - ctx := c - if ctx.parentContext != nil { - ctx = ctx.parentContext - } - for ; ctx != nil && c.globalSetFlags[name] == false; ctx = ctx.parentContext { - ctx.flagSet.Visit(func(f *flag.Flag) { - c.globalSetFlags[f.Name] = true - }) + ctx := c + if ctx.parentContext != nil { + ctx = ctx.parentContext + } + + for ; ctx != nil; ctx = ctx.parentContext { + if ctx.IsSet(name) { + return true } } - return c.globalSetFlags[name] + return false } // FlagNames returns a slice of flag names used in this context. diff --git a/context_test.go b/context_test.go index 5c68fdd..0cf84d1 100644 --- a/context_test.go +++ b/context_test.go @@ -2,6 +2,7 @@ package cli import ( "flag" + "os" "testing" "time" ) @@ -180,6 +181,33 @@ func TestContext_IsSet(t *testing.T) { expect(t, c.IsSet("myflagGlobal"), false) } +// XXX Corresponds to hack in context.IsSet for flags with EnvVar field +// Should be moved to `flag_test` in v2 +func TestContext_IsSet_fromEnv(t *testing.T) { + var timeoutIsSet, tIsSet, noEnvVarIsSet, nIsSet bool + + os.Clearenv() + os.Setenv("APP_TIMEOUT_SECONDS", "15.5") + a := App{ + Flags: []Flag{ + Float64Flag{Name: "timeout, t", EnvVar: "APP_TIMEOUT_SECONDS"}, + Float64Flag{Name: "no-env-var, n"}, + }, + Action: func(ctx *Context) error { + timeoutIsSet = ctx.IsSet("timeout") + tIsSet = ctx.IsSet("t") + noEnvVarIsSet = ctx.IsSet("no-env-var") + nIsSet = ctx.IsSet("n") + return nil + }, + } + a.Run([]string{"run"}) + expect(t, timeoutIsSet, true) + expect(t, tIsSet, true) + expect(t, noEnvVarIsSet, false) + expect(t, nIsSet, false) +} + func TestContext_GlobalIsSet(t *testing.T) { set := flag.NewFlagSet("test", 0) set.Bool("myflag", false, "doc") @@ -199,6 +227,38 @@ func TestContext_GlobalIsSet(t *testing.T) { expect(t, c.GlobalIsSet("bogusGlobal"), false) } +// XXX Corresponds to hack in context.IsSet for flags with EnvVar field +// Should be moved to `flag_test` in v2 +func TestContext_GlobalIsSet_fromEnv(t *testing.T) { + var timeoutIsSet, tIsSet, noEnvVarIsSet, nIsSet bool + + os.Clearenv() + os.Setenv("APP_TIMEOUT_SECONDS", "15.5") + a := App{ + Flags: []Flag{ + Float64Flag{Name: "timeout, t", EnvVar: "APP_TIMEOUT_SECONDS"}, + Float64Flag{Name: "no-env-var, n"}, + }, + Commands: []Command{ + { + Name: "hello", + Action: func(ctx *Context) error { + timeoutIsSet = ctx.GlobalIsSet("timeout") + tIsSet = ctx.GlobalIsSet("t") + noEnvVarIsSet = ctx.GlobalIsSet("no-env-var") + nIsSet = ctx.GlobalIsSet("n") + return nil + }, + }, + }, + } + a.Run([]string{"run", "hello"}) + expect(t, timeoutIsSet, true) + expect(t, tIsSet, true) + expect(t, noEnvVarIsSet, false) + expect(t, nIsSet, false) +} + func TestContext_NumFlags(t *testing.T) { set := flag.NewFlagSet("test", 0) set.Bool("myflag", false, "doc") From 168c95418e66e019fe17b8f4f5c45aa62ff80e23 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 31 Jul 2016 20:09:57 -0700 Subject: [PATCH 11/11] Ensure that EnvVar struct field exists before interrogating it Otherwise you end up with `` which, in practice, would probably work, but this is cleaner. --- context.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/context.go b/context.go index 5ea4f8f..15570c5 100644 --- a/context.go +++ b/context.go @@ -79,9 +79,12 @@ func (c *Context) IsSet(name string) bool { return } - envVars := reflect.ValueOf(f).FieldByName("EnvVar").String() + envVarValue := reflect.ValueOf(f).FieldByName("EnvVar") + if !envVarValue.IsValid() { + return + } - eachName(envVars, func(envVar string) { + eachName(envVarValue.String(), func(envVar string) { envVar = strings.TrimSpace(envVar) if envVal := os.Getenv(envVar); envVal != "" { c.setFlags[name] = true