-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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(usage): clean up usage declarations #2821
Conversation
ac488cb
to
f9fd164
Compare
cedb1e1
to
3018705
Compare
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.
Found a few pretty small bugs. I like where this is going!
'npm access edit [<package>]' | ||
) | ||
/* istanbul ignore next - see test/lib/load-all-commands.js */ | ||
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.
🎉🎉🎉 💖
'--package=<pkg>[@<version>] -- <cmd> [args...]', | ||
'-c \'<cmd> [args...]\'', | ||
'--package=foo -c \'<cmd> [args...]\'', | ||
] |
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.
Dropping the bit about running without --call
or positional args to enter a subshell. Maybe could add something like:
'(without -c or arguments to enter subshell)',
Or tack it onto the description?
Also the npx stuff is missing, but probably that should just be moved to bin/npx-cli.js
anyway.
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.
This is usage, not the man page. This just shows the args you can use. Because it was free-form before it got a little muddied. If I ask for usage on command a, I don't really need to see how to use command b. If we want a "see also" we can talk about that but I really think that belongs in 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.
Having each flag describe what it does is definitely the direction this is moving us towards. For now we are separating our man page from basic usage. Next iteration we can define args instead of calling this "usage"
Usage is something that is built up from components i.e. command name, aliases, command description, args and their descriptions, etc.
56698e5
to
cc2faa4
Compare
@isaacs I tweaked the output to make it a little more nicely for things that don't have a description. I think we should find a "good enough" line here and then bikeshed the next phase. (I still need to make the tests pass for this change on Monday) |
a9b6c58
to
ea57efd
Compare
All output that anything wants to make now goes through `npm.output()`. This is an incremental change getting us closer to where we want to be with testing. PR-URL: #2795 Credit: @wraithgar Close: #2795 Reviewed-by: @ruyadorno, @isaacs
Small refactor of commands to allow usage to be more programmatically generated, leading us in the direction of more tighly coupling each command to the params it accepts. PR-URL: #2821 Credit: @wraithgar Close: #2821 Reviewed-by: @isaacs
Small refactor of commands to allow usage to be more programmatically
generated, leading us in the direction of more tighly coupling each
command to the params it accepts.
Needs #2772 to land first before this is really reviewable.