search

gulpjs / glob-stream

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

Avoid creating multiple Minimatch instances for the same negative glob #33

Closed UltCombo closed 9 years ago

UltCombo commented 9 years ago

This optimizes use cases such as ['a', 'b', '!b'], where a new Minimatch instance for '!b' would be created for each previous positive glob.

Now the Minimatch instances for negative globs are created beforehand, thus positive globs can use the same negative matcher instances. That is, for ['a', 'b', '!b'] both 'a' and 'b' globs will use the same Minimatch instance for '!b' instead of instantiating their own.

Also, as far as I can see, gs.createStream is internal (no docs or tests), so I took the liberty of adapting it a bit as I needed the complete opt object to pass to the Minimatch constructor in gs.create.

Should also note that this change, technically, would instantiate Minimatch instances that would never be used when considering leading negative globs (e.g. ['!b', 'b']), but that is a complete misuse and IIRC Minimatch produces their underlying regex lazily, so this shouldn't be a problem.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.1%) when pulling 5571d414a8c3d556bddf6f6cc80c0a2544e2d072 on UltCombo:mm-optimize into 1f26389f0408ed33201c17f4658b882242fde881 on wearefractal:master.

UltCombo commented 9 years ago

(improved a comment & squashed)

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.1%) when pulling e7f66fe03c7a1e9ea41777bd59849e51355a42d1 on UltCombo:mm-optimize into 1f26389f0408ed33201c17f4658b882242fde881 on wearefractal:master.

UltCombo commented 9 years ago

Changed commit message.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.1%) when pulling 0f6ba0ac9400c8b5400986b0ba659eb259ab595f on UltCombo:mm-optimize into 1f26389f0408ed33201c17f4658b882242fde881 on wearefractal:master.

yocontra commented 9 years ago

Nice catch - thanks!

UltCombo commented 9 years ago

No problem, though, I believe it lost a bit of readability, seeing as negatives' glob property now holds either a RegExp or Minimatch instance, which is a little counterintuitive. Perhaps it would be cleaner if this patch was more similar to the one I submitted to globby (https://github.com/sindresorhus/globby/commit/e402d9fbb6d16f2c4f1464d8323fefd16116af04), that is, iterating over the negatives and creating a matcher property.