search

gulpjs / glob-stream

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

Breaking: Remove path normalization (closes #79) #84

Closed phated closed 7 years ago

phated commented 7 years ago

@contra @jonschlinkert @doowb @es128 if any of you are able to review this, I'd appreciate it.

Originally this PR was going to try to remove some dependencies by using an option I added to node-glob (absolute); however, it was breaking in some subtle ways and I couldn't get my PRs accepted to fix them. That being said, I was still able to streamline this module and remove the fact that we were converting glob paths to Windows-style paths by using path.normalize.

yocontra commented 7 years ago

Could also work it out if you forked and published our own glob impl, since dealing with Isaac seems to be a multi-year process for even minor fixes.

es128 commented 7 years ago

Keep in mind the last set of fixes for certain complex patterns remain unpublished in glob-parent while we wait for the windows auto-fix idea to be vetted.

I'm taking it from this that you do not intend to rely on the open PR in glob-parent in any case.

So that caveat aside, no concerns come to my mind with this PR.

jonschlinkert commented 7 years ago

@phated at a glance it looks good, I'll pull it down ASAP to review. should be able to tomorrow

phated commented 7 years ago

@contra funny enough, @jonschlinkert and I were discussing writing our own implementation from scratch (under glob-fs) using much of the work he's been doing on glob matching. Just need to find time to work on it and I don't know if it belongs in gulp 4.

@jonschlinkert thanks, I appreciate it.

doowb commented 7 years ago

@phated looks good. I pulled it down on windows and ran the tests and didn't run into any issues.

jonschlinkert commented 7 years ago

I found it! I'm so sorry guys, I totally blanked on where this was. My apologies if I was holding this up. I reviewed the code, it looks great. I also just ran this locally on mac and windows and had no issues. I always cringe before any path-related tests run on windows, but I saw lots of green this time lol. nice work!

es128 commented 7 years ago

FYI glob-parent with all the latest improvements has been published as 3.1.0

phated commented 7 years ago

@es128 thanks! I'm pretty sure I've already bumped to ^3 in the most recent publish.