Accidentally truncates file names ending with $$

[krainboltgreene]

So I thought it was a browserify-hmr issue but I looked into their tests and while I could produce an error I discovered via console log that fsExtra.copy was the point at which it incorrectly created a file. I'm now looking at fs-extra's tests to reproduce.

[jprichardson]

I'm now looking at fs-extra's tests to reproduce.

Thank you. This would be very helpful.

[krainboltgreene]

I have written the test and it fails:

➜  node-fs-extra git:(master) ✗ node_modules/.bin/mocha lib/copy/__tests__/copy.test.js --grep 'should copy the file asynchronously'


  fs-extra
    + copy()
      > when the source is a file
        ✓ should copy the file asynchronously
        > with a weird name
          1) should copy the file asynchronously


  1 passing (84ms)
  1 failing

  1) fs-extra + copy() > when the source is a file > with a weird name should copy the file asynchronously:
     Uncaught Error: ENOENT: no such file or directory, open '/var/folders/z3/p9_b08g503jb3ndftzggrp5h0000gn/T/fs-extra/copy/TEST_fs-extra_copy$$'
      at Error (native)
      at Object.fs.openSync (fs.js:584:18)
      at Object.fs.readFileSync (fs.js:431:33)
      at lib/copy/__tests__/copy.test.js:59:58
      at doneOne (lib/copy/ncp.js:237:40)
      at lib/copy/ncp.js:122:11
      at FSReqWrap.oncomplete (fs.js:82:15)

[krainboltgreene]

Here's a snapshot after disabling the afterHook:

ls /var/folders/z3/p9_b08g503jb3ndftzggrp5h0000gn/T/fs-extra/copy/
TEST_fs-extra_copy$ TEST_fs-extra_src$$

[krainboltgreene]

The breaking code appears to be https://github.com/jprichardson/node-fs-extra/blob/master/lib/copy/ncp.js#L82

console.log(targetPath)
/var/folders/z3/p9_b08g503jb3ndftzggrp5h0000gn/T/fs-extra/copy/TEST_fs-extra_copy$$
console.log(target)
/var/folders/z3/p9_b08g503jb3ndftzggrp5h0000gn/T/fs-extra/copy/TEST_fs-extra_copy$

[krainboltgreene]

This appears to be a "feature" of String.prototype.replace(): https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace#Specifying_a_string_as_a_parameter

[jprichardson]

Ah, so it's treating it as Regex string that needs to be escaped.... hmm.

[krainboltgreene]

Not quite? It's just because it thinks you might want special evaluation in the replacement string. It just turns out it always does this instead of only doing this on regexp first arguments.

[krainboltgreene]

You know what I think I'm wrong on the why, but my guess is now after reading spec stuff is that they wanted string patterns to have some capture functionality.

[hhamilto]

Unless any one else is feverishly working on this, I'd be delighted to submit a PR.

@krainboltgreene any chance you still have that test lying around? No worries if not, but I'd like to be sure that my changes fix your problem.

In any event, I'll write a test too.

[krainboltgreene]

@hhamilto Any test you write that includes $$ in the string is sufficient, I think!