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

Negation not working when pattern begins with ! #62

Closed
forty8bits opened this issue Mar 12, 2013 · 11 comments · Fixed by #108
Closed

Negation not working when pattern begins with ! #62

forty8bits opened this issue Mar 12, 2013 · 11 comments · Fixed by #108

Comments

@forty8bits
Copy link

Beginning a pattern with ! seems to be broken. The result actually matches the non-negated pattern... so for example:

glob('!package.json', function(err, res)...

returns res as ['package.json'], whereas

glob('./!(package.json)', function(err, res)...

returns an array containing all files/directories in the cwd except package.json, as expected.

@isaacs
Copy link
Owner

isaacs commented Apr 12, 2013

Yep, this is a bug.

@isaacs
Copy link
Owner

isaacs commented Apr 12, 2013

It's not property respecting the this.minimatch.negate flag, which should flip all the comparisons.

@kschzt
Copy link

kschzt commented May 31, 2013

@isaacs something like !(node_modules) should also list the entire tree, except for node_modules. This case is being incorrectly switched to https://github.com/isaacs/node-glob/blob/master/glob.js#L335 .. It then tries to stat '(node_modules)', which doesn't exist, and returns nothing. Easy to catch via https://github.com/isaacs/node-glob/blob/master/test/00-setup.js#L103

@sindresorhus
Copy link
Contributor

👍

2 similar comments
@passy
Copy link

passy commented Nov 11, 2013

👍

@SBoudrias
Copy link

👍

AluisioASG added a commit to AluisioASG/multiglob that referenced this issue Mar 5, 2014
Work around isaacs/node-glob#62 by matching the whole tree first.
@sheerun
Copy link

sheerun commented Apr 12, 2014

So what about it?

@isaacs
Copy link
Owner

isaacs commented Apr 12, 2014

Patch welcome.

@bcardarella
Copy link
Contributor

@isaacs I believe I have found the cause of the issue but need guidance on how best to fix.

https://github.com/isaacs/minimatch/blob/master/minimatch.js#L237-L242
In minimatch: for any pattern that does not begin with a "!" the for loop is never run so the pattern is never marked as "negated". During the parse function the pattern is iterated over for any !s and will stringSub ^ to negate that pattern: https://github.com/isaacs/minimatch/blob/master/minimatch.js#L553

parse never uses the negate flag to flip the pattern. This is why ./(!whatever) works but !(whatever) doesn't.

There could be two solutions:

  1. The easiest is to just remove parseNegate all together. This might be ideal as the parse function seems to be better about where to negate. the makeRe function doesn't seem to be used, but it does respect the negate flag: https://github.com/isaacs/minimatch/blob/master/minimatch.js#L806. So makeRe could be removed
  2. Refactor parse to use makeRe and respect the negate flag.

Thoughts on how to approach? I can work on the fix either way.

@bcardarella
Copy link
Contributor

@isaacs I'm going to try to implement options 2 as the README seems to imply that makeRe is supposed to be used anyway.

@bcardarella
Copy link
Contributor

Although now I'm confused. Upon further inspection this commit: isaacs/minimatch@b458f5d is from 2 and half years ago is when makeRe was removed from use in favor of parse. The commit message only mentions a "rewrite". Not really sure which is the best path to take. It seems that parse and makeRe are duplicating effort

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 a pull request may close this issue.

8 participants