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

Add optional check for duplicated string and int type flags. #1071

Closed
Tracked by #833
arykalin opened this issue Feb 14, 2020 · 6 comments · Fixed by #1636 or #1638
Closed
Tracked by #833

Add optional check for duplicated string and int type flags. #1071

arykalin opened this issue Feb 14, 2020 · 6 comments · Fixed by #1636 or #1638
Assignees
Labels
area/v3 relates to / is being considered for v3 help wanted please help if you can! status/claimed someone has claimed this issue status/confirmed confirmed to be valid, but work has yet to start
Milestone

Comments

@arykalin
Copy link

Checklist

  • [y] Are you running the latest v2 release? The list of releases is here.
  • [y] Did you check the manual for your release? The v2 manual is here
  • [y] Did you perform a search about this feature? Here's the Github guide about searching.

What problem does this solve?

Sometimes users can accidently duplicate string type flags and then the latest value we be taken. It is not clear and bad for usability.

Solution description

Add an option called Duplicated true\false to flag options. Example:

flagFoo = &cli.StringFlag{
	Name: "foo",
	Duplicated: false,
}

A detailed description of what you want to happen.

if Duplicated set to false command should fail with error
"flag foo can not be duplicated"

Describe alternatives you've considered

For now, as a workaround, we're using StringSliceType for flags we don't want to be duplicated and check that slice have only one element in it.

https://github.com/Venafi/vcert/blob/8e1dc13c0afdd615d7f27b80190ddf3a1d07e848/cmd/vcert/commands.go#L105-L120

@arykalin arykalin added area/v2 relates to / is being considered for v2 status/triage maintainers still need to look into this labels Feb 14, 2020
@coilysiren
Copy link
Member

Sometimes users can accidently duplicate string type flags and then the latest value we be taken. It is not clear and bad for usability.

Oh wow that sucks 😅we should definitely fix this 👍although possibly only in v3? Because I'm not sure if it would be considered a "breaking change"

@coilysiren coilysiren added help wanted please help if you can! status/confirmed confirmed to be valid, but work has yet to start and removed status/triage maintainers still need to look into this labels Feb 26, 2020
@stale
Copy link

stale bot commented May 26, 2020

This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.

@stale stale bot added the status/stale stale due to the age of it's last update label May 26, 2020
@stale
Copy link

stale bot commented Jun 25, 2020

Closing this as it has become stale.

@stale stale bot closed this as completed Jun 25, 2020
@meatballhat meatballhat reopened this Apr 22, 2022
@meatballhat meatballhat removed the status/stale stale due to the age of it's last update label Apr 22, 2022
@meatballhat meatballhat changed the title v2 feature: Add optional check for duplicated string and int type flags. Add optional check for duplicated string and int type flags. Apr 22, 2022
@dearchap dearchap added area/v3 relates to / is being considered for v3 and removed area/v2 relates to / is being considered for v2 labels Oct 17, 2022
@dearchap
Copy link
Contributor

Will consider for v3

@dearchap
Copy link
Contributor

@arykalin If I understand correctly you've defined 2 flags(same type) with same name and only the last defined one will pick up values ?

@dearchap
Copy link
Contributor

dearchap commented Dec 1, 2022

@arykalin Looking at your code you've defined "instance" as a stringslice flag and then enforced that if should not be length > 1 . Is this because if you've defined instance as a string flag and then people use the flag twice like --instance id1 and --instance id2 and then only the last value is taken. That is what you would like to guard against. ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/v3 relates to / is being considered for v3 help wanted please help if you can! status/claimed someone has claimed this issue status/confirmed confirmed to be valid, but work has yet to start
Projects
None yet
4 participants