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

resource globs - normalize drive letters #182259

Merged
merged 1 commit into from May 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/vs/base/common/extpath.ts
Expand Up @@ -325,8 +325,8 @@ export function hasDriveLetter(path: string, isWindowsOS: boolean = isWindows):
return false;
}

export function getDriveLetter(path: string): string | undefined {
return hasDriveLetter(path) ? path[0] : undefined;
export function getDriveLetter(path: string, isWindowsOS: boolean = isWindows): string | undefined {
return hasDriveLetter(path, isWindowsOS) ? path[0] : undefined;
}

export function indexOfPath(path: string, candidate: string, ignoreCase?: boolean): number {
Expand Down
32 changes: 30 additions & 2 deletions src/vs/workbench/common/resources.ts
Expand Up @@ -14,6 +14,7 @@ import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace
import { IConfigurationService, IConfigurationChangeEvent } from 'vs/platform/configuration/common/configuration';
import { Schemas } from 'vs/base/common/network';
import { ResourceSet } from 'vs/base/common/map';
import { getDriveLetter } from 'vs/base/common/extpath';

interface IConfiguredExpression {
readonly expression: IExpression;
Expand Down Expand Up @@ -130,9 +131,36 @@ export class ResourceGlobMatcher extends Disposable {
return undefined;
}

let hasAbsolutePath = false;

// Check the expression for absolute paths/globs
// and specifically for Windows, make sure the
// drive letter is lowercased, because we later
// check with `URI.fsPath` which is always putting
// the drive letter lowercased.

const massagedExpression: IExpression = Object.create(null);
for (const key of keys) {
if (!hasAbsolutePath) {
hasAbsolutePath = isAbsolute(key);
}

let massagedKey = key;

const driveLetter = getDriveLetter(massagedKey, true /* probe for windows */);
if (driveLetter) {
const driveLetterLower = driveLetter.toLowerCase();
if (driveLetter !== driveLetter.toLowerCase()) {
massagedKey = `${driveLetterLower}${massagedKey.substring(1)}`;
}
}

massagedExpression[massagedKey] = expression[key];
}

return {
expression,
hasAbsolutePath: keys.some(key => expression[key] === true && isAbsolute(key))
expression: massagedExpression,
hasAbsolutePath
};
}

Expand Down
17 changes: 14 additions & 3 deletions src/vs/workbench/test/common/resources.test.ts
Expand Up @@ -27,7 +27,7 @@ suite('ResourceGlobMatcher', () => {
});
});

test('Basics', () => {
test('Basics', async () => {
const matcher = new ResourceGlobMatcher(() => configurationService.getValue(SETTING), e => e.affectsConfiguration(SETTING), contextService, configurationService);

// Matching
Expand All @@ -39,18 +39,29 @@ suite('ResourceGlobMatcher', () => {
let eventCounter = 0;
matcher.onExpressionChange(() => eventCounter++);

configurationService.setUserConfiguration(SETTING, { '**/*.foo': true });
await configurationService.setUserConfiguration(SETTING, { '**/*.foo': true });
configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: (key: string) => key === SETTING } as any);
assert.equal(eventCounter, 1);

assert.equal(matcher.matches(URI.file('/foo/bar.md')), false);
assert.equal(matcher.matches(URI.file('/foo/bar.foo')), true);

configurationService.setUserConfiguration(SETTING, undefined);
await configurationService.setUserConfiguration(SETTING, undefined);
configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: (key: string) => key === SETTING } as any);
assert.equal(eventCounter, 2);

assert.equal(matcher.matches(URI.file('/foo/bar.md')), false);
assert.equal(matcher.matches(URI.file('/foo/bar.foo')), false);

await configurationService.setUserConfiguration(SETTING, {
'**/*.md': true,
'**/*.txt': false,
'C:/bar/**': true,
'/bar/**': true
});
configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: (key: string) => key === SETTING } as any);

assert.equal(matcher.matches(URI.file('/bar/foo.1')), true);
assert.equal(matcher.matches(URI.file('C:/bar/foo.1')), true);
});
});