Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use Minimatch class to improve perf, fixes #29; fix RegExp handling #30

Merged
merged 1 commit into from
Dec 31, 2014

Conversation

UltCombo
Copy link
Contributor

Improve negative globs handling perf and fix some issues with RegExp.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.55%) when pulling e7af5f8 on UltCombo:minimatch-class into 127eb2c on wearefractal:master.

@UltCombo
Copy link
Contributor Author

@contra BTW, there is just one line missing to hit 100% coverage. Perhaps we should throw instead of returning true for unknown glob types? https://coveralls.io/files/408135441#L105

@UltCombo
Copy link
Contributor Author

Here are the benchmarks to prove the perf gain.

Before:
image

After:
image

Benchmark source: sindresorhus/globby#10 (irrelevant globby data omitted)
The benchmark with fewer exclusions is slower because it has the overhead of emitting more matched files.

@UltCombo
Copy link
Contributor Author

I may have misunderstood the regex matching part though, it would make more sense if the regexp.test() returns true == keep file, else filter it out.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.55%) when pulling e2f5e34 on UltCombo:minimatch-class into 127eb2c on wearefractal:master.

@UltCombo
Copy link
Contributor Author

@contra PTAL when you have time.

@sindresorhus
Copy link
Contributor

Nice!

@UltCombo
Copy link
Contributor Author

Oh sorry, mixing too many topics here. Moved the glob-stream and globby topic to #31

@yocontra
Copy link
Member

The RegExp stuff should keep the file if the regex returns true, otherwise filter it out as implemented. Want to write some tests to ensure this is happening?

@UltCombo
Copy link
Contributor Author

@contra I did, see the files changed tab. 😃

yocontra pushed a commit that referenced this pull request Dec 31, 2014
use Minimatch class to improve perf, fixes #29; fix RegExp handling
@yocontra yocontra merged commit e96f39f into gulpjs:master Dec 31, 2014
@UltCombo UltCombo deleted the minimatch-class branch December 31, 2014 08:17
phated pushed a commit that referenced this pull request Feb 16, 2017
use Minimatch class to improve perf, fixes #29; fix RegExp handling
phated pushed a commit that referenced this pull request Feb 16, 2017
use Minimatch class to improve perf, fixes #29; fix RegExp handling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants