diff --git a/src/vs/platform/workspaces/electron-main/workspacesMainService.ts b/src/vs/platform/workspaces/electron-main/workspacesMainService.ts index 466f3303398..f6c6ab6035a 100644 --- a/src/vs/platform/workspaces/electron-main/workspacesMainService.ts +++ b/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 = (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}).`); - - return null; // unable to read or parse as workspace file + 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}).`); + } + + 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 { + public saveWorkspace(workspace: IWorkspaceIdentifier, targetConfigPath: string): TPromise { // 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); + } - // Event - this._onWorkspaceSaved.fire({ workspace: savedWorkspaceIdentifier, oldConfigPath: workspace.configPath }); + const sourceConfigFolder = dirname(workspace.configPath); + const targetConfigFolder = dirname(targetConfigPath); - // Delete untitled workspace - this.deleteUntitledWorkspaceSync(workspace); + // 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 + } - return savedWorkspaceIdentifier; + if (isEqualOrParent(folder.path, targetConfigFolder, !isLinux)) { + folder.path = relative(targetConfigFolder, folder.path); // absolute paths get converted to relative ones to workspace location if possible + } + }); + + return writeFile(targetConfigPath, JSON.stringify(storedWorkspace, null, '\t')).then(() => { + const savedWorkspaceIdentifier = { id: this.getWorkspaceId(targetConfigPath), configPath: targetConfigPath }; + + // Event + this._onWorkspaceSaved.fire({ workspace: savedWorkspaceIdentifier, oldConfigPath: workspace.configPath }); + + // Delete untitled workspace + this.deleteUntitledWorkspaceSync(workspace); + + return savedWorkspaceIdentifier; + }); }); } diff --git a/src/vs/platform/workspaces/test/electron-main/workspacesMainService.test.ts b/src/vs/platform/workspaces/test/electron-main/workspacesMainService.test.ts index e9612fa7421..73f43059a1d 100644 --- a/src/vs/platform/workspaces/test/electron-main/workspacesMainService.test.ts +++ b/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); diff --git a/src/vs/workbench/services/workspace/node/workspaceEditingService.ts b/src/vs/workbench/services/workspace/node/workspaceEditingService.ts index 521f780a076..b380e90b5f6 100644 --- a/src/vs/workbench/services/workspace/node/workspaceEditingService.ts +++ b/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 {