Skip to content
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): tie usage to config #2908

Merged
merged 1 commit into from
Mar 22, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions docs/content/using-npm/config.md
Original file line number Diff line number Diff line change
@@ -96,6 +96,8 @@ The following shorthands are parsed on the command-line:
* `-H`: `--usage`
* `--help`: `--usage`
* `-v`: `--version`
* `-w`: `--workspace`
* `--ws`: `--workspaces`
* `-y`: `--yes`

<!-- AUTOGENERATED CONFIG SHORTHANDS END -->
@@ -1311,6 +1313,34 @@ The program to use to view help content.

Set to `"browser"` to view html help content in the default web browser.

#### `which`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which was not defined yet, and I re-ran the make to generate this file. We should think about automating this make command

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's included in the make docs that's part of the release process as a final safety net, but some easier way to run it ahead of time would be nice, too, I agree.


* Default: null
* Type: null or Number

If there are multiple funding sources, which 1-indexed source URL to open.

#### `workspace`

* Default:
* Type: String (can be set multiple times)

Enable running a command in the context of the configured workspaces of the
current project while filtering by running only the workspaces defined by
this configuration option.

Valid values for the `workspace` config are either: - Workspace names - Path
to a workspace directory - Path to a parent workspace directory (will result
to selecting all of the nested workspaces)

#### `workspaces`

* Default: false
* Type: Boolean

Enable running a command in the context of **all** the configured
workspaces.

#### `yes`

* Default: false
8 changes: 6 additions & 2 deletions lib/adduser.js
Original file line number Diff line number Diff line change
@@ -17,8 +17,12 @@ class AddUser extends BaseCommand {
return 'adduser'
}

