Skip to content

Commit 86a07a0

Browse files
committed
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.
1 parent 7be088e commit 86a07a0

5 files changed

Lines changed: 199 additions & 97 deletions

File tree

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

Lines changed: 42 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,15 @@
2121
import java.util.ArrayList;
2222
import java.util.Arrays;
2323
import java.util.Collections;
24+
import java.util.Comparator;
2425
import java.util.HashMap;
25-
import java.util.HashSet;
2626
import java.util.Iterator;
2727
import java.util.LinkedHashMap;
2828
import java.util.LinkedList;
2929
import java.util.List;
3030
import java.util.Map;
3131
import java.util.Map.Entry;
3232
import java.util.Objects;
33-
import java.util.Set;
3433
import java.util.concurrent.atomic.AtomicReference;
3534
import java.util.function.Function;
3635
import java.util.regex.Matcher;
@@ -130,6 +129,15 @@ public abstract class QuickAccessContents {
130129
private TriggerSequence keySequence;
131130
private Job computeProposalsJob;
132131

132+
/**
133+
* Orders entries by descending relevance score, then match quality. Applied
134+
* with a stable sort so each provider's natural order (recency for previous
135+
* picks, alphabetical otherwise) is preserved on ties.
136+
*/
137+
private static final Comparator<QuickAccessEntry> BY_RELEVANCE = Comparator
138+
.comparingInt(QuickAccessEntry::getMatchScore).reversed()
139+
.thenComparingInt(QuickAccessEntry::getMatchQuality);
140+
133141
public QuickAccessContents(QuickAccessProvider[] providers) {
134142
this.providers = providers;
135143
}
@@ -488,9 +496,6 @@ private List<QuickAccessEntry>[] computeMatchingEntries(String filter, QuickAcce
488496
elementsToProviders.put(element, provider);
489497
}
490498
}
491-
if (!filter.isEmpty() && !sortedElements.isEmpty()) {
492-
sortedElements = putPrefixMatchFirst(sortedElements, filter);
493-
}
494499
elementsForProviders.put(provider, new ArrayList<>(sortedElements));
495500
}
496501
}
@@ -523,74 +528,55 @@ private List<QuickAccessEntry>[] computeMatchingEntries(String filter, QuickAcce
523528
}
524529
}
525530
}
531+
QuickAccessMatcher matcher = new QuickAccessMatcher();
526532
LinkedHashMap<QuickAccessProvider, List<QuickAccessEntry>> entriesPerProvider = new LinkedHashMap<>(
527533
elementsForProviders.size());
528534
if (showAllMatches) {
529-
// Map elements to entries
535+
// Map elements to entries, most relevant first within each provider
530536
for (Entry<QuickAccessProvider, List<QuickAccessElement>> elementsPerProvider : elementsForProviders
531537
.entrySet()) {
532538
QuickAccessProvider provider = elementsPerProvider.getKey();
533539
List<QuickAccessEntry> entries = elementsPerProvider.getValue().stream() //
534-
.map(QuickAccessMatcher::new) //
535-
.map(matcher -> matcher.match(finalFilter, provider)) //
540+
.map(element -> matcher.match(element, finalFilter, provider)) //
536541
.filter(Objects::nonNull) //
542+
.sorted(BY_RELEVANCE) //
537543
.collect(Collectors.toList());
538544
if (!entries.isEmpty()) {
539545
entriesPerProvider.put(provider, entries);
540546
}
541547
}
542548
} else {
543-
int numberOfSlotsLeft = perfectMatch != null ? maxNumberOfItemsInTable -1 : maxNumberOfItemsInTable;
544-
while (!elementsForProviders.isEmpty() && numberOfSlotsLeft > 0) {
545-
int nbEntriesPerProvider = numberOfSlotsLeft / elementsForProviders.size();
546-
if (nbEntriesPerProvider > 0) {
547-
for (Entry<QuickAccessProvider, List<QuickAccessElement>> elementsPerProvider : elementsForProviders
548-
.entrySet()) {
549-
QuickAccessProvider provider = elementsPerProvider.getKey();
550-
List<QuickAccessElement> elements = elementsPerProvider.getValue();
551-
int toPickEntries = nbEntriesPerProvider;
552-
while (toPickEntries > 0 && !elements.isEmpty()) {
553-
QuickAccessElement element = elements.remove(0);
554-
QuickAccessEntry entry = new QuickAccessMatcher(element).match(filter, provider);
555-
if (entry != null) {
556-
numberOfSlotsLeft--;
557-
toPickEntries--;
558-
if (!entriesPerProvider.containsKey(provider)) {
559-
entriesPerProvider.put(provider, new LinkedList<>());
560-
}
561-
entriesPerProvider.get(provider).add(entry);
562-
}
563-
}
549+
int numberOfSlotsLeft = perfectMatch != null ? maxNumberOfItemsInTable - 1 : maxNumberOfItemsInTable;
550+
// Score every candidate and keep the globally highest-ranked entries, so a
551+
// strong match wins a slot regardless of which provider produced it
552+
List<QuickAccessEntry> matched = new ArrayList<>();
553+
for (Entry<QuickAccessProvider, List<QuickAccessElement>> elementsPerProvider : elementsForProviders
554+
.entrySet()) {
555+
if (aMonitor.isCanceled()) {
556+
break;
557+
}
558+
QuickAccessProvider provider = elementsPerProvider.getKey();
559+
for (QuickAccessElement element : elementsPerProvider.getValue()) {
560+
if (element == perfectMatch) {
561+
continue;
564562
}
565-
} else {
566-
for (Entry<QuickAccessProvider, List<QuickAccessElement>> elementsForProvider : elementsForProviders
567-
.entrySet()) {
568-
if (numberOfSlotsLeft > 0) {
569-
QuickAccessProvider provider = elementsForProvider.getKey();
570-
List<QuickAccessElement> elements = elementsForProvider.getValue();
571-
boolean entryPicked = false;
572-
while (!entryPicked && !elements.isEmpty()) {
573-
QuickAccessElement element = elements.remove(0);
574-
QuickAccessEntry entry = new QuickAccessMatcher(element).match(filter, provider);
575-
if (entry != null) {
576-
numberOfSlotsLeft--;
577-
entryPicked = true;
578-
if (!entriesPerProvider.containsKey(provider)) {
579-
entriesPerProvider.put(provider, new LinkedList<>());
580-
}
581-
entriesPerProvider.get(provider).add(entry);
582-
}
583-
}
584-
}
563+
QuickAccessEntry entry = matcher.match(element, finalFilter, provider);
564+
if (entry != null) {
565+
matched.add(entry);
585566
}
586567
}
587-
Set<QuickAccessProvider> exhaustedProviders = new HashSet<>();
588-
elementsForProviders.forEach((provider, elements) -> {
589-
if (elements.isEmpty()) {
590-
exhaustedProviders.add(provider);
591-
}
592-
});
593-
exhaustedProviders.forEach(elementsForProviders::remove);
568+
}
569+
matched.sort(BY_RELEVANCE);
570+
int slots = Math.max(0, numberOfSlotsLeft);
571+
List<QuickAccessEntry> winners = matched.subList(0, Math.min(slots, matched.size()));
572+
// Group the winners back per provider for the table, keeping providers in
573+
// registration order and entries in relevance order within each provider
574+
for (QuickAccessProvider provider : elementsForProviders.keySet()) {
575+
List<QuickAccessEntry> group = winners.stream().filter(entry -> entry.provider == provider)
576+
.collect(Collectors.toCollection(LinkedList::new));
577+
if (!group.isEmpty()) {
578+
entriesPerProvider.put(provider, group);
579+
}
594580
}
595581
}
596582
//
@@ -604,34 +590,6 @@ private List<QuickAccessEntry>[] computeMatchingEntries(String filter, QuickAcce
604590
return (List<QuickAccessEntry>[]) res.toArray(new List<?>[res.size()]);
605591
}
606592

607-
/*
608-
* Consider whether we could directly check the "matchQuality" here, but it
609-
* seems to be a more expensive operation
610-
*/
611-
private static List<QuickAccessElement> putPrefixMatchFirst(List<QuickAccessElement> elements, String prefix) {
612-
List<QuickAccessElement> res = new ArrayList<>(elements);
613-
List<Integer> matchingIndexes = new ArrayList<>();
614-
for (int i = 0; i < elements.size(); i++) {
615-
if (elements.get(i).getLabel().toLowerCase().startsWith(prefix.toLowerCase())) {
616-
matchingIndexes.add(Integer.valueOf(i));
617-
}
618-
}
619-
int currentMatchIndex = 0;
620-
int currentNonMatchIndex = matchingIndexes.size();
621-
for (int i = 0; i < res.size(); i++) {
622-
boolean isMatch = !matchingIndexes.isEmpty() && matchingIndexes.iterator().next().intValue() == i;
623-
if (isMatch) {
624-
matchingIndexes.remove(0);
625-
res.set(currentMatchIndex, elements.get(i));
626-
currentMatchIndex++;
627-
} else {
628-
res.set(currentNonMatchIndex, elements.get(i));
629-
currentNonMatchIndex++;
630-
}
631-
}
632-
return res;
633-
}
634-
635593
Pattern categoryPattern;
636594

637595
/**

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ public class QuickAccessEntry {
5656
*/
5757
private final int matchQuality;
5858

59+
/** Continuous relevance score; higher is better. Set by the matcher. */
60+
int matchScore;
61+
5962
/**
6063
* Indicates the filter string was a perfect match to the label or there is no
6164
* filter applied
@@ -273,4 +276,13 @@ public int getMatchQuality() {
273276
return matchQuality;
274277
}
275278

279+
/**
280+
* Returns the continuous relevance score; higher is better.
281+
*
282+
* @return the relevance score
283+
*/
284+
public int getMatchScore() {
285+
return matchScore;
286+
}
287+
276288
}

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

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,6 @@
2626
*/
2727
public final class QuickAccessMatcher {
2828

29-
private final QuickAccessElement element;
30-
31-
public QuickAccessMatcher(QuickAccessElement element) {
32-
this.element = element;
33-
}
34-
3529
private static final int[][] EMPTY_INDICES = new int[0][0];
3630

3731
// whitespaces filter and patterns
@@ -60,16 +54,28 @@ private Pattern getWildcardsPattern(String filter) {
6054
}
6155

6256
/**
63-
* If this element is a match (partial, complete, camel case, etc) to the given
64-
* filter, returns a {@link QuickAccessEntry}. Otherwise returns
65-
* <code>null</code>;
57+
* Returns a {@link QuickAccessEntry} carrying highlight regions and a relevance
58+
* score if {@code element} matches the filter, or <code>null</code> otherwise.
6659
*
67-
* @param filter filter for matching
68-
* @param providerForMatching the provider that will own the entry
69-
* @return a quick access entry or <code>null</code>
7060
* @noreference This method is not intended to be referenced by clients.
7161
*/
72-
public QuickAccessEntry match(String filter, QuickAccessProvider providerForMatching) {
62+
public QuickAccessEntry match(QuickAccessElement element, String filter, QuickAccessProvider providerForMatching) {
63+
QuickAccessEntry entry = doMatch(element, filter, providerForMatching);
64+
if (entry != null) {
65+
entry.matchScore = computeScore(element, filter, providerForMatching);
66+
}
67+
return entry;
68+
}
69+
70+
private static int computeScore(QuickAccessElement element, String filter, QuickAccessProvider provider) {
71+
int score = QuickAccessMatching.score(element.getMatchLabel(), filter);
72+
if (score == QuickAccessMatching.SCORE_NONE) {
73+
score = QuickAccessMatching.score(provider.getName() + ' ' + element.getMatchLabel(), filter);
74+
}
75+
return score == QuickAccessMatching.SCORE_NONE ? 0 : score;
76+
}
77+
78+
private QuickAccessEntry doMatch(QuickAccessElement element, String filter, QuickAccessProvider providerForMatching) {
7379
String matchLabel = element.getMatchLabel();
7480
String label = element.getLabel();
7581
int quality = QuickAccessMatching.substringMatchQuality(matchLabel, label, filter);

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

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,4 +114,82 @@ public static int substringMatchQuality(String matchLabel, String label, String
114114
}
115115
return QuickAccessEntry.MATCH_GOOD;
116116
}
117+
118+
/**
119+
* Sentinel returned by {@link #score} when the filter is not a subsequence of
120+
* the text.
121+
*/
122+
public static final int SCORE_NONE = Integer.MIN_VALUE / 2;
123+
124+
private static final int MATCH_BASE = 16;
125+
private static final int BOUNDARY_BONUS = 30;
126+
private static final int CONSECUTIVE_BONUS = 15;
127+
private static final int PREFIX_BONUS = 8;
128+
private static final int LEADING_GAP_PENALTY = 3;
129+
private static final int MAX_LEADING_GAP = 5;
130+
private static final int MAX_LENGTH_PENALTY = 20;
131+
132+
/**
133+
* Continuous relevance score for ranking; higher is better. Rewards matches at
134+
* word boundaries, consecutive characters and a prefix, and lightly penalises
135+
* leading gaps and longer text. Wildcard and whitespace characters in the
136+
* filter are ignored and matching is case-insensitive.
137+
*
138+
* @param text the candidate text, in its original case
139+
* @param filter the user filter
140+
* @return the score, or {@link #SCORE_NONE} if the filter is not a subsequence
141+
* of the text
142+
*/
143+
public static int score(String text, String filter) {
144+
String needle = filter.toLowerCase().replaceAll("[\\s*?]", ""); //$NON-NLS-1$ //$NON-NLS-2$
145+
if (needle.isEmpty()) {
146+
return 0;
147+
}
148+
int score = 0;
149+
int firstMatch = -1;
150+
int prevMatch = -2;
151+
int j = 0;
152+
for (int i = 0; i < text.length() && j < needle.length(); i++) {
153+
if (Character.toLowerCase(text.charAt(i)) != needle.charAt(j)) {
154+
continue;
155+
}
156+
if (firstMatch < 0) {
157+
firstMatch = i;
158+
}
159+
int charScore = MATCH_BASE;
160+
if (isBoundary(text, i)) {
161+
charScore += BOUNDARY_BONUS;
162+
}
163+
if (prevMatch == i - 1) {
164+
charScore += CONSECUTIVE_BONUS;
165+
}
166+
score += charScore;
167+
prevMatch = i;
168+
j++;
169+
}
170+
if (j < needle.length()) {
171+
return SCORE_NONE;
172+
}
173+
if (firstMatch == 0) {
174+
score += PREFIX_BONUS;
175+
}
176+
score -= Math.min(firstMatch, MAX_LEADING_GAP) * LEADING_GAP_PENALTY;
177+
score -= Math.min(text.length(), MAX_LENGTH_PENALTY);
178+
return score;
179+
}
180+
181+
private static boolean isBoundary(String text, int i) {
182+
if (i == 0) {
183+
return true;
184+
}
185+
char prev = text.charAt(i - 1);
186+
char cur = text.charAt(i);
187+
if (!Character.isLetterOrDigit(prev)) {
188+
return true;
189+
}
190+
if (Character.isUpperCase(cur) && !Character.isUpperCase(prev)) {
191+
return true;
192+
}
193+
return Character.isDigit(cur) && !Character.isDigit(prev);
194+
}
117195
}

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,4 +109,52 @@ public void safeCompileCompilesValidRegex() {
109109
Pattern p = QuickAccessMatching.safeCompile("foo.*");
110110
assertTrue(p.matcher("foobar").matches());
111111
}
112+
113+
@Test
114+
public void scoreReturnsNoneWhenNotSubsequence() {
115+
assertEquals(QuickAccessMatching.SCORE_NONE, QuickAccessMatching.score("Rename", "xyz"));
116+
}
117+
118+
@Test
119+
public void scoreReturnsZeroForEmptyFilter() {
120+
assertEquals(0, QuickAccessMatching.score("Rename", ""));
121+
}
122+
123+
@Test
124+
public void scorePrefersConsecutiveOverScattered() {
125+
int consecutive = QuickAccessMatching.score("abcxyz", "abc");
126+
int scattered = QuickAccessMatching.score("axbxcx", "abc");
127+
assertTrue(consecutive > scattered, "consecutive " + consecutive + " should beat scattered " + scattered);
128+
}
129+
130+
@Test
131+
public void scorePrefersPrefixOverMidWord() {
132+
int prefix = QuickAccessMatching.score("Rename", "re");
133+
int midWord = QuickAccessMatching.score("Score", "re");
134+
assertTrue(prefix > midWord, "prefix " + prefix + " should beat mid-word " + midWord);
135+
}
136+
137+
@Test
138+
public void scorePrefersShorterOnOtherwiseEqualMatch() {
139+
int shorter = QuickAccessMatching.score("Re", "re");
140+
int longer = QuickAccessMatching.score("Renew", "re");
141+
assertTrue(shorter > longer, "shorter " + shorter + " should beat longer " + longer);
142+
}
143+
144+
@Test
145+
public void scoreRewardsWordBoundaryInitials() {
146+
int initials = QuickAccessMatching.score("New File", "nf");
147+
int midWord = QuickAccessMatching.score("Confirm", "nf");
148+
assertTrue(initials > midWord, "word-initial " + initials + " should beat mid-word " + midWord);
149+
}
150+
151+
@Test
152+
public void scoreIgnoresWildcardChars() {
153+
assertTrue(QuickAccessMatching.score("Rename Resource", "re*ce") > QuickAccessMatching.SCORE_NONE);
154+
}
155+
156+
@Test
157+
public void scoreIsCaseInsensitive() {
158+
assertTrue(QuickAccessMatching.score("RENAME", "rename") > QuickAccessMatching.SCORE_NONE);
159+
}
112160
}

0 commit comments

Comments
 (0)