diff --git a/src/vs/platform/configuration/common/model.ts b/src/vs/platform/configuration/common/model.ts index a981336a59d..a3c6df777b1 100644 --- a/src/vs/platform/configuration/common/model.ts +++ b/src/vs/platform/configuration/common/model.ts @@ -13,28 +13,29 @@ export function getDefaultValues(): any { for (let key in properties) { let value = properties[key].default; - addToValueTree(valueTreeRoot, key, value); + addToValueTree(valueTreeRoot, key, value, message => console.error(`Conflict in default settings: ${message}`)); } return valueTreeRoot; } -export function toValuesTree(properties: { [qualifiedKey: string]: any }): any { +export function toValuesTree(properties: { [qualifiedKey: string]: any }, conflictReporter: (message: string) => void): any { const root = Object.create(null); for (let key in properties) { - addToValueTree(root, key, properties[key]); + addToValueTree(root, key, properties[key], conflictReporter); } return root; } -function addToValueTree(settingsTreeRoot: any, key: string, value: any): void { +function addToValueTree(settingsTreeRoot: any, key: string, value: any, conflictReporter: (message: string) => void): void { const segments = key.split('.'); const last = segments.pop(); let curr = settingsTreeRoot; - segments.forEach(s => { + for (let i = 0; i < segments.length; i++) { + let s = segments[i]; let obj = curr[s]; switch (typeof obj) { case 'undefined': @@ -43,13 +44,16 @@ function addToValueTree(settingsTreeRoot: any, key: string, value: any): void { case 'object': break; default: - console.error(`Conflicting configuration setting: ${key} at ${s} with ${JSON.stringify(obj)}`); + conflictReporter(`Ignoring ${key} as ${segments.slice(0, i + 1).join('.')} is ${JSON.stringify(obj)}`); + return; } curr = obj; - }); + }; if (typeof curr === 'object') { curr[last] = value; // workaround https://github.com/Microsoft/vscode/issues/13606 + } else { + conflictReporter(`Ignoring ${key} as ${segments.join('.')} is ${JSON.stringify(curr)}`); } } diff --git a/src/vs/platform/configuration/node/configurationService.ts b/src/vs/platform/configuration/node/configurationService.ts index a6d45964518..fd08d96a29a 100644 --- a/src/vs/platform/configuration/node/configurationService.ts +++ b/src/vs/platform/configuration/node/configurationService.ts @@ -104,7 +104,7 @@ export class ConfigurationService implements IConfigurationService, IDisposab private consolidateConfigurations(): ICache { const defaults = getDefaultValues(); // defaults coming from contributions to registries - const user = toValuesTree(this.rawConfig.getConfig()); // user configured settings + const user = toValuesTree(this.rawConfig.getConfig(), message => console.error(`Conflict in user settings: ${message}`)); // user configured settings const consolidated = objects.mixin( objects.clone(defaults), // target: default values (but dont modify!) diff --git a/src/vs/workbench/api/node/extHostConfiguration.ts b/src/vs/workbench/api/node/extHostConfiguration.ts index 32d39cf84c5..ae94fb8329e 100644 --- a/src/vs/workbench/api/node/extHostConfiguration.ts +++ b/src/vs/workbench/api/node/extHostConfiguration.ts @@ -35,7 +35,7 @@ function createUsefulConfiguration(data: IWorkspaceConfigurationValues): { data: valueMap[key] = data[key].value; } } - const valueTree = toValuesTree(valueMap); + const valueTree = toValuesTree(valueMap, message => console.error(`Conflict in configuration settings: ${message}`)); return { data, valueTree diff --git a/src/vs/workbench/services/configuration/common/model.ts b/src/vs/workbench/services/configuration/common/model.ts index 85ca3f0a7a4..466751a606c 100644 --- a/src/vs/workbench/services/configuration/common/model.ts +++ b/src/vs/workbench/services/configuration/common/model.ts @@ -16,11 +16,11 @@ export interface IConfigFile { parseError?: any; } -export function newConfigFile(value: string): IConfigFile { +export function newConfigFile(value: string, fileName: string): IConfigFile { try { const contents = json.parse(value) || {}; return { - contents: toValuesTree(contents), + contents: toValuesTree(contents, message => console.error(`Conflict in settings file ${fileName}: ${message}`)), raw: contents }; } catch (e) { diff --git a/src/vs/workbench/services/configuration/node/configurationService.ts b/src/vs/workbench/services/configuration/node/configurationService.ts index 8bbe0bb8c48..1c05bcc5d50 100644 --- a/src/vs/workbench/services/configuration/node/configurationService.ts +++ b/src/vs/workbench/services/configuration/node/configurationService.ts @@ -253,7 +253,7 @@ export class WorkspaceConfigurationService implements IWorkspaceConfigurationSer return []; // never fail this call } }).then((contents: IContent[]) => { - contents.forEach(content => this.workspaceFilePathToConfiguration[this.contextService.toWorkspaceRelativePath(content.resource)] = TPromise.as(newConfigFile(content.value))); + contents.forEach(content => this.workspaceFilePathToConfiguration[this.contextService.toWorkspaceRelativePath(content.resource)] = TPromise.as(newConfigFile(content.value, content.resource.toString()))); }, errors.onUnexpectedError); } @@ -298,7 +298,7 @@ export class WorkspaceConfigurationService implements IWorkspaceConfigurationSer break; case FileChangeType.UPDATED: case FileChangeType.ADDED: - this.workspaceFilePathToConfiguration[workspacePath] = resolveContent(resource).then(content => newConfigFile(content.value), errors.onUnexpectedError); + this.workspaceFilePathToConfiguration[workspacePath] = resolveContent(resource).then(content => newConfigFile(content.value, content.resource.toString()), errors.onUnexpectedError); affectedByChanges = true; } } diff --git a/src/vs/workbench/test/node/api/extHostConfiguration.test.ts b/src/vs/workbench/test/node/api/extHostConfiguration.test.ts index 6eaf22734ae..50720b7a321 100644 --- a/src/vs/workbench/test/node/api/extHostConfiguration.test.ts +++ b/src/vs/workbench/test/node/api/extHostConfiguration.test.ts @@ -156,12 +156,24 @@ suite('ExtHostConfiguration', function () { }); test('bogous data, #15834', function () { - const shape = new RecordingShape(); - const allConfig = createExtHostConfiguration({ - ['editor.formatOnSave']: createConfigurationValue(true), - ['editor.formatOnSave.extensions']: createConfigurationValue(['ts']) - }, shape); + let oldLogger = console.error; + let errorLogged = false; + // workaround until we have a proper logging story + console.error = (message, args) => { + errorLogged = true; + }; + let allConfig; + try { + const shape = new RecordingShape(); + allConfig = createExtHostConfiguration({ + ['editor.formatOnSave']: createConfigurationValue(true), + ['editor.formatOnSave.extensions']: createConfigurationValue(['ts']) + }, shape); + } finally { + console.error = oldLogger; + } + assert.ok(errorLogged); assert.ok(allConfig.getConfiguration('').has('editor.formatOnSave')); assert.ok(!allConfig.getConfiguration('').has('editor.formatOnSave.extensions')); });