-
-
Notifications
You must be signed in to change notification settings - Fork 247
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
Backslashes as path-separators #11
Conversation
Landed on b6f01f5. Thanks! |
Published as 0.2.7, if you're looking to update anything. |
I'm confused about this change. Glob's docs say to only use forward slashes for doing matches; this change appears to have broken doing matches on Windows. |
@kylealanhale What behavior are you seeing? Do you have a test that fails? |
So, it turns out that I had added a funky hack to work around the bug that was fixed by the pull request. I removed my hack, but my tests were still failing, so I've had to figure out another work around. Let me see if this makes sense to you. I understand, and agree with using posix-style paths for patterns. However, I would think that the output of the glob would return paths in the format of the current environment. On posix systems, everything is fine, but for Windows I'm seeing all sorts of combinations. I haven't put together a test suite on this yet, but here's an illustration of what I'm seeing. Let's say I have a folder that looks like this:
I would expect the following commands to yield the given outputs: > glob.sync('/test/*')
[ 'C:\\test\\deep',
'C:\\test\\test.js',
'C:\\test\\test.txt' ]
> glob.sync('/test/**')
[ 'C:\\test',
'C:\\test\\deep',
'C:\\test\\deep\\test.js',
'C:\\test\\deep\\test.txt',
'C:\\test\\test.js',
'C:\\test\\test.txt' ]
> glob.sync('test/*', {cwd: 'C:\\'})
[ 'test\\deep',
'test\\test.js',
'test\\test.txt' ]
> glob.sync('test/**', {cwd: 'C:\\'})
[ 'test',
'test\\deep',
'test\\deep\\test.js',
'test\\deep\\test.txt',
'test\\test.js',
'test\\test.txt' ] Instead, I see: > glob.sync('/test/*')
[ '\\test/deep',
'\\test/test.js',
'\\test/test.txt' ]
> glob.sync('/test/**')
[ 'C:\\test',
'C:\\test\\deep',
'C:\\test\\test.js',
'C:\\test\\test.txt' ]
> glob.sync('test/*', {cwd: 'C:\\'})
[ 'test/deep',
'test/test.js',
'test/test.txt' ]
> glob.sync('test/**', {cwd: 'C:\\'})
[ 'test',
'test/deep',
'test/deep/test.js',
'test/deep/test.txt',
'test/test.js',
'test/test.txt' ] Note the anomalies:
So what I'm doing now is using that last one, and then re-prepending the leading slash, and passing it through Any insight into these inconsistencies? |
@isaacs Commit #b6f01f5 seems to be completely wrong! May be I don't understand something, but now we can't write a pattern that exact match filesystem entry process.platform='win32'; // dirty hack, testing purposes only
var mm = require('minimatch');
mm('[a]', '[a]'); // false as suspected
mm('[a]', '\\[a\\]'); // false, THAT BUG
mm('[a]', '?a?'); // true, but not exact match only [a] process.platform='linux'; // dirty hack, testing purposes only
var mm = require('minimatch');
mm('[a]', '[a]'); // false as suspected
mm('[a]', '\\[a\\]'); // true as suspected Please, revert this commit! |
Jake users on Windows are bumping into this.
Looks like slashes are already being normalized in Minimatch.prototype.match. I just used the same code, and duplicated the comment as well. Not really sure where the appropriate place would be to add a test for this, since it looks like from comments in basic.js that they likely don't run under Windows yet anyhow.