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

Throw error for Option to option() or requiredOption() #1655

Merged
merged 2 commits into from Dec 21, 2021

Conversation

shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented Dec 20, 2021

Pull Request

Small possible improvement from personal experience. (Fairly specific, might be too special case?)

Problem

I have made the mistake a couple of times of switching to an Option to use extra features, but forgetting to change to .addOption(). It also looks like something which might work.

e.g. start with

program
    .option('-s, --secret`, 'secret option for developers')

change to

program
    .option(new Option('-s, --secret', 'secret option for developers').hideHelp());

and get error

    this.required = flags.includes('<'); // A value must be supplied when the option is specified.
                          ^

TypeError: flags.includes is not a function

Solution

Throw a specific error to help programmer work out the mistake quickly.

      throw new Error('To add an Option object use addOption() instead of option() or requiredOption()');
      ^

Error: To add an Option object use addOption() instead of option() or requiredOption()

Copy link
Collaborator

@abetomo abetomo left a comment

LGTM!

@shadowspawn shadowspawn merged commit 24ca282 into tj:release/9.x Dec 21, 2021
12 checks passed
@shadowspawn shadowspawn deleted the feature/option-error branch Dec 21, 2021
@shadowspawn shadowspawn added the pending release label Dec 21, 2021
@shadowspawn shadowspawn added this to the Commander v9.0.0 milestone Dec 21, 2021
@shadowspawn shadowspawn removed the pending release label Jan 29, 2022
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

Successfully merging this pull request may close these issues.

None yet

2 participants