Merge pull request #220 from jessevdk/#218/no-unsafe
Remove unnecessary use of unsafe
diff --git a/completion.go b/completion.go
index 708fa9e..7a7a08b 100644
--- a/completion.go
+++ b/completion.go
@@ -73,27 +73,8 @@
}
}
-func (c *completion) completeOptionNames(names map[string]*Option, prefix string, match string) []Completion {
- n := make([]Completion, 0, len(names))
-
- for k, opt := range names {
- if strings.HasPrefix(k, match) && !opt.Hidden {
- n = append(n, Completion{
- Item: prefix + k,
- Description: opt.Description,
- })
- }
- }
-
- return n
-}
-
-func (c *completion) completeLongNames(s *parseState, prefix string, match string) []Completion {
- return c.completeOptionNames(s.lookup.longNames, prefix, match)
-}
-
-func (c *completion) completeShortNames(s *parseState, prefix string, match string) []Completion {
- if len(match) != 0 {
+func (c *completion) completeOptionNames(s *parseState, prefix string, match string, short bool) []Completion {
+ if short && len(match) != 0 {
return []Completion{
Completion{
Item: prefix + match,
@@ -101,7 +82,42 @@
}
}
- return c.completeOptionNames(s.lookup.shortNames, prefix, match)
+ var results []Completion
+ repeats := map[string]bool{}
+
+ for name, opt := range s.lookup.longNames {
+ if strings.HasPrefix(name, match) && !opt.Hidden {
+ results = append(results, Completion{
+ Item: defaultLongOptDelimiter + name,
+ Description: opt.Description,
+ })
+
+ if short {
+ repeats[string(opt.ShortName)] = true
+ }
+ }
+ }
+
+ if short {
+ for name, opt := range s.lookup.shortNames {
+ if _, exist := repeats[name]; !exist && strings.HasPrefix(name, match) && !opt.Hidden {
+ results = append(results, Completion{
+ Item: string(defaultShortOptDelimiter) + name,
+ Description: opt.Description,
+ })
+ }
+ }
+ }
+
+ return results
+}
+
+func (c *completion) completeNamesForLongPrefix(s *parseState, prefix string, match string) []Completion {
+ return c.completeOptionNames(s, prefix, match, false)
+}
+
+func (c *completion) completeNamesForShortPrefix(s *parseState, prefix string, match string) []Completion {
+ return c.completeOptionNames(s, prefix, match, true)
}
func (c *completion) completeCommands(s *parseState, match string) []Completion {
@@ -120,6 +136,9 @@
}
func (c *completion) completeValue(value reflect.Value, prefix string, match string) []Completion {
+ if value.Kind() == reflect.Slice {
+ value = reflect.New(value.Type().Elem())
+ }
i := value.Interface()
var ret []Completion
@@ -139,16 +158,6 @@
return ret
}
-func (c *completion) completeArg(arg *Arg, prefix string, match string) []Completion {
- if arg.isRemaining() {
- // For remaining positional args (that are parsed into a slice), complete
- // based on the element type.
- return c.completeValue(reflect.New(arg.value.Type().Elem()), prefix, match)
- }
-
- return c.completeValue(arg.value, prefix, match)
-}
-
func (c *completion) complete(args []string) []Completion {
if len(args) == 0 {
args = []string{""}
@@ -244,7 +253,7 @@
if opt := s.lookup.shortNames[sname]; opt != nil && opt.canArgument() {
ret = c.completeValue(opt.value, prefix+sname, optname[n:])
} else {
- ret = c.completeShortNames(s, prefix, optname)
+ ret = c.completeNamesForShortPrefix(s, prefix, optname)
}
} else if argument != nil {
if islong {
@@ -257,13 +266,13 @@
ret = c.completeValue(opt.value, prefix+optname+split, *argument)
}
} else if islong {
- ret = c.completeLongNames(s, prefix, optname)
+ ret = c.completeNamesForLongPrefix(s, prefix, optname)
} else {
- ret = c.completeShortNames(s, prefix, optname)
+ ret = c.completeNamesForShortPrefix(s, prefix, optname)
}
} else if len(s.positional) > 0 {
// Complete for positional argument
- ret = c.completeArg(s.positional[0], "", lastarg)
+ ret = c.completeValue(s.positional[0].value, "", lastarg)
} else if len(s.command.commands) > 0 {
// Complete for command
ret = c.completeCommands(s, lastarg)
diff --git a/completion_test.go b/completion_test.go
index 72ee158..26f70e4 100644
--- a/completion_test.go
+++ b/completion_test.go
@@ -38,6 +38,7 @@
var completionTestOptions struct {
Verbose bool `short:"v" long:"verbose" description:"Verbose messages"`
Debug bool `short:"d" long:"debug" description:"Enable debug"`
+ Info bool `short:"i" description:"Display info"`
Version bool `long:"version" description:"Show version"`
Required bool `long:"required" required:"true" description:"This is required"`
Hidden bool `long:"hidden" hidden:"true" description:"This is hidden"`
@@ -52,8 +53,13 @@
Positional struct {
Filename []Filename
} `positional-args:"yes"`
+ Extra []Filename `short:"f"`
} `command:"add-multi" description:"add multiple items"`
+ AddMultiCommandFlag struct {
+ Files []Filename `short:"f"`
+ } `command:"add-multi-flag" description:"add multiple items via flags"`
+
RemoveCommand struct {
Other bool `short:"o"`
File Filename `short:"f" long:"filename"`
@@ -82,7 +88,14 @@
{
// Short names
[]string{"-"},
- []string{"-d", "-v"},
+ []string{"--debug", "--required", "--verbose", "--version", "-i"},
+ false,
+ },
+
+ {
+ // Short names full
+ []string{"-i"},
+ []string{"-i"},
false,
},
@@ -122,7 +135,7 @@
{
// Commands
[]string{""},
- []string{"add", "add-multi", "rename", "rm"},
+ []string{"add", "add-multi", "add-multi-flag", "rename", "rm"},
false,
},
@@ -130,10 +143,11 @@
// Commands with descriptions
[]string{""},
[]string{
- "add # add an item",
- "add-multi # add multiple items",
- "rename # rename an item",
- "rm # remove an item",
+ "add # add an item",
+ "add-multi # add multiple items",
+ "add-multi-flag # add multiple items via flags",
+ "rename # rename an item",
+ "rm # remove an item",
},
true,
},
@@ -219,6 +233,12 @@
[]string{"hello universe"},
false,
},
+ {
+ // Multiple flag filename
+ []string{"add-multi-flag", "-f", filepath.Join(completionTestSourcedir, "completion")},
+ completionTestFilename,
+ false,
+ },
}
}
diff --git a/convert.go b/convert.go
index 938c3ac..984aac8 100644
--- a/convert.go
+++ b/convert.go
@@ -154,6 +154,13 @@
func convertUnmarshal(val string, retval reflect.Value) (bool, error) {
if retval.Type().NumMethod() > 0 && retval.CanInterface() {
if unmarshaler, ok := retval.Interface().(Unmarshaler); ok {
+ if retval.IsNil() {
+ retval.Set(reflect.New(retval.Type().Elem()))
+
+ // Re-assign from the new value
+ unmarshaler = retval.Interface().(Unmarshaler)
+ }
+
return true, unmarshaler.UnmarshalFlag(val)
}
}
diff --git a/group.go b/group.go
index 26a42d1..9e057ab 100644
--- a/group.go
+++ b/group.go
@@ -173,6 +173,10 @@
}
}
+func isStringFalsy(s string) bool {
+ return s == "" || s == "false" || s == "no" || s == "0"
+}
+
func (g *Group) scanStruct(realval reflect.Value, sfield *reflect.StructField, handler scanHandler) error {
stype := realval.Type()
@@ -254,10 +258,10 @@
valueName := mtag.Get("value-name")
defaultMask := mtag.Get("default-mask")
- optional := (mtag.Get("optional") != "")
- required := (mtag.Get("required") != "")
+ optional := !isStringFalsy(mtag.Get("optional"))
+ required := !isStringFalsy(mtag.Get("required"))
choices := mtag.GetMany("choice")
- hidden := (mtag.Get("hidden") != "")
+ hidden := !isStringFalsy(mtag.Get("hidden"))
option := &Option{
Description: description,
diff --git a/help.go b/help.go
index 9167847..d380305 100644
--- a/help.go
+++ b/help.go
@@ -216,8 +216,10 @@
var def string
- if len(option.DefaultMask) != 0 && option.DefaultMask != "-" {
- def = option.DefaultMask
+ if len(option.DefaultMask) != 0 {
+ if option.DefaultMask != "-" {
+ def = option.DefaultMask
+ }
} else {
def = option.defaultLiteral
}
diff --git a/help_test.go b/help_test.go
index 01942a2..bb76640 100644
--- a/help_test.go
+++ b/help_test.go
@@ -1,10 +1,12 @@
package flags
import (
+ "bufio"
"bytes"
"fmt"
"os"
"runtime"
+ "strings"
"testing"
"time"
)
@@ -36,8 +38,9 @@
} `group:"Hidden group" hidden:"yes"`
Group struct {
- Opt string `long:"opt" description:"This is a subgroup option"`
- HiddenInsideGroup string `long:"hidden-inside-group" description:"Hidden inside group" hidden:"yes"`
+ Opt string `long:"opt" description:"This is a subgroup option"`
+ HiddenInsideGroup string `long:"hidden-inside-group" description:"Hidden inside group" hidden:"yes"`
+ NotHiddenInsideGroup string `long:"not-hidden-inside-group" description:"Not hidden inside group" hidden:"false"`
Group struct {
Opt string `long:"opt" description:"This is a subsubgroup option"`
@@ -113,6 +116,7 @@
Subgroup:
/sip.opt: This is a subgroup option
+ /sip.not-hidden-inside-group: Not hidden inside group
Subsubgroup:
/sip.sap.opt: This is a subsubgroup option
@@ -159,6 +163,7 @@
Subgroup:
--sip.opt= This is a subgroup option
+ --sip.not-hidden-inside-group= Not hidden inside group
Subsubgroup:
--sip.sap.opt= This is a subsubgroup option
@@ -261,6 +266,9 @@
.TP
\fB\fB\-\-sip.opt\fR\fP
This is a subgroup option
+.TP
+\fB\fB\-\-sip.not-hidden-inside-group\fR\fP
+Not hidden inside group
.SS Subsubgroup
.TP
\fB\fB\-\-sip.sap.opt\fR\fP
@@ -466,3 +474,65 @@
assertDiff(t, got, expected, "wrapped paragraph")
}
+
+func TestHelpDefaultMask(t *testing.T) {
+ var tests = []struct {
+ opts interface{}
+ present string
+ }{
+ {
+ opts: &struct {
+ Value string `short:"v" default:"123" description:"V"`
+ }{},
+ present: "V (default: 123)\n",
+ },
+ {
+ opts: &struct {
+ Value string `short:"v" default:"123" default-mask:"abc" description:"V"`
+ }{},
+ present: "V (default: abc)\n",
+ },
+ {
+ opts: &struct {
+ Value string `short:"v" default:"123" default-mask:"-" description:"V"`
+ }{},
+ present: "V\n",
+ },
+ {
+ opts: &struct {
+ Value string `short:"v" description:"V"`
+ }{Value: "123"},
+ present: "V (default: 123)\n",
+ },
+ {
+ opts: &struct {
+ Value string `short:"v" default-mask:"abc" description:"V"`
+ }{Value: "123"},
+ present: "V (default: abc)\n",
+ },
+ {
+ opts: &struct {
+ Value string `short:"v" default-mask:"-" description:"V"`
+ }{Value: "123"},
+ present: "V\n",
+ },
+ }
+
+ for _, test := range tests {
+ p := NewParser(test.opts, HelpFlag)
+ _, err := p.ParseArgs([]string{"-h"})
+ if flagsErr, ok := err.(*Error); ok && flagsErr.Type == ErrHelp {
+ err = nil
+ }
+ if err != nil {
+ t.Fatalf("Unexpected error: %v", err)
+ }
+ h := &bytes.Buffer{}
+ w := bufio.NewWriter(h)
+ p.writeHelpOption(w, p.FindOptionByShortName('v'), p.getAlignmentInfo())
+ w.Flush()
+ if strings.Index(h.String(), test.present) < 0 {
+ t.Errorf("Not present %q\n%s", test.present, h.String())
+ }
+ }
+}
diff --git a/ini.go b/ini.go
index f9aea2e..e714d3d 100644
--- a/ini.go
+++ b/ini.go
@@ -98,8 +98,6 @@
// information on the ini file format. The returned errors can be of the type
// flags.Error or flags.IniError.
func (i *IniParser) ParseFile(filename string) error {
- i.parser.clearIsSet()
-
ini, err := readIniFromFile(filename)
if err != nil {
@@ -134,8 +132,6 @@
//
// The returned errors can be of the type flags.Error or flags.IniError.
func (i *IniParser) Parse(reader io.Reader) error {
- i.parser.clearIsSet()
-
ini, err := readIni(reader, "")
if err != nil {
diff --git a/ini_test.go b/ini_test.go
index 1d25d45..ad4852e 100644
--- a/ini_test.go
+++ b/ini_test.go
@@ -93,6 +93,9 @@
; This is a subgroup option
Opt =
+; Not hidden inside group
+NotHiddenInsideGroup =
+
[Subsubgroup]
; This is a subsubgroup option
Opt =
@@ -153,6 +156,9 @@
; This is a subgroup option
; Opt =
+; Not hidden inside group
+; NotHiddenInsideGroup =
+
[Subsubgroup]
; This is a subsubgroup option
; Opt =
@@ -211,6 +217,9 @@
; This is a subgroup option
; Opt =
+; Not hidden inside group
+; NotHiddenInsideGroup =
+
[Subsubgroup]
; This is a subsubgroup option
; Opt =
@@ -846,6 +855,29 @@
assertString(t, opts.ValueWithDefaultOverrideCli, "cli-value")
}
+func TestIniRequired(t *testing.T) {
+ var opts struct {
+ Required string `short:"r" required:"yes" description:"required"`
+ Config func(s string) error `long:"config" default:"no-ini-file" no-ini:"true"`
+ }
+
+ p := NewParser(&opts, Default)
+
+ opts.Config = func(s string) error {
+ inip := NewIniParser(p)
+ inip.ParseAsDefaults = true
+ return inip.Parse(strings.NewReader("Required = ini-value\n"))
+ }
+
+ _, err := p.ParseArgs([]string{"-r", "cli-value"})
+
+ if err != nil {
+ t.Fatalf("Failed to parse arguments: %s", err)
+ }
+
+ assertString(t, opts.Required, "cli-value")
+}
+
func TestWriteFile(t *testing.T) {
file, err := ioutil.TempFile("", "")
if err != nil {
diff --git a/option.go b/option.go
index 17a2189..ea09fb4 100644
--- a/option.go
+++ b/option.go
@@ -179,6 +179,11 @@
return option.isSet
}
+// IsSetDefault returns true if option has been set via the default option tag
+func (option *Option) IsSetDefault() bool {
+ return option.isSetDefault
+}
+
// Set the value of an option to the specified value. An error will be returned
// if the specified value could not be converted to the corresponding option
// value type.
diff --git a/parser.go b/parser.go
index fd2fd5f..0a7922a 100644
--- a/parser.go
+++ b/parser.go
@@ -479,7 +479,7 @@
msg = fmt.Sprintf("%s. You should use the %s command",
msg,
cmdnames[0])
- } else {
+ } else if len(cmdnames) > 1 {
msg = fmt.Sprintf("%s. Please specify one command of: %s or %s",
msg,
strings.Join(cmdnames[:len(cmdnames)-1], ", "),
@@ -490,7 +490,7 @@
if len(cmdnames) == 1 {
msg = fmt.Sprintf("Please specify the %s command", cmdnames[0])
- } else {
+ } else if len(cmdnames) > 1 {
msg = fmt.Sprintf("Please specify one command of: %s or %s",
strings.Join(cmdnames[:len(cmdnames)-1], ", "),
cmdnames[len(cmdnames)-1])
diff --git a/pointer_test.go b/pointer_test.go
index c5ef60a..dc779c7 100644
--- a/pointer_test.go
+++ b/pointer_test.go
@@ -62,6 +62,64 @@
}
}
+type marshalledString string
+
+func (m *marshalledString) UnmarshalFlag(value string) error {
+ *m = marshalledString(value)
+ return nil
+}
+
+func (m marshalledString) MarshalFlag() (string, error) {
+ return string(m), nil
+}
+
+func TestPointerStringMarshalled(t *testing.T) {
+ var opts = struct {
+ Value *marshalledString `short:"v"`
+ }{}
+
+ ret := assertParseSuccess(t, &opts, "-v", "value")
+
+ assertStringArray(t, ret, []string{})
+
+ if opts.Value == nil {
+ t.Error("Expected value not to be nil")
+ return
+ }
+
+ assertString(t, string(*opts.Value), "value")
+}
+
+type marshalledStruct struct {
+ Value string
+}
+
+func (m *marshalledStruct) UnmarshalFlag(value string) error {
+ m.Value = value
+ return nil
+}
+
+func (m marshalledStruct) MarshalFlag() (string, error) {
+ return m.Value, nil
+}
+
+func TestPointerStructMarshalled(t *testing.T) {
+ var opts = struct {
+ Value *marshalledStruct `short:"v"`
+ }{}
+
+ ret := assertParseSuccess(t, &opts, "-v", "value")
+
+ assertStringArray(t, ret, []string{})
+
+ if opts.Value == nil {
+ t.Error("Expected value not to be nil")
+ return
+ }
+
+ assertString(t, opts.Value.Value, "value")
+}
+
type PointerGroup struct {
Value bool `short:"v"`
}
diff --git a/short_test.go b/short_test.go
index 95712c1..5f4106b 100644
--- a/short_test.go
+++ b/short_test.go
@@ -35,6 +35,22 @@
assertParseFail(t, ErrRequired, fmt.Sprintf("the required flag `%cv' was not specified", defaultShortOptDelimiter), &opts)
}
+func TestShortRequiredFalsy1(t *testing.T) {
+ var opts = struct {
+ Value bool `short:"v" required:"false"`
+ }{}
+
+ assertParseSuccess(t, &opts)
+}
+
+func TestShortRequiredFalsy2(t *testing.T) {
+ var opts = struct {
+ Value bool `short:"v" required:"no"`
+ }{}
+
+ assertParseSuccess(t, &opts)
+}
+
func TestShortMultiConcat(t *testing.T) {
var opts = struct {
V bool `short:"v"`
@@ -192,3 +208,27 @@
assertStringArray(t, ret, []string{"f"})
assertString(t, opts.Value, "value")
}
+
+func TestShortOptionalFalsy1(t *testing.T) {
+ var opts = struct {
+ F []bool `short:"f"`
+ Value string `short:"v" optional:"false" optional-value:"value"`
+ }{}
+
+ ret := assertParseSuccess(t, &opts, "-fv", "f")
+
+ assertStringArray(t, ret, []string{})
+ assertString(t, opts.Value, "f")
+}
+
+func TestShortOptionalFalsy2(t *testing.T) {
+ var opts = struct {
+ F []bool `short:"f"`
+ Value string `short:"v" optional:"no" optional-value:"value"`
+ }{}
+
+ ret := assertParseSuccess(t, &opts, "-fv", "f")
+
+ assertStringArray(t, ret, []string{})
+ assertString(t, opts.Value, "f")
+}
