Bugzilla – Full Text Bug Listing
| Summary: | [open type] Open Type - exact match should be listed on top | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | David Corbin <dcorbin> |
| Component: | UI | Assignee: | JDT-UI-Inbox <jdt-ui-inbox> |
| Status: | CLOSED DUPLICATE | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | akurtakov, bsd, daniel_megert, gautier.desaintmartinlacaze, hudsonr, jan, kdevolder, Lars.Vogel, lbullen, lukas.eder, markus.kell.r, noopur_gupta, robert.roth.off, stephan.herrmann |
| Version: | 3.1 | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: |
https://git.eclipse.org/r/73255 https://bugs.eclipse.org/bugs/show_bug.cgi?id=494316 https://git.eclipse.org/r/87929 https://bugs.eclipse.org/bugs/show_bug.cgi?id=525974 https://bugs.eclipse.org/bugs/show_bug.cgi?id=531332 |
||
| Whiteboard: | |||
| Bug Depends on: | 360468 | ||
| Bug Blocks: | |||
|
Description
David Corbin
dirk, please comment. The problem occurs when the Foo is part of the workspace match and FooXY is in the history. We decided to always have the history element first. If the user wants to narrow down the list 'Foo ' will take him to Foo. Would it be feasible (and a is it a good idea) to simply select 'Foo', but leave it where it is at the top of the 'non-history section'? We tried this as well but rejected it since the element might not be visible (to many histroy elements before). And this might even be more surprising for the user. Since I don't see a solution which honors all constraints I opt to leave the behaviour as is (since clients got used to it now). This might change if we rethink the overall filtering strategy of the dialog. Tagging as won't fix for now. I don't think WONTFIX is a good solution. Example: in a JDT workspace try to open type "IType". Even after typing the exact simple name "IType" you still have to manually search among three pages(!) of proposals (If IType is proposed from the history, remove that entry first). If exact match (of simple name) is not ranked first (which I'd prefer) then we'd need at least a gesture to indicate "I mean it, don't try to be smarter, proposing things I didn't say". Appending an extra blank is a well-hidden improvement, it narrows down the list to 22 proposals, the very last of which is the only one I'm interested in. Currently, the best pattern is "*IType " (obscure enough), which still has two matches: IType and JDIType. My proposal: sort exact match of simple name before all others, only then start the alphabetic list. *** Bug 98562 has been marked as a duplicate of this bug. *** I would love to see this improvement. The current behaviour is a bit annoying when you have lots of types with short names. For instance, in jOOQ, we have a very commonly used type called org.jooq.impl.DSL. I want to find it by typing just "DSL". However, the first matches (outside of the history) include things like: - DataSourceLookup - DataSourceLookupFailureException - DocSpecLike - DragSourceListener I believe that the exact match "org.jooq.impl.DSL" should always match first, before any approximate matches. Comment 5 and comment 7 are not about this bug, but rather about bug 387126 / bug 360468. *** Bug 387126 has been marked as a duplicate of this bug. *** *** Bug 394919 has been marked as a duplicate of this bug. *** New Gerrit change created: https://git.eclipse.org/r/73255 We have two cases here: 1. Exact matches should be listed before history matches. 2. Exact matches should be listed before camel case matches. For fixing the first case, see the problem mentioned in bug 98562 comment #1: > The underlying problem is that the history is shown before I have the list > from > the search engine. Either I have to wait until I have all matches (which will > slow down access to matches in histroy) or it will result in reordering the > result when the search results are available. This might get very confusing > for > the user. We can continue to show the history first, followed by the separator, and then the regular matches. In each section, we can show the exact matches before the camel case matches. Patch set 1 implements this behavior. Please try and share your feedback. Also, suggest a possible solution for the first case if it needs to be fixed. (In reply to Noopur Gupta from comment #12) > We can continue to show the history first, followed by the separator, and > then the regular matches. In each section, we can show the exact matches > before the camel case matches. +1. *** Bug 495740 has been marked as a duplicate of this bug. *** Just an FYI. My bug just got closed as a duplicate (which seems about right :-) However my example in that bug has nothing to do with history. There are dozens of type listed before the exact match and they definitely do not come from any history. The just happen to be 'camel-cases' matches to my query. So just to be aware... if someone actually thinks about fixing it. Its not just history matches that are the problem (if it were it would be far less annoying since the list isn't likely to be as large as in my example). Ah... I should have read a bit more before posting my comment. Noopur already mentioned the camel-case case.
> We can continue to show the history first, followed by the separator, and
> then the regular matches. In each section, we can show the exact matches
> before the camel case matches.
+1
The more annoying thing is having dozens of camel-case matches in front.
The history case might be good to fix as well, but because that may be harder to fix, and because it is the lesser annoying problem of the two, it shouldn't block fixing of the camel-case case.
@Noopur: I have tested the patch, and it indeed is an improvement. History shown first (with the same ordering as the part after the separator), separator, exact matches, prefix matches and camelcase matches. @Noopur, is the patch ready to ship? Otherwise I'd offer any necessary help, since this keeps bugging me time and again :) I have installed this patch in my workspace and with that, I have seen the following exception sometimes while using the Open Type dialog: java.lang.IllegalArgumentException: Comparison method violates its general contract! at java.util.TimSort.mergeHi(Unknown Source) at java.util.TimSort.mergeAt(Unknown Source) at java.util.TimSort.mergeCollapse(Unknown Source) at java.util.TimSort.sort(Unknown Source) at java.util.Arrays.sort(Unknown Source) at java.util.ArrayList.sort(Unknown Source) at java.util.Collections$SynchronizedList.sort(Unknown Source) at java.util.Collections.sort(Unknown Source) at org.eclipse.ui.dialogs.FilteredItemsSelectionDialog$ContentProvider.getSortedItems(FilteredItemsSelectionDialog.java:2752) at org.eclipse.ui.dialogs.FilteredItemsSelectionDialog$ContentProvider.rememberResult(FilteredItemsSelectionDialog.java:2765) at org.eclipse.ui.dialogs.FilteredItemsSelectionDialog$FilterJob.filterContent(FilteredItemsSelectionDialog.java:2102) at org.eclipse.ui.dialogs.FilteredItemsSelectionDialog$FilterJob.internalRun(FilteredItemsSelectionDialog.java:2046) at org.eclipse.ui.dialogs.FilteredItemsSelectionDialog$FilterJob.doRun(FilteredItemsSelectionDialog.java:2018) at org.eclipse.ui.dialogs.FilteredItemsSelectionDialog$FilterJob.run(FilteredItemsSelectionDialog.java:2005) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:56) It cannot be reproduced by trying the same thing immediately after the exception and I haven't been able to get any steps to reproduce it. Not sure if code review can detect any obvious problem in the patch regarding this. Using -Djava.util.Arrays.useLegacyMergeSort=true does not give the exception. The problem in the patch could be related to accessing the filter pattern in the #compare method. See bug 295920 also. (In reply to Noopur Gupta from comment #19) > I have installed this patch in my workspace and with that, I have seen the > following exception sometimes while using the Open Type dialog: What do you mean by "sometimes", i.e. how often does it happen? I suggest you add debug information when the exception happens. See org.eclipse.jface.viewers.ViewerComparator.sort(Viewer, Object[]) for details. (In reply to Dani Megert from comment #21) > (In reply to Noopur Gupta from comment #19) > > I have installed this patch in my workspace and with that, I have seen the > > following exception sometimes while using the Open Type dialog: > > What do you mean by "sometimes", i.e. how often does it happen? With normal usage, it happens once in 20-25 times. But when I try to reproduce it continuously with added sysouts in FilteredTypesSelectionDialog.TypeItemsComparator.compare(..), the frequency increases to once in 10 times. These sysouts seem to have a big impact on the frequency. It could be that the filter pattern changes quickly between the #compare calls which results in this issue. > I suggest you add debug information when the exception happens. See > org.eclipse.jface.viewers.ViewerComparator.sort(Viewer, Object[]) for > details. The sysouts added in this method or the lambda body are never printed. (In reply to Noopur Gupta from comment #22) > > I suggest you add debug information when the exception happens. See > > org.eclipse.jface.viewers.ViewerComparator.sort(Viewer, Object[]) for > > details. > > The sysouts added in this method or the lambda body are never printed. Not sure what you mean. AFAIK in our case HistoryComparator is used, which is not related to the ViewerComparator. New Gerrit change created: https://git.eclipse.org/r/87929 (In reply to Noopur Gupta from comment #22) > These sysouts seem to have a big impact on the frequency. It could be that > the filter pattern changes quickly between the #compare calls which results > in this issue. The pattern has to be stable when a particular sort is being performed. Otherwise, we get this exception. But there is no notification from the platform regarding the start and end of a sort operation. So, we don't know when to discard the current pattern and use the new one at the client side. I have just added two methods to FilteredItemsSelectionDialog: #sortStarted and #sortEnded, which are called for every sort operation. Based on this, I am updating the filter pattern in jdt.ui. This approach works fine and resolves the issue. Please have a look at this approach and suggest a better location for these notification methods. (In reply to Noopur Gupta from comment #25) > I have just added two methods to FilteredItemsSelectionDialog: #sortStarted > and #sortEnded, which are called for every sort operation. Based on this, I > am updating the filter pattern in jdt.ui. This approach works fine and > resolves the issue. > > Please have a look at this approach and suggest a better location for these > notification methods. I only took a quick look. I'm not sure this is the right fix. It can reduce the chance to hit the bug, but I think it can still occur because whenever the user modifies the pattern FilteredTypesSelectionDialog#createFilter() creates a new filter and sets it to fFilter. Meanwhile jobs that got cancelled still continue to run on the old data. How about resetting the filter in FilteredTypesSelectionDialog.reloadCache(boolean, IProgressMonitor)? The pattern handling during sort should be done in Platform itself and not in clients. Moving to 4.8. *** Bug 526386 has been marked as a duplicate of this bug. *** Fixed by fix for bug 525974. *** This bug has been marked as a duplicate of bug 525974 *** |