From 179adc5f7fd6fe7f1398b333a176ec511d51e684 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Tue, 15 Dec 2020 20:00:31 +0100 Subject: [PATCH] support removing a setting from all targets --- .../browser/configurationService.ts | 44 +++++++++++-------- .../configurationService.test.ts | 30 +++++++++++++ 2 files changed, 55 insertions(+), 19 deletions(-) diff --git a/src/vs/workbench/services/configuration/browser/configurationService.ts b/src/vs/workbench/services/configuration/browser/configurationService.ts index 3218d90d3ab..d858cfd0b1a 100644 --- a/src/vs/workbench/services/configuration/browser/configurationService.ts +++ b/src/vs/workbench/services/configuration/browser/configurationService.ts @@ -293,19 +293,20 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat async updateValue(key: string, value: any, arg3?: any, arg4?: any, donotNotifyError?: any): Promise { await this.cyclicDependency; const overrides = isConfigurationOverrides(arg3) ? arg3 : undefined; - let target: ConfigurationTarget | undefined = overrides ? arg4 : arg3; + const target: ConfigurationTarget | undefined = overrides ? arg4 : arg3; + const targets: ConfigurationTarget[] = target ? [target] : []; - if (!target) { + if (!targets.length) { const inspect = this.inspect(key, overrides); - target = this.deriveConfigurationTarget(key, value, inspect); - if (target === ConfigurationTarget.USER && equals(value, inspect.defaultValue)) { + targets.push(...this.deriveConfigurationTargets(key, value, inspect)); + + // Remove the setting, if the value is same as default value and is updated only in user target + if (equals(value, inspect.defaultValue) && targets.length === 1 && (targets[0] === ConfigurationTarget.USER || targets[0] === ConfigurationTarget.USER_LOCAL)) { value = undefined; } } - if (target) { - await this.writeConfigurationValue(key, value, target, overrides, donotNotifyError); - } + await Promise.all(targets.map(target => this.writeConfigurationValue(key, value, target, overrides, donotNotifyError))); } async reloadConfiguration(target?: ConfigurationTarget | IWorkspaceFolder): Promise { @@ -759,26 +760,31 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat }); } - private deriveConfigurationTarget(key: string, value: any, inspect: IConfigurationValue): ConfigurationTarget | undefined { - if (value === undefined) { - // Ignore. But expected is to remove the value from all targets - return undefined; - } - + private deriveConfigurationTargets(key: string, value: any, inspect: IConfigurationValue): ConfigurationTarget[] { if (equals(value, inspect.value)) { - // No change. So ignore. - return undefined; + return []; } + const definedTargets: ConfigurationTarget[] = []; if (inspect.workspaceFolderValue !== undefined) { - return ConfigurationTarget.WORKSPACE_FOLDER; + definedTargets.push(ConfigurationTarget.WORKSPACE_FOLDER); } - if (inspect.workspaceValue !== undefined) { - return ConfigurationTarget.WORKSPACE; + definedTargets.push(ConfigurationTarget.WORKSPACE); + } + if (inspect.userRemoteValue !== undefined) { + definedTargets.push(ConfigurationTarget.USER_REMOTE); + } + if (inspect.userLocalValue !== undefined) { + definedTargets.push(ConfigurationTarget.USER_LOCAL); + } + + if (value === undefined) { + // Remove the setting in all defined targets + return definedTargets; } - return ConfigurationTarget.USER; + return [definedTargets[0] || ConfigurationTarget.USER]; } private triggerConfigurationChange(change: IConfigurationChange, previous: { data: IConfigurationData, workspace?: Workspace } | undefined, target: ConfigurationTarget): void { diff --git a/src/vs/workbench/services/configuration/test/electron-browser/configurationService.test.ts b/src/vs/workbench/services/configuration/test/electron-browser/configurationService.test.ts index 27c9d2abe5a..7629a329976 100644 --- a/src/vs/workbench/services/configuration/test/electron-browser/configurationService.test.ts +++ b/src/vs/workbench/services/configuration/test/electron-browser/configurationService.test.ts @@ -1174,6 +1174,20 @@ suite('WorkspaceConfigurationService - Folder', () => { .then(() => assert.ok(target.called)); }); + test('remove setting from all targets', async () => { + const key = 'configurationService.folder.testSetting'; + await testObject.updateValue(key, 'workspaceValue', ConfigurationTarget.WORKSPACE); + await testObject.updateValue(key, 'userValue', ConfigurationTarget.USER); + + await testObject.updateValue(key, undefined); + await testObject.reloadConfiguration(); + + const actual = testObject.inspect(key, { resource: workspaceService.getWorkspace().folders[0].uri }); + assert.equal(actual.userValue, undefined); + assert.equal(actual.workspaceValue, undefined); + assert.equal(actual.workspaceFolderValue, undefined); + }); + test('update user configuration to default value when target is not passed', async () => { await testObject.updateValue('configurationService.folder.testSetting', 'value', ConfigurationTarget.USER); await testObject.updateValue('configurationService.folder.testSetting', 'isSet'); @@ -1805,6 +1819,22 @@ suite('WorkspaceConfigurationService-Multiroot', () => { .then(() => assert.ok(target.called)); }); + test('remove setting from all targets', async () => { + const workspace = workspaceContextService.getWorkspace(); + const key = 'configurationService.workspace.testResourceSetting'; + await testObject.updateValue(key, 'workspaceFolderValue', { resource: workspace.folders[0].uri }, ConfigurationTarget.WORKSPACE_FOLDER); + await testObject.updateValue(key, 'workspaceValue', ConfigurationTarget.WORKSPACE); + await testObject.updateValue(key, 'userValue', ConfigurationTarget.USER); + + await testObject.updateValue(key, undefined, { resource: workspace.folders[0].uri }); + await testObject.reloadConfiguration(); + + const actual = testObject.inspect(key, { resource: workspace.folders[0].uri }); + assert.equal(actual.userValue, undefined); + assert.equal(actual.workspaceValue, undefined); + assert.equal(actual.workspaceFolderValue, undefined); + }); + test('update tasks configuration in a folder', () => { const workspace = workspaceContextService.getWorkspace(); return testObject.updateValue('tasks', { 'version': '1.0.0', tasks: [{ 'taskName': 'myTask' }] }, { resource: workspace.folders[0].uri }, ConfigurationTarget.WORKSPACE_FOLDER) -- GitLab