Skip to content

Commit 2cf9bed

Browse files
alexr00alexr00
authored
Fix hasTrailingPathSeparator for Windows drives (microsoft#73229)
* Fix hasTrailingPathSeparator for Windows drives * Add trailing slash utility and tests
1 parent b57b987 commit 2cf9bed

2 files changed

Lines changed: 44 additions & 8 deletions

File tree

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

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -176,28 +176,46 @@ export function isAbsolutePath(resource: URI): boolean {
176176
/**
177177
* Returns true if the URI path has a trailing path separator
178178
*/
179-
export function hasTrailingPathSeparator(resource: URI): boolean {
179+
export function hasTrailingPathSeparator(resource: URI, sep: string = paths.sep): boolean {
180180
if (resource.scheme === Schemas.file) {
181181
const fsp = originalFSPath(resource);
182-
return fsp.length > extpath.getRoot(fsp).length && fsp[fsp.length - 1] === paths.sep;
182+
return fsp.length > extpath.getRoot(fsp).length && fsp[fsp.length - 1] === sep;
183183
} else {
184184
const p = resource.path;
185185
return p.length > 1 && p.charCodeAt(p.length - 1) === CharCode.Slash; // ignore the slash at offset 0
186186
}
187187
}
188188

189-
190189
/**
191-
* Removes a trailing path seperator, if theres one.
190+
* Removes a trailing path separator, if there's one.
192191
* Important: Doesn't remove the first slash, it would make the URI invalid
193192
*/
194-
export function removeTrailingPathSeparator(resource: URI): URI {
195-
if (hasTrailingPathSeparator(resource)) {
193+
export function removeTrailingPathSeparator(resource: URI, sep: string = paths.sep): URI {
194+
if (hasTrailingPathSeparator(resource, sep)) {
196195
return resource.with({ path: resource.path.substr(0, resource.path.length - 1) });
197196
}
198197
return resource;
199198
}
200199

200+
/**
201+
* Adds a trailing path separator to the URI if there isn't one already.
202+
* For example, c:\ would be unchanged, but c:\users would become c:\users\
203+
*/
204+
export function addTrailingPathSeparator(resource: URI, sep: string = paths.sep): URI {
205+
let isRootSep: boolean = false;
206+
if (resource.scheme === Schemas.file) {
207+
const fsp = originalFSPath(resource);
208+
isRootSep = ((fsp !== undefined) && (fsp.length === extpath.getRoot(fsp).length) && (fsp[fsp.length - 1] === sep));
209+
} else {
210+
sep = '/';
211+
const p = resource.path;
212+
isRootSep = p.length === 1 && p.charCodeAt(p.length - 1) === CharCode.Slash;
213+
}
214+
if (!isRootSep && !hasTrailingPathSeparator(resource, sep)) {
215+
return resource.with({ path: resource.path + '/' });
216+
}
217+
return resource;
218+
}
201219

202220
/**
203221
* Returns a relative path between two URIs. If the URIs don't have the same schema or authority, `undefined` is returned.

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

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55
import * as assert from 'assert';
6-
import { dirname, basename, distinctParents, joinPath, isEqual, isEqualOrParent, hasToIgnoreCase, normalizePath, isAbsolutePath, relativePath, removeTrailingPathSeparator, hasTrailingPathSeparator, resolvePath } from 'vs/base/common/resources';
6+
import { dirname, basename, distinctParents, joinPath, isEqual, isEqualOrParent, hasToIgnoreCase, normalizePath, isAbsolutePath, relativePath, removeTrailingPathSeparator, hasTrailingPathSeparator, resolvePath, addTrailingPathSeparator } from 'vs/base/common/resources';
77
import { URI } from 'vs/base/common/uri';
88
import { isWindows } from 'vs/base/common/platform';
99
import { toSlashes } from 'vs/base/common/extpath';
@@ -178,6 +178,9 @@ suite('Resources', () => {
178178
assertEqualURI(removeTrailingPathSeparator(u1), expected, u1.toString());
179179
}
180180

181+
function assertAddTrailingSeparator(u1: URI, expected: URI) {
182+
assertEqualURI(addTrailingPathSeparator(u1), expected, u1.toString());
183+
}
181184

182185
test('trailingPathSeparator', () => {
183186
assertTrailingSeparator(URI.parse('foo://a/foo'), false);
@@ -190,6 +193,11 @@ suite('Resources', () => {
190193
assertRemoveTrailingSeparator(URI.parse('foo://a/'), URI.parse('foo://a/'));
191194
assertRemoveTrailingSeparator(URI.parse('foo://a'), URI.parse('foo://a'));
192195

196+
assertAddTrailingSeparator(URI.parse('foo://a/foo'), URI.parse('foo://a/foo/'));
197+
assertAddTrailingSeparator(URI.parse('foo://a/foo/'), URI.parse('foo://a/foo/'));
198+
assertAddTrailingSeparator(URI.parse('foo://a/'), URI.parse('foo://a/'));
199+
assertAddTrailingSeparator(URI.parse('foo://a'), URI.parse('foo://a/'));
200+
193201
if (isWindows) {
194202
assertTrailingSeparator(URI.file('c:\\a\\foo'), false);
195203
assertTrailingSeparator(URI.file('c:\\a\\foo\\'), true);
@@ -202,6 +210,12 @@ suite('Resources', () => {
202210
assertRemoveTrailingSeparator(URI.file('c:\\'), URI.file('c:\\'));
203211
assertRemoveTrailingSeparator(URI.file('\\\\server\\share\\some\\'), URI.file('\\\\server\\share\\some'));
204212
assertRemoveTrailingSeparator(URI.file('\\\\server\\share\\'), URI.file('\\\\server\\share\\'));
213+
214+
assertAddTrailingSeparator(URI.file('c:\\a\\foo'), URI.file('c:\\a\\foo\\'));
215+
assertAddTrailingSeparator(URI.file('c:\\a\\foo\\'), URI.file('c:\\a\\foo\\'));
216+
assertAddTrailingSeparator(URI.file('c:\\'), URI.file('c:\\'));
217+
assertAddTrailingSeparator(URI.file('\\\\server\\share\\some'), URI.file('\\\\server\\share\\some\\'));
218+
assertAddTrailingSeparator(URI.file('\\\\server\\share\\some\\'), URI.file('\\\\server\\share\\some\\'));
205219
} else {
206220
assertTrailingSeparator(URI.file('/foo/bar'), false);
207221
assertTrailingSeparator(URI.file('/foo/bar/'), true);
@@ -210,12 +224,16 @@ suite('Resources', () => {
210224
assertRemoveTrailingSeparator(URI.file('/foo/bar'), URI.file('/foo/bar'));
211225
assertRemoveTrailingSeparator(URI.file('/foo/bar/'), URI.file('/foo/bar'));
212226
assertRemoveTrailingSeparator(URI.file('/'), URI.file('/'));
227+
228+
assertAddTrailingSeparator(URI.file('/foo/bar'), URI.file('/foo/bar/'));
229+
assertAddTrailingSeparator(URI.file('/foo/bar/'), URI.file('/foo/bar/'));
230+
assertAddTrailingSeparator(URI.file('/'), URI.file('/'));
213231
}
214232
});
215233

216234
function assertEqualURI(actual: URI, expected: URI, message?: string) {
217235
if (!isEqual(expected, actual)) {
218-
assert.equal(expected.toString(), actual.toString(), message);
236+
assert.equal(actual.toString(), expected.toString(), message);
219237
}
220238
}
221239

0 commit comments

Comments
 (0)