Community
Participate
Working Groups
If I have to search for the usage of a class or it's inherited classes I need to run the search multiple times with 1 class as the search input. There should be a way to search a class and it's inherited classes in a single step.
Sarika, would you like to work on this?
(In reply to Dani Megert from comment #1) > Sarika, would you like to work on this? Yes, Will take it up after Bug 84916.
This is related to bug 368527, which requests searches for multiple elements (based on arbitrary user selection, not on a computed type hierarchy). Please clarify the request. "Inherited classes" would mean nested classes that are inherited by a subclass. That's probably not what you meant. Or do you actually want to search for subtypes? E.g. when searching for references to AbstractList, also search for ArrayList, Vector, etc.?
Bug 26752 requests multi-selection in the Type Hierarchy tree. That and bug 368527 together would allow this functionality without requiring any new UI.
(In reply to Markus Keller from comment #3) > This is related to bug 368527, which requests searches for multiple elements > (based on arbitrary user selection, not on a computed type hierarchy). > > Please clarify the request. "Inherited classes" would mean nested classes > that are inherited by a subclass. That's probably not what you meant. > > Or do you actually want to search for subtypes? E.g. when searching for > references to AbstractList, also search for ArrayList, Vector, etc.? Yes the aim of this bug was to request for when searching for references to AbstractList, also search for ArrayList, Vector, etc
Created attachment 259756 [details] Multi selection search This patch enables multi selection search from Hierarchy View, Outline View, Package explorer View and Search View. Open question - How should multi selection for References with Hierarchy scope work ? Right now it is taking the default Workspace Scope. 1. Should we disable the Hierarchy option for multi select? 2. Any other better way to handle it ?
(In reply to Sarika Sinha from comment #6) > Created attachment 259756 [details] > Multi selection search > > This patch enables multi selection search from Hierarchy View, Outline View, > Package explorer View and Search View. > > Open question - How should multi selection for References with Hierarchy > scope work ? Right now it is taking the default Workspace Scope. Hierarchy scope for multiple types may not make much sense if they themselves are not in hieararchy. Of course artificially this scope can be made as union of the two but then what would be the use case?
Created attachment 260379 [details] Multi selection search without adding any new Query Specification Based on Markus suggestion on Bug 487291, I have removed the addition of new QuerySpecification, But have added a new constructor to set multiple selected elements. Also a new method is added to get the selected multiple elements. This will not impact the clients until they use the new API to set multi selected elements. Facing issues with gerrit patch creation for JDT UI so added the patch.
(In reply to Sarika Sinha from comment #8) This affects existing clients that implement IQueryParticipant. They expect that ElementQuerySpecification#getElement() returns the element they should look for, and they would disregard any additional elements in ElementQuerySpecification#getElements(). To avoid such breakages, the JavaSearchQuery should call participant.search(..) multiple times for the same participant, and pass separate ElementQuerySpecifications to cover all the elements we're looking for. If participants need access to all elements at once (e.g. to improve performance), then we could add an optional extension interface for IQueryParticipant. But we should only consider that if we get requests from actual clients. Please format new code in JDT UI style. E.g. if-else statements always use blocks, and there spaces between keywords and bracketing characters, i.e. not "}else{".
Created attachment 260661 [details] Multi selection search with multiple search This patch supports more that one QuerySpecification for JavaSearchQuery. So now, JavaSearchQuery invokes search for each data and search filters accordingly process all the QuerySpecifications.
(In reply to Sarika Sinha from comment #10) > Created attachment 260661 [details] [diff] > Multi selection search with multiple search I didn't look at the code yet. On trying the patch, found these issues: - Space required before and after 'and' in the message string to be displayed in Search view. - In this example, select 'ArrayList' and press Ctrl+Shift+G. In Search view's filters, "In Imports" option is missing after the patch. import java.util.ArrayList; public class C { ArrayList<String> a; }
Created attachment 260757 [details] Multi selection search Thanks Noopur !!
Created attachment 260774 [details] Patch Attaching the refactored patch. There is still a problem with "right-click on an element in a view > References > any Working Set from history". The scope/limitTo is not set correctly and results in search in the entire workspace with all the limitTo flags. This also exposes a CCE in jdt.core when invoked on "ArrayList" element in a view. Need to check which limitTo flag results in CCE to get a reproducible scenario without this patch. java.lang.ClassCastException: org.eclipse.jdt.internal.compiler.ast.IntersectionCastTypeReference cannot be cast to org.eclipse.jdt.internal.compiler.ast.QualifiedTypeReference at org.eclipse.jdt.internal.core.search.matching.TypeReferenceLocator.match(TypeReferenceLocator.java:99) at org.eclipse.jdt.internal.core.search.matching.MatchLocatorParser.consumeCastExpressionWithGenericsArray(MatchLocatorParser.java:232) ...
The problem is that org.eclipse.jdt.ui.actions.WorkingSetFindAction.getLimitTo() returns -1 always. So there is no limit defined for WorkingSetFindAction. I think this should actually return fAction.getLimitTo() as fAction will have correct limit defined.
(In reply to Sarika Sinha from comment #14) > The problem is that > org.eclipse.jdt.ui.actions.WorkingSetFindAction.getLimitTo() returns -1 > always. So there is no limit defined for WorkingSetFindAction. > > I think this should actually return fAction.getLimitTo() as fAction will > have correct limit defined. We should also check how it was working before the patch using the same return value.
New Gerrit change created: https://git.eclipse.org/r/70532
Previously there was no support for multi select, so it was working fine. To solve this I had to introduce an new public method run with List of IJavaElement as the input. After this is delegated from WorkingSetFindAction, it works fine. After that I found another issue for Find References/Declaration/Implementors in Working Set, pops up multiple Working Set selection dialog for multi select find. This has been handled in FindReferencesInWorkingSetAction/ FindDeclarationsInWorkingSetAction/ FindImplementorsInWorkingSetAction The attached gerrit patch takes care of both the cases.
(In reply to Eclipse Genie from comment #16) > New Gerrit change created: https://git.eclipse.org/r/70532 Please use one Gerrit per bug. Bug 26752 is about allowing multi-select in the type hierarchy, and bug 26752 comment 7 explains why SWT.MULTI is not enough. Bug 368527 is for search on multi-selections. The commit message in the Gerrit should refer to that bug. And it should use the correct bug summary, not a wrongly cut version. Re org.eclipse.jdt.ui.actions.FindAction#run(List<IJavaElement>): In the past, we Eclipse APIs just used arrays, i.e. run(IJavaElement[]). I'd prefer to keep it that way unless there's a good reason to start something new. If we used a collection, the argument type should be the most general possible type, i.e. run(Collection<? extends IJavaElement>). Please use the enhanced for loop in new code. The code in JavaSearchQuery#getSearchPatternDescription() is not properly NLS'd. String concatenation is a no-go. All messages need to be composed using Messages.format(..). Since these strings tend to become very long, we can't always print all elements. I guess the best solution would be to just show the first selected element, e.g. like this: "Multiple elements, including '<first element label>'"
Please fix the name of JavaSearchQuery#getPatternData(). It only returns the first QuerySpecification, so use a name like getFirstSpecification(). And to ensure it can always return something, the constructor should ensure that the list size >= 1
(In reply to Markus Keller from comment #19) And that will also require follow-up fixes in the new API method and in callers of that method.
(In reply to Markus Keller from comment #20) > (In reply to Markus Keller from comment #19) > And that will also require follow-up fixes in the new API method and in > callers of that method. The constructor will make sure that the list is not empty and for new API run, canOperateOn actually takes care that if the selected list is empty, it returns false. Do you mean we should still add checks in the new run that id the array is coming as empty ?
Have incorporated the review comments. 1. Change in description 2. Change the input to Array 3. Change to getFirstSpecification() and make sure the list is not empty, added IllegalArgumentException in the new API 4. Commit message changed 5. Removed the MULTI select for hierarchy from this patch
(In reply to Sarika Sinha from comment #17) > The attached gerrit patch takes care of both the cases. There are still problems with this in patch set 4: Scenario 1: - Select two elements and invoke References > Working Set... - In the 'Select Working Sets' dialog, select a working set. - Repeat the above two steps thrice (not necessarily with same elements and working set). => No 'Select Working Sets' dialog is shown from the third trial onwards. Scenario 2: - Select two elements and invoke References > Working Set... - In the 'Select Working Sets' dialog, select the radio button 'No Working Sets' and click OK. => Issue: The 'Select Working Sets' dialog pops up again. => If you select a working set this time, it is ignored for this search. But is assumed to be the default working set on invoking References > Working Set... the next time, without showing the dialog even once. (In reply to Sarika Sinha from comment #22) > Have incorporated the review comments. > 4. Commit message changed This one is missed: (In reply to Markus Keller from comment #18) > Bug 368527 is for search on multi-selections. The commit message in the > Gerrit should refer to that bug.
The first problem was that I assumed FindAction is created for every Find Action performed by user, but it is not so. I have added the rest of processedElementIndex in run. Also a check before popping the working set selection dialog solves the second problem. I can see the complete commit message in gerrit, so not sure which one is being referred to?
(In reply to Sarika Sinha from comment #24) > Also a check before popping the working set selection dialog solves the > second problem. The check should be added in FindImplementorsInWorkingSetAction also. > I can see the complete commit message in gerrit, so not sure which one is > being referred to? >> Bug 368527 is for search on multi-selections. The commit message in the >> Gerrit should refer to that bug. I don't see a reference to bug 368527 in the commit message. Also, the following scenario results in NPE: public class C1 { class C2 { } } - Select C1.java in Package explorer > right-click > Refernces > Workspace. - In the 'Search' dialog, press Cancel. Error log has this NPE: java.lang.NullPointerException at org.eclipse.jdt.internal.ui.actions.ActionUtil.isOnBuildPath(ActionUtil.java:97) at org.eclipse.jdt.internal.ui.actions.ActionUtil.isProcessable(ActionUtil.java:83) at org.eclipse.jdt.ui.actions.FindAction.run(FindAction.java:227) ...
The scenario mentioned in Bug 368527 is taken care by the attached patch, co I think that can be marked duplicate of this bug. NPE has been taken care, When we cancel on Working Set selection, we get the RETURN_WITHOUT_BEEP element, and we need not call isProcessable for that.
(In reply to Sarika Sinha from comment #26) > NPE has been taken care, When we cancel on Working Set selection, we get the > RETURN_WITHOUT_BEEP element, and we need not call isProcessable for that. Thanks, we don't get the NPE now on pressing Cancel. Further to that, if two such files are selected and we press Cancel in the 'Search' dialog of the first one, the dialog for the second file should not be shown. Currently, it is shown and selecting a type in that has no impact as the search does not take place due to Cancel on the first dialog. Example: public class C1 { class C2 { } } public class C3 { class C4 { } } - Select C1.java and C3.java in Package explorer > press Ctrl+Shift+G. - Press Cancel in first 'Search' dialog. => Second 'Search dialog' still pops up, and selecting an element in it is of no use.
we thought of Finding the types for all the elements, but yes may be it makes more sense to stop the action at the first cancel.
Thanks, Sarika. Released the current Gerrit patch for bug 368527. Keeping this open for bug 26752.
(In reply to Noopur Gupta from comment #29) > Thanks, Sarika. Released the current Gerrit patch for bug 368527. > > Keeping this open for bug 26752. Thanks Noopur!! Should we keep this open for Bug 26752 ? Resolution of Bug 26752 would have solved this bug but the solution to that is independent and does not impact this bug.
(In reply to Sarika Sinha from comment #30) > (In reply to Noopur Gupta from comment #29) > > Thanks, Sarika. Released the current Gerrit patch for bug 368527. > > > > Keeping this open for bug 26752. > > Thanks Noopur!! Should we keep this open for Bug 26752 ? > Resolution of Bug 26752 would have solved this bug but the solution to that > is independent and does not impact this bug. We can keep both open as bug 26752 is only for allowing multi-select in the type hierarchy, and this bug is about search on multiple elements. We can verify if the solution in this bug works for multi-selection in type hierarchy also after bug 26752 is fixed.
(In reply to Manoj Palat from comment #7) > (In reply to Sarika Sinha from comment #6) > > Created attachment 259756 [details] [diff] > > Multi selection search > > > > This patch enables multi selection search from Hierarchy View, Outline View, > > Package explorer View and Search View. > > > > Open question - How should multi selection for References with Hierarchy > > scope work ? Right now it is taking the default Workspace Scope. > > Hierarchy scope for multiple types may not make much sense if they > themselves are not in hieararchy. Of course artificially this scope can be > made as union of the two but then what would be the use case? Hierarchy scope for fields/methods may still be useful with multi-select. Example: Project 1: ---------- package p1; public class C2 { int i; String s; void foo1(int i) { } void foo2(String s) { } } class X { void method() { C2 c2 = new C2(); c2.foo1(c2.i); c2.foo2(c2.s); } } Project 2 (depends on Project 1): --------------------------------- package p1; public class C1 extends C2 { public void test() { foo1(i); foo2(s); } } For C2.java, select the field(s)/method(s) from Outline view > right-click > References > Project / Hierarchy - both are useful. Also, currently the Hierarchy scope is still offered with multi-select, only on right-click > 'Declarations' for fields/methods. It should be consistent or offered depending on the usefulness.
Created Bug 493040 to track the inconsistency between References and Declarations for RC1. As opening up multi select for References will need new API from JDT Core to calculate unified JavaSearchScope, proposal is to disable the multi select for Declarations also till we have the API.
