search

hparra / gulp-rename

Rename files easily
MIT License
692 stars 73 forks source link

Don't check what's returned from passed function #24

Closed adam-lynch closed 9 years ago

adam-lynch commented 10 years ago

  1. By default, CoffeeScript will return the last line in a function. If you had the following:

    .pipe($.rename (path) =>
       path.extname = '.html'
)

    Then your gulp task would fail with TypeError: Arguments to path.join must be strings, unless you end the function with return path / return / return a falsy value.

  2. It's kinda ambiguous / confusing. I can't think of a use case where you'd edit the original but return something else (since the original will overridden with what you return anyway).

I didn't check the tests, I just made this change on github.com. If they fail, I'll come back to them later.

adam-lynch commented 10 years ago

Ok, I almost forgot to come back to this. I'll sort this over the weekend.

adam-lynch commented 10 years ago

Done, ready for merge. Travis is reporting a build error above as you can see. The tests passed with node 0.10 but on 0.9 it failed due to an npm glitch. Re-run the build or just merge as is, you can see the tests passing here.

shinnn commented 9 years ago

+1

shinnn commented 9 years ago

OK, I'll merge this PR since it's more suitable for the bahavior described in README. I regard this change as a bug fix.

However, it's debatable whether using return value is more useful for the users or not.

Thanks, @adam-lynch :)