search

gulpjs / glob-stream

Readable streamx interface over anymatch.
MIT License
178 stars 53 forks source link

Use Minimatch class for improved perf #29

Closed UltCombo closed 9 years ago

UltCombo commented 9 years ago

While node-glob doesn't implement the ignore option, it would be faster to handle handle globs with the Minimatch class (see https://github.com/sindresorhus/globby/pull/6#issuecomment-68418533 and the comment below).

UltCombo commented 9 years ago

Before I send a PR to fix this, does glob-stream's globs argument support anything other than strings? There's this typeof pattern !== 'string' and pattern instanceof RegExp, but there are no docs nor tests regarding these apparently. Can these be removed safely?

yocontra commented 9 years ago

@UltCombo We do support custom RegExps at the post-scan level https://github.com/wearefractal/glob-stream/blob/master/index.js#L104 I'd like to keep that if possible

UltCombo commented 9 years ago

@contra Oh I see. But https://github.com/wearefractal/glob-stream/blob/master/index.js#L109 shouldn't be return false;? Or are RegExps considered as negative globs for some reason? I can't seem to find any unit test using RegExps.

yocontra commented 9 years ago

@UltCombo All RegExps are treated as negative since they are used to filter post-scan which means they cannot be positive. You can't give a regex to node-glob so there is no way it could be positive. There probably aren't tests if you can't find any, but there should be

UltCombo commented 9 years ago

Oh I see, thanks for the explanation :) I'll write some tests for those as well after I get this working.