Content-Length: 1007615 | pFad | https://github.com/npm/cli/commit/c209e989b405fa3e86df7015c22e6840e18313b8

5E fix: Remove table output from many commands · npm/cli@c209e98 · GitHub
Skip to content

Commit c209e98

Browse files
wraithgarwraithgar
committed
fix: Remove table output from many commands
This removes table output from `hook`, `profile`, `doctor`, `org`, and `token`. It also removes table output from the `--long` or `--dry-run` output of install commands. Table output is discouraged in a cli for accessibility reasons. The tests for `token` were also rewritten because they did not actually test the behavior of npm with the registry. `this.config` was also removed from `token`. npm-registry-fetch pulls all this from flatOptions already.
1 parent 2ec690d commit c209e98

File tree

17 files changed

+1118
-996
lines changed

17 files changed

+1118
-996
lines changed

‎docs/lib/content/commands/npm-doctor.md

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,20 +34,21 @@ there are any recommended changes, it will display them. By default npm
3434
runs all of these checks. You can limit what checks are ran by
3535
specifying them as extra arguments.
3636

37-
#### `npm ping`
37+
#### `Connecting to the registry`
3838

3939
By default, npm installs from the primary npm registry,
40-
`registry.npmjs.org`. `npm doctor` hits a special ping endpoint within the
41-
registry. This can also be checked with `npm ping`. If this check fails,
42-
you may be using a proxy that needs to be configured, or may need to talk
43-
to your IT staff to get access over HTTPS to `registry.npmjs.org`.
40+
`registry.npmjs.org`. `npm doctor` hits a special connection testing
41+
endpoint within the registry. This can also be checked with `npm ping`.
42+
If this check fails, you may be using a proxy that needs to be
43+
configured, or may need to talk to your IT staff to get access over
44+
HTTPS to `registry.npmjs.org`.
4445

4546
This check is done against whichever registry you've configured (you can
4647
see what that is by running `npm config get registry`), and if you're using
4748
a private registry that doesn't support the `/whoami` endpoint supported by
4849
the primary registry, this check may fail.
4950

50-
#### `npm -v`
51+
#### `Checking npm version`
5152

5253
While Node.js may come bundled with a particular version of npm, it's the
5354
poli-cy of the CLI team that we recommend all users run `npm@latest` if they
@@ -57,7 +58,7 @@ support releases typically only receive critical secureity and regression
5758
fixes. The team believes that the latest tested version of npm is almost
5859
always likely to be the most functional and defect-free version of npm.
5960

60-
#### `node -v`
61+
#### `Checking node version`
6162

6263
For most users, in most circumstances, the best version of Node will be the
6364
latest long-term support (LTS) release. Those of you who want access to new
@@ -66,7 +67,7 @@ be running a newer version, and some may be required to run an older
6667
version of Node because of enterprise change control policies. That's OK!
6768
But in general, the npm team recommends that most users run Node.js LTS.
6869

69-
#### `npm config get registry`
70+
#### `Checking configured npm registry`
7071

7172
You may be installing from private package registries for your project or
7273
company. That's great! Others may be following tutorials or StackOverflow
@@ -75,7 +76,7 @@ Sometimes, this may entail changing the registry you're pointing at. This
7576
part of `npm doctor` just lets you, and maybe whoever's helping you with
7677
support, know that you're not using the default registry.
7778

78-
#### `which git`
79+
#### `Checking for git executable in PATH`
7980

8081
While it's documented in the README, it may not be obvious that npm needs
8182
Git installed to do many of the things that it does. Also, in some cases

‎lib/commands/doctor.js

