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.

Author: wraithgar

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

@@ -34,20 +34,21 @@ there are any recommended changes, it will display them.  By default npm
 runs all of these checks. You can limit what checks are ran by
 specifying them as extra arguments.
 
-#### `npm ping`
+#### `Connecting to the registry`
 
 By default, npm installs from the primary npm registry,
-`registry.npmjs.org`.  `npm doctor` hits a special ping endpoint within the
-registry. This can also be checked with `npm ping`. If this check fails,
-you may be using a proxy that needs to be configured, or may need to talk
-to your IT staff to get access over HTTPS to `registry.npmjs.org`.
+`registry.npmjs.org`.  `npm doctor` hits a special connection testing
+endpoint within the registry. This can also be checked with `npm ping`.
+If this check fails, you may be using a proxy that needs to be
+configured, or may need to talk to your IT staff to get access over
+HTTPS to `registry.npmjs.org`.
 
 This check is done against whichever registry you've configured (you can
 see what that is by running `npm config get registry`), and if you're using
 a private registry that doesn't support the `/whoami` endpoint supported by
 the primary registry, this check may fail.
 
-#### `npm -v`
+#### `Checking npm version`
 
 While Node.js may come bundled with a particular version of npm, it's the
 policy of the CLI team that we recommend all users run `npm@latest` if they
@@ -57,7 +58,7 @@ support releases typically only receive critical security and regression
 fixes. The team believes that the latest tested version of npm is almost
 always likely to be the most functional and defect-free version of npm.
 
-#### `node -v`
+#### `Checking node version`
 
 For most users, in most circumstances, the best version of Node will be the
 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
 version of Node because of enterprise change control policies. That's OK!
 But in general, the npm team recommends that most users run Node.js LTS.
 
-#### `npm config get registry`
+#### `Checking configured npm registry`
 
 You may be installing from private package registries for your project or
 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
 part of `npm doctor` just lets you, and maybe whoever's helping you with
 support, know that you're not using the default registry.
 
-#### `which git`
+#### `Checking for git executable in PATH`
 
 While it's documented in the README, it may not be obvious that npm needs
 Git installed to do many of the things that it does. Also, in some cases

lib/commands/doctor.js

@@ -1,7 +1,6 @@
 const cacache = require('cacache')
 const fs = require('fs')
 const fetch = require('make-fetch-happen')
-const Table = require('cli-table3')
 const which = require('which')
 const pacote = require('pacote')
 const { resolve } = require('path')
