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

Mark options as required together #1802

Closed
whitlockjc opened this issue Sep 20, 2022 · 6 comments
Closed

Mark options as required together #1802

whitlockjc opened this issue Sep 20, 2022 · 6 comments

Comments

@whitlockjc
Copy link
Contributor

I have a use case where I could like to require my command options to be conditionally required, and exclusive. For example, let's say my testing command has three options:

  • -x, --one <number>
  • -y, --two <number>
  • -z, --three <number>

I'd like to make it where the command either needs -x provided or both -y and -z together. The .conflicts() support makes it easy to ensure that they aren't used together where they shouldn't be but there doesn't seem to be a way to mark -x as required when -y and/or -z isn't used, nor is there a way to mark it such that both -y and -z must be used in combination together when -x isn't used.

I could craft a way to do this myself, and I've searched the project to see if this has been discussed before, but I can't find anything.

@shadowspawn
Copy link
Collaborator

Short version: yes, craft yourself.

Long version

Related: #1358 #1408 #1555

Until recently there was not really any support for options interacting. There still isn't any support for conditional option dependencies (and none planned), but there is now support for implies and conflicts, and error for crafting your own and hooking into the Commander error handling.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Oct 23, 2022

Some routines and proposals in other packages.

Oclif has dependsOn and exclusive and exactlyOne and relationships:

Python argParse has add_argument with required which allows a conditional requirement, and add_mutually_exclusive_group:

Yargs has implies which means one option "implies" the requirement for another:

A description of suggestion for requires and requiresOne in node:util parseArgs():

@shadowspawn
Copy link
Collaborator

This issue has not had any activity in a few months. It isn't likely to get acted on due to this report.

Feel free to open a new issue if it comes up again, with new information and renewed interest.

Thank you for your contributions.

@himat
Copy link

himat commented Feb 16, 2024

Hi, do you have an example of if this is actually possible to do ourselves?

I tried this

program
  .command("run")
  .option("-case, --case <caseFileName>", "Name of the test case file in the cases/ dir")
  .option("-all, --all", "Run all tests in the cases/ dir")
  .requiredOption("-N, --N <numReps>", "Number of times to run the test", parseInt)
  .description("Run a eval test")
  .action((opts) => {
    if (!opts.case && !opts.all) {
      throw new commander.InvalidArgumentError('Must provide a specific test case or use --all');
    }
    runTestcaseMain(opts.case, opts.all, opts.N)
  })

and I want the CLI tool itself to say that one of "--case" or "--all" is required.
The best I've gotten is this where I have to throw an error, but the issue with that is that in the console, you see a stacktrace and the error message, which makes it feel like something has totally broken, rather than just that you needed to pass in one of the args.

(I was also separately assuming that if I threw a commander.InvalidArgumentError that commander would catch that and actually show it as a normal error message in the console, rather than it just letting the error go all the way up. So some way of making this happen could also work.)

So is there a better way to actually make this work right now?

@shadowspawn
Copy link
Collaborator

shadowspawn commented Feb 16, 2024

That was a reasonable thing to try, but Commander only catches InvalidArgumentError thrown from custom option and argument parsers and not in general.

There is support for invoking the Commander error handling to display an error using .error(), intended for cases like this.

  .action((opts, cmd) => {
    if (!opts.case && !opts.all) {
      cmd.error('Must provide a specific test case or use --all');
    }
    runTestcaseMain(opts.case, opts.all, opts.N)
  })

Separately, a short option is a single dash and a single letter. It is optional. So you might use .option("-a, --all") or .option("--all") but .option("-all, --all" is not an intended use.

@himat
Copy link

himat commented Feb 16, 2024

Ah didn't realize that .error exists on cmd since the documentation showed program.error there only
That works, thank you so much!

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

No branches or pull requests

3 participants