multi root - do not store empty paths back, fallback to "."

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

@@ -87,7 +87,7 @@ export class WorkspacesMainService implements IWorkspacesMainService {
 
 			// relative paths get resolved against the workspace location
 			workspace.folders.forEach(folder => {
-				if (folder.path && !isAbsolute(folder.path)) {
+				if (!isAbsolute(folder.path)) {
 					folder.path = resolve(dirname(path), folder.path);
 				}
 			});
@@ -105,6 +105,8 @@ export class WorkspacesMainService implements IWorkspacesMainService {
 	}
 
 	private doParseStoredWorkspace(path: string, contents: string): IStoredWorkspace {
+
+		// Parse workspace file
 		let storedWorkspace: IStoredWorkspace;
 		try {
 			storedWorkspace = JSON.parse(contents);
@@ -112,6 +114,12 @@ export class WorkspacesMainService implements IWorkspacesMainService {
 			throw new Error(`${path} cannot be parsed as JSON file (${error}).`);
 		}
 
+		// Filter out folders which do not have a path set
+		if (Array.isArray(storedWorkspace.folders)) {
+			storedWorkspace.folders = storedWorkspace.folders.filter(folder => !!folder.path);
+		}
+
+		// Validate
 		if (!Array.isArray(storedWorkspace.folders) || storedWorkspace.folders.length === 0) {
 			throw new Error(`${path} looks like an invalid workspace file.`);
 		}
@@ -201,14 +209,12 @@ export class WorkspacesMainService implements IWorkspacesMainService {
 			// is a parent of the location of the workspace file itself. Otherwise keep
 			// using absolute paths.
 			storedWorkspace.folders.forEach(folder => {
-				if (folder.path) {
-					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
-					}
+				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
 				}
 			});
 

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

@@ -177,7 +177,7 @@ suite('WorkspacesMainService', () => {
 			deletedEvent = e;
 		});
 
-		return service.createWorkspace([process.cwd(), os.tmpdir()]).then(workspace => {
+		return service.createWorkspace([process.cwd(), os.tmpdir(), path.join(os.tmpdir(), 'somefolder')]).then(workspace => {
 			const workspaceConfigPath = path.join(os.tmpdir(), `myworkspace.${Date.now()}.${WORKSPACE_EXTENSION}`);
 
 			return service.saveWorkspace(workspace, workspaceConfigPath).then(savedWorkspace => {
@@ -188,9 +188,10 @@ suite('WorkspacesMainService', () => {
 				assert.equal(service.deleteWorkspaceCall, workspace);
 
 				const ws = JSON.parse(fs.readFileSync(savedWorkspace.configPath).toString()) as IStoredWorkspace;
-				assert.equal(ws.folders.length, 2);
+				assert.equal(ws.folders.length, 3);
 				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(ws.folders[1].path, '.'); // relative
+				assert.equal(ws.folders[2].path, path.relative(path.dirname(workspaceConfigPath), path.join(os.tmpdir(), 'somefolder'))); // relative
 
 				assert.equal(savedWorkspace, savedEvent.workspace);
 				assert.equal(workspace.configPath, savedEvent.oldConfigPath);
@@ -208,7 +209,7 @@ suite('WorkspacesMainService', () => {
 	});
 
 	test('saveWorkspace (saved workspace)', done => {
-		return service.createWorkspace([process.cwd(), os.tmpdir()]).then(workspace => {
+		return service.createWorkspace([process.cwd(), os.tmpdir(), path.join(os.tmpdir(), 'somefolder')]).then(workspace => {
 			const workspaceConfigPath = path.join(os.tmpdir(), `myworkspace.${Date.now()}.${WORKSPACE_EXTENSION}`);
 			const newWorkspaceConfigPath = path.join(os.tmpdir(), `mySavedWorkspace.${Date.now()}.${WORKSPACE_EXTENSION}`);
 
@@ -219,9 +220,10 @@ suite('WorkspacesMainService', () => {
 					assert.equal(newSavedWorkspace.configPath, newWorkspaceConfigPath);
 
 					const ws = JSON.parse(fs.readFileSync(newSavedWorkspace.configPath).toString()) as IStoredWorkspace;
-					assert.equal(ws.folders.length, 2);
+					assert.equal(ws.folders.length, 3);
 					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
+					assert.equal(ws.folders[1].path, '.'); // relative path because inside of tmpdir
+					assert.equal(ws.folders[2].path, path.relative(path.dirname(workspaceConfigPath), path.join(os.tmpdir(), 'somefolder'))); // relative
 
 					extfs.delSync(workspaceConfigPath);
 					extfs.delSync(newWorkspaceConfigPath);

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

@@ -75,7 +75,7 @@ export class WorkspaceEditingService implements IWorkspaceEditingService {
 			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
+					newWorkspaceRoot = relative(workspaceConfigFolder, newWorkspaceRoot) || '.'; // absolute paths get converted to relative ones to workspace location if possible
 				}
 
 				return { path: newWorkspaceRoot };