Sort settings by ToC when ToC shown

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

@@ -194,6 +194,7 @@ export class SettingsEditor2 extends EditorPane {
 	private _searchResultModel: SearchResultModel | null = null;
 	private searchResultLabel: string | null = null;
 	private lastSyncedLabel: string | null = null;
+	private settingsIndex: Map<string, number> | null = null;
 
 	private tocRowFocused: IContextKey<boolean>;
 	private settingRowFocused: IContextKey<boolean>;
@@ -1235,9 +1236,31 @@ export class SettingsEditor2 extends EditorPane {
 		return undefined;
 	}
 
+	private createSettingsIndex(resolvedSettingsRoot: ITOCEntry<ISetting>): Map<string, number> {
+		const index = new Map<string, number>();
+		function indexSettings(resolvedSettingsRoot: ITOCEntry<ISetting>, counter = 0): number {
+			if (resolvedSettingsRoot.settings) {
+				for (const setting of resolvedSettingsRoot.settings) {
+					if (!index.has(setting.key)) {
+						index.set(setting.key, counter++);
+					}
+				}
+			}
+			if (resolvedSettingsRoot.children) {
+				for (const child of resolvedSettingsRoot.children) {
+					counter = indexSettings(child, counter);
+				}
+			}
+			return counter;
+		}
+		indexSettings(resolvedSettingsRoot);
+		return index;
+	}
+
 	private refreshModels(resolvedSettingsRoot: ITOCEntry<ISetting>) {
 		this.settingsTreeModel.update(resolvedSettingsRoot);
-		this.tocTreeModel.settingsTreeRoot = this.settingsTreeModel.root as SettingsTreeGroupElement;
+		this.tocTreeModel.settingsTreeRoot = this.settingsTreeModel.root;
+		this.settingsIndex = this.createSettingsIndex(resolvedSettingsRoot);
 	}
 
 	private async onConfigUpdate(keys?: ReadonlySet<string>, forceRefresh = false, schemaChange = false): Promise<void> {
@@ -1553,7 +1576,7 @@ export class SettingsEditor2 extends EditorPane {
 	 * 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, this.workspaceTrustManagementService.isWorkspaceTrusted());
+		const filterModel = this.instantiationService.createInstance(SearchResultModel, this.viewState, this.settingsIndex, this.workspaceTrustManagementService.isWorkspaceTrusted());
 
 		const fullResult: ISearchResult = {
 			filterMatches: []
@@ -1657,7 +1680,7 @@ export class SettingsEditor2 extends EditorPane {
 			return null;
 		}
 		if (!this.searchResultModel) {
-			this.searchResultModel = this.instantiationService.createInstance(SearchResultModel, this.viewState, this.workspaceTrustManagementService.isWorkspaceTrusted());
+			this.searchResultModel = this.instantiationService.createInstance(SearchResultModel, this.viewState, this.settingsIndex, this.workspaceTrustManagementService.isWorkspaceTrusted());
 			// Must be called before this.renderTree()
 			// to make sure the search results count is set.
 			this.searchResultModel.setResult(type, result);

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

@@ -62,7 +62,7 @@ import { ISettingOverrideClickEvent, SettingsTreeIndicatorsLabel, getIndicatorsL
 import { ITOCEntry } from 'vs/workbench/contrib/preferences/browser/settingsLayout';
 import { ISettingsEditorViewState, SettingsTreeElement, SettingsTreeGroupChild, SettingsTreeGroupElement, SettingsTreeNewExtensionsElement, SettingsTreeSettingElement, inspectSetting, settingKeyToDisplayFormat } from 'vs/workbench/contrib/preferences/browser/settingsTreeModels';
 import { ExcludeSettingWidget, IListDataItem, IObjectDataItem, IObjectEnumOption, IObjectKeySuggester, IObjectValueSuggester, ISettingListChangeEvent, IncludeSettingWidget, ListSettingWidget, ObjectSettingCheckboxWidget, ObjectSettingDropdownWidget, ObjectValue } from 'vs/workbench/contrib/preferences/browser/settingsWidgets';
-import { LANGUAGE_SETTING_TAG, SETTINGS_EDITOR_COMMAND_SHOW_CONTEXT_MENU } from 'vs/workbench/contrib/preferences/common/preferences';
+import { LANGUAGE_SETTING_TAG, SETTINGS_EDITOR_COMMAND_SHOW_CONTEXT_MENU, compareTwoNullableNumbers } from 'vs/workbench/contrib/preferences/common/preferences';
 import { settingsNumberInputBackground, settingsNumberInputBorder, settingsNumberInputForeground, settingsSelectBackground, settingsSelectBorder, settingsSelectForeground, settingsSelectListBorder, settingsTextInputBackground, settingsTextInputBorder, settingsTextInputForeground } from 'vs/workbench/contrib/preferences/common/settingsEditorColorRegistry';
 import { APPLY_ALL_PROFILES_SETTING, IWorkbenchConfigurationService } from 'vs/workbench/services/configuration/common/configuration';
 import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService';
@@ -409,12 +409,6 @@ export function resolveConfiguredUntrustedSettings(groups: ISettingsGroup[], tar
 	return [...allSettings].filter(setting => setting.restricted && inspectSetting(setting.key, target, languageFilter, configurationService).isConfigured);
 }
 
-function compareNullableIntegers(a?: number, b?: number) {
-	const firstElem = a ?? Number.MAX_SAFE_INTEGER;
-	const secondElem = b ?? Number.MAX_SAFE_INTEGER;
-	return firstElem - secondElem;
-}
-
 export async function createTocTreeForExtensionSettings(extensionService: IExtensionService, groups: ISettingsGroup[]): Promise<ITOCEntry<ISetting>> {
 	const extGroupTree = new Map<string, ITOCEntry<ISetting>>();
 	const addEntryToTree = (extensionId: string, extensionName: string, childEntry: ITOCEntry<ISetting>) => {
@@ -452,9 +446,10 @@ export async function createTocTreeForExtensionSettings(extensionService: IExten
 		const extGroups: ITOCEntry<ISetting>[] = [];
 		for (const extensionRootEntry of extGroupTree.values()) {
 			for (const child of extensionRootEntry.children!) {
-				// Sort the individual settings of the child.
+				// Sort the individual settings of the child by order.
+				// Leave the undefined order settings untouched.
 				child.settings?.sort((a, b) => {
-					return compareNullableIntegers(a.order, b.order);
+					return compareTwoNullableNumbers(a.order, b.order);
 				});
 			}
 
@@ -468,8 +463,9 @@ export async function createTocTreeForExtensionSettings(extensionService: IExten
 				});
 			} else {
 				// Sort the categories.
+				// Leave the undefined order categories untouched.
 				extensionRootEntry.children!.sort((a, b) => {
-					return compareNullableIntegers(a.order, b.order);
+					return compareTwoNullableNumbers(a.order, b.order);
 				});
 
 				// If there is a category that matches the setting name,

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

@@ -10,7 +10,7 @@ import { URI } from 'vs/base/common/uri';
 import { ConfigurationTarget, IConfigurationValue } from 'vs/platform/configuration/common/configuration';
 import { SettingsTarget } from 'vs/workbench/contrib/preferences/browser/preferencesWidgets';
 import { ITOCEntry, knownAcronyms, knownTermMappings, tocData } from 'vs/workbench/contrib/preferences/browser/settingsLayout';
-import { ENABLE_EXTENSION_TOGGLE_SETTINGS, ENABLE_LANGUAGE_FILTER, MODIFIED_SETTING_TAG, POLICY_SETTING_TAG, REQUIRE_TRUSTED_WORKSPACE_SETTING_TAG } from 'vs/workbench/contrib/preferences/common/preferences';
+import { ENABLE_EXTENSION_TOGGLE_SETTINGS, ENABLE_LANGUAGE_FILTER, MODIFIED_SETTING_TAG, POLICY_SETTING_TAG, REQUIRE_TRUSTED_WORKSPACE_SETTING_TAG, compareTwoNullableNumbers } from 'vs/workbench/contrib/preferences/common/preferences';
 import { IExtensionSetting, ISearchResult, ISetting, ISettingMatch, SettingMatchType, SettingValueType } from 'vs/workbench/services/preferences/common/preferences';
 import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService';
 import { FOLDER_SCOPES, WORKSPACE_SCOPES, REMOTE_MACHINE_SCOPES, LOCAL_MACHINE_SCOPES, IWorkbenchConfigurationService, APPLICATION_SCOPES } from 'vs/workbench/services/configuration/common/configuration';
@@ -858,35 +858,44 @@ export class SearchResultModel extends SettingsTreeModel {
 	private cachedUniqueSearchResults: ISearchResult | null = null;
 	private newExtensionSearchResults: ISearchResult | null = null;
 	private searchResultCount: number | null = null;
+	private settingsOrderIndex: Map<string, number> | null;
 
 	readonly id = 'searchResultModel';
 
 	constructor(
 		viewState: ISettingsEditorViewState,
+		settingsIndex: Map<string, number> | null,
 		isWorkspaceTrusted: boolean,
-		@IWorkbenchConfigurationService configurationService: IWorkbenchConfigurationService,
-		@IWorkbenchEnvironmentService private environmentService: IWorkbenchEnvironmentService,
+		@IWorkbenchConfigurationService private readonly configurationService: IWorkbenchConfigurationService,
+		@IWorkbenchEnvironmentService private readonly environmentService: IWorkbenchEnvironmentService,
 		@ILanguageService languageService: ILanguageService,
 		@IUserDataProfileService userDataProfileService: IUserDataProfileService,
 		@IProductService productService: IProductService
 	) {
 		super(viewState, isWorkspaceTrusted, configurationService, languageService, userDataProfileService, productService);
+		this.settingsOrderIndex = settingsIndex;
 		this.update({ id: 'searchResultModel', label: '' });
 	}
 
-	private compareTwoNullableNumbers(a: number | undefined, b: number | undefined): number {
-		const aOrMax = a ?? Number.MAX_SAFE_INTEGER;
-		const bOrMax = b ?? Number.MAX_SAFE_INTEGER;
-		if (aOrMax < bOrMax) {
-			return -1;
-		} else if (aOrMax > bOrMax) {
-			return 1;
-		} else {
-			return 0;
+	private sortResults(filterMatches: ISettingMatch[]): ISettingMatch[] {
+		if (this.settingsOrderIndex) {
+			for (const match of filterMatches) {
+				match.setting.internalOrder = this.settingsOrderIndex.get(match.setting.key);
+			}
 		}
-	}
 
-	private sortResults(filterMatches: ISettingMatch[]): ISettingMatch[] {
+		const tocHiddenDuringSearch = this.configurationService.getValue('workbench.settings.settingsSearchTocBehavior') === 'hide';
+		if (!tocHiddenDuringSearch) {
+			// Sort the settings according to internal order if indexed.
+			if (this.settingsOrderIndex) {
+				filterMatches.sort((a, b) => compareTwoNullableNumbers(a.setting.internalOrder, b.setting.internalOrder));
+			}
+			return filterMatches;
+		}
+
+		// The table of contents is hidden during the search.
+		// The settings could appear in a more haphazard order.
+		// Sort the settings according to their score.
 		filterMatches.sort((a, b) => {
 			if (a.matchType !== b.matchType) {
 				// Sort by match type if the match types are not the same.
@@ -897,24 +906,9 @@ export class SearchResultModel extends SettingsTreeModel {
 				// Sort by score.
 				return b.score - a.score;
 			} else {
-				// The match types are the same.
-				if (a.setting.extensionInfo && b.setting.extensionInfo
-					&& a.setting.extensionInfo.id === b.setting.extensionInfo.id) {
-					// These settings belong to the same extension.
-					if (a.setting.categoryLabel !== b.setting.categoryLabel
-						&& (a.setting.categoryOrder !== undefined || b.setting.categoryOrder !== undefined)
-						&& a.setting.categoryOrder !== b.setting.categoryOrder) {
-						// These two settings don't belong to the same category and have different category orders.
-						return this.compareTwoNullableNumbers(a.setting.categoryOrder, b.setting.categoryOrder);
-					} else if (a.setting.categoryLabel === b.setting.categoryLabel
-						&& (a.setting.order !== undefined || b.setting.order !== undefined)
-						&& a.setting.order !== b.setting.order) {
-						// These two settings belong to the same category, but have different orders.
-						return this.compareTwoNullableNumbers(a.setting.order, b.setting.order);
-					}
-				}
-				// In the worst case, go back to lexicographical order.
-				return b.score - a.score;
+				// The match types are the same but are not RemoteMatch.
+				// Sort by their order in the table of contents.
+				return compareTwoNullableNumbers(a.setting.internalOrder, b.setting.internalOrder);
 			}
 		});
 		return filterMatches;
@@ -946,7 +940,6 @@ export class SearchResultModel extends SettingsTreeModel {
 			this.newExtensionSearchResults = this.rawSearchResults[SearchResultIdx.NewExtensions];
 		}
 
-		// Combine and sort results.
 		combinedFilterMatches = this.sortResults(combinedFilterMatches);
 
 		this.cachedUniqueSearchResults = {

src/vs/workbench/contrib/preferences/common/preferences.ts

@@ -128,3 +128,18 @@ export async function getExperimentalExtensionToggleData(workbenchAssignmentServ
 	}
 	return undefined;
 }
+
+/**
+ * Compares two nullable numbers such that null values always come after defined ones.
+ */
+export function compareTwoNullableNumbers(a: number | undefined, b: number | undefined): number {
+	const aOrMax = a ?? Number.MAX_SAFE_INTEGER;
+	const bOrMax = b ?? Number.MAX_SAFE_INTEGER;
+	if (aOrMax < bOrMax) {
+		return -1;
+	} else if (aOrMax > bOrMax) {
+		return 1;
+	} else {
+		return 0;
+	}
+}

src/vs/workbench/contrib/preferences/common/preferencesContribution.ts

@@ -152,8 +152,8 @@ registry.registerConfiguration({
 			'type': 'string',
 			'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."),
+				nls.localize('settingsSearchTocBehavior.hide', "Hide the Table of Contents while searching. The search results will not be grouped by category, and instead will be sorted by similarity to the query, with exact keyword matches coming first."),
+				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. The search results will be grouped by category."),
 			],
 			'description': nls.localize('settingsSearchTocBehavior', "Controls the behavior of the settings editor Table of Contents while searching."),
 			'default': 'filter',

src/vs/workbench/services/preferences/common/preferences.ts

@@ -93,15 +93,15 @@ export interface ISetting {
 	editPresentation?: EditPresentationTypes;
 	nonLanguageSpecificDefaultValueSource?: string | IExtensionInfo;
 	isLanguageTagSetting?: boolean;
-	categoryOrder?: number;
 	categoryLabel?: string;
 
-	// For ExtensionToggle settings
+	// Internal properties
 	displayExtensionId?: string;
 	stableExtensionId?: string;
 	prereleaseExtensionId?: string;
 	title?: string;
 	extensionGroupTitle?: string;
+	internalOrder?: number;
 }
 
 export interface IExtensionSetting extends ISetting {

src/vs/workbench/services/preferences/common/preferencesModels.ts

@@ -634,7 +634,6 @@ export class DefaultSettings extends Disposable {
 		// Try using the title if the category id wasn't given
 		// (in which case the category id is the same as the extension id)
 		const categoryLabel = config.extensionInfo?.id === config.id ? config.title : config.id;
-		const categoryOrder = config.order;
 
 		for (const key in settingsObject) {
 			const prop: IConfigurationPropertySchema = settingsObject[key];
@@ -721,8 +720,7 @@ export class DefaultSettings extends Disposable {
 					order: prop.order,
 					nonLanguageSpecificDefaultValueSource: defaultValueSource,
 					isLanguageTagSetting,
-					categoryLabel,
-					categoryOrder
+					categoryLabel
 				});
 			}
 		}