From 40798a95cdb3a79798f0579d7d51e1cacfaca7a9 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Thu, 25 Jan 2018 09:03:48 +0100 Subject: [PATCH] use live objects and avoid promises --- src/vs/platform/workspace/common/workspace.ts | 4 +- src/vs/vscode.proposed.d.ts | 5 +- .../electron-browser/mainThreadWorkspace.ts | 4 +- src/vs/workbench/api/node/extHost.protocol.ts | 2 +- src/vs/workbench/api/node/extHostWorkspace.ts | 163 +++++++++++++----- 5 files changed, 124 insertions(+), 54 deletions(-) diff --git a/src/vs/platform/workspace/common/workspace.ts b/src/vs/platform/workspace/common/workspace.ts index 1493935a013..9ba87d62535 100644 --- a/src/vs/platform/workspace/common/workspace.ts +++ b/src/vs/platform/workspace/common/workspace.ts @@ -205,8 +205,8 @@ export class Workspace implements IWorkspace { export class WorkspaceFolder implements IWorkspaceFolder { readonly uri: URI; - readonly name: string; - readonly index: number; + name: string; + index: number; constructor(data: IWorkspaceFolderData, readonly raw?: IStoredWorkspaceFolder) { diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 3abc6a2361b..09bafe76fc7 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -188,9 +188,10 @@ declare module 'vscode' { * @param deleteCount the optional number of workspace folders to remove. * @param workspaceFoldersToAdd the optional variable set of workspace folders to add in place of the deleted ones. * Each workspace is identified with a mandatory URI and an optional name. - * @return A thenable that resolves when the workspace folder was removed successfully. + * @return true if the operation was successfully started. Use the [onDidChangeWorkspaceFolders()](#onDidChangeWorkspaceFolders) + * event to get notified when the workspace folders have been updated. */ - export function updateWorkspaceFolders(start: number, deleteCount: number, ...workspaceFoldersToAdd: { uri: Uri, name?: string }[]): Thenable; + export function updateWorkspaceFolders(start: number, deleteCount: number, ...workspaceFoldersToAdd: { uri: Uri, name?: string }[]): boolean; } export namespace window { diff --git a/src/vs/workbench/api/electron-browser/mainThreadWorkspace.ts b/src/vs/workbench/api/electron-browser/mainThreadWorkspace.ts index 9cb29c82601..b1cd93ae038 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadWorkspace.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadWorkspace.ts @@ -50,13 +50,13 @@ export class MainThreadWorkspace implements MainThreadWorkspaceShape { // --- workspace --- - $updateWorkspaceFolders(extensionName: string, index: number, deleteCount: number, foldersToAdd: { uri: UriComponents, name?: string }[]): Thenable { + $updateWorkspaceFolders(extensionName: string, index: number, deleteCount: number, foldersToAdd: { uri: UriComponents, name?: string }[]): Thenable { const workspaceFoldersToAdd = foldersToAdd.map(f => ({ uri: URI.revive(f.uri), name: f.name })); // Indicate in status message this._statusbarService.setStatusMessage(this.getStatusMessage(extensionName, workspaceFoldersToAdd.length, deleteCount), 10 * 1000 /* 10s */); - return this._workspaceEditingService.updateFolders(index, deleteCount, workspaceFoldersToAdd, true).then(() => true); + return this._workspaceEditingService.updateFolders(index, deleteCount, workspaceFoldersToAdd, true); } private getStatusMessage(extensionName, addCount: number, removeCount: number): string { diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index f6238833558..d04a8c74f1a 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -355,7 +355,7 @@ export interface MainThreadWorkspaceShape extends IDisposable { $startSearch(includePattern: string, includeFolder: string, excludePattern: string, maxResults: number, requestId: number): Thenable; $cancelSearch(requestId: number): Thenable; $saveAll(includeUntitled?: boolean): Thenable; - $updateWorkspaceFolders(extensionName: string, index: number, deleteCount: number, workspaceFoldersToAdd: { uri: UriComponents, name?: string }[]): Thenable; + $updateWorkspaceFolders(extensionName: string, index: number, deleteCount: number, workspaceFoldersToAdd: { uri: UriComponents, name?: string }[]): Thenable; } export interface IFileChangeDto { diff --git a/src/vs/workbench/api/node/extHostWorkspace.ts b/src/vs/workbench/api/node/extHostWorkspace.ts index 898a3499ada..946b0aed99b 100644 --- a/src/vs/workbench/api/node/extHostWorkspace.ts +++ b/src/vs/workbench/api/node/extHostWorkspace.ts @@ -16,20 +16,78 @@ import { compare } from 'vs/base/common/strings'; import { TernarySearchTree } from 'vs/base/common/map'; import { basenameOrAuthority, isEqual } from 'vs/base/common/resources'; import { isLinux } from 'vs/base/common/platform'; +import { onUnexpectedError } from 'vs/base/common/errors'; + +function isFolderEqual(folderA: URI, folderB: URI): boolean { + return isEqual(folderA, folderB, !isLinux); +} class Workspace2 extends Workspace { - static fromData(data: IWorkspaceData) { + static acceptWorkspaceData(data: IWorkspaceData, oldWorkspace?: Workspace2): { workspace: Workspace2, added: vscode.WorkspaceFolder[], removed: vscode.WorkspaceFolder[] } { if (!data) { - return null; + return { workspace: null, added: [], removed: [] }; + } + + const { id, name, folders } = data; + const newWorkspaceFolders: WorkspaceFolder[] = []; + + // If we have an existing workspace, we try to find the folders that match our + // data and update their properties. It could be that an extension stored them + // for later use and we want to keep them "live" if they are still present. + if (oldWorkspace) { + folders.forEach((folderData, index) => { + const folderUri = URI.revive(folderData.uri); + const existingFolder = Workspace2._findFolder(oldWorkspace, folderUri); + + if (existingFolder) { + existingFolder.name = folderData.name; + existingFolder.index = folderData.index; + + newWorkspaceFolders.push(existingFolder); + } else { + newWorkspaceFolders.push(new WorkspaceFolder({ name: folderData.name, index, uri: folderUri })); + } + }); } else { - const { id, name, folders } = data; - return new Workspace2( - id, - name, - folders.map(({ uri, name, index }) => new WorkspaceFolder({ name, index, uri: URI.revive(uri) })) - ); + newWorkspaceFolders.push(...folders.map(({ uri, name, index }) => new WorkspaceFolder({ name, index, uri: URI.revive(uri) }))); + } + + const workspace = new Workspace2(id, name, newWorkspaceFolders); + + const oldRoots = oldWorkspace ? oldWorkspace.workspaceFolders.sort(Workspace2.compareWorkspaceFolderByUri) : []; + const newRoots = workspace.workspaceFolders.sort(Workspace2.compareWorkspaceFolderByUri); + + const { added, removed } = delta(oldRoots, newRoots, Workspace2.compareWorkspaceFolderByUri); + + return { workspace, added, removed }; + } + + static compareWorkspaceFolderByUri(a: vscode.WorkspaceFolder, b: vscode.WorkspaceFolder, includeName?: boolean): number { + if (isFolderEqual(a.uri, b.uri)) { + return 0; + } + + return compare(a.uri.toString(), b.uri.toString()); + } + + static compareWorkspaceFolderByUriAndName(a: vscode.WorkspaceFolder, b: vscode.WorkspaceFolder): number { + if (isFolderEqual(a.uri, b.uri) && compare(a.name, b.name) === 0) { + return 0; + } + + return compare(a.uri.toString(), b.uri.toString()) + compare(a.name, b.name); + } + + private static _findFolder(workspace: Workspace2, folderUriToFind: URI): WorkspaceFolder { + for (let i = 0; i < workspace.folders.length; i++) { + const folder = workspace.folders[i]; + if (isFolderEqual(folder.uri, folderUriToFind)) { + return folder; + } } + + return undefined; } private readonly _workspaceFolders: vscode.WorkspaceFolder[] = []; @@ -65,77 +123,95 @@ export class ExtHostWorkspace implements ExtHostWorkspaceShape { private readonly _onDidChangeWorkspace = new Emitter(); private readonly _proxy: MainThreadWorkspaceShape; - private _workspace: Workspace2; + + private _confirmedWorkspace: Workspace2; + private _unconfirmedWorkspace: Workspace2; readonly onDidChangeWorkspace: Event = this._onDidChangeWorkspace.event; constructor(mainContext: IMainContext, data: IWorkspaceData) { this._proxy = mainContext.getProxy(MainContext.MainThreadWorkspace); - this._workspace = Workspace2.fromData(data); + this._confirmedWorkspace = Workspace2.acceptWorkspaceData(data).workspace; } // --- workspace --- get workspace(): Workspace { - return this._workspace; + return this._actualWorkspace; + } + + private get _actualWorkspace(): Workspace2 { + return this._unconfirmedWorkspace || this._confirmedWorkspace; } getWorkspaceFolders(): vscode.WorkspaceFolder[] { - if (!this._workspace) { + if (!this._actualWorkspace) { return undefined; - } else { - return this._workspace.workspaceFolders.slice(0); } + return this._actualWorkspace.workspaceFolders.slice(0); } - updateWorkspaceFolders(extensionName: string, index: number, deleteCount: number, ...workspaceFoldersToAdd: { uri: vscode.Uri, name?: string }[]): Thenable { + updateWorkspaceFolders(extensionName: string, index: number, deleteCount: number, ...workspaceFoldersToAdd: { uri: vscode.Uri, name?: string }[]): boolean { const validatedDistinctWorkspaceFoldersToAdd: { uri: vscode.Uri, name?: string }[] = []; if (Array.isArray(workspaceFoldersToAdd)) { workspaceFoldersToAdd.forEach(folderToAdd => { - if (URI.isUri(folderToAdd.uri) && !validatedDistinctWorkspaceFoldersToAdd.some(f => isEqual(f.uri, folderToAdd.uri, !isLinux))) { + if (URI.isUri(folderToAdd.uri) && !validatedDistinctWorkspaceFoldersToAdd.some(f => isFolderEqual(f.uri, folderToAdd.uri))) { validatedDistinctWorkspaceFoldersToAdd.push(folderToAdd); } }); } if ([index, deleteCount].some(i => typeof i !== 'number' || i < 0)) { - return Promise.resolve(false); // validate numbers + return false; // validate numbers } if (deleteCount === 0 && validatedDistinctWorkspaceFoldersToAdd.length === 0) { - return Promise.resolve(false); // nothing to delete or add + return false; // nothing to delete or add } - const currentWorkspaceFolders: vscode.WorkspaceFolder[] = this._workspace ? this._workspace.workspaceFolders : []; + const currentWorkspaceFolders: vscode.WorkspaceFolder[] = this._actualWorkspace ? this._actualWorkspace.workspaceFolders : []; if (index + deleteCount > currentWorkspaceFolders.length) { - return Promise.resolve(false); // cannot delete more than we have + return false; // cannot delete more than we have } const newWorkspaceFolders = currentWorkspaceFolders.slice(0); newWorkspaceFolders.splice(index, deleteCount, ...validatedDistinctWorkspaceFoldersToAdd.map((f, index) => ({ uri: f.uri, name: f.name || basenameOrAuthority(f.uri), index }))); - const { added, removed } = delta(currentWorkspaceFolders, newWorkspaceFolders, ExtHostWorkspace._compareWorkspaceFolderByUriAndName); + const oldRoots = currentWorkspaceFolders.sort(Workspace2.compareWorkspaceFolderByUri); + const newRoots = newWorkspaceFolders.sort(Workspace2.compareWorkspaceFolderByUri); + const { added, removed } = delta(oldRoots, newRoots, Workspace2.compareWorkspaceFolderByUriAndName); if (added.length === 0 && removed.length === 0) { - return Promise.resolve(false); // nothing actually changed + return false; // nothing actually changed + } + + // Trigger on main side + this._proxy.$updateWorkspaceFolders(extensionName, index, deleteCount, validatedDistinctWorkspaceFoldersToAdd).then(null, onUnexpectedError); + + // Update directly here. The workspace is unconfirmed as long as we did not get an + // acknowledgement from the main side (via acceptWorkspaceData) + if (this._actualWorkspace) { + this._unconfirmedWorkspace = Workspace2.acceptWorkspaceData({ id: this._actualWorkspace.id, name: this._actualWorkspace.name, configuration: this._actualWorkspace.configuration, folders: newWorkspaceFolders }, this._actualWorkspace).workspace; } - return this._proxy.$updateWorkspaceFolders(extensionName, index, deleteCount, validatedDistinctWorkspaceFoldersToAdd); + return true; } getWorkspaceFolder(uri: vscode.Uri, resolveParent?: boolean): vscode.WorkspaceFolder { - if (!this._workspace) { + if (!this._actualWorkspace) { return undefined; } - return this._workspace.getWorkspaceFolder(uri, resolveParent); + return this._actualWorkspace.getWorkspaceFolder(uri, resolveParent); } getPath(): string { + // this is legacy from the days before having // multi-root and we keep it only alive if there // is just one workspace folder. - if (!this._workspace) { + if (!this._actualWorkspace) { return undefined; } - const { folders } = this._workspace; + + const { folders } = this._actualWorkspace; if (folders.length === 0) { return undefined; } @@ -165,7 +241,7 @@ export class ExtHostWorkspace implements ExtHostWorkspaceShape { } if (typeof includeWorkspace === 'undefined') { - includeWorkspace = this.workspace.folders.length > 1; + includeWorkspace = this._actualWorkspace.folders.length > 1; } let result = relative(folder.uri.fsPath, path); @@ -177,27 +253,20 @@ export class ExtHostWorkspace implements ExtHostWorkspaceShape { $acceptWorkspaceData(data: IWorkspaceData): void { - // keep old workspace folder, build new workspace, and - // capture new workspace folders. Compute delta between - // them send that as event - const oldRoots = this._workspace ? this._workspace.workspaceFolders.sort(ExtHostWorkspace._compareWorkspaceFolderByUri) : []; - - this._workspace = Workspace2.fromData(data); - const newRoots = this._workspace ? this._workspace.workspaceFolders.sort(ExtHostWorkspace._compareWorkspaceFolderByUri) : []; + const { workspace, added, removed } = Workspace2.acceptWorkspaceData(data, this._confirmedWorkspace /* use confirmed workspace to produce the true delta from last time */); - const { added, removed } = delta(oldRoots, newRoots, ExtHostWorkspace._compareWorkspaceFolderByUri); - this._onDidChangeWorkspace.fire(Object.freeze({ - added: Object.freeze(added), - removed: Object.freeze(removed) - })); - } + // Update our workspace object. We have a confirmed workspace, so we drop our + // unconfirmed workspace. + this._confirmedWorkspace = workspace; + this._unconfirmedWorkspace = undefined; - private static _compareWorkspaceFolderByUri(a: vscode.WorkspaceFolder, b: vscode.WorkspaceFolder, includeName?: boolean): number { - return compare(a.uri.toString(), b.uri.toString()); - } - - private static _compareWorkspaceFolderByUriAndName(a: vscode.WorkspaceFolder, b: vscode.WorkspaceFolder): number { - return compare(a.uri.toString(), b.uri.toString()) + compare(a.name, b.name); + // Events + if (added.length || removed.length) { + this._onDidChangeWorkspace.fire(Object.freeze({ + added: Object.freeze(added), + removed: Object.freeze(removed) + })); + } } // --- search --- -- GitLab