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

Triage: Exit #945

Closed
shadowspawn opened this issue Apr 1, 2019 · 7 comments
Closed

Triage: Exit #945

shadowspawn opened this issue Apr 1, 2019 · 7 comments
Milestone

Comments

@shadowspawn
Copy link
Collaborator

Commander calls exit under various circumstances, such as after displaying an error or the help. This is pretty convenient for the caller of a one-shot parse in a CLI, but is a problem when Commander is being used as part of a long running program or if caller wants more control over the failure handling.

Issues:

Pull Requests:

This was referenced May 17, 2019
@ngocdaothanh
Copy link

Yes, please simply throw errors. The outer program can catch the errors and handle them properly.

@shadowspawn
Copy link
Collaborator Author

I think being able to opt-in to using a throw makes good sense. But I think it is a bit untidy for default handling of expected errors with the source line number and code? Compared with just a message intended for humans.

@ngocdaothanh
Copy link

a bit untidy for default handling

To be backward compatible, we can add an option to:
Command.prototype.parse = function(argv)

like this:
Command.prototype.parse = function(argv, exitOnError = true)

@shadowspawn
Copy link
Collaborator Author

I am experimenting with a routine to optionally replace calls to process.exit. Since the most likely implementation is to throw, it is passed an Error ready to throw. e.g.

program.exitOverride((err) => { throw err; });

The custom error has properties for exitCode, code, and message. For example, the --version handling creates an error like this:

CommanderError(0, 'commander.version', this._version)

I'm currently just replacing the call to process.exit and not changing where or how the preceding messages are displayed. One thing at a time.

@shadowspawn
Copy link
Collaborator Author

There are improvements on the develop branch for the next release (#1040).

@shadowspawn
Copy link
Collaborator Author

v4.0.0-0 prerelease published: #1067

@shadowspawn
Copy link
Collaborator Author

v4.0.0 has been released. It does not cover all the exit related issues found in triage, but addresses the most common issue of the unconditional call to exit.

Feel free to open a new issue if still issues, with new information and renewed interest.

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

No branches or pull requests

2 participants