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

Ability to avoid autoreplace \\ in patterns even on Windows #24

Closed
mrmlnc opened this issue Sep 5, 2019 · 3 comments
Closed

Ability to avoid autoreplace \\ in patterns even on Windows #24

mrmlnc opened this issue Sep 5, 2019 · 3 comments

Comments

@mrmlnc
Copy link
Contributor

mrmlnc commented Sep 5, 2019

I have one pattern that uses escaping for parenthesis:

file-\\(suffix\\).md

As a result of the work I get:

globParent('file-\\(suffix\\).md')
// file-

// expected result: .

Obviously, this is an incorrect parent directory. Yeap, I understand that this is described in the documentation, but I find it difficult to get all users to use the new format. Also I see no reason to change user input inside my package, because it can lead to problems.

Maybe we can add an option to control automatic replacement?

glob-parent/index.js

Lines 15 to 17 in d497548

if (isWin32 && str.indexOf(slash) < 0) {
str = str.replace(backslash, slash);
}

Found this issue in mrmlnc/fast-glob#223. The fast-glob package only accepts patterns with forward slashes.

@phated
Copy link
Member

phated commented Sep 5, 2019

This is definitely something we can look into. How would you feel about adding an options object and an option to disable that? I'm thinking the option makes sense so we can have backwards compatibility but maybe it's time to drop the replacement hack. I'm not sure.

@mrmlnc
Copy link
Contributor Author

mrmlnc commented Sep 8, 2019

IMHO, I would prefer to remove this hack (glob patterns by design works only with forward slashes). However, this is a major change for gulp users.

I can create a PR with with these changes (without major changes):

mrmlnc@a326e3b

@phated
Copy link
Member

phated commented Sep 12, 2019

@mrmlnc looks great! Please send a PR. I will release a minor version with that change and then we can drop the hack all together in a major.

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