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

Return an error if the same flag is provided twice with different values #72

Open
ncw opened this issue May 30, 2016 · 5 comments
Open

Comments

@ncw
Copy link

ncw commented May 30, 2016

In rclone/rclone#506 a user is rightly complaining about the fact that two --exclude flags don't work in rclone and it didn't warn him.

Could pflag return an error if you provide the same flag twice with different values?

Thanks

Nick

@eparis
Copy link
Collaborator

eparis commented Mar 26, 2017

I feel like that is a custom flag type. I don't know of any unix command that complains when the same flag is given twice. All of them I know of either use both values or the last value, depending on the type of flag...

@ncw
Copy link
Author

ncw commented Mar 27, 2017

Now that pflag supports StringArray and friends I'm now using those in rclone so this issue is less important for me personally now.

However duplicated flags especially with arguments, when the parser can't cope with them, should make an error. Just because getopt(3) is lazy doesn't mean we have to be!

@dixudx
Copy link

dixudx commented Nov 13, 2017

I think we should apply this as default for all non-sliced types.

PTAL #151. I introduced a new method AllowingMultipleSet() to Flag. For all sliced/array types, this function is enforced as default.

@normanr
Copy link

normanr commented May 10, 2020

I feel like that is a custom flag type. I don't know of any unix command that complains when the same flag is given twice. All of them I know of either use both values or the last value, depending on the type of flag...

here's an example (not a core unix command, but it's on most systems these days)

$ ssh localhost -W host:1234 -W host:1235
stdio forward already specified

Luap99 pushed a commit to Luap99/libpod that referenced this issue Nov 20, 2020
We allow a container to be connected to several cni networks
but only if they are listed comma sperated. This is not intuitive
for users especially since the flag parsing allows multiple string
flags but only would take the last value. see: spf13/pflag#72

Also get rid of the extra parsing logic for pods. The invalid options
are already handled by `pkg/specgen`.

A test is added to prevent a future regression.

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
@niamster
Copy link

I upvote this feature request.

Here's my use case: depending on the context of a sub-command, some flags might have different meaning.
In one case --foo can be given only once, while in other cases --foo might be given multiple times.
It might be confusing for a user if it's allowed to specify multiple --foo but only the last value is used.
I agree that it's reasonable to argue that it might be a bad design, but in some cases reworking an application to add a new flag --foos or converting all --foo to array and checking the length manually and failing if len(foo) != 1 might not be acceptable either.

The other use case is that the users might copy/paste or reuse commands from history and modify some of the arguments. It might happen that the command looks like --foo --bar ..... --zoo=A and the user wants to replace --bar with --bin --zoo=B. The user might be surprised by the result.

I'm happy to help designing this feature and working on PRs.
It seems like it's a reasonably small change, it's possible to reuse existing Changed field

pflag/flag.go

Line 481 in d5e0c06

if !flag.Changed {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants