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

Issue:
Additional param for options.filter() on copy()
At the moment when using `copy()`, the `options.filter()` function receives the source `path` as it's only param. It would be nice to have both the source and destination paths:

```javascript
fs.copy(srcFolder, destFolder, {
  filter: (srcPath, destPath) => {
    return true
  }
}, cb)
```

This would make checking last mod dates much easier.


Pull request description:
Add dest parameter to filter option in copy and copySync
This resolves #351.

I added dest as an additional parameter to filter option in both `copy` and `copySync`.

Patch:
diff --git a/docs/copy.md b/docs/copy.md
index 617510f..95e0ceb 100644
--- a/docs/copy.md
+++ b/docs/copy.md
@@ -26,3 +26,19 @@ fs.copy('/tmp/mydir', '/tmp/mynewdir', function (err) {
   console.log('success!')
 }) // copies directory, even if it has subdirectories or files
 ```
+
+**Using filter function**
+
+```js
+var fs = require('fs-extra')
+
+var filterFunc = function (src, dest) {
+  // your logic here
+  // it will be copied if return true
+}
+
+fs.copy('/tmp/mydir', '/tmp/mynewdir', { filter: filterFunc }, function (err) {
+  if (err) return console.error(err)
+  console.log('success!')
+})
+```
diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js
index eadc80a..9d5639c 100644
--- a/lib/copy-sync/copy-sync.js
+++ b/lib/copy-sync/copy-sync.js
@@ -36,7 +36,7 @@ function copySync (src, dest, options) {
   if (options.filter instanceof RegExp) {
     console.warn('Warning: fs-extra: Passing a RegExp filter is deprecated, use a function')
     performCopy = options.filter.test(src)
-  } else if (typeof options.filter === 'function') performCopy = options.filter(src)
+  } else if (typeof options.filter === 'function') performCopy = options.filter(src, dest)
 
   if (stats.isFile() && performCopy) {
     if (!destFolderExists) mkdir.mkdirsSync(destFolder)
diff --git a/lib/copy/ncp.js b/lib/copy/ncp.js
index 7137e16..9670ee0 100644
--- a/lib/copy/ncp.js
+++ b/lib/copy/ncp.js
@@ -41,7 +41,7 @@ function ncp (source, dest, options, callback) {
           return doneOne(true)
         }
       } else if (typeof filter === 'function') {
-        if (!filter(source)) {
+        if (!filter(source, dest)) {
           return doneOne(true)
         }
       }


Test patch:
diff --git a/lib/copy-sync/__tests__/copy-sync-dir.test.js b/lib/copy-sync/__tests__/copy-sync-dir.test.js
index 2b3cd0a..82c360e 100644
--- a/lib/copy-sync/__tests__/copy-sync-dir.test.js
+++ b/lib/copy-sync/__tests__/copy-sync-dir.test.js
@@ -27,7 +27,7 @@ describe('+ copySync()', () => {
       src = path.join(TEST_DIR, 'src')
       dest = path.join(TEST_DIR, 'dest')
 
-      fs.mkdirsSync(src)
+      fs.mkdirSync(src)
 
       for (let i = 0; i < FILES; ++i) {
         fs.writeFileSync(path.join(src, i.toString()), crypto.randomBytes(SIZE))
@@ -35,7 +35,7 @@ describe('+ copySync()', () => {
 
       const subdir = path.join(src, 'subdir')
 
-      fs.mkdirsSync(subdir)
+      fs.mkdirSync(subdir)
 
       for (let i = 0; i < FILES; ++i) {
         fs.writeFileSync(path.join(subdir, i.toString()), crypto.randomBytes(SIZE))
@@ -55,7 +55,7 @@ describe('+ copySync()', () => {
     })
 
     it('should preserve symbolic links', () => {
-      fs.mkdirsSync(src)
+      fs.mkdirSync(src)
       fs.symlinkSync('destination', path.join(src, 'symlink'))
 
       fs.copySync(src, dest)
@@ -64,19 +64,44 @@ describe('+ copySync()', () => {
       assert.strictEqual(link, 'destination')
     })
 
+    describe('> when the destination dir does not exist', () => {
+      it('should create the destination directory and copy the file', () => {
+        const src = path.join(TEST_DIR, 'data/')
+        fs.mkdirSync(src)
+
+        const d1 = 'file1'
+        const d2 = 'file2'
+
+        fs.writeFileSync(path.join(src, 'f1.txt'), d1)
+        fs.writeFileSync(path.join(src, 'f2.txt'), d2)
+
+        const dest = path.join(TEST_DIR, 'this/path/does/not/exist/outputDir')
+
+        fs.copySync(src, dest)
+
+        const o1 = fs.readFileSync(path.join(dest, 'f1.txt'), 'utf8')
+        const o2 = fs.readFileSync(path.join(dest, 'f2.txt'), 'utf8')
+
+        assert.strictEqual(d1, o1)
+        assert.strictEqual(d2, o2)
+      })
+    })
+  })
+
+  describe('> when filter is used', () => {
     it('should should apply filter recursively', () => {
       const FILES = 2
       // Don't match anything that ends with a digit higher than 0:
       const filter = s => /(0|\D)$/i.test(s)
 
-      fs.mkdirsSync(src)
+      fs.mkdirSync(src)
 
       for (let i = 0; i < FILES; ++i) {
         fs.writeFileSync(path.join(src, i.toString()), crypto.randomBytes(SIZE))
       }
 
       const subdir = path.join(src, 'subdir')
-      fs.mkdirsSync(subdir)
+      fs.mkdirSync(subdir)
 
       for (let i = 0; i < FILES; ++i) {
         fs.writeFileSync(path.join(subdir, i.toString()), crypto.randomBytes(SIZE))
@@ -110,10 +135,10 @@ describe('+ copySync()', () => {
       const IGNORE = 'ignore'
       const filter = p => !~p.indexOf(IGNORE)
 
-      fs.mkdirsSync(src)
+      fs.mkdirSync(src)
 
       const ignoreDir = path.join(src, IGNORE)
-      fs.mkdirsSync(ignoreDir)
+      fs.mkdirSync(ignoreDir)
 
       fs.writeFileSync(path.join(ignoreDir, 'file'), crypto.randomBytes(SIZE))
 
@@ -123,27 +148,57 @@ describe('+ copySync()', () => {
       assert(!fs.existsSync(path.join(dest, IGNORE, 'file')), 'file was not ignored')
     })
 
-    describe('> when the destination dir does not exist', () => {
-      it('should create the destination directory and copy the file', () => {
-        const src = path.join(TEST_DIR, 'data/')
+    it('should apply filter when it is applied only to dest', done => {
+      const timeCond = new Date().getTime()
+
+      const filter = (s, d) => fs.statSync(d).birthtime.getTime() < timeCond
+
+      const dest = path.join(TEST_DIR, 'dest')
+
+      setTimeout(() => {
         fs.mkdirSync(src)
+        fs.writeFileSync(path.join(src, 'somefile.html'), 'some data')
+        fs.mkdirSync(dest)
+        try {
+          fs.copySync(src, dest, filter)
+        } catch (err) {
+          assert.ifError(err)
+        }
+        assert(!fs.existsSync(path.join(dest, 'somefile.html')))
+        done()
+      }, 1000)
+    })
 
-        const d1 = 'file1'
-        const d2 = 'file2'
+    it('should apply filter when it is applied to both src and dest', done => {
+      const timeCond = new Date().getTime()
+      const filter = (s, d) => s.split('.').pop() !== 'css' && fs.statSync(path.dirname(d)).birthtime.getTime() > timeCond
 
-        fs.writeFileSync(path.join(src, 'f1.txt'), d1)
-        fs.writeFileSync(path.join(src, 'f2.txt'), d2)
+      const dest = path.join(TEST_DIR, 'dest')
 
-        const dest = path.join(TEST_DIR, 'this/path/does/not/exist/outputDir')
+      setTimeout(() => {
+        const srcFile1 = path.join(TEST_DIR, '1.html')
+        const srcFile2 = path.join(TEST_DIR, '2.css')
+        const srcFile3 = path.join(TEST_DIR, '3.jade')
 
-        fs.copySync(src, dest)
+        fs.writeFileSync(srcFile1, '')
+        fs.writeFileSync(srcFile2, '')
+        fs.writeFileSync(srcFile3, '')
 
-        const o1 = fs.readFileSync(path.join(dest, 'f1.txt'), 'utf8')
-        const o2 = fs.readFileSync(path.join(dest, 'f2.txt'), 'utf8')
+        const destFile1 = path.join(dest, 'dest1.html')
+        const destFile2 = path.join(dest, 'dest2.css')
+        const destFile3 = path.join(dest, 'dest3.jade')
 
-        assert.strictEqual(d1, o1)
-        assert.strictEqual(d2, o2)
-      })
+        fs.mkdirSync(dest)
+
+        fs.copySync(srcFile1, destFile1, filter)
+        fs.copySync(srcFile2, destFile2, filter)
+        fs.copySync(srcFile3, destFile3, filter)
+
+        assert(fs.existsSync(destFile1))
+        assert(!fs.existsSync(destFile2))
+        assert(fs.existsSync(destFile3))
+        done()
+      }, 1000)
     })
   })
 })
diff --git a/lib/copy/__tests__/copy.test.js b/lib/copy/__tests__/copy.test.js
index 4469597..beedfd2 100644
--- a/lib/copy/__tests__/copy.test.js
+++ b/lib/copy/__tests__/copy.test.js
@@ -58,28 +58,6 @@ describe('fs-extra', () => {
         })
       })
 
-      it('should only copy files allowed by filter fn', done => {
-        const srcFile1 = path.join(TEST_DIR, '1.css')
-        fs.writeFileSync(srcFile1, '')
-        const destFile1 = path.join(TEST_DIR, 'dest1.css')
-        const filter = s => s.split('.').pop() !== 'css'
-        fse.copy(srcFile1, destFile1, filter, () => {
-          assert(!fs.existsSync(destFile1))
-          done()
-        })
-      })
-
-      it('accepts options object in place of filter', done => {
-        const srcFile1 = path.join(TEST_DIR, '1.jade')
-        fs.writeFileSync(srcFile1, '')
-        const destFile1 = path.join(TEST_DIR, 'dest1.jade')
-        const options = { filter: s => /.html$|.css$/i.test(s) }
-        fse.copy(srcFile1, destFile1, options, () => {
-          assert(!fs.existsSync(destFile1))
-          done()
-        })
-      })
-
       it('should copy to a destination file with two \'$\' characters in name (eg: TEST_fs-extra_$$_copy)', done => {
         const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_src')
         const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_$$_copy')
@@ -192,5 +170,161 @@ describe('fs-extra', () => {
         })
       })
     })
+
+    describe('> when filter is used', () => {
+      it('should only copy files allowed by filter fn', done => {
+        const srcFile1 = path.join(TEST_DIR, '1.css')
+        fs.writeFileSync(srcFile1, '')
+        const destFile1 = path.join(TEST_DIR, 'dest1.css')
+        const filter = s => s.split('.').pop() !== 'css'
+
+        fse.copy(srcFile1, destFile1, filter, err => {
+          assert(!err)
+          assert(!fs.existsSync(destFile1))
+          done()
+        })
+      })
+
+      it('accepts options object in place of filter', done => {
+        const srcFile1 = path.join(TEST_DIR, '1.jade')
+        fs.writeFileSync(srcFile1, '')
+        const destFile1 = path.join(TEST_DIR, 'dest1.jade')
+        const options = { filter: s => /.html$|.css$/i.test(s) }
+
+        fse.copy(srcFile1, destFile1, options, (err) => {
+          assert(!err)
+          assert(!fs.existsSync(destFile1))
+          done()
+        })
+      })
+
+      it('should should apply filter recursively', done => {
+        const FILES = 2
+        // Don't match anything that ends with a digit higher than 0:
+        const filter = s => /(0|\D)$/i.test(s)
+
+        const src = path.join(TEST_DIR, 'src')
+        fse.mkdirsSync(src)
+
+        for (let i = 0; i < FILES; ++i) {
+          fs.writeFileSync(path.join(src, i.toString()), crypto.randomBytes(SIZE))
+        }
+
+        const subdir = path.join(src, 'subdir')
+        fse.mkdirsSync(subdir)
+
+        for (let i = 0; i < FILES; ++i) {
+          fs.writeFileSync(path.join(subdir, i.toString()), crypto.randomBytes(SIZE))
+        }
+        const dest = path.join(TEST_DIR, 'dest')
+        fse.copy(src, dest, filter, err => {
+          assert(!err)
+
+          assert(fs.existsSync(dest))
+          assert(FILES > 1)
+
+          for (let i = 0; i < FILES; ++i) {
+            if (i === 0) {
+              assert(fs.existsSync(path.join(dest, i.toString())))
+            } else {
+              assert(!fs.existsSync(path.join(dest, i.toString())))
+            }
+          }
+
+          const destSub = path.join(dest, 'subdir')
+
+          for (let j = 0; j < FILES; ++j) {
+            if (j === 0) {
+              assert(fs.existsSync(path.join(destSub, j.toString())))
+            } else {
+              assert(!fs.existsSync(path.join(destSub, j.toString())))
+            }
+          }
+          done()
+        })
+      })
+
+      it('should apply the filter to directory names', done => {
+        const IGNORE = 'ignore'
+        const filter = p => !~p.indexOf(IGNORE)
+
+        const src = path.join(TEST_DIR, 'src')
+        fse.mkdirsSync(src)
+
+        const ignoreDir = path.join(src, IGNORE)
+        fse.mkdirsSync(ignoreDir)
+
+        fse.writeFileSync(path.join(ignoreDir, 'file'), crypto.randomBytes(SIZE))
+
+        const dest = path.join(TEST_DIR, 'dest')
+
+        fse.copySync(src, dest, filter)
+
+        assert(!fs.existsSync(path.join(dest, IGNORE)), 'directory was not ignored')
+        assert(!fs.existsSync(path.join(dest, IGNORE, 'file')), 'file was not ignored')
+        done()
+      })
+
+      it('should apply filter when it is applied only to dest', done => {
+        const timeCond = new Date().getTime()
+
+        const filter = (s, d) => fs.statSync(d).birthtime.getTime() < timeCond
+
+        const src = path.join(TEST_DIR, 'src')
+        fse.mkdirsSync(src)
+        const subdir = path.join(src, 'subdir')
+        fse.mkdirsSync(subdir)
+
+        const dest = path.join(TEST_DIR, 'dest')
+
+        setTimeout(() => {
+          fse.mkdirsSync(dest)
+
+          fse.copy(src, dest, filter, err => {
+            assert(!err)
+            assert(!fs.existsSync(path.join(dest, 'subdir')))
+            done()
+          })
+        }, 1000)
+      })
+
+      it('should apply filter when it is applied to both src and dest', done => {
+        const timeCond = new Date().getTime()
+        const filter = (s, d) => s.split('.').pop() !== 'css' && fs.statSync(path.dirname(d)).birthtime.getTime() > timeCond
+
+        const dest = path.join(TEST_DIR, 'dest')
+        setTimeout(() => {
+          fse.mkdirsSync(dest)
+
+          const srcFile1 = path.join(TEST_DIR, '1.html')
+          const srcFile2 = path.join(TEST_DIR, '2.css')
+          const srcFile3 = path.join(TEST_DIR, '3.jade')
+
+          fse.writeFileSync(srcFile1, '')
+          fse.writeFileSync(srcFile2, '')
+          fse.writeFileSync(srcFile3, '')
+
+          const destFile1 = path.join(dest, 'dest1.html')
+          const destFile2 = path.join(dest, 'dest2.css')
+          const destFile3 = path.join(dest, 'dest3.jade')
+
+          fse.copy(srcFile1, destFile1, filter, err => {
+            assert(!err)
+            assert(fs.existsSync(destFile1))
+
+            fse.copy(srcFile2, destFile2, filter, err => {
+              assert(!err)
+              assert(!fs.existsSync(destFile2))
+
+              fse.copy(srcFile3, destFile3, filter, err => {
+                assert(!err)
+                assert(fs.existsSync(destFile3))
+                done()
+              })
+            })
+          })
+        }, 1000)
+      })
+    })
   })
 })