Merge pull request #414 from codegangsta/remove-boolt

Replace BoolTFlag type with BoolFlag.Value

CHANGELOG.md

@@ -7,6 +7,7 @@
 - `NewStringSlice` and `NewIntSlice` for creating their related types
 - `Context.Lineage` to get all contexts from current up to global
 - `Context.LocalFlagNames` to get the flag names from *only* the current context
+- `BoolFlag.Value` to handle both default-false and default-true
 
 ### Changed
 - `Context.FlagNames` now returns all flags in the context lineage
@@ -28,9 +29,7 @@
 - All `Context.Global*` methods, as the non-global versions now traverse up
   the context lineage automatically.
 - `Context.Parent` method, as this is now available via `Context.Lineage`
-
-### Fixed
-- `Context.BoolT` now returns `true` when not found
+- `BoolTFlag` and related code, as this is now available via `BoolFlag.Value`
 
 ## [Unreleased] - (1.x series)
 ### Added

README.md

@@ -472,8 +472,9 @@ import (
 func main() {
   app := cli.NewApp()
   app.Flags = []cli.Flag{
-    cli.BoolTFlag{
+    cli.BoolFlag{
       Name:  "ginger-crouton",
+      Value: true,
       Usage: "is it in the soup?",
     },
   }

altsrc/flag.go

@@ -220,44 +220,6 @@ func (f *BoolFlag) Apply(set *flag.FlagSet) {
 	f.BoolFlag.Apply(set)
 }
 
-// BoolTFlag is the flag type that wraps cli.BoolTFlag to allow
-// for other values to be specified
-type BoolTFlag struct {
-	cli.BoolTFlag
-	set *flag.FlagSet
-}
-
-// NewBoolTFlag creates a new BoolTFlag
-func NewBoolTFlag(flag cli.BoolTFlag) *BoolTFlag {
-	return &BoolTFlag{BoolTFlag: flag, set: nil}
-}
-
-// ApplyInputSourceValue applies a BoolT value to the flagSet if required
-func (f *BoolTFlag) ApplyInputSourceValue(context *cli.Context, isc InputSourceContext) error {
-	if f.set != nil {
-		if !context.IsSet(f.Name) && !isEnvVarSet(f.EnvVar) {
-			value, err := isc.BoolT(f.BoolTFlag.Name)
-			if err != nil {
-				return err
-			}
-			if !value {
-				eachName(f.Name, func(name string) {
-					f.set.Set(f.Name, strconv.FormatBool(value))
-				})
-			}
-		}
-	}
-	return nil
-}
-
-// Apply saves the flagSet for later usage then calls
-// the wrapped BoolTFlag.Apply
-func (f *BoolTFlag) Apply(set *flag.FlagSet) {
-	f.set = set
-
-	f.BoolTFlag.Apply(set)
-}
-
 // StringFlag is the flag type that wraps cli.StringFlag to allow
 // for other values to be specified
 type StringFlag struct {

altsrc/flag_test.go

@@ -145,36 +145,6 @@ func TestBoolApplyInputSourceMethodEnvVarSet(t *testing.T) {
 	expect(t, true, c.Bool("test"))
 }
 
-func TestBoolTApplyInputSourceMethodSet(t *testing.T) {
-	c := runTest(t, testApplyInputSource{
-		Flag:     NewBoolTFlag(cli.BoolTFlag{Name: "test"}),
-		FlagName: "test",
-		MapValue: false,
-	})
-	expect(t, false, c.BoolT("test"))
-}
-
-func TestBoolTApplyInputSourceMethodContextSet(t *testing.T) {
-	c := runTest(t, testApplyInputSource{
-		Flag:               NewBoolTFlag(cli.BoolTFlag{Name: "test"}),
-		FlagName:           "test",
-		MapValue:           true,
-		ContextValueString: "false",
-	})
-	expect(t, false, c.BoolT("test"))
-}
-
-func TestBoolTApplyInputSourceMethodEnvVarSet(t *testing.T) {
-	c := runTest(t, testApplyInputSource{
-		Flag:        NewBoolTFlag(cli.BoolTFlag{Name: "test", EnvVar: "TEST"}),
-		FlagName:    "test",
-		MapValue:    true,
-		EnvVarName:  "TEST",
-		EnvVarValue: "false",
-	})
-	expect(t, false, c.BoolT("test"))
-}
-
 func TestStringApplyInputSourceMethodSet(t *testing.T) {
 	c := runTest(t, testApplyInputSource{
 		Flag:     NewStringFlag(cli.StringFlag{Name: "test"}),

altsrc/input_source_context.go

@@ -17,5 +17,4 @@ type InputSourceContext interface {
 	IntSlice(name string) ([]int, error)
 	Generic(name string) (cli.Generic, error)
 	Bool(name string) (bool, error)
-	BoolT(name string) (bool, error)
 }

altsrc/map_input_source.go

@@ -215,28 +215,6 @@ func (fsm *MapInputSource) Bool(name string) (bool, error) {
 	return false, nil
 }
 
-// BoolT returns an bool from the map otherwise returns true
-func (fsm *MapInputSource) BoolT(name string) (bool, error) {
-	otherGenericValue, exists := fsm.valueMap[name]
-	if exists {
-		otherValue, isType := otherGenericValue.(bool)
-		if !isType {
-			return true, incorrectTypeForFlagError(name, "bool", otherGenericValue)
-		}
-		return otherValue, nil
-	}
-	nestedGenericValue, exists := nestedVal(name, fsm.valueMap)
-	if exists {
-		otherValue, isType := nestedGenericValue.(bool)
-		if !isType {
-			return true, incorrectTypeForFlagError(name, "bool", nestedGenericValue)
-		}
-		return otherValue, nil
-	}
-
-	return true, nil
-}
-
 func incorrectTypeForFlagError(name, expectedTypeName string, value interface{}) error {
 	valueType := reflect.TypeOf(value)
 	valueTypeName := ""

context.go

@@ -59,14 +59,6 @@ func (c *Context) Bool(name string) bool {
 	return false
 }
 
-// BoolT looks up the value of a local boolT flag, returns false if no bool flag exists
-func (c *Context) BoolT(name string) bool {
-	if fs := lookupFlagSet(name, c); fs != nil {
-		return lookupBoolT(name, fs)
-	}
-	return true
-}
-
 // String looks up the value of a local string flag, returns "" if no string flag exists
 func (c *Context) String(name string) string {
 	if fs := lookupFlagSet(name, c); fs != nil {
@@ -303,19 +295,6 @@ func lookupBool(name string, set *flag.FlagSet) bool {
 	return false
 }
 
-func lookupBoolT(name string, set *flag.FlagSet) bool {
-	f := set.Lookup(name)
-	if f != nil {
-		val, err := strconv.ParseBool(f.Value.String())
-		if err != nil {
-			return true
-		}
-		return val
-	}
-
-	return true
-}
-
 func copyFlag(name string, ff *flag.Flag, set *flag.FlagSet) {
 	switch ff.Value.(type) {
 	case Serializeder:

context_test.go

@@ -58,13 +58,6 @@ func TestContext_Bool(t *testing.T) {
 	expect(t, c.Bool("myflag"), false)
 }
 
-func TestContext_BoolT(t *testing.T) {
-	set := flag.NewFlagSet("test", 0)
-	set.Bool("myflag", true, "doc")
-	c := NewContext(nil, set, nil)
-	expect(t, c.BoolT("myflag"), true)
-}
-
 func TestContext_Args(t *testing.T) {
 	set := flag.NewFlagSet("test", 0)
 	set.Bool("myflag", false, "doc")

flag.go

@@ -321,6 +321,7 @@ func (f IntSliceFlag) GetName() string {
 // BoolFlag is a switch that defaults to false
 type BoolFlag struct {
 	Name        string
+	Value       bool
 	Usage       string
 	EnvVar      string
 	Destination *bool
@@ -334,14 +335,13 @@ func (f BoolFlag) String() string {
 
 // Apply populates the flag given the flag set and environment
 func (f BoolFlag) Apply(set *flag.FlagSet) {
-	val := false
 	if f.EnvVar != "" {
 		for _, envVar := range strings.Split(f.EnvVar, ",") {
 			envVar = strings.TrimSpace(envVar)
 			if envVal := os.Getenv(envVar); envVal != "" {
 				envValBool, err := strconv.ParseBool(envVal)
 				if err == nil {
-					val = envValBool
+					f.Value = envValBool
 				}
 				break
 			}
@@ -350,10 +350,10 @@ func (f BoolFlag) Apply(set *flag.FlagSet) {
 
 	eachName(f.Name, func(name string) {
 		if f.Destination != nil {
-			set.BoolVar(f.Destination, name, val, f.Usage)
+			set.BoolVar(f.Destination, name, f.Value, f.Usage)
 			return
 		}
-		set.Bool(name, val, f.Usage)
+		set.Bool(name, f.Value, f.Usage)
 	})
 }
 
@@ -362,51 +362,6 @@ func (f BoolFlag) GetName() string {
 	return f.Name
 }
 
-// BoolTFlag this represents a boolean flag that is true by default, but can
-// still be set to false by --some-flag=false
-type BoolTFlag struct {
-	Name        string
-	Usage       string
-	EnvVar      string
-	Destination *bool
-	Hidden      bool
-}
-
-// String returns a readable representation of this value (for usage defaults)
-func (f BoolTFlag) String() string {
-	return FlagStringer(f)
-}
-
-// Apply populates the flag given the flag set and environment
-func (f BoolTFlag) Apply(set *flag.FlagSet) {
-	val := true
-	if f.EnvVar != "" {
-		for _, envVar := range strings.Split(f.EnvVar, ",") {
-			envVar = strings.TrimSpace(envVar)
-			if envVal := os.Getenv(envVar); envVal != "" {
-				envValBool, err := strconv.ParseBool(envVal)
-				if err == nil {
-					val = envValBool
-					break
-				}
-			}
-		}
-	}
-
-	eachName(f.Name, func(name string) {
-		if f.Destination != nil {
-			set.BoolVar(f.Destination, name, val, f.Usage)
-			return
-		}
-		set.Bool(name, val, f.Usage)
-	})
-}
-
-// GetName returns the name of the flag.
-func (f BoolTFlag) GetName() string {
-	return f.Name
-}
-
 // StringFlag represents a flag that takes as string value
 type StringFlag struct {
 	Name        string
@@ -669,7 +624,7 @@ func stringifyFlag(f Flag) string {
 	defaultValueString := ""
 	val := fv.FieldByName("Value")
 
-	if val.IsValid() {
+	if val.IsValid() && val.Kind() != reflect.Bool {
 		needsPlaceholder = true
 		defaultValueString = fmt.Sprintf(" (default: %v)", val.Interface())
 

flag_test.go

@@ -843,55 +843,61 @@ func TestParseMultiBoolFromEnvCascade(t *testing.T) {
 	a.Run([]string{"run"})
 }
 
-func TestParseMultiBoolT(t *testing.T) {
+func TestParseMultiBoolTrue(t *testing.T) {
 	a := App{
 		Flags: []Flag{
-			BoolTFlag{Name: "serve, s"},
+			BoolFlag{Name: "implode, i", Value: true},
 		},
 		Action: func(ctx *Context) error {
-			if ctx.BoolT("serve") != true {
+			if ctx.Bool("implode") {
 				t.Errorf("main name not set")
 			}
-			if ctx.BoolT("s") != true {
+			if ctx.Bool("i") {
 				t.Errorf("short name not set")
 			}
 			return nil
 		},
 	}
-	a.Run([]string{"run", "--serve"})
+	a.Run([]string{"run", "--implode=false"})
 }
 
-func TestParseDestinationBoolT(t *testing.T) {
-	var dest bool
+func TestParseDestinationBoolTrue(t *testing.T) {
+	dest := true
+
 	a := App{
 		Flags: []Flag{
-			BoolTFlag{
+			BoolFlag{
 				Name:        "dest",
+				Value:       true,
 				Destination: &dest,
 			},
 		},
 		Action: func(ctx *Context) error {
-			if dest != true {
-				t.Errorf("expected destination BoolT true")
+			if dest {
+				t.Errorf("expected destination Bool false")
 			}
 			return nil
 		},
 	}
-	a.Run([]string{"run", "--dest"})
+	a.Run([]string{"run", "--dest=false"})
 }
 
-func TestParseMultiBoolTFromEnv(t *testing.T) {
+func TestParseMultiBoolTrueFromEnv(t *testing.T) {
 	os.Clearenv()
 	os.Setenv("APP_DEBUG", "0")
 	a := App{
 		Flags: []Flag{
-			BoolTFlag{Name: "debug, d", EnvVar: "APP_DEBUG"},
+			BoolFlag{
+				Name:   "debug, d",
+				Value:  true,
+				EnvVar: "APP_DEBUG",
+			},
 		},
 		Action: func(ctx *Context) error {
-			if ctx.BoolT("debug") != false {
+			if ctx.Bool("debug") {
 				t.Errorf("main name not set from env")
 			}
-			if ctx.BoolT("d") != false {
+			if ctx.Bool("d") {
 				t.Errorf("short name not set from env")
 			}
 			return nil
@@ -900,18 +906,22 @@ func TestParseMultiBoolTFromEnv(t *testing.T) {
 	a.Run([]string{"run"})
 }
 
-func TestParseMultiBoolTFromEnvCascade(t *testing.T) {
+func TestParseMultiBoolTrueFromEnvCascade(t *testing.T) {
 	os.Clearenv()
 	os.Setenv("APP_DEBUG", "0")
 	a := App{
 		Flags: []Flag{
-			BoolTFlag{Name: "debug, d", EnvVar: "COMPAT_DEBUG,APP_DEBUG"},
+			BoolFlag{
+				Name:   "debug, d",
+				Value:  true,
+				EnvVar: "COMPAT_DEBUG,APP_DEBUG",
+			},
 		},
 		Action: func(ctx *Context) error {
-			if ctx.BoolT("debug") != false {
+			if ctx.Bool("debug") {
 				t.Errorf("main name not set from env")
 			}
-			if ctx.BoolT("d") != false {
+			if ctx.Bool("d") {
 				t.Errorf("short name not set from env")
 			}
 			return nil