From fc4074b251215ce7dbeb644865412b2dfa108209 Mon Sep 17 00:00:00 2001 From: Andrey Loskutov Date: Tue, 24 Sep 2024 11:20:29 +0200 Subject: [PATCH 1/2] File search: show "results filtered" message also in the tree mode This change unifies "filter" label handling for tree/table mode in the file search, we show now that the matches are filtered in both cases. See https://github.com/eclipse-platform/eclipse.platform.ui/issues/2279 --- .../ui/text/AbstractTextSearchResult.java | 12 +++ .../internal/ui/text/FileSearchPage.java | 76 +++++++++---------- .../ui/text/FileTableContentProvider.java | 17 +++-- .../ui/text/FileTreeContentProvider.java | 19 +++++ .../ui/text/IFileSearchContentProvider.java | 7 ++ 5 files changed, 86 insertions(+), 45 deletions(-) diff --git a/bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java b/bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java index f666fbf224b..bd9ec3711b6 100644 --- a/bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java +++ b/bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java @@ -351,6 +351,18 @@ public Object[] getElements() { return fElementsToMatches.keySet().toArray(); } + /** + * Returns a number of all elements that have matches. Note that count of + * all elements that contain matches are returned. The filter state of the + * matches is not relevant. + * + * @return the number of elements in this search result + * @since 3.17 + */ + public int getElementsCount() { + return fElementsToMatches.size(); + } + /** * Sets the active match filters for this result. If set to non-null, the match filters will be used to update the filter * state ({@link Match#isFiltered()} of matches and the {@link AbstractTextSearchViewPage} will only diff --git a/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileSearchPage.java b/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileSearchPage.java index 228048b18e0..0c7e04c2dd7 100644 --- a/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileSearchPage.java +++ b/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileSearchPage.java @@ -25,14 +25,24 @@ import java.util.Objects; import java.util.Set; +import org.eclipse.swt.dnd.DND; +import org.eclipse.swt.dnd.Transfer; +import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Item; +import org.eclipse.swt.widgets.Table; +import org.eclipse.swt.widgets.Tree; + +import org.eclipse.core.runtime.IAdaptable; + +import org.eclipse.core.resources.IContainer; +import org.eclipse.core.resources.IFile; +import org.eclipse.core.resources.IResource; + import org.eclipse.core.filebuffers.FileBuffers; import org.eclipse.core.filebuffers.ITextFileBuffer; import org.eclipse.core.filebuffers.ITextFileBufferManager; import org.eclipse.core.filebuffers.LocationKind; -import org.eclipse.core.resources.IContainer; -import org.eclipse.core.resources.IFile; -import org.eclipse.core.resources.IResource; -import org.eclipse.core.runtime.IAdaptable; + import org.eclipse.jface.action.IMenuManager; import org.eclipse.jface.action.MenuManager; import org.eclipse.jface.dialogs.ErrorDialog; @@ -50,21 +60,7 @@ import org.eclipse.jface.viewers.TreeViewer; import org.eclipse.jface.viewers.Viewer; import org.eclipse.jface.viewers.ViewerComparator; -import org.eclipse.search.internal.ui.Messages; -import org.eclipse.search.internal.ui.SearchMessages; -import org.eclipse.search.ui.IContextMenuConstants; -import org.eclipse.search.ui.ISearchResultViewPart; -import org.eclipse.search.ui.NewSearchUI; -import org.eclipse.search.ui.text.AbstractTextSearchResult; -import org.eclipse.search.ui.text.AbstractTextSearchViewPage; -import org.eclipse.search.ui.text.Match; -import org.eclipse.search2.internal.ui.OpenSearchPreferencesAction; -import org.eclipse.swt.dnd.DND; -import org.eclipse.swt.dnd.Transfer; -import org.eclipse.swt.widgets.Display; -import org.eclipse.swt.widgets.Item; -import org.eclipse.swt.widgets.Table; -import org.eclipse.swt.widgets.Tree; + import org.eclipse.ui.IMemento; import org.eclipse.ui.IPageLayout; import org.eclipse.ui.IWorkbenchPage; @@ -77,6 +73,17 @@ import org.eclipse.ui.part.ResourceTransfer; import org.eclipse.ui.part.ShowInContext; +import org.eclipse.search.internal.ui.Messages; +import org.eclipse.search.internal.ui.SearchMessages; +import org.eclipse.search.ui.IContextMenuConstants; +import org.eclipse.search.ui.ISearchResultViewPart; +import org.eclipse.search.ui.NewSearchUI; +import org.eclipse.search.ui.text.AbstractTextSearchResult; +import org.eclipse.search.ui.text.AbstractTextSearchViewPage; +import org.eclipse.search.ui.text.Match; + +import org.eclipse.search2.internal.ui.OpenSearchPreferencesAction; + public class FileSearchPage extends AbstractTextSearchViewPage implements IAdaptable { @@ -432,29 +439,22 @@ private boolean isQueryRunning() { @Override public String getLabel() { String label= super.getLabel(); - StructuredViewer viewer= getViewer(); AbstractTextSearchResult result = getInput(); String msg = label; if (result != null) { - if (viewer instanceof TableViewer) { - TableViewer tv = (TableViewer) viewer; - - int itemCount = ((FileTableContentProvider) tv.getContentProvider()) - .getUnfilteredElements(getInput()).length; - if (showLineMatches()) { - int matchCount= getInput().getMatchCount(); - if (itemCount < matchCount) { - msg = Messages.format(SearchMessages.FileSearchPage_limited_format_matches, - new Object[] { label, Integer.valueOf(itemCount), Integer.valueOf(matchCount) }); - } - } else { - int fileCount= getInput().getElements().length; - if (itemCount < fileCount) { - msg = Messages.format(SearchMessages.FileSearchPage_limited_format_files, - new Object[] { label, Integer.valueOf(itemCount), Integer.valueOf(fileCount) }); - } + int itemCount = fContentProvider.getLeafCount(result); + if (showLineMatches()) { + int matchCount = result.getMatchCount(); + if (itemCount < matchCount) { + msg = Messages.format(SearchMessages.FileSearchPage_limited_format_matches, + new Object[] { label, Integer.valueOf(itemCount), Integer.valueOf(matchCount) }); + } + } else { + int fileCount = result.getElementsCount(); + if (itemCount < fileCount) { + msg = Messages.format(SearchMessages.FileSearchPage_limited_format_files, + new Object[] { label, Integer.valueOf(itemCount), Integer.valueOf(fileCount) }); } - } if (result.getActiveMatchFilters() != null && result.getActiveMatchFilters().length > 0) { if (isQueryRunning()) { diff --git a/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileTableContentProvider.java b/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileTableContentProvider.java index c93c89c97d8..d00767127aa 100644 --- a/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileTableContentProvider.java +++ b/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileTableContentProvider.java @@ -20,6 +20,7 @@ import org.eclipse.jface.viewers.IStructuredContentProvider; import org.eclipse.jface.viewers.TableViewer; import org.eclipse.jface.viewers.Viewer; + import org.eclipse.search.ui.text.AbstractTextSearchResult; public class FileTableContentProvider implements IStructuredContentProvider, IFileSearchContentProvider { @@ -37,15 +38,17 @@ public void dispose() { // nothing to do } - public Object[] getUnfilteredElements(AbstractTextSearchResult searchResult) { + @Override + public int getLeafCount(Object parentElement) { + if (!(parentElement instanceof AbstractTextSearchResult searchResult)) { + return 0; + } int elementLimit = getElementLimit(); - Object[] elements = searchResult.getElements(); - if (elementLimit != -1 && elements.length > elementLimit) { - Object[] shownElements = new Object[elementLimit]; - System.arraycopy(elements, 0, shownElements, 0, elementLimit); - return shownElements; + int elementsCount = searchResult.getElementsCount(); + if (elementLimit != -1 && elementsCount > elementLimit) { + return elementLimit; } - return elements; + return elementsCount; } @Override diff --git a/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileTreeContentProvider.java b/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileTreeContentProvider.java index da6a211104b..1dfa6cf15c8 100644 --- a/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileTreeContentProvider.java +++ b/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileTreeContentProvider.java @@ -54,6 +54,7 @@ public class FileTreeContentProvider implements ITreeContentProvider, IFileSearc FileTreeContentProvider(FileSearchPage page, AbstractTreeViewer viewer) { fPage= page; fTreeViewer= viewer; + fChildrenMap = new HashMap<>(); } @Override @@ -218,6 +219,24 @@ public Object[] getChildren(Object parentElement) { return children.toArray(); } + @Override + public int getLeafCount(Object parentElement) { + Object[] children = getChildren(parentElement); + if (children.length == 0) { + return 0; + } + int count = 0; + for (Object object : children) { + boolean leaf = !hasChildren(object); + if (leaf) { + count++; + } else { + count += getLeafCount(object); + } + } + return count; + } + @Override public boolean hasChildren(Object element) { Set children = fChildrenMap.get(element); diff --git a/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/IFileSearchContentProvider.java b/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/IFileSearchContentProvider.java index 8c20a265f71..4404249285d 100644 --- a/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/IFileSearchContentProvider.java +++ b/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/IFileSearchContentProvider.java @@ -19,4 +19,11 @@ public interface IFileSearchContentProvider { public abstract void clear(); + /** + * @param parentElement + * parent element or input + * @return number of leaf elements in the tree maintained by the provider + */ + public abstract int getLeafCount(Object parentElement); + } \ No newline at end of file From ef3d2bf056bb3bc167d66a9d7e43c4bda498ab97 Mon Sep 17 00:00:00 2001 From: Andrey Loskutov Date: Tue, 24 Sep 2024 11:24:36 +0200 Subject: [PATCH 2/2] File search: cache getMatchCount() result to avoid UI hangs If the search view has matches for 100.000 files, it would hang forever on updating the view via F5 because it asks getMatchCount() for every element and that traverses entire result again and again. This change remembers last computed result and returns that value if there were no further changes on the search result itself. See https://github.com/eclipse-platform/eclipse.platform.ui/issues/2279 --- .../ui/text/AbstractTextSearchResult.java | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java b/bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java index bd9ec3711b6..8d01b6bb65b 100644 --- a/bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java +++ b/bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java @@ -25,6 +25,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.atomic.AtomicInteger; import org.eclipse.search.ui.ISearchResult; import org.eclipse.search.ui.ISearchResultListener; @@ -44,6 +45,7 @@ public abstract class AbstractTextSearchResult implements ISearchResult { private final ConcurrentMap> fElementsToMatches; private final List fListeners; private final MatchEvent fMatchEvent; + private final AtomicInteger matchCount; private MatchFilter[] fMatchFilters; @@ -54,7 +56,7 @@ protected AbstractTextSearchResult() { fElementsToMatches= new ConcurrentHashMap<>(); fListeners= new ArrayList<>(); fMatchEvent= new MatchEvent(this); - + matchCount = new AtomicInteger(0); fMatchFilters= null; // filtering disabled by default } @@ -156,6 +158,7 @@ private MatchEvent getSearchResultEvent(Collection matches, int eventKind } private boolean didAddMatch(Match match) { + matchCount.set(0); updateFilterState(match); return fElementsToMatches.computeIfAbsent(match.getElement(), k -> ConcurrentHashMap.newKeySet()).add(match); } @@ -178,6 +181,7 @@ public void removeAll() { fireChange(new RemoveAllEvent(this)); } private void doRemoveAll() { + matchCount.set(0); fElementsToMatches.clear(); } @@ -215,6 +219,7 @@ public void removeMatches(Match[] matches) { private boolean didRemoveMatch(Match match) { + matchCount.set(0); boolean[] existed = new boolean[1]; fElementsToMatches.computeIfPresent(match.getElement(), (f, matches) -> { existed[0] = matches.remove(match); @@ -302,11 +307,21 @@ private boolean updateFilterState(Match match) { * @return total number of matches */ public int getMatchCount() { - int count = 0; + final int oldCount = matchCount.get(); + if (oldCount != 0) { + return oldCount; + } + // The oldCount is zero here => we have to calculate again + int newCount = 0; for (Set element : fElementsToMatches.values()) { - count += element.size(); + newCount += element.size(); + } + if (matchCount.compareAndSet(0, newCount)) { + // Only return if not changed meanwhile + return newCount; } - return count; + // Changed once again, fetch again latest value + return getMatchCount(); } /**