Fix #121804 (#169128)

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

@@ -99,6 +99,17 @@ export interface IConfigurationValue<T> {
  readonly overrideIdentifiers?: string[];
 }
 
+export interface IConfigurationUpdateOptions {
+ /**
+ * If `true`, do not notifies the error to user by showing the message box. Default is `false`.
+ */
+ donotNotifyError?: boolean;
+ /**
+ * How to handle dirty file when updating the configuration.
+ */
+ handleDirtyFile?: 'save' | 'revert';
+}
+
 export interface IConfigurationService {
  readonly _serviceBrand: undefined;
 
@@ -140,7 +151,7 @@ export interface IConfigurationService {
  updateValue(key: string, value: any): Promise<void>;
  updateValue(key: string, value: any, target: ConfigurationTarget): Promise<void>;
  updateValue(key: string, value: any, overrides: IConfigurationOverrides | IConfigurationUpdateOverrides): Promise<void>;
- updateValue(key: string, value: any, overrides: IConfigurationOverrides | IConfigurationUpdateOverrides, target: ConfigurationTarget, donotNotifyError?: boolean): Promise<void>;
+ updateValue(key: string, value: any, overrides: IConfigurationOverrides | IConfigurationUpdateOverrides, target: ConfigurationTarget, options?: IConfigurationUpdateOptions): Promise<void>;
 
  inspect<T>(key: string, overrides?: IConfigurationOverrides): IConfigurationValue<Readonly<T>>;
 

src/vs/workbench/api/browser/mainThreadConfiguration.ts

@@ -77,7 +77,7 @@ export class MainThreadConfiguration implements MainThreadConfigurationShape {
  : scopeToLanguage === false ? { resource: overrides.resource }
  : overrides.overrideIdentifier && overriddenValue !== undefined ? overrides
  : { resource: overrides.resource };
- return this.configurationService.updateValue(key, value, overrides, configurationTarget, true);
+ return this.configurationService.updateValue(key, value, overrides, configurationTarget, { donotNotifyError: true });
  }
 
  private deriveConfigurationTarget(key: string, overrides: IConfigurationOverrides): ConfigurationTarget {

src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts

@@ -1078,7 +1078,7 @@ export class SettingsEditor2 extends EditorPane {
  value = undefined;
  }
 
- return this.configurationService.updateValue(key, value, overrides, configurationTarget)
+ return this.configurationService.updateValue(key, value, overrides, configurationTarget, { handleDirtyFile: 'save' })
  .then(() => {
  const query = this.searchWidget.getValue();
  if (query.includes(`@${MODIFIED_SETTING_TAG}`)) {

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

@@ -12,7 +12,7 @@ import { Queue, Barrier, runWhenIdle, Promises } from 'vs/base/common/async';
 import { IJSONContributionRegistry, Extensions as JSONExtensions } from 'vs/platform/jsonschemas/common/jsonContributionRegistry';
 import { IWorkspaceContextService, Workspace as BaseWorkspace, WorkbenchState, IWorkspaceFolder, IWorkspaceFoldersChangeEvent, WorkspaceFolder, toWorkspaceFolder, isWorkspaceFolder, IWorkspaceFoldersWillChangeEvent, IEmptyWorkspaceIdentifier, ISingleFolderWorkspaceIdentifier, isSingleFolderWorkspaceIdentifier, isWorkspaceIdentifier, IWorkspaceIdentifier, IAnyWorkspaceIdentifier } from 'vs/platform/workspace/common/workspace';
 import { ConfigurationModel, ConfigurationChangeEvent, mergeChanges } from 'vs/platform/configuration/common/configurationModels';
-import { IConfigurationChangeEvent, ConfigurationTarget, IConfigurationOverrides, isConfigurationOverrides, IConfigurationData, IConfigurationValue, IConfigurationChange, ConfigurationTargetToString, IConfigurationUpdateOverrides, isConfigurationUpdateOverrides, IConfigurationService } from 'vs/platform/configuration/common/configuration';
+import { IConfigurationChangeEvent, ConfigurationTarget, IConfigurationOverrides, isConfigurationOverrides, IConfigurationData, IConfigurationValue, IConfigurationChange, ConfigurationTargetToString, IConfigurationUpdateOverrides, isConfigurationUpdateOverrides, IConfigurationService, IConfigurationUpdateOptions } from 'vs/platform/configuration/common/configuration';
 import { IPolicyConfiguration, NullPolicyConfiguration, PolicyConfiguration } from 'vs/platform/configuration/common/configurations';
 import { Configuration } from 'vs/workbench/services/configuration/common/configurationModels';
 import { FOLDER_CONFIG_FOLDER_NAME, defaultSettingsSchemaId, userSettingsSchemaId, workspaceSettingsSchemaId, folderSettingsSchemaId, IConfigurationCache, machineSettingsSchemaId, LOCAL_MACHINE_SCOPES, IWorkbenchConfigurationService, RestrictedSettings, PROFILE_SCOPES, LOCAL_MACHINE_PROFILE_SCOPES, profileSettingsSchemaId } from 'vs/workbench/services/configuration/common/configuration';
@@ -330,8 +330,8 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat
  updateValue(key: string, value: any): Promise<void>;
  updateValue(key: string, value: any, overrides: IConfigurationOverrides | IConfigurationUpdateOverrides): Promise<void>;
  updateValue(key: string, value: any, target: ConfigurationTarget): Promise<void>;
- updateValue(key: string, value: any, overrides: IConfigurationOverrides | IConfigurationUpdateOverrides, target: ConfigurationTarget, donotNotifyError?: boolean): Promise<void>;
- async updateValue(key: string, value: any, arg3?: any, arg4?: any, donotNotifyError?: any): Promise<void> {
+ updateValue(key: string, value: any, overrides: IConfigurationOverrides | IConfigurationUpdateOverrides, target: ConfigurationTarget, options?: IConfigurationUpdateOptions): Promise<void>;
+ async updateValue(key: string, value: any, arg3?: any, arg4?: any, options?: any): Promise<void> {
  const overrides: IConfigurationUpdateOverrides | undefined = isConfigurationUpdateOverrides(arg3) ? arg3
  : isConfigurationOverrides(arg3) ? { resource: arg3.resource, overrideIdentifiers: arg3.overrideIdentifier ? [arg3.overrideIdentifier] : undefined } : undefined;
  const target: ConfigurationTarget | undefined = overrides ? arg4 : arg3;
@@ -355,7 +355,7 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat
  }
  }
 
- await Promises.settled(targets.map(target => this.writeConfigurationValue(key, value, target, overrides, donotNotifyError)));
+ await Promises.settled(targets.map(target => this.writeConfigurationValue(key, value, target, overrides, options)));
  }
 
  async reloadConfiguration(target?: ConfigurationTarget | IWorkspaceFolder): Promise<void> {
@@ -971,7 +971,7 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat
  return validWorkspaceFolders;
  }
 
- private async writeConfigurationValue(key: string, value: any, target: ConfigurationTarget, overrides: IConfigurationUpdateOverrides | undefined, donotNotifyError: boolean): Promise<void> {
+ private async writeConfigurationValue(key: string, value: any, target: ConfigurationTarget, overrides: IConfigurationUpdateOverrides | undefined, options?: IConfigurationUpdateOverrides): Promise<void> {
  if (!this.instantiationService) {
  throw new Error('Cannot write configuration because the configuration service is not yet ready to accept writes.');
  }
@@ -998,7 +998,7 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat
 
  // 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, donotNotifyError });
+ await this.configurationEditing.writeConfiguration(editableConfigurationTarget, { key, value }, { scopes: overrides, ...options });
  switch (editableConfigurationTarget) {
  case EditableConfigurationTarget.USER_LOCAL:
  if (this.applicationConfiguration && this.configurationRegistry.getConfigurationProperties()[key].scope === ConfigurationScope.APPLICATION) {

src/vs/workbench/services/configuration/common/configurationEditing.ts

@@ -12,7 +12,7 @@ import { Edit, FormattingOptions } from 'vs/base/common/jsonFormatter';
 import { Registry } from 'vs/platform/registry/common/platform';
 import { IWorkspaceContextService, WorkbenchState } from 'vs/platform/workspace/common/workspace';
 import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles';
-import { IConfigurationService, IConfigurationUpdateOverrides } from 'vs/platform/configuration/common/configuration';
+import { IConfigurationService, IConfigurationUpdateOptions, IConfigurationUpdateOverrides } from 'vs/platform/configuration/common/configuration';
 import { FOLDER_SETTINGS_PATH, WORKSPACE_STANDALONE_CONFIGURATIONS, TASKS_CONFIGURATION_KEY, LAUNCH_CONFIGURATION_KEY, USER_STANDALONE_CONFIGURATIONS, TASKS_DEFAULT, FOLDER_SCOPES } from 'vs/workbench/services/configuration/common/configuration';
 import { FileOperationError, FileOperationResult, IFileService } from 'vs/platform/files/common/files';
 import { IResolvedTextEditorModel, ITextModelService } from 'vs/editor/common/services/resolverService';
@@ -114,11 +114,7 @@ export interface IConfigurationValue {
  value: any;
 }
 
-export interface IConfigurationEditingOptions {
- /**
- * If `true`, do not notifies the error to user by showing the message box. Default is `false`.
- */
- donotNotifyError?: boolean;
+export interface IConfigurationEditingOptions extends IConfigurationUpdateOptions {
  /**
  * Scope of configuration to be written into.
  */
@@ -139,10 +135,6 @@ interface IConfigurationEditOperation extends IConfigurationValue {
  workspaceStandAloneConfigurationKey?: string;
 }
 
-interface ConfigurationEditingOptions extends IConfigurationEditingOptions {
- handleDirtyFile?: 'save' | 'revert';
-}
-
 export class ConfigurationEditing {
 
  public _serviceBrand: undefined;
@@ -181,7 +173,7 @@ export class ConfigurationEditing {
  });
  }
 
- private async doWriteConfiguration(operation: IConfigurationEditOperation, options: ConfigurationEditingOptions): Promise<void> {
+ private async doWriteConfiguration(operation: IConfigurationEditOperation, options: IConfigurationEditingOptions): Promise<void> {
  await this.validate(operation.target, operation, !options.handleDirtyFile, options.scopes || {});
  const resource: URI = operation.resource!;
  const reference = await this.resolveModelReference(resource);
@@ -193,7 +185,7 @@ export class ConfigurationEditing {
  }
  }
 
- private async updateConfiguration(operation: IConfigurationEditOperation, model: ITextModel, formattingOptions: FormattingOptions, options: ConfigurationEditingOptions): Promise<void> {
+ private async updateConfiguration(operation: IConfigurationEditOperation, model: ITextModel, formattingOptions: FormattingOptions, options: IConfigurationEditingOptions): Promise<void> {
  if (this.hasParseErrors(model.getValue(), operation)) {
  throw this.toConfigurationEditingError(ConfigurationEditingErrorCode.ERROR_INVALID_CONFIGURATION, operation.target, operation);
  }
@@ -301,7 +293,7 @@ export class ConfigurationEditing {
  label: nls.localize('saveAndRetry', "Save and Retry"),
  run: () => {
  const key = operation.key ? `${operation.workspaceStandAloneConfigurationKey}.${operation.key}` : operation.workspaceStandAloneConfigurationKey!;
- this.writeConfiguration(operation.target, { key, value: operation.value }, <ConfigurationEditingOptions>{ handleDirtyFile: 'save', scopes });
+ this.writeConfiguration(operation.target, { key, value: operation.value }, <IConfigurationEditingOptions>{ handleDirtyFile: 'save', scopes });
  }
  },
  {
@@ -313,7 +305,7 @@ export class ConfigurationEditing {
  this.notificationService.prompt(Severity.Error, error.message,
  [{
  label: nls.localize('saveAndRetry', "Save and Retry"),
- run: () => this.writeConfiguration(operation.target, { key: operation.key, value: operation.value }, <ConfigurationEditingOptions>{ handleDirtyFile: 'save', scopes })
+ run: () => this.writeConfiguration(operation.target, { key: operation.key, value: operation.value }, <IConfigurationEditingOptions>{ handleDirtyFile: 'save', scopes })
  },
  {
  label: nls.localize('open', "Open Settings"),

src/vs/workbench/services/configuration/common/jsonEditing.ts

@@ -11,11 +11,6 @@ export const IJSONEditingService = createDecorator<IJSONEditingService>('jsonEdi
 
 export const enum JSONEditingErrorCode {
 
- /**
- * Error when trying to write and save to the file while it is dirty in the editor.
- */
- ERROR_FILE_DIRTY,
-
  /**
  * Error when trying to write to a file that contains JSON errors.
  */

src/vs/workbench/services/configuration/common/jsonEditingService.ts

@@ -34,26 +34,26 @@ export class JSONEditingService implements IJSONEditingService {
  this.queue = new Queue<void>();
  }
 
- write(resource: URI, values: IJSONValue[], save: boolean): Promise<void> {
- return Promise.resolve(this.queue.queue(() => this.doWriteConfiguration(resource, values, save))); // queue up writes to prevent race conditions
+ write(resource: URI, values: IJSONValue[]): Promise<void> {
+ return Promise.resolve(this.queue.queue(() => this.doWriteConfiguration(resource, values))); // queue up writes to prevent race conditions
  }
 
- private async doWriteConfiguration(resource: URI, values: IJSONValue[], save: boolean): Promise<void> {
- const reference = await this.resolveAndValidate(resource, save);
+ private async doWriteConfiguration(resource: URI, values: IJSONValue[]): Promise<void> {
+ const reference = await this.resolveAndValidate(resource, true);
  try {
- await this.writeToBuffer(reference.object.textEditorModel, values, save);
+ await this.writeToBuffer(reference.object.textEditorModel, values);
  } finally {
  reference.dispose();
  }
  }
 
- private async writeToBuffer(model: ITextModel, values: IJSONValue[], save: boolean): Promise<any> {
+ private async writeToBuffer(model: ITextModel, values: IJSONValue[]): Promise<any> {
  let hasEdits: boolean = false;
  for (const value of values) {
  const edit = this.getEdits(model, value)[0];
  hasEdits = this.applyEditsToBuffer(edit, model);
  }
- if (hasEdits && save) {
+ if (hasEdits) {
  return this.textFileService.save(model.uri);
  }
  }
@@ -113,12 +113,6 @@ export class JSONEditingService implements IJSONEditingService {
  return this.reject<IReference<IResolvedTextEditorModel>>(JSONEditingErrorCode.ERROR_INVALID_FILE);
  }
 
- // Target cannot be dirty if not writing into buffer
- if (checkDirty && this.textFileService.isDirty(resource)) {
- reference.dispose();
- return this.reject<IReference<IResolvedTextEditorModel>>(JSONEditingErrorCode.ERROR_FILE_DIRTY);
- }
-
  return reference;
  }
 
@@ -133,9 +127,6 @@ export class JSONEditingService implements IJSONEditingService {
  case JSONEditingErrorCode.ERROR_INVALID_FILE: {
  return nls.localize('errorInvalidFile', "Unable to write into the file. Please open the file to correct errors/warnings in the file and try again.");
  }
- case JSONEditingErrorCode.ERROR_FILE_DIRTY: {
- return nls.localize('errorFileDirty', "Unable to write into the file because the file has unsaved changes. Please save the file and try again.");
- }
  }
  }
 }

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

@@ -1277,12 +1277,12 @@ suite('WorkspaceConfigurationService - Folder', () => {
  }));
 
  test('update application setting into workspace configuration in a workspace is not supported', () => {
- return testObject.updateValue('configurationService.folder.applicationSetting', 'workspaceValue', {}, ConfigurationTarget.WORKSPACE, true)
+ return testObject.updateValue('configurationService.folder.applicationSetting', 'workspaceValue', {}, ConfigurationTarget.WORKSPACE, { donotNotifyError: true })
  .then(() => assert.fail('Should not be supported'), (e) => assert.strictEqual(e.code, ConfigurationEditingErrorCode.ERROR_INVALID_WORKSPACE_CONFIGURATION_APPLICATION));
  });
 
  test('update machine setting into workspace configuration in a workspace is not supported', () => {
- return testObject.updateValue('configurationService.folder.machineSetting', 'workspaceValue', {}, ConfigurationTarget.WORKSPACE, true)
+ return testObject.updateValue('configurationService.folder.machineSetting', 'workspaceValue', {}, ConfigurationTarget.WORKSPACE, { donotNotifyError: true })
  .then(() => assert.fail('Should not be supported'), (e) => assert.strictEqual(e.code, ConfigurationEditingErrorCode.ERROR_INVALID_WORKSPACE_CONFIGURATION_MACHINE));
  });
 
@@ -1949,7 +1949,7 @@ suite('WorkspaceConfigurationService-Multiroot', () => {
  assert.strictEqual(testObject.getValue('configurationService.workspace.testNewApplicationSetting', { resource: workspaceContextService.getWorkspace().folders[0].uri }), 'userValue');
  }));
 
- test('application settings are not read from workspace folder after defaults are registered', () => runWithFakedTimers<void>({ useFakeTimers: true }, async () => {
+ test('machine settings are not read from workspace folder after defaults are registered', () => runWithFakedTimers<void>({ useFakeTimers: true }, async () => {
  await fileService.writeFile(userDataProfileService.currentProfile.settingsResource, VSBuffer.fromString('{ "configurationService.workspace.testNewMachineSetting": "userValue" }'));
  await fileService.writeFile(workspaceContextService.getWorkspace().folders[0].toResource('.vscode/settings.json'), VSBuffer.fromString('{ "configurationService.workspace.testNewMachineSetting": "workspaceFolderValue" }'));
  await testObject.reloadConfiguration();
@@ -2261,12 +2261,12 @@ suite('WorkspaceConfigurationService-Multiroot', () => {
  }));
 
  test('update application setting into workspace configuration in a workspace is not supported', () => {
- return testObject.updateValue('configurationService.workspace.applicationSetting', 'workspaceValue', {}, ConfigurationTarget.WORKSPACE, true)
+ return testObject.updateValue('configurationService.workspace.applicationSetting', 'workspaceValue', {}, ConfigurationTarget.WORKSPACE, { donotNotifyError: true })
  .then(() => assert.fail('Should not be supported'), (e) => assert.strictEqual(e.code, ConfigurationEditingErrorCode.ERROR_INVALID_WORKSPACE_CONFIGURATION_APPLICATION));
  });
 
  test('update machine setting into workspace configuration in a workspace is not supported', () => {
- return testObject.updateValue('configurationService.workspace.machineSetting', 'workspaceValue', {}, ConfigurationTarget.WORKSPACE, true)
+ return testObject.updateValue('configurationService.workspace.machineSetting', 'workspaceValue', {}, ConfigurationTarget.WORKSPACE, { donotNotifyError: true })
  .then(() => assert.fail('Should not be supported'), (e) => assert.strictEqual(e.code, ConfigurationEditingErrorCode.ERROR_INVALID_WORKSPACE_CONFIGURATION_MACHINE));
  });
 
@@ -2341,14 +2341,14 @@ suite('WorkspaceConfigurationService-Multiroot', () => {
 
  test('update launch configuration in a workspace', () => runWithFakedTimers<void>({ useFakeTimers: true }, async () => {
  const workspace = workspaceContextService.getWorkspace();
- await testObject.updateValue('launch', { 'version': '1.0.0', configurations: [{ 'name': 'myLaunch' }] }, { resource: workspace.folders[0].uri }, ConfigurationTarget.WORKSPACE, true);
+ await testObject.updateValue('launch', { 'version': '1.0.0', configurations: [{ 'name': 'myLaunch' }] }, { resource: workspace.folders[0].uri }, ConfigurationTarget.WORKSPACE, { donotNotifyError: true });
  assert.deepStrictEqual(testObject.getValue('launch'), { 'version': '1.0.0', configurations: [{ 'name': 'myLaunch' }] });
  }));
 
  test('update tasks configuration in a workspace', () => runWithFakedTimers<void>({ useFakeTimers: true }, async () => {
  const workspace = workspaceContextService.getWorkspace();
  const tasks = { 'version': '2.0.0', tasks: [{ 'label': 'myTask' }] };
- await testObject.updateValue('tasks', tasks, { resource: workspace.folders[0].uri }, ConfigurationTarget.WORKSPACE, true);
+ await testObject.updateValue('tasks', tasks, { resource: workspace.folders[0].uri }, ConfigurationTarget.WORKSPACE, { donotNotifyError: true });
  assert.deepStrictEqual(testObject.getValue(TasksSchemaProperties.Tasks), tasks);
  }));