Accept globs as excludes #71
Conversation
BREAKING CHANGE: Excludes of directories and filenames including globbing characters (+, @, *, ?, [, ], !, {, }) will now be interpreted as globs.
|
Nice! I am super busy today, but I'll have a look at this ASAP and try to merge it by tomorrow. (Pester me if I don't. ;) |
|
Will do! ;) |
| @@ -64,6 +65,15 @@ exports.excludeFile = (fileName, options) -> | |||
| # Only instrument files that are inside the project. | |||
| excluded = true | |||
|
|
|||
| # For each exclude value try to use it as a pattern to exclude files | |||
| exclude.map (pattern) -> | |||
| glob.sync pattern, | |||
There was a problem hiding this comment.
The one thing that's bad about this is that we'll end up re-running glob.sync for the same variables over and over again. But, without rewriting half this project, I guess this works. :P I'll merge this, and see if I can come up with a clever way to make this a little better. We could cache the result of glob.sync() for each value in options.exclude...
There was a problem hiding this comment.
That should be fine: glob caches these kind of requests internally.
There was a problem hiding this comment.
Although I'm 90% through making this work by just doing one call to glob. Haha. :P I also made one very small change - historically this would let you specify an include starting with a "/" where / was the project root. This works the same as, say, .gitignore or .npmignore. If we pass in root: basePath in addition it cwd: basePath, then we get exactly this behavior - the leading / is effectively optional. I'm also going to strip out the old style "prefix match" code - if we're going to make a breaking change, may as well break it good. :)
There was a problem hiding this comment.
Have a look at #73 and tell me what you think?
There was a problem hiding this comment.
Not completely sure where you're heading. If you could explain a little, I might be able to help :)
Not sure though, if glob is the real performance bottleneck for coffee-coverage :)
There was a problem hiding this comment.
I wanted to make this "all glob based", and get rid of the prefix matching found here. It's a bit weird that we prefix match glob patterns.
So, I figured I'd take each match returned by glob, add a '/' to the end, and compare against that. The problem is, this works fine when your exclude list is ['/node_modules'](since the matched globs will be the same list.) When your exclude list is ['/node_modules/*/'], on the other hand, glob returns potentially thousands of strings for us to compare against. In my largish project, this was the difference between a 9 second run and a 20 second run.
I think I'm just trying to optimize prematurely though. :P
There was a problem hiding this comment.
Now I get it. And thinking about it I realized that I was thinking about the problem all wrong: If we're just excluding files there is no need to hit the filesystem at all to make the comparison, which should significantly speed up the globbing process and allow for something like what you just suggested.
I'll be working on a PR on one of the next days :)
| # For each exclude value try to use it as a pattern to exclude files | ||
| exclude.map (pattern) -> | ||
| glob.sync pattern, | ||
| dot: true |
There was a problem hiding this comment.
Without dot: true, patterns that should normally include files starting with . would not include these files. That is fine if you are using patterns to include files, but if you use patterns to exclude files you usually want the dot files to be excluded, too.
|
Ok, published as v1.0.0. Thanks again! This is a much requested feature. |
Based on #65.
BREAKING CHANGE: Excludes of directories and filenames including globbing characters (+, @, *, ?, [, ], !, {, }) will now be interpreted as globs. This shouldn't be that relevant, except for maybe + and @