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

How to query directories? #80

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

webpro
Copy link

@webpro webpro commented Dec 16, 2024

When trying out tinyglobby, I noticed it doesn't return directories the way I'd expect. So I wonder whether using the single star like this is invalid syntax and expected behavior. Is there a way with different syntax and/or options to make the tests pass already?

Or should we try and make this happen? :)

Copy link

pkg-pr-new bot commented Dec 17, 2024

Open in Stackblitz

npm i https://pkg.pr.new/tinyglobby@80

commit: 882e3c7

@SuperchupuDev
Copy link
Owner

it's expected if it matches fast-glob. if it doesn't let me know and i'll fix it :)

@webpro
Copy link
Author

webpro commented Dec 17, 2024

I think the tests should pass when using onlyDirectories, fast-glob does this too:

My observations only consider directories, but should align for onlyFiles:

Also note that fast-glob does not suffix directories with /, whereas tinyglobby does. Personally I can appreciate tinyglobby's behavior, but it's good to know there's a diff.

@SuperchupuDev
Copy link
Owner

SuperchupuDev commented Dec 18, 2024

tested, it's a regression on main (works on 0.2.10). thanks for reporting! would have been chaotic if this bug made its way into a release

@SuperchupuDev
Copy link
Owner

SuperchupuDev commented Dec 18, 2024

looks like the bug isn't present in #76, so should be fixed once that gets merged

@webpro
Copy link
Author

webpro commented Dec 18, 2024

Splendid! Perhaps good to have tests to prevent regressions, the ones in this PR or others.

Let me know, I'd be happy to refactor or add more. Or close this PR.

@SuperchupuDev
Copy link
Owner

yep, will merge this once the bug is fixed :) can you refactor them so that they don't create a new fixture? should be possible to use the already created one

@webpro webpro force-pushed the feat/single-star-for-directories branch from 7adc53d to 882e3c7 Compare December 18, 2024 15:20
@webpro
Copy link
Author

webpro commented Dec 18, 2024

of course!

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

Successfully merging this pull request may close these issues.

2 participants