-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Support custom error messages for option argument coercion failures #1392
Support custom error messages for option argument coercion failures #1392
Conversation
Looks great! Exactly what I need. Though maybe a little more low-level than I anticipated. [Edit: refers to older version of code, with this comment leading to changes.] Two thoughts:
function invalidArgument(message) {
return new commander.CommanderError(1, 'commander.optionArgumentRejected', message);
}
// Later, somewhere:
throw invalidArgument('Must be a number.') Do we want a shorter way to do this out of the box? I’m thinking you can’t customize the exit code for other errors, such as “missing option”, right? So maybe we don’t need that ability here. Is there a use case for using any other code than |
Fair comment, and good feedback thanks. I was trying a quick repurpose of the existing internals. 😊 |
Added somewhat higher level wrapper: .option('-p, --port <port>', 'port number', (arg) => {
const portNumber = parseInt(arg);
if (isNaN(portNumber)) {
throw new commander.InvalidOptionArgumentError('Must be a number.');
}
return portNumber;
}) |
Looks good! |
Finished missing pieces, ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate it.
Pull Request
Problem
It can be tricky to do nice error messages for custom option processing failures since the option name is not available.
Issues: #1207 #518 (comment)
Solution
We already added a custom exception to process the error message for the new
.choices()
routine. This takes that a little further to make the exception usable by custom option processing routines and not just Option methods.For example:
Release Note
InvalidOptionArgumentError
from custom option processing for detailed error message.To Do