From dd80ce4454e76b2a245cb89b0e90cb5553197258 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Fri, 3 Jul 2026 12:20:30 +0200 Subject: [PATCH] Cap Quick Access providers at a fair share of the visible slots Since the continuous relevance ranking (86a07a08b3) 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 https://github.com/eclipse-platform/eclipse.platform.ui/issues/4155 --- .../quickaccess/QuickAccessContents.java | 6 +-- .../quickaccess/QuickAccessMatching.java | 40 +++++++++++++++++ .../quickaccess/QuickAccessMatchingTest.java | 43 +++++++++++++++++++ 3 files changed, 86 insertions(+), 3 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 65747387fec..1c407d2454d 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 @@ -574,8 +574,7 @@ private List[] 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 matched = new ArrayList<>(); for (Entry> elementsPerProvider : elementsForProviders .entrySet()) { @@ -595,7 +594,8 @@ private List[] assembleEntries( } matched.sort(BY_RELEVANCE); int slots = Math.max(0, numberOfSlotsLeft); - List winners = matched.subList(0, Math.min(slots, matched.size())); + // a prolific provider (e.g. workspace files) must not flood out the others + List 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()) { 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 7be4d95a0b5..2cb3bff3d67 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 @@ -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 List pickFairly(List sortedByRelevance, Function 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 winners = new ArrayList<>(Math.max(0, slots)); + List overflow = new ArrayList<>(); + Map 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 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; 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 c6b7c33f447..3b4626eb9ef 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 @@ -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 BY_PREFIX = s -> s.replaceAll("\\d", ""); + + @Test + public void pickFairlyReturnsAllWhenEnoughSlots() { + List 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 ranked = List.of("file1", "file2", "file3", "file4", "file5", "view1", "command1"); + List 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 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 ranked = List.of("view1", "file1", "file2", "file3", "command1"); + List winners = QuickAccessMatching.pickFairly(ranked, BY_PREFIX, 3); + assertEquals(List.of("view1", "file1", "command1"), winners); + } + + @Test + public void pickFairlyHandlesZeroSlots() { + List ranked = List.of("file1", "file2"); + assertTrue(QuickAccessMatching.pickFairly(ranked, BY_PREFIX, 0).isEmpty()); + } }