Rank Quick Access results by a continuous relevance score

bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessContents.java

@@ -21,16 +21,15 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Objects;
-import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Function;
 import java.util.regex.Matcher;
@@ -130,6 +129,15 @@ public abstract class QuickAccessContents {
 	private TriggerSequence keySequence;
 	private Job computeProposalsJob;
 
+	/**
+	 * Orders entries by descending relevance score, then match quality. Applied
+	 * with a stable sort so each provider's natural order (recency for previous
+	 * picks, alphabetical otherwise) is preserved on ties.
+	 */
+	private static final Comparator<QuickAccessEntry> BY_RELEVANCE = Comparator
+			.comparingInt(QuickAccessEntry::getMatchScore).reversed()
+			.thenComparingInt(QuickAccessEntry::getMatchQuality);
+
 	public QuickAccessContents(QuickAccessProvider[] providers) {
 		this.providers = providers;
 	}
@@ -447,9 +455,6 @@ public IStatus runInUIThread(IProgressMonitor monitor) {
 						elementsToProviders.put(element, provider);
 					}
 				}
-				if (!filter.isEmpty() && !sortedElements.isEmpty()) {
-					sortedElements = putPrefixMatchFirst(sortedElements, filter);
-				}
 				elementsForProviders.put(provider, new ArrayList<>(sortedElements));
 			}
 		}
@@ -482,74 +487,55 @@ public IStatus runInUIThread(IProgressMonitor monitor) {
 				}
 			}
 		}
