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

*Bug* When specify none-defined options followed by values don't exit program with error #1039

Closed
snowman opened this issue Sep 8, 2019 · 5 comments
Labels
bug Commander is not working as intended

Comments

@snowman
Copy link

snowman commented Sep 8, 2019

// File: index.js
const program = require('commander');

program
  .version('0.0.1')
  .parse(process.argv);

const args = program.args;

console.log(args);
$ node index.js --bug
error: unknown option '--bug'

$ node index.js --bug 0
error: unknown option '--bug'

$ node index.js --bug 0 1 2 3
[ '1', '2', '3' ]

dependency:

$ cat package.json
{
  "dependencies": {
    "commander": "^3.0.1"
  }
}

Expecting exit code with an error instead of continue of it.

@snowman snowman changed the title *Bug* When specify none-define options followed by option don't complain an error *Bug* When specify none-define options followed by two values don't complain an error Sep 8, 2019
@snowman snowman changed the title *Bug* When specify none-define options followed by two values don't complain an error *Bug* When specify none-define options followed by values don't complain an error Sep 8, 2019
@snowman snowman changed the title *Bug* When specify none-define options followed by values don't complain an error *Bug* When specify none-defined options followed by values don't complain an error Sep 8, 2019
@snowman snowman changed the title *Bug* When specify none-defined options followed by values don't complain an error *Bug* When specify none-defined options followed by values don't exit program with error Sep 8, 2019
@shadowspawn shadowspawn added the bug Commander is not working as intended label Sep 8, 2019
@shadowspawn
Copy link
Collaborator

Reproduced.

shadowspawn added a commit to shadowspawn/commander.js that referenced this issue Sep 9, 2019
@shadowspawn
Copy link
Collaborator

This old PR looks like same issue: #121

@shadowspawn
Copy link
Collaborator

Likely to be same issue as #561

shadowspawn added a commit that referenced this issue Sep 20, 2019
* Prototype Jest as testing framework

* Add comment to direct ports

* Add bool tests, not direct ports

* Add tests for option with required value

* Add jest tests for optional option

* Add tests for options follow by options

* Add multiple short flags test

* Improve test description

* Add single dash test

* Add idea for value tests

* Add old camelcase tests and new flags tests

* Add more testing ideas

* Add testing for custom option processing

* Add extra tests for non-boolean default

* Add placeholder for test file skipped on first pass

* Add comments above describe groups to make more obvious

* Change option names to make easier to understand (more realistic)

* Expand overview comment

* Add tests for the ways values are specified

* Add Jest tests for .opts()

* Add negative number as an interesting dash case

* Add Jest dashdash test for stop processing options

* Add Jest regex tests

* Move dash testing and include full variety of ways values are specified

* Rename spy to mock

* Add Jest .version tests

* Port literal test and subsume dashdash test

* Remove implemented test notes

* Port some of command.name tests

* Add tests when have version command and .version

* Add note that whitebox test

* Add variadic test

* Add experimental _exitOverride to allow throw instead of process.exit

* Rework version tests using experimental exitOverride

* Add empty string test to option values

* Add test for command alias appearing in help

* Add unknownOption support for exitOverride

* Add test for allowUnknownOption

* Call added commands subcommand to clarify

* Add tests for .usage

* Note to tidy up how testing strings for better error messages

* Add first signal test in Jest

* Make new signal listener more predictable

* Add @types/jest

* Add full set of signals to test

* Add Jest --inspect tests

* Add Jest lookup tests

* Add exitOveride support to variadicArgNotLast

* Switch to execFile

* Switch to execFIle (and formally async)

* Fix typos

* Make case consistent in filename

* Add more Jest alias tests

* Add Jest executableSubcommand tests for default and executableFile

* Add Jest subsubcommand test

* Add to Jest alias tests

* Add Jest test for .commandHelp

* Delete a couple of ported tests covered better elsewhere

* Replace more process.exit with exit override

* Add Jest help tests

* Add Jest helpOption tests

* Add Jest noHelp test

* Add Jest asterisk test

* Not porting complex generic command tests

* Jest test for executable with no command

* Add default throw for _exitOverride, and tidy _exit and CommanderError usage

* Add Jest tests for _exitOverride and simplify usage with new default behaviour

* Added commented out tests for process.exit called after spawn

* Fix typo

* Add test on error class, because calling code may rely on it

* Clarify parameter type for exitOverride callback

* Lint

* Add test matching issue #1039

* Add test showing #1032

* Skip open issues normally

* Switch to Jest

- remove sinon and should dependencies
- switch over package script
- delete old tests
- rename folder to tests
- fix paths for executable tests

* Fix error detection on node 8

* Change exitOveride to allow carrying on, and intercept exits from executable subcommand.

* Rework exitOverride for async

- _exit never returns (as before)
- callback handled explicitly in executeSubcommand
- mark _exit as private (copy and paste omission)
- support executeSubCommandAsync i default override handler to prevent call to process.exit, as expected

* Replace listen with listen2 code: lint, and produce output so caller knows ready

* Add explanation in functional empty file

* Lint on pm test files, and remove the listen2 missed in earlier commit

* Add more test for open issues

* Remove testing ideas, not tracking them in file

* Add tests to `npm run lint` and fix errors

* Rename skipped tests for open issues to avoid warning in normal lint
@shadowspawn
Copy link
Collaborator

Closing this as duplicate of #561

@shadowspawn
Copy link
Collaborator

Opened a PR to rework option parsing to avoid dropping options and arguments: #1138

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Commander is not working as intended
Projects
None yet
Development

No branches or pull requests

2 participants