From 124a368eeaa68dc78390d38b91616d307c28ca3b Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 13 May 2020 16:35:27 +0200 Subject: [PATCH] don't change/invent path when comparing uris, https://github.com/microsoft/vscode/issues/93368 --- src/vs/base/common/resources.ts | 36 +++++++++++++---------- src/vs/base/test/common/resources.test.ts | 10 ++++--- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/vs/base/common/resources.ts b/src/vs/base/common/resources.ts index e698ed3ac21..94de48d718e 100644 --- a/src/vs/base/common/resources.ts +++ b/src/vs/base/common/resources.ts @@ -27,22 +27,26 @@ function _hasToIgnoreCase(resource: URI | undefined): boolean { /** * Creates a key from a resource URI to be used to resource comparison and for resource maps. * - * **!!! This function is not compatible with URI.toString() !!!** + * @param resource Uri + * @param caseInsensitivePath Ignore casing when comparing path component (defaults mostly to `true`) + * @param ignoreFragment Ignore the fragment (defaults to `false`) */ -export function getComparisonKey(resource: URI, caseInsensitivePath = _hasToIgnoreCase(resource), ignoreFragment: boolean = false): string { - let path = resource.path || '/'; // VERY bogous as it changes the uri - if (caseInsensitivePath) { - path = path.toLowerCase(); - } - return resource.with({ path, fragment: ignoreFragment ? null : undefined }).toString(); +export function getComparisonKey(resource: URI, caseInsensitivePath: boolean = _hasToIgnoreCase(resource), ignoreFragment: boolean = false): string { + return resource.with({ + path: caseInsensitivePath ? resource.path.toLowerCase() : undefined, + fragment: ignoreFragment ? null : undefined + }).toString(); } /** - * Tests whether two resources are the same. + * Tests whether two uris are equal * - * **!!! This function is not compatible with uriA.toString() === uriB.toString() !!!** + * @param first Uri + * @param second Uri + * @param caseInsensitivePath Ignore casing when comparing path component (defaults mostly to `true`) + * @param ignoreFragment Ignore the fragment (defaults to `false`) */ -export function isEqual(first: URI | undefined, second: URI | undefined, caseInsensitivePath = _hasToIgnoreCase(first), ignoreFragment: boolean = false): boolean { +export function isEqual(first: URI | undefined, second: URI | undefined, caseInsensitivePath: boolean = _hasToIgnoreCase(first), ignoreFragment: boolean = false): boolean { if (first === second) { return true; } @@ -52,23 +56,25 @@ export function isEqual(first: URI | undefined, second: URI | undefined, caseIns if (first.scheme !== second.scheme || !isEqualAuthority(first.authority, second.authority)) { return false; } - const p1 = first.path || '/', p2 = second.path || '/'; + const p1 = first.path, p2 = second.path; return (p1 === p2 || caseInsensitivePath && equalsIgnoreCase(p1, p2)) && first.query === second.query && (ignoreFragment || first.fragment === second.fragment); } /** * Tests whether a `candidate` URI is a parent or equal of a given `base` URI. - * URI queries must match, fragments are ignored. + * * @param base A uri which is "longer" * @param parentCandidate A uri which is "shorter" then `base` + * @param caseInsensitivePath Ignore casing when comparing path component (defaults mostly to `true`) + * @param ignoreFragment Ignore the fragment (defaults to `false`) */ -export function isEqualOrParent(base: URI, parentCandidate: URI, ignoreCase = _hasToIgnoreCase(base), ignoreFragment: boolean = false): boolean { +export function isEqualOrParent(base: URI, parentCandidate: URI, caseInsensitivePath: boolean = _hasToIgnoreCase(base), ignoreFragment: boolean = false): boolean { if (base.scheme === parentCandidate.scheme) { if (base.scheme === Schemas.file) { - return extpath.isEqualOrParent(originalFSPath(base), originalFSPath(parentCandidate), ignoreCase) && base.query === parentCandidate.query && (ignoreFragment || base.fragment === parentCandidate.fragment); + return extpath.isEqualOrParent(originalFSPath(base), originalFSPath(parentCandidate), caseInsensitivePath) && base.query === parentCandidate.query && (ignoreFragment || base.fragment === parentCandidate.fragment); } if (isEqualAuthority(base.authority, parentCandidate.authority)) { - return extpath.isEqualOrParent(base.path || '/', parentCandidate.path || '/', ignoreCase, '/') && base.query === parentCandidate.query && (ignoreFragment || base.fragment === parentCandidate.fragment); + return extpath.isEqualOrParent(base.path, parentCandidate.path, caseInsensitivePath, '/') && base.query === parentCandidate.query && (ignoreFragment || base.fragment === parentCandidate.fragment); } } return false; diff --git a/src/vs/base/test/common/resources.test.ts b/src/vs/base/test/common/resources.test.ts index c0afcf399cd..2e2837f0011 100644 --- a/src/vs/base/test/common/resources.test.ts +++ b/src/vs/base/test/common/resources.test.ts @@ -254,14 +254,14 @@ suite('Resources', () => { assertRelativePath(URI.parse('foo://a/foo'), URI.parse('foo://a/foo/bar/goo'), 'bar/goo'); assertRelativePath(URI.parse('foo://a/'), URI.parse('foo://a/foo/bar/goo'), 'foo/bar/goo'); assertRelativePath(URI.parse('foo://a/foo/xoo'), URI.parse('foo://a/foo/bar'), '../bar'); - assertRelativePath(URI.parse('foo://a/foo/xoo/yoo'), URI.parse('foo://a'), '../../..'); + assertRelativePath(URI.parse('foo://a/foo/xoo/yoo'), URI.parse('foo://a'), '../../..', true); assertRelativePath(URI.parse('foo://a/foo'), URI.parse('foo://a/foo/'), ''); assertRelativePath(URI.parse('foo://a/foo/'), URI.parse('foo://a/foo'), ''); assertRelativePath(URI.parse('foo://a/foo/'), URI.parse('foo://a/foo/'), ''); assertRelativePath(URI.parse('foo://a/foo'), URI.parse('foo://a/foo'), ''); - assertRelativePath(URI.parse('foo://a'), URI.parse('foo://a'), ''); + assertRelativePath(URI.parse('foo://a'), URI.parse('foo://a'), '', true); assertRelativePath(URI.parse('foo://a/'), URI.parse('foo://a/'), ''); - assertRelativePath(URI.parse('foo://a/'), URI.parse('foo://a'), ''); + assertRelativePath(URI.parse('foo://a/'), URI.parse('foo://a'), '', true); assertRelativePath(URI.parse('foo://a/foo?q'), URI.parse('foo://a/foo/bar#h'), 'bar', true); assertRelativePath(URI.parse('foo://'), URI.parse('foo://a/b'), undefined); assertRelativePath(URI.parse('foo://a2/b'), URI.parse('foo://a/b'), undefined); @@ -372,7 +372,9 @@ suite('Resources', () => { assertIsEqual(fileURI, fileURI3, true, false); - assertIsEqual(URI.parse('foo://server'), URI.parse('foo://server/'), true, true); + assertIsEqual(URI.parse('file://server'), URI.parse('file://server/'), true, true); + assertIsEqual(URI.parse('http://server'), URI.parse('http://server/'), true, true); + assertIsEqual(URI.parse('foo://server'), URI.parse('foo://server/'), true, false); // only selected scheme have / as the default path assertIsEqual(URI.parse('foo://server/foo'), URI.parse('foo://server/foo/'), true, false); assertIsEqual(URI.parse('foo://server/foo'), URI.parse('foo://server/foo?'), true, true); -- GitLab