Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow multiple extensions to provide default values for object settings #217179

Merged
merged 5 commits into from
Jun 24, 2024
Merged

Allow multiple extensions to provide default values for object settings #217179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ec6cdfd
Allow multiple extensions to provide default values for object settings
benibenj Jun 24, 2024
8ffe8cb
Merge branch 'main' into benibenj/radical-skink
benibenj Jun 24, 2024
c2cbc65
Fix merge conflict
benibenj Jun 24, 2024
d6c4595
type check
benibenj Jun 24, 2024
bff2171
fix tests
benibenj Jun 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 137 additions & 17 deletions src/vs/platform/configuration/common/configurationRegistry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,21 +233,24 @@ export interface IConfigurationNode {
restrictedProperties?: string[];
}

export type ConfigurationDefaultSource = IExtensionInfo | string;
export type ConfigurationDefaultValueSource = ConfigurationDefaultSource | Map<string, ConfigurationDefaultSource>;

export interface IConfigurationDefaults {
overrides: IStringDictionary<any>;
source?: IExtensionInfo | string;
source?: ConfigurationDefaultSource;
}

export type IRegisteredConfigurationPropertySchema = IConfigurationPropertySchema & {
defaultDefaultValue?: any;
source?: IExtensionInfo; // Source of the Property
defaultValueSource?: IExtensionInfo | string; // Source of the Default Value
defaultValueSource?: ConfigurationDefaultValueSource; // Source of the Default Value
};

export type IConfigurationDefaultOverride = {
readonly value: any;
readonly source?: IExtensionInfo | string; // Source of the default override
readonly valuesSources?: Map<string, IExtensionInfo | string>; // Source of each value in default language overrides
readonly source?: ConfigurationDefaultValueSource; // Source of the default override
readonly valuesSources?: Map<string, ConfigurationDefaultValueSource>; // Source of each value in default language overrides
};

export const allSettings: { properties: IStringDictionary<IConfigurationPropertySchema>; patternProperties: IStringDictionary<IConfigurationPropertySchema> } = { properties: {}, patternProperties: {} };
Expand Down Expand Up @@ -351,13 +354,42 @@ class ConfigurationRegistry implements IConfigurationRegistry {

if (OVERRIDE_PROPERTY_REGEX.test(key)) {
const configurationDefaultOverride = this.configurationDefaultsOverrides.get(key);
const valuesSources = configurationDefaultOverride?.valuesSources ?? new Map<string, IExtensionInfo | string>();
if (source) {
for (const configuration of Object.keys(overrides[key])) {
valuesSources.set(configuration, source);
const valuesSources = configurationDefaultOverride?.valuesSources ?? new Map<string, ConfigurationDefaultValueSource>();

const defaultValue = configurationDefaultOverride?.value || {};
for (const configuration of Object.keys(overrides[key])) {
const overrideValue = overrides[key][configuration];

const isObjectSetting = types.isObject(overrideValue) && (types.isUndefined(defaultValue[configuration]) || types.isObject(defaultValue[configuration]));
if (isObjectSetting) {
// Objects are merged instead of overridden
defaultValue[configuration] = { ...(defaultValue[configuration] ?? {}), ...overrideValue };

// Track the source of each value in the object
if (source) {
let objectConfigurationSources = valuesSources.get(configuration);
if (!objectConfigurationSources) {
objectConfigurationSources = new Map<string, ConfigurationDefaultSource>();
valuesSources.set(configuration, objectConfigurationSources);
}
if (!(objectConfigurationSources instanceof Map)) {
console.error('objectConfigurationSources is not a Map');
continue;
}

for (const objectKey in overrideValue) {
objectConfigurationSources.set(objectKey, source);
}
}
} else {
// Primitive values are overridden
defaultValue[configuration] = overrideValue;
if (source) {
valuesSources.set(configuration, source);
}
}
}
const defaultValue = { ...(configurationDefaultOverride?.value || {}), ...overrides[key] };

this.configurationDefaultsOverrides.set(key, { source, value: defaultValue, valuesSources });
const plainKey = getLanguageTagSettingPlainKey(key);
const property: IRegisteredConfigurationPropertySchema = {
Expand All @@ -373,8 +405,33 @@ class ConfigurationRegistry implements IConfigurationRegistry {
this.configurationProperties[key] = property;
this.defaultLanguageConfigurationOverridesNode.properties![key] = property;
} else {
this.configurationDefaultsOverrides.set(key, { value: overrides[key], source });
const property = this.configurationProperties[key];
let defaultValue = overrides[key];
let defaultValueSource: ConfigurationDefaultValueSource | undefined = source;

// If the default value is an object, merge the objects and store the source of each keys
if (property.type === 'object' && types.isObject(overrides[key])) {
const objectDefaults = this.configurationDefaultsOverrides.get(key);
const existingDefaultValue = objectDefaults?.value ?? property.defaultDefaultValue ?? {};
defaultValue = { ...existingDefaultValue, ...overrides[key] };

defaultValueSource = objectDefaults?.source ?? new Map<string, ConfigurationDefaultSource>();
if (!(defaultValueSource instanceof Map)) {
console.error('defaultValueSource is not a Map');
continue;
}

for (const objectKey in overrides[key]) {
if (source) {
defaultValueSource.set(objectKey, source);
} else {
defaultValueSource.delete(objectKey);
}
}
}

this.configurationDefaultsOverrides.set(key, { value: defaultValue, source: defaultValueSource });

if (property) {
this.updatePropertyDefaultValue(key, property);
this.updateSchema(key, property);
Expand All @@ -397,24 +454,87 @@ class ConfigurationRegistry implements IConfigurationRegistry {

for (const { overrides, source } of defaultConfigurations) {
for (const key in overrides) {
const configurationDefaultsOverride = this.configurationDefaultsOverrides.get(key);
const id = types.isString(source) ? source : source?.id;
const configurationDefaultsOverrideSourceId = types.isString(configurationDefaultsOverride?.source) ? configurationDefaultsOverride?.source : configurationDefaultsOverride?.source?.id;
if (id !== configurationDefaultsOverrideSourceId) {

const configurationDefaultsOverride = this.configurationDefaultsOverrides.get(key);
if (!configurationDefaultsOverride) {
continue;
}
bucket.add(key);
this.configurationDefaultsOverrides.delete(key);

if (OVERRIDE_PROPERTY_REGEX.test(key)) {
delete this.configurationProperties[key];
delete this.defaultLanguageConfigurationOverridesNode.properties![key];
for (const configuration of Object.keys(overrides[key])) {
const overrideValue = overrides[key][configuration];

if (types.isObject(overrideValue)) {
const configurationSource = configurationDefaultsOverride.valuesSources?.get(configuration) as Map<string, ConfigurationDefaultSource> | undefined;

for (const overrideObjectKey of Object.keys(overrideValue)) {
const keySource = configurationSource?.get(overrideObjectKey);
const keySourceId = types.isString(keySource) ? keySource : keySource?.id;
if (keySourceId === id) {
configurationSource?.delete(overrideObjectKey);
delete configurationDefaultsOverride.value[configuration][overrideObjectKey];
}
}

if (Object.keys(configurationDefaultsOverride.value[configuration]).length === 0) {
delete configurationDefaultsOverride.value[configuration];
configurationDefaultsOverride.valuesSources?.delete(configuration);
}
} else {
const configurationSource = configurationDefaultsOverride.valuesSources?.get(configuration) as string | IExtensionInfo | undefined;

const keySourceId = types.isString(configurationSource) ? configurationSource : configurationSource?.id;
if (keySourceId === id) {
configurationDefaultsOverride.valuesSources?.delete(configuration);
delete configurationDefaultsOverride.value[configuration];
}
}
}
// Remove language configuration if empty ({[css]: {}} => {})
const languageValues = this.configurationDefaultsOverrides.get(key);
if (languageValues && Object.keys(languageValues.value).length === 0) {
this.configurationDefaultsOverrides.delete(key);
delete this.configurationProperties[key];
delete this.defaultLanguageConfigurationOverridesNode.properties![key];
}
} else {
// If the default value is an object, remove the source of each key
if (configurationDefaultsOverride.source instanceof Map) {

const keySources = configurationDefaultsOverride.source;
for (const objectKey in overrides[key]) {
const keySource = keySources.get(objectKey);
const keySourceId = types.isString(keySource) ? keySource : keySource?.id;

if (keySourceId === id) {
keySources.delete(objectKey);
delete configurationDefaultsOverride.value[objectKey];
}
}

if (keySources.size === 0) {
this.configurationDefaultsOverrides.delete(key);
}
}
// Otherwise, remove the default value if the source matches
else {
const configurationDefaultsOverrideSourceId = types.isString(configurationDefaultsOverride.source) ? configurationDefaultsOverride.source : configurationDefaultsOverride.source?.id;
if (id !== configurationDefaultsOverrideSourceId) {
continue; // Another source is overriding this default value
}

this.configurationDefaultsOverrides.delete(key);

}
const property = this.configurationProperties[key];
if (property) {
this.updatePropertyDefaultValue(key, property);
this.updateSchema(key, property);
}
}

bucket.add(key);
}
}

Expand Down
32 changes: 30 additions & 2 deletions src/vs/platform/configuration/test/common/configurationRegistry.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ suite('ConfigurationRegistry', () => {
assert.deepStrictEqual(configurationRegistry.getConfigurationProperties()['[lang]'].default, { a: 2, b: 2, c: 3 });
});

test('configuration defaults - overrides defaults', async () => {
test('configuration defaults - merge object default overrides', async () => {
configurationRegistry.registerConfiguration({
'id': '_test_default',
'type': 'object',
Expand All @@ -51,7 +51,7 @@ suite('ConfigurationRegistry', () => {
configurationRegistry.registerDefaultConfigurations([{ overrides: { 'config': { a: 1, b: 2 } } }]);
configurationRegistry.registerDefaultConfigurations([{ overrides: { 'config': { a: 2, c: 3 } } }]);

assert.deepStrictEqual(configurationRegistry.getConfigurationProperties()['config'].default, { a: 2, c: 3 });
assert.deepStrictEqual(configurationRegistry.getConfigurationProperties()['config'].default, { a: 2, b: 2, c: 3 });
});

test('registering multiple settings with same policy', async () => {
Expand Down Expand Up @@ -79,4 +79,32 @@ suite('ConfigurationRegistry', () => {
assert.ok(actual['policy1'] !== undefined);
assert.ok(actual['policy2'] === undefined);
});

test('configuration defaults - deregister merged object default override', async () => {
configurationRegistry.registerConfiguration({
'id': '_test_default',
'type': 'object',
'properties': {
'config': {
'type': 'object',
}
}
});

const overrides1 = [{ overrides: { 'config': { a: 1, b: 2 } }, source: 'source1' }];
const overrides2 = [{ overrides: { 'config': { a: 2, c: 3 } }, source: 'source2' }];

configurationRegistry.registerDefaultConfigurations(overrides1);
configurationRegistry.registerDefaultConfigurations(overrides2);

assert.deepStrictEqual(configurationRegistry.getConfigurationProperties()['config'].default, { a: 2, b: 2, c: 3 });

configurationRegistry.deregisterDefaultConfigurations(overrides2);

assert.deepStrictEqual(configurationRegistry.getConfigurationProperties()['config'].default, { b: 2 }); // TODO this should actualy equal overrides1

configurationRegistry.deregisterDefaultConfigurations(overrides1);

assert.deepStrictEqual(configurationRegistry.getConfigurationProperties()['config'].default, {});
});
});
61 changes: 55 additions & 6 deletions src/vs/platform/configuration/test/common/configurations.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ suite('DefaultConfiguration', () => {

assert.ok(equals(actual.getValue('a'), { b: { c: '2' } }));
assert.ok(equals(actual.contents, { 'a': { b: { c: '2' } } }));
assert.deepStrictEqual(actual.keys, ['a.b', 'a.b.c']);
assert.deepStrictEqual(actual.keys.sort(), ['a.b', 'a.b.c']);
});

test('Test registering the same property again', async () => {
Expand Down Expand Up @@ -158,7 +158,7 @@ suite('DefaultConfiguration', () => {
assert.ok(equals(actual.getValue('[a]'), { 'b': true }));
assert.ok(equals(actual.contents, { '[a]': { 'b': true } }));
assert.ok(equals(actual.overrides, [{ contents: { 'b': true }, identifiers: ['a'], keys: ['b'] }]));
assert.deepStrictEqual(actual.keys, ['[a]']);
assert.deepStrictEqual(actual.keys.sort(), ['[a]']);
assert.strictEqual(actual.getOverrideValue('b', 'a'), true);
});

Expand Down Expand Up @@ -191,7 +191,7 @@ suite('DefaultConfiguration', () => {
assert.ok(equals(actual.getValue('[a]'), { 'b': true }));
assert.ok(equals(actual.contents, { 'b': false, '[a]': { 'b': true } }));
assert.ok(equals(actual.overrides, [{ contents: { 'b': true }, identifiers: ['a'], keys: ['b'] }]));
assert.deepStrictEqual(actual.keys, ['b', '[a]']);
assert.deepStrictEqual(actual.keys.sort(), ['[a]', 'b']);
assert.strictEqual(actual.getOverrideValue('b', 'a'), true);
});

Expand Down Expand Up @@ -227,7 +227,7 @@ suite('DefaultConfiguration', () => {
assert.ok(equals(actual.getValue('[a]'), { 'b': true }));
assert.ok(equals(actual.contents, { 'b': false, '[a]': { 'b': true } }));
assert.ok(equals(actual.overrides, [{ contents: { 'b': true }, identifiers: ['a'], keys: ['b'] }]));
assert.deepStrictEqual(actual.keys, ['[a]', 'b']);
assert.deepStrictEqual(actual.keys.sort(), ['[a]', 'b']);
assert.strictEqual(actual.getOverrideValue('b', 'a'), true);
assert.deepStrictEqual(properties, ['b']);
});
Expand Down Expand Up @@ -263,7 +263,7 @@ suite('DefaultConfiguration', () => {
assert.ok(equals(actual.getValue('[a]'), { 'b': true }));
assert.ok(equals(actual.contents, { 'b': false, '[a]': { 'b': true } }));
assert.ok(equals(actual.overrides, [{ contents: { 'b': true }, identifiers: ['a'], keys: ['b'] }]));
assert.deepStrictEqual(actual.keys, ['b', '[a]']);
assert.deepStrictEqual(actual.keys.sort(), ['[a]', 'b']);
assert.strictEqual(actual.getOverrideValue('b', 'a'), true);
assert.deepStrictEqual(properties, ['[a]']);
});
Expand Down Expand Up @@ -299,7 +299,7 @@ suite('DefaultConfiguration', () => {
assert.ok(equals(actual.getValue('[a]'), { 'b': true }));
assert.ok(equals(actual.contents, { 'b': false, '[a]': { 'b': true } }));
assert.ok(equals(actual.overrides, [{ contents: { 'b': true }, identifiers: ['a'], keys: ['b'] }]));
assert.deepStrictEqual(actual.keys, ['b', '[a]']);
assert.deepStrictEqual(actual.keys.sort(), ['[a]', 'b']);
assert.strictEqual(actual.getOverrideValue('b', 'a'), true);
});

Expand Down Expand Up @@ -361,4 +361,53 @@ suite('DefaultConfiguration', () => {
assert.deepStrictEqual(testObject.configurationModel.keys, ['b']);
assert.strictEqual(testObject.configurationModel.getOverrideValue('b', 'a'), undefined);
});

test('Test deregistering a merged language object setting', async () => {
const testObject = disposables.add(new DefaultConfiguration(new NullLogService()));
configurationRegistry.registerConfiguration({
'id': 'b',
'order': 1,
'title': 'b',
'type': 'object',
'properties': {
'b': {
'description': 'b',
'type': 'object',
'default': {},
}
}
});
const node1 = {
overrides: {
'[a]': {
'b': {
'aa': '1',
'bb': '2'
}
}
},
source: 'source1'
};

const node2 = {
overrides: {
'[a]': {
'b': {
'bb': '20',
'cc': '30'
}
}
},
source: 'source2'
};
configurationRegistry.registerDefaultConfigurations([node1]);
configurationRegistry.registerDefaultConfigurations([node2]);
await testObject.initialize();
configurationRegistry.deregisterDefaultConfigurations([node1]);
assert.ok(equals(testObject.configurationModel.getValue('[a]'), { 'b': { 'bb': '20', 'cc': '30' } }));
assert.ok(equals(testObject.configurationModel.contents, { '[a]': { 'b': { 'bb': '20', 'cc': '30' } }, 'b': {} }));
//assert.ok(equals(testObject.configurationModel.overrides, [{ '[a]': { 'b': { 'bb': '20', 'cc': '30' } } }])); TODO: Check this later
//assert.deepStrictEqual(testObject.configurationModel.keys.sort(), ['[a]', 'b']);
assert.ok(equals(testObject.configurationModel.getOverrideValue('b', 'a'), { 'bb': '20', 'cc': '30' }));
});
});
2 changes: 1 addition & 1 deletion src/vs/workbench/browser/workbench.contribution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ const registry = Registry.as<IConfigurationRegistry>(ConfigurationExtensions.Con
})(),
additionalProperties:
{
type: 'string',
type: ['string', 'null'],
markdownDescription: localize('workbench.editor.label.template', "The template which should be rendered when the pattern matches. May include the variables ${dirname}, ${filename} and ${extname}."),
minLength: 1,
pattern: '.*[a-zA-Z0-9].*'
Expand Down
Loading
Loading