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

Any best practice to imitate the "either-or" flags? #1216

Closed
stevefan1999-personal opened this issue Sep 3, 2020 · 19 comments
Closed

Any best practice to imitate the "either-or" flags? #1216

stevefan1999-personal opened this issue Sep 3, 2020 · 19 comments
Assignees
Labels
lifecycle/needs-pr Ready for a PR from the community triage/needs-info Needs more investigation from maintainers or more info from the issue provider

Comments

@stevefan1999-personal
Copy link

Background

Suppose you want to make a simple app that lets you migrate from database to database, and I want to allow the end user to supply either a DSN string or fill in the connection structure, both represented as flags. If you know Java, this is comparable to Spring DB config.

Now, I have problems trying to imitate such relation: it's an either-or relation, i.e. logically mutually exclusive. However, whether the (colloquial) either-or is XOR or IMPLY is still debatable, so I want to discuss with others if they had any more clues here.

Current workarounds

  1. Use priority, so we basically allow both DSN and struct to be filled in, and weight in which one to override which during tiebreaking. Usually DSN takes the precedence.
  2. Error at runtime if both are supplied. This is the intuitive solution that most people including me can figure out instantly. This is also the least effort cobra needs to do. You can even panic if you will because this is an undefined, exceptional case.
  3. Error at cobra init time. This can be tricky because you have to make up your own generic error message and figure out the condition.

Ideal solution

Implement a MarkMutualExclusionForFlagGroups method to let it correctly represent the relation: only one flag group among the "mutual exclusion set" can be activated.

Before that, we also need to implement FlagGroups -- flags that are chained to be required. It should set error if more than one groups are fulfilled. This is the exact solution to this problem but it is a very heavy engineering, i.e. no one will do this.

@jharshman jharshman self-assigned this Sep 24, 2020
@jharshman
Copy link
Collaborator

@stevefan1999-personal, this is an interesting problem but I'm not so sure that there needs to be a bespoke solution for this.
Also as an aside - the project where you would request this feature would be github.com/spf13/pflag.

Have you considered separating your flags into two FlagSets? One being explicitly for mutually exclusive flags? You could then iterate through the flags in the set you're expecting mutual exclusivity in and throw an error if more than one is set.

@BeyondEvil
Copy link

I'm faced with this need as well.

Would you mind elaborating on the FlagSets solution? @jharshman

@github-actions
Copy link

This issue is being marked as stale due to a long period of inactivity

@stevefan1999-personal
Copy link
Author

/stale go away

@h3adache
Copy link

I'm not sure what is meant by flagset
I tried

	var a string
	var b string
	aOrB := pflag.NewFlagSet("aorb", pflag.ContinueOnError)
	aOrB.StringVarP(&a, "a", "a", "", "Set A")
	aOrB.StringVarP(&b, "b", "b", "", "Or Set B")
	aCmd.Flags().AddFlagSet(aOrB)
	if err := aCmd.MarkFlagRequired("aorb"); err != nil {
		log.Err(err).Send()
	}

and I got
{"level":"error","error":"no such flag -aorb","time":"2021-01-15T16:44:30-05:00"}

A FlagSet has the same API methods as a Flag so there's no other way I can tell of how enable this.
Unless your suggestion was to go through it manually to see what is set using cmd.Flags().VisitAll(func(f *pflag.Flag)?

@github-actions
Copy link

This issue is being marked as stale due to a long period of inactivity

@Rhahkeem
Copy link

I need this functionality as well actually. Can anyone expand a bit on the flagsets solution?

@johnSchnake
Copy link
Collaborator

Related to numerous other flag issues including #1238. After using cobra/pflag for so long and looking at the backlog these are one of the most common places for improvement.

@fmenezes
Copy link

doesn't #1654 solves this ?

@eriweb
Copy link

eriweb commented Sep 21, 2022

@fmenezes I've been trying to solve the same issue, and #1654 seems to solve a bit different use case.

By my understanding, what you can currently do is this:

  1. Require multiple flags, if either flag is specified, by using MarkFlagsRequiredTogether.
  2. Require that only one of either flag is specified, if either flag is specified, by using MarkFlagsMutuallyExclusive.

What you cannot, as far as I can see, and what I am trying to do is:
3) Require that one of either flag is set, but not both.

As an example I have this code:

cmd.Flags().StringVar(&message, "message", "", "Specify message as text" )
cmd.Flags().StringVar(&messageFile, "message-file", "", "Read message from a file")

I want to require that either message or message-file is set, but not both.

@fmenezes
Copy link

fmenezes commented Sep 21, 2022

@terbolous I think I understand now, MarkFlagsMutuallyExclusive errors if in a group of flags (flag1, flag2, flag3) we set more than one (flag1, flag3) but does not error when all of them were omitted.

If we follow that logic you're missing a MarkFlagsMutuallyExclusiveAndRequired type of check.

@dyegoe
Copy link

dyegoe commented Oct 6, 2022

+1

@kpramesh2212
Copy link

Yeah, MarkFlagsMutuallyExclusiveAndRequired would be great I am currently stuck without this feature.

Advance thanks to all contributors working this :)

@mih-kopylov
Copy link

mih-kopylov commented Nov 4, 2022

@kpramesh2212 In the same situation I've worked around with PreRunE hook - taking flag values and validating them, returning an error. Works fine, but looks a bit like a kludge

I'm curious if there's a standard way of implementing a custom flag validator that will be found by the Cobra core and will work with flag annotations? I believe, there's a variety of validation use cases where developers need complex validation that definitely will be missed in the Cobra core.

@andrewsingley-glv
Copy link

I also would like to have this behavior. There is no way to require one of a group of flags and this would be excellent if you could add it.

My use case is to configure credentials with the format:

cmd --server=example.com --username=bob --password=ross
or
cmd --server=example.com --delete

This cannot be logically done with cobra's API.

@baruchiro
Copy link

Hi, do you want me to implement the MarkFlagsMutuallyExclusiveAndRequired?

@dominiwe
Copy link

dominiwe commented Jun 5, 2023

I also would like to have this behavior. There is no way to require one of a group of flags and this would be excellent if you could add it.

My use case is to configure credentials with the format:

cmd --server=example.com --username=bob --password=ross or cmd --server=example.com --delete

This cannot be logically done with cobra's API.

I think the question here is whether that is even in the scope of cobra/pflag. Personally I was not sure so I just evaluated this for my flags afterwards. But the fact that MarkFlagsRequiredTogether and MarkFlagsMutuallyExclusive exist makes me think it is intended to be in scope...

@iTrooz
Copy link

iTrooz commented Nov 7, 2023

I think this can be closed due to #1952

@marckhouzam
Copy link
Collaborator

Thanks @iTrooz
So yes, with the new MarkFlagsOneRequired combined with MarkFlagsMutuallyExclusive this issue is addressed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/needs-pr Ready for a PR from the community triage/needs-info Needs more investigation from maintainers or more info from the issue provider
Projects
None yet
Development

Successfully merging a pull request may close this issue.