Incorrect matching for parenthesis

[mrmlnc]

Source

• Difference behavior for ( and ) on windows and linux mrmlnc/fast-glob#160

Actual behavior

Now parenthesis are not escaped.

Expected behavior

Brackets are escaped or noextglob can affect on results. Works fine with minimatch and fnmatch (Python).

import fnmatch

fnmatch.translate('(se).txt')
// → \\(se\\)\\.txt\\Z(?ms)

Code sample

const minimatch = require('minimatch');
const picomatch = require('picomatch');

const stable = minimatch.makeRe('(special).txt');
const beta = picomatch.makeRe('(special).txt'); // noextglob has no effect here

console.log(stable); 
// → /^(?:\(special\)\.txt)$/
console.log(beta)
// → /^(?:(?=.)(special)\.txt)$/

[alexander-akait]

/cc @jonschlinkert can we fix it too? It is blocker (#8 too) for updating globby (which uses fast-glob in latest version) for copy-webpack-plugin, big thanks

[jonschlinkert]

Sorry for the late reply!

@mrmlnc that isn't a valid extglob. Exglobs must be preceeded by one of these characters: @!?+*. Picomatch treats non-extglobs parens as regex characters.

You can do the following to achieve what you want:

const picomatch = require('picomatch');
const correct = picomatch.makeRe('\\(special\\).txt'); // parentheses are not valid extglobs
console.log(correct);
//=> /^(?:\(special\)\.txt)$/

I think minimatch just escapes everything it doesn't recognize, which is a lot. But minimatch also is just plain wrong a lot of the time, I don't recommend using it as the "correct source". For example:

console.log(minimatch.makeRe('mu!(*(c))?.pa!(*(z))?'));
//=> /^(?:(?=.)mu(?:(?!(?:(?:c)*)[^\/]\.pa(?:(?!(?:(?:z)*)[^\/])[^\/]*?)[^\/])[^\/]*?)[^\/]\.pa(?:(?!(?:(?:z)*)[^\/])[^\/]*?)[^\/])$/

Notice the double generation of the pattern?

[jonschlinkert]

@mrmlnc @evilebottnawi ping.

Seems like this should be closed, correct?

[mrmlnc]

IMHO, the *match packages should work like Bash wildcards on all systems. Because we don't have any standard here. The following behavior applies to PowerShell on Windows.

mrmlnc@mrmlnc-w10:~$

$ ls
'qwe(q).txt'

$ ls qwe(q*
-bash: syntax error near unexpected token `('

$ ls qwe\(q*
'qwe(q).txt'

This is correct behaviour for Bash wildcards:

\ (backslash)
is used as an "escape" character, i.e. to protect a subsequent special character. Thus, \\ searches for a backslash. Note you may need to use quotation marks and backslash(es).

But… but after the minimatch package it confuses. Perhaps we can write about it explicitly in the documentation. Or provide an option that will work as a Q option in the grep-ack.

   -Q, --literal                 Quote all metacharacters; PATTERN is literal

[jonschlinkert]

@mrmlnc it seems like picomatch is doing what Bash does, and it sounds like you're saying that you agree that following Bash is what we should do. But then you say:

But… but after the minimatch package it confuses.

Minimatch doesn't handle non-extglob parentheses at all. It just escapes them.

Perhaps we can write about it explicitly in the documentation.

It is already mentioned here: https://github.com/micromatch/picomatch#matching-special-characters-as-literals

Or provide an option that will work as a Q option in the grep-ack.

I'm open to adding support for a literal option, but which characters would be treated literally? All of them, or just parentheses?

[mrmlnc]

My apologies for the delay. I think we can make a function that will escape all special characters.

Just a function that escapes all special characters in the pattern. Something similar: https://facelessuser.github.io/wcmatch/glob/#globescape

Also it can also be useful for the fast-glob package: mrmlnc/fast-glob#158

I can do a PR if you don't mind.

[jonschlinkert]

My apologies for the delay. I think we can make a function that will escape all special characters.

If you knew that all special characters should be escaped, why would you pass the pattern to picomatch at all?

Also it can also be useful for the fast-glob

Since you'd want to escape the file path before joining it to the glob, I can see how this would make more sense in fast-glob.