Relative paths in .code-workspace files are turned into absolute ones on save (fixes #33188)

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

@@ -9,15 +9,14 @@ import { IWorkspacesMainService, IWorkspaceIdentifier, IStoredWorkspace, WORKSPA
 import { TPromise } from 'vs/base/common/winjs.base';
 import { isParent } from 'vs/platform/files/common/files';
 import { IEnvironmentService } from 'vs/platform/environment/common/environment';
-import { extname, join, dirname, isAbsolute, resolve } from 'path';
+import { extname, join, dirname, isAbsolute, resolve, relative } from 'path';
 import { mkdirp, writeFile, readFile } from 'vs/base/node/pfs';
 import { readFileSync, writeFileSync, existsSync, mkdirSync } from 'fs';
 import { isLinux } from 'vs/base/common/platform';
-import { copy, delSync, readdirSync } from 'vs/base/node/extfs';
-import { nfcall } from 'vs/base/common/async';
+import { delSync, readdirSync } from 'vs/base/node/extfs';
 import Event, { Emitter } from 'vs/base/common/event';
 import { ILogService } from 'vs/platform/log/common/log';
-import { isEqual } from 'vs/base/common/paths';
+import { isEqual, isEqualOrParent } from 'vs/base/common/paths';
 import { coalesce } from 'vs/base/common/arrays';
 import { createHash } from 'crypto';
 import URI from 'vs/base/common/uri';
@@ -77,20 +76,13 @@ export class WorkspacesMainService implements IWorkspacesMainService {
 
 	private doResolveWorkspace(path: string, contents: string): IResolvedWorkspace {
 		try {
-			const rawWorkspace = JSON.parse(contents);
-
-			const workspace = rawWorkspace as IStoredWorkspace;
-			if (!Array.isArray(workspace.folders) || workspace.folders.length === 0) {
-				this.logService.log(`${path} looks like an invalid workspace file.`);
-
-				return null; // looks like an invalid workspace file
-			}
+			const workspace = this.doParseStoredWorkspace(path, contents);
 
 			// TODO@Ben migration
-			const legacyStoredWorkspace = rawWorkspace as ILegacyStoredWorkspace;
+			const legacyStoredWorkspace = (<any>workspace) as ILegacyStoredWorkspace;
 			if (legacyStoredWorkspace.folders.some(folder => typeof folder === 'string')) {
-				(rawWorkspace as IStoredWorkspace).folders = legacyStoredWorkspace.folders.map(folder => ({ path: URI.parse(folder).fsPath }));
-				writeFileSync(path, JSON.stringify(rawWorkspace, null, '\t'));
+				workspace.folders = legacyStoredWorkspace.folders.map(folder => ({ path: URI.parse(folder).fsPath }));
+				writeFileSync(path, JSON.stringify(workspace, null, '\t'));
 			}
 
 			let absoluteFolders: IStoredWorkspaceFolder[] = [];
@@ -110,10 +102,25 @@ export class WorkspacesMainService implements IWorkspacesMainService {
 				folders: absoluteFolders
 			};
 		} catch (error) {
-			this.logService.log(`${path} cannot be parsed as JSON file (${error}).`);
+			this.logService.log(error.toString());
+		}
+
+		return null;
+	}
+
+	private doParseStoredWorkspace(path: string, contents: string): IStoredWorkspace {
+		let storedWorkspace: IStoredWorkspace;
+		try {
+			storedWorkspace = JSON.parse(contents);
+		} catch (error) {
+			throw new Error(`${path} cannot be parsed as JSON file (${error}).`);
+		}
 
-			return null; // unable to read or parse as workspace file
+		if (!Array.isArray(storedWorkspace.folders) || storedWorkspace.folders.length === 0) {
+			throw new Error(`${path} looks like an invalid workspace file.`);
 		}
+
+		return storedWorkspace;
 	}
 
 	private isInsideWorkspacesHome(path: string): boolean {
@@ -175,24 +182,49 @@ export class WorkspacesMainService implements IWorkspacesMainService {
 		return this.isInsideWorkspacesHome(workspace.configPath);
 	}
 
-	public saveWorkspace(workspace: IWorkspaceIdentifier, target: string): TPromise<IWorkspaceIdentifier> {
+	public saveWorkspace(workspace: IWorkspaceIdentifier, targetConfigPath: string): TPromise<IWorkspaceIdentifier> {
 
 		// Return early if target is same as source
-		if (isEqual(workspace.configPath, target, !isLinux)) {
+		if (isEqual(workspace.configPath, targetConfigPath, !isLinux)) {
 			return TPromise.as(workspace);
 		}
 
-		// Copy to new target
-		return nfcall(copy, workspace.configPath, target).then(() => {
-			const savedWorkspaceIdentifier = { id: this.getWorkspaceId(target), configPath: target };
+		// Read the contents of the workspace file and resolve it
+		return readFile(workspace.configPath).then(rawWorkspaceContents => {
+			let storedWorkspace: IStoredWorkspace;
+			try {
+				storedWorkspace = this.doParseStoredWorkspace(workspace.configPath, rawWorkspaceContents.toString());
+			} catch (error) {
+				return TPromise.wrapError(error);
+			}
+
+			const sourceConfigFolder = dirname(workspace.configPath);
+			const targetConfigFolder = dirname(targetConfigPath);
+
+			// Rewrite absolute paths to relative paths if the target workspace folder
+			// is a parent of the location of the workspace file itself. Otherwise keep
+			// using absolute paths.
+			storedWorkspace.folders.forEach(folder => {
+				if (!isAbsolute(folder.path)) {
+					folder.path = resolve(sourceConfigFolder, folder.path); // relative paths get resolved against the workspace location
+				}
+
+				if (isEqualOrParent(folder.path, targetConfigFolder, !isLinux)) {
+					folder.path = relative(targetConfigFolder, folder.path); // absolute paths get converted to relative ones to workspace location if possible
+				}
+			});
 
-			// Event
-			this._onWorkspaceSaved.fire({ workspace: savedWorkspaceIdentifier, oldConfigPath: workspace.configPath });
+			return writeFile(targetConfigPath, JSON.stringify(storedWorkspace, null, '\t')).then(() => {
+				const savedWorkspaceIdentifier = { id: this.getWorkspaceId(targetConfigPath), configPath: targetConfigPath };
 
-			// Delete untitled workspace
-			this.deleteUntitledWorkspaceSync(workspace);
+				// Event
+				this._onWorkspaceSaved.fire({ workspace: savedWorkspaceIdentifier, oldConfigPath: workspace.configPath });
 
-			return savedWorkspaceIdentifier;
+				// Delete untitled workspace
+				this.deleteUntitledWorkspaceSync(workspace);
+
+				return savedWorkspaceIdentifier;
+			});
 		});
 	}
 

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

@@ -189,8 +189,8 @@ suite('WorkspacesMainService', () => {
 
 				const ws = JSON.parse(fs.readFileSync(savedWorkspace.configPath).toString()) as IStoredWorkspace;
 				assert.equal(ws.folders.length, 2);
-				assert.equal(ws.folders[0].path, process.cwd());
-				assert.equal(ws.folders[1].path, os.tmpdir());
+				assert.equal(ws.folders[0].path, process.cwd()); // absolute
+				assert.equal(ws.folders[1].path, path.relative(path.dirname(workspaceConfigPath), os.tmpdir())); // relative
 
 				assert.equal(savedWorkspace, savedEvent.workspace);
 				assert.equal(workspace.configPath, savedEvent.oldConfigPath);
@@ -220,8 +220,8 @@ suite('WorkspacesMainService', () => {
 
 					const ws = JSON.parse(fs.readFileSync(newSavedWorkspace.configPath).toString()) as IStoredWorkspace;
 					assert.equal(ws.folders.length, 2);
-					assert.equal(ws.folders[0].path, process.cwd());
-					assert.equal(ws.folders[1].path, os.tmpdir());
+					assert.equal(ws.folders[0].path, process.cwd()); // absolute path because outside of tmpdir
+					assert.equal(ws.folders[1].path, path.relative(path.dirname(workspaceConfigPath), os.tmpdir())); // relative path because inside of tmpdir
 
 					extfs.delSync(workspaceConfigPath);
 					extfs.delSync(newWorkspaceConfigPath);

src/vs/workbench/services/workspace/node/workspaceEditingService.ts

@@ -15,6 +15,8 @@ import { IEnvironmentService } from 'vs/platform/environment/common/environment'
 import { IJSONEditingService } from 'vs/workbench/services/configuration/common/jsonEditing';
 import { IWorkspacesService, IStoredWorkspaceFolder } from 'vs/platform/workspaces/common/workspaces';
 import { isLinux } from 'vs/base/common/platform';
+import { dirname, relative } from 'path';
+import { isEqualOrParent } from 'vs/base/common/paths';
 
 export class WorkspaceEditingService implements IWorkspaceEditingService {
 
@@ -70,7 +72,14 @@ export class WorkspaceEditingService implements IWorkspaceEditingService {
 
 		// Apply to config
 		if (newWorkspaceRoots.length) {
-			const value: IStoredWorkspaceFolder[] = newWorkspaceRoots.map(newWorkspaceRoot => ({ path: newWorkspaceRoot }));
+			const workspaceConfigFolder = dirname(workspace.configuration.fsPath);
+			const value: IStoredWorkspaceFolder[] = newWorkspaceRoots.map(newWorkspaceRoot => {
+				if (isEqualOrParent(newWorkspaceRoot, workspaceConfigFolder, !isLinux)) {
+					newWorkspaceRoot = relative(workspaceConfigFolder, newWorkspaceRoot); // absolute paths get converted to relative ones to workspace location if possible
+				}
+
+				return { path: newWorkspaceRoot };
+			});
 
 			return this.jsonEditingService.write(workspace.configuration, { key: 'folders', value }, true);
 		} else {