@@ -34,57 +33,59 @@ const maskLabel = mask => {
 
 const subcommands = [
   {
-    groups: ['ping', 'registry'],
-    title: 'npm ping',
+    // Ping is left in as a legacy command but is listed as "connection" to
+    // make more sense to more people
+    groups: ['connection', 'ping', 'registry'],
+    title: 'Connecting to the registry',
     cmd: 'checkPing',
   }, {
     groups: ['versions'],
-    title: 'npm -v',
+    title: 'Checking npm version',
     cmd: 'getLatestNpmVersion',
   }, {
     groups: ['versions'],
-    title: 'node -v',
+    title: 'Checking node version',
     cmd: 'getLatestNodejsVersion',
   }, {
     groups: ['registry'],
-    title: 'npm config get registry',
+    title: 'Checking configured npm registry',
     cmd: 'checkNpmRegistry',
   }, {
     groups: ['environment'],
-    title: 'git executable in PATH',
+    title: 'Checking for git executable in PATH',
     cmd: 'getGitPath',
   }, {
     groups: ['environment'],
-    title: 'global bin folder in PATH',
+    title: 'Checking for global bin folder in PATH',
     cmd: 'getBinPath',
   }, {
     groups: ['permissions', 'cache'],
-    title: 'Perms check on cached files',
+    title: 'Checking permissions on cached files (this may take awhile)',
     cmd: 'checkCachePermission',
     windows: false,
   }, {
     groups: ['permissions'],
-    title: 'Perms check on local node_modules',
+    title: 'Checking permissions on local node_modules (this may take awhile)',
     cmd: 'checkLocalModulesPermission',
     windows: false,
   }, {
     groups: ['permissions'],
-    title: 'Perms check on global node_modules',
+    title: 'Checking permissions on global node_modules (this may take awhile)',
     cmd: 'checkGlobalModulesPermission',
     windows: false,
   }, {
     groups: ['permissions'],
-    title: 'Perms check on local bin folder',
+    title: 'Checking permissions on local bin folder',
     cmd: 'checkLocalBinPermission',
     windows: false,
   }, {
     groups: ['permissions'],
-    title: 'Perms check on global bin folder',
+    title: 'Checking permissions on global bin folder',
     cmd: 'checkGlobalBinPermission',
     windows: false,
   }, {
     groups: ['cache'],
-    title: 'Verify cache contents',
+    title: 'Verifying cache contents (this may take awhile)',
     cmd: 'verifyCachedFiles',
     windows: false,
   },
@@ -104,43 +105,28 @@ class Doctor extends BaseCommand {
   static params = ['registry']
   static ignoreImplicitWorkspace = false
   static usage = [`[${subcommands.flatMap(s => s.groups)
-    .filter((value, index, self) => self.indexOf(value) === index)
+    .filter((value, index, self) => self.indexOf(value) === index && value !== 'ping')
     .join('] [')}]`]
 
   static subcommands = subcommands
 
-  // minimum width of check column, enough for the word `Check`
-  #checkWidth = 5
-
   async exec (args) {
     log.info('doctor', 'Running checkup')
     let allOk = true
 
     const actions = this.actions(args)
-    this.#checkWidth = actions.reduce((length, item) =>
-      Math.max(item.title.length, length), this.#checkWidth)
 
-    if (!this.npm.silent) {
-      this.output(['Check', 'Value', 'Recommendation/Notes'].map(h => this.npm.chalk.underline(h)))
-    }
-    // Do the actual work
+    const chalk = this.npm.chalk
     for (const { title, cmd } of actions) {
-      const item = [title]
+      this.output(title)
+      // TODO when we have an in progress indicator that could go here
+      let result
       try {
-        item.push(true, await this[cmd]())
+        result = await this[cmd]()
+        this.output(`${chalk.green('Ok')}${result ? `\n${result}` : ''}\n`)
       } catch (err) {
-        item.push(false, err)
-      }
-      if (!item[1]) {
         allOk = false
-        item[0] = this.npm.chalk.red(item[0])
-        item[1] = this.npm.chalk.red('not ok')
-        item[2] = this.npm.chalk.cyan(String(item[2]))
-      } else {
-        item[1] = this.npm.chalk.green('ok')
-      }
-      if (!this.npm.silent) {
-        this.output(item)
+        this.output(`${chalk.red('Not ok')}\n${chalk.cyan(err)}\n`)
       }
     }
 
@@ -343,38 +329,11 @@ class Doctor extends BaseCommand {
     }
   }
 
-  output (row) {
-    const t = new Table({
-      chars: {
-        top: '',
-        'top-mid': '',
-        'top-left': '',
-        'top-right': '',
-        bottom: '',
-        'bottom-mid': '',
-        'bottom-left': '',
-        'bottom-right': '',
-        left: '',
-        'left-mid': '',
-        mid: '',
-        'mid-mid': '',
-        right: '',
-        'right-mid': '',
-        middle: '  ',
-      },
-      style: {
-        'padding-left': 0,
-        'padding-right': 0,
-        // setting border here is not necessary visually since we've already
-        // zeroed out all the chars above, but without it cli-table3 will wrap
-        // some of the separator spaces with ansi codes which show up in
-        // snapshots.
-        border: 0,
-      },
-      colWidths: [this.#checkWidth, 6],
-    })
-    t.push(row)
-    output.standard(t.toString())
+  output (...args) {
+    // TODO display layer should do this
+    if (!this.npm.silent) {
+      output.standard(...args)
+    }
   }
 
   actions (params) {

lib/commands/hook.js

@@ -1,7 +1,6 @@
 const hookApi = require('libnpmhook')
 const otplease = require('../utils/otplease.js')
 const relativeDate = require('tiny-relative-date')
-const Table = require('cli-table3')
 const { output } = require('proc-log')
 
 const BaseCommand = require('../base-command.js')
@@ -52,6 +51,7 @@ class Hook extends BaseCommand {
 
   async ls (pkg, opts) {
     const hooks = await hookApi.ls({ ...opts, package: pkg })
+
     if (opts.json) {
       output.standard(JSON.stringify(hooks, null, 2))
     } else if (opts.parseable) {
@@ -62,32 +62,18 @@ class Hook extends BaseCommand {
     } else if (!hooks.length) {
       output.standard("You don't have any hooks configured yet.")
     } else if (!this.npm.silent) {
-      if (hooks.length === 1) {
-        output.standard('You have one hook configured.')
-      } else {
-        output.standard(`You have ${hooks.length} hooks configured.`)
-      }
+      output.standard(`You have ${hooks.length} hook${hooks.length !== 1 ? 's' : ''} configured.`)
 
-      const table = new Table({ head: ['id', 'target', 'endpoint'] })
-      hooks.forEach((hook) => {
-        table.push([
-          { rowSpan: 2, content: hook.id },
-          this.hookName(hook),
-          hook.endpoint,
-        ])
+      for (const hook of hooks) {
+        output.standard(`Hook ${hook.id}: ${this.hookName(hook)}`)
+        output.standard(`Endpoint: ${hook.endpoint}`)
         if (hook.last_delivery) {
-          table.push([
-            {
-              colSpan: 1,
-              content: `triggered ${relativeDate(hook.last_delivery)}`,
-            },
-            hook.response_code,
-          ])
+          /* eslint-disable-next-line max-len */
+          output.standard(`Triggered ${relativeDate(hook.last_delivery)}, response code was "${hook.response_code}"\n`)
         } else {
-          table.push([{ colSpan: 2, content: 'never triggered' }])
+          output.standard('Never triggered\n')
         }
-      })
-      output.standard(table.toString())
+      }
     }
   }
 
@@ -116,11 +102,7 @@ class Hook extends BaseCommand {
   }
 
   hookName (hook) {
-    let target = hook.name
-    if (hook.type === 'owner') {
-      target = '~' + target
-    }
-    return target
+    return `${hook.type === 'owner' ? '~' : ''}${hook.name}`
   }
 }
 module.exports = Hook

lib/commands/org.js

@@ -1,6 +1,5 @@
 const liborg = require('libnpmorg')
 const otplease = require('../utils/otplease.js')
-const Table = require('cli-table3')
 const BaseCommand = require('../base-command.js')
 const { output } = require('proc-log')
 
@@ -143,13 +142,10 @@ class Org extends BaseCommand {
         output.standard([u, roster[u]].join('\t'))
       })
     } else if (!this.npm.silent) {
-      const table = new Table({ head: ['user', 'role'] })
-      Object.keys(roster)
-        .sort()
-        .forEach(u => {
-          table.push([u, roster[u]])
-        })
-      output.standard(table.toString())
+      const chalk = this.npm.chalk
+      for (const u of Object.keys(roster).sort()) {
+        output.standard(`${u} - ${chalk.cyan(roster[u])}`)
+      }
     }
   }
 }

lib/commands/profile.js

@@ -3,7 +3,6 @@ const { URL } = require('url')
 const { log, output } = require('proc-log')
 const npmProfile = require('npm-profile')
 const qrcodeTerminal = require('qrcode-terminal')
-const Table = require('cli-table3')
 
 const otplease = require('../utils/otplease.js')
 const readUserInfo = require('../utils/read-user-info.js')
@@ -153,12 +152,9 @@ class Profile extends BaseCommand {
           }
         }
       } else {
-        const table = new Table()
-        for (const key of Object.keys(cleaned)) {
-          table.push({ [this.npm.chalk.bold(key)]: cleaned[key] })
+        for (const [key, value] of Object.entries(cleaned)) {
+          output.standard(`${key}: ${value}`)
         }
-
-        output.standard(table.toString())
       }
     }
   }