Skip to content

Commit 124a368

Browse files
jriekenjrieken
committed
don't change/invent path when comparing uris, microsoft#93368 
1 parent 1e6b94d commit 124a368

2 files changed

Lines changed: 27 additions & 19 deletions

File tree

‎src/vs/base/common/resources.ts‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,26 @@ function _hasToIgnoreCase(resource: URI | undefined): boolean {
2727
/**
2828
* Creates a key from a resource URI to be used to resource comparison and for resource maps.
2929
*
30-
* **!!! This function is not compatible with URI.toString() !!!**
30+
* @param resource Uri
31+
* @param caseInsensitivePath Ignore casing when comparing path component (defaults mostly to `true`)
32+
* @param ignoreFragment Ignore the fragment (defaults to `false`)
3133
*/
32-
export function getComparisonKey(resource: URI, caseInsensitivePath = _hasToIgnoreCase(resource), ignoreFragment: boolean = false): string {
33-
let path = resource.path || '/'; // VERY bogous as it changes the uri
34-
if (caseInsensitivePath) {
35-
path = path.toLowerCase();
36-
}
37-
return resource.with({ path, fragment: ignoreFragment ? null : undefined }).toString();
34+
export function getComparisonKey(resource: URI, caseInsensitivePath: boolean = _hasToIgnoreCase(resource), ignoreFragment: boolean = false): string {
35+
return resource.with({
36+
path: caseInsensitivePath ? resource.path.toLowerCase() : undefined,
37+
fragment: ignoreFragment ? null : undefined
38+
}).toString();
3839
}
3940

4041
/**
41-
* Tests whether two resources are the same.
42+
* Tests whether two uris are equal
4243
*
43-
* **!!! This function is not compatible with uriA.toString() === uriB.toString() !!!**
44+
* @param first Uri
45+
* @param second Uri
46+
* @param caseInsensitivePath Ignore casing when comparing path component (defaults mostly to `true`)
47+
* @param ignoreFragment Ignore the fragment (defaults to `false`)
4448
*/
45-
export function isEqual(first: URI | undefined, second: URI | undefined, caseInsensitivePath = _hasToIgnoreCase(first), ignoreFragment: boolean = false): boolean {
49+
export function isEqual(first: URI | undefined, second: URI | undefined, caseInsensitivePath: boolean = _hasToIgnoreCase(first), ignoreFragment: boolean = false): boolean {
4650
if (first === second) {
4751
return true;
4852
}
@@ -52,23 +56,25 @@ export function isEqual(first: URI | undefined, second: URI | undefined, caseIns
5256
if (first.scheme !== second.scheme || !isEqualAuthority(first.authority, second.authority)) {
5357
return false;
5458
}
55-
const p1 = first.path || '/', p2 = second.path || '/';
59+
const p1 = first.path, p2 = second.path;
5660
return (p1 === p2 || caseInsensitivePath && equalsIgnoreCase(p1, p2)) && first.query === second.query && (ignoreFragment || first.fragment === second.fragment);
5761
}
5862

5963
/**
6064
* Tests whether a `candidate` URI is a parent or equal of a given `base` URI.
61-
* URI queries must match, fragments are ignored.
65+
*
6266
* @param base A uri which is "longer"
6367
* @param parentCandidate A uri which is "shorter" then `base`
68+
* @param caseInsensitivePath Ignore casing when comparing path component (defaults mostly to `true`)
69+
* @param ignoreFragment Ignore the fragment (defaults to `false`)
6470
*/
65-
export function isEqualOrParent(base: URI, parentCandidate: URI, ignoreCase = _hasToIgnoreCase(base), ignoreFragment: boolean = false): boolean {
71+
export function isEqualOrParent(base: URI, parentCandidate: URI, caseInsensitivePath: boolean = _hasToIgnoreCase(base), ignoreFragment: boolean = false): boolean {
6672
if (base.scheme === parentCandidate.scheme) {
6773
if (base.scheme === Schemas.file) {
68-
return extpath.isEqualOrParent(originalFSPath(base), originalFSPath(parentCandidate), ignoreCase) && base.query === parentCandidate.query && (ignoreFragment || base.fragment === parentCandidate.fragment);
74+
return extpath.isEqualOrParent(originalFSPath(base), originalFSPath(parentCandidate), caseInsensitivePath) && base.query === parentCandidate.query && (ignoreFragment || base.fragment === parentCandidate.fragment);
6975
}
7076
if (isEqualAuthority(base.authority, parentCandidate.authority)) {
71-
return extpath.isEqualOrParent(base.path || '/', parentCandidate.path || '/', ignoreCase, '/') && base.query === parentCandidate.query && (ignoreFragment || base.fragment === parentCandidate.fragment);
77+
return extpath.isEqualOrParent(base.path, parentCandidate.path, caseInsensitivePath, '/') && base.query === parentCandidate.query && (ignoreFragment || base.fragment === parentCandidate.fragment);
7278
}
7379
}
7480
return false;

‎src/vs/base/test/common/resources.test.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -254,14 +254,14 @@ suite('Resources', () => {
254254
assertRelativePath(URI.parse('foo://a/foo'), URI.parse('foo://a/foo/bar/goo'), 'bar/goo');
255255
assertRelativePath(URI.parse('foo://a/'), URI.parse('foo://a/foo/bar/goo'), 'foo/bar/goo');
256256
assertRelativePath(URI.parse('foo://a/foo/xoo'), URI.parse('foo://a/foo/bar'), '../bar');
257-
assertRelativePath(URI.parse('foo://a/foo/xoo/yoo'), URI.parse('foo://a'), '../../..');
257+
assertRelativePath(URI.parse('foo://a/foo/xoo/yoo'), URI.parse('foo://a'), '../../..', true);
258258
assertRelativePath(URI.parse('foo://a/foo'), URI.parse('foo://a/foo/'), '');
259259
assertRelativePath(URI.parse('foo://a/foo/'), URI.parse('foo://a/foo'), '');
260260
assertRelativePath(URI.parse('foo://a/foo/'), URI.parse('foo://a/foo/'), '');
261261
assertRelativePath(URI.parse('foo://a/foo'), URI.parse('foo://a/foo'), '');
262-
assertRelativePath(URI.parse('foo://a'), URI.parse('foo://a'), '');
262+
assertRelativePath(URI.parse('foo://a'), URI.parse('foo://a'), '', true);
263263
assertRelativePath(URI.parse('foo://a/'), URI.parse('foo://a/'), '');
264-
assertRelativePath(URI.parse('foo://a/'), URI.parse('foo://a'), '');
264+
assertRelativePath(URI.parse('foo://a/'), URI.parse('foo://a'), '', true);
265265
assertRelativePath(URI.parse('foo://a/foo?q'), URI.parse('foo://a/foo/bar#h'), 'bar', true);
266266
assertRelativePath(URI.parse('foo://'), URI.parse('foo://a/b'), undefined);
267267
assertRelativePath(URI.parse('foo://a2/b'), URI.parse('foo://a/b'), undefined);
@@ -372,7 +372,9 @@ suite('Resources', () => {
372372

373373
assertIsEqual(fileURI, fileURI3, true, false);
374374

375-
assertIsEqual(URI.parse('foo://server'), URI.parse('foo://server/'), true, true);
375+
assertIsEqual(URI.parse('file://server'), URI.parse('file://server/'), true, true);
376+
assertIsEqual(URI.parse('http://server'), URI.parse('http://server/'), true, true);
377+
assertIsEqual(URI.parse('foo://server'), URI.parse('foo://server/'), true, false); // only selected scheme have / as the default path
376378
assertIsEqual(URI.parse('foo://server/foo'), URI.parse('foo://server/foo/'), true, false);
377379
assertIsEqual(URI.parse('foo://server/foo'), URI.parse('foo://server/foo?'), true, true);
378380

0 commit comments

Comments
 (0)