+		QuickAccessMatcher matcher = new QuickAccessMatcher();
 		LinkedHashMap<QuickAccessProvider, List<QuickAccessEntry>> entriesPerProvider = new LinkedHashMap<>(
 				elementsForProviders.size());
 		if (showAllMatches) {
-			// Map elements to entries
+			// Map elements to entries, most relevant first within each provider
 			for (Entry<QuickAccessProvider, List<QuickAccessElement>> elementsPerProvider : elementsForProviders
 					.entrySet()) {
 				QuickAccessProvider provider = elementsPerProvider.getKey();
 				List<QuickAccessEntry> entries = elementsPerProvider.getValue().stream() //
-						.map(QuickAccessMatcher::new) //
-						.map(matcher -> matcher.match(finalFilter, provider)) //
+						.map(element -> matcher.match(element, finalFilter, provider)) //
 						.filter(Objects::nonNull) //
+						.sorted(BY_RELEVANCE) //
 						.collect(Collectors.toList());
 				if (!entries.isEmpty()) {
 					entriesPerProvider.put(provider, entries);
 				}
 			}
 		} else {
-			int numberOfSlotsLeft = perfectMatch != null ? maxNumberOfItemsInTable -1 : maxNumberOfItemsInTable;
-			while (!elementsForProviders.isEmpty() && numberOfSlotsLeft > 0) {
-				int nbEntriesPerProvider = numberOfSlotsLeft / elementsForProviders.size();
-				if (nbEntriesPerProvider > 0) {
-					for (Entry<QuickAccessProvider, List<QuickAccessElement>> elementsPerProvider : elementsForProviders
-							.entrySet()) {
-						QuickAccessProvider provider = elementsPerProvider.getKey();
-						List<QuickAccessElement> elements = elementsPerProvider.getValue();
-						int toPickEntries = nbEntriesPerProvider;
-						while (toPickEntries > 0 && !elements.isEmpty()) {
-							QuickAccessElement element = elements.remove(0);
-							QuickAccessEntry entry = new QuickAccessMatcher(element).match(filter, provider);
-							if (entry != null) {
-								numberOfSlotsLeft--;
-								toPickEntries--;
-								if (!entriesPerProvider.containsKey(provider)) {
-									entriesPerProvider.put(provider, new LinkedList<>());
-								}
-								entriesPerProvider.get(provider).add(entry);
-							}
-						}
+			int numberOfSlotsLeft = perfectMatch != null ? maxNumberOfItemsInTable - 1 : maxNumberOfItemsInTable;
+			// Score every candidate and keep the globally highest-ranked entries, so a
+			// strong match wins a slot regardless of which provider produced it
+			List<QuickAccessEntry> matched = new ArrayList<>();
+			for (Entry<QuickAccessProvider, List<QuickAccessElement>> elementsPerProvider : elementsForProviders
+					.entrySet()) {
+				if (aMonitor.isCanceled()) {
+					break;
+				}
+				QuickAccessProvider provider = elementsPerProvider.getKey();
+				for (QuickAccessElement element : elementsPerProvider.getValue()) {
+					if (element == perfectMatch) {
+						continue;
 					}
-				} else {
-					for (Entry<QuickAccessProvider, List<QuickAccessElement>> elementsForProvider : elementsForProviders
-							.entrySet()) {
-						if (numberOfSlotsLeft > 0) {
-							QuickAccessProvider provider = elementsForProvider.getKey();
-							List<QuickAccessElement> elements = elementsForProvider.getValue();
-							boolean entryPicked = false;
-							while (!entryPicked && !elements.isEmpty()) {
-								QuickAccessElement element = elements.remove(0);
-								QuickAccessEntry entry = new QuickAccessMatcher(element).match(filter, provider);
-								if (entry != null) {
-									numberOfSlotsLeft--;
-									entryPicked = true;
-									if (!entriesPerProvider.containsKey(provider)) {
-										entriesPerProvider.put(provider, new LinkedList<>());
-									}
-									entriesPerProvider.get(provider).add(entry);
-								}
-							}
-						}
+					QuickAccessEntry entry = matcher.match(element, finalFilter, provider);
+					if (entry != null) {
+						matched.add(entry);
 					}
 				}
-				Set<QuickAccessProvider> exhaustedProviders = new HashSet<>();
-				elementsForProviders.forEach((provider, elements) -> {
-					if (elements.isEmpty()) {
-						exhaustedProviders.add(provider);
-					}
-				});
-				exhaustedProviders.forEach(elementsForProviders::remove);
+			}
+			matched.sort(BY_RELEVANCE);
+			int slots = Math.max(0, numberOfSlotsLeft);
+			List<QuickAccessEntry> winners = matched.subList(0, Math.min(slots, matched.size()));
+			// Group the winners back per provider for the table, keeping providers in
+			// registration order and entries in relevance order within each provider
+			for (QuickAccessProvider provider : elementsForProviders.keySet()) {
+				List<QuickAccessEntry> group = winners.stream().filter(entry -> entry.provider == provider)
+						.collect(Collectors.toCollection(LinkedList::new));
+				if (!group.isEmpty()) {
+					entriesPerProvider.put(provider, group);
+				}
 			}
 		}
 		//
@@ -563,34 +549,6 @@ public IStatus runInUIThread(IProgressMonitor monitor) {
 		return (List<QuickAccessEntry>[]) res.toArray(new List<?>[res.size()]);
 	}
 
-	/*
-	 * Consider whether we could directly check the "matchQuality" here, but it
-	 * seems to be a more expensive operation
-	 */
-	private static List<QuickAccessElement> putPrefixMatchFirst(List<QuickAccessElement> elements, String prefix) {
-		List<QuickAccessElement> res = new ArrayList<>(elements);
-		List<Integer> matchingIndexes = new ArrayList<>();
-		for (int i = 0; i < elements.size(); i++) {
-			if (elements.get(i).getLabel().toLowerCase().startsWith(prefix.toLowerCase())) {
-				matchingIndexes.add(Integer.valueOf(i));
-			}
-		}
-		int currentMatchIndex = 0;
-		int currentNonMatchIndex = matchingIndexes.size();
-		for (int i = 0; i < res.size(); i++) {
-			boolean isMatch = !matchingIndexes.isEmpty() && matchingIndexes.iterator().next().intValue() == i;
-			if (isMatch) {
-				matchingIndexes.remove(0);
-				res.set(currentMatchIndex, elements.get(i));
-				currentMatchIndex++;
-			} else {
-				res.set(currentNonMatchIndex, elements.get(i));
-				currentNonMatchIndex++;
-			}
-		}
-		return res;
-	}
-
 	Pattern categoryPattern;
 
 	/**

bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessEntry.java

@@ -56,6 +56,9 @@ public class QuickAccessEntry {
 	 */
 	private final int matchQuality;
 
+	/** Continuous relevance score; higher is better. Set by the matcher. */
+	int matchScore;
+
 	/**
 	 * Indicates the filter string was a perfect match to the label or there is no
 	 * filter applied
@@ -273,4 +276,13 @@ public int getMatchQuality() {
 		return matchQuality;
 	}
 
+	/**
+	 * Returns the continuous relevance score; higher is better.
+	 *
+	 * @return the relevance score
+	 */
+	public int getMatchScore() {
+		return matchScore;
+	}
+
 }

bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessMatcher.java

@@ -26,12 +26,6 @@
  */
 public final class QuickAccessMatcher {
 
-	private final QuickAccessElement element;
-
-	public QuickAccessMatcher(QuickAccessElement element) {
-		this.element = element;
-	}
-
 	private static final int[][] EMPTY_INDICES = new int[0][0];
 
 	// whitespaces filter and patterns
@@ -60,16 +54,28 @@ private Pattern getWildcardsPattern(String filter) {
 	}
 
 	/**
-	 * If this element is a match (partial, complete, camel case, etc) to the given
-	 * filter, returns a {@link QuickAccessEntry}. Otherwise returns
-	 * <code>null</code>;
+	 * Returns a {@link QuickAccessEntry} carrying highlight regions and a relevance
+	 * score if {@code element} matches the filter, or <code>null</code> otherwise.
 	 *
-	 * @param filter              filter for matching
-	 * @param providerForMatching the provider that will own the entry
-	 * @return a quick access entry or <code>null</code>
 	 * @noreference This method is not intended to be referenced by clients.
 	 */
-	public QuickAccessEntry match(String filter, QuickAccessProvider providerForMatching) {
+	public QuickAccessEntry match(QuickAccessElement element, String filter, QuickAccessProvider providerForMatching) {
+		QuickAccessEntry entry = doMatch(element, filter, providerForMatching);
+		if (entry != null) {
+			entry.matchScore = computeScore(element, filter, providerForMatching);
+		}
+		return entry;
+	}
+
+	private static int computeScore(QuickAccessElement element, String filter, QuickAccessProvider provider) {
+		int score = QuickAccessMatching.score(element.getMatchLabel(), filter);
+		if (score == QuickAccessMatching.SCORE_NONE) {
+			score = QuickAccessMatching.score(provider.getName() + ' ' + element.getMatchLabel(), filter);
+		}
+		return score == QuickAccessMatching.SCORE_NONE ? 0 : score;
+	}
+
+	private QuickAccessEntry doMatch(QuickAccessElement element, String filter, QuickAccessProvider providerForMatching) {
 		String matchLabel = element.getMatchLabel();
 		String label = element.getLabel();
 		int quality = QuickAccessMatching.substringMatchQuality(matchLabel, label, filter);

bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessMatching.java

@@ -114,4 +114,82 @@ public static int substringMatchQuality(String matchLabel, String label, String
 		}
 		return QuickAccessEntry.MATCH_GOOD;
 	}
+
+	/**
+	 * Sentinel returned by {@link #score} when the filter is not a subsequence of
+	 * the text.
+	 */
+	public static final int SCORE_NONE = Integer.MIN_VALUE / 2;
+
+	private static final int MATCH_BASE = 16;
+	private static final int BOUNDARY_BONUS = 30;
+	private static final int CONSECUTIVE_BONUS = 15;
+	private static final int PREFIX_BONUS = 8;
+	private static final int LEADING_GAP_PENALTY = 3;
+	private static final int MAX_LEADING_GAP = 5;
+	private static final int MAX_LENGTH_PENALTY = 20;
+
+	/**
+	 * Continuous relevance score for ranking; higher is better. Rewards matches at
+	 * word boundaries, consecutive characters and a prefix, and lightly penalises
+	 * leading gaps and longer text. Wildcard and whitespace characters in the
+	 * filter are ignored and matching is case-insensitive.
+	 *
+	 * @param text   the candidate text, in its original case
+	 * @param filter the user filter
+	 * @return the score, or {@link #SCORE_NONE} if the filter is not a subsequence
+	 *         of the text
+	 */
+	public static int score(String text, String filter) {
+		String needle = filter.toLowerCase().replaceAll("[\\s*?]", ""); //$NON-NLS-1$ //$NON-NLS-2$
+		if (needle.isEmpty()) {
+			return 0;
+		}
+		int score = 0;
+		int firstMatch = -1;
+		int prevMatch = -2;
+		int j = 0;
+		for (int i = 0; i < text.length() && j < needle.length(); i++) {
+			if (Character.toLowerCase(text.charAt(i)) != needle.charAt(j)) {
+				continue;
+			}
+			if (firstMatch < 0) {
+				firstMatch = i;
+			}
+			int charScore = MATCH_BASE;
+			if (isBoundary(text, i)) {
+				charScore += BOUNDARY_BONUS;
+			}
+			if (prevMatch == i - 1) {
+				charScore += CONSECUTIVE_BONUS;
+			}
+			score += charScore;
+			prevMatch = i;
+			j++;
+		}
+		if (j < needle.length()) {
+			return SCORE_NONE;
+		}
+		if (firstMatch == 0) {
+			score += PREFIX_BONUS;
+		}
+		score -= Math.min(firstMatch, MAX_LEADING_GAP) * LEADING_GAP_PENALTY;
+		score -= Math.min(text.length(), MAX_LENGTH_PENALTY);
+		return score;
+	}
+
+	private static boolean isBoundary(String text, int i) {
+		if (i == 0) {
+			return true;
+		}
+		char prev = text.charAt(i - 1);
+		char cur = text.charAt(i);
+		if (!Character.isLetterOrDigit(prev)) {
+			return true;
+		}
+		if (Character.isUpperCase(cur) && !Character.isUpperCase(prev)) {
+			return true;
+		}
+		return Character.isDigit(cur) && !Character.isDigit(prev);
+	}
 }

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/quickaccess/QuickAccessMatchingTest.java

@@ -109,4 +109,52 @@ public void safeCompileCompilesValidRegex() {
 		Pattern p = QuickAccessMatching.safeCompile("foo.*");
 		assertTrue(p.matcher("foobar").matches());
 	}
+
+	@Test
+	public void scoreReturnsNoneWhenNotSubsequence() {
+		assertEquals(QuickAccessMatching.SCORE_NONE, QuickAccessMatching.score("Rename", "xyz"));
+	}
+
+	@Test
+	public void scoreReturnsZeroForEmptyFilter() {
+		assertEquals(0, QuickAccessMatching.score("Rename", ""));
+	}
+
+	@Test
+	public void scorePrefersConsecutiveOverScattered() {
+		int consecutive = QuickAccessMatching.score("abcxyz", "abc");
+		int scattered = QuickAccessMatching.score("axbxcx", "abc");
+		assertTrue(consecutive > scattered, "consecutive " + consecutive + " should beat scattered " + scattered);
+	}
+
+	@Test
+	public void scorePrefersPrefixOverMidWord() {
+		int prefix = QuickAccessMatching.score("Rename", "re");
+		int midWord = QuickAccessMatching.score("Score", "re");
+		assertTrue(prefix > midWord, "prefix " + prefix + " should beat mid-word " + midWord);
+	}
+
+	@Test
+	public void scorePrefersShorterOnOtherwiseEqualMatch() {
+		int shorter = QuickAccessMatching.score("Re", "re");
+		int longer = QuickAccessMatching.score("Renew", "re");
+		assertTrue(shorter > longer, "shorter " + shorter + " should beat longer " + longer);
+	}
+
+	@Test
+	public void scoreRewardsWordBoundaryInitials() {
+		int initials = QuickAccessMatching.score("New File", "nf");
+		int midWord = QuickAccessMatching.score("Confirm", "nf");
+		assertTrue(initials > midWord, "word-initial " + initials + " should beat mid-word " + midWord);
+	}
+
+	@Test
+	public void scoreIgnoresWildcardChars() {
+		assertTrue(QuickAccessMatching.score("Rename Resource", "re*ce") > QuickAccessMatching.SCORE_NONE);
+	}
+
+	@Test
+	public void scoreIsCaseInsensitive() {
+		assertTrue(QuickAccessMatching.score("RENAME", "rename") > QuickAccessMatching.SCORE_NONE);
+	}
 }