fix: refactor help to use @npmcli/promise-spawn

This also refactors the test to use `mockNpm`

Author: lukekarrys

lib/commands/help.js

@@ -1,4 +1,4 @@
-const { spawn } = require('child_process')
+const spawn = require('@npmcli/promise-spawn')
 const path = require('path')
 const openUrl = require('../utils/open-url.js')
 const { promisify } = require('util')
@@ -14,19 +14,26 @@ const BaseCommand = require('../base-command.js')
 const manNumberRegex = /\.(\d+)(\.[^/\\]*)?$/
 // Searches for the "npm-" prefix in page names, to prefer those.
 const manNpmPrefixRegex = /\/npm-/
+// hardcoded names for mansections
+// XXX: these are used in the docs workspace and should be exported
+// from npm so section names can changed more easily
+const manSectionNames = {
+  1: 'commands',
+  5: 'configuring-npm',
+  7: 'using-npm',
+}
 
 class Help extends BaseCommand {
   static description = 'Get help on npm'
   static name = 'help'
   static usage = ['<term> [<terms..>]']
   static params = ['viewer']
-  static ignoreImplicitWorkspace = true
 
   async completion (opts) {
     if (opts.conf.argv.remain.length > 2) {
       return []
     }
-    const g = path.resolve(__dirname, '../../man/man[0-9]/*.[0-9]')
+    const g = path.resolve(this.npm.npmRoot, 'man/man[0-9]/*.[0-9]')
     const files = await glob(globify(g))
 
     return Object.keys(files.reduce(function (acc, file) {
@@ -40,10 +47,7 @@ class Help extends BaseCommand {
   async exec (args) {
     // By default we search all of our man subdirectories, but if the user has
     // asked for a specific one we limit the search to just there
-    let manSearch = 'man*'
-    if (/^\d+$/.test(args[0])) {
-      manSearch = `man${args.shift()}`
-    }
+    const manSearch = /^\d+$/.test(args[0]) ? `man${args.shift()}` : 'man*'
 
     if (!args.length) {
       return this.npm.output(await this.npm.usage)
@@ -54,20 +58,18 @@ class Help extends BaseCommand {
       return this.helpSearch(args)
     }
 
-    let section = this.npm.deref(args[0]) || args[0]
-
-    // support `npm help package.json`
-    section = section.replace('.json', '-json')
+    // `npm help package.json`
+    const arg = (this.npm.deref(args[0]) || args[0]).replace('.json', '-json')
 
-    const manroot = path.resolve(__dirname, '..', '..', 'man')
     // find either section.n or npm-section.n
-    const f = `${manroot}/${manSearch}/?(npm-)${section}.[0-9]*`
-    let mans = await glob(globify(f))
-    mans = mans.sort((a, b) => {
+    const f = globify(path.resolve(this.npm.npmRoot, `man/${manSearch}/?(npm-)${arg}.[0-9]*`))
+
+    const [man] = await glob(f).then(r => r.sort((a, b) => {
       // Prefer the page with an npm prefix, if there's only one.
       const aHasPrefix = manNpmPrefixRegex.test(a)
       const bHasPrefix = manNpmPrefixRegex.test(b)
       if (aHasPrefix !== bHasPrefix) {
+        /* istanbul ignore next */
         return aHasPrefix ? -1 : 1
       }
 
@@ -76,6 +78,7 @@ class Help extends BaseCommand {
       const aManNumberMatch = a.match(manNumberRegex)
       const bManNumberMatch = b.match(manNumberRegex)
       if (aManNumberMatch) {
+        /* istanbul ignore next */
         if (!bManNumberMatch) {
           return -1
         }
@@ -88,77 +91,41 @@ class Help extends BaseCommand {
       }
 
       return localeCompare(a, b)
-    })
-    const man = mans[0]
+    }))
 
-    if (man) {
-      await this.viewMan(man)
-    } else {
-      return this.helpSearch(args)
-    }
+    return man ? this.viewMan(man) : this.helpSearch(args)
   }
 
   helpSearch (args) {
     return this.npm.exec('help-search', args)
   }
 
   async viewMan (man) {
-    const env = {}
-    Object.keys(process.env).forEach(function (i) {
-      env[i] = process.env[i]
-    })
     const viewer = this.npm.config.get('viewer')
 
-    const opts = {
-      env,
-      stdio: 'inherit',
+    if (viewer === 'browser') {
+      return openUrl(this.npm, this.htmlMan(man), 'help available at the following URL', true)
     }
 
-    let bin = 'man'
-    const args = []
-    switch (viewer) {
-      case 'woman':
-        bin = 'emacsclient'
-        args.push('-e', `(woman-find-file '${man}')`)
-        break
-
-      case 'browser':
-        await openUrl(this.npm, this.htmlMan(man), 'help available at the following URL', true)
-        return
-
-      default:
-        args.push(man)
-        break
+    let args = ['man', [man]]
+    if (viewer === 'woman') {
+      args = ['emacsclient', ['-e', `(woman-find-file '${man}')`]]
     }
 
-    const proc = spawn(bin, args, opts)
-    return new Promise((resolve, reject) => {
-      proc.on('exit', (code) => {
-        if (code) {
-          return reject(new Error(`help process exited with code: ${code}`))
-        }
-
-        return resolve()
-      })
+    return spawn(...args, { stdio: 'inherit' }).catch(err => {
+      if (err.code) {
+        throw new Error(`help process exited with code: ${err.code}`)
+      } else {
+        throw err
+      }
     })
   }
 
   // Returns the path to the html version of the man page
   htmlMan (man) {
-    let sect = man.match(manNumberRegex)[1]
+    const sect = manSectionNames[man.match(manNumberRegex)[1]]
     const f = path.basename(man).replace(manNumberRegex, '')
-    switch (sect) {
-      case '1':
-        sect = 'commands'
-        break
-      case '5':
-        sect = 'configuring-npm'
-        break
-      case '7':
-        sect = 'using-npm'
-        break
-    }
-    return 'file:///' + path.resolve(__dirname, '..', '..', 'docs', 'output', sect, f + '.html')
+    return 'file:///' + path.resolve(this.npm.npmRoot, `docs/output/${sect}/${f}.html`)
   }
 }
 module.exports = Help