search

SuperchupuDev / tinyglobby

A fast and minimal alternative to globby and fast-glob
https://npmjs.com/package/tinyglobby
MIT License
150 stars 9 forks source link

Support negated ignore patterns #70

Open webpro opened 3 weeks ago

webpro commented 3 weeks ago

Fixes #32

This PR adds support for negated ignore patterns. Please see #32 for more details and use cases.

This is a solution that should not impact performance at all if no negated ignore patterns are used. However, it does require a patched picomatch, and I'm not sure whether there's interest in a PR upstream. But before I'm even trying to go there I wanted to gauge interest here.

The idea/patch: we could leverage picomatch's onIgnore function to un-ignore if ignored matches also match a negated ignore pattern.

The problem: it requires to un-ignore matches within picomatch and this is a relatively low-impact solution for picomatch, but I'm afraid even requiring an explicit true (or false?) return value might still be a breaking change looking at the scale it's being used. Yet at the same time it could also actually mean a fix in certain cases and removing a major hurdle when looking to migrate from globby to tinyglobby.

This PR still needs some work:

pkg-pr-new[bot] commented 3 weeks ago

Open in Stackblitz

pnpm add https://pkg.pr.new/tinyglobby@70

commit: d81fc09

SuperchupuDev commented 3 weeks ago

oh, wow. first of all, thanks a lot for taking your time into implementing this, such a nice solution. how would patching picomatch work for users who download tinyglobby from npm? sadly, i don't think patching it is viable long term, but we can figure out something else or send a PR to picomatch

webpro commented 3 weeks ago

how would patching picomatch work for users who download tinyglobby from npm? sadly, i don't think patching it is viable long term

Oh no, bad idea indeed, this wasn't my intention at all

but we can figure out something else or send a PR to picomatch

Yes, this PR is mostly to keep the conversation going. And for you and others the ability to actually try it out e.g. when looking to migrate away from globby and see if this is a viable route.

Would you be interested in this solution given that picomatch would accept my PR? Tbh I don't fancy publishing (and maintaining) a fork of picomatch, so I'm not sure what other options we have here.

SuperchupuDev commented 3 weeks ago

i think i'm fine with anything that doesn't involve patching, whether it is a picomatch PR or something more complex

webpro commented 3 weeks ago

Sorry I phrased that poorly. With "this solution" I meant this PR without the patch part. Thanks! WIP.

SuperchupuDev commented 3 weeks ago

i think so? i think i can see some things that could be refactored (unsure though) and probably needs a small cleanup when that happens but other than that yes

webpro commented 3 weeks ago

Here's the PR: https://github.com/micromatch/picomatch/pull/137