microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
163.28k stars 28.89k forks source link

Glob parser: Wrong result for '**/bower.json" #5419

Closed aeschli closed 8 years ago

aeschli commented 8 years ago

Steps to Reproduce:

In my extension I have a completionItemProvider that is registered like that:

    let jsonDocumentFilter = [{ language: 'json', pattern: '**∕bower.json' }];
    context.subscriptions.push(languages.registerCompletionItemProvider(jsonDocumentFilter, new JSONCompletionItemProvider(), '.', '$'));

The LanguageFeatureRegistry uses base/common/glob.ts: glob.match(filter.pattern, uri.fsPath)

globToRegExp creates the following regex for this: ^[^/\\]*?[^/\\]*?∕bower\.json$ which doesn't match a path like "/home/abc/workspaces/samples/languages/json/bower.json"

From debugging it seems the bug is in splitGlobAware:

        for (var i = 0; i < pattern.length; i++) {
            char = pattern[i];
            switch (char) {
                case splitChar:
                    if (!inBraces && !inBrackets) {
                        segments.push(curVal);
                        curVal = '';
                        continue;
                    }
                    break;

splitChar is "/", but the case clause is never reached, also not for the / character

I also wonder if this works on Windows as uri.fsPath returns a path with \

bpasero commented 8 years ago

@aeschli you seem to be using a character which is not the slash character in the glob pattern but rather this one: http://www.marathon-studios.com/unicode/U2215/Division_Slash

Works for me if I replace it with the proper '/'

aeschli commented 8 years ago

@bpasero Hm, very strange. I wonder how I ended up with that character. Sorry, I didn't realize this while debugging.

bpasero commented 8 years ago

@aeschli no worries, I found some spare slashes on my desktop, attaching here: / / / /

:]

aeschli commented 8 years ago

BTW, the regex is now ^(?:[\/\\]|[^\/\\]+[\/\\]|[\/\\][^\/\\]+)*?bower\.json$ which looks rather complicated. It seems to work but runs in a timeout on a long content (I tried with the default content of http://regexr.com/)

So its a ('separator' or 'everything but a separator followed by a separator' or a 'separator followed by a everything but a separator') repeated and optional. A very complicated way of saying /bower\.json$/

aeschli commented 8 years ago

I created #5438 for the complicated regex.