fix #194427

Author: sandy081

src/vs/platform/configuration/common/configurationModels.ts

@@ -912,7 +912,7 @@ export class Configuration {
 		return this._workspaceConfiguration;
 	}
 
-	protected get folderConfigurations(): ResourceMap<ConfigurationModel> {
+	get folderConfigurations(): ResourceMap<ConfigurationModel> {
 		return this._folderConfigurations;
 	}
 

src/vs/workbench/common/configuration.ts

@@ -13,6 +13,8 @@ import { Disposable } from 'vs/base/common/lifecycle';
 import { Emitter } from 'vs/base/common/event';
 import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
 import { OperatingSystem, isWindows } from 'vs/base/common/platform';
+import { URI } from 'vs/base/common/uri';
+import { equals } from 'vs/base/common/objects';
 
 export const applicationConfigurationNodeBase = Object.freeze<IConfigurationNode>({
 	'id': 'application',
@@ -89,35 +91,56 @@ export class ConfigurationMigrationWorkbenchContribution extends Disposable impl
 	}
 
 	private async migrateConfigurationsForFolder(folder: IWorkspaceFolder | undefined, migrations: ConfigurationMigration[]): Promise<void> {
-		await Promise.all(migrations.map(migration => this.migrateConfigurationsForFolderAndOverride(migration, { resource: folder?.uri })));
+		await Promise.all([migrations.map(migration => this.migrateConfigurationsForFolderAndOverride(migration, folder?.uri))]);
 	}
 
-	private async migrateConfigurationsForFolderAndOverride(migration: ConfigurationMigration, overrides: IConfigurationOverrides): Promise<void> {
-		const data = this.configurationService.inspect(migration.key, overrides);
+	private async migrateConfigurationsForFolderAndOverride(migration: ConfigurationMigration, resource?: URI): Promise<void> {
+		const inspectData = this.configurationService.inspect(migration.key, { resource });
+
+		const targetPairs: [keyof IConfigurationValue<any>, ConfigurationTarget][] = [
+			['userValue', ConfigurationTarget.USER],
+			['userLocalValue', ConfigurationTarget.USER_LOCAL],
+			['userRemoteValue', ConfigurationTarget.USER_REMOTE],
+			['workspaceValue', ConfigurationTarget.WORKSPACE],
+			['workspaceFolderValue', ConfigurationTarget.WORKSPACE_FOLDER],
+		];
+		for (const [dataKey, target] of targetPairs) {
+			const migrationValues: [[string, ConfigurationValue], string[]][] = [];
+
+			// Collect migrations for language overrides
+			for (const overrideIdentifier of inspectData.overrideIdentifiers ?? []) {
+				const keyValuePairs = await this.runMigration(migration, { resource, overrideIdentifier }, dataKey);
+				for (const keyValuePair of keyValuePairs ?? []) {
+					let keyValueAndOverridesPair = migrationValues.find(([[k, v]]) => k === keyValuePair[0] && equals(v.value, keyValuePair[1].value));
+					if (!keyValueAndOverridesPair) {
+						migrationValues.push(keyValueAndOverridesPair = [keyValuePair, []]);
+					}
+					keyValueAndOverridesPair[1].push(overrideIdentifier);
+				}
+			}
 
-		await this.migrateConfigurationForFolderOverrideAndTarget(migration, overrides, data, 'userValue', ConfigurationTarget.USER);
-		await this.migrateConfigurationForFolderOverrideAndTarget(migration, overrides, data, 'userLocalValue', ConfigurationTarget.USER_LOCAL);
-		await this.migrateConfigurationForFolderOverrideAndTarget(migration, overrides, data, 'userRemoteValue', ConfigurationTarget.USER_REMOTE);
-		await this.migrateConfigurationForFolderOverrideAndTarget(migration, overrides, data, 'workspaceFolderValue', ConfigurationTarget.WORKSPACE_FOLDER);
-		await this.migrateConfigurationForFolderOverrideAndTarget(migration, overrides, data, 'workspaceValue', ConfigurationTarget.WORKSPACE);
+			// Collect migrations
+			const keyValuePairs = await this.runMigration(migration, { resource }, dataKey, inspectData);
+			for (const keyValuePair of keyValuePairs ?? []) {
+				migrationValues.push([keyValuePair, []]);
+			}
 
-		if (typeof overrides.overrideIdentifier === 'undefined' && typeof data.overrideIdentifiers !== 'undefined') {
-			for (const overrideIdentifier of data.overrideIdentifiers) {
-				await this.migrateConfigurationsForFolderAndOverride(migration, { resource: overrides.resource, overrideIdentifier });
+			if (migrationValues.length) {
+				// apply migrations
+				await Promise.allSettled(migrationValues.map(async ([[key, value], overrideIdentifiers]) =>
+					this.configurationService.updateValue(key, value.value, { resource, overrideIdentifiers }, target)));
 			}
 		}
 	}
 
-	private async migrateConfigurationForFolderOverrideAndTarget(migration: ConfigurationMigration, overrides: IConfigurationOverrides, data: IConfigurationValue<any>, dataKey: keyof IConfigurationValue<any>, target: ConfigurationTarget): Promise<void> {
-		const value = data[dataKey];
-		if (typeof value === 'undefined') {
-			return;
+	private async runMigration(migration: ConfigurationMigration, overrides: IConfigurationOverrides, dataKey: keyof IConfigurationValue<any>, data?: IConfigurationValue<any>): Promise<ConfigurationKeyValuePairs | undefined> {
+		const value = (data ?? this.configurationService.inspect(migration.key, overrides))[dataKey];
+		if (value === undefined) {
+			return undefined;
 		}
-
 		const valueAccessor = (key: string) => this.configurationService.inspect(key, overrides)[dataKey];
 		const result = await migration.migrateFn(value, valueAccessor);
-		const keyValuePairs: ConfigurationKeyValuePairs = Array.isArray(result) ? result : [[migration.key, result]];
-		await Promise.allSettled(keyValuePairs.map(async ([key, value]) => this.configurationService.updateValue(key, value.value, overrides, target)));
+		return Array.isArray(result) ? result : [[migration.key, result]];
 	}
 }
 

src/vs/workbench/services/configuration/browser/configurationService.ts

@@ -33,7 +33,7 @@ import { ILogService } from 'vs/platform/log/common/log';
 import { toErrorMessage } from 'vs/base/common/errorMessage';
 import { IUriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentity';
 import { IWorkspaceTrustManagementService } from 'vs/platform/workspace/common/workspaceTrust';
-import { delta, distinct } from 'vs/base/common/arrays';
+import { delta, distinct, equals as arrayEquals } from 'vs/base/common/arrays';
 import { IStringDictionary } from 'vs/base/common/collections';
 import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
 import { IWorkbenchAssignmentService } from 'vs/workbench/services/assignment/common/assignmentService';
@@ -1017,6 +1017,17 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat
 			throw new Error('Invalid configuration target');
 		}
 
+		if (overrides?.overrideIdentifiers?.length && overrides.overrideIdentifiers.length > 1) {
+			const configurationModel = this.getConfigurationModel(editableConfigurationTarget, overrides.resource);
+			if (configurationModel) {
+				const overrideIdentifiers = overrides.overrideIdentifiers.sort();
+				const existingOverrides = configurationModel.overrides.find(override => arrayEquals([...override.identifiers].sort(), overrideIdentifiers));
+				if (existingOverrides) {
+					overrides.overrideIdentifiers = existingOverrides.identifiers;
+				}
+			}
+		}
+
 		// Use same instance of ConfigurationEditing to make sure all writes go through the same queue
 		this.configurationEditing = this.configurationEditing ?? this.instantiationService.createInstance(ConfigurationEditing, (await this.remoteAgentService.getEnvironment())?.settingsPath ?? null);
 		await this.configurationEditing.writeConfiguration(editableConfigurationTarget, { key, value }, { scopes: overrides, ...options });
@@ -1041,6 +1052,15 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat
 		}
 	}
 
+	private getConfigurationModel(target: EditableConfigurationTarget, resource?: URI | null): ConfigurationModel | undefined {
+		switch (target) {
+			case EditableConfigurationTarget.USER_LOCAL: return this._configuration.localUserConfiguration;
+			case EditableConfigurationTarget.USER_REMOTE: return this._configuration.remoteUserConfiguration;
+			case EditableConfigurationTarget.WORKSPACE: return this._configuration.workspaceConfiguration;
+			case EditableConfigurationTarget.WORKSPACE_FOLDER: return resource ? this._configuration.folderConfigurations.get(resource) : undefined;
+		}
+	}
+
 	private deriveConfigurationTargets(key: string, value: any, inspect: IConfigurationValue<any>): ConfigurationTarget[] {
 		if (equals(value, inspect.value)) {
 			return [];

src/vs/workbench/services/configuration/test/browser/configurationService.test.ts

@@ -1295,12 +1295,22 @@ suite('WorkspaceConfigurationService - Folder', () => {
 	}));
 
 	test('update language configuration for multiple languages', () => runWithFakedTimers<void>({ useFakeTimers: true }, async () => {
-		await testObject.updateValue('configurationService.folder.languageSetting', 'multiLangValue', { overrideIdentifiers: ['deflang', 'xyzlang'] }, ConfigurationTarget.USER);
+		await testObject.updateValue('configurationService.folder.languageSetting', 'multiLangValue', { overrideIdentifiers: ['xyzlang', 'deflang'] }, ConfigurationTarget.USER);
 		assert.strictEqual(testObject.getValue('configurationService.folder.languageSetting', { overrideIdentifier: 'deflang' }), 'multiLangValue');
 		assert.strictEqual(testObject.getValue('configurationService.folder.languageSetting', { overrideIdentifier: 'xyzlang' }), 'multiLangValue');
 		assert.deepStrictEqual(testObject.getValue(keyFromOverrideIdentifiers(['deflang', 'xyzlang'])), { 'configurationService.folder.languageSetting': 'multiLangValue' });
 	}));
 
+	test('update language configuration for multiple languages when already set', () => runWithFakedTimers<void>({ useFakeTimers: true }, async () => {
+		await fileService.writeFile(userDataProfileService.currentProfile.settingsResource, VSBuffer.fromString('{ "[deflang][xyzlang]": { "configurationService.folder.languageSetting": "userValue" }}'));
+		await testObject.updateValue('configurationService.folder.languageSetting', 'multiLangValue', { overrideIdentifiers: ['xyzlang', 'deflang'] }, ConfigurationTarget.USER);
+		assert.strictEqual(testObject.getValue('configurationService.folder.languageSetting', { overrideIdentifier: 'deflang' }), 'multiLangValue');
+		assert.strictEqual(testObject.getValue('configurationService.folder.languageSetting', { overrideIdentifier: 'xyzlang' }), 'multiLangValue');
+		assert.deepStrictEqual(testObject.getValue(keyFromOverrideIdentifiers(['deflang', 'xyzlang'])), { 'configurationService.folder.languageSetting': 'multiLangValue' });
+		const actualContent = (await fileService.readFile(userDataProfileService.currentProfile.settingsResource)).value.toString();
+		assert.deepStrictEqual(JSON.parse(actualContent), { '[deflang][xyzlang]': { 'configurationService.folder.languageSetting': 'multiLangValue' } });
+	}));
+
 	test('update resource language configuration', () => runWithFakedTimers<void>({ useFakeTimers: true }, async () => {
 		await testObject.updateValue('configurationService.folder.languageSetting', 'value', { resource: workspaceService.getWorkspace().folders[0].uri }, ConfigurationTarget.WORKSPACE_FOLDER);
 		assert.strictEqual(testObject.getValue('configurationService.folder.languageSetting'), 'value');