refactor edit, add tests

Author: nlf

lib/edit.js

@@ -1,52 +1,36 @@
 // npm edit <pkg>
 // open the package folder in the $EDITOR
 
-module.exports = edit
-edit.usage = 'npm edit <pkg>[/<subpkg>...]'
+const { resolve } = require('path')
+const fs = require('graceful-fs')
+const { spawn } = require('child_process')
+const npm = require('./npm.js')
+const usageUtil = require('./utils/usage.js')
+const splitPackageNames = require('./utils/split-package-names.js')
 
-edit.completion = require('./utils/completion/installed-shallow.js')
-
-var npm = require('./npm.js')
-var path = require('path')
-var fs = require('graceful-fs')
-var editor = require('editor')
-var noProgressTillDone = require('./utils/no-progress-while-running').tillDone
+const usage = usageUtil('edit', 'npm edit <pkg>[/<subpkg>...]')
+const completion = require('./utils/completion/installed-shallow.js')
 
 function edit (args, cb) {
-  var p = args[0]
-  if (args.length !== 1 || !p)
-    return cb(edit.usage)
-  var e = npm.config.get('editor')
-  if (!e) {
-    return cb(new Error(
-      "No editor set.  Set the 'editor' config, or $EDITOR environ."
-    ))
-  }
-  p = p.split('/')
-    // combine scoped parts
-    .reduce(function (parts, part) {
-      if (parts.length === 0)
-        return [part]
-
-      var lastPart = parts[parts.length - 1]
-      // check if previous part is the first part of a scoped package
-      if (lastPart[0] === '@' && !lastPart.includes('/'))
-        parts[parts.length - 1] += '/' + part
-      else
-        parts.push(part)
-
-      return parts
-    }, [])
-    .join('/node_modules/')
-    .replace(/(\/node_modules)+/, '/node_modules')
-  var f = path.resolve(npm.dir, p)
-  fs.lstat(f, function (er) {
-    if (er)
-      return cb(er)
-    editor(f, { editor: e }, noProgressTillDone(function (er) {
-      if (er)
-        return cb(er)
-      npm.commands.rebuild(args, cb)
-    }))
+  if (args.length !== 1)
+    return cb(usage)
+
+  const path = splitPackageNames(args[0])
+  const dir = resolve(npm.dir, path)
+
+  fs.lstat(dir, (err) => {
+    if (err)
+      return cb(err)
+
+    const [bin, ...args] = npm.config.get('editor').split(/\s+/)
+    const editor = spawn(bin, [...args, dir], { stdio: 'inherit' })
+    editor.on('exit', (code) => {
+      if (code)
+        return cb(new Error(`editor process exited with code: ${code}`))
+
+      npm.commands.rebuild([dir], cb)
+    })
   })
 }
+
+module.exports = Object.assign(edit, { completion, usage })

test/lib/edit.js

@@ -0,0 +1,123 @@
+const { test } = require('tap')
+const { resolve } = require('path')
+const requireInject = require('require-inject')
+const { EventEmitter } = require('events')
+
+let editorBin = null
+let editorArgs = null
+let editorOpts = null
+let EDITOR_CODE = 0
+const childProcess = {
+  spawn: (bin, args, opts) => {
+    // save for assertions
+    editorBin = bin
+    editorArgs = args
+    editorOpts = opts
+
+    const editorEvents = new EventEmitter()
+    process.nextTick(() => {
+      editorEvents.emit('exit', EDITOR_CODE)
+    })
+    return editorEvents
+  },
+}
+
+let rebuildArgs = null
+let EDITOR = 'vim'
+const npm = {
+  config: {
+    get: () => EDITOR,
+  },
+  dir: resolve(__dirname, '../../node_modules'),
+  commands: {
+    rebuild: (args, cb) => {
+      rebuildArgs = args
+      return cb()
+    },
+  },
+}
+
+const gracefulFs = require('graceful-fs')
+const edit = requireInject('../../lib/edit.js', {
+  '../../lib/npm.js': npm,
+  child_process: childProcess,
+  'graceful-fs': gracefulFs,
+})
+
+test('npm edit', t => {
+  t.teardown(() => {
+    rebuildArgs = null
+    editorBin = null
+    editorArgs = null
+    editorOpts = null
+  })
+
+  return edit(['semver'], (err) => {
+    if (err)
+      throw err
+
+    const path = resolve(__dirname, '../../node_modules/semver')
+    t.strictSame(editorBin, EDITOR, 'used the correct editor')
+    t.strictSame(editorArgs, [path], 'edited the correct directory')
+    t.strictSame(editorOpts, { stdio: 'inherit' }, 'passed the correct opts')
+    t.strictSame(rebuildArgs, [path], 'passed the correct path to rebuild')
+    t.end()
+  })
+})
+
+test('npm edit editor has flags', t => {
+  EDITOR = 'code -w'
+  t.teardown(() => {
+    rebuildArgs = null
+    editorBin = null
+    editorArgs = null
+    editorOpts = null
+    EDITOR = 'vim'
+  })
+
+  return edit(['semver'], (err) => {
+    if (err)
+      throw err
+
+    const path = resolve(__dirname, '../../node_modules/semver')
+    t.strictSame(editorBin, 'code', 'used the correct editor')
+    t.strictSame(editorArgs, ['-w', path], 'edited the correct directory, keeping flags')
+    t.strictSame(editorOpts, { stdio: 'inherit' }, 'passed the correct opts')
+    t.strictSame(rebuildArgs, [path], 'passed the correct path to rebuild')
+    t.end()
+  })
+})
+
+test('npm edit no args', t => {
+  return edit([], (err) => {
+    t.match(err, /npm edit/, 'throws usage error')
+    t.end()
+  })
+})
+
+test('npm edit lstat error propagates', t => {
+  const _lstat = gracefulFs.lstat
+  gracefulFs.lstat = (dir, cb) => {
+    return cb(new Error('lstat failed'))
+  }
+  t.teardown(() => {
+    gracefulFs.lstat = _lstat
+  })
+
+  return edit(['semver'], (err) => {
+    t.match(err, /lstat failed/, 'user received correct error')
+    t.end()
+  })
+})
+
+test('npm edit editor exit code error propagates', t => {
+  EDITOR_CODE = 137
+  t.teardown(() => {
+    EDITOR_CODE = 0
+  })
+
+  return edit(['semver'], (err) => {
+    t.match(err, /exited with code: 137/, 'user received correct error')
+    t.end()
+  })
+})