From 3da22b5803a9634dd361b871a521e96332557307 Mon Sep 17 00:00:00 2001
From: RyanZim <opensrc@ryanzim.com>
Date: Wed, 26 Oct 2016 19:10:01 -0400
Subject: [PATCH] BREAKING: Refactor lib/copy/ncp.js

- Fix #233 - First argument passed to callback should be an Error
  object, not an array.
- Fix #272 - Should error when options.clobber is false and the
  destination exists.
- Remove options.limit, it is no longer needed with graceful-fs.
---
 lib/copy/__tests__/ncp/broken-symlink.test.js |  3 +-
 lib/copy/__tests__/ncp/ncp-error-perm.test.js |  2 +-
 lib/copy/__tests__/ncp/ncp.test.js            | 15 ++++++--
 lib/copy/ncp.js                               | 38 +++++++------------
 4 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/lib/copy/__tests__/ncp/broken-symlink.test.js b/lib/copy/__tests__/ncp/broken-symlink.test.js
index 3ab067e3..33b0d435 100644
--- a/lib/copy/__tests__/ncp/broken-symlink.test.js
+++ b/lib/copy/__tests__/ncp/broken-symlink.test.js
@@ -33,8 +33,7 @@ describe('ncp broken symlink', function () {
 
   it('should return an error when dereference=true', function (done) {
     ncp(src, out, {dereference: true}, function (err) {
-      assert.equal(err.length, 1)
-      assert.equal(err[0].code, 'ENOENT')
+      assert.equal(err.code, 'ENOENT')
       done()
     })
   })
diff --git a/lib/copy/__tests__/ncp/ncp-error-perm.test.js b/lib/copy/__tests__/ncp/ncp-error-perm.test.js
index 5aaec3da..50389598 100644
--- a/lib/copy/__tests__/ncp/ncp-error-perm.test.js
+++ b/lib/copy/__tests__/ncp/ncp-error-perm.test.js
@@ -43,7 +43,7 @@ describe('ncp / error / dest-permission', function () {
 
     ncp(src, subdest, function (err) {
       assert(err)
-      assert.equal(err[0].code, 'EACCES')
+      assert.equal(err.code, 'EACCES')
       done()
     })
   })
diff --git a/lib/copy/__tests__/ncp/ncp.test.js b/lib/copy/__tests__/ncp/ncp.test.js
index 73c5b817..e091306c 100644
--- a/lib/copy/__tests__/ncp/ncp.test.js
+++ b/lib/copy/__tests__/ncp/ncp.test.js
@@ -5,7 +5,7 @@ var rimraf = require('rimraf')
 var readDirFiles = require('read-dir-files').read // temporary, will remove
 var ncp = require('../../ncp')
 
-/* global after, before, describe, it */
+/* eslint-env mocha */
 
 var fixturesDir = path.join(__dirname, 'fixtures')
 
@@ -88,10 +88,19 @@ describe('ncp', function () {
     })
 
     describe('when using clobber=false', function () {
-      it('the copy is completed successfully', function (cb) {
+      beforeEach(function (done) {
+        rimraf(out, done)
+      })
+      it('works', function (cb) {
+        ncp(src, out, {clobber: false}, function (err) {
+          assert.ifError(err)
+          cb()
+        })
+      })
+      it('errors if files exist', function (cb) {
         ncp(src, out, function () {
           ncp(src, out, {clobber: false}, function (err) {
-            assert.ifError(err)
+            assert(err)
             cb()
           })
         })
diff --git a/lib/copy/ncp.js b/lib/copy/ncp.js
index 4fded87d..46d5f8c4 100644
--- a/lib/copy/ncp.js
+++ b/lib/copy/ncp.js
@@ -16,18 +16,15 @@ function ncp (source, dest, options, callback) {
 
   var filter = options.filter
   var transform = options.transform
-  var clobber = options.clobber !== false
+  var clobber = options.clobber !== false // default true
   var dereference = options.dereference
   var preserveTimestamps = options.preserveTimestamps === true
 
-  var errs = null
-
   var started = 0
   var finished = 0
   var running = 0
-  // this is pretty useless now that we're using graceful-fs
-  // consider removing
-  var limit = options.limit || 512
+
+  var errored = false
 
   startCopy(currentPath)
 
@@ -49,11 +46,6 @@ function ncp (source, dest, options, callback) {
 
   function getStats (source) {
     var stat = dereference ? fs.stat : fs.lstat
-    if (running >= limit) {
-      return setImmediate(function () {
-        getStats(source)
-      })
-    }
     running++
     stat(source, function (err, stats) {
       if (err) return onError(err)
@@ -89,7 +81,11 @@ function ncp (source, dest, options, callback) {
             copyFile(file, target)
           })
         } else {
-          doneOne()
+          var err = new Error('EEXIST: ' + target + ' already exists.')
+          err.code = 'EEXIST'
+          err.errno = -17
+          err.path = target
+          onError(err)
         }
       }
     })
@@ -133,7 +129,7 @@ function ncp (source, dest, options, callback) {
   }
 
   function onDir (dir) {
-    var target = dir.name.replace(currentPath, targetPath)
+    var target = dir.name.replace(currentPath, targetPath.replace('$', '$$$$')) // escapes '$' with '$$'
     isWritable(target, function (writable) {
       if (writable) {
         return mkDir(dir, target)
@@ -214,19 +210,11 @@ function ncp (source, dest, options, callback) {
   }
 
   function onError (err) {
-    if (options.stopOnError) {
+    // ensure callback is defined & called only once:
+    if (!errored && callback !== undefined) {
+      errored = true
       return callback(err)
-    } else if (!errs && options.errs) {
-      errs = fs.createWriteStream(options.errs)
-    } else if (!errs) {
-      errs = []
-    }
-    if (typeof errs.write === 'undefined') {
-      errs.push(err)
-    } else {
-      errs.write(err.stack + '\n\n')
     }
-    return doneOne()
   }
 
   function doneOne (skipped) {
@@ -234,7 +222,7 @@ function ncp (source, dest, options, callback) {
     finished++
     if ((started === finished) && (running === 0)) {
       if (callback !== undefined) {
-        return errs ? callback(errs) : callback(null)
+        return callback(null)
       }
     }
   }