preferences: fix duplicated resolution

[paul-marechal]

The WorkspacePreferenceProvider can do one of two things: It may return preferences from the current workspace folder if the workspace is a single root workspace, or it may return preferences as defined in the workspace file if the workspace is a multi-root workspace.

Whenever we want to resolve a preference, the following scopes are queried: Default, User, Workspace, and Folder.

When a single root workspace is opened the following happens:

• Query Default scope [...]
• Query User scope [...]
• Query Workspace scope: use WorkspacePreferenceProvider which delegates to FoldersPreferenceProvider.
• Query Folder scope: use FoldersPreferenceProvider.

Conclusion: The FoldersPreferenceProvider is queried twice.

In order to fix that issue this commit introduces a new component: TogglePreferenceProvider which extends PreferenceProvider and wraps another provider so that it can be enabled/disabled.

Whenever we are running a single root workspace, we'll enable the FoldersPreferenceProvider used by WorkspacePreferenceProvider and disable the one bound to the Folder scope.

Whenever we are running a multi root workspace, we'll disable the FoldersPreferenceProvider used by WorkspacePreferenceProvider and enable the one bound to the Folder scope.

Fixes #12153

How to test

See instructions in #12153

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

[paul-marechal]

cc @AlexandraBuzila

[paul-marechal]

I didn't know what to do with our current preference infrastructure. It's a mess, and this patch doesn't make things really nicer... In the meantime, this patch seems to work (on my machine) and it's not super invasive. If anyone has a better idea please open an alternative PR, or push directly to this one if you can.

[AlexandraBuzila]

Hi @paul-marechal

I also found a bug in the PreferenceProvider while investigating this issue: the merge of array values was producing duplicates. I pushed my changes here eclipsesource@eb8d246
If you agree with them, it would be fine for me if you were to integrate them in your PR.

With my changes alone, some tests from examples/api-tests/src/launch-preferences.spec.js were still failling. While investigating them, I saw that preferences were merged twice, so your changes might have fixed the issues I was seeing when running the tests. I couldn't verify this ATM since the tests won't run now anymore, I think they might need to be adapted to account for the new provider.

[planger]

@paul-marechal I saw that this PR impacts the Playwright page objects. On a first glance, it looks like the selector of the modification indicator in the gutter needs to be adapted. Please let me know if you'd like us to assist with that.

[paul-marechal]

@AlexandraBuzila my problem is that from the code alone:

theia/packages/core/src/browser/preferences/preference-provider.ts

Lines 225 to 246 in 371ab02

	static merge(source: JSONValue | undefined, target: JSONValue): JSONValue {
	if (source === undefined || !JSONExt.isObject(source)) {
	return JSONExt.deepCopy(target);
	}
	if (JSONExt.isPrimitive(target)) {
	return {};
	}
	for (const key of Object.keys(target)) {
	const value = (target as any)[key];
	if (key in source) {
	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);
	}
	return source;
	}

I can't see why values would be duplicated, unless we pass two arrays containing the same data together.

If you read the PR description I highlighted the root cause of the duplication: that is the preference system calls twice the same handler. Since the previous logic didn't properly merge arrays, that didn't seem to cause an issue before.

With that in mind, this PR addresses this problem of FoldersPreferencesProvider being called twice, and so far this seemed to have fixed the duplicated config issue.

[paul-marechal]

@planger I don't think this PR should require updating Playwright object models, let me see if I can fix the issues that appeared in CI first.

[paul-marechal]

Closing in favor of #12174