Skip to content

Commit

Permalink
Change languageFilter to nullable string type
Browse files Browse the repository at this point in the history 
With this change, we explicitly make it so that only the first
language filter given is used.
  • Loading branch information
@rzhao271
rzhao271 committed Mar 17, 2022
1 parent c87aa93 commit 7d6f6f7
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 51 deletions.
27 changes: 13 additions & 14 deletions src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ export class SettingsEditor2 extends EditorPane {

private settingFastUpdateDelayer: Delayer<void>;
private settingSlowUpdateDelayer: Delayer<void>;
private pendingSettingUpdate: { key: string; value: any; languageFilters: string[] | null } | null = null;
private pendingSettingUpdate: { key: string; value: any; languageFilter: string | undefined } | null = null;

private readonly viewState: ISettingsEditorViewState;
private _searchResultModel: SearchResultModel | null = null;
Expand Down Expand Up @@ -875,17 +875,16 @@ export class SettingsEditor2 extends EditorPane {

private onDidChangeSetting(key: string, value: any, type: SettingValueType | SettingValueType[], manualReset: boolean): void {
const parsedQuery = parseQuery(this.searchWidget.getValue());
const languageFilters = (parsedQuery && parsedQuery.languageFilters.length) ?
parsedQuery.languageFilters : null;
const languageFilter = parsedQuery.languageFilter;
if (this.pendingSettingUpdate && this.pendingSettingUpdate.key !== key) {
this.updateChangedSetting(key, value, manualReset, languageFilters);
this.updateChangedSetting(key, value, manualReset, languageFilter);
}

this.pendingSettingUpdate = { key, value, languageFilters };
this.pendingSettingUpdate = { key, value, languageFilter };
if (SettingsEditor2.shouldSettingUpdateFast(type)) {
this.settingFastUpdateDelayer.trigger(() => this.updateChangedSetting(key, value, manualReset, languageFilters));
this.settingFastUpdateDelayer.trigger(() => this.updateChangedSetting(key, value, manualReset, languageFilter));
} else {
this.settingSlowUpdateDelayer.trigger(() => this.updateChangedSetting(key, value, manualReset, languageFilters));
this.settingSlowUpdateDelayer.trigger(() => this.updateChangedSetting(key, value, manualReset, languageFilter));
}
}

Expand Down Expand Up @@ -955,17 +954,17 @@ export class SettingsEditor2 extends EditorPane {
return ancestors.reverse();
}

private updateChangedSetting(key: string, value: any, manualReset: boolean, languageFilters: string[] | null): Promise<void> {
private updateChangedSetting(key: string, value: any, manualReset: boolean, languageFilter: string | undefined): Promise<void> {
// ConfigurationService displays the error if this fails.
// Force a render afterwards because onDidConfigurationUpdate doesn't fire if the update doesn't result in an effective setting value change
const settingsTarget = this.settingsTargetsWidget.settingsTarget;
const resource = URI.isUri(settingsTarget) ? settingsTarget : undefined;
const configurationTarget = <ConfigurationTarget>(resource ? ConfigurationTarget.WORKSPACE_FOLDER : settingsTarget);
const overrides: IConfigurationUpdateOverrides = { resource, overrideIdentifiers: languageFilters };
const overrides: IConfigurationUpdateOverrides = { resource, overrideIdentifiers: languageFilter ? [languageFilter] : undefined };

const configurationTargetIsWorkspace = configurationTarget === ConfigurationTarget.WORKSPACE || configurationTarget === ConfigurationTarget.WORKSPACE_FOLDER;

const userPassedInManualReset = configurationTargetIsWorkspace || !!languageFilters?.length;
const userPassedInManualReset = configurationTargetIsWorkspace || !!languageFilter;
const isManualReset = userPassedInManualReset ? manualReset : value === undefined;

// If the user is changing the value back to the default, and we're not targeting a workspace scope, do a 'reset' instead
Expand Down Expand Up @@ -1106,7 +1105,7 @@ export class SettingsEditor2 extends EditorPane {
resolvedSettingsRoot.children!.push(await createTocTreeForExtensionSettings(this.extensionService, dividedGroups.extension || []));

if (!this.workspaceTrustManagementService.isWorkspaceTrusted() && (this.viewState.settingsTarget instanceof URI || this.viewState.settingsTarget === ConfigurationTarget.WORKSPACE)) {
const configuredUntrustedWorkspaceSettings = resolveConfiguredUntrustedSettings(groups, this.viewState.settingsTarget, this.viewState.languageFilters, this.configurationService);
const configuredUntrustedWorkspaceSettings = resolveConfiguredUntrustedSettings(groups, this.viewState.settingsTarget, this.viewState.languageFilter, this.configurationService);
if (configuredUntrustedWorkspaceSettings.length) {
resolvedSettingsRoot.children!.unshift({
id: 'workspaceTrust',
Expand Down Expand Up @@ -1274,22 +1273,22 @@ export class SettingsEditor2 extends EditorPane {
this.viewState.extensionFilters = new Set<string>();
this.viewState.featureFilters = new Set<string>();
this.viewState.idFilters = new Set<string>();
this.viewState.languageFilters = new Set<string>();
this.viewState.languageFilter = undefined;
if (query) {
const parsedQuery = parseQuery(query);
query = parsedQuery.query;
parsedQuery.tags.forEach(tag => this.viewState.tagFilters!.add(tag));
parsedQuery.extensionFilters.forEach(extensionId => this.viewState.extensionFilters!.add(extensionId));
parsedQuery.featureFilters!.forEach(feature => this.viewState.featureFilters!.add(feature));
parsedQuery.idFilters!.forEach(id => this.viewState.idFilters!.add(id));
parsedQuery.languageFilters!.forEach(id => this.viewState.languageFilters!.add(id));
this.viewState.languageFilter = parsedQuery.languageFilter;
}

if (query && query !== '@') {
query = this.parseSettingFromJSON(query) || query;
return this.triggerFilterPreferences(query);
} else {
if (this.viewState.tagFilters.size || this.viewState.extensionFilters.size || this.viewState.featureFilters.size || this.viewState.idFilters.size || this.viewState.languageFilters.size) {
if (this.viewState.tagFilters.size || this.viewState.extensionFilters.size || this.viewState.featureFilters.size || this.viewState.idFilters.size || this.viewState.languageFilter) {
this.searchResultModel = this.createFilterModel();
} else {
this.searchResultModel = null;
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/preferences/browser/settingsTree.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,9 +391,9 @@ export function resolveSettingsTree(tocData: ITOCEntry<string>, coreSettingsGrou
};
}

export function resolveConfiguredUntrustedSettings(groups: ISettingsGroup[], target: SettingsTarget, languageFilters: Set<string> | undefined, configurationService: IWorkbenchConfigurationService): ISetting[] {
export function resolveConfiguredUntrustedSettings(groups: ISettingsGroup[], target: SettingsTarget, languageFilter: string | undefined, configurationService: IWorkbenchConfigurationService): ISetting[] {
const allSettings = getFlatSettings(groups);
return [...allSettings].filter(setting => setting.restricted && inspectSetting(setting.key, target, languageFilters, configurationService).isConfigured);
return [...allSettings].filter(setting => setting.restricted && inspectSetting(setting.key, target, languageFilter, configurationService).isConfigured);
}

function compareNullableIntegers(a?: number, b?: number) {
Expand Down
34 changes: 16 additions & 18 deletions src/vs/workbench/contrib/preferences/browser/settingsTreeModels.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export interface ISettingsEditorViewState {
extensionFilters?: Set<string>;
featureFilters?: Set<string>;
idFilters?: Set<string>;
languageFilters?: Set<string>;
languageFilter?: string;
filterToCategory?: SettingsTreeGroupElement;
}

Expand Down Expand Up @@ -179,7 +179,7 @@ export class SettingsTreeSettingElement extends SettingsTreeElement {
}

update(inspectResult: IInspectResult, isWorkspaceTrusted: boolean): void {
const { isConfigured, inspected, targetSelector, inspectedLanguageOverrides, languageSelectors } = inspectResult;
const { isConfigured, inspected, targetSelector, inspectedLanguageOverrides, languageSelector: languageSelectors } = inspectResult;

switch (targetSelector) {
case 'workspaceFolderValue':
Expand Down Expand Up @@ -211,6 +211,7 @@ export class SettingsTreeSettingElement extends SettingsTreeElement {
}
}

// TODO: fix this section.
if (isConfigured && languageSelectors) {
for (const languageSelector of languageSelectors) {
// We already targeted the scope to use above.
Expand Down Expand Up @@ -374,8 +375,8 @@ export class SettingsTreeSettingElement extends SettingsTreeElement {
return idFilters.has(this.setting.key);
}

matchesAllLanguages(languageFilters?: Set<string>): boolean {
if (!languageFilters || !languageFilters.size
matchesAllLanguages(languageFilter?: string): boolean {
if (!languageFilter
|| this.setting.scope === ConfigurationScope.LANGUAGE_OVERRIDABLE) {
return true;
}
Expand Down Expand Up @@ -460,7 +461,7 @@ export class SettingsTreeModel {

private updateSettings(settings: SettingsTreeSettingElement[]): void {
settings.forEach(element => {
const inspectResult = inspectSetting(element.setting.key, this._viewState.settingsTarget, this._viewState.languageFilters, this._configurationService);
const inspectResult = inspectSetting(element.setting.key, this._viewState.settingsTarget, this._viewState.languageFilter, this._configurationService);
element.update(inspectResult, this._isWorkspaceTrusted);
});
}
Expand Down Expand Up @@ -496,7 +497,7 @@ export class SettingsTreeModel {
}

private createSettingsTreeSettingElement(setting: ISetting, parent: SettingsTreeGroupElement): SettingsTreeSettingElement {
const inspectResult = inspectSetting(setting.key, this._viewState.settingsTarget, this._viewState.languageFilters, this._configurationService);
const inspectResult = inspectSetting(setting.key, this._viewState.settingsTarget, this._viewState.languageFilter, this._configurationService);
const element = new SettingsTreeSettingElement(setting, parent, inspectResult, this._isWorkspaceTrusted);

const nameElements = this._treeElementsBySettingName.get(setting.key) || [];
Expand All @@ -511,10 +512,10 @@ interface IInspectResult {
inspected: IConfigurationValue<unknown>;
targetSelector: 'userLocalValue' | 'userRemoteValue' | 'workspaceValue' | 'workspaceFolderValue';
inspectedLanguageOverrides: Map<string, IConfigurationValue<unknown>>;
languageSelectors: Set<string> | undefined;
languageSelector: string | undefined;
}

export function inspectSetting(key: string, target: SettingsTarget, languageFilters: Set<string> | undefined, configurationService: IWorkbenchConfigurationService): IInspectResult {
export function inspectSetting(key: string, target: SettingsTarget, languageFilter: string | undefined, configurationService: IWorkbenchConfigurationService): IInspectResult {
const inspectOverrides = URI.isUri(target) ? { resource: target } : undefined;
const inspected = configurationService.inspect(key, inspectOverrides);
const targetSelector = target === ConfigurationTarget.USER_LOCAL ? 'userLocalValue' :
Expand Down Expand Up @@ -545,18 +546,15 @@ export function inspectSetting(key: string, target: SettingsTarget, languageFilt
// If we're viewing a setting under a language filter, pretend it's
// not configured again.
// For all language filters, see if there's an override for that filter.
if (languageFilters) {
if (languageFilter) {
isConfigured = false;
for (const language of languageFilters) {
if (inspectedLanguageOverrides.has(language) && typeof inspectedLanguageOverrides.get(language)![targetSelector] !== 'undefined') {
isConfigured = true;
break;
}
if (inspectedLanguageOverrides.has(languageFilter) && typeof inspectedLanguageOverrides.get(languageFilter)![targetSelector] !== 'undefined') {
isConfigured = true;
}
}
}

return { isConfigured, inspected, targetSelector, inspectedLanguageOverrides, languageSelectors: languageFilters };
return { isConfigured, inspected, targetSelector, inspectedLanguageOverrides, languageSelector: languageFilter };
}

function sanitizeId(id: string): string {
Expand Down Expand Up @@ -798,7 +796,7 @@ export class SearchResultModel extends SettingsTreeModel {
const isRemote = !!this.environmentService.remoteAuthority;

this.root.children = this.root.children
.filter(child => child instanceof SettingsTreeSettingElement && child.matchesAllTags(this._viewState.tagFilters) && child.matchesScope(this._viewState.settingsTarget, isRemote) && child.matchesAnyExtension(this._viewState.extensionFilters) && child.matchesAnyId(this._viewState.idFilters) && child.matchesAnyFeature(this._viewState.featureFilters) && child.matchesAllLanguages(this._viewState.languageFilters));
.filter(child => child instanceof SettingsTreeSettingElement && child.matchesAllTags(this._viewState.tagFilters) && child.matchesScope(this._viewState.settingsTarget, isRemote) && child.matchesAnyExtension(this._viewState.extensionFilters) && child.matchesAnyId(this._viewState.idFilters) && child.matchesAnyFeature(this._viewState.featureFilters) && child.matchesAllLanguages(this._viewState.languageFilter));

if (this.newExtensionSearchResults && this.newExtensionSearchResults.filterMatches.length) {
const resultExtensionIds = this.newExtensionSearchResults.filterMatches
Expand Down Expand Up @@ -830,7 +828,7 @@ export interface IParsedQuery {
extensionFilters: string[];
idFilters: string[];
featureFilters: string[];
languageFilters: string[];
languageFilter: string | undefined;
}

const tagRegex = /(^|\s)@tag:("([^"]*)"|[^"]\S*)/g;
Expand Down Expand Up @@ -885,7 +883,7 @@ export function parseQuery(query: string): IParsedQuery {
extensionFilters: extensions,
featureFilters: features,
idFilters: ids,
languageFilters: langs.length ? [langs[0]] : [],
languageFilter: langs.length ? langs[0] : undefined,
query,
};
}
34 changes: 17 additions & 17 deletions src/vs/workbench/contrib/preferences/test/browser/settingsTreeModels.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ suite('SettingsTree', () => {
query: '',
featureFilters: [],
idFilters: [],
languageFilters: []
languageFilter: undefined
});

testParseQuery(
Expand All @@ -161,7 +161,7 @@ suite('SettingsTree', () => {
query: '',
featureFilters: [],
idFilters: [],
languageFilters: []
languageFilter: undefined
});

testParseQuery(
Expand All @@ -172,7 +172,7 @@ suite('SettingsTree', () => {
query: '',
featureFilters: [],
idFilters: [],
languageFilters: []
languageFilter: undefined
});

testParseQuery(
Expand All @@ -183,7 +183,7 @@ suite('SettingsTree', () => {
query: 'foo',
featureFilters: [],
idFilters: [],
languageFilters: []
languageFilter: undefined
});

testParseQuery(
Expand All @@ -194,7 +194,7 @@ suite('SettingsTree', () => {
query: '',
featureFilters: [],
idFilters: [],
languageFilters: []
languageFilter: undefined
});

testParseQuery(
Expand All @@ -205,7 +205,7 @@ suite('SettingsTree', () => {
query: 'my query',
featureFilters: [],
idFilters: [],
languageFilters: []
languageFilter: undefined
});

testParseQuery(
Expand All @@ -216,7 +216,7 @@ suite('SettingsTree', () => {
query: 'test query',
featureFilters: [],
idFilters: [],
languageFilters: []
languageFilter: undefined
});

testParseQuery(
Expand All @@ -227,7 +227,7 @@ suite('SettingsTree', () => {
query: 'test',
featureFilters: [],
idFilters: [],
languageFilters: []
languageFilter: undefined
});

testParseQuery(
Expand All @@ -238,7 +238,7 @@ suite('SettingsTree', () => {
query: 'query has @ for some reason',
featureFilters: [],
idFilters: [],
languageFilters: []
languageFilter: undefined
});

testParseQuery(
Expand All @@ -249,7 +249,7 @@ suite('SettingsTree', () => {
query: '',
featureFilters: [],
idFilters: [],
languageFilters: []
languageFilter: undefined
});

testParseQuery(
Expand All @@ -260,7 +260,7 @@ suite('SettingsTree', () => {
query: '',
featureFilters: [],
idFilters: [],
languageFilters: []
languageFilter: undefined
});
testParseQuery(
'@feature:scm',
Expand All @@ -270,7 +270,7 @@ suite('SettingsTree', () => {
featureFilters: ['scm'],
query: '',
idFilters: [],
languageFilters: []
languageFilter: undefined
});

testParseQuery(
Expand All @@ -281,7 +281,7 @@ suite('SettingsTree', () => {
featureFilters: ['scm', 'terminal'],
query: '',
idFilters: [],
languageFilters: []
languageFilter: undefined
});
testParseQuery(
'@id:files.autoSave',
Expand All @@ -291,7 +291,7 @@ suite('SettingsTree', () => {
featureFilters: [],
query: '',
idFilters: ['files.autoSave'],
languageFilters: []
languageFilter: undefined
});

testParseQuery(
Expand All @@ -302,7 +302,7 @@ suite('SettingsTree', () => {
featureFilters: [],
query: '',
idFilters: ['files.autoSave', 'terminal.integrated.commandsToSkipShell'],
languageFilters: []
languageFilter: undefined
});

testParseQuery(
Expand All @@ -313,7 +313,7 @@ suite('SettingsTree', () => {
featureFilters: [],
query: '',
idFilters: [],
languageFilters: ['cpp']
languageFilter: 'cpp'
});

testParseQuery(
Expand All @@ -324,7 +324,7 @@ suite('SettingsTree', () => {
featureFilters: [],
query: '',
idFilters: [],
languageFilters: ['cpp']
languageFilter: 'cpp'
});
});
});

0 comments on commit 7d6f6f7

Please sign in to comment.