-
Notifications
You must be signed in to change notification settings - Fork 3.8k
feat(help): refactor npm help/help-search #2859
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
Conversation
0c9d405 to
bf6c3a8
Compare
|
|
||
| class Access extends BaseCommand { | ||
| static get description () { | ||
| return 'Set access level on published packages' |
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.
These all now should match the descriptions in the docs for all the commands, which means we are one step away from being able to programmatically generate everything before the longhand "Description" sections of those docs.
2e175cf to
7f6195e
Compare
| constructor (npm) { | ||
| this.npm = npm | ||
| const BaseCommand = require('./base-command.js') | ||
| class Docs extends BaseCommand { |
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 added a test to test/lib/load-all-commands.js that caught this bug. Yay.
| /* istanbul ignore next - see test/lib/load-all-commands.js */ | ||
| get usage () { | ||
| return usageUtil('docs', 'npm docs [<pkgname> [<pkgname> ...]]') | ||
| static get usage () { |
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 added a test to test/lib/load-all-commands.js that caught this bug. Yay.
| async helpSearch (args) { | ||
| if (!args.length) | ||
| throw this.usage | ||
| return this.npm.output(this.usage) |
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.
Now that this isn't pulling triple duty it can just work like everything else and return usage on no params
| if (!formatted.trim()) | ||
| npmUsage(this.npm, false) | ||
| else { | ||
| this.npm.output(`No matches in help for: ${args.join(' ')}\n`) |
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.
Now that this isn't the usage output for the cli itself we can act in isolation and not have to return usage here.
lib/help.js
Outdated
| } | ||
| let section = this.npm.deref(args[0]) || args[0] | ||
|
|
||
| let pref = [1, 5, 7] |
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.
The old method would treat the number in the args as a suggestion, there is no need to search everywhere if we are explicitly asking for a given man number. This was the source of most of the streamlining here.
7f6195e to
4dc3de1
Compare
4dc3de1 to
a60c5a8
Compare
a60c5a8 to
41facf6
Compare
Lots of dead code removed thanks to streamlining of logic.
npm helpnpm <command>andnpm help-searchare all now separatedconcerns, handling their own use cases.
helpcallshelp-searchas alast resort, but
npm <command>no longer tries to wind its way throughto
help-searchjust to get the basic npm usage displayed.The
did you meanoutput has been expanded. It now always suggests toplevel commands, scripts, and bins, and suggests them in the way they
should be called.
References
Closes npm/statusboard#277