diff --git a/src/vs/workbench/services/configuration/node/configurationService.ts b/src/vs/workbench/services/configuration/node/configurationService.ts index 577c4f78dcdfd..f715607641466 100644 --- a/src/vs/workbench/services/configuration/node/configurationService.ts +++ b/src/vs/workbench/services/configuration/node/configurationService.ts @@ -5,9 +5,8 @@ 'use strict'; import URI from 'vs/base/common/uri'; -import * as paths from 'vs/base/common/paths'; import { TPromise } from 'vs/base/common/winjs.base'; -import { dirname } from 'path'; +import { dirname, basename } from 'path'; import * as assert from 'vs/base/common/assert'; import Event, { Emitter } from 'vs/base/common/event'; import { StrictResourceMap } from 'vs/base/common/map'; @@ -111,13 +110,16 @@ export class WorkspaceService extends Disposable implements IWorkspaceConfigurat } public addFolders(foldersToAdd: IWorkspaceFolderCreationData[], index?: number): TPromise { - assert.ok(this.jsonEditingService, 'Workbench is not initialized yet'); - return this.workspaceEditingQueue.queue(() => this.doAddFolders(foldersToAdd, index)); + return this.updateFolders(foldersToAdd, [], index); } public removeFolders(foldersToRemove: URI[]): TPromise { + return this.updateFolders([], foldersToRemove); + } + + public updateFolders(foldersToAdd: IWorkspaceFolderCreationData[], foldersToRemove: URI[], index?: number): TPromise { assert.ok(this.jsonEditingService, 'Workbench is not initialized yet'); - return this.workspaceEditingQueue.queue(() => this.doRemoveFolders(foldersToRemove)); + return this.workspaceEditingQueue.queue(() => this.doUpdateFolders(foldersToAdd, foldersToRemove, index)); } public isInsideWorkspace(resource: URI): boolean { @@ -134,80 +136,82 @@ export class WorkspaceService extends Disposable implements IWorkspaceConfigurat return false; } - private doAddFolders(foldersToAdd: IWorkspaceFolderCreationData[], index?: number): TPromise { + private doUpdateFolders(foldersToAdd: IWorkspaceFolderCreationData[], foldersToRemove: URI[], index?: number): TPromise { if (this.getWorkbenchState() !== WorkbenchState.WORKSPACE) { return TPromise.as(void 0); // we need a workspace to begin with } - const currentWorkspaceFolders = this.getWorkspace().folders; - const currentWorkspaceFolderUris = currentWorkspaceFolders.map(folder => folder.uri); - const currentStoredFolders = currentWorkspaceFolders.map(folder => folder.raw); - - const storedFoldersToAdd: IStoredWorkspaceFolder[] = []; + if (foldersToAdd.length + foldersToRemove.length === 0) { + return TPromise.as(void 0); // nothing to do + } - const workspaceConfigFolder = dirname(this.getWorkspace().configuration.fsPath); + let foldersHaveChanged = false; - foldersToAdd.forEach(folderToAdd => { - if (this.contains(currentWorkspaceFolderUris, folderToAdd.uri)) { - return; // already existing + // Remove first (if any) + let currentWorkspaceFolders = this.getWorkspace().folders; + let newStoredFolders: IStoredWorkspaceFolder[] = currentWorkspaceFolders.map(f => f.raw).filter((folder, index) => { + if (!isStoredWorkspaceFolder(folder)) { + return true; // keep entries which are unrelated } - let storedFolder: IStoredWorkspaceFolder; + return !this.contains(foldersToRemove, currentWorkspaceFolders[index].uri); // keep entries which are unrelated + }); - // File resource: use "path" property - if (folderToAdd.uri.scheme === Schemas.file) { - storedFolder = { - path: massageFolderPathForWorkspace(folderToAdd.uri.fsPath, workspaceConfigFolder, currentStoredFolders) - }; - } + foldersHaveChanged = currentWorkspaceFolders.length !== newStoredFolders.length; - // Any other resource: use "uri" property - else { - storedFolder = { - uri: folderToAdd.uri.toString(true) - }; - } + // Add afterwards (if any) + if (foldersToAdd.length) { - if (folderToAdd.name) { - storedFolder.name = folderToAdd.name; - } + // Recompute current workspace folders if we have folders to add + const workspaceConfigFolder = dirname(this.getWorkspace().configuration.fsPath); + currentWorkspaceFolders = toWorkspaceFolders(newStoredFolders, URI.file(workspaceConfigFolder)); + const currentWorkspaceFolderUris = currentWorkspaceFolders.map(folder => folder.uri); - storedFoldersToAdd.push(storedFolder); - }); + const storedFoldersToAdd: IStoredWorkspaceFolder[] = []; - if (storedFoldersToAdd.length > 0) { - let newStoredWorkspaceFolders: IStoredWorkspaceFolder[] = []; + foldersToAdd.forEach(folderToAdd => { + if (this.contains(currentWorkspaceFolderUris, folderToAdd.uri)) { + return; // already existing + } - if (typeof index === 'number' && index >= 0 && index < currentStoredFolders.length) { - newStoredWorkspaceFolders = currentStoredFolders.slice(0); - newStoredWorkspaceFolders.splice(index, 0, ...storedFoldersToAdd); - } else { - newStoredWorkspaceFolders = [...currentStoredFolders, ...storedFoldersToAdd]; - } + let storedFolder: IStoredWorkspaceFolder; - return this.setFolders(newStoredWorkspaceFolders); - } + // File resource: use "path" property + if (folderToAdd.uri.scheme === Schemas.file) { + storedFolder = { + path: massageFolderPathForWorkspace(folderToAdd.uri.fsPath, workspaceConfigFolder, newStoredFolders) + }; + } - return TPromise.as(void 0); - } + // Any other resource: use "uri" property + else { + storedFolder = { + uri: folderToAdd.uri.toString(true) + }; + } - private doRemoveFolders(foldersToRemove: URI[]): TPromise { - if (this.getWorkbenchState() !== WorkbenchState.WORKSPACE) { - return TPromise.as(void 0); // we need a workspace to begin with - } + if (folderToAdd.name) { + storedFolder.name = folderToAdd.name; + } - const currentWorkspaceFolders = this.getWorkspace().folders; - const currentStoredFolders = currentWorkspaceFolders.map(folder => folder.raw); + storedFoldersToAdd.push(storedFolder); + }); - const newStoredFolders: IStoredWorkspaceFolder[] = currentStoredFolders.filter((folder, index) => { - if (!isStoredWorkspaceFolder(folder)) { - return true; // keep entries which are unrelated - } + // Apply to array of newStoredFolders + if (storedFoldersToAdd.length > 0) { + foldersHaveChanged = true; - return !this.contains(foldersToRemove, currentWorkspaceFolders[index].uri); // keep entries which are unrelated - }); + if (typeof index === 'number' && index >= 0 && index < newStoredFolders.length) { + newStoredFolders = newStoredFolders.slice(0); + newStoredFolders.splice(index, 0, ...storedFoldersToAdd); + } else { + newStoredFolders = [...newStoredFolders, ...storedFoldersToAdd]; + } + } + } - if (newStoredFolders.length !== currentStoredFolders.length) { + // Set folders if we recorded a change + if (foldersHaveChanged) { return this.setFolders(newStoredFolders); } @@ -331,7 +335,7 @@ export class WorkspaceService extends Disposable implements IWorkspaceConfigurat const workspaceConfigPath = URI.file(workspaceIdentifier.configPath); return this.workspaceConfiguration.load(workspaceConfigPath) .then(() => { - const workspaceFolders = toWorkspaceFolders(this.workspaceConfiguration.getFolders(), URI.file(paths.dirname(workspaceConfigPath.fsPath))); + const workspaceFolders = toWorkspaceFolders(this.workspaceConfiguration.getFolders(), URI.file(dirname(workspaceConfigPath.fsPath))); const workspaceId = workspaceIdentifier.id; const workspaceName = getWorkspaceLabel({ id: workspaceId, configPath: workspaceConfigPath.fsPath }, this.environmentService); return new Workspace(workspaceId, workspaceName, workspaceFolders, workspaceConfigPath); @@ -350,7 +354,7 @@ export class WorkspaceService extends Disposable implements IWorkspaceConfigurat } private createEmptyWorkspace(configuration: IWindowConfiguration): TPromise { - let id = configuration.backupPath ? URI.from({ path: paths.basename(configuration.backupPath), scheme: 'empty' }).toString() : ''; + let id = configuration.backupPath ? URI.from({ path: basename(configuration.backupPath), scheme: 'empty' }).toString() : ''; return TPromise.as(new Workspace(id)); } @@ -506,7 +510,7 @@ export class WorkspaceService extends Disposable implements IWorkspaceConfigurat private onWorkspaceConfigurationChanged(): TPromise { if (this.workspace && this.workspace.configuration && this._configuration) { const workspaceConfigurationChangeEvent = this._configuration.compareAndUpdateWorkspaceConfiguration(this.workspaceConfiguration.getConfiguration()); - let configuredFolders = toWorkspaceFolders(this.workspaceConfiguration.getFolders(), URI.file(paths.dirname(this.workspace.configuration.fsPath))); + let configuredFolders = toWorkspaceFolders(this.workspaceConfiguration.getFolders(), URI.file(dirname(this.workspace.configuration.fsPath))); const changes = this.compareFolders(this.workspace.folders, configuredFolders); if (changes.added.length || changes.removed.length || changes.changed.length) { this.workspace.folders = configuredFolders; diff --git a/src/vs/workbench/services/configuration/test/node/configurationService.test.ts b/src/vs/workbench/services/configuration/test/node/configurationService.test.ts index 6563fd4a42c7d..2ef62d58898c1 100644 --- a/src/vs/workbench/services/configuration/test/node/configurationService.test.ts +++ b/src/vs/workbench/services/configuration/test/node/configurationService.test.ts @@ -274,6 +274,55 @@ suite('WorkspaceContextService - Workspace', () => { }); }); + test('update folders (remove last and add to end)', () => { + const target = sinon.spy(); + testObject.onDidChangeWorkspaceFolders(target); + const workspaceDir = path.dirname(testObject.getWorkspace().folders[0].uri.fsPath); + const addedFolders = [{ uri: URI.file(path.join(workspaceDir, 'd')) }, { uri: URI.file(path.join(workspaceDir, 'c')) }]; + const removedFolders = [testObject.getWorkspace().folders[1]].map(f => f.uri); + return testObject.updateFolders(addedFolders, removedFolders) + .then(() => { + assert.ok(target.calledOnce); + const actual = target.args[0][0]; + assert.deepEqual(actual.added.map(r => r.uri.toString()), addedFolders.map(a => a.uri.toString())); + assert.deepEqual(actual.removed.map(r => r.uri.toString()), removedFolders.map(a => a.toString())); + assert.deepEqual(actual.changed, []); + }); + }); + + test('update folders (rename first via add and remove)', () => { + const target = sinon.spy(); + testObject.onDidChangeWorkspaceFolders(target); + const workspaceDir = path.dirname(testObject.getWorkspace().folders[0].uri.fsPath); + const addedFolders = [{ uri: URI.file(path.join(workspaceDir, 'a')), name: 'The Folder' }]; + const removedFolders = [testObject.getWorkspace().folders[0]].map(f => f.uri); + return testObject.updateFolders(addedFolders, removedFolders, 0) + .then(() => { + assert.ok(target.calledOnce); + const actual = target.args[0][0]; + assert.deepEqual(actual.added, []); + assert.deepEqual(actual.removed, []); + assert.deepEqual(actual.changed.map(r => r.uri.toString()), removedFolders.map(a => a.toString())); + }); + }); + + test('update folders (remove first and add to end)', () => { + const target = sinon.spy(); + testObject.onDidChangeWorkspaceFolders(target); + const workspaceDir = path.dirname(testObject.getWorkspace().folders[0].uri.fsPath); + const addedFolders = [{ uri: URI.file(path.join(workspaceDir, 'd')) }, { uri: URI.file(path.join(workspaceDir, 'c')) }]; + const removedFolders = [testObject.getWorkspace().folders[0]].map(f => f.uri); + const changedFolders = [testObject.getWorkspace().folders[1]].map(f => f.uri); + return testObject.updateFolders(addedFolders, removedFolders) + .then(() => { + assert.ok(target.calledOnce); + const actual = target.args[0][0]; + assert.deepEqual(actual.added.map(r => r.uri.toString()), addedFolders.map(a => a.uri.toString())); + assert.deepEqual(actual.removed.map(r => r.uri.toString()), removedFolders.map(a => a.toString())); + assert.deepEqual(actual.changed.map(r => r.uri.toString()), changedFolders.map(a => a.toString())); + }); + }); + test('reorder folders trigger change event', () => { const target = sinon.spy(); testObject.onDidChangeWorkspaceFolders(target); diff --git a/src/vs/workbench/services/workspace/node/workspaceEditingService.ts b/src/vs/workbench/services/workspace/node/workspaceEditingService.ts index c35254fc87fe5..6cf2752ff79bd 100644 --- a/src/vs/workbench/services/workspace/node/workspaceEditingService.ts +++ b/src/vs/workbench/services/workspace/node/workspaceEditingService.ts @@ -73,15 +73,28 @@ export class WorkspaceEditingService implements IWorkspaceEditingService { } // Add & Delete Folders - if (this.includesSingleFolderWorkspace(foldersToDelete)) { + else { + // if we are in single-folder state and the folder is replaced with // other folders, we handle this specially and just enter workspace // mode with the folders that are being added. - return this.createAndEnterWorkspace(foldersToAdd); + if (this.includesSingleFolderWorkspace(foldersToDelete)) { + return this.createAndEnterWorkspace(foldersToAdd); + } + + // if we are not in workspace-state, we just add the folders + if (this.contextService.getWorkbenchState() !== WorkbenchState.WORKSPACE) { + return this.doAddFolders(foldersToAdd, index, donotNotifyError); + } + + // finally, update folders within the workspace + return this.doUpdateFolders(foldersToAdd, foldersToDelete, index, donotNotifyError); } + } - // Make sure to first remove folders and then add them to account for folders being updated - return this.removeFolders(foldersToDelete).then(() => this.doAddFolders(foldersToAdd, index, donotNotifyError)); + private doUpdateFolders(foldersToAdd: IWorkspaceFolderCreationData[], foldersToDelete: URI[], index?: number, donotNotifyError: boolean = false): TPromise { + return this.contextService.updateFolders(foldersToAdd, foldersToDelete, index) + .then(() => null, error => donotNotifyError ? TPromise.wrapError(error) : this.handleWorkspaceConfigurationEditingError(error)); } public addFolders(foldersToAdd: IWorkspaceFolderCreationData[], donotNotifyError: boolean = false): TPromise {