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
fix: Show message when showHelpOnFail is chained globally #2154
Conversation
lib/usage.ts
Outdated
|
||
// If global context, set globalFailMessage | ||
// Addresses: https://github.com/yargs/yargs/issues/2085 | ||
if (yargs.getInternalMethods().getContext().commands.length === 0) { |
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.
Is there an easier way to figure out that this is in the global context? (I can move this logic to a helper function and call it here)
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 like the idea of pulling this into a private helper method, then if we can think of a better way to detect we're in the global context we can swap out the logic.
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.
One suggested refactor.
lib/usage.ts
Outdated
|
||
// If global context, set globalFailMessage | ||
// Addresses: https://github.com/yargs/yargs/issues/2085 | ||
if (yargs.getInternalMethods().getContext().commands.length === 0) { |
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 like the idea of pulling this into a private helper method, then if we can think of a better way to detect we're in the global context we can swap out the logic.
LGTM, with a suggested refactor. I would be surprised if we're not already detecting the global context, vs., a command handler context somewhere else in the codebase. Perhaps look in |
I realized that checking |
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.
As always, thank you for the contributions.
Addresses: #2085
Problem
When
showHelpOnFail
is used globally, it doesn't work.This is because
usage.reset()
sets failMessage to null.Solution
I added a
globalFailMessage
variable to capture the global mesage.On fail,
failMessage || globalFailMessage
is printed. I figured if both are provided, the more specific message should be shown.Other
I saw that
null | undefined
was used a lot, so I made typenil
. (Idea borrowed from lodash isNil)