static get usage () {
return ['[--registry=url] [--scope=@orgname] [--always-auth]']
static get params () {
return [
'registry',
'scope',
'always-auth',
]
}

exec (args, cb) {
14 changes: 11 additions & 3 deletions lib/audit.js
Original file line number Diff line number Diff line change
@@ -16,13 +16,21 @@ class Audit extends BaseCommand {
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get usage () {
static get params () {
return [
'[--json] [--production]',
'fix [--force|--package-lock-only|--dry-run|--production|--only=(dev|prod)]',
'dry-run',
'force',
'json',
'package-lock-only',
'production',
]
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get usage () {
return ['[fix]']
}

async completion (opts) {
const argv = opts.conf.argv.remain

4 changes: 4 additions & 0 deletions lib/base-command.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Base class for npm.commands[cmd]
const usageUtil = require('./utils/usage.js')
const ConfigDefinitions = require('./utils/config/definitions.js')

class BaseCommand {
constructor (npm) {
@@ -25,6 +26,9 @@ class BaseCommand {
else
usage = `${usage}${this.constructor.usage.map(u => `npm ${this.constructor.name} ${u}`).join('\n')}`

if (this.constructor.params)
usage = `${usage}\n\nOptions:\n[${this.constructor.params.map(p => ConfigDefinitions[p].usage).join('] [')}]`

// Mostly this just appends aliases, this could be more clear
usage = usageUtil(this.constructor.name, usage)
usage = `${usage}\n\nRun "npm help ${this.constructor.name}" for more info`
4 changes: 2 additions & 2 deletions lib/bin.js
Original file line number Diff line number Diff line change
@@ -10,8 +10,8 @@ class Bin extends BaseCommand {
return 'bin'
}

static get usage () {
return ['[-g]']
static get params () {
return ['global']
}

exec (args, cb) {
12 changes: 11 additions & 1 deletion lib/fund.js
Original file line number Diff line number Diff line change
@@ -32,9 +32,19 @@ class Fund extends BaseCommand {
return 'fund'
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get params () {
return [
'json',
'browser',
'unicode',
'which',
]
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get usage () {
return ['[--json] [--browser] [--unicode] [[<@scope>/]<pkg> [--which=<fundingSourceNumber>]']
return ['[[<@scope>/]<pkg>]']
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
10 changes: 9 additions & 1 deletion lib/install.js
Original file line number Diff line number Diff line change
@@ -21,6 +21,14 @@ class Install extends BaseCommand {
return 'install'
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get params () {
return [
'save',
'save-exact',
]
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get usage () {
return [
@@ -33,7 +41,7 @@ class Install extends BaseCommand {
'<tarball file>',
'<tarball url>',
'<git:// url>',
'<github username>/<github project> [--save-prod|--save-dev|--save-optional|--save-peer] [--save-exact] [--no-save]',
'<github username>/<github project>',
]
}

7 changes: 5 additions & 2 deletions lib/logout.js
Original file line number Diff line number Diff line change
@@ -15,8 +15,11 @@ class Logout extends BaseCommand {
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get usage () {
return ['[--registry=<url>] [--scope=<@scope>]']
static get params () {
return [
'registry',
'scope',
]
}

exec (args, cb) {
7 changes: 6 additions & 1 deletion lib/pack.js
Original file line number Diff line number Diff line change
@@ -21,9 +21,14 @@ class Pack extends BaseCommand {
return 'pack'
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get params () {
return ['dry-run']
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get usage () {
return ['[[<@scope>/]<pkg>...] [--dry-run]']
return ['[[<@scope>/]<pkg>...]']
}

exec (args, cb) {
5 changes: 5 additions & 0 deletions lib/ping.js
Original file line number Diff line number Diff line change
@@ -8,6 +8,11 @@ class Ping extends BaseCommand {
return 'Ping npm registry'
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get params () {
return ['registry']
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get name () {
return 'ping'
7 changes: 6 additions & 1 deletion lib/prune.js
Original file line number Diff line number Diff line change
@@ -14,9 +14,14 @@ class Prune extends BaseCommand {
return 'prune'
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get params () {
return ['production']
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get usage () {
return ['[[<@scope>/]<pkg>...] [--production]']
return ['[[<@scope>/]<pkg>...]']
}

exec (args, cb) {
7 changes: 6 additions & 1 deletion lib/publish.js
Original file line number Diff line number Diff line change
@@ -28,10 +28,15 @@ class Publish extends BaseCommand {
return 'publish'
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get params () {
return ['tag', 'access', 'dry-run']
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get usage () {
return [
'[<folder>] [--tag <tag>] [--access <public|restricted>] [--dry-run]',
'[<folder>]',
]
}

4 changes: 2 additions & 2 deletions lib/root.js
Original file line number Diff line number Diff line change
@@ -11,8 +11,8 @@ class Root extends BaseCommand {
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get usage () {
return ['[-g]']
static get params () {
return ['global']
}

exec (args, cb) {
12 changes: 11 additions & 1 deletion lib/search.js
Original file line number Diff line number Diff line change
@@ -36,9 +36,19 @@ class Search extends BaseCommand {
return 'search'
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get params () {
return [
'long',
'json',
'parseable',
'description',
]
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get usage () {
return ['[-l|--long] [--json] [--parseable] [--no-description] [search terms ...]']
return ['[search terms ...]']
}

exec (args, cb) {
7 changes: 6 additions & 1 deletion lib/uninstall.js
Original file line number Diff line number Diff line change
@@ -16,9 +16,14 @@ class Uninstall extends BaseCommand {
return 'uninstall'
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get params () {
return ['save']
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get usage () {
return ['[<@scope>/]<pkg>[@<version>]... [-S|--save|--no-save]']
return ['[<@scope>/]<pkg>...']
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
7 changes: 6 additions & 1 deletion lib/update.js
Original file line number Diff line number Diff line change
@@ -18,9 +18,14 @@ class Update extends BaseCommand {
return 'update'
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get params () {
return ['global']
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get usage () {
return ['[-g] [<pkg>...]']
return ['[<pkg>...]']
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
36 changes: 32 additions & 4 deletions lib/utils/config/definition.js
Original file line number Diff line number Diff line change
@@ -15,14 +15,16 @@ const required = [

const allowed = [
'default',
'type',
'defaultDescription',
'deprecated',
'description',
'flatten',
'hint',
'key',
'short',
'type',
'typeDescription',
'defaultDescription',
'deprecated',
'key',
'usage',
]

const {
@@ -43,6 +45,10 @@ class Definition {
this.defaultDescription = describeValue(this.default)
if (!this.typeDescription)
this.typeDescription = describeType(this.type)
if (!this.hint)
this.hint = `<${this.key}>`
if (!this.usage)
this.usage = describeUsage(this)
}

validate () {
@@ -73,6 +79,28 @@ ${description}
}
}

// Usage for a single param, abstracted because we have arrays of types in
// config definition
const paramUsage = (type, def) => {
let key = `--${def.key}`
if (def.short && typeof def.short === 'string')
key = `-${def.short}|${key}`
if (type === Boolean)
return `${key}`
else
return `${key} ${def.hint}`
}

const describeUsage = (def) => {
if (Array.isArray(def.type)) {
if (!def.type.some(d => d !== null && typeof d !== 'string'))
return `--${def.key} <${def.type.filter(d => d).join('|')}>`
else
return def.type.filter(d => d).map((t) => paramUsage(t, def)).join('|')
}
return paramUsage(def.type, def)
}

const describeType = type => {
if (Array.isArray(type)) {
const descriptions = type
13 changes: 13 additions & 0 deletions lib/utils/config/definitions.js
Original file line number Diff line number Diff line change
@@ -467,6 +467,7 @@ define('depth', {
define('description', {
default: true,
type: Boolean,
usage: '--no-description',
description: `
Show the description in \`npm search\`
`,
@@ -1287,6 +1288,7 @@ define('otp', {

define('package', {
default: [],
hint: '<pkg>[@<version>]',
type: [String, Array],
description: `
The package to install for [\`npm exec\`](/commands/npm-exec)
@@ -1448,6 +1450,7 @@ define('registry', {

define('save', {
default: true,
usage: '-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer',
type: Boolean,
short: 'S',
description: `
@@ -1611,6 +1614,7 @@ define('scope', {
the scope of the current project, if any, or ""
`,
type: String,
hint: '<@scope>',
description: `
Associate an operation with a scope for a scoped registry.
@@ -2003,6 +2007,15 @@ define('viewer', {
`,
})

define('which', {
default: null,
hint: '<fundingSourceNumber>',
type: [null, Number],
description: `
If there are multiple funding sources, which 1-indexed source URL to open.
`,
})

define('workspace', {
default: [],
type: [String, Array],
4 changes: 2 additions & 2 deletions lib/whoami.js
Original file line number Diff line number Diff line change
@@ -13,8 +13,8 @@ class Whoami extends BaseCommand {
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get usage () {
return ['[--registry <registry>]']
static get params () {
return ['registry']
}

exec (args, cb) {
5 changes: 4 additions & 1 deletion tap-snapshots/test-lib-publish.js-TAP.test.js
Original file line number Diff line number Diff line change
@@ -11,7 +11,10 @@ npm publish
Publish a package
Usage:
npm publish [<folder>] [--tag <tag>] [--access <public|restricted>] [--dry-run]
npm publish [<folder>]
Options:
[--tag <tag>] [--access <restricted|public>] [--dry-run]
Run "npm help publish" for more info
`
Original file line number Diff line number Diff line change
@@ -144,6 +144,7 @@ Array [
"version",
"versions",
"viewer",
"which",
"workspace",
"workspaces",
"yes",
Original file line number Diff line number Diff line change
@@ -1192,6 +1192,13 @@ The program to use to view help content.
Set to \`"browser"\` to view html help content in the default web browser.
#### \`which\`
* Default: null
* Type: null or Number
If there are multiple funding sources, which 1-indexed source URL to open.
#### \`workspace\`
* Default:
84 changes: 67 additions & 17 deletions tap-snapshots/test-lib-utils-npm-usage.js-TAP.test.js
Original file line number Diff line number Diff line change
@@ -187,7 +187,10 @@ All commands:
Add a registry user account
Usage:
npm adduser [--registry=url] [--scope=@orgname] [--always-auth]
npm adduser
Options:
[--registry <registry>] [--scope <@scope>] [--always-auth]
aliases: login, add-user
@@ -198,8 +201,10 @@ All commands:
Run a security audit
Usage:
npm audit [--json] [--production]
npm audit fix [--force|--package-lock-only|--dry-run|--production|--only=(dev|prod)]
npm audit [fix]
Options:
[--dry-run] [-f|--force] [--json] [--package-lock-only] [--production]
Run "npm help audit" for more info
@@ -208,7 +213,10 @@ All commands:
Display npm bin folder
Usage:
npm bin [-g]
npm bin
Options:
[-g|--global]
Run "npm help bin" for more info
@@ -396,7 +404,10 @@ All commands:
Retrieve funding information
Usage:
npm fund [--json] [--browser] [--unicode] [[<@scope>/]<pkg> [--which=<fundingSourceNumber>]
npm fund [[<@scope>/]<pkg>]
Options:
[--json] [--browser|--browser <browser>] [--unicode] [--which <fundingSourceNumber>]
Run "npm help fund" for more info
@@ -459,7 +470,10 @@ All commands:
npm install <tarball file>
npm install <tarball url>
npm install <git:// url>
npm install <github username>/<github project> [--save-prod|--save-dev|--save-optional|--save-peer] [--save-exact] [--no-save]
npm install <github username>/<github project>
Options:
[-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer] [-E|--save-exact]
aliases: i, in, ins, inst, insta, instal, isnt, isnta, isntal, add
@@ -490,7 +504,10 @@ All commands:
npm install-test <tarball file>
npm install-test <tarball url>
npm install-test <git:// url>
npm install-test <github username>/<github project> [--save-prod|--save-dev|--save-optional|--save-peer] [--save-exact] [--no-save]
npm install-test <github username>/<github project>
Options:
[-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer] [-E|--save-exact]
alias: it
@@ -524,7 +541,10 @@ All commands:
Add a registry user account
Usage:
npm adduser [--registry=url] [--scope=@orgname] [--always-auth]
npm adduser
Options:
[--registry <registry>] [--scope <@scope>] [--always-auth]
aliases: login, add-user
@@ -535,7 +555,10 @@ All commands:
Log out of the registry
Usage:
npm logout [--registry=<url>] [--scope=<@scope>]
npm logout
Options:
[--registry <registry>] [--scope <@scope>]
Run "npm help logout" for more info
@@ -590,7 +613,10 @@ All commands:
Create a tarball from a package
Usage:
npm pack [[<@scope>/]<pkg>...] [--dry-run]
npm pack [[<@scope>/]<pkg>...]
Options:
[--dry-run]
Run "npm help pack" for more info
@@ -601,6 +627,9 @@ All commands:
Usage:
npm ping
Options:
[--registry <registry>]
Run "npm help ping" for more info
prefix npm prefix
@@ -629,7 +658,10 @@ All commands:
Remove extraneous packages
Usage:
npm prune [[<@scope>/]<pkg>...] [--production]
npm prune [[<@scope>/]<pkg>...]
Options:
[--production]
Run "npm help prune" for more info
@@ -638,7 +670,10 @@ All commands:
Publish a package
Usage:
npm publish [<folder>] [--tag <tag>] [--access <public|restricted>] [--dry-run]
npm publish [<folder>]
Options:
[--tag <tag>] [--access <restricted|public>] [--dry-run]
Run "npm help publish" for more info
@@ -676,7 +711,10 @@ All commands:
Display npm root
Usage:
npm root [-g]
npm root
Options:
[-g|--global]
Run "npm help root" for more info
@@ -696,7 +734,10 @@ All commands:
Search for pacakges
Usage:
npm search [-l|--long] [--json] [--parseable] [--no-description] [search terms ...]
npm search [search terms ...]
Options:
[-l|--long] [--json] [-p|--parseable] [--no-description]
aliases: s, se, find
@@ -805,7 +846,10 @@ All commands:
Remove a package
Usage:
npm uninstall [<@scope>/]<pkg>[@<version>]... [-S|--save|--no-save]
npm uninstall [<@scope>/]<pkg>...
Options:
[-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer]
aliases: un, unlink, remove, rm, r
@@ -834,7 +878,10 @@ All commands:
Update packages
Usage:
npm update [-g] [<pkg>...]
npm update [<pkg>...]
Options:
[-g|--global]
aliases: up, upgrade, udpate
@@ -867,7 +914,10 @@ All commands:
Display npm username
Usage:
npm whoami [--registry <registry>]
npm whoami
Options:
[--registry <registry>]
Run "npm help whoami" for more info
6 changes: 4 additions & 2 deletions test/lib/load-all-commands.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Thanks to nyc not working properly with proxies this doesn't affect
// coverage. but it does ensure that every command has a usage that contains
// its name, a description, and if it has completion it is a function
// coverage. but it does ensure that every command has a usage that renders,
// contains its name, a description, and if it has completion it is a function.
// That it renders also ensures that any params we've defined in our commands
// work.
const requireInject = require('require-inject')
const npm = requireInject('../../lib/npm.js')
const t = require('tap')
5 changes: 5 additions & 0 deletions test/lib/set-script.js
Original file line number Diff line number Diff line change
@@ -31,6 +31,7 @@ test.test('fails when package.json not found', (t) => {
})
test.test('fails on invalid JSON', (t) => {
const SetScript = requireInject('../../lib/set-script.js', {
'../../lib/utils/config/definitions.js': {},
fs: {
readFile: () => {}, // read-package-json-fast explodes w/o this
readFileSync: (name, charcode) => {
@@ -45,6 +46,7 @@ test.test('fails on invalid JSON', (t) => {
test.test('creates scripts object', (t) => {
var mockFile = ''
const SetScript = requireInject('../../lib/set-script.js', {
'../../lib/utils/config/definitions.js': {},
fs: {
readFileSync: (name, charcode) => {
return '{}'
@@ -70,6 +72,7 @@ test.test('creates scripts object', (t) => {
test.test('warns before overwriting', (t) => {
var warningListened = ''
const SetScript = requireInject('../../lib/set-script.js', {
'../../lib/utils/config/definitions.js': {},
fs: {
readFileSync: (name, charcode) => {
return JSON.stringify({
@@ -102,6 +105,7 @@ test.test('warns before overwriting', (t) => {
test.test('provided indentation and eol is used', (t) => {
var mockFile = ''
const SetScript = requireInject('../../lib/set-script.js', {
'../../lib/utils/config/definitions.js': {},
fs: {
readFileSync: (name, charcode) => {
return '{}'
@@ -128,6 +132,7 @@ test.test('provided indentation and eol is used', (t) => {
test.test('goes to default when undefined indent and eol provided', (t) => {
var mockFile = ''
const SetScript = requireInject('../../lib/set-script.js', {
'../../lib/utils/config/definitions.js': {},
fs: {
readFileSync: (name, charcode) => {
return '{}'
1 change: 1 addition & 0 deletions test/lib/shrinkwrap.js
Original file line number Diff line number Diff line change
@@ -37,6 +37,7 @@ const mocks = {
}
},
'../../lib/utils/usage.js': () => 'usage instructions',
'../../lib/utils/config/definitions.js': {},
}

t.afterEach(cb => {
35 changes: 35 additions & 0 deletions test/lib/utils/config/definition.js
Original file line number Diff line number Diff line change
@@ -21,6 +21,8 @@ t.test('basic definition', async t => {
default: 'some default value',
defaultDescription: '"some default value"',
type: [Number, String],
hint: '<key>',
usage: '--key <key>|--key <key>',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can address this once we add a config item that has this kind of type to a command's params.

typeDescription: 'Number or String',
description: 'just a test thingie',
})
@@ -78,6 +80,39 @@ t.test('basic definition', async t => {
description: 'asdf',
})
t.equal(multi123Semver.typeDescription, '1, 2, 3, or SemVer string (can be set multiple times)')
const hasUsage = new Definition('key', {
default: 'test default',
type: String,
description: 'test description',
usage: 'test usage',
})
t.equal(hasUsage.usage, 'test usage')
const hasShort = new Definition('key', {
default: 'test default',
short: 't',
type: String,
description: 'test description',
})
t.equal(hasShort.usage, '-t|--key <key>')
const hardCodedTypes = new Definition('key', {
default: 'test default',
type: ['string1', 'string2'],
description: 'test description',
})
t.equal(hardCodedTypes.usage, '--key <string1|string2>')
const hardCodedOptionalTypes = new Definition('key', {
default: 'test default',
type: [null, 'string1', 'string2'],
description: 'test description',
})
t.equal(hardCodedOptionalTypes.usage, '--key <string1|string2>')
const hasHint = new Definition('key', {
default: 'test default',
type: String,
description: 'test description',
hint: '<testparam>',
})
t.equal(hasHint.usage, '--key <testparam>')
})

t.test('missing fields', async t => {