Skip to content

Commit

Permalink
preferences: merge debug configurations (#12174)
Browse files Browse the repository at this point in the history
* add `PreferenceProvider.canHandleScope(scope)`

This is useful to disable the `PreferenceScope.Folders` scope when it
no longer applies upon preference resolution.

* preferences: fix `PreferenceProvider.merge` method

This reverts commit a3b9735 and fixes
the launch-preferences tests

---------

Co-authored-by: Alexandra Buzila <[email protected]>
Co-authored-by: Vincent Fugnitto <[email protected]>
  • Loading branch information
3 people authored Mar 23, 2023
1 parent cd93bd1 commit 656cd9b
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 37 deletions.
101 changes: 66 additions & 35 deletions examples/api-tests/src/launch-preferences.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,8 @@ describe('Launch Preferences', function () {
},
expectation: {
'version': '0.2.0',
'configurations': [validConfiguration, bogusConfiguration],
'compounds': [bogusCompound, bogusCompound2]
'configurations': [validConfiguration2, validConfiguration, bogusConfiguration],
'compounds': [validCompound, bogusCompound, bogusCompound2]
}
});

Expand All @@ -306,15 +306,15 @@ describe('Launch Preferences', function () {
expectation: {
'version': '0.2.0',
'configurations': [validConfiguration2],
'compounds': [bogusCompound, bogusCompound2]
'compounds': [validCompound, bogusCompound, bogusCompound2]
},
inspectExpectation: {
preferenceName: 'launch',
defaultValue: defaultLaunch,
workspaceValue: {
'version': '0.2.0',
'configurations': [validConfiguration2],
'compounds': [bogusCompound, bogusCompound2]
'compounds': [validCompound, bogusCompound, bogusCompound2]
}
}
});
Expand Down Expand Up @@ -461,6 +461,53 @@ describe('Launch Preferences', function () {
]);
}


function mergeLaunchConfigurations(config1, config2) {
if (config1 === undefined && config2 === undefined) {
return undefined;
}
if (config2 === undefined) {
return config1;
}

let result;
// skip invalid configs
if (typeof config1 === 'object' && !Array.isArray(config1)) {
result = { ...config1 };
}
if (typeof config2 === 'object' && !Array.isArray(config2)) {
result = { ...(result ?? {}), ...config2 }
}
// merge configurations and compounds arrays
const mergedConfigurations = mergeArrays(config1?.configurations, config2?.configurations);
if (mergedConfigurations) {
result.configurations = mergedConfigurations
}
const mergedCompounds = mergeArrays(config1?.compounds, config2?.compounds);
if (mergedCompounds) {
result.compounds = mergedCompounds;
}
return result;
}

function mergeArrays(array1, array2) {
if (array1 === undefined && array2 === undefined) {
return undefined;
}
if (!Array.isArray(array1) && !Array.isArray(array2)) {
return undefined;
}
let result = [];
if (Array.isArray(array1)) {
result = [...array1];
}
if (Array.isArray(array2)) {
result = [...result, ...array2];
}
return result;
}


const originalShouldOverwrite = fileResourceResolver['shouldOverwrite'];

before(async () => {
Expand Down Expand Up @@ -560,8 +607,8 @@ describe('Launch Preferences', function () {
preferenceName: 'launch',
defaultValue: defaultLaunch
};
const workspaceValue = launch || settingsLaunch;
if (workspaceValue !== undefined) {
const workspaceValue = mergeLaunchConfigurations(settingsLaunch, launch);
if (workspaceValue !== undefined && JSON.stringify(workspaceValue) !== '{}') {
Object.assign(expected, { workspaceValue });
}
}
Expand All @@ -582,8 +629,8 @@ describe('Launch Preferences', function () {
workspaceFolderValue: inspectExpectation.workspaceValue
});
} else {
const value = launch || settingsLaunch;
if (value !== undefined) {
const value = mergeLaunchConfigurations(settingsLaunch, launch);
if (value !== undefined && JSON.stringify(value) !== '{}') {
Object.assign(expected, {
workspaceValue: value,
workspaceFolderValue: value
Expand All @@ -599,7 +646,7 @@ describe('Launch Preferences', function () {

const inspect = preferences.inspect('launch');
const actual = inspect && inspect.workspaceValue;
const expected = settingsLaunch && !Array.isArray(settingsLaunch) ? { ...settingsLaunch, ...validLaunch } : validLaunch;
const expected = mergeLaunchConfigurations(settingsLaunch, validLaunch);
assert.deepStrictEqual(actual, expected);
});

Expand All @@ -608,7 +655,7 @@ describe('Launch Preferences', function () {

const inspect = preferences.inspect('launch');
const actual = inspect && inspect.workspaceValue;
const expected = settingsLaunch && !Array.isArray(settingsLaunch) ? { ...settingsLaunch, ...validLaunch } : validLaunch;
const expected = mergeLaunchConfigurations(settingsLaunch, validLaunch);
assert.deepStrictEqual(actual, expected);
});

Expand All @@ -626,7 +673,7 @@ describe('Launch Preferences', function () {

const inspect = preferences.inspect('launch');
const actual = inspect && inspect.workspaceValue;
const expected = settingsLaunch && !Array.isArray(settingsLaunch) ? { ...settingsLaunch, ...validLaunch } : validLaunch;
const expected = mergeLaunchConfigurations(settingsLaunch, validLaunch);
assert.deepStrictEqual(actual, expected);
});

Expand All @@ -636,7 +683,11 @@ describe('Launch Preferences', function () {

const inspect = preferences.inspect('launch');
const actual = inspect && inspect.workspaceValue && inspect.workspaceValue.configurations;
assert.deepStrictEqual(actual, [validConfiguration, validConfiguration2]);
let expect = [validConfiguration, validConfiguration2];
if (Array.isArray(settingsLaunch?.configurations)) {
expect = [...(settingsLaunch.configurations), ...expect]
}
assert.deepStrictEqual(actual, expect);
});
}

Expand All @@ -650,11 +701,8 @@ describe('Launch Preferences', function () {
if (Array.isArray(expected)) {
expected = { ...expected };
}
if (expected && !expected.configurations && settingsLaunch && settingsLaunch.configurations !== undefined) {
expected.configurations = settingsLaunch.configurations;
}
}
expected = expected || settingsLaunch;
expected = mergeLaunchConfigurations(settingsLaunch, expected);
assert.deepStrictEqual(actual && actual.workspaceValue, expected);
});

Expand All @@ -665,31 +713,14 @@ describe('Launch Preferences', function () {
const actual = preferences.inspect('launch');
const actualWorkspaceValue = actual && actual.workspaceValue;

let expected = undefined;
let expected = { ...launch };
if (launch) {
expected = { ...launch };
delete expected['configurations'];
}
if (settings) {
let _settingsLaunch = undefined;
if (typeof settingsLaunch === 'object' && !Array.isArray(settings['launch']) && settings['launch'] !== null) {
_settingsLaunch = settingsLaunch;
} else {
_settingsLaunch = expectation;
}
if (expected) {
if (_settingsLaunch.configurations !== undefined) {
expected.configurations = _settingsLaunch.configurations;
}
} else {
expected = _settingsLaunch;
}
}

expected = mergeLaunchConfigurations(settingsLaunch, expected);
assert.deepStrictEqual(actualWorkspaceValue, expected);
});
}

});

}
Expand Down
36 changes: 36 additions & 0 deletions packages/core/src/browser/preferences/preference-provider.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// *****************************************************************************
// Copyright (C) 2023 EclipseSource and others.
//
// This program and the accompanying materials are made available under the
// terms of the Eclipse Public License v. 2.0 which is available at
// http://www.eclipse.org/legal/epl-2.0.
//
// This Source Code may also be made available under the following Secondary
// Licenses when the conditions for such availability set forth in the Eclipse
// Public License v. 2.0 are satisfied: GNU General Public License, version 2
// with the GNU Classpath Exception which is available at
// https://www.gnu.org/software/classpath/license.html.
//
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
// *****************************************************************************

import { PreferenceProvider } from './preference-provider';
const { expect } = require('chai');

describe('PreferenceProvider', () => {
it('should preserve extra source fields on merge', () => {
const result = PreferenceProvider.merge({ 'configurations': [], 'compounds': [] }, { 'configurations': [] });
expect(result).deep.equals({ 'configurations': [], 'compounds': [] });
});
it('should preserve extra target fields on merge', () => {
const result = PreferenceProvider.merge({ 'configurations': [] }, { 'configurations': [], 'compounds': [] });
expect(result).deep.equals({ 'configurations': [], 'compounds': [] });
});
it('should merge array values', () => {
const result = PreferenceProvider.merge(
{ 'configurations': [{ 'name': 'test1', 'request': 'launch' }], 'compounds': [] },
{ 'configurations': [{ 'name': 'test2' }] }
);
expect(result).deep.equals({ 'configurations': [{ 'name': 'test1', 'request': 'launch' }, { 'name': 'test2' }], 'compounds': [] });
});
});
7 changes: 7 additions & 0 deletions packages/core/src/browser/preferences/preference-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,9 @@ export abstract class PreferenceProvider implements Disposable {
if (JSONExt.isObject(source[key]) && JSONExt.isObject(value)) {
this.merge(source[key], value);
continue;
} else if (JSONExt.isArray(source[key]) && JSONExt.isArray(value)) {
source[key] = [...JSONExt.deepCopy(source[key] as any), ...JSONExt.deepCopy(value)];
continue;
}
}
source[key] = JSONExt.deepCopy(value);
Expand Down Expand Up @@ -267,4 +270,8 @@ export abstract class PreferenceProvider implements Disposable {
}
return preferences;
}

canHandleScope(scope: PreferenceScope): boolean {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ export class PreferenceServiceImpl implements PreferenceService {
for (const scope of PreferenceScope.getScopes()) {
if (this.schema.isValidInScope(preferenceName, scope)) {
const provider = this.getProvider(scope);
if (provider) {
if (provider?.canHandleScope(scope)) {
const { configUri, value } = provider.resolve<T>(preferenceName, resourceUri);
if (value !== undefined) {
result.configUri = configUri;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

import { inject, injectable, postConstruct } from '@theia/core/shared/inversify';
import URI from '@theia/core/lib/common/uri';
import { PreferenceProvider, PreferenceResolveResult } from '@theia/core/lib/browser/preferences/preference-provider';
import { PreferenceProvider, PreferenceResolveResult, PreferenceScope } from '@theia/core/lib/browser/preferences';
import { WorkspaceService } from '@theia/workspace/lib/browser/workspace-service';
import { PreferenceConfigurations } from '@theia/core/lib/browser/preferences/preference-configurations';
import { FolderPreferenceProvider, FolderPreferenceProviderFactory } from './folder-preference-provider';
Expand Down Expand Up @@ -189,6 +189,10 @@ export class FoldersPreferencesProvider extends PreferenceProvider {
return false;
}

override canHandleScope(scope: PreferenceScope): boolean {
return this.workspaceService.isMultiRootWorkspaceOpened && scope === PreferenceScope.Folder || scope === PreferenceScope.Workspace;
}

protected groupProvidersByConfigName(resourceUri?: string): Map<string, FolderPreferenceProvider[]> {
const groups = new Map<string, FolderPreferenceProvider[]>();
const providers = this.getFolderProviders(resourceUri);
Expand Down

0 comments on commit 656cd9b

Please sign in to comment.