#93960 feedback

- show edit for all machines - include platform in default name
microsoft · May 20, 2020 · 11878fd · 11878fd
1 parent 8253b52
commit 11878fd
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 34 deletions.
diff --git a/src/vs/platform/userDataSync/common/userDataSyncIpc.ts b/src/vs/platform/userDataSync/common/userDataSyncIpc.ts
@@ -177,9 +177,10 @@ export class UserDataSyncMachinesServiceChannel implements IServerChannel {
 	async call(context: any, command: string, args?: any): Promise<any> {
 		switch (command) {
 			case 'getMachines': return this.service.getMachines();
-			case 'updateName': return this.service.updateName(args[0]);
-			case 'unset': return this.service.unset();
-			case 'disable': return this.service.disable(args[0]);
+			case 'addCurrentMachine': return this.service.addCurrentMachine(args[0]);
+			case 'removeCurrentMachine': return this.service.removeCurrentMachine();
+			case 'renameMachine': return this.service.renameMachine(args[0], args[1]);
+			case 'disableMachine': return this.service.disableMachine(args[0]);
 		}
 		throw new Error('Invalid call');
 	}

diff --git a/src/vs/platform/userDataSync/common/userDataSyncMachines.ts b/src/vs/platform/userDataSync/common/userDataSyncMachines.ts
@@ -32,10 +32,12 @@ export interface IUserDataSyncMachinesService {
 	_serviceBrand: any;
 
 	getMachines(manifest?: IUserDataManifest): Promise<IUserDataSyncMachine[]>;
-	updateName(name: string, manifest?: IUserDataManifest): Promise<void>;
-	disable(id: string): Promise<void>
 
-	unset(): Promise<void>;
+	addCurrentMachine(name: string, manifest?: IUserDataManifest): Promise<void>;
+	removeCurrentMachine(manifest?: IUserDataManifest): Promise<void>;
+
+	renameMachine(machineId: string, name: string): Promise<void>;
+	disableMachine(machineId: string): Promise<void>
 }
 
 export class UserDataSyncMachinesService extends Disposable implements IUserDataSyncMachinesService {
@@ -66,7 +68,7 @@ export class UserDataSyncMachinesService extends Disposable implements IUserData
 		return machineData.machines.map<IUserDataSyncMachine>(machine => ({ ...machine, ...{ isCurrent: machine.id === currentMachineId } }));
 	}
 
-	async updateName(name: string, manifest?: IUserDataManifest): Promise<void> {
+	async addCurrentMachine(name: string, manifest?: IUserDataManifest): Promise<void> {
 		const currentMachineId = await this.currentMachineIdPromise;
 		const machineData = await this.readMachinesData(manifest);
 		let currentMachine = machineData.machines.find(({ id }) => id === currentMachineId);
@@ -78,17 +80,26 @@ export class UserDataSyncMachinesService extends Disposable implements IUserData
 		await this.writeMachinesData(machineData);
 	}
 
-	async unset(): Promise<void> {
+	async removeCurrentMachine(manifest?: IUserDataManifest): Promise<void> {
 		const currentMachineId = await this.currentMachineIdPromise;
-		const machineData = await this.readMachinesData();
+		const machineData = await this.readMachinesData(manifest);
 		const updatedMachines = machineData.machines.filter(({ id }) => id !== currentMachineId);
 		if (updatedMachines.length !== machineData.machines.length) {
 			machineData.machines = updatedMachines;
 			await this.writeMachinesData(machineData);
 		}
 	}
 
-	async disable(machineId: string): Promise<void> {
+	async renameMachine(machineId: string, name: string, manifest?: IUserDataManifest): Promise<void> {
+		const machineData = await this.readMachinesData(manifest);
+		const currentMachine = machineData.machines.find(({ id }) => id === machineId);
+		if (currentMachine) {
+			currentMachine.name = name;
+			await this.writeMachinesData(machineData);
+		}
+	}
+
+	async disableMachine(machineId: string): Promise<void> {
 		const machineData = await this.readMachinesData();
 		const machine = machineData.machines.find(({ id }) => id === machineId);
 		if (machine) {

diff --git a/src/vs/platform/userDataSync/common/userDataSyncService.ts b/src/vs/platform/userDataSync/common/userDataSyncService.ts
@@ -20,9 +20,10 @@ import { SettingsSynchroniser } from 'vs/platform/userDataSync/common/settingsSy
 import { isEqual } from 'vs/base/common/resources';
 import { SnippetsSynchroniser } from 'vs/platform/userDataSync/common/snippetsSync';
 import { Throttler } from 'vs/base/common/async';
-import { IUserDataSyncMachinesService } from 'vs/platform/userDataSync/common/userDataSyncMachines';
+import { IUserDataSyncMachinesService, IUserDataSyncMachine } from 'vs/platform/userDataSync/common/userDataSyncMachines';
 import { IProductService } from 'vs/platform/product/common/productService';
-import { isWeb } from 'vs/base/common/platform';
+import { platform, PlatformToString } from 'vs/base/common/platform';
+import { escapeRegExpCharacters } from 'vs/base/common/strings';
 
 type SyncClassification = {
 	source?: { classification: 'SystemMetaData', purpose: 'FeatureInsight', isMeasurement: true };
@@ -152,7 +153,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
 			// Check if sync was turned off from other machine
 			if (currentMachine?.disabled) {
 				// Unset the current machine
-				await this.userDataSyncMachinesService.unset();
+				await this.userDataSyncMachinesService.removeCurrentMachine(manifest || undefined);
 				// Throw TurnedOff error
 				throw new UserDataSyncError(localize('turned off machine', "Cannot sync because syncing is turned off on this machine from another machine."), UserDataSyncErrorCode.TurnedOff);
 			}
@@ -177,17 +178,8 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
 			}
 
 			if (!currentMachine) {
-				// add current machine to sync server
-				const namePrefix = `${this.productService.nameLong}${isWeb ? ' (Web)' : ''}`;
-				const nameRegEx = new RegExp(`${namePrefix}\\s#(\\d)`);
-
-				let nameIndex = 0;
-				for (const machine of machines) {
-					const matches = nameRegEx.exec(machine.name);
-					const index = matches ? parseInt(matches[1]) : 0;
-					nameIndex = index > nameIndex ? index : nameIndex;
-				}
-				await this.userDataSyncMachinesService.updateName(`${namePrefix} #${nameIndex + 1}`, manifest || undefined);
+				const name = this.computeDefaultMachineName(machines);
+				await this.userDataSyncMachinesService.addCurrentMachine(name, manifest || undefined);
 			}
 
 			this.logService.info(`Sync done. Took ${new Date().getTime() - startTime}ms`);
@@ -290,7 +282,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
 		this.storageService.remove(SESSION_ID_KEY, StorageScope.GLOBAL);
 		this.storageService.remove(LAST_SYNC_TIME_KEY, StorageScope.GLOBAL);
 		if (!donotUnsetMachine) {
-			await this.userDataSyncMachinesService.unset();
+			await this.userDataSyncMachinesService.removeCurrentMachine();
 		}
 		for (const synchroniser of this.synchronisers) {
 			try {
@@ -396,6 +388,20 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
 			.map(s => ({ syncResource: s.resource, conflicts: s.conflicts }));
 	}
 
+	private computeDefaultMachineName(machines: IUserDataSyncMachine[]): string {
+		const namePrefix = `${this.productService.nameLong} (${PlatformToString(platform)})`;
+		const nameRegEx = new RegExp(`${escapeRegExpCharacters(namePrefix)}\\s#(\\d)`);
+
+		let nameIndex = 0;
+		for (const machine of machines) {
+			const matches = nameRegEx.exec(machine.name);
+			const index = matches ? parseInt(matches[1]) : 0;
+			nameIndex = index > nameIndex ? index : nameIndex;
+		}
+
+		return `${namePrefix} #${nameIndex + 1}`;
+	}
+
 	getSynchroniser(source: SyncResource): IUserDataSynchroniser {
 		return this.synchronisers.filter(s => s.resource === source)[0];
 	}

diff --git a/src/vs/workbench/contrib/userDataSync/browser/userDataSyncViews.ts b/src/vs/workbench/contrib/userDataSync/browser/userDataSyncViews.ts
@@ -191,7 +191,7 @@ export class UserDataSyncDataViews extends Disposable {
 					icon: Codicon.edit,
 					menu: {
 						id: MenuId.ViewItemContext,
-						when: ContextKeyExpr.and(ContextKeyEqualsExpr.create('view', id), ContextKeyEqualsExpr.create('viewItem', 'current-machine')),
+						when: ContextKeyExpr.and(ContextKeyEqualsExpr.create('view', id), ContextKeyEqualsExpr.create('viewItem', 'sync-machine')),
 						group: 'inline',
 					},
 				});
@@ -207,7 +207,7 @@ export class UserDataSyncDataViews extends Disposable {
 						inputBox.dispose();
 						if (name) {
 							try {
-								await that.userDataSyncMachinesService.updateName(name);
+								await that.userDataSyncMachinesService.renameMachine(handle.$treeItemHandle, name);
 								await treeView.refresh();
 								c();
 							} catch (error) {
@@ -470,7 +470,7 @@ class UserDataSyncMachinesViewDataProvider implements ITreeViewDataProvider {
 			label: { label: name },
 			description: isCurrent ? localize({ key: 'current', comment: ['Current machine'] }, "Current") : undefined,
 			themeIcon: Codicon.vm,
-			contextValue: isCurrent ? 'current-machine' : 'other-machine'
+			contextValue: 'sync-machine'
 		}));
 	}
 }

diff --git a/src/vs/workbench/services/userDataSync/electron-browser/userDataSyncMachinesService.ts b/src/vs/workbench/services/userDataSync/electron-browser/userDataSyncMachinesService.ts
@@ -26,16 +26,20 @@ class UserDataSyncMachinesService extends Disposable implements IUserDataSyncMac
 		return this.channel.call<IUserDataSyncMachine[]>('getMachines');
 	}
 
-	updateName(name: string): Promise<void> {
-		return this.channel.call('updateName', [name]);
+	addCurrentMachine(name: string): Promise<void> {
+		return this.channel.call('addCurrentMachine', [name]);
 	}
 
-	unset(): Promise<void> {
-		return this.channel.call('unset');
+	removeCurrentMachine(): Promise<void> {
+		return this.channel.call('removeCurrentMachine');
 	}
 
-	disable(id: string): Promise<void> {
-		return this.channel.call('disable', [id]);
+	renameMachine(machineId: string, name: string): Promise<void> {
+		return this.channel.call('renameMachine', [machineId, name]);
+	}
+
+	disableMachine(machineId: string): Promise<void> {
+		return this.channel.call('disableMachine', [machineId]);
 	}
 
 }