Merge pull request #342 from sv99/help-pos-required

Help for positional args without allcmd.ArgsRequired dependent from arg.Required
diff --git a/.travis.yml b/.travis.yml
index 06dbe72..30dbe17 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -4,14 +4,15 @@
   - linux
   - osx
 
+arch:
+    - amd64
+    - ppc64le
+
 go:
-  - 1.x
-  - 1.9.x
-  - 1.10.x
+  - 1.16.x
 
 install:
   # go-flags
-  - go get -d -v ./...
   - go build -v ./...
 
   # linting
diff --git a/completion.go b/completion.go
index 74bd5a3..8ed61f1 100644
--- a/completion.go
+++ b/completion.go
@@ -82,7 +82,7 @@
 func (c *completion) completeOptionNames(s *parseState, prefix string, match string, short bool) []Completion {
 	if short && len(match) != 0 {
 		return []Completion{
-			Completion{
+			{
 				Item: prefix + match,
 			},
 		}
diff --git a/convert.go b/convert.go
index cda29b2..ef06d4b 100644
--- a/convert.go
+++ b/convert.go
@@ -53,7 +53,7 @@
 
 func convertMarshal(val reflect.Value) (bool, string, error) {
 	// Check first for the Marshaler interface
-	if val.Type().NumMethod() > 0 && val.CanInterface() {
+	if val.IsValid() && val.Type().NumMethod() > 0 && val.CanInterface() {
 		if marshaler, ok := val.Interface().(Marshaler); ok {
 			ret, err := marshaler.MarshalFlag()
 			return true, ret, err
@@ -68,6 +68,10 @@
 		return ret, err
 	}
 
+	if !val.IsValid() {
+		return "", nil
+	}
+
 	tp := val.Type()
 
 	// Support for time.Duration
diff --git a/go.mod b/go.mod
new file mode 100644
index 0000000..a626c5d
--- /dev/null
+++ b/go.mod
@@ -0,0 +1,5 @@
+module github.com/jessevdk/go-flags
+
+go 1.15
+
+require golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4
diff --git a/go.sum b/go.sum
new file mode 100644
index 0000000..7503251
--- /dev/null
+++ b/go.sum
@@ -0,0 +1,2 @@
+golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4 h1:EZ2mChiOa8udjfp6rRmswTbtZN/QzUQp4ptM4rnjHvc=
+golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
diff --git a/help.go b/help.go
index bbf764b..8fd3244 100644
--- a/help.go
+++ b/help.go
@@ -72,15 +72,15 @@
 	var prevcmd *Command
 
 	p.eachActiveGroup(func(c *Command, grp *Group) {
-		if !grp.showInHelp() {
-			return
-		}
 		if c != prevcmd {
 			for _, arg := range c.args {
 				ret.updateLen(arg.Name, c != p.Command)
 			}
+			prevcmd = c
 		}
-
+		if !grp.showInHelp() {
+			return
+		}
 		for _, info := range grp.options {
 			if !info.showInHelp() {
 				continue
diff --git a/help_test.go b/help_test.go
index 27c3d0e..c713bba 100644
--- a/help_test.go
+++ b/help_test.go
@@ -216,7 +216,7 @@
 	var opts helpOptions
 	p := NewNamedParser("TestMan", HelpFlag)
 	p.ShortDescription = "Test manpage generation"
-	p.LongDescription = "This is a somewhat `longer' description of what this does"
+	p.LongDescription = "This is a somewhat `longer' description of what this does.\nWith multiple lines."
 	p.AddGroup("Application Options", "The application options", &opts)
 
 	for _, cmd := range p.Commands() {
@@ -244,7 +244,8 @@
 .SH SYNOPSIS
 \fBTestMan\fP [OPTIONS]
 .SH DESCRIPTION
-This is a somewhat \fBlonger\fP description of what this does
+This is a somewhat \fBlonger\fP description of what this does.
+With multiple lines.
 .SH OPTIONS
 .SS Application Options
 The application options
@@ -389,6 +390,91 @@
 	}
 }
 
+func TestHiddenCommandNoBuiltinHelp(t *testing.T) {
+	oldEnv := EnvSnapshot()
+	defer oldEnv.Restore()
+	os.Setenv("ENV_DEFAULT", "env-def")
+
+	// no auto added help group
+	p := NewNamedParser("TestHelpCommand", 0)
+	// and no usage information either
+	p.Usage = ""
+
+	// add custom help group which is not listed in --help output
+	var help struct {
+		ShowHelp func() error `short:"h" long:"help"`
+	}
+	help.ShowHelp = func() error {
+		return &Error{Type: ErrHelp}
+	}
+	hlpgrp, err := p.AddGroup("Help Options", "", &help)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+	hlpgrp.Hidden = true
+	hlp := p.FindOptionByLongName("help")
+	hlp.Description = "Show this help message"
+	// make sure the --help option is hidden
+	hlp.Hidden = true
+
+	// add a hidden command
+	var hiddenCmdOpts struct {
+		Foo        bool `short:"f" long:"very-long-foo-option" description:"Very long foo description"`
+		Bar        bool `short:"b" description:"Option bar"`
+		Positional struct {
+			PositionalFoo string `positional-arg-name:"<positional-foo>" description:"positional foo"`
+		} `positional-args:"yes"`
+	}
+	cmdHidden, err := p.Command.AddCommand("hidden", "Hidden command description", "Long hidden command description", &hiddenCmdOpts)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+	// make it hidden
+	cmdHidden.Hidden = true
+	if len(cmdHidden.Options()) != 2 {
+		t.Fatalf("unexpected options count")
+	}
+	// which help we ask for explicitly
+	_, err = p.ParseArgs([]string{"hidden", "--help"})
+
+	if err == nil {
+		t.Fatalf("Expected help error")
+	}
+	if e, ok := err.(*Error); !ok {
+		t.Fatalf("Expected flags.Error, but got %T", err)
+	} else {
+		if e.Type != ErrHelp {
+			t.Errorf("Expected flags.ErrHelp type, but got %s", e.Type)
+		}
+
+		var expected string
+
+		if runtime.GOOS == "windows" {
+			expected = `Usage:
+  TestHelpCommand hidden [hidden-OPTIONS] [<positional-foo>]
+
+Long hidden command description
+
+[hidden command arguments]
+  <positional-foo>:         positional foo
+`
+		} else {
+			expected = `Usage:
+  TestHelpCommand hidden [hidden-OPTIONS] [<positional-foo>]
+
+Long hidden command description
+
+[hidden command arguments]
+  <positional-foo>:         positional foo
+`
+		}
+		h := &bytes.Buffer{}
+		p.WriteHelp(h)
+
+		assertDiff(t, h.String(), expected, "help message")
+	}
+}
+
 func TestHelpDefaults(t *testing.T) {
 	var expected string
 
@@ -590,10 +676,10 @@
 		isHelp bool
 	}
 	tests := map[string]testInfo{
-		"No error":    testInfo{value: nil, isHelp: false},
-		"Plain error": testInfo{value: errors.New("an error"), isHelp: false},
-		"ErrUnknown":  testInfo{value: newError(ErrUnknown, "an error"), isHelp: false},
-		"ErrHelp":     testInfo{value: newError(ErrHelp, "an error"), isHelp: true},
+		"No error":    {value: nil, isHelp: false},
+		"Plain error": {value: errors.New("an error"), isHelp: false},
+		"ErrUnknown":  {value: newError(ErrUnknown, "an error"), isHelp: false},
+		"ErrHelp":     {value: newError(ErrHelp, "an error"), isHelp: true},
 	}
 
 	for name, test := range tests {
diff --git a/ini.go b/ini.go
index 5debe74..6e3194e 100644
--- a/ini.go
+++ b/ini.go
@@ -499,6 +499,10 @@
 func (i *IniParser) parse(ini *ini) error {
 	p := i.parser
 
+	p.eachOption(func(cmd *Command, group *Group, option *Option) {
+		option.clearReferenceBeforeSet = true
+	})
+
 	var quotesLookup = make(map[*Option]bool)
 
 	for name, section := range ini.Sections {
@@ -541,9 +545,8 @@
 				continue
 			}
 
-			// ini value is ignored if override is set and
-			// value was previously set from non default
-			if i.ParseAsDefaults && !opt.isSetDefault {
+			// ini value is ignored if parsed as default but defaults are prevented
+			if i.ParseAsDefaults && opt.preventDefault {
 				continue
 			}
 
@@ -576,7 +579,15 @@
 				}
 			}
 
-			if err := opt.set(pval); err != nil {
+			var err error
+
+			if i.ParseAsDefaults {
+				err = opt.setDefault(pval)
+			} else {
+				err = opt.Set(pval)
+			}
+
+			if err != nil {
 				return &IniError{
 					Message:    err.Error(),
 					File:       ini.File,
@@ -584,6 +595,9 @@
 				}
 			}
 
+			// Defaults from ini files take precendence over defaults from parser
+			opt.preventDefault = true
+
 			// either all INI values are quoted or only values who need quoting
 			if _, ok := quotesLookup[opt]; !inival.Quoted || !ok {
 				quotesLookup[opt] = inival.Quoted
diff --git a/ini_test.go b/ini_test.go
index 65fc3e9..72c890c 100644
--- a/ini_test.go
+++ b/ini_test.go
@@ -903,6 +903,51 @@
 	assertString(t, opts.ValueWithDefaultOverrideCli, "cli-value")
 }
 
+func TestIniOverridesFromConfigFlag(t *testing.T) {
+	file, err := ioutil.TempFile("", "")
+
+	if err != nil {
+		t.Fatalf("Cannot create temporary file: %s", err)
+	}
+
+	defer os.Remove(file.Name())
+
+	_, err = file.WriteString("value-with-default = \"ini-value\"\n")
+	_, err = file.WriteString("value-with-default-override-cli = \"ini-value\"\n")
+
+	if err != nil {
+		t.Fatalf("Cannot write to temporary file: %s", err)
+	}
+
+	file.Close()
+
+	var opts struct {
+		Config                      func(filename string) `long:"config"`
+		ValueWithDefault            string                `long:"value-with-default" default:"value"`
+		ValueWithDefaultOverrideCli string                `long:"value-with-default-override-cli" default:"value"`
+	}
+
+	p := NewParser(&opts, Default)
+
+	opt := p.FindOptionByLongName("config")
+	opt.Default = []string{file.Name()}
+
+	opts.Config = func(filename string) {
+		parser := NewIniParser(p)
+		parser.ParseAsDefaults = true
+		parser.ParseFile(filename)
+	}
+
+	_, err = p.ParseArgs([]string{"--value-with-default-override-cli", "cli-value"})
+
+	if err != nil {
+		t.Fatalf("Failed to parse arguments: %s", err)
+	}
+
+	assertString(t, opts.ValueWithDefault, "ini-value")
+	assertString(t, opts.ValueWithDefaultOverrideCli, "cli-value")
+}
+
 func TestIniRequired(t *testing.T) {
 	var opts struct {
 		Required string               `short:"r" required:"yes" description:"required"`
@@ -926,6 +971,30 @@
 	assertString(t, opts.Required, "cli-value")
 }
 
+func TestIniRequiredSlice_ShouldNotNeedToBeSpecifiedOnCli(t *testing.T) {
+	type options struct {
+		Items []string `long:"item" required:"true"`
+	}
+	var opts options
+	ini := `
+[Application Options]
+item=abc`
+	args := []string{}
+
+	parser := NewParser(&opts, Default)
+	inip := NewIniParser(parser)
+
+	inip.Parse(strings.NewReader(ini))
+
+	_, err := parser.ParseArgs(args)
+
+	if err != nil {
+		t.Fatalf("Unexpected failure: %v", err)
+	}
+
+	assertString(t, opts.Items[0], "abc")
+}
+
 func TestWriteFile(t *testing.T) {
 	file, err := ioutil.TempFile("", "")
 	if err != nil {
diff --git a/man.go b/man.go
index 7e746af..82572f9 100644
--- a/man.go
+++ b/man.go
@@ -10,30 +10,41 @@
 	"time"
 )
 
+func manQuoteLines(s string) string {
+	lines := strings.Split(s, "\n")
+	parts := []string{}
+
+	for _, line := range lines {
+		parts = append(parts, manQuote(line))
+	}
+
+	return strings.Join(parts, "\n")
+}
+
 func manQuote(s string) string {
 	return strings.Replace(s, "\\", "\\\\", -1)
 }
 
-func formatForMan(wr io.Writer, s string) {
+func formatForMan(wr io.Writer, s string, quoter func(s string) string) {
 	for {
 		idx := strings.IndexRune(s, '`')
 
 		if idx < 0 {
-			fmt.Fprintf(wr, "%s", manQuote(s))
+			fmt.Fprintf(wr, "%s", quoter(s))
 			break
 		}
 
-		fmt.Fprintf(wr, "%s", manQuote(s[:idx]))
+		fmt.Fprintf(wr, "%s", quoter(s[:idx]))
 
 		s = s[idx+1:]
 		idx = strings.IndexRune(s, '\'')
 
 		if idx < 0 {
-			fmt.Fprintf(wr, "%s", manQuote(s))
+			fmt.Fprintf(wr, "%s", quoter(s))
 			break
 		}
 
-		fmt.Fprintf(wr, "\\fB%s\\fP", manQuote(s[:idx]))
+		fmt.Fprintf(wr, "\\fB%s\\fP", quoter(s[:idx]))
 		s = s[idx+1:]
 	}
 }
@@ -50,7 +61,7 @@
 			fmt.Fprintf(wr, ".SS %s\n", group.ShortDescription)
 
 			if group.LongDescription != "" {
-				formatForMan(wr, group.LongDescription)
+				formatForMan(wr, group.LongDescription, manQuoteLines)
 				fmt.Fprintln(wr, "")
 			}
 		}
@@ -100,7 +111,7 @@
 			fmt.Fprintln(wr, "\\fP")
 
 			if len(opt.Description) != 0 {
-				formatForMan(wr, opt.Description)
+				formatForMan(wr, opt.Description, manQuoteLines)
 				fmt.Fprintln(wr, "")
 			}
 		}
@@ -139,10 +150,10 @@
 		if strings.HasPrefix(command.LongDescription, cmdstart) {
 			fmt.Fprintf(wr, "The \\fI%s\\fP command", manQuote(command.Name))
 
-			formatForMan(wr, command.LongDescription[len(cmdstart):])
+			formatForMan(wr, command.LongDescription[len(cmdstart):], manQuoteLines)
 			fmt.Fprintln(wr, "")
 		} else {
-			formatForMan(wr, command.LongDescription)
+			formatForMan(wr, command.LongDescription, manQuoteLines)
 			fmt.Fprintln(wr, "")
 		}
 	}
@@ -185,7 +196,7 @@
 
 	fmt.Fprintf(wr, ".TH %s 1 \"%s\"\n", manQuote(p.Name), t.Format("2 January 2006"))
 	fmt.Fprintln(wr, ".SH NAME")
-	fmt.Fprintf(wr, "%s \\- %s\n", manQuote(p.Name), manQuote(p.ShortDescription))
+	fmt.Fprintf(wr, "%s \\- %s\n", manQuote(p.Name), manQuoteLines(p.ShortDescription))
 	fmt.Fprintln(wr, ".SH SYNOPSIS")
 
 	usage := p.Usage
@@ -197,7 +208,7 @@
 	fmt.Fprintf(wr, "\\fB%s\\fP %s\n", manQuote(p.Name), manQuote(usage))
 	fmt.Fprintln(wr, ".SH DESCRIPTION")
 
-	formatForMan(wr, p.LongDescription)
+	formatForMan(wr, p.LongDescription, manQuoteLines)
 	fmt.Fprintln(wr, "")
 
 	fmt.Fprintln(wr, ".SH OPTIONS")
diff --git a/option.go b/option.go
index bc41916..257996a 100644
--- a/option.go
+++ b/option.go
@@ -80,10 +80,11 @@
 	// Determines if the option will be always quoted in the INI output
 	iniQuote bool
 
-	tag            multiTag
-	isSet          bool
-	isSetDefault   bool
-	preventDefault bool
+	tag                     multiTag
+	isSet                   bool
+	isSetDefault            bool
+	preventDefault          bool
+	clearReferenceBeforeSet bool
 
 	defaultLiteral string
 }
@@ -238,15 +239,16 @@
 // 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.
-func (option *Option) set(value *string) error {
+func (option *Option) Set(value *string) error {
 	kind := option.value.Type().Kind()
 
-	if (kind == reflect.Map || kind == reflect.Slice) && !option.isSet {
+	if (kind == reflect.Map || kind == reflect.Slice) && option.clearReferenceBeforeSet {
 		option.empty()
 	}
 
 	option.isSet = true
 	option.preventDefault = true
+	option.clearReferenceBeforeSet = false
 
 	if len(option.Choices) != 0 {
 		found := false
@@ -280,6 +282,21 @@
 	return convert("", option.value, option.tag)
 }
 
+func (option *Option) setDefault(value *string) error {
+	if option.preventDefault {
+		return nil
+	}
+
+	if err := option.Set(value); err != nil {
+		return err
+	}
+
+	option.isSetDefault = true
+	option.preventDefault = false
+
+	return nil
+}
+
 func (option *Option) showInHelp() bool {
 	return !option.Hidden && (option.ShortName != 0 || len(option.LongName) != 0)
 }
@@ -309,6 +326,10 @@
 }
 
 func (option *Option) clearDefault() error {
+	if option.preventDefault {
+		return nil
+	}
+
 	usedDefault := option.Default
 
 	if envKey := option.EnvKeyWithNamespace(); envKey != "" {
@@ -327,11 +348,11 @@
 		option.empty()
 
 		for _, d := range usedDefault {
-			err := option.set(&d)
+			err := option.setDefault(&d)
+
 			if err != nil {
 				return err
 			}
-			option.isSetDefault = true
 		}
 	} else {
 		tp := option.value.Type()
diff --git a/parser.go b/parser.go
index 0c31b94..738f568 100644
--- a/parser.go
+++ b/parser.go
@@ -9,6 +9,7 @@
 	"fmt"
 	"os"
 	"path"
+	"reflect"
 	"sort"
 	"strings"
 	"unicode/utf8"
@@ -207,8 +208,7 @@
 	}
 
 	p.eachOption(func(c *Command, g *Group, option *Option) {
-		option.isSet = false
-		option.isSetDefault = false
+		option.clearReferenceBeforeSet = true
 		option.updateDefaultLiteral()
 	})
 
@@ -310,17 +310,10 @@
 
 	if s.err == nil {
 		p.eachOption(func(c *Command, g *Group, option *Option) {
-			if option.preventDefault {
-				return
-			}
-
 			err := option.clearDefault()
 			if err != nil {
 				if _, ok := err.(*Error); !ok {
-					err = newErrorf(ErrMarshal, "invalid argument for flag `%s' (expected %s): %s",
-						option,
-						option.value.Type(),
-						err.Error())
+					err = p.marshalError(option, err)
 				}
 				s.err = err
 			}
@@ -532,7 +525,7 @@
 			return newErrorf(ErrNoArgumentForBool, "bool flag `%s' cannot have an argument", option)
 		}
 
-		err = option.set(nil)
+		err = option.Set(nil)
 	} else if argument != nil || (canarg && !s.eof()) {
 		var arg string
 
@@ -553,13 +546,13 @@
 		}
 
 		if err == nil {
-			err = option.set(&arg)
+			err = option.Set(&arg)
 		}
 	} else if option.OptionalArgument {
 		option.empty()
 
 		for _, v := range option.OptionalValue {
-			err = option.set(&v)
+			err = option.Set(&v)
 
 			if err != nil {
 				break
@@ -571,16 +564,37 @@
 
 	if err != nil {
 		if _, ok := err.(*Error); !ok {
-			err = newErrorf(ErrMarshal, "invalid argument for flag `%s' (expected %s): %s",
-				option,
-				option.value.Type(),
-				err.Error())
+			err = p.marshalError(option, err)
 		}
 	}
 
 	return err
 }
 
+func (p *Parser) marshalError(option *Option, err error) *Error {
+	s := "invalid argument for flag `%s'"
+
+	expected := p.expectedType(option)
+
+	if expected != "" {
+		s = s + " (expected " + expected + ")"
+	}
+
+	return newErrorf(ErrMarshal, s+": %s",
+		option,
+		err.Error())
+}
+
+func (p *Parser) expectedType(option *Option) string {
+	valueType := option.value.Type()
+
+	if valueType.Kind() == reflect.Func {
+		return ""
+	}
+
+	return valueType.String()
+}
+
 func (p *Parser) parseLong(s *parseState, name string, argument *string) error {
 	if option := s.lookup.longNames[name]; option != nil {
 		// Only long options that are required can consume an argument
@@ -698,13 +712,3 @@
 
 	return err
 }
-
-func (p *Parser) clearIsSet() {
-	p.eachCommand(func(c *Command) {
-		c.eachGroup(func(g *Group) {
-			for _, option := range g.options {
-				option.isSet = false
-			}
-		})
-	}, true)
-}
diff --git a/tag_test.go b/tag_test.go
index 9daa740..2c34323 100644
--- a/tag_test.go
+++ b/tag_test.go
@@ -6,7 +6,7 @@
 
 func TestTagMissingColon(t *testing.T) {
 	var opts = struct {
-		Value bool `short`
+		TestValue bool `short`
 	}{}
 
 	assertParseFail(t, ErrTag, "expected `:' after key name, but got end of tag (in `short`)", &opts, "")
@@ -14,7 +14,7 @@
 
 func TestTagMissingValue(t *testing.T) {
 	var opts = struct {
-		Value bool `short:`
+		TestValue bool `short:`
 	}{}
 
 	assertParseFail(t, ErrTag, "expected `\"' to start tag value at end of tag (in `short:`)", &opts, "")
@@ -22,7 +22,7 @@
 
 func TestTagMissingQuote(t *testing.T) {
 	var opts = struct {
-		Value bool `short:"v`
+		TestValue bool `short:"v`
 	}{}
 
 	assertParseFail(t, ErrTag, "expected end of tag value `\"' at end of tag (in `short:\"v`)", &opts, "")
@@ -30,7 +30,7 @@
 
 func TestTagNewline(t *testing.T) {
 	var opts = struct {
-		Value bool `long:"verbose" description:"verbose
+		TestValue bool `long:"verbose" description:"verbose
 something"`
 	}{}