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

Add 'allowWindowsEscape' option #103

Closed
wants to merge 1 commit into from
Closed

Conversation

frandiox
Copy link
Contributor

@isaacs This is a minimum PR that fixes #50 #64 isaacs/node-glob#212 and maybe something else.

Since node-glob passes the options object directly to minimatch it shouldn't be necessary to update node-glob. It won't break existing code either since by default this flag is undefined. Basically if this is set, the caller have to make sure the provided string does not contain \ for path separator, only for escaping characters.

@andipavllo
Copy link

+1

@litmit
Copy link
Contributor

litmit commented Feb 2, 2017

Glob pattern is a some kind of language. It SHOULD be one for any platform.
I'm the Windows developer and don't see any working solutions for Windows at all. Therefore i fork minimatch and now happy (https://github.com/litmit/minimatch/tree/match-for-windows). It works as needed!. Don't modify pattern at all and only convert the path separator in file paths.

@@ -709,7 +709,7 @@ function match (f, partial) {
var options = this.options

// windows: need to use /, not \
if (path.sep !== '/') {
if (!options.allowWindowsEscape && path.sep !== '/') {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should still be done. Even if we aren't forcing \ to be a path separator in the pattern, we still need to make the path we're matching use / characters.

@isaacs isaacs closed this in 0b5dea1 Feb 13, 2022
@isaacs
Copy link
Owner

isaacs commented Feb 13, 2022

This is a good step towards just not treating \ special in the pattern on Windows at all (ie, making this the only behavior starting with the next major after this.)

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 this pull request may close these issues.

Directories can contain square brakets
4 participants