Skip to content

Commit bbb516c

Browse files
jriekenjrieken
committed
polish, extract util functions, fix issue with overeager hex detection
1 parent 67c8c94 commit bbb516c

3 files changed

Lines changed: 81 additions & 59 deletions

File tree

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

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,34 @@ export function compareIgnoreCase(a: string, b: string): number {
338338
}
339339
}
340340

341+
/**
342+
* [0-9]
343+
*/
344+
export function isAsciiDigit(code: number): boolean {
345+
return code >= CharCode.Digit0 && code <= CharCode.Digit9;
346+
}
347+
348+
/**
349+
* [a-f]
350+
*/
351+
export function isLowerAsciiHex(code: number): boolean {
352+
return code >= CharCode.a && code <= CharCode.f;
353+
}
354+
355+
/**
356+
* [A-F]
357+
*/
358+
export function isUpperAsciiHex(code: number): boolean {
359+
return code >= CharCode.A && code <= CharCode.F;
360+
}
361+
362+
/**
363+
* [0-9a-fA-F]
364+
*/
365+
export function isAsciiHex(code: number): boolean {
366+
return isAsciiDigit(code) || isLowerAsciiHex(code) || isUpperAsciiHex(code);
367+
}
368+
341369
export function isLowerAsciiLetter(code: number): boolean {
342370
return code >= CharCode.a && code <= CharCode.z;
343371
}
@@ -346,7 +374,7 @@ export function isUpperAsciiLetter(code: number): boolean {
346374
return code >= CharCode.A && code <= CharCode.Z;
347375
}
348376

