Skip to content

Conversation

shadowspawn
Copy link
Collaborator

Pull Request

Problem

The number of command-arguments is only checked if there is an action handler. This means no checking for simple programs which fall-through to processing the results of the parsing in following code.

Fixes #1493

Solution

Follow in the footsteps of the refactored tests for options. Both the option and command-argument tests were originally tied to the implementation of the action handler. They have now been separated out to be called where appropriate.

Check for missing required command-arguments, or excess command-arguments if enabled, in programs without an action handler.

ChangeLog

  • the number of command-arguments is now checked for programs without an action handler

@shadowspawn shadowspawn added the semver: major Releasing requires a major version bump, not backwards compatible label Apr 10, 2021
@shadowspawn shadowspawn added this to the v8.0.0 milestone Apr 10, 2021
Copy link
Collaborator

@abetomo abetomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I commented on the naming, but the fix is LGTM.

@shadowspawn shadowspawn merged commit 0504945 into tj:release/8.x Apr 11, 2021
@shadowspawn shadowspawn deleted the feature/check_arguments_without_action_handler_2 branch April 11, 2021 03:26
@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Apr 11, 2021
@shadowspawn
Copy link
Collaborator Author

Note for future readers. Missing command-arguments will now cause an error whether or not there is an action handler. This changes the behaviour for simple single command programs.

program.argument('<file>');
program.parse();
$ pizza
error: missing required argument 'file'

If you want to just display the help if the user does not supply any arguments, you could check before calling parse like:

if (process.argv.length === 2) program.help();
program.parse();

Also, see .showHelpAfterError() to change the default behaviour to include extra help.

@shadowspawn shadowspawn removed the pending release Merged into a branch for a future release, but not released yet label Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: major Releasing requires a major version bump, not backwards compatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants