Fix #55616 - render 'filters' the same as search. And clean up and fix lots of...

Fix #55616 - render 'filters' the same as search. And clean up and fix lots of filter + search + TOC filter issues

src/vs/workbench/electron-browser/main.contribution.ts

@@ -635,7 +635,11 @@ configurationRegistry.registerConfiguration({
 		},
 		'workbench.settings.settingsSearchTocBehavior': {
 			'type': 'string',
-			'enum': ['hide', 'filter', 'show'],
+			'enum': ['hide', 'filter'],
+			'enumDescriptions': [
+				nls.localize('settingsSearchTocBehavior.hide', "Hide the Table of Contents while searching."),
+				nls.localize('settingsSearchTocBehavior.filter', "Filter the Table of Contents to just categories that have matching settings. Clicking a category will filter the results to that category."),
+			],
 			'description': nls.localize('settingsSearchTocBehavior', "Controls the behavior of the settings editor Table of Contents while searching."),
 			'default': 'filter',
 			'scope': ConfigurationScope.WINDOW

src/vs/workbench/parts/preferences/browser/settingsEditor2.ts

@@ -302,12 +302,6 @@ export class SettingsEditor2 extends BaseEditor {
 						this.settingsTree.reveal(lastElement, 0.9);
 						return true;
 					}
-				} else {
-					const controls = this.settingsTree.getHTMLElement().querySelectorAll(SettingsRenderer.CONTROL_SELECTOR);
-					const lastControl = controls && controls[controls.length];
-					if (lastControl) {
-						(<HTMLElement>lastControl).focus();
-					}
 				}
 
 				return false;
@@ -336,7 +330,7 @@ export class SettingsEditor2 extends BaseEditor {
 	}
 
 	private createTOC(parent: HTMLElement): void {
-		this.tocTreeModel = new TOCTreeModel();
+		this.tocTreeModel = new TOCTreeModel(this.viewState);
 		this.tocTreeContainer = DOM.append(parent, $('.settings-toc-container'));
 
 		const tocRenderer = this.instantiationService.createInstance(TOCRenderer);
@@ -673,22 +667,54 @@ export class SettingsEditor2 extends BaseEditor {
 				this.searchInProgress = null;
 			});
 		} else {
+			if (this.viewState.tagFilters && this.viewState.tagFilters.size) {
+				this.searchResultModel = this.createFilterModel();
+			} else {
+				this.searchResultModel = null;
+			}
+
 			this.localSearchDelayer.cancel();
 			this.remoteSearchThrottle.cancel();
 			if (this.searchInProgress && this.searchInProgress.cancel) {
 				this.searchInProgress.cancel();
 			}
 
-			this.searchResultModel = null;
-			this.tocTreeModel.currentSearchModel = null;
 			this.viewState.filterToCategory = null;
+			this.tocTreeModel.currentSearchModel = this.searchResultModel;
 			this.tocTree.refresh();
 			this.toggleSearchMode();
 			collapseAll(this.tocTree);
-			return this.settingsTree.setInput(this.settingsTreeModel.root);
+
+			if (this.searchResultModel) {
+				return this.settingsTree.setInput(this.searchResultModel);
+			} else {
+				return this.settingsTree.setInput(this.settingsTreeModel.root);
+			}
 		}
 	}
 
+	/**
+	 * Return a fake SearchResultModel which can hold a flat list of all settings, to be filtered (@modified etc)
+	 */
+	private createFilterModel(): SearchResultModel {
+		const filterModel = this.instantiationService.createInstance(SearchResultModel, this.viewState);
+
+		const fullResult: ISearchResult = {
+			filterMatches: []
+		};
+		for (let g of this.defaultSettingsEditorModel.settingsGroups.slice(1)) {
+			for (let sect of g.sections) {
+				for (let setting of sect.settings) {
+					fullResult.filterMatches.push({ setting, matches: [], score: 0 });
+				}
+			}
+		}
+
+		filterModel.setResult(0, fullResult);
+
+		return filterModel;
+	}
+
 	private reportFilteringUsed(query: string, results: ISearchResult[]): void {
 		const nlpResult = results[SearchResultIdx.Remote];
 		const nlpMetadata = nlpResult && nlpResult.metadata;

src/vs/workbench/parts/preferences/browser/settingsTree.ts

@@ -52,6 +52,7 @@ export abstract class SettingsTreeElement {
 
 export class SettingsTreeGroupElement extends SettingsTreeElement {
 	children: (SettingsTreeGroupElement | SettingsTreeSettingElement)[];
+	count?: number;
 	label: string;
 	level: number;
 	isFirstGroup: boolean;
@@ -91,6 +92,22 @@ export class SettingsTreeSettingElement extends SettingsTreeElement {
 	overriddenScopeList: string[];
 	description: string;
 	valueType: 'enum' | 'string' | 'integer' | 'number' | 'boolean' | 'exclude' | 'complex';
+
+	matchesAllTags(tagFilters?: Set<string>): boolean {
+		if (!tagFilters || !tagFilters.size) {
+			return true;
+		}
+
+		if (this.tags) {
+			let hasFilteredTag = true;
+			tagFilters.forEach(tag => {
+				hasFilteredTag = hasFilteredTag && this.tags.has(tag);
+			});
+			return hasFilteredTag;
+		} else {
+			return false;
+		}
+	}
 }
 
 export interface ITOCEntry {
@@ -174,7 +191,7 @@ function sanitizeId(id: string): string {
 	return id.replace(/[\.\/]/, '_');
 }
 
-function createSettingsTreeSettingElement(setting: ISetting, parent: any, settingsTarget: SettingsTarget, configurationService: IConfigurationService): SettingsTreeSettingElement {
+function createSettingsTreeSettingElement(setting: ISetting, parent: SearchResultModel | SettingsTreeGroupElement, settingsTarget: SettingsTarget, configurationService: IConfigurationService): SettingsTreeSettingElement {
 	const element = new SettingsTreeSettingElement();
 	element.id = sanitizeId(parent.id + '_' + setting.key);
 	element.parent = parent;
@@ -1287,25 +1304,22 @@ export class SettingsTreeFilter implements IFilter {
 	) { }
 
 	isVisible(tree: ITree, element: SettingsTreeElement): boolean {
+		// Filter during search
 		if (this.viewState.filterToCategory && element instanceof SettingsTreeSettingElement) {
 			if (!this.settingContainedInGroup(element.setting, this.viewState.filterToCategory)) {
 				return false;
 			}
 		}
 
-		if (element instanceof SettingsTreeSettingElement && this.viewState.tagFilters && this.viewState.tagFilters.size) {
-			if (element.tags) {
-				let hasFilteredTag = true;
-				this.viewState.tagFilters.forEach(tag => {
-					hasFilteredTag = hasFilteredTag && element.tags.has(tag);
-				});
-				return hasFilteredTag;
-			} else {
-				return false;
-			}
+		if (element instanceof SettingsTreeSettingElement && this.viewState.tagFilters) {
+			return element.matchesAllTags(this.viewState.tagFilters);
 		}
 
-		if (element instanceof SettingsTreeGroupElement && this.viewState.tagFilters && this.viewState.tagFilters.size) {
+		if (element instanceof SettingsTreeGroupElement) {
+			if (typeof element.count === 'number') {
+				return element.count > 0;
+			}
+
 			return element.children.some(child => this.isVisible(tree, child));
 		}
 
@@ -1419,15 +1433,15 @@ export class SearchResultModel {
 		return this.rawSearchResults;
 	}
 
-	setResult(type: SearchResultIdx, result: ISearchResult): void {
+	setResult(order: SearchResultIdx, result: ISearchResult): void {
 		this.cachedUniqueSearchResults = null;
 		this.rawSearchResults = this.rawSearchResults || [];
 		if (!result) {
-			delete this.rawSearchResults[type];
+			delete this.rawSearchResults[order];
 			return;
 		}
 
-		this.rawSearchResults[type] = result;
+		this.rawSearchResults[order] = result;
 		this.updateChildren();
 	}
 

src/vs/workbench/parts/preferences/browser/tocTree.ts

@@ -25,6 +25,10 @@ export class TOCTreeModel {
 	private _currentSearchModel: SearchResultModel;
 	private _settingsTreeRoot: SettingsTreeGroupElement;
 
+	constructor(private viewState: ISettingsEditorViewState) {
+
+	}
+
 	public set settingsTreeRoot(value: SettingsTreeGroupElement) {
 		this._settingsTreeRoot = value;
 		this.update();
@@ -44,7 +48,7 @@ export class TOCTreeModel {
 	}
 
 	private updateGroupCount(group: SettingsTreeGroupElement): void {
-		(<any>group).count = this._currentSearchModel ?
+		group.count = this._currentSearchModel ?
 			this.getSearchResultChildrenCount(group) :
 			undefined;
 
@@ -64,7 +68,7 @@ export class TOCTreeModel {
 	private groupContainsSetting(group: SettingsTreeGroupElement, setting: ISetting): boolean {
 		return group.children.some(child => {
 			if (child instanceof SettingsTreeSettingElement) {
-				return child.setting.key === setting.key;
+				return child.setting.key === setting.key && child.matchesAllTags(this.viewState.tagFilters);
 			} else if (child instanceof SettingsTreeGroupElement) {
 				return this.groupContainsSetting(child, setting);
 			} else {
@@ -77,11 +81,6 @@ export class TOCTreeModel {
 export type TOCTreeElement = SettingsTreeGroupElement | TOCTreeModel;
 
 export class TOCDataSource implements IDataSource {
-	constructor(
-		@IConfigurationService private configService: IConfigurationService
-	) {
-	}
-
 	getId(tree: ITree, element: SettingsTreeGroupElement): string {
 		return element.id;
 	}
@@ -96,14 +95,6 @@ export class TOCDataSource implements IDataSource {
 	}
 
 	private _getChildren(element: TOCTreeElement): SettingsTreeElement[] {
-		// TODO@roblou hack. Clean up or remove this option
-		if (this.configService.getValue('workbench.settings.settingsSearchTocBehavior') === 'filter') {
-			const children = element.children as SettingsTreeElement[]; // TS????
-			return children.filter(group => {
-				return (<any>group).count !== 0;
-			});
-		}
-
 		return element.children;
 	}
 
@@ -136,7 +127,7 @@ export class TOCRenderer implements IRenderer {
 	}
 
 	renderElement(tree: ITree, element: SettingsTreeGroupElement, templateId: string, template: ITOCEntryTemplate): void {
-		const count = (<any>element).count;
+		const count = element.count;
 		const label = element.label;
 
 		DOM.toggleClass(template.labelElement, 'no-results', count === 0);