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

Issue:
Add docs to explain whether `move` supports directories as destination
Given that `mv somefile.txt textfiles` will happily move a file `somefile.txt` into a dir `textfiles`, one might expect `move('somefile.txt', 'textfiles')` to work, but the docs currently do not explain what a valid `dest` is. Could the docs be amended to make it explicit whether `dest` must be a file path, or whether any path is valid?

Pull request:
Clarify move*() docs
Fixes #830

Patch:
diff --git a/docs/move-sync.md b/docs/move-sync.md
index 53fde49..8cc6770 100644
--- a/docs/move-sync.md
+++ b/docs/move-sync.md
@@ -3,7 +3,7 @@
 Moves a file or directory, even across devices.
 
 - `src` `<String>`
-- `dest` `<String>`
+- `dest` `<String>` Note: When `src` is a file, `dest` must be a file and when `src` is a directory, `dest` must be a directory.
 - `options` `<Object>`
   - `overwrite` `<boolean>`: overwrite existing file or directory, default is `false`.
 
diff --git a/docs/move.md b/docs/move.md
index 586d46c..7afed78 100644
--- a/docs/move.md
+++ b/docs/move.md
@@ -3,7 +3,7 @@
 Moves a file or directory, even across devices.
 
 - `src` `<String>`
-- `dest` `<String>`
+- `dest` `<String>` Note: When `src` is a file, `dest` must be a file and when `src` is a directory, `dest` must be a directory.
 - `options` `<Object>`
   - `overwrite` `<boolean>`: overwrite existing file or directory, default is `false`.
 - `callback` `<Function>`
diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js
index 2d9523e..5e245b9 100644
--- a/lib/copy-sync/copy-sync.js
+++ b/lib/copy-sync/copy-sync.js
@@ -30,7 +30,7 @@ function handleFilterAndCopy (destStat, src, dest, opts) {
   if (opts.filter && !opts.filter(src, dest)) return
   const destParent = path.dirname(dest)
   if (!fs.existsSync(destParent)) mkdirsSync(destParent)
-  return startCopy(destStat, src, dest, opts)
+  return getStats(destStat, src, dest, opts)
 }
 
 function startCopy (destStat, src, dest, opts) {
diff --git a/lib/copy/copy.js b/lib/copy/copy.js
index c12578d..2a2874f 100644
--- a/lib/copy/copy.js
+++ b/lib/copy/copy.js
@@ -42,10 +42,10 @@ function checkParentDir (destStat, src, dest, opts, cb) {
   const destParent = path.dirname(dest)
   pathExists(destParent, (err, dirExists) => {
     if (err) return cb(err)
-    if (dirExists) return startCopy(destStat, src, dest, opts, cb)
+    if (dirExists) return getStats(destStat, src, dest, opts, cb)
     mkdirs(destParent, err => {
       if (err) return cb(err)
-      return startCopy(destStat, src, dest, opts, cb)
+      return getStats(destStat, src, dest, opts, cb)
     })
   })
 }


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 6ffff60..f656e04 100644
--- a/lib/copy-sync/__tests__/copy-sync-file.test.js
+++ b/lib/copy-sync/__tests__/copy-sync-file.test.js
@@ -104,6 +104,25 @@ describe('+ copySync() / file', () => {
       assert(fs.existsSync(destFile3))
     })
 
+    it('should not call filter fn more than needed', () => {
+      const src = path.join(TEST_DIR, 'foo')
+
+      fs.writeFileSync(src, '')
+
+      const dest = path.join(TEST_DIR, 'bar')
+
+      let filterCallCount = 0
+      const filter = () => {
+        filterCallCount++
+        return true
+      }
+
+      fs.copySync(src, dest, filter)
+
+      assert.strictEqual(filterCallCount, 1)
+      assert(fs.existsSync(dest))
+    })
+
     describe('> when the destination dir does not exist', () => {
       it('should create the destination directory and copy the file', () => {
         const src = path.join(TEST_DIR, 'file.txt')
diff --git a/lib/copy/__tests__/copy.test.js b/lib/copy/__tests__/copy.test.js
index e7eed97..71470a2 100644
--- a/lib/copy/__tests__/copy.test.js
+++ b/lib/copy/__tests__/copy.test.js
@@ -283,6 +283,25 @@ describe('fs-extra', () => {
         })
       })
 
+      it('should not call filter fn more than needed', done => {
+        const src = path.join(TEST_DIR, 'foo')
+        fs.writeFileSync(src, '')
+        const dest = path.join(TEST_DIR, 'bar')
+
+        let filterCallCount = 0
+        const filter = () => {
+          filterCallCount++
+          return true
+        }
+
+        fse.copy(src, dest, filter, err => {
+          assert(!err)
+          assert.strictEqual(filterCallCount, 1)
+          assert(fs.existsSync(dest))
+          done()
+        })
+      })
+
       it('accepts options object in place of filter', done => {
         const srcFile1 = path.join(TEST_DIR, '1.jade')
         fs.writeFileSync(srcFile1, '')