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

Allow require and conflicts on the same arguments #2392

Open
atartanian opened this issue Mar 15, 2024 · 5 comments
Open

Allow require and conflicts on the same arguments #2392

atartanian opened this issue Mar 15, 2024 · 5 comments

Comments

@atartanian
Copy link

atartanian commented Mar 15, 2024

Context:

#189
#1294
#1093

Story:

I would like to be able to create two flags on a command, one MUST be included, but both MUST NOT be included.
Put another way, I would like my command to be accepted IFF (flag1, flag2) => XOR(flag1, flag2) === true

Workarounds:

It is possible to enforce this kind of thing with check() but this has a few issues:

  1. this is a feature that naturally should fall into the usage of yargs, without each user having to roll their own solution
  2. the help documentation for the command doesn't accurately communicate how to use said command, forcing the end user to use trial and error to figure out what combination of flags are valid

Example:

This yargs command currently doesn't work:

yargs
.options({
  mutually_exclusive_option_1: {
    require: true,
    conflicts: 'mutually_exclusive_option_2'
  },
  mutually_exclusive_option_2: {
    require: true,
    conflicts: 'mutually_exclusive_option_1'
  },
})

If I call this with:

  • mycommand --mutually_exclusive_option_1
    • I get: --mutually_exclusive_option_2 is required
  • mycommand --mutually_exclusive_option_2
    • I get: --mutually_exclusive_option_1 is required
  • mycommand --mutually_exclusive_option_1 --mutually_exclusive_option_2
    • I get: --mutually_exclusive_option_1 and --mutually_exclusive_option_2 are mutually exclusive

Proposal:

This should allow one, but not both of the options which are mutually exclusive to be passed. Alternatively, if this is deemed too likely to be misunderstood (I can see that more complex cases might be hard to grok), we could allow a new parameter on group allowing us to set a require_amount or just require on the group itself to say that at LEAST one option from the group is required. In conjunction with conflicts, this gives us the tool to create a required, yet mutually exclusive set of options.

In the case that this feature proposal isn't accepted, the parser should at least be updated to throw an error (hopefully with some helpful guidance on how to work around the issue) if a dev tries to create a set of options that are both required and mutually exclusive.

Group Example:

yargs
.options({
  mutually_exclusive_option_1: {
    conflicts: 'mutually_exclusive_option_2'
  },
  mutually_exclusive_option_2: {
    conflicts: 'mutually_exclusive_option_1'
  },
})
.group({
  keys: ['mutually_exclusive_option_1', 'mutually_exclusive_option_2'], //type String | Array
  groupName: 'Name for Mutually Exclusive Options', // type String
  require: true // type boolean | number
  /*  `boolean:true` if you want 1 and only 1 from the group,
      `number` if you want to specify a certain number of these grouped arguments as required
      `number` is probably the most flexible option, but I don't personally have a use case
      this would be important for... so smells like premature optimization
  */
})
@shadowspawn
Copy link
Member

Thanks for the links to previous issues and description.

I did some research last year on what other packages offer, and there are a couple of packages with support for this case.

Oclif has exactlyOne (which is like demandOneOfOption from #1093):

Python argParse has add_mutually_exclusive_group (which is like your group suggestion):

@atartanian
Copy link
Author

atartanian commented Mar 15, 2024

@shadowspawn Ultimately the two you mentioned have the same result (just different APIs for achieving it): you can specify that only 1 of the mutually exclusive options are chosen. Either of these seem clunky given the existing conflicts API, which already provides for mutual exclusivity WITHOUT having to explicitly declare exclusivity groups (I prefer this to the approaches the other libraries take!)

My initial suggestion was actually to consider any options that are BOTH mutually exclusive AND required as if they were a single anonymous option with an alias for each of the different choices. Another way to look at it is, you would consider the required constraint as a mutual constraint over the ENTIRE mutually exclusive set of options. Basically, to consider the required constraint fulfilled if ANY of them is passed in.

This would be more like creating a 'virtual' mutually exclusive group (as is manually created with argParse), but without having to use the existing groups API for it (since they have somewhat different semantics, namely for logically/visually grouping options which may or may not depend on each other).

A benefit of this approach comes from the fact that the CURRENT behavior of setting both conflicts and require produces a broken output (arguably), since there is no set of options you can pass that will allow the command to run! I'd argue this is an uncaught parsing error, which nicely leaves a space in the existing API for this functionality, without needing to extend the API.

That's why I'm suggesting it as the best way to solve the problem, since it shouldn't break anyone's existing code and doesn't add to the complexity of the yargs API (though arguably could be harder to reason about than solutions that involve a more explicit declaration by the user).

All this being said, I'm not a yargs expert, and there very well may be a reason this actually doesn't work, so definitely down to hear your thoughts!

@shadowspawn
Copy link
Member

My initial suggestion was actually to consider any group options that are BOTH mutually exclusive AND required as if they were a single option with an alias for each of the different choices.

I did see that, and it is interesting that it turns a broken combination into something which covers another case. That is clever.

But I don't like it since it is not clear that this particular combination should/could work in this way. As you said: may be "deemed too likely to be misunderstood" and "arguably could be harder to reason about". Yes, that is me, I am concerned.

@atartanian
Copy link
Author

atartanian commented Mar 15, 2024

yeah fair, it does change the semantics of require, but if you set something to required and you also set it to be mutually exclusive, this is the only way that can be resolved short of an "overconstrained" error

anyway, happy to have the discussion! To be clear, I'd be just as happy with one of the other solutions (and the addition of an error check on the "conflict" "require" situation), just thought it was cool that there was space in the existing API for it....

@atartanian
Copy link
Author

atartanian commented Mar 15, 2024

to extend the evaluation of possible cases with intersections between the conflicts and require parameters:

  1. the set of mutually exclusive options are all NOT required - this works great now, no change
  2. the set of mutually exclusive options are ALL required - this is broken now, could be used for the above proposal, but if left should get an error case since it makes the command completely unusable
  3. the set of mutually exclusive options have some number of members required >1 and some not - this is also broken now, and will prevent the command from running at all, this should get an error case
  4. the set of mutually exclusive options have exactly 1 member required and the rest not - this is a degenerate case, where only the required option can ever be picked, but it's not completely broken, so probably should have a warning instead of an error

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

2 participants