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

Combine normalize and parseoptions, and update combined option handling #1152

Merged
merged 10 commits into from
Jan 21, 2020

Conversation

shadowspawn
Copy link
Collaborator

Pull Request

(This is the same PR as #1145, but rebased to clean up the commits listed.)

Problem

.parse() called a private routine .normalize() to split short options, turn --long=value into --long value, et al. This caused a few subtle problems:

  • the extra functionality was not available when called public routine .parseOptions()
  • the expansion was done before the types of the flags for subcommands were known, so support for including value in argument like -fValue (added in #395 fix, short options do not understand adjacent values #599) only worked for top level command
  • an argument into a subcommand option could get incorrectly processed as a global flag due to early expansion, like do sub --flag=--version

Solution

  1. roll the support into .parseOptions and delete .normalise
  2. expand arguments only for known options
  3. as a bonus, add support for multiple boolean short flags followed by a flag expecting an argument (in subcommands as well as top command)

So for example:

program
  .command('sub')
  .option('-w,--www')
  .option('-s,--secure')
  .option('-p,--port <number>')
  .action((cmd) => {
    console.log(cmd.opts());
  });
program.parse(process.argv);

Can now call like:

# my-program sub -wsp443
{ www: true, secure: true, port: '443' }

ChangeLog

Added

Changed

Fixed

Removed

  • private function normalize (the functionality has been integrated into parseOptions)

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.

LGTM!

@shadowspawn
Copy link
Collaborator Author

Thanks @abetomo , merging now.

@shadowspawn shadowspawn merged commit 1691344 into tj:release/5.x Jan 21, 2020
@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Jan 21, 2020
@shadowspawn shadowspawn added this to the v5.0.0 milestone Jan 21, 2020
@shadowspawn shadowspawn deleted the feature/normalize-rework-2 branch February 1, 2020 03:30
@shadowspawn shadowspawn removed the pending release Merged into a branch for a future release, but not released yet label Apr 5, 2020
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.

2 participants