Globstar returns its parent directory as a match when it should not

[e2tha-e]

When trying to match the contents of a parent directory, using a globstar returns the parent as a match. This does not abide by the behavior of globstars in BASH.

Environment

shell BASH
node v12.1.0
picomatch v2.0.7

Sample code

bash:

mkdir foo
touch foo/bar
ls foo/**

node:

const picomatch = require('picomatch');
const isMatch = picomatch('foo/**');
const foo = 'foo';
const bar = 'foo/bar';
console.log(foo, isMatch(foo));
console.log(bar, isMatch(bar));

Expected result

bash:

foo/bar

node:

foo false
foo/bar true

Actual result

bash:

foo/bar

node:

foo true
foo/bar true

[mrmlnc]

Looks like micromatch/micromatch#167.

[e2tha-e]

I agree. And it appears that micromatch/micromatch#167 is caused by micromatch's dependency on picomatch. If you think it will help the debugging effort, I can try and issue a pr.

[jonschlinkert]

Let me know what you think. I can solve this in picomatch in a couple of different ways:

Solution 1

On this line of code:

picomatch/lib/parse.js

Line 794 in 8ebe96d

prev.output = globstar(opts) + '|$)';

We can just make patterns that end in /** stricter by doing the following:

prev.output = globstar(opts) + ')';

Solution 2

Or, on that same line of code, we can make this optional by doing the following:

prev.output = globstar(opts) + (opts.strictSlashes ? ')' : '|$)');

---

FWIW, I think I implemented it this way based on previous discussions on micromatch, where the consensus was that foo/** should also match foo. However, I'm going to look for those discussions and see if I was remembering wrong... maybe the pattern was **/foo.

In the meantime, you can do the following to work around this:

const isMatch = picomatch('foo/**', { ignore: ['foo'] });

Not the most elegant solution, but there are tradeoffs either way. If we make foo/** strictly not match foo, then you'll need to do foo{,/**}, or ['foo', 'foo/**'], etc.

[mrmlnc]

I prefer current behavior. However, we previously chose Bash as the source.

Of these two options, I prefer the second, because this behavior is useful for my packages (previously, we check the original entry and entry with trailing slash for directories).

[poelstra]

My vote (if I would have any ;)) would be for the 'old' (micromatch v3) behavior, because it's less surprising.

I can imagine just changing it to Solution 1 will upset current v4 users though, so probably Solution 2 would be better.

If it is actually more of a 'bug' (accidental change since v3, rather), maybe the default value for strictSlashes could be changed to true (together with your Solution 2). That way, behavior is the same as v3 and bash, and people can explicitly set it to false to get opt-in to the 'convenience feature' of not needing `['foo', 'foo/**'].

[jonschlinkert]

maybe the default value for strictSlashes could be changed to true (together with your Solution 2). That way, behavior is the same as v3 and bash, and people can explicitly set it to false to get opt-in to the 'convenience feature' of not needing `['foo', 'foo/**'].

I'm inclined to agree with you. This seems like a regression since I mistakenly thought that we had decided that foo/** should match foo.

Given that, and based on this comment from @mrmlnc:

Of these two options, I prefer the second, because this behavior is useful for my packages (previously, we check the original entry and entry with trailing slash for directories).

I'm going to go with option 2, with strictSlashes as the default, unless I get any pushback. I'm open to more dialog on this if there are other considerations.

[mrmlnc]

Good. I will change the strictSlashes option value to false in the fast-glob package tomorrow.

[e2tha-e]

Good day all. With the latest release, setting strictSlashes to true has worked to produce the BASH-like globbing I was expecting. 👍

Just a thought: in a future release, it would be a good idea to have strictSlashes be true by default. (I had to explicitly set this option.) Or if that causes more unintended effects beyond the scope of the original post, implement a solution to limit those effects. In my case, I was upgrading from anymatch 2.x.x to anymatch 3.x.x and was taken by surprise to have BASH-like globbing off.