From 0d87da04f3a031ec5aa682c43174d59490af83f4 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Fri, 12 Jun 2026 17:48:23 +0200 Subject: [PATCH] Rank Quick Access results by a continuous relevance score Quick Access ranked results by provider order, then alphabetically, with only a binary prefix boost and a round-robin slot quota per provider. A weak Preferences hit could therefore outrank a strong Command hit, and the per-entry match quality barely affected the order. Add an fzf-style relevance score in QuickAccessMatching that rewards word-boundary, consecutive-character and prefix matches and lightly penalises leading gaps and length. QuickAccessContents now scores every candidate, ranks them with a single comparator (score, then match quality) applied with a stable sort, and fills the table with the globally highest-ranked entries instead of a round-robin quota. Results stay grouped per provider in registration order, and each provider's natural order (recency for previous picks, alphabetical otherwise) is preserved on ties. QuickAccessMatcher is reused across all candidates of a compute pass so the wildcard and whitespace patterns are compiled once per filter rather than once per element. --- .../quickaccess/QuickAccessContents.java | 126 ++++++------------ .../quickaccess/QuickAccessEntry.java | 12 ++ .../quickaccess/QuickAccessMatcher.java | 32 +++-- .../quickaccess/QuickAccessMatching.java | 78 +++++++++++ .../quickaccess/QuickAccessMatchingTest.java | 48 +++++++ 5 files changed, 199 insertions(+), 97 deletions(-) diff --git a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessContents.java b/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessContents.java index c7590d06739..df92f8f7848 100644 --- a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessContents.java +++ b/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessContents.java @@ -21,8 +21,8 @@ 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; @@ -30,7 +30,6 @@ 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 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> entriesPerProvider = new LinkedHashMap<>( elementsForProviders.size()); if (showAllMatches) { - // Map elements to entries + // Map elements to entries, most relevant first within each provider for (Entry> elementsPerProvider : elementsForProviders .entrySet()) { QuickAccessProvider provider = elementsPerProvider.getKey(); List 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> elementsPerProvider : elementsForProviders - .entrySet()) { - QuickAccessProvider provider = elementsPerProvider.getKey(); - List 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 matched = new ArrayList<>(); + for (Entry> elementsPerProvider : elementsForProviders + .entrySet()) { + if (aMonitor.isCanceled()) { + break; + } + QuickAccessProvider provider = elementsPerProvider.getKey(); + for (QuickAccessElement element : elementsPerProvider.getValue()) { + if (element == perfectMatch) { + continue; } - } else { - for (Entry> elementsForProvider : elementsForProviders - .entrySet()) { - if (numberOfSlotsLeft > 0) { - QuickAccessProvider provider = elementsForProvider.getKey(); - List 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 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 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 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[]) 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 putPrefixMatchFirst(List elements, String prefix) { - List res = new ArrayList<>(elements); - List 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; /** diff --git a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessEntry.java b/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessEntry.java index d27ea8eed76..41e2fba46fa 100644 --- a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessEntry.java +++ b/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; + } + } \ No newline at end of file diff --git a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessMatcher.java b/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessMatcher.java index fa6f27957e9..abd28ff9c1c 100644 --- a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessMatcher.java +++ b/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 - * null; + * Returns a {@link QuickAccessEntry} carrying highlight regions and a relevance + * score if {@code element} matches the filter, or null otherwise. * - * @param filter filter for matching - * @param providerForMatching the provider that will own the entry - * @return a quick access entry or null * @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); diff --git a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessMatching.java b/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessMatching.java index 05df8faa3e2..7be4d95a0b5 100644 --- a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/quickaccess/QuickAccessMatching.java +++ b/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); + } } diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/quickaccess/QuickAccessMatchingTest.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/quickaccess/QuickAccessMatchingTest.java index aec387e3318..c6b7c33f447 100644 --- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/quickaccess/QuickAccessMatchingTest.java +++ b/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); + } }