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.

Sign up for GitHub

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

Jump to bottom

Adopt path.join() over extpath.joinWithSlashes() #69101

Merged
merged 2 commits into from
Feb 21, 2019
Merged

Adopt path.join() over extpath.joinWithSlashes() #69101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
83de5de
fix #68927
bpasero Feb 21, 2019
0040f7b
fix tests
bpasero Feb 21, 2019
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
102 changes: 2 additions & 100 deletions src/vs/base/common/extpath.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,12 @@
import { isWindows } from 'vs/base/common/platform';
import { startsWithIgnoreCase, equalsIgnoreCase } from 'vs/base/common/strings';
import { CharCode } from 'vs/base/common/charCode';
import { sep, posix } from 'vs/base/common/path';
import { sep } from 'vs/base/common/path';

function isPathSeparator(code: number) {
return code === CharCode.Slash || code === CharCode.Backslash;
}

const _posixBadPath = /(\/\.\.?\/)|(\/\.\.?)$|^(\.\.?\/)|(\/\/+)|(\\)/;
const _winBadPath = /(\\\.\.?\\)|(\\\.\.?)$|^(\.\.?\\)|(\\\\+)|(\/)/;

function _isNormal(path: string, win: boolean): boolean {
return win
? !_winBadPath.test(path)
: !_posixBadPath.test(path);
}

