Skip to content

Commit 5b9cf3d

Browse files
authored
* Fix microsoft#176813 * fix tests * fix tests * address feedback
1 parent 4529b4a commit 5b9cf3d

File tree

13 files changed

+407
-103
lines changed

13 files changed

+407
-103
lines changed

src/vs/platform/configuration/common/configurationModels.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,10 @@ export class ConfigurationModel implements IConfigurationModel {
269269
}
270270

271271
export interface ConfigurationParseOptions {
272-
scopes: ConfigurationScope[] | undefined;
272+
scopes?: ConfigurationScope[];
273273
skipRestricted?: boolean;
274+
include?: string[];
275+
exclude?: string[];
274276
}
275277

276278
export class ConfigurationModelParser {
@@ -383,7 +385,7 @@ export class ConfigurationModelParser {
383385

384386
private filter(properties: any, configurationProperties: { [qualifiedKey: string]: IConfigurationPropertySchema | undefined }, filterOverriddenProperties: boolean, options?: ConfigurationParseOptions): { raw: {}; restricted: string[]; hasExcludedProperties: boolean } {
385387
let hasExcludedProperties = false;
386-
if (!options?.scopes && !options?.skipRestricted) {
388+
if (!options?.scopes && !options?.skipRestricted && !options?.exclude?.length) {
387389
return { raw: properties, restricted: [], hasExcludedProperties };
388390
}
389391
const raw: any = {};
@@ -400,9 +402,10 @@ export class ConfigurationModelParser {
400402
if (propertySchema?.restricted) {
401403
restricted.push(key);
402404
}
403-
// Load unregistered configurations always.
404-
if ((scope === undefined || options.scopes === undefined || options.scopes.includes(scope)) // Check scopes
405-
&& !(options.skipRestricted && propertySchema?.restricted)) { // Check restricted
405+
if (!options.exclude?.includes(key) /* Check exclude */
406+
&& (options.include?.includes(key) /* Check include */
407+
|| ((scope === undefined || options.scopes === undefined || options.scopes.includes(scope)) /* Check scopes */
408+
&& !(options.skipRestricted && propertySchema?.restricted)))) /* Check restricted */ {
406409
raw[key] = properties[key];
407410
} else {
408411
hasExcludedProperties = true;
@@ -435,19 +438,17 @@ export class ConfigurationModelParser {
435438
export class UserSettings extends Disposable {
436439

437440
private readonly parser: ConfigurationModelParser;
438-
private readonly parseOptions: ConfigurationParseOptions;
439441
protected readonly _onDidChange: Emitter<void> = this._register(new Emitter<void>());
440442
readonly onDidChange: Event<void> = this._onDidChange.event;
441443

442444
constructor(
443445
private readonly userSettingsResource: URI,
444-
private readonly scopes: ConfigurationScope[] | undefined,
446+
protected parseOptions: ConfigurationParseOptions,
445447
extUri: IExtUri,
446448
private readonly fileService: IFileService
447449
) {
448450
super();
449451
this.parser = new ConfigurationModelParser(this.userSettingsResource.toString());
450-
this.parseOptions = { scopes: this.scopes };
451452
this._register(this.fileService.watch(extUri.dirname(this.userSettingsResource)));
452453
// Also listen to the resource incase the resource is a symlink - https://github.com/microsoft/vscode/issues/118134
453454
this._register(this.fileService.watch(this.userSettingsResource));
@@ -467,7 +468,10 @@ export class UserSettings extends Disposable {
467468
}
468469
}
469470

470-
reparse(): ConfigurationModel {
471+
reparse(parseOptions?: ConfigurationParseOptions): ConfigurationModel {
472+
if (parseOptions) {
473+
this.parseOptions = parseOptions;
474+
}
471475
this.parser.reparse(this.parseOptions);
472476
return this.parser.configurationModel;
473477
}

src/vs/platform/configuration/common/configurationService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ export class ConfigurationService extends Disposable implements IConfigurationSe
3737
super();
3838
this.defaultConfiguration = this._register(new DefaultConfiguration());
3939
this.policyConfiguration = policyService instanceof NullPolicyService ? new NullPolicyConfiguration() : this._register(new PolicyConfiguration(this.defaultConfiguration, policyService, logService));
40-
this.userConfiguration = this._register(new UserSettings(this.settingsResource, undefined, extUriBiasedIgnorePathCase, fileService));
40+
this.userConfiguration = this._register(new UserSettings(this.settingsResource, {}, extUriBiasedIgnorePathCase, fileService));
4141
this.configuration = new Configuration(this.defaultConfiguration.configurationModel, this.policyConfiguration.configurationModel, new ConfigurationModel(), new ConfigurationModel());
4242

4343
this.reloadConfigurationScheduler = this._register(new RunOnceScheduler(() => this.reloadConfiguration(), 50));

src/vs/platform/configuration/test/common/configurationModels.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,26 @@ import * as assert from 'assert';
66
import { join } from 'vs/base/common/path';
77
import { URI } from 'vs/base/common/uri';
88
import { Configuration, ConfigurationChangeEvent, ConfigurationModel, ConfigurationModelParser, mergeChanges } from 'vs/platform/configuration/common/configurationModels';
9+
import { IConfigurationRegistry, Extensions, ConfigurationScope } from 'vs/platform/configuration/common/configurationRegistry';
10+
import { Registry } from 'vs/platform/registry/common/platform';
911
import { WorkspaceFolder } from 'vs/platform/workspace/common/workspace';
1012
import { Workspace } from 'vs/platform/workspace/test/common/testWorkspace';
1113

1214
suite('ConfigurationModelParser', () => {
1315

16+
suiteSetup(() => {
17+
Registry.as<IConfigurationRegistry>(Extensions.Configuration).registerConfiguration({
18+
'id': 'ConfigurationModelParserTest',
19+
'type': 'object',
20+
'properties': {
21+
'ConfigurationModelParserTest.windowSetting': {
22+
'type': 'string',
23+
'default': 'isSet',
24+
}
25+
}
26+
});
27+
});
28+
1429
test('parse configuration model with single override identifier', () => {
1530
const testObject = new ConfigurationModelParser('');
1631

@@ -35,6 +50,39 @@ suite('ConfigurationModelParser', () => {
3550
assert.deepStrictEqual(JSON.stringify(testObject.configurationModel.overrides), JSON.stringify([{ identifiers: ['x', 'y', 'z'], keys: ['a'], contents: { 'a': 1 } }]));
3651
});
3752

53+
test('parse configuration model with exclude option', () => {
54+
const testObject = new ConfigurationModelParser('');
55+
56+
testObject.parse(JSON.stringify({ 'a': 1, 'b': 2 }), { exclude: ['a'] });
57+
58+
assert.strictEqual(testObject.configurationModel.getValue('a'), undefined);
59+
assert.strictEqual(testObject.configurationModel.getValue('b'), 2);
60+
});
61+
62+
test('parse configuration model with exclude option even included', () => {
63+
const testObject = new ConfigurationModelParser('');
64+
65+
testObject.parse(JSON.stringify({ 'a': 1, 'b': 2 }), { exclude: ['a'], include: ['a'] });
66+
67+
assert.strictEqual(testObject.configurationModel.getValue('a'), undefined);
68+
assert.strictEqual(testObject.configurationModel.getValue('b'), 2);
69+
});
70+
71+
test('parse configuration model with scopes filter', () => {
72+
const testObject = new ConfigurationModelParser('');
73+
74+
testObject.parse(JSON.stringify({ 'ConfigurationModelParserTest.windowSetting': '1' }), { scopes: [ConfigurationScope.APPLICATION] });
75+
76+
assert.strictEqual(testObject.configurationModel.getValue('ConfigurationModelParserTest.windowSetting'), undefined);
77+
});
78+
79+
test('parse configuration model with include option', () => {
80+
const testObject = new ConfigurationModelParser('');
81+
82+
testObject.parse(JSON.stringify({ 'ConfigurationModelParserTest.windowSetting': '1' }), { include: ['ConfigurationModelParserTest.windowSetting'], scopes: [ConfigurationScope.APPLICATION] });
83+
84+
assert.strictEqual(testObject.configurationModel.getValue('ConfigurationModelParserTest.windowSetting'), '1');
85+
});
3886

3987
});
4088

src/vs/platform/configuration/test/common/configurations.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,24 +62,24 @@ suite('DefaultConfiguration', () => {
6262

6363
test('Test registering a property after initialize', async () => {
6464
const testObject = new DefaultConfiguration();
65-
const promise = Event.toPromise(testObject.onDidChangeConfiguration);
6665
await testObject.initialize();
66+
const promise = Event.toPromise(testObject.onDidChangeConfiguration);
6767
configurationRegistry.registerConfiguration({
6868
'id': 'a',
6969
'order': 1,
7070
'title': 'a',
7171
'type': 'object',
7272
'properties': {
73-
'a': {
73+
'defaultConfiguration.testSetting1': {
7474
'description': 'a',
7575
'type': 'boolean',
7676
'default': false,
7777
}
7878
}
7979
});
8080
const { defaults: actual, properties } = await promise;
81-
assert.strictEqual(actual.getValue('a'), false);
82-
assert.deepStrictEqual(properties, ['a']);
81+
assert.strictEqual(actual.getValue('defaultConfiguration.testSetting1'), false);
82+
assert.deepStrictEqual(properties, ['defaultConfiguration.testSetting1']);
8383
});
8484

8585
test('Test registering nested properties', async () => {

src/vs/workbench/contrib/preferences/browser/preferencesRenderers.ts

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import { IUserDataProfileService } from 'vs/workbench/services/userDataProfile/c
4444
import { isEqual } from 'vs/base/common/resources';
4545
import { IUserDataProfilesService } from 'vs/platform/userDataProfile/common/userDataProfile';
4646
import { IStringDictionary } from 'vs/base/common/collections';
47+
import { APPLY_ALL_PROFILES_SETTING, IWorkbenchConfigurationService } from 'vs/workbench/services/configuration/common/configuration';
4748

4849
export interface IPreferencesRenderer extends IDisposable {
4950
render(): void;
@@ -481,7 +482,7 @@ class UnsupportedSettingsRenderer extends Disposable implements languages.CodeAc
481482
private readonly settingsEditorModel: SettingsEditorModel,
482483
@IMarkerService private readonly markerService: IMarkerService,
483484
@IWorkbenchEnvironmentService private readonly environmentService: IWorkbenchEnvironmentService,
484-
@IConfigurationService private readonly configurationService: IConfigurationService,
485+
@IWorkbenchConfigurationService private readonly configurationService: IWorkbenchConfigurationService,
485486
@IWorkspaceTrustManagementService private readonly workspaceTrustManagementService: IWorkspaceTrustManagementService,
486487
@IUriIdentityService private readonly uriIdentityService: IUriIdentityService,
487488
@ILanguageFeaturesService languageFeaturesService: ILanguageFeaturesService,
@@ -603,19 +604,27 @@ class UnsupportedSettingsRenderer extends Disposable implements languages.CodeAc
603604

604605
private handleLocalUserConfiguration(setting: ISetting, configuration: IConfigurationPropertySchema, markerData: IMarkerData[]): void {
605606
if (!this.userDataProfileService.currentProfile.isDefault && !this.userDataProfileService.currentProfile.useDefaultFlags?.settings) {
606-
if (isEqual(this.userDataProfilesService.defaultProfile.settingsResource, this.settingsEditorModel.uri) && configuration.scope !== ConfigurationScope.APPLICATION) {
607-
// If we're in the default profile setting file, and the setting is not
608-
// application-scoped, fade it out.
607+
if (isEqual(this.userDataProfilesService.defaultProfile.settingsResource, this.settingsEditorModel.uri) && !this.configurationService.isSettingAppliedForAllProfiles(setting.key)) {
608+
// If we're in the default profile setting file, and the setting cannot be applied in all profiles
609609
markerData.push({
610610
severity: MarkerSeverity.Hint,
611611
tags: [MarkerTag.Unnecessary],
612612
...setting.range,
613613
message: nls.localize('defaultProfileSettingWhileNonDefaultActive', "This setting cannot be applied while a non-default profile is active. It will be applied when the default profile is active.")
614614
});
615-
} else if (isEqual(this.userDataProfileService.currentProfile.settingsResource, this.settingsEditorModel.uri) && configuration.scope === ConfigurationScope.APPLICATION) {
616-
// If we're in a profile setting file, and the setting is
617-
// application-scoped, fade it out.
618-
markerData.push(this.generateUnsupportedApplicationSettingMarker(setting));
615+
} else if (isEqual(this.userDataProfileService.currentProfile.settingsResource, this.settingsEditorModel.uri)) {
616+
if (configuration.scope === ConfigurationScope.APPLICATION) {
617+
// If we're in a profile setting file, and the setting is application-scoped, fade it out.
618+
markerData.push(this.generateUnsupportedApplicationSettingMarker(setting));
619+
} else if (this.configurationService.isSettingAppliedForAllProfiles(setting.key)) {
620+
// If we're in the non-default profile setting file, and the setting can be applied in all profiles, fade it out.
621+
markerData.push({
622+
severity: MarkerSeverity.Hint,
623+
tags: [MarkerTag.Unnecessary],
624+
...setting.range,
625+
message: nls.localize('allProfileSettingWhileInNonDefaultProfileSetting', "This setting cannot be applied because it is configured to be applied in all profiles using setting {0}. Value from the default profile will be used instead.", APPLY_ALL_PROFILES_SETTING)
626+
});
627+
}
619628
}
620629
}
621630
if (this.environmentService.remoteAuthority && (configuration.scope === ConfigurationScope.MACHINE || configuration.scope === ConfigurationScope.MACHINE_OVERRIDABLE)) {

src/vs/workbench/contrib/preferences/browser/settingsEditorSettingIndicators.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,12 @@ import { IDisposable, DisposableStore } from 'vs/base/common/lifecycle';
1515
import { ILanguageService } from 'vs/editor/common/languages/language';
1616
import { localize } from 'vs/nls';
1717
import { ICommandService } from 'vs/platform/commands/common/commands';
18-
import { ConfigurationTarget, IConfigurationService } from 'vs/platform/configuration/common/configuration';
18+
import { ConfigurationTarget } from 'vs/platform/configuration/common/configuration';
1919
import { IUserDataProfilesService } from 'vs/platform/userDataProfile/common/userDataProfile';
2020
import { IUserDataSyncEnablementService } from 'vs/platform/userDataSync/common/userDataSync';
2121
import { SettingsTreeSettingElement } from 'vs/workbench/contrib/preferences/browser/settingsTreeModels';
2222
import { POLICY_SETTING_TAG } from 'vs/workbench/contrib/preferences/common/preferences';
23+
import { IWorkbenchConfigurationService } from 'vs/workbench/services/configuration/common/configuration';
2324
import { IHoverOptions, IHoverService, IHoverWidget } from 'vs/workbench/services/hover/browser/hover';
2425

2526
const $ = DOM.$;
@@ -75,7 +76,7 @@ export class SettingsTreeIndicatorsLabel implements IDisposable {
7576

7677
constructor(
7778
container: HTMLElement,
78-
@IConfigurationService private readonly configurationService: IConfigurationService,
79+
@IWorkbenchConfigurationService private readonly configurationService: IWorkbenchConfigurationService,
7980
@IHoverService private readonly hoverService: IHoverService,
8081
@IUserDataSyncEnablementService private readonly userDataSyncEnablementService: IUserDataSyncEnablementService,
8182
@ILanguageService private readonly languageService: ILanguageService,
@@ -330,14 +331,11 @@ export class SettingsTreeIndicatorsLabel implements IDisposable {
330331
}, focus);
331332
};
332333
this.addHoverDisposables(this.scopeOverridesIndicator.disposables, this.scopeOverridesIndicator.element, showHover);
333-
} else if (this.profilesEnabled && element.matchesScope(ConfigurationTarget.APPLICATION, false)) {
334-
// If the setting is an application-scoped setting, there are no overrides so we can use this
335-
// indicator to display that information instead.
334+
} else if (this.profilesEnabled && element.settingsTarget === ConfigurationTarget.USER_LOCAL && this.configurationService.isSettingAppliedForAllProfiles(element.setting.key)) {
336335
this.scopeOverridesIndicator.element.style.display = 'inline';
337336
this.scopeOverridesIndicator.element.classList.add('setting-indicator');
338337

339-
const applicationSettingText = localize('applicationSetting', "Applies to all profiles");
340-
this.scopeOverridesIndicator.label.text = applicationSettingText;
338+
this.scopeOverridesIndicator.label.text = localize('applicationSetting', "Applies to all profiles");
341339

342340
const content = localize('applicationSettingDescription', "The setting is not specific to the current profile, and will retain its value when switching profiles.");
343341
const showHover = (focus: boolean) => {
@@ -499,18 +497,17 @@ function getAccessibleScopeDisplayMidSentenceText(completeScope: string, languag
499497
return localizedScope;
500498
}
501499

502-
export function getIndicatorsLabelAriaLabel(element: SettingsTreeSettingElement, configurationService: IConfigurationService, userDataProfilesService: IUserDataProfilesService, languageService: ILanguageService): string {
500+
export function getIndicatorsLabelAriaLabel(element: SettingsTreeSettingElement, configurationService: IWorkbenchConfigurationService, userDataProfilesService: IUserDataProfilesService, languageService: ILanguageService): string {
503501
const ariaLabelSections: string[] = [];
504502

505503
// Add workspace trust text
506504
if (element.isUntrusted) {
507505
ariaLabelSections.push(localize('workspaceUntrustedAriaLabel', "Workspace untrusted; setting value not applied"));
508506
}
509507

510-
const profilesEnabled = userDataProfilesService.isEnabled();
511508
if (element.hasPolicyValue) {
512509
ariaLabelSections.push(localize('policyDescriptionAccessible', "Managed by organization policy; setting value not applied"));
513-
} else if (profilesEnabled && element.matchesScope(ConfigurationTarget.APPLICATION, false)) {
510+
} else if (userDataProfilesService.isEnabled() && element.settingsTarget === ConfigurationTarget.USER_LOCAL && configurationService.isSettingAppliedForAllProfiles(element.setting.key)) {
514511
ariaLabelSections.push(localize('applicationSettingDescriptionAccessible', "Setting value retained when switching profiles"));
515512
} else {
516513
// Add other overrides text

0 commit comments

Comments
 (0)