Add option to exclude files based on regular expression

[MarcelRobitaille]

I wanted to exclude /node_modules/.

[MarcelRobitaille]

Is this likely to get merged or should I publish a fork?

[xzyfer]

Exclusions are likely to be merged. Globbing will not.

…

On 27 Jun. 2017 12:21 pm, "Marcel Robitaille" ***@***.***> wrote: Is this likely to get merged or should I publish a fork? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#86 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAjZWPYFX8WoBN7gtCsanmnzJrXy7_2Zks5sIGdEgaJpZM4OClFb> .

[MarcelRobitaille]

Do you mind if I publish my fork on npm then?

[xzyfer]

If you wish :)

[xzyfer]

FWIW this implementation isn't complete. It's important to also do the exclusion when determine the ancestors and descendants. You'd also probably want to cache the regex check in the constructor to avoid doing it many times.

[MarcelRobitaille]

I didn't want to check ancestors and descendants because I assumed the usual use case would be /node_modules/ and I wanted to keep things simple. To add the check to ancestors and descendants would it just be as simple as doing the same check in Graph.prototype.visitAncestors and Graph.prototype.visitDescendents?

What do you mean by caching the regex check? You mean this.exclude instanceof RegExp?

[MarcelRobitaille]

Something like that?

[xzyfer]

Not dealing with ancestors and descendants will mean that if a file imports a file from node_modules is appear in the dependency graph. This may be your intention but it's "surprising" and breaks the primary usecase of this library - the node-sass watcher.

[xzyfer]

If files and/or directories are being excluded there should be no way for those exclude to appear in the graph.

[MarcelRobitaille]

That makes sense. Is what I suggested the best way to do that?

[xzyfer]

The regex caching is better. You'll still need to prevent exclude files being referenced in as ancestors and descendants.

[MarcelRobitaille]

I'll write some tests I guess.

[xzyfer]

Please remove package-lock.json

[MarcelRobitaille]

Sure thing. BTW you should have write access, no?

[xzyfer]

I do but I'm mostly catching up whilst in bed. Open source isn't my day job :)

[xzyfer]

This is looking good, thanks for your time.

I have some performance concerns, but they can be address separately. I'll get this released in the next day or so.

[xzyfer]

The option is exclude, not excludes

[MarcelRobitaille]

Oops

[MarcelRobitaille]

Yeah it's true that it checks a lot of regexes but idk how to avoid that.

[MarcelRobitaille]

Any update on this?

[MarcelRobitaille]

What are we waiting on? I'd like to make my fork after this has been merged.

[xzyfer]

Thanks @Iambecomeroot. Apologies for the delay I've be occupied with other projects.