diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index b93eced9b75..bdf9cd2ff7f 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -27,7 +27,7 @@ import * as editorCommon from 'vs/editor/common/editorCommon'; import * as modes from 'vs/editor/common/modes'; import {IResourceEdit} from 'vs/editor/common/services/bulkEdit'; -import {ConfigurationTarget, ConfigurationEditingResult} from 'vs/workbench/services/configuration/common/configurationEditing'; +import {ConfigurationTarget} from 'vs/workbench/services/configuration/common/configurationEditing'; import {IPickOpenEntry, IPickOptions} from 'vs/workbench/services/quickopen/common/quickOpenService'; import {IWorkspaceSymbol} from 'vs/workbench/parts/search/common/search'; @@ -84,7 +84,7 @@ export abstract class MainThreadCommandsShape { } export abstract class MainThreadConfigurationShape { - $updateConfigurationOption(target: ConfigurationTarget, key: string, value: any): TPromise { throw ni(); } + $updateConfigurationOption(target: ConfigurationTarget, key: string, value: any): TPromise { throw ni(); } } export abstract class MainThreadDiagnosticsShape { diff --git a/src/vs/workbench/api/node/extHostConfiguration.ts b/src/vs/workbench/api/node/extHostConfiguration.ts index 896f81c5a30..760a4a7c9f7 100644 --- a/src/vs/workbench/api/node/extHostConfiguration.ts +++ b/src/vs/workbench/api/node/extHostConfiguration.ts @@ -9,7 +9,7 @@ import {illegalState} from 'vs/base/common/errors'; import Event, {Emitter} from 'vs/base/common/event'; import {WorkspaceConfiguration} from 'vscode'; import {ExtHostConfigurationShape, MainThreadConfigurationShape} from './extHost.protocol'; -import {ConfigurationTarget, ConfigurationEditingResult} from 'vs/workbench/services/configuration/common/configurationEditing'; +import {ConfigurationTarget} from 'vs/workbench/services/configuration/common/configurationEditing'; export class ExtHostConfiguration extends ExtHostConfigurationShape { @@ -56,11 +56,7 @@ export class ExtHostConfiguration extends ExtHostConfigurationShape { update: (key: string, value: any, global: boolean) => { key = section ? `${section}.${key}` : key; const target = global ? ConfigurationTarget.USER : ConfigurationTarget.WORKSPACE; - return this._proxy.$updateConfigurationOption(target, key, value).then(value => { - if (value !== ConfigurationEditingResult.OK) { - throw new Error(ConfigurationEditingResult[value]); - } - }); + return this._proxy.$updateConfigurationOption(target, key, value); } }; diff --git a/src/vs/workbench/api/node/mainThreadConfiguration.ts b/src/vs/workbench/api/node/mainThreadConfiguration.ts index deab01bda60..acaa75a4124 100644 --- a/src/vs/workbench/api/node/mainThreadConfiguration.ts +++ b/src/vs/workbench/api/node/mainThreadConfiguration.ts @@ -8,7 +8,7 @@ import {TPromise} from 'vs/base/common/winjs.base'; import {IDisposable, dispose} from 'vs/base/common/lifecycle'; import {IThreadService} from 'vs/workbench/services/thread/common/threadService'; import {IWorkbenchConfigurationService} from 'vs/workbench/services/configuration/common/configuration'; -import {IConfigurationEditingService, ConfigurationTarget, ConfigurationEditingResult} from 'vs/workbench/services/configuration/common/configurationEditing'; +import {IConfigurationEditingService, ConfigurationTarget} from 'vs/workbench/services/configuration/common/configurationEditing'; import {MainThreadConfigurationShape, ExtHostContext} from './extHost.protocol'; export class MainThreadConfiguration extends MainThreadConfigurationShape { @@ -32,7 +32,7 @@ export class MainThreadConfiguration extends MainThreadConfigurationShape { this._toDispose = dispose(this._toDispose); } - $updateConfigurationOption(target: ConfigurationTarget, key: string, value: any): TPromise { + $updateConfigurationOption(target: ConfigurationTarget, key: string, value: any): TPromise { return this._configurationEditingService.writeConfiguration(target, [{ key, value }]); } } diff --git a/src/vs/workbench/services/configuration/common/configurationEditing.ts b/src/vs/workbench/services/configuration/common/configurationEditing.ts index 15d476c9315..d76af24b691 100644 --- a/src/vs/workbench/services/configuration/common/configurationEditing.ts +++ b/src/vs/workbench/services/configuration/common/configurationEditing.ts @@ -9,16 +9,44 @@ import {createDecorator, ServiceIdentifier} from 'vs/platform/instantiation/comm export const IConfigurationEditingService = createDecorator('configurationEditingService'); -export enum ConfigurationEditingResult { - OK, +export enum ConfigurationEditingErrorCode { + + /** + * Error when trying to write a configuration key that is not registered. + */ ERROR_UNKNOWN_KEY, + + /** + * Error when trying to write to the workspace configuration without having a workspace opened. + */ ERROR_NO_WORKSPACE_OPENED, + + /** + * Error when trying to write to the configuration file while it is dirty in the editor. + */ ERROR_CONFIGURATION_FILE_DIRTY, + + /** + * Error when trying to write to a configuration file that contains JSON errors. + */ ERROR_INVALID_CONFIGURATION } +export interface IConfigurationEditingError { + code: ConfigurationEditingErrorCode; + message: string; +} + export enum ConfigurationTarget { + + /** + * Targets the user configuration file for writing. + */ USER, + + /** + * Targets the workspace configuration file for writing. This only works if a workspace is opened. + */ WORKSPACE } @@ -31,5 +59,9 @@ export interface IConfigurationEditingService { _serviceBrand: ServiceIdentifier; - writeConfiguration(target: ConfigurationTarget, values: IConfigurationValue[]): TPromise; + /** + * Allows to write to either the user or workspace configuration file. The returned promise will be + * in error state in any of the error cases from [ConfigurationEditingErrorCode](#ConfigurationEditingErrorCode) + */ + writeConfiguration(target: ConfigurationTarget, values: IConfigurationValue[]): TPromise; } \ No newline at end of file diff --git a/src/vs/workbench/services/configuration/node/configurationEditingService.ts b/src/vs/workbench/services/configuration/node/configurationEditingService.ts index e1114a78f46..ae5be45730f 100644 --- a/src/vs/workbench/services/configuration/node/configurationEditingService.ts +++ b/src/vs/workbench/services/configuration/node/configurationEditingService.ts @@ -5,6 +5,7 @@ 'use strict'; +import nls = require('vs/nls'); import {TPromise} from 'vs/base/common/winjs.base'; import URI from 'vs/base/common/uri'; import * as json from 'vs/base/common/json'; @@ -18,10 +19,10 @@ import {IWorkspaceContextService} from 'vs/platform/workspace/common/workspace'; import {IEnvironmentService} from 'vs/platform/environment/common/environment'; import {IConfigurationService} from 'vs/platform/configuration/common/configuration'; import {WORKSPACE_CONFIG_DEFAULT_PATH} from 'vs/workbench/services/configuration/common/configuration'; -import {IConfigurationEditingService, ConfigurationEditingResult, ConfigurationTarget, IConfigurationValue} from 'vs/workbench/services/configuration/common/configurationEditing'; +import {IConfigurationEditingService, ConfigurationEditingErrorCode, IConfigurationEditingError, ConfigurationTarget, IConfigurationValue} from 'vs/workbench/services/configuration/common/configurationEditing'; interface IValidationResult { - result: ConfigurationEditingResult; + error?: ConfigurationEditingErrorCode; exists?: boolean; contents?: string; } @@ -38,12 +39,12 @@ export class ConfigurationEditingService implements IConfigurationEditingService ) { } - public writeConfiguration(target: ConfigurationTarget, values: IConfigurationValue[]): TPromise { + public writeConfiguration(target: ConfigurationTarget, values: IConfigurationValue[]): TPromise { // First validate before making any edits return this.validate(target, values).then(validation => { - if (validation.result !== ConfigurationEditingResult.OK) { - return validation.result; + if (typeof validation.error === 'number') { + return this.wrapError(validation.error); } // Create configuration file if missing @@ -65,12 +66,31 @@ export class ConfigurationEditingService implements IConfigurationEditingService return pfs.writeFile(resource.fsPath, result, encoding.UTF8).then(() => { // Reload the configuration so that we make sure all parties are updated - return this.configurationService.reloadConfiguration().then(() => ConfigurationEditingResult.OK); + return this.configurationService.reloadConfiguration().then(() => void 0); }); }); }); } + private wrapError(code: ConfigurationEditingErrorCode): TPromise { + const message = this.toErrorMessage(code); + + return TPromise.wrapError({ + code, + message, + toString: () => message + }); + } + + private toErrorMessage(error: ConfigurationEditingErrorCode): string { + switch (error) { + case ConfigurationEditingErrorCode.ERROR_UNKNOWN_KEY: return nls.localize('errorUnknownKey', "Unable to write to the configuration file (Unknown Key)"); + case ConfigurationEditingErrorCode.ERROR_NO_WORKSPACE_OPENED: return nls.localize('errorWorkspaceOpened', "Unable to write to the configuration file (No Workspace Opened)"); + case ConfigurationEditingErrorCode.ERROR_INVALID_CONFIGURATION: return nls.localize('errorInvalidConfiguration', "Unable to write to the configuration file (Invalid Configuration Found)"); + case ConfigurationEditingErrorCode.ERROR_CONFIGURATION_FILE_DIRTY: return nls.localize('errorConfigurationFileDirty', "Unable to write to the configuration file (Configuration File Dirty)"); + } + } + private applyEdits(content: string, values: IConfigurationValue[]): string { const {tabSize, insertSpaces} = this.configurationService.getConfiguration<{ tabSize: number; insertSpaces: boolean }>('editor'); const {eol} = this.configurationService.getConfiguration<{ eol: string }>('files'); @@ -90,25 +110,25 @@ export class ConfigurationEditingService implements IConfigurationEditingService // 1.) Any key must be a known setting from the registry const validKeys = getConfigurationKeys(); if (values.some(v => validKeys.indexOf(v.key) < 0)) { - return TPromise.as({ result: ConfigurationEditingResult.ERROR_UNKNOWN_KEY }); + return TPromise.as({ error: ConfigurationEditingErrorCode.ERROR_UNKNOWN_KEY }); } // 2.) Target cannot be workspace if no workspace opened if (target === ConfigurationTarget.WORKSPACE && !this.contextService.getWorkspace()) { - return TPromise.as({ result: ConfigurationEditingResult.ERROR_NO_WORKSPACE_OPENED }); + return TPromise.as({ error: ConfigurationEditingErrorCode.ERROR_NO_WORKSPACE_OPENED }); } // 3.) Target cannot be dirty const resource = this.getConfigurationResource(target); return this.editorService.createInput({ resource }).then(typedInput => { if (typedInput.isDirty()) { - return { result: ConfigurationEditingResult.ERROR_CONFIGURATION_FILE_DIRTY }; + return { error: ConfigurationEditingErrorCode.ERROR_CONFIGURATION_FILE_DIRTY }; } // 4.) Target cannot contain JSON errors return pfs.exists(resource.fsPath).then(exists => { if (!exists) { - return { result: ConfigurationEditingResult.OK, exists }; + return { exists }; } return pfs.readFile(resource.fsPath).then(contentsRaw => { @@ -117,10 +137,10 @@ export class ConfigurationEditingService implements IConfigurationEditingService json.parse(contents, parseErrors); if (parseErrors.length > 0) { - return { result: ConfigurationEditingResult.ERROR_INVALID_CONFIGURATION }; + return { error: ConfigurationEditingErrorCode.ERROR_INVALID_CONFIGURATION }; } - return { result: ConfigurationEditingResult.OK, exists, contents }; + return { exists, contents }; }); }); }); diff --git a/src/vs/workbench/services/configuration/test/node/configurationEditingService.test.ts b/src/vs/workbench/services/configuration/test/node/configurationEditingService.test.ts index ed1b060d0d6..617f0240313 100644 --- a/src/vs/workbench/services/configuration/test/node/configurationEditingService.test.ts +++ b/src/vs/workbench/services/configuration/test/node/configurationEditingService.test.ts @@ -23,7 +23,7 @@ import {IConfigurationRegistry, Extensions as ConfigurationExtensions} from 'vs/ import {WorkspaceConfigurationService} from 'vs/workbench/services/configuration/node/configurationService'; import URI from 'vs/base/common/uri'; import {ConfigurationEditingService} from 'vs/workbench/services/configuration/node/configurationEditingService'; -import {ConfigurationEditingResult, ConfigurationTarget} from 'vs/workbench/services/configuration/common/configurationEditing'; +import {ConfigurationTarget, IConfigurationEditingError, ConfigurationEditingErrorCode} from 'vs/workbench/services/configuration/common/configurationEditing'; import {IResourceInput} from 'vs/platform/editor/common/editor'; class SettingsTestEnvironmentService extends EnvironmentService { @@ -113,7 +113,8 @@ suite('WorkspaceConfigurationEditingService - Node', () => { createWorkspace((workspaceDir, globalSettingsFile, cleanUp) => { return createServices(workspaceDir, globalSettingsFile, false, true /* no workspace */).then(services => { return services.configurationEditingService.writeConfiguration(ConfigurationTarget.WORKSPACE, [{ key: 'unknown.key', value: 'value' }]).then(res => { - assert.equal(res, ConfigurationEditingResult.ERROR_UNKNOWN_KEY); + }, (error:IConfigurationEditingError) => { + assert.equal(error.code, ConfigurationEditingErrorCode.ERROR_UNKNOWN_KEY); services.configurationService.dispose(); cleanUp(done); }); @@ -125,7 +126,8 @@ suite('WorkspaceConfigurationEditingService - Node', () => { createWorkspace((workspaceDir, globalSettingsFile, cleanUp) => { return createServices(workspaceDir, globalSettingsFile, false, true /* no workspace */).then(services => { return services.configurationEditingService.writeConfiguration(ConfigurationTarget.WORKSPACE, [{ key: 'configurationEditing.service.testSetting', value: 'value' }]).then(res => { - assert.equal(res, ConfigurationEditingResult.ERROR_NO_WORKSPACE_OPENED); + }, (error: IConfigurationEditingError) => { + assert.equal(error.code, ConfigurationEditingErrorCode.ERROR_NO_WORKSPACE_OPENED); services.configurationService.dispose(); cleanUp(done); }); @@ -139,7 +141,8 @@ suite('WorkspaceConfigurationEditingService - Node', () => { fs.writeFileSync(globalSettingsFile, ',,,,,,,,,,,,,,'); return services.configurationEditingService.writeConfiguration(ConfigurationTarget.USER, [{ key: 'configurationEditing.service.testSetting', value: 'value' }]).then(res => { - assert.equal(res, ConfigurationEditingResult.ERROR_INVALID_CONFIGURATION); + }, (error: IConfigurationEditingError) => { + assert.equal(error.code, ConfigurationEditingErrorCode.ERROR_INVALID_CONFIGURATION); services.configurationService.dispose(); cleanUp(done); }); @@ -151,7 +154,8 @@ suite('WorkspaceConfigurationEditingService - Node', () => { createWorkspace((workspaceDir, globalSettingsFile, cleanUp) => { return createServices(workspaceDir, globalSettingsFile, true).then(services => { return services.configurationEditingService.writeConfiguration(ConfigurationTarget.USER, [{ key: 'configurationEditing.service.testSetting', value: 'value' }]).then(res => { - assert.equal(res, ConfigurationEditingResult.ERROR_CONFIGURATION_FILE_DIRTY); + }, (error: IConfigurationEditingError) => { + assert.equal(error.code, ConfigurationEditingErrorCode.ERROR_CONFIGURATION_FILE_DIRTY); services.configurationService.dispose(); cleanUp(done); }); @@ -163,8 +167,6 @@ suite('WorkspaceConfigurationEditingService - Node', () => { createWorkspace((workspaceDir, globalSettingsFile, cleanUp) => { return createServices(workspaceDir, globalSettingsFile).then(services => { return services.configurationEditingService.writeConfiguration(ConfigurationTarget.USER, [{ key: 'configurationEditing.service.testSetting', value: 'value' }]).then(res => { - assert.equal(res, ConfigurationEditingResult.OK); - const contents = fs.readFileSync(globalSettingsFile).toString('utf8'); const parsed = json.parse(contents); assert.equal(parsed['configurationEditing.service.testSetting'], 'value'); @@ -183,8 +185,6 @@ suite('WorkspaceConfigurationEditingService - Node', () => { fs.writeFileSync(globalSettingsFile, '{ "my.super.setting": "my.super.value" }'); return services.configurationEditingService.writeConfiguration(ConfigurationTarget.USER, [{ key: 'configurationEditing.service.testSetting', value: 'value' }]).then(res => { - assert.equal(res, ConfigurationEditingResult.OK); - const contents = fs.readFileSync(globalSettingsFile).toString('utf8'); const parsed = json.parse(contents); assert.equal(parsed['configurationEditing.service.testSetting'], 'value'); @@ -208,8 +208,6 @@ suite('WorkspaceConfigurationEditingService - Node', () => { { key: 'configurationEditing.service.testSettingTwo', value: { complex: { value: true } } }, { key: 'configurationEditing.service.testSettingThree', value: 55 } ]).then(res => { - assert.equal(res, ConfigurationEditingResult.OK); - const contents = fs.readFileSync(globalSettingsFile).toString('utf8'); const parsed = json.parse(contents); assert.equal(parsed['configurationEditing.service.testSetting'], 'value'); @@ -236,8 +234,6 @@ suite('WorkspaceConfigurationEditingService - Node', () => { { key: 'configurationEditing.service.testSettingTwo', value: { complex: { value: true } } }, { key: 'configurationEditing.service.testSettingThree', value: 55 } ]).then(res => { - assert.equal(res, ConfigurationEditingResult.OK); - const contents = fs.readFileSync(globalSettingsFile).toString('utf8'); const parsed = json.parse(contents); assert.equal(parsed['configurationEditing.service.testSetting'], 'value'); diff --git a/src/vs/workbench/test/node/api/extHostConfiguration.test.ts b/src/vs/workbench/test/node/api/extHostConfiguration.test.ts index c44d719e003..f0dbfd633c3 100644 --- a/src/vs/workbench/test/node/api/extHostConfiguration.test.ts +++ b/src/vs/workbench/test/node/api/extHostConfiguration.test.ts @@ -9,15 +9,15 @@ import * as assert from 'assert'; import {ExtHostConfiguration} from 'vs/workbench/api/node/extHostConfiguration'; import {MainThreadConfigurationShape} from 'vs/workbench/api/node/extHost.protocol'; import {TPromise} from 'vs/base/common/winjs.base'; -import {ConfigurationTarget, ConfigurationEditingResult} from 'vs/workbench/services/configuration/common/configurationEditing'; +import {ConfigurationTarget, ConfigurationEditingErrorCode, IConfigurationEditingError} from 'vs/workbench/services/configuration/common/configurationEditing'; suite('ExtHostConfiguration', function () { class RecordingShape extends MainThreadConfigurationShape { lastArgs: [ConfigurationTarget, string, any]; - $updateConfigurationOption(target: ConfigurationTarget, key: string, value: any): TPromise { + $updateConfigurationOption(target: ConfigurationTarget, key: string, value: any): TPromise { this.lastArgs = [target, key, value]; - return TPromise.as(ConfigurationEditingResult.OK); + return TPromise.as(void 0); } }; @@ -42,7 +42,6 @@ suite('ExtHostConfiguration', function () { let config = allConfig.getConfiguration('foo'); config.update('bar', 42, true); - assert.equal(shape.lastArgs[0], ConfigurationEditingResult.OK); assert.equal(shape.lastArgs[1], 'foo.bar'); assert.equal(shape.lastArgs[2], 42); @@ -57,8 +56,8 @@ suite('ExtHostConfiguration', function () { test('update / error-state not OK', function () { const shape = new class extends MainThreadConfigurationShape { - $updateConfigurationOption(target: ConfigurationTarget, key: string, value: any): TPromise { - return TPromise.as(ConfigurationEditingResult.ERROR_UNKNOWN_KEY); // something !== OK + $updateConfigurationOption(target: ConfigurationTarget, key: string, value: any): TPromise { + return TPromise.wrapError({ code: ConfigurationEditingErrorCode.ERROR_UNKNOWN_KEY, message: 'Unknown Key' }); // something !== OK } };