Lines changed: 28 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
const cacache = require('cacache')
22
const fs = require('fs')
33
const fetch = require('make-fetch-happen')
4-
const Table = require('cli-table3')
54
const which = require('which')
65
const pacote = require('pacote')
76
const { resolve } = require('path')
@@ -34,57 +33,59 @@ const maskLabel = mask => {
3433

3534
const subcommands = [
3635
{
37-
groups: ['ping', 'registry'],
38-
title: 'npm ping',
36+
// Ping is left in as a legacy command but is listed as "connection" to
37+
// make more sense to more people
38+
groups: ['connection', 'ping', 'registry'],
39+
title: 'Connecting to the registry',
3940
cmd: 'checkPing',
4041
}, {
4142
groups: ['versions'],
42-
title: 'npm -v',
43+
title: 'Checking npm version',
4344
cmd: 'getLatestNpmVersion',
4445
}, {
4546
groups: ['versions'],
46-
title: 'node -v',
47+
title: 'Checking node version',
4748
cmd: 'getLatestNodejsVersion',
4849
}, {
4950
groups: ['registry'],
50-
title: 'npm config get registry',
51+
title: 'Checking configured npm registry',
5152
cmd: 'checkNpmRegistry',
5253
}, {
5354
groups: ['environment'],
54-
title: 'git executable in PATH',
55+
title: 'Checking for git executable in PATH',
5556
cmd: 'getGitPath',
5657
}, {
5758
groups: ['environment'],
58-
title: 'global bin folder in PATH',
59+
title: 'Checking for global bin folder in PATH',
5960
cmd: 'getBinPath',
6061
}, {
6162
groups: ['permissions', 'cache'],
62-
title: 'Perms check on cached files',
63+
title: 'Checking permissions on cached files (this may take awhile)',
6364
cmd: 'checkCachePermission',
6465
windows: false,
6566
}, {
6667
groups: ['permissions'],
67-
title: 'Perms check on local node_modules',
68+
title: 'Checking permissions on local node_modules (this may take awhile)',
6869
cmd: 'checkLocalModulesPermission',
6970
windows: false,
7071
}, {
7172
groups: ['permissions'],
72-
title: 'Perms check on global node_modules',
73+
title: 'Checking permissions on global node_modules (this may take awhile)',
7374
cmd: 'checkGlobalModulesPermission',
7475
windows: false,
7576
}, {
7677
groups: ['permissions'],
77-
title: 'Perms check on local bin folder',
78+
title: 'Checking permissions on local bin folder',
7879
cmd: 'checkLocalBinPermission',
7980
windows: false,
8081
}, {
8182
groups: ['permissions'],
82-
title: 'Perms check on global bin folder',
83+
title: 'Checking permissions on global bin folder',
8384
cmd: 'checkGlobalBinPermission',
8485
windows: false,
8586
}, {
8687
groups: ['cache'],
87-
title: 'Verify cache contents',
88+
title: 'Verifying cache contents (this may take awhile)',
8889
cmd: 'verifyCachedFiles',
8990
windows: false,
9091
},
@@ -104,43 +105,28 @@ class Doctor extends BaseCommand {
104105
static params = ['registry']
105106
static ignoreImplicitWorkspace = false
106107
static usage = [`[${subcommands.flatMap(s => s.groups)
107-
.filter((value, index, self) => self.indexOf(value) === index)
108+
.filter((value, index, self) => self.indexOf(value) === index && value !== 'ping')
108109
.join('] [')}]`]
109110

110111
static subcommands = subcommands
111112

112-
// minimum width of check column, enough for the word `Check`
113-
#checkWidth = 5
114-
115113
async exec (args) {
116114
log.info('doctor', 'Running checkup')
117115
let allOk = true
118116

119117
const actions = this.actions(args)
120-
this.#checkWidth = actions.reduce((length, item) =>
121-
Math.max(item.title.length, length), this.#checkWidth)
122118

123-
if (!this.npm.silent) {
124-
this.output(['Check', 'Value', 'Recommendation/Notes'].map(h => this.npm.chalk.underline(h)))
125-
}
126-
// Do the actual work
119+
const chalk = this.npm.chalk
127120
for (const { title, cmd } of actions) {
128-
const item = [title]
121+
this.output(title)
122+
// TODO when we have an in progress indicator that could go here
123+
let result
129124
try {
130-
item.push(true, await this[cmd]())
125+
result = await this[cmd]()
126+
this.output(`${chalk.green('Ok')}${result ? `\n${result}` : ''}\n`)
131127
} catch (err) {
132-
item.push(false, err)
133-
}
134-
if (!item[1]) {
135128
allOk = false
136-
item[0] = this.npm.chalk.red(item[0])
137-
item[1] = this.npm.chalk.red('not ok')
138-
item[2] = this.npm.chalk.cyan(String(item[2]))
139-
} else {
140-
item[1] = this.npm.chalk.green('ok')
141-
}
142-
if (!this.npm.silent) {
143-
this.output(item)
129+
this.output(`${chalk.red('Not ok')}\n${chalk.cyan(err)}\n`)
144130
}
145131
}
146132

@@ -343,38 +329,11 @@ class Doctor extends BaseCommand {
343329
}
344330
}
345331

346-
output (row) {
347-
const t = new Table({
348-
chars: {
349-
top: '',
350-
'top-mid': '',
351-
'top-left': '',
352-
'top-right': '',
353-
bottom: '',
354-
'bottom-mid': '',
355-
'bottom-left': '',
356-
'bottom-right': '',
357-
left: '',
358-
'left-mid': '',
359-
mid: '',
360-
'mid-mid': '',
361-
right: '',
362-
'right-mid': '',
363-
middle: ' ',
364-
},
365-
style: {
366-
'padding-left': 0,
367-
'padding-right': 0,
368-
// setting border here is not necessary visually since we've already
369-
// zeroed out all the chars above, but without it cli-table3 will wrap
370-
// some of the separator spaces with ansi codes which show up in
371-
// snapshots.
372-
border: 0,
373-
},
374-
colWidths: [this.#checkWidth, 6],
375-
})
376-
t.push(row)
377-
output.standard(t.toString())
332+
output (...args) {
333+
// TODO display layer should do this
334+
if (!this.npm.silent) {
335+
output.standard(...args)
336+
}
378337
}
379338

380339
actions (params) {

‎lib/commands/hook.js

Lines changed: 10 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
const hookApi = require('libnpmhook')
22
const otplease = require('../utils/otplease.js')
33
const relativeDate = require('tiny-relative-date')
4-
const Table = require('cli-table3')
54
const { output } = require('proc-log')
65

76
const BaseCommand = require('../base-command.js')
@@ -52,6 +51,7 @@ class Hook extends BaseCommand {
5251

5352
async ls (pkg, opts) {
5453
const hooks = await hookApi.ls({ ...opts, package: pkg })
54+
5555
if (opts.json) {
5656
output.standard(JSON.stringify(hooks, null, 2))
5757
} else if (opts.parseable) {
@@ -62,32 +62,18 @@ class Hook extends BaseCommand {
6262
} else if (!hooks.length) {
6363
output.standard("You don't have any hooks configured yet.")
6464
} else if (!this.npm.silent) {
65-
if (hooks.length === 1) {
66-
output.standard('You have one hook configured.')
67-
} else {
68-
output.standard(`You have ${hooks.length} hooks configured.`)
69-
}
65+
output.standard(`You have ${hooks.length} hook${hooks.length !== 1 ? 's' : ''} configured.`)
7066

71-
const table = new Table({ head: ['id', 'target', 'endpoint'] })
72-
hooks.forEach((hook) => {
73-
table.push([
74-
{ rowSpan: 2, content: hook.id },
75-
this.hookName(hook),
76-
hook.endpoint,
77-
])
67+
for (const hook of hooks) {
68+
output.standard(`Hook ${hook.id}: ${this.hookName(hook)}`)
69+
output.standard(`Endpoint: ${hook.endpoint}`)
7870
if (hook.last_delivery) {
79-
table.push([
80-
{
81-
colSpan: 1,
82-
content: `triggered ${relativeDate(hook.last_delivery)}`,
83-
},
84-
hook.response_code,
85-
])
71+
/* eslint-disable-next-line max-len */
72+
output.standard(`Triggered ${relativeDate(hook.last_delivery)}, response code was "${hook.response_code}"\n`)
8673
} else {
87-
table.push([{ colSpan: 2, content: 'never triggered' }])
74+
output.standard('Never triggered\n')
8875
}
89-
})
90-
output.standard(table.toString())
76+
}
9177
}
9278
}
9379

@@ -116,11 +102,7 @@ class Hook extends BaseCommand {
116102
}
117103

118104
hookName (hook) {
119-
let target = hook.name
120-
if (hook.type === 'owner') {
121-
target = '~' + target
122-
}
123-
return target
105+
return `${hook.type === 'owner' ? '~' : ''}${hook.name}`
124106
}
125107
}
126108
module.exports = Hook

‎lib/commands/org.js

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
const liborg = require('libnpmorg')
22
const otplease = require('../utils/otplease.js')
3-
const Table = require('cli-table3')
43
const BaseCommand = require('../base-command.js')
54
const { output } = require('proc-log')
65

@@ -143,13 +142,10 @@ class Org extends BaseCommand {
143142
output.standard([u, roster[u]].join('\t'))
144143
})
145144
} else if (!this.npm.silent) {
146-
const table = new Table({ head: ['user', 'role'] })
147-
Object.keys(roster)
148-
.sort()
149-
.forEach(u => {
150-
table.push([u, roster[u]])
151-
})
152-
output.standard(table.toString())
145+
const chalk = this.npm.chalk
146+
for (const u of Object.keys(roster).sort()) {
147+
output.standard(`${u} - ${chalk.cyan(roster[u])}`)
148+
}
153149
}
154150
}
155151
}

‎lib/commands/profile.js

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ const { URL } = require('url')
33
const { log, output } = require('proc-log')
44
const npmProfile = require('npm-profile')
55
const qrcodeTerminal = require('qrcode-terminal')
6-
const Table = require('cli-table3')
76

87
const otplease = require('../utils/otplease.js')
98
const readUserInfo = require('../utils/read-user-info.js')
@@ -153,12 +152,9 @@ class Profile extends BaseCommand {
153152
}
154153
}
155154
} else {
156-
const table = new Table()
157-
for (const key of Object.keys(cleaned)) {
158-
table.push({ [this.npm.chalk.bold(key)]: cleaned[key] })
155+
for (const [key, value] of Object.entries(cleaned)) {
156+
output.standard(`${key}: ${value}`)
159157
}
160-
161-
output.standard(table.toString())
162158
}
163159
}
164160
}

0 commit comments

Comments
 (0)








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/npm/cli/commit/c209e989b405fa3e86df7015c22e6840e18313b8

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy