Cap Quick Access providers at a fair share of the visible slots

Since the continuous relevance ranking (86a07a0) fills the limited
table with the globally highest-scored entries, a single prolific
provider can flood out all other categories: typing "Error" lists
workspace files instead of the Error Log view, because short file
names outscore the category-suffixed view label.

Keep the relevance ranking, but cap each provider at its fair share of
the visible slots and give slots a provider does not use to the best
remaining entries, so strong matches from every category stay visible.

Fixes #4155

Author: vogella

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

@@ -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()) {
@@ -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()) {

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

@@ -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;
 
 /**
@@ -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;

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

@@ -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;
@@ -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());
+	}
 }