From edc346b2639562797f707432191a090aad4bb7d6 Mon Sep 17 00:00:00 2001 From: Colin Grant Date: Wed, 10 Mar 2021 11:59:04 -0600 Subject: [PATCH] Implement updateValue method for PreferenceService Signed-off-by: Colin Grant --- .../api-tests/src/launch-preferences.spec.js | 23 ++-- .../preferences/preference-service.spec.ts | 126 +++++++++++++++--- .../browser/preferences/preference-service.ts | 81 +++++++++-- .../test/mock-preference-service.ts | 11 +- .../common/preferences/preference-scope.ts | 6 + packages/editor/src/browser/editor-command.ts | 2 +- 6 files changed, 195 insertions(+), 54 deletions(-) diff --git a/examples/api-tests/src/launch-preferences.spec.js b/examples/api-tests/src/launch-preferences.spec.js index c5d376e262b76..f76a18fa43eac 100644 --- a/examples/api-tests/src/launch-preferences.spec.js +++ b/examples/api-tests/src/launch-preferences.spec.js @@ -346,11 +346,15 @@ describe('Launch Preferences', function () { }); } + /** + * @typedef {Partial>} PreferenceInspection + */ + /** * @typedef {Object} SuiteOptions * @property {string} name * @property {any} expectation - * @property {any} [inspectExpectation] + * @property {PreferenceInspection} [inspectExpectation] * @property {any} [launch] * @property {any} [settings] * @property {boolean} [only] @@ -518,6 +522,7 @@ describe('Launch Preferences', function () { testIt('inspect in undefined', () => { const inspect = preferences.inspect('launch'); + /** @type {PreferenceInspection} */ let expected = inspectExpectation; if (!expected) { expected = { @@ -529,11 +534,13 @@ describe('Launch Preferences', function () { Object.assign(expected, { workspaceValue }); } } - assert.deepStrictEqual(JSON.parse(JSON.stringify(inspect)), expected); + const expectedValue = expected.workspaceFolderValue || expected.workspaceValue || expected.globalValue || expected.defaultValue; + assert.deepStrictEqual(JSON.parse(JSON.stringify(inspect)), { ...expected, value: expectedValue }); }); testIt('inspect in rootUri', () => { const inspect = preferences.inspect('launch', rootUri.toString()); + /** @type {PreferenceInspection} */ const expected = { preferenceName: 'launch', defaultValue: defaultLaunch @@ -552,7 +559,8 @@ describe('Launch Preferences', function () { }); } } - assert.deepStrictEqual(JSON.parse(JSON.stringify(inspect)), expected); + const expectedValue = expected.workspaceFolderValue || expected.workspaceValue || expected.globalValue || expected.defaultValue; + assert.deepStrictEqual(JSON.parse(JSON.stringify(inspect)), { ...expected, value: expectedValue }); }); testIt('update launch', async () => { @@ -564,15 +572,6 @@ describe('Launch Preferences', function () { assert.deepStrictEqual(actual, expected); }); - testIt('update launch Global', async () => { - try { - await preferences.set('launch', validLaunch, PreferenceScope.User); - assert.fail('should not be possible to update User Settings'); - } catch (e) { - assert.deepStrictEqual(e.message, 'Unable to write to User Settings because launch does not support for global scope.'); - } - }); - testIt('update launch Workspace', async () => { await preferences.set('launch', validLaunch, PreferenceScope.Workspace); diff --git a/packages/core/src/browser/preferences/preference-service.spec.ts b/packages/core/src/browser/preferences/preference-service.spec.ts index bbc7bed5b00ef..cdb13a9b9209a 100644 --- a/packages/core/src/browser/preferences/preference-service.spec.ts +++ b/packages/core/src/browser/preferences/preference-service.spec.ts @@ -257,6 +257,107 @@ describe('Preference Service', () => { assert.strictEqual(prefService.get('[go].editor.insertSpaces'), undefined, 'get after overridden'); }); + function prepareServices(options?: { schema: PreferenceSchema }): { + preferences: PreferenceServiceImpl; + schema: PreferenceSchemaProvider; + } { + prefSchema.setSchema(options && options.schema || { + properties: { + 'editor.tabSize': { + type: 'number', + description: '', + overridable: true, + default: 4 + } + } + }); + + return { preferences: prefService, schema: prefSchema }; + } + + describe('PreferenceService.updateValues()', () => { + const TAB_SIZE = 'editor.tabSize'; + const DUMMY_URI = 'dummy_uri'; + async function generateAndCheckValues( + preferences: PreferenceService, + globalValue: number | undefined, + workspaceValue: number | undefined, + workspaceFolderValue: number | undefined + ): Promise { + await preferences.set(TAB_SIZE, globalValue, PreferenceScope.User); + await preferences.set(TAB_SIZE, workspaceValue, PreferenceScope.Workspace); + await preferences.set(TAB_SIZE, workspaceFolderValue, PreferenceScope.Folder, DUMMY_URI); + const expectedValue = workspaceFolderValue ?? workspaceValue ?? globalValue ?? 4; + checkValues(preferences, globalValue, workspaceValue, workspaceFolderValue, expectedValue); + } + + function checkValues( + preferences: PreferenceService, + globalValue: number | undefined, + workspaceValue: number | undefined, + workspaceFolderValue: number | undefined, + value: number = 4, + ): void { + const expected = { + preferenceName: 'editor.tabSize', + defaultValue: 4, + globalValue, + workspaceValue, + workspaceFolderValue, + value, + }; + const inspection = preferences.inspect(TAB_SIZE, DUMMY_URI); + assert.deepStrictEqual(inspection, expected); + } + + it('should modify the narrowest scope.', async () => { + const { preferences } = prepareServices(); + + await generateAndCheckValues(preferences, 1, 2, 3); + await preferences.updateValue(TAB_SIZE, 8, DUMMY_URI); + checkValues(preferences, 1, 2, 8, 8); + + await generateAndCheckValues(preferences, 1, 2, undefined); + await preferences.updateValue(TAB_SIZE, 8, DUMMY_URI); + checkValues(preferences, 1, 8, undefined, 8); + + await generateAndCheckValues(preferences, 1, undefined, undefined); + await preferences.updateValue(TAB_SIZE, 8, DUMMY_URI); + checkValues(preferences, 8, undefined, undefined, 8); + }); + + it('defaults to user scope.', async () => { + const { preferences } = prepareServices(); + checkValues(preferences, undefined, undefined, undefined); + await preferences.updateValue(TAB_SIZE, 8, DUMMY_URI); + checkValues(preferences, 8, undefined, undefined, 8); + }); + + it('clears all settings when input is undefined.', async () => { + const { preferences } = prepareServices(); + + await generateAndCheckValues(preferences, 1, 2, 3); + await preferences.updateValue(TAB_SIZE, undefined, DUMMY_URI); + checkValues(preferences, undefined, undefined, undefined); + }); + + it('deletes user setting if user is only defined scope and target is default value', async () => { + const { preferences } = prepareServices(); + + await generateAndCheckValues(preferences, 8, undefined, undefined); + await preferences.updateValue(TAB_SIZE, 4, DUMMY_URI); + checkValues(preferences, undefined, undefined, undefined); + }); + + it('does not delete setting in lower scopes, even if target is default', async () => { + const { preferences } = prepareServices(); + + await generateAndCheckValues(preferences, undefined, 2, undefined); + await preferences.updateValue(TAB_SIZE, 4, DUMMY_URI); + checkValues(preferences, undefined, 4, undefined); + }); + }); + describe('overridden preferences', () => { it('get #0', () => { @@ -320,6 +421,7 @@ describe('Preference Service', () => { globalValue: undefined, workspaceValue: undefined, workspaceFolderValue: undefined, + value: 4, }; assert.deepStrictEqual(expected, preferences.inspect('editor.tabSize')); assert.ok(!preferences.has('[json].editor.tabSize')); @@ -342,6 +444,7 @@ describe('Preference Service', () => { globalValue: 2, workspaceValue: undefined, workspaceFolderValue: undefined, + value: 2 }; preferences.set('editor.tabSize', 2, PreferenceScope.User); @@ -366,6 +469,7 @@ describe('Preference Service', () => { globalValue: undefined, workspaceValue: undefined, workspaceFolderValue: undefined, + value: 4 }; assert.deepStrictEqual(expected, preferences.inspect('editor.tabSize')); assert.ok(!preferences.has('[json].editor.tabSize')); @@ -377,7 +481,8 @@ describe('Preference Service', () => { assert.deepStrictEqual({ ...expected, preferenceName: '[json].editor.tabSize', - globalValue: 2 + globalValue: 2, + value: 2, }, preferences.inspect('[json].editor.tabSize')); }); @@ -511,25 +616,6 @@ describe('Preference Service', () => { assert.strictEqual(false, preferences.get('editor.formatOnSave')); assert.strictEqual(true, preferences.get('[go].editor.formatOnSave')); }); - - function prepareServices(options?: { schema: PreferenceSchema }): { - preferences: PreferenceServiceImpl; - schema: PreferenceSchemaProvider; - } { - prefSchema.setSchema(options && options.schema || { - properties: { - 'editor.tabSize': { - type: 'number', - description: '', - overridable: true, - default: 4 - } - } - }); - - return { preferences: prefService, schema: prefSchema }; - } - }); }); diff --git a/packages/core/src/browser/preferences/preference-service.ts b/packages/core/src/browser/preferences/preference-service.ts index 6fd3e8f07daad..3340c35936d46 100644 --- a/packages/core/src/browser/preferences/preference-service.ts +++ b/packages/core/src/browser/preferences/preference-service.ts @@ -24,6 +24,7 @@ import { PreferenceSchemaProvider, OverridePreferenceName } from './preference-c import URI from '../../common/uri'; import { PreferenceScope } from './preference-scope'; import { PreferenceConfigurations } from './preference-configurations'; +import { JSONExt } from '@phosphor/coreutils/lib/json'; export { PreferenceScope }; @@ -156,6 +157,18 @@ export interface PreferenceService extends Disposable { * with an error. */ set(preferenceName: string, value: any, scope?: PreferenceScope, resourceUri?: string): Promise; + + /** + * Determines and applies the changes necessary to apply `value` to either the `resourceUri` supplied or the active session. + * If there is no setting for the `preferenceName`, the change will be applied in user scope. + * If there is a setting conflicting with the specified `value`, the change will be applied in the most specific scope with a conflicting value. + * + * @param preferenceName the identifier of the preference to modify. + * @param value the value to which to set the preference. `undefined` will reset the preference to its default value. + * @param resourceUri the uri of the resource to which the change is to apply. If none is provided, folder scope will be ignored. + */ + updateValue(preferenceName: string, value: any, resourceUri?: string): Promise + /** * Registers a callback which will be called whenever a preference is changed. */ @@ -244,7 +257,11 @@ export interface PreferenceInspection { /** * Value in folder scope. */ - workspaceFolderValue: T | undefined + workspaceFolderValue: T | undefined, + /** + * The value that is active, i.e. the value set in the lowest scope available. + */ + value: T | undefined; } /** @@ -401,10 +418,7 @@ export class PreferenceServiceImpl implements PreferenceService { } async set(preferenceName: string, value: any, scope: PreferenceScope | undefined, resourceUri?: string): Promise { - const resolvedScope = scope !== undefined ? scope : (!resourceUri ? PreferenceScope.Workspace : PreferenceScope.Folder); - if (resolvedScope === PreferenceScope.User && this.configurations.isSectionName(preferenceName.split('.', 1)[0])) { - throw new Error(`Unable to write to User Settings because ${preferenceName} does not support for global scope.`); - } + const resolvedScope = scope ?? (!resourceUri ? PreferenceScope.Workspace : PreferenceScope.Folder); if (resolvedScope === PreferenceScope.Folder && !resourceUri) { throw new Error('Unable to write to Folder Settings because no resource is provided.'); } @@ -451,20 +465,17 @@ export class PreferenceServiceImpl implements PreferenceService { return Number(value); } - inspect(preferenceName: string, resourceUri?: string): { - preferenceName: string, - defaultValue: T | undefined, - globalValue: T | undefined, // User Preference - workspaceValue: T | undefined, // Workspace Preference - workspaceFolderValue: T | undefined // Folder Preference - } | undefined { + inspect(preferenceName: string, resourceUri?: string): PreferenceInspection | undefined { const defaultValue = this.inspectInScope(preferenceName, PreferenceScope.Default, resourceUri); const globalValue = this.inspectInScope(preferenceName, PreferenceScope.User, resourceUri); const workspaceValue = this.inspectInScope(preferenceName, PreferenceScope.Workspace, resourceUri); const workspaceFolderValue = this.inspectInScope(preferenceName, PreferenceScope.Folder, resourceUri); - return { preferenceName, defaultValue, globalValue, workspaceValue, workspaceFolderValue }; + const valueApplied = workspaceFolderValue ?? workspaceValue ?? globalValue ?? defaultValue; + + return { preferenceName, defaultValue, globalValue, workspaceValue, workspaceFolderValue, value: valueApplied }; } + protected inspectInScope(preferenceName: string, scope: PreferenceScope, resourceUri?: string): T | undefined { const value = this.doInspectInScope(preferenceName, scope, resourceUri); if (value === undefined) { @@ -476,6 +487,50 @@ export class PreferenceServiceImpl implements PreferenceService { return value; } + protected getScopedValueFromInspection(inspection: PreferenceInspection, scope: PreferenceScope): T | undefined { + switch (scope) { + case PreferenceScope.Default: + return inspection.defaultValue; + case PreferenceScope.User: + return inspection.globalValue; + case PreferenceScope.Workspace: + return inspection.workspaceValue; + case PreferenceScope.Folder: + return inspection.workspaceFolderValue; + } + return ((unhandledScope: never): never => { throw new Error('Must handle all enum values!'); })(scope); + } + + async updateValue(preferenceName: string, value: any, resourceUri?: string): Promise { + const inspection = this.inspect(preferenceName, resourceUri); + if (inspection) { + const scopesToChange = this.getScopesToChange(inspection, value); + const isDeletion = value === undefined + || (scopesToChange.length === 1 && scopesToChange[0] === PreferenceScope.User && JSONExt.deepEqual(value, inspection.defaultValue)); + const effectiveValue = isDeletion ? undefined : value; + await Promise.all(scopesToChange.map(scope => this.set(preferenceName, effectiveValue, scope, resourceUri))); + } + } + + protected getScopesToChange(inspection: PreferenceInspection, intendedValue: any): PreferenceScope[] { + if (JSONExt.deepEqual(inspection.value, intendedValue)) { + return []; + } + + // Scopes in ascending order of scope breadth. + const allScopes = PreferenceScope.getReversedScopes(); + // Get rid of Default scope. We can't set anything there. + allScopes.pop(); + + const isScopeDefined = (scope: PreferenceScope) => this.getScopedValueFromInspection(inspection, scope) !== undefined; + + if (intendedValue === undefined) { + return allScopes.filter(isScopeDefined); + } + + return [allScopes.find(isScopeDefined) ?? PreferenceScope.User]; + } + overridePreferenceName(options: OverridePreferenceName): string { return this.schema.overridePreferenceName(options); } diff --git a/packages/core/src/browser/preferences/test/mock-preference-service.ts b/packages/core/src/browser/preferences/test/mock-preference-service.ts index 4a3df2651e8cb..779588e644bb8 100644 --- a/packages/core/src/browser/preferences/test/mock-preference-service.ts +++ b/packages/core/src/browser/preferences/test/mock-preference-service.ts @@ -19,7 +19,7 @@ import { PreferenceService, PreferenceChange } from '../'; import { Emitter, Event } from '../../../common'; import { OverridePreferenceName } from '../preference-contribution'; import URI from '../../../common/uri'; -import { PreferenceChanges } from '../preference-service'; +import { PreferenceChanges, PreferenceInspection } from '../preference-service'; import { PreferenceScope } from '../preference-scope'; @injectable() @@ -38,17 +38,12 @@ export class MockPreferenceService implements PreferenceService { } { return {}; } - inspect(preferenceName: string, resourceUri?: string): { - preferenceName: string, - defaultValue: T | undefined, - globalValue: T | undefined, // User Preference - workspaceValue: T | undefined, // Workspace Preference - workspaceFolderValue: T | undefined // Folder Preference - } | undefined { + inspect(preferenceName: string, resourceUri?: string): PreferenceInspection | undefined { return undefined; } // eslint-disable-next-line @typescript-eslint/no-explicit-any set(preferenceName: string, value: any): Promise { return Promise.resolve(); } + updateValue(): Promise { return Promise.resolve(); } ready: Promise = Promise.resolve(); readonly onPreferenceChanged: Event = new Emitter().event; readonly onPreferencesChanged: Event = new Emitter().event; diff --git a/packages/core/src/common/preferences/preference-scope.ts b/packages/core/src/common/preferences/preference-scope.ts index 2b24938c2b8dd..6e38ad87bee60 100644 --- a/packages/core/src/common/preferences/preference-scope.ts +++ b/packages/core/src/common/preferences/preference-scope.ts @@ -28,12 +28,18 @@ export namespace PreferenceScope { return typeof scope === 'number' && getScopes().findIndex(s => s === scope) >= 0; } + /** + * @returns preference scopes from broadest to narrowest: Default -> Folder. + */ export function getScopes(): PreferenceScope[] { return Object.keys(PreferenceScope) .filter(k => typeof PreferenceScope[k as any] === 'string') .map(v => Number(v)); } + /** + * @returns preference scopes from narrowest to broadest. Folder -> Default. + */ export function getReversedScopes(): PreferenceScope[] { return getScopes().reverse(); } diff --git a/packages/editor/src/browser/editor-command.ts b/packages/editor/src/browser/editor-command.ts index 51d52ec98cff8..03c41deaf719b 100644 --- a/packages/editor/src/browser/editor-command.ts +++ b/packages/editor/src/browser/editor-command.ts @@ -333,6 +333,6 @@ export class EditorCommandContribution implements CommandContribution { return autoSave === 'on' || autoSave === undefined; } private async toggleAutoSave(): Promise { - this.preferencesService.set(EditorCommandContribution.AUTOSAVE_PREFERENCE, this.isAutoSaveOn() ? 'off' : 'on'); + this.preferencesService.updateValue(EditorCommandContribution.AUTOSAVE_PREFERENCE, this.isAutoSaveOn() ? 'off' : 'on'); } }