Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -574,8 +574,7 @@ private List<QuickAccessEntry>[] assembleEntries(
}
} else {
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
// Score every candidate so the most relevant entries compete for the slots
List<QuickAccessEntry> matched = new ArrayList<>();
for (Entry<QuickAccessProvider, List<QuickAccessElement>> elementsPerProvider : elementsForProviders
.entrySet()) {
Expand All @@ -595,7 +594,8 @@ private List<QuickAccessEntry>[] assembleEntries(
}
matched.sort(BY_RELEVANCE);
int slots = Math.max(0, numberOfSlotsLeft);
List<QuickAccessEntry> winners = matched.subList(0, Math.min(slots, matched.size()));
// a prolific provider (e.g. workspace files) must not flood out the others
List<QuickAccessEntry> winners = QuickAccessMatching.pickFairly(matched, entry -> entry.provider, slots);
// 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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@

package org.eclipse.ui.internal.quickaccess;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.regex.Pattern;

/**
Expand Down Expand Up @@ -178,6 +184,40 @@ public static int score(String text, String filter) {
return score;
}

/**
* Picks up to {@code slots} items from a relevance-sorted list, capping each
* group at its fair share of the slots so a single prolific group cannot flood
* out all the others. Slots a group does not use go to the best remaining
* items regardless of group.
*/
public static <T> List<T> pickFairly(List<T> sortedByRelevance, Function<T, Object> groupOf, int slots) {
if (sortedByRelevance.size() <= slots) {
return sortedByRelevance;
}
long groupCount = sortedByRelevance.stream().map(groupOf).distinct().count();
int fairShare = Math.max(1, slots / (int) groupCount);
List<T> winners = new ArrayList<>(Math.max(0, slots));
List<T> overflow = new ArrayList<>();
Map<Object, Integer> taken = new HashMap<>();
for (T item : sortedByRelevance) {
if (winners.size() >= slots) {
return winners;
}
Object group = groupOf.apply(item);
int alreadyTaken = taken.getOrDefault(group, 0);
if (alreadyTaken < fairShare) {
winners.add(item);
taken.put(group, alreadyTaken + 1);
} else {
overflow.add(item);
}
}
for (Iterator<T> it = overflow.iterator(); winners.size() < slots && it.hasNext();) {
winners.add(it.next());
}
return winners;
}

private static boolean isBoundary(String text, int i) {
if (i == 0) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.util.List;
import java.util.function.Function;
import java.util.regex.Pattern;

import org.eclipse.ui.internal.quickaccess.QuickAccessEntry;
Expand Down Expand Up @@ -157,4 +159,45 @@ public void scoreIgnoresWildcardChars() {
public void scoreIsCaseInsensitive() {
assertTrue(QuickAccessMatching.score("RENAME", "rename") > QuickAccessMatching.SCORE_NONE);
}

/** Groups items by their prefix up to the first digit, e.g. "file1" -> "file". */
private static final Function<String, Object> BY_PREFIX = s -> s.replaceAll("\\d", "");

@Test
public void pickFairlyReturnsAllWhenEnoughSlots() {
List<String> ranked = List.of("file1", "file2", "view1");
assertEquals(ranked, QuickAccessMatching.pickFairly(ranked, BY_PREFIX, 5));
}

@Test
public void pickFairlyCapsFloodingGroup() {
// files dominate the top of the relevance ranking, see issue #4155
List<String> ranked = List.of("file1", "file2", "file3", "file4", "file5", "view1", "command1");
List<String> winners = QuickAccessMatching.pickFairly(ranked, BY_PREFIX, 4);
assertEquals(4, winners.size());
assertTrue(winners.contains("view1"), "flooded-out view should win a slot: " + winners);
assertTrue(winners.contains("command1"), "flooded-out command should win a slot: " + winners);
}

@Test
public void pickFairlyGivesUnusedShareToBestRemaining() {
List<String> ranked = List.of("file1", "file2", "file3", "file4", "view1");
// 2 groups, 4 slots -> fair share is 2 each; the view only needs 1,
// so the leftover slot goes to the next best file
assertEquals(List.of("file1", "file2", "view1", "file3"),
QuickAccessMatching.pickFairly(ranked, BY_PREFIX, 4));
}

@Test
public void pickFairlyKeepsRelevanceOrderWithinTheCap() {
List<String> ranked = List.of("view1", "file1", "file2", "file3", "command1");
List<String> winners = QuickAccessMatching.pickFairly(ranked, BY_PREFIX, 3);
assertEquals(List.of("view1", "file1", "command1"), winners);
}

@Test
public void pickFairlyHandlesZeroSlots() {
List<String> ranked = List.of("file1", "file2");
assertTrue(QuickAccessMatching.pickFairly(ranked, BY_PREFIX, 0).isEmpty());
}
}
Loading