Navigation Menu

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

{extension: false} and expandDirectories.extensions don't work together #97

Closed
garyking opened this issue Jan 6, 2019 · 11 comments
Closed
Labels
bug 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted

Comments

@garyking
Copy link
Contributor

garyking commented Jan 6, 2019

Issuehunt badges

If using noext: true (or extension: false, which is the same thing), and expandDirectories.extensions, then no results will be returned.

Example:

globby.sync(dirPath, {
  expandDirectories: {
    // Must have more than one extension here.
    extensions: ['md', 'txt'],
  },
  extension: false,
})

IssueHunt Summary

janat08 janat08 has been rewarded.

Backers (Total: $40.00)

Submitted pull Requests


Tips


IssueHunt has been backed by the following sponsors. Become a sponsor

@sindresorhus
Copy link
Owner

sindresorhus commented Jan 9, 2019

Would you be able to submit a pull request with one or more failing tests? That would help getting this fixed faster.

@garyking
Copy link
Contributor Author

garyking commented Jan 9, 2019

@sindresorhus Done

@sindresorhus sindresorhus changed the title extension/noext and expandDirectories.extensions don't work together {extension: false} and expandDirectories.extensions don't work together Feb 15, 2019
sindresorhus pushed a commit that referenced this issue Feb 15, 2019
@IssueHuntBot
Copy link

@IssueHunt has funded $40.00 to this issue. See it on IssueHunt

@medusalix
Copy link
Contributor

medusalix commented Feb 19, 2019

dir-glob is appending all extensions to the glob pattern: tmp\\**\\*.{md,tmp}. The option noext turns extglob support off, so fast-glob is unable to resolve the pattern.

@sindresorhus Should we just display a warning/error when both options are used simultaneously? That would of course need some kind of documentation.

@sindresorhus
Copy link
Owner

sindresorhus commented Feb 25, 2019

@medusalix According to the fast-glob docs, the noext options disables patterns like +(a|b)). dir-glob doesn't use that pattern syntax, it uses brace expansion, which is a different option nobrace.

@medusalix
Copy link
Contributor

medusalix commented Feb 25, 2019

@sindresorhus fast-glob uses micromatch, which turns tmp\\**\\*.{md,tmp} into tmp/**/*.(md|tmp) using brace expansion. When noext is specified, literal parentheses get matched:

it('should match literal parens when noext is true (issue #116)', function() {
    assert(mm.isMatch('a/(dir)', 'a/(dir)', {noext: true}));
});

@medusalix
Copy link
Contributor

@sindresorhus We could just throw an error if noext: true and expandDirectories.extensions are used together.

@sindresorhus
Copy link
Owner

That sounds like a leaked implementation detail and should be reported to fast-glob. Can you open an issue on fast-glob about this?

But yes, we should still throw an error for now until it's fixed in fast-glob.

@janat08
Copy link

janat08 commented May 2, 2019

the issue I opened isn't actionable apparently.

@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label May 10, 2019
@janat08
Copy link

janat08 commented May 15, 2019

mrmlnc/fast-glob#181

@issuehunt-oss issuehunt-oss bot added 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt and removed 💵 Funded on Issuehunt This issue has been funded on Issuehunt labels Jan 6, 2020
@issuehunt-oss
Copy link

issuehunt-oss bot commented Jan 6, 2020

@sindresorhus has rewarded $36.00 to @janat08. See it on IssueHunt

  • 💰 Total deposit: $40.00
  • 🎉 Repository reward(0%): $0.00
  • 🔧 Service fee(10%): $4.00

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants