Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: flag: Add MaxArgs and MinArgs to check for required number of arguments #35768

Closed
mattn opened this issue Nov 22, 2019 · 3 comments
Closed

Comments

@mattn
Copy link
Member

mattn commented Nov 22, 2019

Current implementation of flag does not have a way to check whether the meaningless arguments are given. We must check with NArg() == 0 in our self. but afaik most of applications does not do the check.

flag.Int("i", 0, "int value")
flag.Parse()
$ ./app -i 0 blah blah blah blah blah blah blah

I guess there are many applications which does not check maximum count of arguments. So I suggest that flag add simply two fields MinArgs/MaxArgs to check count of arguments.

diff --git a/src/flag/flag.go b/src/flag/flag.go
index 6a51617524..193737342e 100644
--- a/src/flag/flag.go
+++ b/src/flag/flag.go
@@ -325,6 +325,14 @@ type FlagSet struct {
 	// to ExitOnError, which exits the program after calling Usage.
 	Usage func()
 
+	// MinArgs request minimum count of arguments given. Default value is -1
+	// for no checking. This value must not be greater than MaxArgs.
+	MinArgs int
+
+	// MaxArgs request maximum count of arguments given. Default value is -1
+	// for no checking.
+	MaxArgs int
+
 	name          string
 	parsed        bool
 	actual        map[string]*Flag
@@ -984,6 +992,12 @@ func (f *FlagSet) Parse(arguments []string) error {
 			panic(err)
 		}
 	}
+	if f.MinArgs >= 0 && len(f.args) < f.MinArgs {
+		return f.failf("too few arguments")
+	}
+	if f.MaxArgs >= 0 && len(f.args) > f.MaxArgs {
+		return f.failf("too many arguments")
+	}
 	return nil
 }
 
@@ -1028,6 +1042,8 @@ func NewFlagSet(name string, errorHandling ErrorHandling) *FlagSet {
 	f := &FlagSet{
 		name:          name,
 		errorHandling: errorHandling,
+		MinArgs:       -1,
+		MaxArgs:       -1,
 	}
 	f.Usage = f.defaultUsage
 	return f
diff --git a/src/flag/flag_test.go b/src/flag/flag_test.go
index 0d9491c020..ea7b99163f 100644
--- a/src/flag/flag_test.go
+++ b/src/flag/flag_test.go
@@ -544,3 +544,29 @@ func TestRangeError(t *testing.T) {
 		}
 	}
 }
+
+func TestMinArgs(t *testing.T) {
+	var flags FlagSet
+	var buf bytes.Buffer
+	flags.SetOutput(&buf)
+	flags.Init("test", ContinueOnError)
+	flags.Int("i", 0, "int value")
+	flags.MinArgs = 2
+	flags.Parse([]string{"-i", "0", "foo"})
+	if out := buf.String(); !strings.Contains(out, "too few arguments") {
+		t.Errorf("expected output mentioning too few arguments; got %q", out)
+	}
+}
+
+func TestMaxArgs(t *testing.T) {
+	var flags FlagSet
+	var buf bytes.Buffer
+	flags.SetOutput(&buf)
+	flags.Init("test", ContinueOnError)
+	flags.Int("i", 0, "int value")
+	flags.MaxArgs = 1
+	flags.Parse([]string{"-i", "0", "foo", "bar"})
+	if out := buf.String(); !strings.Contains(out, "too many arguments") {
+		t.Errorf("expected output mentioning too many arguments; got %q", out)
+	}
+}

This is not for MUST but for BETTER.

@gopherbot gopherbot added this to the Proposal milestone Nov 22, 2019
@ianlancetaylor ianlancetaylor changed the title Proposal: Add MaxArgs to flag for check the meaningless arguments proposal: flag: Add MaxArgs and MinArgs to check for required number of arguments Nov 22, 2019
@ianlancetaylor
Copy link
Contributor

This seems to me to mix flag handling with other argument handling. The flag package is for flags, not for general purpose argument handling.

@mvdan
Copy link
Member

mvdan commented Nov 23, 2019

I agree with Ian. If we did this, it would be inconsistent to not add proper argument handling too. For example, the first argument is a string, the second is a duration, etc.

Doing this yourself is also just six lines of code. If you want to handle arguments, you need extra code or a third party library anyway, since the flag package doesn't do that for you.

@mattn
Copy link
Member Author

mattn commented Nov 27, 2019

Thank you for your opinion.

@mattn mattn closed this as completed Nov 27, 2019
@golang golang locked and limited conversation to collaborators Nov 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants