Merge pull request #97200 from relmify/sort-order-fixes

Fix filename sort order edge cases in explorer.

src/vs/base/common/comparers.ts

@@ -6,7 +6,12 @@
 import { sep } from 'vs/base/common/path';
 import { IdleValue } from 'vs/base/common/async';
 
-const intlFileNameCollator: IdleValue<{ collator: Intl.Collator, collatorIsNumeric: boolean }> = new IdleValue(() => {
+// When comparing large numbers of strings, such as in sorting large arrays, is better for
+// performance to create an Intl.Collator object and use the function provided by its compare
+// property than it is to use String.prototype.localeCompare()
+
+// A collator with numeric sorting enabled, and no sensitivity to case or to accents
+const intlFileNameCollatorBaseNumeric: IdleValue<{ collator: Intl.Collator, collatorIsNumeric: boolean }> = new IdleValue(() => {
 	const collator = new Intl.Collator(undefined, { numeric: true, sensitivity: 'base' });
 	return {
 		collator: collator,
@@ -14,20 +19,64 @@ const intlFileNameCollator: IdleValue<{ collator: Intl.Collator, collatorIsNumer
 	};
 });
 
+// A collator with numeric sorting enabled.
+const intlFileNameCollatorNumeric: IdleValue<{ collator: Intl.Collator }> = new IdleValue(() => {
+	const collator = new Intl.Collator(undefined, { numeric: true });
+	return {
+		collator: collator
+	};
+});
+
+// A collator with numeric sorting enabled, and sensitivity to accents and diacritics but not case.
+const intlFileNameCollatorNumericCaseInsenstive: IdleValue<{ collator: Intl.Collator }> = new IdleValue(() => {
+	const collator = new Intl.Collator(undefined, { numeric: true, sensitivity: 'accent' });
+	return {
+		collator: collator
+	};
+});
+
 export function compareFileNames(one: string | null, other: string | null, caseSensitive = false): number {
 	const a = one || '';
 	const b = other || '';
-	const result = intlFileNameCollator.getValue().collator.compare(a, b);
+	const result = intlFileNameCollatorBaseNumeric.getValue().collator.compare(a, b);
 
 	// Using the numeric option in the collator will
 	// make compare(`foo1`, `foo01`) === 0. We must disambiguate.
-	if (intlFileNameCollator.getValue().collatorIsNumeric && result === 0 && a !== b) {
+	if (intlFileNameCollatorBaseNumeric.getValue().collatorIsNumeric && result === 0 && a !== b) {
 		return a < b ? -1 : 1;
 	}
 
 	return result;
 }
 
+/** Compares filenames by name then extension, sorting numbers numerically instead of alphabetically. */
+export function compareFileNamesNumeric(one: string | null, other: string | null): number {
+	const [oneName, oneExtension] = extractNameAndExtension(one, true);
+	const [otherName, otherExtension] = extractNameAndExtension(other, true);
+	const collatorNumeric = intlFileNameCollatorNumeric.getValue().collator;
+	const collatorNumericCaseInsensitive = intlFileNameCollatorNumericCaseInsenstive.getValue().collator;
+	let result;
+
+	// Check for name differences, comparing numbers numerically instead of alphabetically.
+	result = compareAndDisambiguateByLength(collatorNumeric, oneName, otherName);
+	if (result !== 0) {
+		return result;
+	}
+
+	// Check for case insensitive extension differences, comparing numbers numerically instead of alphabetically.
+	result = compareAndDisambiguateByLength(collatorNumericCaseInsensitive, oneExtension, otherExtension);
+	if (result !== 0) {
+		return result;
+	}
+
+	// Disambiguate the extension case if needed.
+	if (oneExtension !== otherExtension) {
+		return collatorNumeric.compare(oneExtension, otherExtension);
+	}
+
+	return 0;
+}
+
 const FileNameMatch = /^(.*?)(\.([^.]*))?$/;
 
 export function noIntlCompareFileNames(one: string | null, other: string | null, caseSensitive = false): number {
@@ -54,19 +103,19 @@ export function compareFileExtensions(one: string | null, other: string | null):
 	const [oneName, oneExtension] = extractNameAndExtension(one);
 	const [otherName, otherExtension] = extractNameAndExtension(other);
 
-	let result = intlFileNameCollator.getValue().collator.compare(oneExtension, otherExtension);
+	let result = intlFileNameCollatorBaseNumeric.getValue().collator.compare(oneExtension, otherExtension);
 
 	if (result === 0) {
 		// Using the numeric option in the collator will
 		// make compare(`foo1`, `foo01`) === 0. We must disambiguate.
-		if (intlFileNameCollator.getValue().collatorIsNumeric && oneExtension !== otherExtension) {
+		if (intlFileNameCollatorBaseNumeric.getValue().collatorIsNumeric && oneExtension !== otherExtension) {
 			return oneExtension < otherExtension ? -1 : 1;
 		}
 
 		// Extensions are equal, compare filenames
-		result = intlFileNameCollator.getValue().collator.compare(oneName, otherName);
+		result = intlFileNameCollatorBaseNumeric.getValue().collator.compare(oneName, otherName);
 
-		if (intlFileNameCollator.getValue().collatorIsNumeric && result === 0 && oneName !== otherName) {
+		if (intlFileNameCollatorBaseNumeric.getValue().collatorIsNumeric && result === 0 && oneName !== otherName) {
 			return oneName < otherName ? -1 : 1;
 		}
 	}
@@ -74,10 +123,63 @@ export function compareFileExtensions(one: string | null, other: string | null):
 	return result;
 }
 
-function extractNameAndExtension(str?: string | null): [string, string] {
+/** Compares filenames by extenson, then by name. Sorts numbers numerically, not alphabetically. */
+export function compareFileExtensionsNumeric(one: string | null, other: string | null): number {
+	const [oneName, oneExtension] = extractNameAndExtension(one, true);
+	const [otherName, otherExtension] = extractNameAndExtension(other, true);
+	const collatorNumeric = intlFileNameCollatorNumeric.getValue().collator;
+	const collatorNumericCaseInsensitive = intlFileNameCollatorNumericCaseInsenstive.getValue().collator;
+	let result;
+
+	// Check for extension differences, ignoring differences in case and comparing numbers numerically.
+	result = compareAndDisambiguateByLength(collatorNumericCaseInsensitive, oneExtension, otherExtension);
+	if (result !== 0) {
+		return result;
+	}
+
+	// Compare names.
+	result = compareAndDisambiguateByLength(collatorNumeric, oneName, otherName);
+	if (result !== 0) {
+		return result;
+	}
+
+	// Disambiguate extension case if needed.
+	if (oneExtension !== otherExtension) {
+		return collatorNumeric.compare(oneExtension, otherExtension);
+	}
+
+	return 0;
+}
+
+/** Extracts the name and extension from a full filename, with optional special handling for dotfiles */
+function extractNameAndExtension(str?: string | null, dotfilesAsNames = false): [string, string] {
 	const match = str ? FileNameMatch.exec(str) as Array<string> : ([] as Array<string>);
 
-	return [(match && match[1]) || '', (match && match[3]) || ''];
+	let result: [string, string] = [(match && match[1]) || '', (match && match[3]) || ''];
+
+	// if the dotfilesAsNames option is selected, treat an empty filename with an extension,
+	// or a filename that starts with a dot, as a dotfile name
+	if (dotfilesAsNames && (!result[0] && result[1] || result[0] && result[0].charAt(0) === '.')) {
+		result = [result[0] + '.' + result[1], ''];
+	}
+
+	return result;
+}
+
+function compareAndDisambiguateByLength(collator: Intl.Collator, one: string, other: string) {
+	// Check for differences
+	let result = collator.compare(one, other);
+	if (result !== 0) {
+		return result;
+	}
+
+	// In a numeric comparison, `foo1` and `foo01` will compare as equivalent.
+	// Disambiguate by sorting the shorter string first.
+	if (one.length !== other.length) {
+		return one.length < other.length ? -1 : 1;
+	}
+
+	return 0;
 }
 
 function comparePathComponents(one: string, other: string, caseSensitive = false): number {

src/vs/workbench/contrib/files/browser/views/explorerViewer.ts

@@ -29,7 +29,7 @@ import { IKeyboardEvent } from 'vs/base/browser/keyboardEvent';
 import { equals, deepClone } from 'vs/base/common/objects';
 import * as path from 'vs/base/common/path';
 import { ExplorerItem, NewExplorerItem } from 'vs/workbench/contrib/files/common/explorerModel';
-import { compareFileExtensions, compareFileNames } from 'vs/base/common/comparers';
+import { compareFileExtensionsNumeric, compareFileNamesNumeric } from 'vs/base/common/comparers';
 import { fillResourceDataTransfers, CodeDataTransfers, extractResources, containsDragType } from 'vs/workbench/browser/dnd';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { IDragAndDropData, DataTransfers } from 'vs/base/browser/dnd';
@@ -654,7 +654,7 @@ export class FileSorter implements ITreeSorter<ExplorerItem> {
 				}
 
 				if (statA.isDirectory && statB.isDirectory) {
-					return compareFileNames(statA.name, statB.name);
+					return compareFileNamesNumeric(statA.name, statB.name);
 				}
 
 				break;
@@ -688,17 +688,17 @@ export class FileSorter implements ITreeSorter<ExplorerItem> {
 		// Sort Files
 		switch (sortOrder) {
 			case 'type':
-				return compareFileExtensions(statA.name, statB.name);
+				return compareFileExtensionsNumeric(statA.name, statB.name);
 
 			case 'modified':
 				if (statA.mtime !== statB.mtime) {
 					return (statA.mtime && statB.mtime && statA.mtime < statB.mtime) ? 1 : -1;
 				}
 
-				return compareFileNames(statA.name, statB.name);
+				return compareFileNamesNumeric(statA.name, statB.name);
 
 			default: /* 'default', 'mixed', 'filesFirst' */
-				return compareFileNames(statA.name, statB.name);
+				return compareFileNamesNumeric(statA.name, statB.name);
 		}
 	}
 }