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

gitignore support #92

Open
benmccann opened this issue Feb 19, 2025 · 6 comments
Open

gitignore support #92

benmccann opened this issue Feb 19, 2025 · 6 comments
Labels
enhancement New feature or request

Comments

@benmccann
Copy link
Contributor

from nitrojs/nitro#3108 (comment):

it has a big difference from globby where it does not have integration with npm ignore to auto support gitignore and other repo ignore paths (gitignore patterns are not same as glob ignore patterns, and they are relative to git root)

@SuperchupuDev
Copy link
Owner

i feel like #32 could probably work for these use cases

@pi0
Copy link

pi0 commented Feb 19, 2025

Thanks for the issue and quick replies.

nagated support would be cool however gitignore and glob ignore patterns are different (gitignore patterns are absolute paths relative to git root like /test/* which need to be translated to <glob_base>/**, I assume there are be more diff like this)

For the context of my experience, With unjs/mkdist#265 I tried to workaround, however, regressions like unjs/unbuild#472 happened which finally had to do completely ignore gitignore story (unjs/mkdist#279)

If supporting gitignore feels out of this project scope, I can understand, however it feel like a really missing part to make tinyglobby full replacement of globby

@SuperchupuDev
Copy link
Owner

supporting a globby option is not out of the scope of this project, but i'm not sure how feasible it would be to implement it without adding any more dependencies (minimal dependencies is the other goal of this project). so i'm open to adding it as long as no new deps are introduced (even if the gitignore feature is used by many, it's still a really low % of usages compared to total globby usages (i have not verified this but i remember it being this way))

@Torathion
Copy link
Contributor

Torathion commented Feb 20, 2025

We were already talking about it in #70 . globby uses ignore for its gitignore option while also handling everything itself.

But it's essentially: check for option -> find gitignore file -> parse file content -> use ignore for each line inside the gitignore file.

@webpro
Copy link
Contributor

webpro commented Mar 6, 2025

From reading the docs, I wonder the ignore package handles this as expected: https://github.com/kaelzhang/node-ignore/tree/master?tab=readme-ov-file#2-filenames-and-dirnames

That's just not how gitignore patterns work. From https://git-scm.com/docs/gitignore:

If there is a separator at the end of the pattern then the pattern will only match directories, otherwise the pattern can match both files and directories.

I might be wrong, but from this I gather it wouldn't make sense to use the ignore package while claiming to support .gitignore files properly.

negated support would be cool however gitignore and glob ignore patterns are different (gitignore patterns are absolute paths relative to git root like /test/* which need to be translated to <glob_base>/**, I assume there are be more diff like this)

Agreed with this, insofar I think it should be translated to both <glob_base> and <glob_base>/** (ugly but we don't want to do fs ops).

For the record, Knip uses an implementation and some tests around this topic:

Nothing to be proud of, but does seem to do the job.

Just a shame there's no glob lib that supports negated ignore patterns 😬😬

supporting a globby option is not out of the scope of this project, but i'm not sure how feasible it would be to implement it without adding any more dependencies (minimal dependencies is the other goal of this project). so i'm open to adding it as long as no new deps are introduced (even if the gitignore feature is used by many, it's still a really low % of usages compared to total globby usages (i have not verified this but i remember it being this way))

Such numbers are hard to get by, but usage of gitignore: true is not zero (see e.g. ¹). But as long as tinyglobby is not a drop-in replacement (which is absolutely fine, I think we all can get behind tinyglobby's vision) it might be worth considering clarifying this in tinyglobby's docs. Because users often won't notice. Not sure how to call it, something with "ignorance" and "confirmation bias" on the user's end, idk.

¹ https://github.com/search?q=globby+%22gitignore%3A+true%22+%28language%3ATypeScript+OR+language%3AJavaScript%29&type=code

@webpro
Copy link
Contributor

webpro commented Mar 6, 2025

Maybe it's interesting to consider this approach: instead of adding the gitignore: boolean option, model it after Node.js' built-in fs.glob options.exclude function. And thus keep the complexity out of tinyglobby.

Then, we could do something like this (simplified):

import tinyglobby from 'tinyglobby';
import ignore from 'ignore';

const files = tinyglobby(["**/*.js"], {
  exclude: filePath => ignore(filePath)
});

There are still some concerns with this approach, I can think of at least:

  1. The API, valid options include:
    1. add exclude as a function (like fs.glob), or
    2. extend ignore to also accept a function
  2. Still requires a match lib that supports to "unignore" → i.e. like Support negated ignore patterns #70 with unmatch yet not with the internally created unignoreMatcher function, but using the user-provided function of the previous point 1. instead

The main idea is to basically pass on the exclude (or ignore) function to the underlying matcher lib.

This has the benefit that users can use node-ignore, something else, experiment with it, etc. It will also ease support for cq the migration to the Node.js built-in fs.glob.

WDYT?

I could try to whip up some PR + examples when I'll find some time. But only if there's interest and we have agreement on point 2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants