Skip to content

Commit 1967cd3

Browse files
author
Benjamin Pasero
committed
Improve format of workspace paths when writing
- fixes microsoft#33270 - fixes microsoft#33328
1 parent e58b856 commit 1967cd3

12 files changed

Lines changed: 200 additions & 116 deletions

File tree

‎src/vs/editor/standalone/browser/simpleServices.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,7 @@ export class SimpleWorkspaceContextService implements IWorkspaceContextService {
535535

536536
constructor() {
537537
const resource = URI.from({ scheme: SimpleWorkspaceContextService.SCHEME, authority: 'model', path: '/' });
538-
this.workspace = { id: '4064f6ec-cb38-4ad0-af64-ee6467e63c82', folders: [{ uri: resource, raw: resource.toString(), name: '', index: 0, }], name: resource.fsPath };
538+
this.workspace = { id: '4064f6ec-cb38-4ad0-af64-ee6467e63c82', folders: [{ uri: resource, raw: { path: resource.toString() }, name: '', index: 0, }], name: resource.fsPath };
539539
}
540540

541541
public getWorkspace(): IWorkspace {

‎src/vs/platform/workspace/common/workspace.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ export interface WorkspaceFolder {
120120
/**
121121
* The raw path of this workspace folder
122122
*/
123-
readonly raw: string;
123+
readonly raw: IStoredWorkspaceFolder;
124124
}
125125

126126
export class Workspace implements IWorkspace {
@@ -190,12 +190,12 @@ export function toWorkspaceFolders(configuredFolders: IStoredWorkspaceFolder[],
190190
}
191191

192192
function parseWorkspaceFolders(configuredFolders: IStoredWorkspaceFolder[], relativeTo: URI): WorkspaceFolder[] {
193-
return configuredFolders.map(({ path, name }, index) => {
194-
const uri = toUri(path, relativeTo);
193+
return configuredFolders.map((configuredFolder, index) => {
194+
const uri = toUri(configuredFolder.path, relativeTo);
195195
if (!uri) {
196196
return void 0;
197197
}
198-
return { uri, raw: path, index, name };
198+
return { uri, raw: configuredFolder, index, name: configuredFolder.name };
199199
});
200200
}
201201

‎src/vs/platform/workspace/test/common/workspace.test.ts‎

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ suite('Workspace', () => {
5151

5252
assert.equal(actual.length, 1);
5353
assert.equal(actual[0].uri.fsPath, '/src/test');
54-
assert.equal(actual[0].raw, '/src/test');
54+
assert.equal(actual[0].raw.path, '/src/test');
5555
assert.equal(actual[0].index, 0);
5656
assert.equal(actual[0].name, 'test');
5757
});
@@ -61,7 +61,7 @@ suite('Workspace', () => {
6161

6262
assert.equal(actual.length, 1);
6363
assert.equal(actual[0].uri.fsPath, '/src/test');
64-
assert.equal(actual[0].raw, './test');
64+
assert.equal(actual[0].raw.path, './test');
6565
assert.equal(actual[0].index, 0);
6666
assert.equal(actual[0].name, 'test');
6767
});
@@ -71,7 +71,7 @@ suite('Workspace', () => {
7171

7272
assert.equal(actual.length, 1);
7373
assert.equal(actual[0].uri.fsPath, '/src/test');
74-
assert.equal(actual[0].raw, '/src/test');
74+
assert.equal(actual[0].raw.path, '/src/test');
7575
assert.equal(actual[0].index, 0);
7676
assert.equal(actual[0].name, 'hello');
7777
});
@@ -81,17 +81,17 @@ suite('Workspace', () => {
8181

8282
assert.equal(actual.length, 3);
8383
assert.equal(actual[0].uri.fsPath, '/src/test2');
84-
assert.equal(actual[0].raw, '/src/test2');
84+
assert.equal(actual[0].raw.path, '/src/test2');
8585
assert.equal(actual[0].index, 0);
8686
assert.equal(actual[0].name, 'test2');
8787

8888
assert.equal(actual[1].uri.fsPath, '/src/test3');
89-
assert.equal(actual[1].raw, '/src/test3');
89+
assert.equal(actual[1].raw.path, '/src/test3');
9090
assert.equal(actual[1].index, 1);
9191
assert.equal(actual[1].name, 'test3');
9292

9393
assert.equal(actual[2].uri.fsPath, '/src/test1');
94-
assert.equal(actual[2].raw, '/src/test1');
94+
assert.equal(actual[2].raw.path, '/src/test1');
9595
assert.equal(actual[2].index, 2);
9696
assert.equal(actual[2].name, 'test1');
9797
});
@@ -101,17 +101,17 @@ suite('Workspace', () => {
101101

102102
assert.equal(actual.length, 3);
103103
assert.equal(actual[0].uri.fsPath, '/src/test2');
104-
assert.equal(actual[0].raw, '/src/test2');
104+
assert.equal(actual[0].raw.path, '/src/test2');
105105
assert.equal(actual[0].index, 0);
106106
assert.equal(actual[0].name, 'test2');
107107

108108
assert.equal(actual[1].uri.fsPath, '/src/test3');
109-
assert.equal(actual[1].raw, '/src/test3');
109+
assert.equal(actual[1].raw.path, '/src/test3');
110110
assert.equal(actual[1].index, 1);
111111
assert.equal(actual[1].name, 'noName');
112112

113113
assert.equal(actual[2].uri.fsPath, '/src/test1');
114-
assert.equal(actual[2].raw, '/src/test1');
114+
assert.equal(actual[2].raw.path, '/src/test1');
115115
assert.equal(actual[2].index, 2);
116116
assert.equal(actual[2].name, 'test1');
117117
});
@@ -121,17 +121,17 @@ suite('Workspace', () => {
121121

122122
assert.equal(actual.length, 3);
123123
assert.equal(actual[0].uri.fsPath, '/src/test2');
124-
assert.equal(actual[0].raw, '/src/test2');
124+
assert.equal(actual[0].raw.path, '/src/test2');
125125
assert.equal(actual[0].index, 0);
126126
assert.equal(actual[0].name, 'test2');
127127

128128
assert.equal(actual[1].uri.fsPath, '/abc/test3');
129-
assert.equal(actual[1].raw, '/abc/test3');
129+
assert.equal(actual[1].raw.path, '/abc/test3');
130130
assert.equal(actual[1].index, 1);
131131
assert.equal(actual[1].name, 'noName');
132132

133133
assert.equal(actual[2].uri.fsPath, '/src/test1');
134-
assert.equal(actual[2].raw, './test1');
134+
assert.equal(actual[2].raw.path, './test1');
135135
assert.equal(actual[2].index, 2);
136136
assert.equal(actual[2].name, 'test1');
137137
});
@@ -141,12 +141,12 @@ suite('Workspace', () => {
141141

142142
assert.equal(actual.length, 2);
143143
assert.equal(actual[0].uri.fsPath, '/src/test2');
144-
assert.equal(actual[0].raw, '/src/test2');
144+
assert.equal(actual[0].raw.path, '/src/test2');
145145
assert.equal(actual[0].index, 0);
146146
assert.equal(actual[0].name, 'test2');
147147

148148
assert.equal(actual[1].uri.fsPath, '/src/test1');
149-
assert.equal(actual[1].raw, '/src/test1');
149+
assert.equal(actual[1].raw.path, '/src/test1');
150150
assert.equal(actual[1].index, 1);
151151
assert.equal(actual[1].name, 'test1');
152152
});
@@ -156,17 +156,17 @@ suite('Workspace', () => {
156156

157157
assert.equal(actual.length, 3);
158158
assert.equal(actual[0].uri.fsPath, '/src/test2');
159-
assert.equal(actual[0].raw, '/src/test2');
159+
assert.equal(actual[0].raw.path, '/src/test2');
160160
assert.equal(actual[0].index, 0);
161161
assert.equal(actual[0].name, 'test2');
162162

163163
assert.equal(actual[1].uri.fsPath, '/src/test3');
164-
assert.equal(actual[1].raw, '/src/test3');
164+
assert.equal(actual[1].raw.path, '/src/test3');
165165
assert.equal(actual[1].index, 1);
166166
assert.equal(actual[1].name, 'noName');
167167

168168
assert.equal(actual[2].uri.fsPath, '/abc/test1');
169-
assert.equal(actual[2].raw, '/abc/test1');
169+
assert.equal(actual[2].raw.path, '/abc/test1');
170170
assert.equal(actual[2].index, 2);
171171
assert.equal(actual[2].name, 'test1');
172172
});
@@ -176,23 +176,25 @@ suite('Workspace', () => {
176176

177177
assert.equal(actual.length, 3);
178178
assert.equal(actual[0].uri.fsPath, '/src/test2');
179-
assert.equal(actual[0].raw, '/src/test2');
179+
assert.equal(actual[0].raw.path, '/src/test2');
180180
assert.equal(actual[0].index, 0);
181181
assert.equal(actual[0].name, 'test2');
182182

183183
assert.equal(actual[1].uri.fsPath, '/src/test3');
184-
assert.equal(actual[1].raw, './test3');
184+
assert.equal(actual[1].raw.path, './test3');
185185
assert.equal(actual[1].index, 1);
186186
assert.equal(actual[1].name, 'test3');
187187

188188
assert.equal(actual[2].uri.fsPath, '/abc/test1');
189-
assert.equal(actual[2].raw, '/abc/test1');
189+
assert.equal(actual[2].raw.path, '/abc/test1');
190190
assert.equal(actual[2].index, 2);
191191
assert.equal(actual[2].name, 'test1');
192192
});
193193

194194
function aWorkspaceFolder(uri: URI, index: number = 0): WorkspaceFolder {
195-
return { uri, raw: uri.fsPath, index, name: '' };
195+
return {
196+
uri, raw: { path: uri.fsPath }, index, name: ''
197+
};
196198
}
197199

198200
});

‎src/vs/platform/workspaces/electron-main/workspacesMainService.ts‎

Lines changed: 5 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,20 @@ import { IWorkspacesMainService, IWorkspaceIdentifier, IStoredWorkspace, WORKSPA
99
import { TPromise } from 'vs/base/common/winjs.base';
1010
import { isParent } from 'vs/platform/files/common/files';
1111
import { IEnvironmentService } from 'vs/platform/environment/common/environment';
12-
import { extname, join, dirname, isAbsolute, resolve, relative } from 'path';
12+
import { extname, join, dirname, isAbsolute, resolve } from 'path';
1313
import { mkdirp, writeFile, readFile } from 'vs/base/node/pfs';
1414
import { readFileSync, writeFileSync, existsSync, mkdirSync } from 'fs';
15-
import { isLinux, isMacintosh, isWindows } from 'vs/base/common/platform';
15+
import { isLinux, isMacintosh } from 'vs/base/common/platform';
1616
import { delSync, readdirSync } from 'vs/base/node/extfs';
1717
import Event, { Emitter } from 'vs/base/common/event';
1818
import { ILogService } from 'vs/platform/log/common/log';
19-
import { isEqual, isEqualOrParent, normalize } from 'vs/base/common/paths';
19+
import { isEqual } from 'vs/base/common/paths';
2020
import { coalesce } from 'vs/base/common/arrays';
2121
import { createHash } from 'crypto';
2222
import * as json from 'vs/base/common/json';
2323
import * as jsonEdit from 'vs/base/common/jsonEdit';
2424
import { applyEdit } from 'vs/base/common/jsonFormatter';
25-
import { normalizeDriveLetter } from 'vs/base/common/labels';
26-
27-
const SLASH = '/';
25+
import { massageFolderPathForWorkspace } from 'vs/platform/workspaces/node/workspaces';
2826

2927
export class WorkspacesMainService implements IWorkspacesMainService {
3028

@@ -198,18 +196,6 @@ export class WorkspacesMainService implements IWorkspacesMainService {
198196
const sourceConfigFolder = dirname(workspace.configPath);
199197
const targetConfigFolder = dirname(targetConfigPath);
200198

201-
// Determine which path separator to use:
202-
// - macOS/Linux: slash
203-
// - Windows: use slash if already used in that file
204-
let useSlashesForPath = !isWindows;
205-
if (isWindows) {
206-
storedWorkspace.folders.forEach(folder => {
207-
if (folder.path.indexOf(SLASH) >= 0) {
208-
useSlashesForPath = true;
209-
}
210-
});
211-
}
212-
213199
// Rewrite absolute paths to relative paths if the target workspace folder
214200
// is a parent of the location of the workspace file itself. Otherwise keep
215201
// using absolute paths.
@@ -218,24 +204,7 @@ export class WorkspacesMainService implements IWorkspacesMainService {
218204
folder.path = resolve(sourceConfigFolder, folder.path); // relative paths get resolved against the workspace location
219205
}
220206

221-
if (isEqualOrParent(folder.path, targetConfigFolder, !isLinux)) {
222-
folder.path = relative(targetConfigFolder, folder.path) || '.'; // absolute paths get converted to relative ones to workspace location if possible
223-
}
224-
225-
// Windows gets special treatment:
226-
// - normalize all paths to get nice casing of drive letters
227-
// - convert to slashes if we want to use slashes for paths
228-
if (isWindows) {
229-
if (isAbsolute(folder.path)) {
230-
if (useSlashesForPath) {
231-
folder.path = normalize(folder.path, false /* do not use OS path separator */);
232-
}
233-
234-
folder.path = normalizeDriveLetter(folder.path);
235-
} else if (useSlashesForPath) {
236-
folder.path = folder.path.replace(/[\\]/g, SLASH);
237-
}
238-
}
207+
folder.path = massageFolderPathForWorkspace(folder.path, targetConfigFolder, storedWorkspace.folders);
239208
});
240209

241210
// Preserve as much of the existing workspace as possible by using jsonEdit

‎src/vs/platform/workspaces/node/workspaces.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
'use strict';
7+
8+
import { IStoredWorkspaceFolder } from 'vs/platform/workspaces/common/workspaces';
9+
import { isWindows, isLinux } from 'vs/base/common/platform';
10+
import { isAbsolute, relative } from 'path';
11+
import { isEqualOrParent, normalize } from 'vs/base/common/paths';
12+
import { normalizeDriveLetter } from 'vs/base/common/labels';
13+
14+
const SLASH = '/';
15+
16+
/**
17+
* Given the absolute path to a folder, massage it in a way that it fits
18+
* into an existing set of workspace folders of a workspace.
19+
*
20+
* @param absoluteFolderPath the absolute path of a workspace folder
21+
* @param targetConfigFolder the folder where the workspace is living in
22+
* @param existingFolders a set of existing folders of the workspace
23+
*/
24+
export function massageFolderPathForWorkspace(absoluteFolderPath: string, targetConfigFolder: string, existingFolders: IStoredWorkspaceFolder[]): string {
25+
const useSlashesForPath = shouldUseSlashForPath(existingFolders);
26+
27+
// Convert path to relative path if the target config folder
28+
// is a parent of the path.
29+
if (isEqualOrParent(absoluteFolderPath, targetConfigFolder, !isLinux)) {
30+
absoluteFolderPath = relative(targetConfigFolder, absoluteFolderPath) || '.';
31+
}
32+
33+
// Windows gets special treatment:
34+
// - normalize all paths to get nice casing of drive letters
35+
// - convert to slashes if we want to use slashes for paths
36+
if (isWindows) {
37+
if (isAbsolute(absoluteFolderPath)) {
38+
if (useSlashesForPath) {
39+
absoluteFolderPath = normalize(absoluteFolderPath, false /* do not use OS path separator */);
40+
}
41+
42+
absoluteFolderPath = normalizeDriveLetter(absoluteFolderPath);
43+
} else if (useSlashesForPath) {
44+
absoluteFolderPath = absoluteFolderPath.replace(/[\\]/g, SLASH);
45+
}
46+
}
47+
48+
return absoluteFolderPath;
49+
}
50+
51+
function shouldUseSlashForPath(storedFolders: IStoredWorkspaceFolder[]): boolean {
52+
53+
// Determine which path separator to use:
54+
// - macOS/Linux: slash
55+
// - Windows: use slash if already used in that file
56+
let useSlashesForPath = !isWindows;
57+
if (isWindows) {
58+
storedFolders.forEach(folder => {
59+
if (!useSlashesForPath && folder.path.indexOf(SLASH) >= 0) {
60+
useSlashesForPath = true;
61+
}
62+
});
63+
}
64+
65+
return useSlashesForPath;
66+
}

‎src/vs/platform/workspaces/test/electron-main/workspacesMainService.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,32 @@ suite('WorkspacesMainService', () => {
271271
});
272272
});
273273

274+
test('saveWorkspace (saved workspace, preserves forward slashes)', done => {
275+
return service.createWorkspace([process.cwd(), os.tmpdir(), path.join(os.tmpdir(), 'somefolder')]).then(workspace => {
276+
const workspaceConfigPath = path.join(os.tmpdir(), `myworkspace.${Date.now()}.${WORKSPACE_EXTENSION}`);
277+
const newWorkspaceConfigPath = path.join(os.tmpdir(), `mySavedWorkspace.${Date.now()}.${WORKSPACE_EXTENSION}`);
278+
279+
return service.saveWorkspace(workspace, workspaceConfigPath).then(savedWorkspace => {
280+
const contents = fs.readFileSync(savedWorkspace.configPath).toString();
281+
fs.writeFileSync(savedWorkspace.configPath, contents.replace(/[\\]/g, '/')); // convert backslash to slash
282+
283+
return service.saveWorkspace(savedWorkspace, newWorkspaceConfigPath).then(newSavedWorkspace => {
284+
assert.ok(newSavedWorkspace.id);
285+
assert.notEqual(newSavedWorkspace.id, workspace.id);
286+
assert.equal(newSavedWorkspace.configPath, newWorkspaceConfigPath);
287+
288+
const ws = JSON.parse(fs.readFileSync(newSavedWorkspace.configPath).toString()) as IStoredWorkspace;
289+
assert.ok(ws.folders.every(f => f.path.indexOf('\\') < 0));
290+
291+
extfs.delSync(workspaceConfigPath);
292+
extfs.delSync(newWorkspaceConfigPath);
293+
294+
done();
295+
});
296+
});
297+
});
298+
});
299+
274300
test('deleteUntitledWorkspaceSync (untitled)', done => {
275301
return service.createWorkspace([process.cwd(), os.tmpdir()]).then(workspace => {
276302
assert.ok(fs.existsSync(workspace.configPath));

‎src/vs/workbench/services/configuration/test/node/configuration.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ suite('WorkspaceContextService - Folder', () => {
100100
assert.equal(actual.folders[0].uri.fsPath, workspaceResource);
101101
assert.equal(actual.folders[0].name, workspaceName);
102102
assert.equal(actual.folders[0].index, 0);
103-
assert.equal(actual.folders[0].raw, workspaceResource);
103+
assert.equal(actual.folders[0].raw.path, workspaceResource);
104104
assert.ok(!actual.configuration);
105105
});
106106

‎src/vs/workbench/services/files/test/node/fileService.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ suite('FileService', () => {
3838
return onError(error, done);
3939
}
4040

41-
service = new FileService(new TestContextService(new Workspace(testDir, testDir, [{ uri: uri.file(testDir), raw: testDir, index: 0, name: '' }])), new TestTextResourceConfigurationService(), new TestConfigurationService(), { disableWatcher: true });
41+
service = new FileService(new TestContextService(new Workspace(testDir, testDir, [{ uri: uri.file(testDir), raw: { path: testDir }, index: 0, name: '' }])), new TestTextResourceConfigurationService(), new TestConfigurationService(), { disableWatcher: true });
4242
done();
4343
});
4444
});
@@ -853,11 +853,11 @@ suite('FileService', () => {
853853
});
854854
});
855855

856-
function aWorkspaceFolder(raw: string, index: number, name: string = ''): WorkspaceFolder {
856+
function aWorkspaceFolder(path: string, index: number, name: string = ''): WorkspaceFolder {
857857
return {
858-
uri: uri.file(raw),
858+
uri: uri.file(path),
859859
index,
860-
raw,
860+
raw: { path: path },
861861
name
862862
};
863863
}

0 commit comments

Comments
 (0)