-
-
Notifications
You must be signed in to change notification settings - Fork 603
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: help and version functionality #1972
Conversation
// Use customized help output if available | ||
parser.on('option:help', () => { | ||
// Use customized help output | ||
if (args.includes('--help')) { |
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.
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.
Good catch. It should be removed from there.
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.
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 will update 👍🏻
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.
/cc @webpack/cli-team
@@ -4,11 +4,6 @@ const { run } = require('../utils/test-utils'); | |||
const helpHeader = 'The build tool for modern web applications'; | |||
|
|||
describe('commands help', () => { | |||
it('throws error if supplied as an argument for subcommands', () => { | |||
const { stderr } = run(__dirname, ['serve', 'help'], false); |
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.
Supplying help as an argument to any of the subcommands shouldn't be allowed.
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 think we can allow it for consistency.
webpack --flag help
- Allowed
webpack help --help
- Allowed
webpack help cmd
- Allowed
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.
webpack help --help
interesting, but I think it is good
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.
oh!, I wanted to type webpack help --flag
.
Just checked webpack help --help
is allowed as well and is equivalent to webpack help
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.
/cc @jamesgeorge007
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.
@snitin315 Can we refactor this test to allow using help
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.
Yes.
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.
updated 👍🏻
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.
/cc @webpack/cli-team need review
What kind of change does this PR introduce?
bugfix
Fixes #1917
Did you add tests for your changes?
Yes
If relevant, did you update the documentation?
NA
Summary
Does this PR introduce a breaking change?
No
Other information
No