Only modify files that match the replace expression

[bassmanpaul]

From what I can tell, the gulp-replace modifies all files regardless of whether they match the replace condition or not.

My scenario is that gulp-replace correctly updates my files but also my fonts, images and text files that are Unicode format. The skipBinary option successfully skips the fonts & images but my .txt files that have unicode characters in them are still left broken.

Some other skip options would be nice, but surely it would be better to skip modifying files unless they actually have a match?

Thoughts?

P.S. Thanks for providing a very useful module :)

[lazd]

Interesting... This definitely sounds like a bug. I'd accept a PR to fix it.

[bassmanpaul]

Sure thing.

How about something like the following before the skipBinary check?

options = options || {};

if(options.testFirst === null || options.testFirst)
{
  if(!String(file.contents).match(search))
  {
    return callback(null, file);
  }
}

This way users can opt out of the extra regular expression check if they are more concerned with performance than stream modification.

[lazd]

Hmm, I'd rather do it without introducing a new option... Perhaps we can perform the replacement as normal, test for equality, and if they're equal, then don't modify the file?

[bassmanpaul]

Okay, fair enough. How about placing the check (without options) at the start of the doReplace(); function?

function doReplace()
{
  if(!String(file.contents).match(search))
  {
    return callback(null, file);
  }
...

To do it before this point would probably require the check in two places (the skipBinary check and after the skipBinary check). Doing the check before the skipBinary would mean unnecessary checks for skipBinary files?

[akaleeroy]

Is there a problem preventing this fix?

[lazd]

@akaleeroy the problem is there is no PR with the fix and tests :) Feel free to send one along and I'll review it!

[akaleeroy]

@lazd Haha, yeah, I figured it out as soon as I ran the tests

function doReplace() {
   if(!String(file.contents).match(search)) {
     return callback(null, null);
   }
  // ...

Swallowing the file by passing null will work to not overwrite unnecessarily, but it makes the tests fail. We can't do that, right? I don't understand enough to figure out what to do instead.

[akaleeroy]

I worked around this issue using gulp-ignore, here is a gist: Gulp replace only relevant files

Maybe there should be an option, passthrough all or replaced, I don't know.

[bassmanpaul]

@akaleeroy Nice gist.

It's a shame a fix can't be directly incorporated into gulp-replace as I haven't got much time at the mo to try a PR :s