workspace editing - move path normalization into service

src/vs/platform/workspaces/common/workspaces.ts

@@ -27,10 +27,10 @@ export interface IWorkspacesService {
 	readonly _serviceBrand: undefined;
 
 	// Workspaces Management
-	enterWorkspace(path: URI): Promise<IEnterWorkspaceResult | undefined>;
+	enterWorkspace(workspaceUri: URI): Promise<IEnterWorkspaceResult | undefined>;
 	createUntitledWorkspace(folders?: IWorkspaceFolderCreationData[], remoteAuthority?: string): Promise<IWorkspaceIdentifier>;
 	deleteUntitledWorkspace(workspace: IWorkspaceIdentifier): Promise<void>;
-	getWorkspaceIdentifier(workspacePath: URI): Promise<IWorkspaceIdentifier>;
+	getWorkspaceIdentifier(workspaceUri: URI): Promise<IWorkspaceIdentifier>;
 
 	// Workspaces History
 	readonly onDidChangeRecentlyOpened: Event<void>;

src/vs/workbench/browser/actions/workspaceCommands.ts

@@ -6,7 +6,7 @@
 import { localize } from 'vs/nls';
 import { hasWorkspaceFileExtension, IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
 import { IWorkspaceEditingService } from 'vs/workbench/services/workspaces/common/workspaceEditing';
-import { dirname, removeTrailingPathSeparator } from 'vs/base/common/resources';
+import { dirname } from 'vs/base/common/resources';
 import { CancellationToken } from 'vs/base/common/cancellation';
 import { mnemonicButtonLabel } from 'vs/base/common/labels';
 import { CommandsRegistry, ICommandService } from 'vs/platform/commands/common/commands';
@@ -69,7 +69,7 @@ CommandsRegistry.registerCommand({
 			return;
 		}
 
-		await workspaceEditingService.addFolders(folders.map(folder => ({ uri: removeTrailingPathSeparator(folder) })));
+		await workspaceEditingService.addFolders(folders.map(folder => ({ uri: folder })));
 	}
 });
 
@@ -84,7 +84,7 @@ CommandsRegistry.registerCommand({
 			return;
 		}
 
-		await workspaceEditingService.updateFolders(0, contextService.getWorkspace().folders.length, folders.map(folder => ({ uri: removeTrailingPathSeparator(folder) })));
+		await workspaceEditingService.updateFolders(0, contextService.getWorkspace().folders.length, folders.map(folder => ({ uri: folder })));
 	}
 });
 

src/vs/workbench/services/workspaces/browser/abstractWorkspaceEditingService.ts

@@ -14,7 +14,7 @@ import { ConfigurationScope, IConfigurationRegistry, Extensions as Configuration
 import { Registry } from 'vs/platform/registry/common/platform';
 import { ICommandService } from 'vs/platform/commands/common/commands';
 import { distinct, firstOrDefault } from 'vs/base/common/arrays';
-import { basename, isEqual, isEqualAuthority } from 'vs/base/common/resources';
+import { basename, isEqual, isEqualAuthority, removeTrailingPathSeparator } from 'vs/base/common/resources';
 import { INotificationService, Severity } from 'vs/platform/notification/common/notification';
 import { IFileService } from 'vs/platform/files/common/files';
 import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService';
@@ -96,23 +96,28 @@ export abstract class AbstractWorkspaceEditingService implements IWorkspaceEditi
 		return `workspace.${WORKSPACE_EXTENSION}`;
 	}
 
-	updateFolders(index: number, deleteCount?: number, foldersToAdd?: IWorkspaceFolderCreationData[], donotNotifyError?: boolean): Promise<void> {
+	async updateFolders(index: number, deleteCount?: number, foldersToAddCandidates?: IWorkspaceFolderCreationData[], donotNotifyError?: boolean): Promise<void> {
 		const folders = this.contextService.getWorkspace().folders;
 
 		let foldersToDelete: URI[] = [];
 		if (typeof deleteCount === 'number') {
-			foldersToDelete = folders.slice(index, index + deleteCount).map(f => f.uri);
+			foldersToDelete = folders.slice(index, index + deleteCount).map(folder => folder.uri);
+		}
+
+		let foldersToAdd: IWorkspaceFolderCreationData[] = [];
+		if (Array.isArray(foldersToAddCandidates)) {
+			foldersToAdd = foldersToAddCandidates.map(folderToAdd => ({ uri: removeTrailingPathSeparator(folderToAdd.uri), name: folderToAdd.name })); // Normalize
 		}
 
 		const wantsToDelete = foldersToDelete.length > 0;
-		const wantsToAdd = Array.isArray(foldersToAdd) && foldersToAdd.length > 0;
+		const wantsToAdd = foldersToAdd.length > 0;
 
 		if (!wantsToAdd && !wantsToDelete) {
-			return Promise.resolve(); // return early if there is nothing to do
+			return; // return early if there is nothing to do
 		}
 
 		// Add Folders
-		if (wantsToAdd && !wantsToDelete && Array.isArray(foldersToAdd)) {
+		if (wantsToAdd && !wantsToDelete) {
 			return this.doAddFolders(foldersToAdd, index, donotNotifyError);
 		}
 
@@ -128,16 +133,16 @@ export abstract class AbstractWorkspaceEditingService implements IWorkspaceEditi
 			// other folders, we handle this specially and just enter workspace
 			// mode with the folders that are being added.
 			if (this.includesSingleFolderWorkspace(foldersToDelete)) {
-				return this.createAndEnterWorkspace(foldersToAdd!);
+				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);
+				return this.doAddFolders(foldersToAdd, index, donotNotifyError);
 			}
 
 			// finally, update folders within the workspace
-			return this.doUpdateFolders(foldersToAdd!, foldersToDelete, index, donotNotifyError);
+			return this.doUpdateFolders(foldersToAdd, foldersToDelete, index, donotNotifyError);
 		}
 	}
 
