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

Braced expansion match issue #108

Closed
sntran opened this issue Oct 16, 2017 · 3 comments
Closed

Braced expansion match issue #108

sntran opened this issue Oct 16, 2017 · 3 comments

Comments

@sntran
Copy link

sntran commented Oct 16, 2017

Please describe the minimum necessary steps to reproduce this issue:

> const mm = require('micromatch');
> mm(['./css/critical/header.css'], './css/critical{.css,**/*.css}')

What is happening (but shouldn't):

It returns an empty array.

Further more, running mm.isMatch('./css/critical/header.css', './css/critical{.css,**/*.css}') returns false.

What should be happening instead?

It should return ['./css/critical/header.css'], and .isMatch check above should return true.

@jonschlinkert
Copy link
Member

Why not use css/critical/**/*.css? The brace pattern you used is confusing, it would match css/critical.css.

Also, the leading ./ is probably causing a problem in the glob. Using ./ is only necessary for require(), everywhere else it's unnecessary.

@sntran
Copy link
Author

sntran commented Oct 16, 2017

Why not use css/critical/**/*.css? The brace pattern you used is confusing, it would match css/critical.css.

Because that is my intention. I want to include both "css/critical.css" and all the CSS files under "css/critical".

The leading ./ is there because my glob is given by outside user, which is used to Bash and the like. I can check and remove it, but this is something path module already handles, so I did not give much thought.

With that said, removing the leading ./ returns correct results. Please close this issue if you think that it should be handled by end user and not micromatch.

@jonschlinkert
Copy link
Member

Because that is my intention.

Got it, thanks for clarifying.

removing the leading ./ returns correct results

Thanks for confirming, I'm going to close this issue and open a new one. I think that should be handled.

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

No branches or pull requests

2 participants