/**
* Takes a Windows OS path and changes backward slashes to forward slashes.
* This should only be done for OS paths from Windows (or user provided paths potentially from Windows).
Expand All @@ -30,93 +21,6 @@ export function toSlashes(osPath: string) {
return osPath.replace(/[\\/]/g, '/');
}

export function normalizeWithSlashes(path: undefined): undefined;
export function normalizeWithSlashes(path: null): null;
export function normalizeWithSlashes(path: string): string;
export function normalizeWithSlashes(path: string | null | undefined): string | null | undefined {

if (path === null || path === undefined) {
return path;
}

const len = path.length;
if (len === 0) {
return '.';
}

if (_isNormal(path, false)) {
return path;
}

const sep = '/';
const root = getRoot(path, sep);

// skip the root-portion of the path
let start = root.length;
let skip = false;
let res = '';

for (let end = root.length; end <= len; end++) {

// either at the end or at a path-separator character
if (end === len || isPathSeparator(path.charCodeAt(end))) {

if (streql(path, start, end, '..')) {
// skip current and remove parent (if there is already something)
let prev_start = res.lastIndexOf(sep);
let prev_part = res.slice(prev_start + 1);
if ((root || prev_part.length > 0) && prev_part !== '..') {
res = prev_start === -1 ? '' : res.slice(0, prev_start);
skip = true;
}
} else if (streql(path, start, end, '.') && (root || res || end < len - 1)) {
// skip current (if there is already something or if there is more to come)
skip = true;
}

if (!skip) {
let part = path.slice(start, end);
if (res !== '' && res[res.length - 1] !== sep) {
res += sep;
}
res += part;
}
start = end + 1;
skip = false;
}
}

return root + res;
}

function streql(value: string, start: number, end: number, other: string): boolean {
return start + other.length === end && value.indexOf(other, start) === start;
}

export function joinWithSlashes(...parts: string[]): string {
let value = '';
for (let i = 0; i < parts.length; i++) {
let part = parts[i];
if (i > 0) {
// add the separater between two parts unless
// there already is one
let last = value.charCodeAt(value.length - 1);
if (!isPathSeparator(last)) {
let next = part.charCodeAt(0);
if (!isPathSeparator(next)) {
value += posix.sep;
}
}
}
value += part;
}

return normalizeWithSlashes(value);
}


// #region extpath

/**
* Computes the _root_ this path, like `getRoot('c:\files') === c:\`,
* `getRoot('files:///files/path') === files:///`,
Expand Down Expand Up @@ -323,6 +227,4 @@ export function isEqualOrParent(path: string, candidate: string, ignoreCase?: bo

export function isWindowsDriveLetter(char0: number): boolean {
return char0 >= CharCode.A && char0 <= CharCode.Z || char0 >= CharCode.a && char0 <= CharCode.z;
}

// #endregion
}
74 changes: 0 additions & 74 deletions src/vs/base/test/common/extpath.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,53 +15,6 @@ suite('Paths', () => {
assert.equal(extpath.toSlashes('/user/far'), '/user/far');
});


test('normalize', () => {
assert.equal(extpath.normalizeWithSlashes(''), '.');
assert.equal(extpath.normalizeWithSlashes('.'), '.');
assert.equal(extpath.normalizeWithSlashes('.'), '.');
assert.equal(extpath.normalizeWithSlashes('../../far'), '../../far');
assert.equal(extpath.normalizeWithSlashes('../bar'), '../bar');
assert.equal(extpath.normalizeWithSlashes('../far'), '../far');
assert.equal(extpath.normalizeWithSlashes('./'), './');
assert.equal(extpath.normalizeWithSlashes('./././'), './');
assert.equal(extpath.normalizeWithSlashes('./ff/./'), 'ff/');
assert.equal(extpath.normalizeWithSlashes('./foo'), 'foo');
assert.equal(extpath.normalizeWithSlashes('/'), '/');
assert.equal(extpath.normalizeWithSlashes('/..'), '/');
assert.equal(extpath.normalizeWithSlashes('///'), '/');
assert.equal(extpath.normalizeWithSlashes('//foo'), '/foo');
assert.equal(extpath.normalizeWithSlashes('//foo//'), '/foo/');
assert.equal(extpath.normalizeWithSlashes('/foo'), '/foo');
assert.equal(extpath.normalizeWithSlashes('/foo/bar.test'), '/foo/bar.test');
assert.equal(extpath.normalizeWithSlashes('\\\\\\'), '/');
assert.equal(extpath.normalizeWithSlashes('c:/../ff'), 'c:/ff');
assert.equal(extpath.normalizeWithSlashes('c:\\./'), 'c:/');
assert.equal(extpath.normalizeWithSlashes('foo/'), 'foo/');
assert.equal(extpath.normalizeWithSlashes('foo/../../bar'), '../bar');
assert.equal(extpath.normalizeWithSlashes('foo/./'), 'foo/');
assert.equal(extpath.normalizeWithSlashes('foo/./bar'), 'foo/bar');
assert.equal(extpath.normalizeWithSlashes('foo//'), 'foo/');
assert.equal(extpath.normalizeWithSlashes('foo//'), 'foo/');
assert.equal(extpath.normalizeWithSlashes('foo//bar'), 'foo/bar');
assert.equal(extpath.normalizeWithSlashes('foo//bar/far'), 'foo/bar/far');
assert.equal(extpath.normalizeWithSlashes('foo/bar/../../far'), 'far');
assert.equal(extpath.normalizeWithSlashes('foo/bar/../far'), 'foo/far');
assert.equal(extpath.normalizeWithSlashes('foo/far/../../bar'), 'bar');
assert.equal(extpath.normalizeWithSlashes('foo/far/../../bar'), 'bar');
assert.equal(extpath.normalizeWithSlashes('foo/xxx/..'), 'foo');
assert.equal(extpath.normalizeWithSlashes('foo/xxx/../bar'), 'foo/bar');
assert.equal(extpath.normalizeWithSlashes('foo/xxx/./..'), 'foo');
assert.equal(extpath.normalizeWithSlashes('foo/xxx/./../bar'), 'foo/bar');
assert.equal(extpath.normalizeWithSlashes('foo/xxx/./bar'), 'foo/xxx/bar');
assert.equal(extpath.normalizeWithSlashes('foo\\bar'), 'foo/bar');
assert.equal(extpath.normalizeWithSlashes(null), null);
assert.equal(extpath.normalizeWithSlashes(undefined), undefined);

// https://github.com/Microsoft/vscode/issues/7234
assert.equal(extpath.joinWithSlashes('/home/aeschli/workspaces/vscode/extensions/css', './syntaxes/css.plist'), '/home/aeschli/workspaces/vscode/extensions/css/syntaxes/css.plist');
});

test('getRoot', () => {

assert.equal(extpath.getRoot('/user/far'), '/');
Expand All @@ -79,33 +32,6 @@ suite('Paths', () => {

});

test('join', () => {
assert.equal(extpath.joinWithSlashes('.', 'bar'), 'bar');
assert.equal(extpath.joinWithSlashes('../../foo/bar', '../../foo'), '../../foo');
assert.equal(extpath.joinWithSlashes('../../foo/bar', '../bar/foo'), '../../foo/bar/foo');
assert.equal(extpath.joinWithSlashes('../foo/bar', '../bar/foo'), '../foo/bar/foo');
assert.equal(extpath.joinWithSlashes('/', 'bar'), '/bar');
assert.equal(extpath.joinWithSlashes('//server/far/boo', '../file.txt'), '//server/far/file.txt');
assert.equal(extpath.joinWithSlashes('/foo/', '/bar'), '/foo/bar');
assert.equal(extpath.joinWithSlashes('\\\\server\\far\\boo', '../file.txt'), '//server/far/file.txt');
assert.equal(extpath.joinWithSlashes('\\\\server\\far\\boo', './file.txt'), '//server/far/boo/file.txt');
assert.equal(extpath.joinWithSlashes('\\\\server\\far\\boo', '.\\file.txt'), '//server/far/boo/file.txt');
assert.equal(extpath.joinWithSlashes('\\\\server\\far\\boo', 'file.txt'), '//server/far/boo/file.txt');
assert.equal(extpath.joinWithSlashes('file:///c/users/test', 'test'), 'file:///c/users/test/test');
assert.equal(extpath.joinWithSlashes('file://localhost/c$/GitDevelopment/express', './settings'), 'file://localhost/c$/GitDevelopment/express/settings'); // unc
assert.equal(extpath.joinWithSlashes('file://localhost/c$/GitDevelopment/express', '.settings'), 'file://localhost/c$/GitDevelopment/express/.settings'); // unc
assert.equal(extpath.joinWithSlashes('foo', '/bar'), 'foo/bar');
assert.equal(extpath.joinWithSlashes('foo', 'bar'), 'foo/bar');
assert.equal(extpath.joinWithSlashes('foo', 'bar/'), 'foo/bar/');
assert.equal(extpath.joinWithSlashes('foo/', '/bar'), 'foo/bar');
assert.equal(extpath.joinWithSlashes('foo/', '/bar/'), 'foo/bar/');
assert.equal(extpath.joinWithSlashes('foo/', 'bar'), 'foo/bar');
assert.equal(extpath.joinWithSlashes('foo/bar', '../bar/foo'), 'foo/bar/foo');
assert.equal(extpath.joinWithSlashes('foo/bar', './bar/foo'), 'foo/bar/bar/foo');
assert.equal(extpath.joinWithSlashes('http://localhost/test', '../next'), 'http://localhost/next');
assert.equal(extpath.joinWithSlashes('http://localhost/test', 'test'), 'http://localhost/test/test');
});

test('isUNC', () => {
if (platform.isWindows) {
assert.ok(!extpath.isUNC('foo'));
Expand Down
5 changes: 2 additions & 3 deletions src/vs/workbench/contrib/debug/node/debugAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import * as stream from 'stream';
import * as nls from 'vs/nls';
import * as net from 'net';
import * as path from 'vs/base/common/path';
import * as extpath from 'vs/base/common/extpath';
import * as strings from 'vs/base/common/strings';
import * as objects from 'vs/base/common/objects';
import * as platform from 'vs/base/common/platform';
Expand Down Expand Up @@ -441,7 +440,7 @@ export class ExecutableDebugAdapter extends StreamDebugAdapter {
const result: IDebuggerContribution = Object.create(null);
if (contribution.runtime) {
if (contribution.runtime.indexOf('./') === 0) { // TODO
result.runtime = extpath.joinWithSlashes(extensionFolderPath, contribution.runtime);
result.runtime = path.join(extensionFolderPath, contribution.runtime);
} else {
result.runtime = contribution.runtime;
}
Expand All @@ -451,7 +450,7 @@ export class ExecutableDebugAdapter extends StreamDebugAdapter {
}
if (contribution.program) {
if (!path.isAbsolute(contribution.program)) {
result.program = extpath.joinWithSlashes(extensionFolderPath, contribution.program);
result.program = path.join(extensionFolderPath, contribution.program);
} else {
result.program = contribution.program;
}
Expand Down
6 changes: 3 additions & 3 deletions src/vs/workbench/contrib/debug/test/node/debugger.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import * as assert from 'assert';
import * as extpath from 'vs/base/common/extpath';
import { join, normalize } from 'vs/base/common/path';
import * as platform from 'vs/base/common/platform';
import { IDebugAdapterExecutable, IConfigurationManager, IConfig, IDebugSession } from 'vs/workbench/contrib/debug/common/debug';
import { Debugger } from 'vs/workbench/contrib/debug/node/debugger';
Expand Down Expand Up @@ -144,7 +144,7 @@ suite('Debug - Debugger', () => {

const ae = ExecutableDebugAdapter.platformAdapterExecutable([extensionDescriptor0], 'mock');

assert.equal(ae!.command, extpath.joinWithSlashes(extensionFolderPath, debuggerContribution.program));
assert.equal(ae!.command, join(extensionFolderPath, debuggerContribution.program));
assert.deepEqual(ae!.args, debuggerContribution.args);
});

Expand All @@ -166,7 +166,7 @@ suite('Debug - Debugger', () => {
const ae = ExecutableDebugAdapter.platformAdapterExecutable([extensionDescriptor1, extensionDescriptor2], 'mock')!;
assert.equal(ae.command, platform.isLinux ? 'linuxRuntime' : (platform.isMacintosh ? 'osxRuntime' : 'winRuntime'));
const xprogram = platform.isLinux ? 'linuxProgram' : (platform.isMacintosh ? 'osxProgram' : 'winProgram');
assert.deepEqual(ae.args, ['rarg', '/e2/b/c/' + xprogram, 'parg']);
assert.deepEqual(ae.args, ['rarg', normalize('/e2/b/c/') + xprogram, 'parg']);
});

test('initial config file content', () => {
Expand Down