search

sindresorhus / gulp-filter

Filter files in a `vinyl` stream
MIT License
315 stars 37 forks source link

make relative path inference configurable #61

Closed jimmytheneutrino closed 7 years ago

jimmytheneutrino commented 8 years ago

See https://github.com/sindresorhus/gulp-filter/pull/59 for details.

sindresorhus commented 8 years ago

Sorry, not really interested in having an option for this.

// @cb1kenobi

jimmytheneutrino commented 8 years ago

How about a more general option for providing a conversion function with a sane default instead? E.g., the usage would be something like filter(pattern, {chunkToString: function(file){return file.relative;}}).

cb1kenobi commented 8 years ago

Before I submitted #59, my code looked identical to your PR. I too had an option called options.relativeToCwd, but I just couldn't think of a scenario where you would want the path to not be relative to the currrent working directory and thus I removed the flag before submitting the PR.

So, given your example on #59, why can't you just do the following?

filter(['**', '!**/*.html'])

That will return all non-html files. Note that the first argument must be a string or an array of strings and the second argument is an options object. You forgot to wrap your pattern in an array.

jimmytheneutrino commented 8 years ago

why can't you just do the following?

I want all files except the html files in the root dir.

I.e., this is an angular app and I want to concat files based on what is in index.html (which is in the root dir) but not include index.html (and some other files, e.g., 404.html) into angular cache.

jimmytheneutrino commented 8 years ago

I modified the PR so that the option is a function now and relativeToCwd is default. If you think it is still contrary to your vision, please close this PR.

(Then I will have to go with a fork (they are easy to create, since almost everything of this nice plugin is now separated into streamfilter). E.g., I sketched one that uses micromatch instead of multimatch, see micromatch-streamfilter branch of my fork.)

peterjuras commented 8 years ago

I would be really interested in this. I have a lot of tests for template generators and they usually end up with weird process.cwd() outputs. I noticed that updating from v3 to v4 is a pain since all my filters are broken now.

drgullin commented 8 years ago

@peterjuras why would you need these lines?

module.exports.relative = relative;
module.exports.relativeToCwd = relativeToCwd;

these are likely irrelevant, since paths are returned, not a vinyl objects (multimatch needed).

drgullin commented 8 years ago

We suggest a useCwd option for a file.relative filtering, which would fallback gulp-filter to v3 conditions when needed https://github.com/sindresorhus/gulp-filter/issues/65.

I've created a tiny patch without the tests https://github.com/sindresorhus/gulp-filter/pull/66 Feel free to add tests and use it.

sinedied commented 8 years ago

@sindresorhus why not having an option? In my case (and it seems I'm not the only one) the new behavior based on cwd is not desirable: not only it makes the path filters unnecessarily complicated, but if like me you have your gulp tasks separated in modules in a different folder, it is just problematic.

Why change something that worked?

cb1kenobi commented 8 years ago

@sinedied Please read my comment here: https://github.com/sindresorhus/gulp-filter/issues/67#issuecomment-213309299.