Merge pull request #2443 from ipfs/feat/default-option-vals

add default option value support to commands lib

commands/option.go

 package commands
 
 import (
+	"fmt"
 	"reflect"
 
 	"gx/ipfs/QmZNVWh8LLjAavuQ2JXuFmuYH3C11xo988vSgp7UQrTRj1/go-ipfs-util"
@@ -18,15 +19,18 @@ const (
 
 // Option is used to specify a field that will be provided by a consumer
 type Option interface {
-	Names() []string     // a list of unique names matched with user-provided flags
-	Type() reflect.Kind  // value must be this type
-	Description() string // a short string that describes this option
+	Names() []string            // a list of unique names matched with user-provided flags
+	Type() reflect.Kind         // value must be this type
+	Description() string        // a short string that describes this option
+	Default(interface{}) Option // sets the default value of the option
+	DefaultVal() interface{}
 }
 
 type option struct {
 	names       []string
 	kind        reflect.Kind
 	description string
+	defaultVal  interface{}
 }
 
 func (o *option) Names() []string {
@@ -38,6 +42,13 @@ func (o *option) Type() reflect.Kind {
 }
 
 func (o *option) Description() string {
+	if o.description[len(o.description)-1] != '.' {
+		o.description += "."
+	}
+
+	if o.defaultVal != nil {
+		return fmt.Sprintf("%s Default: %v.", o.description, o.defaultVal)
+	}
 	return o.description
 }
 
@@ -58,6 +69,15 @@ func NewOption(kind reflect.Kind, names ...string) Option {
 	}
 }
 
+func (o *option) Default(v interface{}) Option {
+	o.defaultVal = v
+	return o
+}
+
+func (o *option) DefaultVal() interface{} {
+	return o.defaultVal
+}
+
 // TODO handle description separately. this will take care of the panic case in
 // NewOption
 
@@ -98,7 +118,7 @@ func (ov OptionValue) Definition() Option {
 
 // value accessor methods, gets the value as a certain type
 func (ov OptionValue) Bool() (value bool, found bool, err error) {
-	if !ov.found {
+	if !ov.found && ov.value == nil {
 		return false, false, nil
 	}
 	val, ok := ov.value.(bool)
@@ -109,7 +129,7 @@ func (ov OptionValue) Bool() (value bool, found bool, err error) {
 }
 
 func (ov OptionValue) Int() (value int, found bool, err error) {
-	if !ov.found {
+	if !ov.found && ov.value == nil {
 		return 0, false, nil
 	}
 	val, ok := ov.value.(int)
@@ -120,7 +140,7 @@ func (ov OptionValue) Int() (value int, found bool, err error) {
 }
 
 func (ov OptionValue) Uint() (value uint, found bool, err error) {
-	if !ov.found {
+	if !ov.found && ov.value == nil {
 		return 0, false, nil
 	}
 	val, ok := ov.value.(uint)
@@ -131,7 +151,7 @@ func (ov OptionValue) Uint() (value uint, found bool, err error) {
 }
 
 func (ov OptionValue) Float() (value float64, found bool, err error) {
-	if !ov.found {
+	if !ov.found && ov.value == nil {
 		return 0, false, nil
 	}
 	val, ok := ov.value.(float64)
@@ -142,7 +162,7 @@ func (ov OptionValue) Float() (value float64, found bool, err error) {
 }
 
 func (ov OptionValue) String() (value string, found bool, err error) {
-	if !ov.found {
+	if !ov.found && ov.value == nil {
 		return "", false, nil
 	}
 	val, ok := ov.value.(string)

commands/option_test.go

@@ -9,13 +9,6 @@ func TestOptionValueExtractBoolNotFound(t *testing.T) {
 	if err != nil {
 		t.Fatal("Found was false. Err should have been nil")
 	}
-
-	t.Log("ensure that no error is returned when value is not found (even if value exists)")
-	optval = &OptionValue{value: "wrong type: a string", found: false}
-	_, _, err = optval.Bool()
-	if err != nil {
-		t.Fatal("Found was false. Err should have been nil")
-	}
 }
 
 func TestOptionValueExtractWrongType(t *testing.T) {

commands/request.go

@@ -118,8 +118,7 @@ func (r *request) Option(name string) *OptionValue {
 		}
 	}
 
-	// MAYBE_TODO: use default value instead of nil
-	return &OptionValue{nil, false, option}
+	return &OptionValue{option.DefaultVal(), false, option}
 }
 
 // Options returns a copy of the option map

core/commands/pin.go

@@ -41,7 +41,7 @@ var addPinCmd = &cmds.Command{
 		cmds.StringArg("ipfs-path", true, true, "Path to object(s) to be pinned.").EnableStdin(),
 	},
 	Options: []cmds.Option{
-		cmds.BoolOption("recursive", "r", "Recursively pin the object linked to by the specified object(s)."),
+		cmds.BoolOption("recursive", "r", "Recursively pin the object linked to by the specified object(s).").Default(true),
 	},
 	Type: PinOutput{},
 	Run: func(req cmds.Request, res cmds.Response) {
@@ -54,14 +54,11 @@ var addPinCmd = &cmds.Command{
 		defer n.Blockstore.PinLock().Unlock()
 
 		// set recursive flag
-		recursive, found, err := req.Option("recursive").Bool()
+		recursive, _, err := req.Option("recursive").Bool()
 		if err != nil {
 			res.SetError(err, cmds.ErrNormal)
 			return
 		}
-		if !found {
-			recursive = true
-		}
 
 		added, err := corerepo.Pin(n, req.Context(), req.Arguments(), recursive)
 		if err != nil {
@@ -108,7 +105,7 @@ collected if needed. (By default, recursively. Use -r=false for direct pins)
 		cmds.StringArg("ipfs-path", true, true, "Path to object(s) to be unpinned.").EnableStdin(),
 	},
 	Options: []cmds.Option{
-		cmds.BoolOption("recursive", "r", "Recursively unpin the object linked to by the specified object(s)."),
+		cmds.BoolOption("recursive", "r", "Recursively unpin the object linked to by the specified object(s).").Default(true),
 	},
 	Type: PinOutput{},
 	Run: func(req cmds.Request, res cmds.Response) {
@@ -119,14 +116,11 @@ collected if needed. (By default, recursively. Use -r=false for direct pins)
 		}
 
 		// set recursive flag
-		recursive, found, err := req.Option("recursive").Bool()
+		recursive, _, err := req.Option("recursive").Bool()
 		if err != nil {
 			res.SetError(err, cmds.ErrNormal)
 			return
 		}
-		if !found {
-			recursive = true // default
-		}
 
 		removed, err := corerepo.Unpin(n, req.Context(), req.Arguments(), recursive)
 		if err != nil {