From 43b5deb706e405d14d1c57328bda44d374b34697 Mon Sep 17 00:00:00 2001
From: Leila Pearson <873990+leilapearson@users.noreply.github.com>
Date: Fri, 8 May 2020 06:20:28 -0700
Subject: [PATCH] Address PR comments.

---
 src/vs/base/common/comparers.ts | 46 +++++++++++++++------------------
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/src/vs/base/common/comparers.ts b/src/vs/base/common/comparers.ts
index 4696e91bb54..23ab2129be7 100644
--- a/src/vs/base/common/comparers.ts
+++ b/src/vs/base/common/comparers.ts
@@ -58,27 +58,17 @@ export function compareFileNamesNumeric(one: string | null, other: string | null
 	let result;
 
 	// Check for name differences, comparing numbers numerically instead of alphabetically.
-	result = collatorNumeric.compare(oneName, otherName);
+	result = compareAndDisambiguateByLength(collatorNumeric, oneName, otherName);
 	if (result !== 0) {
 		return result;
 	}
 
-	// Using the numeric option in the collator will make compare(`foo1`, `foo01`) === 0. Sort the shorter name first.
-	if (oneName.length !== otherName.length) {
-		return oneName.length < otherName.length ? -1 : 1;
-	}
-
 	// Check for case insensitive extension differences, comparing numbers numerically instead of alphabetically.
-	result = collatorNumericCaseInsensitive.compare(oneExtension, otherExtension);
+	result = compareAndDisambiguateByLength(collatorNumericCaseInsensitive, oneExtension, otherExtension);
 	if (result !== 0) {
 		return result;
 	}
 
-	// If extensions are numerically equal but not equal in length, sort the shorter extension first.
-	if (oneExtension.length !== otherExtension.length) {
-		return oneExtension.length < otherExtension.length ? -1 : 1;
-	}
-
 	// Disambiguate the extension case if needed.
 	if (oneExtension !== otherExtension) {
 		return collatorNumeric.compare(oneExtension, otherExtension);
@@ -142,27 +132,17 @@ export function compareFileExtensionsNumeric(one: string | null, other: string |
 	let result;
 
 	// Check for extension differences, ignoring differences in case and comparing numbers numerically.
-	result = collatorNumericCaseInsensitive.compare(oneExtension, otherExtension);
+	result = compareAndDisambiguateByLength(collatorNumericCaseInsensitive, oneExtension, otherExtension);
 	if (result !== 0) {
 		return result;
 	}
 
-	// Disambiguate equivalent numbers in extensions.
-	if (oneExtension.length !== otherExtension.length) {
-		return oneExtension.length < otherExtension.length ? -1 : 1;
-	}
-
 	// Compare names.
-	result = collatorNumeric.compare(oneName, otherName);
+	result = compareAndDisambiguateByLength(collatorNumeric, oneName, otherName);
 	if (result !== 0) {
 		return result;
 	}
 
-	// Disambiguate equivalent numbers in names.
-	if (oneName.length !== otherName.length) {
-		return oneName.length < otherName.length ? -1 : 1;
-	}
-
 	// Disambiguate extension case if needed.
 	if (oneExtension !== otherExtension) {
 		return collatorNumeric.compare(oneExtension, otherExtension);
@@ -172,7 +152,7 @@ export function compareFileExtensionsNumeric(one: string | null, other: string |
 }
 
 /** Extracts the name and extension from a full filename, with optional special handling for dotfiles */
-export function extractNameAndExtension(str?: string | null, dotfilesAsNames = false): [string, string] {
+function extractNameAndExtension(str?: string | null, dotfilesAsNames = false): [string, string] {
 	const match = str ? FileNameMatch.exec(str) as Array<string> : ([] as Array<string>);
 
 	let result: [string, string] = [(match && match[1]) || '', (match && match[3]) || ''];
@@ -186,6 +166,22 @@ export function extractNameAndExtension(str?: string | null, dotfilesAsNames = f
 	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 {
 	if (!caseSensitive) {
 		one = one && one.toLowerCase();
-- 
GitLab