@@ -153,7 +158,11 @@ export abstract class AbstractWorkspaceEditingService implements IWorkspaceEditi
 		}
 	}
 
-	addFolders(foldersToAdd: IWorkspaceFolderCreationData[], donotNotifyError: boolean = false): Promise<void> {
+	addFolders(foldersToAddCandidates: IWorkspaceFolderCreationData[], donotNotifyError: boolean = false): Promise<void> {
+
+		// Normalize
+		const foldersToAdd = foldersToAddCandidates.map(folderToAdd => ({ uri: removeTrailingPathSeparator(folderToAdd.uri), name: folderToAdd.name }));
+
 		return this.doAddFolders(foldersToAdd, undefined, donotNotifyError);
 	}
 
@@ -240,29 +249,29 @@ export abstract class AbstractWorkspaceEditingService implements IWorkspaceEditi
 		return this.enterWorkspace(path);
 	}
 
-	async saveAndEnterWorkspace(path: URI): Promise<void> {
+	async saveAndEnterWorkspace(workspaceUri: URI): Promise<void> {
 		const workspaceIdentifier = this.getCurrentWorkspaceIdentifier();
 		if (!workspaceIdentifier) {
 			return;
 		}
 
 		// Allow to save the workspace of the current window
 		// if we have an identical match on the path
-		if (isEqual(workspaceIdentifier.configPath, path)) {
+		if (isEqual(workspaceIdentifier.configPath, workspaceUri)) {
 			return this.saveWorkspace(workspaceIdentifier);
 		}
 
 		// From this moment on we require a valid target that is not opened already
-		if (!await this.isValidTargetWorkspacePath(path)) {
+		if (!await this.isValidTargetWorkspacePath(workspaceUri)) {
 			return;
 		}
 
-		await this.saveWorkspaceAs(workspaceIdentifier, path);
+		await this.saveWorkspaceAs(workspaceIdentifier, workspaceUri);
 
-		return this.enterWorkspace(path);
+		return this.enterWorkspace(workspaceUri);
 	}
 
-	async isValidTargetWorkspacePath(path: URI): Promise<boolean> {
+	async isValidTargetWorkspacePath(workspaceUri: URI): Promise<boolean> {
 		return true; // OK
 	}
 
@@ -339,14 +348,14 @@ export abstract class AbstractWorkspaceEditingService implements IWorkspaceEditi
 		);
 	}
 
-	abstract enterWorkspace(path: URI): Promise<void>;
+	abstract enterWorkspace(workspaceUri: URI): Promise<void>;
 
-	protected async doEnterWorkspace(path: URI): Promise<IEnterWorkspaceResult | undefined> {
+	protected async doEnterWorkspace(workspaceUri: URI): Promise<IEnterWorkspaceResult | undefined> {
 		if (!!this.environmentService.extensionTestsLocationURI) {
 			throw new Error('Entering a new workspace is not possible in tests.');
 		}
 
-		const workspace = await this.workspacesService.getWorkspaceIdentifier(path);
+		const workspace = await this.workspacesService.getWorkspaceIdentifier(workspaceUri);
 
 		// Settings migration (only if we come from a folder workspace)
 		if (this.contextService.getWorkbenchState() === WorkbenchState.FOLDER) {
@@ -356,7 +365,7 @@ export abstract class AbstractWorkspaceEditingService implements IWorkspaceEditi
 		const workspaceImpl = this.contextService as WorkspaceService;
 		await workspaceImpl.initialize(workspace);
 
-		return this.workspacesService.enterWorkspace(path);
+		return this.workspacesService.enterWorkspace(workspaceUri);
 	}
 
 	private migrateWorkspaceSettings(toWorkspace: IWorkspaceIdentifier): Promise<void> {

src/vs/workbench/services/workspaces/browser/workspaceEditingService.ts

@@ -43,12 +43,12 @@ export class BrowserWorkspaceEditingService extends AbstractWorkspaceEditingServ
 		super(jsonEditingService, contextService, configurationService, notificationService, commandService, fileService, textFileService, workspacesService, environmentService, fileDialogService, dialogService, hostService, uriIdentityService, workspaceTrustManagementService);
 	}
 
-	async enterWorkspace(path: URI): Promise<void> {
-		const result = await this.doEnterWorkspace(path);
+	async enterWorkspace(workspaceUri: URI): Promise<void> {
+		const result = await this.doEnterWorkspace(workspaceUri);
 		if (result) {
 
 			// Open workspace in same window
-			await this.hostService.openWindow([{ workspaceUri: path }], { forceReuseWindow: true });
+			await this.hostService.openWindow([{ workspaceUri }], { forceReuseWindow: true });
 		}
 	}
 }

src/vs/workbench/services/workspaces/browser/workspaces.ts

@@ -11,21 +11,21 @@ import { hash } from 'vs/base/common/hash';
 // NOTE: DO NOT CHANGE. IDENTIFIERS HAVE TO REMAIN STABLE
 // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 
-export function getWorkspaceIdentifier(workspacePath: URI): IWorkspaceIdentifier {
+export function getWorkspaceIdentifier(workspaceUri: URI): IWorkspaceIdentifier {
 	return {
-		id: getWorkspaceId(workspacePath),
-		configPath: workspacePath
+		id: getWorkspaceId(workspaceUri),
+		configPath: workspaceUri
 	};
 }
 
 // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 // NOTE: DO NOT CHANGE. IDENTIFIERS HAVE TO REMAIN STABLE
 // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 
-export function getSingleFolderWorkspaceIdentifier(folderPath: URI): ISingleFolderWorkspaceIdentifier {
+export function getSingleFolderWorkspaceIdentifier(folderUri: URI): ISingleFolderWorkspaceIdentifier {
 	return {
-		id: getWorkspaceId(folderPath),
-		uri: folderPath
+		id: getWorkspaceId(folderUri),
+		uri: folderUri
 	};
 }
 

src/vs/workbench/services/workspaces/browser/workspacesService.ts

@@ -167,8 +167,8 @@ export class BrowserWorkspacesService extends Disposable implements IWorkspacesS
 
 	//#region Workspace Management
 
-	async enterWorkspace(path: URI): Promise<IEnterWorkspaceResult | undefined> {
-		return { workspace: await this.getWorkspaceIdentifier(path) };
+	async enterWorkspace(workspaceUri: URI): Promise<IEnterWorkspaceResult | undefined> {
+		return { workspace: await this.getWorkspaceIdentifier(workspaceUri) };
 	}
 
 	async createUntitledWorkspace(folders?: IWorkspaceFolderCreationData[], remoteAuthority?: string): Promise<IWorkspaceIdentifier> {
@@ -200,8 +200,8 @@ export class BrowserWorkspacesService extends Disposable implements IWorkspacesS
 		}
 	}
 
-	async getWorkspaceIdentifier(workspacePath: URI): Promise<IWorkspaceIdentifier> {
-		return getWorkspaceIdentifier(workspacePath);
+	async getWorkspaceIdentifier(workspaceUri: URI): Promise<IWorkspaceIdentifier> {
+		return getWorkspaceIdentifier(workspaceUri);
 	}
 
 	//#endregion

src/vs/workbench/services/workspaces/electron-sandbox/workspaceEditingService.ts

@@ -139,14 +139,14 @@ export class NativeWorkspaceEditingService extends AbstractWorkspaceEditingServi
 		}
 	}
 
-	override async isValidTargetWorkspacePath(path: URI): Promise<boolean> {
+	override async isValidTargetWorkspacePath(workspaceUri: URI): Promise<boolean> {
 		const windows = await this.nativeHostService.getWindows();
 
 		// Prevent overwriting a workspace that is currently opened in another window
-		if (windows.some(window => isWorkspaceIdentifier(window.workspace) && this.uriIdentityService.extUri.isEqual(window.workspace.configPath, path))) {
+		if (windows.some(window => isWorkspaceIdentifier(window.workspace) && this.uriIdentityService.extUri.isEqual(window.workspace.configPath, workspaceUri))) {
 			await this.dialogService.show(
 				Severity.Info,
-				localize('workspaceOpenedMessage', "Unable to save workspace '{0}'", basename(path)),
+				localize('workspaceOpenedMessage', "Unable to save workspace '{0}'", basename(workspaceUri)),
 				undefined,
 				{
 					detail: localize('workspaceOpenedDetail', "The workspace is already opened in another window. Please close that window first and then try again.")
@@ -159,8 +159,8 @@ export class NativeWorkspaceEditingService extends AbstractWorkspaceEditingServi
 		return true; // OK
 	}
 
-	async enterWorkspace(path: URI): Promise<void> {
-		const result = await this.doEnterWorkspace(path);
+	async enterWorkspace(workspaceUri: URI): Promise<void> {
+		const result = await this.doEnterWorkspace(workspaceUri);
 		if (result) {
 
 			// Migrate storage to new workspace