Repository: jprichardson/node-fs-extra (js)

Issue:
rename `clobber` flags to `overwrite`

Pull request:
Rename clobber to overwrite
Keep clobber for backwards compatibility

Fixes #172

Patch:
diff --git a/README.md b/README.md
index 87f7e4d..bdc5eca 100644
--- a/README.md
+++ b/README.md
@@ -127,8 +127,8 @@ Methods
 Copy a file or directory. The directory can have contents. Like `cp -r`.
 
 Options:
-- clobber (boolean): overwrite existing file or directory, default is `true`. _Note that the copy operation will silently fail if you set this to `false` and the destination exists._ Use the `errorOnExist` option to change this behavior.
-- errorOnExist (boolean): when `clobber` is `false` and the destination exists, throw an error. Default is `false`.
+- overwrite (boolean): overwrite existing file or directory, default is `true`. _Note that the copy operation will silently fail if you set this to `false` and the destination exists._ Use the `errorOnExist` option to change this behavior.
+- errorOnExist (boolean): when `overwrite` is `false` and the destination exists, throw an error. Default is `false`.
 - dereference (boolean): dereference symlinks, default is `false`.
 - preserveTimestamps (boolean): will set last modification and access times to the ones of the original source files, default is `false`.
 - filter: Function to filter copied files. Return `true` to include, `false` to exclude. This can also be a RegExp, however this is deprecated (See [issue #239](https://github.com/jprichardson/node-fs-extra/issues/239) for background). _Warning: `copySync` currently applies the filter only to files (see [#180](https://github.com/jprichardson/node-fs-extra/issues/180)). This will be fixed in a future release._
@@ -284,7 +284,7 @@ fs.mkdirsSync('/tmp/another/path')
 Moves a file or directory, even across devices.
 
 Options:
-- clobber (boolean): overwrite existing file or directory
+- overwrite (boolean): overwrite existing file or directory, default is `false`
 
 Example:
 
diff --git a/lib/copy-sync/copy-file-sync.js b/lib/copy-sync/copy-file-sync.js
index 3023505..fb9fad6 100644
--- a/lib/copy-sync/copy-file-sync.js
+++ b/lib/copy-sync/copy-file-sync.js
@@ -4,12 +4,12 @@ var BUF_LENGTH = 64 * 1024
 var _buff = new Buffer(BUF_LENGTH)
 
 function copyFileSync (srcFile, destFile, options) {
-  var clobber = options.clobber
+  var overwrite = options.overwrite
   var errorOnExist = options.errorOnExist
   var preserveTimestamps = options.preserveTimestamps
 
   if (fs.existsSync(destFile)) {
-    if (clobber) {
+    if (overwrite) {
       fs.unlinkSync(destFile)
     } else if (errorOnExist) {
       throw new Error(destFile + ' already exists')
diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js
index 532508b..29812f8 100644
--- a/lib/copy-sync/copy-sync.js
+++ b/lib/copy-sync/copy-sync.js
@@ -13,6 +13,8 @@ function copySync (src, dest, options) {
 
   // default to true for now
   options.clobber = 'clobber' in options ? !!options.clobber : true
+  // overwrite falls back to clobber
+  options.overwrite = 'overwrite' in options ? !!options.overwrite : options.clobber
   options.dereference = 'dereference' in options ? !!options.dereference : false
   options.preserveTimestamps = 'preserveTimestamps' in options ? !!options.preserveTimestamps : false
 
@@ -37,7 +39,7 @@ function copySync (src, dest, options) {
   if (stats.isFile() && performCopy) {
     if (!destFolderExists) mkdir.mkdirsSync(destFolder)
     copyFileSync(src, dest, {
-      clobber: options.clobber,
+      overwrite: options.overwrite,
       errorOnExist: options.errorOnExist,
       preserveTimestamps: options.preserveTimestamps
     })
diff --git a/lib/copy/ncp.js b/lib/copy/ncp.js
index fd82577..7137e16 100644
--- a/lib/copy/ncp.js
+++ b/lib/copy/ncp.js
@@ -16,7 +16,10 @@ function ncp (source, dest, options, callback) {
 
   var filter = options.filter
   var transform = options.transform
-  var clobber = options.clobber !== false // default true
+  var overwrite = options.overwrite
+  // If overwrite is undefined, use clobber, otherwise default to true:
+  if (overwrite === undefined) overwrite = options.clobber
+  if (overwrite === undefined) overwrite = true
   var errorOnExist = options.errorOnExist
   var dereference = options.dereference
   var preserveTimestamps = options.preserveTimestamps === true
@@ -78,7 +81,7 @@ function ncp (source, dest, options, callback) {
       if (writable) {
         copyFile(file, target)
       } else {
-        if (clobber) {
+        if (overwrite) {
           rmFile(target, function () {
             copyFile(file, target)
           })
diff --git a/lib/move/index.js b/lib/move/index.js
index a411243..5874673 100644
--- a/lib/move/index.js
+++ b/lib/move/index.js
@@ -17,7 +17,7 @@ function mv (source, dest, options, callback) {
   }
 
   var shouldMkdirp = ('mkdirp' in options) ? options.mkdirp : true
-  var clobber = ('clobber' in options) ? options.clobber : false
+  var overwrite = options.overwrite || options.clobber || false
 
   if (shouldMkdirp) {
     mkdirs()
@@ -33,14 +33,14 @@ function mv (source, dest, options, callback) {
   }
 
   function doRename () {
-    if (clobber) {
+    if (overwrite) {
       fs.rename(source, dest, function (err) {
         if (!err) return callback()
 
         if (err.code === 'ENOTEMPTY' || err.code === 'EEXIST') {
           remove(dest, function (err) {
             if (err) return callback(err)
-            options.clobber = false // just clobbered it, no need to do it again
+            options.overwrite = false // just overwriteed it, no need to do it again
             mv(source, dest, options, callback)
           })
           return
@@ -51,7 +51,7 @@ function mv (source, dest, options, callback) {
           setTimeout(function () {
             remove(dest, function (err) {
               if (err) return callback(err)
-              options.clobber = false
+              options.overwrite = false
               mv(source, dest, options, callback)
             })
           }, 200)
@@ -59,13 +59,13 @@ function mv (source, dest, options, callback) {
         }
 
         if (err.code !== 'EXDEV') return callback(err)
-        moveAcrossDevice(source, dest, clobber, callback)
+        moveAcrossDevice(source, dest, overwrite, callback)
       })
     } else {
       fs.link(source, dest, function (err) {
         if (err) {
           if (err.code === 'EXDEV' || err.code === 'EISDIR' || err.code === 'EPERM') {
-            moveAcrossDevice(source, dest, clobber, callback)
+            moveAcrossDevice(source, dest, overwrite, callback)
             return
           }
           callback(err)
@@ -77,7 +77,7 @@ function mv (source, dest, options, callback) {
   }
 }
 
-function moveAcrossDevice (source, dest, clobber, callback) {
+function moveAcrossDevice (source, dest, overwrite, callback) {
   fs.stat(source, function (err, stat) {
     if (err) {
       callback(err)
@@ -85,15 +85,15 @@ function moveAcrossDevice (source, dest, clobber, callback) {
     }
 
     if (stat.isDirectory()) {
-      moveDirAcrossDevice(source, dest, clobber, callback)
+      moveDirAcrossDevice(source, dest, overwrite, callback)
     } else {
-      moveFileAcrossDevice(source, dest, clobber, callback)
+      moveFileAcrossDevice(source, dest, overwrite, callback)
     }
   })
 }
 
-function moveFileAcrossDevice (source, dest, clobber, callback) {
-  var outFlags = clobber ? 'w' : 'wx'
+function moveFileAcrossDevice (source, dest, overwrite, callback) {
+  var outFlags = overwrite ? 'w' : 'wx'
   var ins = fs.createReadStream(source)
   var outs = fs.createWriteStream(dest, {flags: outFlags})
 
@@ -108,7 +108,7 @@ function moveFileAcrossDevice (source, dest, clobber, callback) {
     fs.unlink(dest, function () {
       // note: `err` here is from the input stream errror
       if (err.code === 'EISDIR' || err.code === 'EPERM') {
-        moveDirAcrossDevice(source, dest, clobber, callback)
+        moveDirAcrossDevice(source, dest, overwrite, callback)
       } else {
         callback(err)
       }
@@ -130,10 +130,10 @@ function moveFileAcrossDevice (source, dest, clobber, callback) {
   }
 }
 
-function moveDirAcrossDevice (source, dest, clobber, callback) {
+function moveDirAcrossDevice (source, dest, overwrite, callback) {
   var options = {
     stopOnErr: true,
-    clobber: false
+    overwrite: false
   }
 
   function startNcp () {
@@ -143,7 +143,7 @@ function moveDirAcrossDevice (source, dest, clobber, callback) {
     })
   }
 
-  if (clobber) {
+  if (overwrite) {
     remove(dest, function (err) {
       if (err) return callback(err)
       startNcp()


Test patch:
diff --git a/lib/copy-sync/__tests__/copy-sync-file.test.js b/lib/copy-sync/__tests__/copy-sync-file.test.js
index 4365814..70e2e5c 100644
--- a/lib/copy-sync/__tests__/copy-sync-file.test.js
+++ b/lib/copy-sync/__tests__/copy-sync-file.test.js
@@ -121,7 +121,7 @@ describe('+ copySync()', function () {
       })
     })
 
-    describe('> when clobber option is passed', function () {
+    describe('> when overwrite option is passed', function () {
       var src, dest
       var srcData = 'some src data'
 
@@ -134,17 +134,17 @@ describe('+ copySync()', function () {
       })
 
       describe('> when destination file does NOT exist', function () {
-        describe('> when clobber is true', function () {
+        describe('> when overwrite is true', function () {
           it('should copy the file and not throw an error', function () {
-            fs.copySync(src, dest, {clobber: true})
+            fs.copySync(src, dest, {overwrite: true})
             var destData = fs.readFileSync(dest, 'utf8')
             assert.strictEqual(srcData, destData)
           })
         })
 
-        describe('> when clobber is false', function () {
+        describe('> when overwrite is false', function () {
           it('should copy the file and not throw an error', function () {
-            fs.copySync(src, dest, {clobber: false})
+            fs.copySync(src, dest, {overwrite: false})
             var destData = fs.readFileSync(dest, 'utf8')
             assert.strictEqual(srcData, destData)
           })
@@ -158,17 +158,17 @@ describe('+ copySync()', function () {
           fs.writeFileSync(dest, destData)
         })
 
-        describe('> when clobber is true', function () {
+        describe('> when overwrite is true', function () {
           it('should copy the file and not throw an error', function () {
-            fs.copySync(src, dest, {clobber: true})
+            fs.copySync(src, dest, {overwrite: true})
             destData = fs.readFileSync(dest, 'utf8')
             assert.strictEqual(srcData, destData)
           })
         })
 
-        describe('> when clobber is false', function () {
+        describe('> when overwrite is false', function () {
           it('should not throw an error', function () {
-            fs.copySync(src, dest, {clobber: false})
+            fs.copySync(src, dest, {overwrite: false})
 
             // copy never happened
             var destDataNew = fs.readFileSync(dest, 'utf8')
@@ -176,7 +176,7 @@ describe('+ copySync()', function () {
           })
           it('should throw an error when errorOnExist is true', function () {
             assert.throws(function () {
-              fs.copySync(src, dest, {clobber: false, errorOnExist: true})
+              fs.copySync(src, dest, {overwrite: false, errorOnExist: true})
             })
 
             // copy never happened
@@ -185,11 +185,11 @@ describe('+ copySync()', function () {
           })
         })
 
-        describe('> when clobber is true and dest is readonly', function () {
+        describe('> when overwrite is true and dest is readonly', function () {
           it('should copy the file and not throw an error', function () {
             try {
               fs.chmodSync(dest, parseInt('444', 8))
-              fs.copySync(src, dest, {clobber: true})
+              fs.copySync(src, dest, {overwrite: true})
               destData = fs.readFileSync(dest, 'utf8')
               assert.strictEqual(srcData, destData)
             } finally {
@@ -200,5 +200,25 @@ describe('+ copySync()', function () {
         })
       })
     })
+    describe('clobber', function () {
+      var src, dest, srcData, destData
+
+      beforeEach(function () {
+        src = path.join(TEST_DIR, 'src-file')
+        dest = path.join(TEST_DIR, 'des-file')
+        srcData = 'some src data'
+        destData = 'some dest data'
+        fs.writeFileSync(src, srcData)
+        fs.writeFileSync(dest, destData)
+      })
+
+      it('is an alias for overwrite', function () {
+        fs.copySync(src, dest, {clobber: false})
+
+        // copy never happened
+        var destDataNew = fs.readFileSync(dest, 'utf8')
+        assert.strictEqual(destData, destDataNew)
+      })
+    })
   })
 })
diff --git a/lib/copy/__tests__/ncp/ncp.test.js b/lib/copy/__tests__/ncp/ncp.test.js
index 62dcfd5..acb0f42 100644
--- a/lib/copy/__tests__/ncp/ncp.test.js
+++ b/lib/copy/__tests__/ncp/ncp.test.js
@@ -67,7 +67,7 @@ describe('ncp', function () {
       })
     })
 
-    describe('when using clobber=true', function () {
+    describe('when using overwrite=true', function () {
       before(function () {
         this.originalCreateReadStream = fs.createReadStream
       })
@@ -77,7 +77,7 @@ describe('ncp', function () {
       })
 
       it('the copy is complete after callback', function (done) {
-        ncp(src, out, {clobber: true}, function (err) {
+        ncp(src, out, {overwrite: true}, function (err) {
           fs.createReadStream = function () {
             done(new Error('createReadStream after callback'))
           }
@@ -87,25 +87,43 @@ describe('ncp', function () {
       })
     })
 
-    describe('when using clobber=false', function () {
+    describe('when using overwrite=false', function () {
       beforeEach(function (done) {
         rimraf(out, done)
       })
       it('works', function (cb) {
-        ncp(src, out, {clobber: false}, function (err) {
+        ncp(src, out, {overwrite: false}, function (err) {
           assert.ifError(err)
           cb()
         })
       })
       it('should not error if files exist', function (cb) {
         ncp(src, out, function () {
-          ncp(src, out, {clobber: false}, function (err) {
+          ncp(src, out, {overwrite: false}, function (err) {
             assert.ifError(err)
             cb()
           })
         })
       })
       it('should error if errorOnExist and file exists', function (cb) {
+        ncp(src, out, function () {
+          ncp(src, out, {
+            overwrite: false,
+            errorOnExist: true
+          }, function (err) {
+            assert(err)
+            cb()
+          })
+        })
+      })
+    })
+
+    describe('clobber', function () {
+      beforeEach(function (done) {
+        rimraf(out, done)
+      })
+
+      it('is an alias for overwrite', function (cb) {
         ncp(src, out, function () {
           ncp(src, out, {
             clobber: false,
diff --git a/lib/move/__tests__/move-clobber.test.js b/lib/move/__tests__/move-clobber.test.js
index 7a2368e..484ce18 100644
--- a/lib/move/__tests__/move-clobber.test.js
+++ b/lib/move/__tests__/move-clobber.test.js
@@ -6,7 +6,7 @@ var fse = require(process.cwd())
 
 /* global afterEach, beforeEach, describe, it */
 
-describe('move / clobber', function () {
+describe('move / overwrite', function () {
   var TEST_DIR, FIXTURES_DIR
 
   beforeEach(function (done) {
@@ -26,9 +26,9 @@ describe('move / clobber', function () {
     fse.remove(TEST_DIR, done)
   })
 
-  describe('> when clobber = true', function () {
+  describe('> when overwrite = true', function () {
     describe('> when dest is a directory', function () {
-      it('should clobber the destination', function (done) {
+      it('should overwrite the destination', function (done) {
         // Tests fail on appveyor/Windows due to
         // https://github.com/isaacs/node-graceful-fs/issues/98.
         // Workaround by increasing the timeout by a minute (because
@@ -54,7 +54,7 @@ describe('move / clobber', function () {
         assert(paths.indexOf('some-file') >= 0)
         assert(paths.indexOf('some-folder') >= 0)
 
-        fse.move(src, dest, {clobber: true}, function (err) {
+        fse.move(src, dest, {overwrite: true}, function (err) {
           if (err) return done(err)
 
           // verify dest does not have old stuff
@@ -71,4 +71,42 @@ describe('move / clobber', function () {
       })
     })
   })
+  describe('> clobber', function () {
+    it('is an alias for overwrite', function (done) {
+      // Tests fail on appveyor/Windows due to
+      // https://github.com/isaacs/node-graceful-fs/issues/98.
+      // Workaround by increasing the timeout by a minute (because
+      // graceful times out after a minute).
+      this.timeout(90000)
+
+      // use fixtures dir as dest since it has stuff
+      var dest = FIXTURES_DIR
+      var paths = fs.readdirSync(dest)
+
+      // verify dest has stuff
+      assert(paths.indexOf('a-file') >= 0)
+
+      // create new source dir
+      var src = path.join(TEST_DIR, 'src')
+      fse.ensureDirSync(src)
+      fs.writeFileSync(path.join(src, 'some-file'), 'hi')
+
+      // verify source has stuff
+      paths = fs.readdirSync(src)
+      assert(paths.indexOf('some-file') >= 0)
+
+      fse.move(src, dest, {clobber: true}, function (err) {
+        if (err) return done(err)
+
+        // verify dest does not have old stuff
+        var paths = fs.readdirSync(dest)
+        assert.strictEqual(paths.indexOf('a-file'), -1)
+
+        // verify dest has new stuff
+        assert(paths.indexOf('some-file') >= 0)
+
+        done()
+      })
+    })
+  })
 })
diff --git a/lib/move/__tests__/move.test.js b/lib/move/__tests__/move.test.js
index 67e137d..8e99ad2 100644
--- a/lib/move/__tests__/move.test.js
+++ b/lib/move/__tests__/move.test.js
@@ -68,14 +68,14 @@ describe('move', function () {
     })
   })
 
-  it('should not overwrite if clobber = false', function (done) {
+  it('should not overwrite if overwrite = false', function (done) {
     var src = FIXTURES_DIR + '/a-file'
     var dest = FIXTURES_DIR + '/a-folder/another-file'
 
     // verify file exists already
     assert(fs.existsSync(dest))
 
-    fse.move(src, dest, {clobber: false}, function (err) {
+    fse.move(src, dest, {overwrite: false}, function (err) {
       assert.ok(err && err.code === 'EEXIST', 'throw EEXIST')
       done()
     })
@@ -173,7 +173,7 @@ describe('move', function () {
     })
   })
 
-  it('should clobber folders across devices', function (done) {
+  it('should overwrite folders across devices', function (done) {
     var src = FIXTURES_DIR + '/a-folder'
     var dest = FIXTURES_DIR + '/a-folder-dest'
 
@@ -181,7 +181,7 @@ describe('move', function () {
 
     setUpMockFs('EXDEV')
 
-    fse.move(src, dest, {clobber: true}, function (err) {
+    fse.move(src, dest, {overwrite: true}, function (err) {
       assert.ifError(err)
       assert.strictEqual(fs.rename.callCount, 1)