349-
function isAsciiLetter(code: number): boolean {
377+
export function isAsciiLetter(code: number): boolean {
350378
return isLowerAsciiLetter(code) || isUpperAsciiLetter(code);
351379
}
352380

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

Lines changed: 22 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import { isWindows } from 'vs/base/common/platform';
77
import { CharCode } from 'vs/base/common/charCode';
8-
import { isHighSurrogate, isLowSurrogate } from 'vs/base/common/strings';
8+
import { isHighSurrogate, isLowSurrogate, isLowerAsciiHex, isAsciiLetter } from 'vs/base/common/strings';
99

1010
const _schemeRegExp = /^\w[\w\d+.-]*$/;
1111

@@ -207,10 +207,7 @@ export class URI implements UriComponents {
207207
* with URIs that represent files on disk (`file` scheme).
208208
*/
209209
get fsPath(): string {
210-
// if (this.scheme !== 'file') {
211-
// console.warn(`[UriError] calling fsPath with scheme ${this.scheme}`);
212-
// }
213-
return _makeFsPath(this);
210+
return _toFsPath(this.scheme, this.authority, this.path);
214211
}
215212

216213
// ---- modify to new -------------------------
@@ -287,7 +284,7 @@ export class URI implements UriComponents {
287284
percentDecode(query),
288285
percentDecode(fragment),
289286
);
290-
result._formatted = _toString(_normalEncoder, scheme, authority, path, query, fragment);
287+
result._external = _toExternal(_normalEncoder, scheme, authority, path, query, fragment);
291288
return result;
292289
}
293290

@@ -375,7 +372,7 @@ export class URI implements UriComponents {
375372
* @param skipEncoding Do not encode the result, default is `false`
376373
*/
377374
toString(skipEncoding: boolean = false): string {
378-
return _toString(skipEncoding ? _minimalEncoder : _normalEncoder, this.scheme, this.authority, this.path, this.query, this.fragment);
375+
return _toExternal(skipEncoding ? _minimalEncoder : _normalEncoder, this.scheme, this.authority, this.path, this.query, this.fragment);
379376
}
380377

381378
toJSON(): UriComponents {
@@ -393,7 +390,7 @@ export class URI implements UriComponents {
393390
return data;
394391
} else {
395392
const result = new _URI(data);
396-
result._formatted = (<UriState>data).external;
393+
result._external = (<UriState>data).external;
397394
result._fsPath = (<UriState>data)._sep === _pathSepMarker ? (<UriState>data).fsPath : null;
398395
return result;
399396
}
@@ -420,25 +417,25 @@ const _pathSepMarker = isWindows ? 1 : undefined;
420417
// tslint:disable-next-line:class-name
421418
class _URI extends URI {
422419

423-
_formatted: string | null = null;
420+
_external: string | null = null;
424421
_fsPath: string | null = null;
425422

426423
get fsPath(): string {
427424
if (!this._fsPath) {
428-
this._fsPath = _makeFsPath(this);
425+
this._fsPath = _toFsPath(this.scheme, this.authority, this.path);
429426
}
430427
return this._fsPath;
431428
}
432429

433430
toString(skipEncoding: boolean = false): string {
434431
if (skipEncoding) {
435432
// we don't cache that
436-
return _toString(_minimalEncoder, this.scheme, this.authority, this.path, this.query, this.fragment);
433+
return _toExternal(_minimalEncoder, this.scheme, this.authority, this.path, this.query, this.fragment);
437434
}
438-
if (!this._formatted) {
439-
this._formatted = _toString(_normalEncoder, this.scheme, this.authority, this.path, this.query, this.fragment);
435+
if (!this._external) {
436+
this._external = _toExternal(_normalEncoder, this.scheme, this.authority, this.path, this.query, this.fragment);
440437
}
441-
return this._formatted;
438+
return this._external;
442439
}
443440

444441
toJSON(): UriComponents {
@@ -450,8 +447,8 @@ class _URI extends URI {
450447
res.fsPath = this._fsPath;
451448
res._sep = _pathSepMarker;
452449
}
453-
if (this._formatted) {
454-
res.external = this._formatted;
450+
if (this._external) {
451+
res.external = this._external;
455452
}
456453
// uri components
457454
if (this.path) {
@@ -476,22 +473,22 @@ class _URI extends URI {
476473
/**
477474
* Compute `fsPath` for the given uri
478475
*/
479-
function _makeFsPath(uri: URI): string {
476+
function _toFsPath(scheme: string, authority: string, path: string): string {
480477

481478
let value: string;
482-
if (uri.authority && uri.path.length > 1 && uri.scheme === 'file') {
479+
if (authority && path.length > 1 && scheme === 'file') {
483480
// unc path: file://shares/c$/far/boo
484-
value = `//${uri.authority}${uri.path}`;
481+
value = `//${authority}${path}`;
485482
} else if (
486-
uri.path.charCodeAt(0) === CharCode.Slash
487-
&& (uri.path.charCodeAt(1) >= CharCode.A && uri.path.charCodeAt(1) <= CharCode.Z || uri.path.charCodeAt(1) >= CharCode.a && uri.path.charCodeAt(1) <= CharCode.z)
488-
&& uri.path.charCodeAt(2) === CharCode.Colon
483+
path.charCodeAt(0) === CharCode.Slash
484+
&& isAsciiLetter(path.charCodeAt(1))
485+
&& path.charCodeAt(2) === CharCode.Colon
489486
) {
490487
// windows drive letter: file:///c:/far/boo
491-
value = uri.path[1].toLowerCase() + uri.path.substr(2);
488+
value = path[1].toLowerCase() + path.substr(2);
492489
} else {
493490
// other path
494-
value = uri.path;
491+
value = path;
495492
}
496493
if (isWindows) {
497494
value = value.replace(_slashRegExp, '\\');
@@ -562,11 +559,6 @@ function isHashOrQuestionMark(code: number): boolean {
562559
return code === CharCode.Hash || code === CharCode.QuestionMark;
563560
}
564561

565-
function isLowerAsciiHex(code: number): boolean {
566-
return code >= CharCode.Digit0 && code <= CharCode.Digit9
567-
|| code >= CharCode.a && code <= CharCode.z;
568-
}
569-
570562
const _encodeTable: string[] = (function () {
571563
let table: string[] = [];
572564
for (let code = 0; code < 128; code++) {
@@ -639,7 +631,7 @@ const _driveLetterRegExp = /^(\/?[a-z])(:|%3a)/i;
639631
/**
640632
* Create the external version of a uri
641633
*/
642-
function _toString(encoder: { (code: number): boolean }[], scheme: string, authority: string, path: string, query: string, fragment: string): string {
634+
function _toExternal(encoder: { (code: number): boolean }[], scheme: string, authority: string, path: string, query: string, fragment: string): string {
643635

644636
let res = '';
645637
if (scheme) {

‎src/vs/base/test/node/uri.test.ts‎

Lines changed: 30 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ suite('URI', () => {
8888
assert.equal(uri2.fragment, uri3.fragment);
8989
});
9090

91-
test('with, identity', () => {
91+
test('URI#with, identity', () => {
9292
let uri = URI.parse('foo:bar/path');
9393

9494
let uri2 = uri.with(null!);
@@ -101,7 +101,7 @@ suite('URI', () => {
101101
assert.ok(uri === uri2);
102102
});
103103

104-
test('with, changes', () => {
104+
test('URI#with, changes', () => {
105105
assert.equal(URI.parse('before:some/file/path').with({ scheme: 'after' }).toString(), 'after:some/file/path');
106106
assert.equal(URI.from({ scheme: 's' }).with({ scheme: 'http', path: '/api/files/test.me', query: 't=1234' }).toString(), 'http:/api/files/test.me?t=1234');
107107
assert.equal(URI.from({ scheme: 's' }).with({ scheme: 'http', authority: '', path: '/api/files/test.me', query: 't=1234', fragment: '' }).toString(), 'http:/api/files/test.me?t=1234');
@@ -111,7 +111,7 @@ suite('URI', () => {
111111
assert.equal(URI.from({ scheme: 's' }).with({ scheme: 'boo', authority: '', path: '/api/files/test.me', query: 't=1234', fragment: '' }).toString(), 'boo:/api/files/test.me?t=1234');
112112
});
113113

114-
test('with, remove components #8465', () => {
114+
test('URI#with, remove components #8465', () => {
115115
assert.equal(URI.parse('scheme://authority/path').with({ authority: '' }).toString(), 'scheme:/path');
116116
assert.equal(URI.parse('scheme:/path').with({ authority: 'authority' }).with({ authority: '' }).toString(), 'scheme:/path');
117117
assert.equal(URI.parse('scheme:/path').with({ authority: 'authority' }).with({ authority: null }).toString(), 'scheme:/path');
@@ -121,7 +121,7 @@ suite('URI', () => {
121121
assert.equal(URI.parse('scheme:/path').with({ authority: null }).toString(), 'scheme:/path');
122122
});
123123

124-
test('with, validation', () => {
124+
test('URI#with, validation', () => {
125125
let uri = URI.parse('foo:bar/path');
126126
assert.throws(() => uri.with({ scheme: 'fai:l' }));
127127
assert.throws(() => uri.with({ scheme: 'fäil' }));
@@ -310,50 +310,52 @@ suite('URI', () => {
310310
assert.equal(value.toString(), 'file:///a.file');
311311
});
312312

313+
314+
function assertToString(input: string | URI, expected: string) {
315+
if (typeof input === 'string') {
316+
input = URI.parse(input);
317+
}
318+
const actual = input.toString();
319+
assert.equal(actual, expected.toString());
320+
}
321+
313322
test('URI.toString, only scheme and query', () => {
314-
const value = URI.parse('stuff:?qüery');
315-
assert.equal(value.toString(), 'stuff:?q%C3%BCery');
323+
assertToString('stuff:?qüery', 'stuff:?q%C3%BCery');
316324
});
317325

318326
test('URI#toString, upper-case percent espaces', () => {
319-
const value = URI.parse('file://sh%c3%a4res/path');
320-
assert.equal(value.toString(), 'file://sh%C3%A4res/path');
327+
assertToString('file://sh%c3%a4res/path', 'file://sh%C3%A4res/path');
328+
assertToString('file://sh%c3%z4res/path', 'file://sh%C3%z4res/path');
329+
assertToString('file:///sh%a0res/path', 'file:///sh%A0res/path'); // also upper-cased invalid sequence
321330
});
322331

323332
test('URI#toString, lower-case windows drive letter', () => {
324-
assert.equal(URI.parse('untitled:c:/Users/jrieken/Code/abc.txt').toString(), 'untitled:c%3A/Users/jrieken/Code/abc.txt');
325-
assert.equal(URI.parse('untitled:C:/Users/jrieken/Code/abc.txt').toString(), 'untitled:c%3A/Users/jrieken/Code/abc.txt');
333+
assertToString('untitled:c:/Users/jrieken/Code/abc.txt', 'untitled:c%3A/Users/jrieken/Code/abc.txt');
334+
assertToString('untitled:C:/Users/jrieken/Code/abc.txt', 'untitled:c%3A/Users/jrieken/Code/abc.txt');
326335
});
327336

328337
test('URI#toString, escape all the bits', () => {
329-
330338
const value = URI.file('/Users/jrieken/Code/_samples/18500/Mödel + Other Thîngß/model.js');
331-
assert.equal(value.toString(), 'file:///Users/jrieken/Code/_samples/18500/M%C3%B6del%20+%20Other%20Th%C3%AEng%C3%9F/model.js');
339+
assertToString(value, 'file:///Users/jrieken/Code/_samples/18500/M%C3%B6del%20+%20Other%20Th%C3%AEng%C3%9F/model.js');
332340
});
333341

334342
test('URI#toString, don\'t encode port', () => {
335343
let value = URI.parse('http://localhost:8080/far');
336-
assert.equal(value.toString(), 'http://localhost:8080/far');
344+
assertToString(value, 'http://localhost:8080/far');
337345

338346
value = URI.from({ scheme: 'http', authority: 'löcalhost:8080', path: '/far', query: undefined, fragment: undefined });
339-
assert.equal(value.toString(), 'http://l%C3%B6calhost:8080/far');
347+
assertToString(value, 'http://l%C3%B6calhost:8080/far');
340348
});
341349

342350
test('URI#toString, user information in authority', () => {
343-
let value = URI.parse('http://foo:bar@localhost/far');
344-
assert.equal(value.toString(), 'http://foo:bar@localhost/far');
345-
346-
value = URI.parse('http://foo@localhost/far');
347-
assert.equal(value.toString(), 'http://foo@localhost/far');
348-
349-
value = URI.parse('http://foo:bAr@localhost:8080/far');
350-
assert.equal(value.toString(), 'http://foo:bAr@localhost:8080/far');
351-
352-
value = URI.parse('http://foo@localhost:8080/far');
353-
assert.equal(value.toString(), 'http://foo@localhost:8080/far');
354-
355-
value = URI.from({ scheme: 'http', authority: 'föö:bör@löcalhost:8080', path: '/far', query: undefined, fragment: undefined });
356-
assert.equal(value.toString(), 'http://f%C3%B6%C3%B6:b%C3%B6r@l%C3%B6calhost:8080/far');
351+
assertToString('http://foo:bar@localhost/far', 'http://foo:bar@localhost/far');
352+
assertToString('http://foo@localhost/far', 'http://foo@localhost/far');
353+
assertToString('http://foo:bAr@localhost:8080/far', 'http://foo:bAr@localhost:8080/far');
354+
assertToString('http://foo@localhost:8080/far', 'http://foo@localhost:8080/far');
355+
assertToString(
356+
URI.from({ scheme: 'http', authority: 'föö:bör@löcalhost:8080', path: '/far', query: undefined, fragment: undefined }),
357+
'http://f%C3%B6%C3%B6:b%C3%B6r@l%C3%B6calhost:8080/far'
358+
);
357359
});
358360

359361
test('correctFileUriToFilePath2', () => {

0 commit comments

Comments
 (0)