Community
Participate
Working Groups
It would be very cool to have a search box in the property sheet, just like the one in the preferences dialog. When a property sheet becomes large (for example EMF generator model properties), this could be very useful.
Could you provide a patch for the suggested feature?
See http://wiki.eclipse.org/Platform_UI/How_to_Contribute PW
OK. Challenge taken :-) Here is the functionality that I could implement: https://github.com/vainolo/eclipse.platform.ui/tree/bug400835 (clone of eclipse.platform.ui) I created a new composite (FilterablePropertySheet) which contains a text-box for in which the user can enter the filter text. The property sheet pagebook is added to a composite below this text-box. When the text in the text-box changes, the entries shown in the properties view change according to the text - only properties which contain the text in their display name or string value are displayed. Would love to receive some feedback on the implementation. If this looks OK, I can extend it to manage regular expressions in the filter also.
(In reply to comment #3) > OK. Challenge taken :-) > Here is the functionality that I could implement: > > https://github.com/vainolo/eclipse.platform.ui/tree/bug400835 > (clone of eclipse.platform.ui) Hi Arieh, Some comments on your original implementation: 1) I wouldn't try and encapsulate the text box and label within FilterablePropertySheet. The PropertySheet view should set that up within its createPartControl ... it can be grouped in a method call for clarity. 2) In the end the work has to be done by the PropertySheetViewer. Have a look at org.eclipse.jface.viewers.StructuredViewer.setFilters(ViewerFilter[]), and the use of the ViewerFilter class. This viewer only extends Viewer (so it's not already a structured viewer) but PropertySheetViewer should probably implement its filtering using the same/similar pattern as seen in StructuredViewer. Thank you for your interest in this. I'll just add that these are API classes, which means we can only add to them during M6 (the next 3 weeks or so) or after Kepler ships (July). PW
(In reply to comment #4) > I'll just add that these are API classes, which means we can only add to > them during M6 (the next 3 weeks or so) or after Kepler ships (July). > > PW Also, I assume the patch will be larger than 250 lines, which means it needs an CQ. However, the deadline was last Friday for Kepler CQs. I suggest we take this slowly and target Kepler+1.
Hi Arieh, a bit more infos about Viewer filter can be found here: http://www.vogella.com/articles/EclipseJFaceTableAdvanced/article.html#jfacetable_filter and http://www.vogella.com/articles/EclipseJFaceTableAdvanced/article.html#filtercontent I think the example should be similar to your use case.
Maybe you can make it under 250 lines... :-)
Another good place to start is org.eclipse.ui.dialogs.FilteredTree which is quite mature. PW
> 1) I wouldn't try and encapsulate the text box and label within > FilterablePropertySheet. The PropertySheet view should set that up within > its createPartControl ... it can be grouped in a method call for clarity. Do you mean doing this in PropertySheetViewer.java?
Hi Everyone Updated my solution to include only changes in the PropertySheetViewer.java file. A textbox was added above the property tree in which the user can enter the text to filter. I also added a "clear" button to clear the filter. Some thoughts and answers to the comments made before: 1. While implementing a change similar to the functionality of a Structured viewer would be nice, it adds a lot of generic code to solve a specific problem. Does anyone think more filter types are going to the added in the future to this pane? if not, I don't think doing this is worth the time. 2. My implementation is very short (89 line changes overall), with no API changes. 3. I didn't add code to check if filtering is not needed (see the code in FilteredTree). I don't think this is necessary because it is a very small change, occupies little screen space and shouldn't bother anyone not using the feature. Anyway, the updated code is again @github: https://github.com/vainolo/eclipse.platform.ui/tree/a7c2ca2ab188c1fec47a7a6d3996b8a1139a4dee Your comments are welcome Arieh
Hi Arieh, Thanks for the updated commit. Some comments: 1) I get a compile error: Description Resource Path Location Type The method contains(String) is undefined for the type String PropertySheetViewer.java /org.eclipse.ui.views/src/org/eclipse/ui/views/properties line 709 Java Problem It's because the views plugin is compiled against J2SE-1.4. You can see them as well, just go to Window>Preferences>Java>Installed JREs. Add a JRE for each of 1.4, 1.5, 1.6, and 1.7. Eclipse will then use the libraries from the appropriate JVM during compile 2) Please put braces around complex if statements as in org.eclipse.ui.views.properties.PropertySheetViewer.textFilteredEntries(List) ... preferrably around all if statements (it's our coding standard). 3) I get a ClassCastException in org.eclipse.ui.tests.multipageeditor.MultiPageEditorSelectionTest.testPropertiesView(). You've changed what getControl() returns (understandably) but internals might cast this to a Tree. You would either have to search all callers of getControl(*) or still return the Tree contained within this viewer. 4) see how FilteredTree reacts to a ModifyListener in org.eclipse.ui.dialogs.FilteredTree.textChanged() PW
Hi Paul for all the comments, and sorry for the time it took me to answer. Regarding your comments: > 1) I get a compile error: > > Description Resource Path Location Type > The method contains(String) is undefined for the type String > PropertySheetViewer.java > /org.eclipse.ui.views/src/org/eclipse/ui/views/properties line 709 Java > Problem > > It's because the views plugin is compiled against J2SE-1.4. You can see > them as well, just go to Window>Preferences>Java>Installed JREs. Add a JRE > for each of 1.4, 1.5, 1.6, and 1.7. Eclipse will then use the libraries > from the appropriate JVM during compile > function contains(String) is supported since 1.5. Java 1.4 is EOL (as per the Oracle web site: http://www.oracle.com/technetwork/java/javase/index-jsp-138567.html). But anyway, changed my installed JRE and changed the code to match it (copied the implementation of contains from class String). > 2) Please put braces around complex if statements as in > org.eclipse.ui.views.properties.PropertySheetViewer. > textFilteredEntries(List) ... preferrably around all if statements (it's our > coding standard). Done > > 3) I get a ClassCastException in > org.eclipse.ui.tests.multipageeditor.MultiPageEditorSelectionTest. > testPropertiesView(). You've changed what getControl() returns > (understandably) but internals might cast this to a Tree. You would either > have to search all callers of getControl(*) or still return the Tree > contained within this viewer. I think returning the tree is not a good option... It is just not right. I searched for references to the method and found a cast in the PropertySheetPage class (line 444). The only solution I could think of was to add a new method to the PropertySheetViewer that returns this tree and call this method from the PropertySheetPage. I made the method package scoped to limit its access. BTW, I tried running the tests but was unable to do so. Is there a guide somewhere where I can learn how to do this? > 4) see how FilteredTree reacts to a ModifyListener in > org.eclipse.ui.dialogs.FilteredTree.textChanged() Sorry but I was unable to understand what I should get from there. If I understood it correctly, it uses an external job to manage refreshing of the list. Do you think this is needed here? isn't it an overkill? Once again, thanks for your time and patience. The latest commit of the code is at: https://github.com/vainolo/eclipse.platform.ui/tree/bug400835 Arieh
(In reply to comment #12) > > > > It's because the views plugin is compiled against J2SE-1.4. You can see > > them as well, just go to Window>Preferences>Java>Installed JREs. Add a JRE > > for each of 1.4, 1.5, 1.6, and 1.7. Eclipse will then use the libraries > > from the appropriate JVM during compile > > > > function contains(String) is supported since 1.5. Java 1.4 is EOL (as per > the Oracle web site: > http://www.oracle.com/technetwork/java/javase/index-jsp-138567.html). But > anyway, changed my installed JRE and changed the code to match it (copied > the implementation of contains from class String). We wouldn't be able to accept it if it's copied out of any GPLed or copyrighted classes. But name.toLowerCase().indexOf(filter.toLowerCase())>=0 would be acceptable. Could you check what you have and amend the commit? Thanks for getting back to us. PW
(In reply to comment #13) > (In reply to comment #12) > > > > > > It's because the views plugin is compiled against J2SE-1.4. You can see > > > them as well, just go to Window>Preferences>Java>Installed JREs. Add a JRE > > > for each of 1.4, 1.5, 1.6, and 1.7. Eclipse will then use the libraries > > > from the appropriate JVM during compile > > > > > > > function contains(String) is supported since 1.5. Java 1.4 is EOL (as per > > the Oracle web site: > > http://www.oracle.com/technetwork/java/javase/index-jsp-138567.html). But > > anyway, changed my installed JRE and changed the code to match it (copied > > the implementation of contains from class String). > > We wouldn't be able to accept it if it's copied out of any GPLed or > copyrighted classes. > > But name.toLowerCase().indexOf(filter.toLowerCase())>=0 would be acceptable. > Could you check what you have and amend the commit? > > Thanks for getting back to us. > > PW Done. You know where to find the code. Arieh
Hi everyone... Did my work go to /dev/null?
(In reply to comment #15) > Hi everyone... Did my work go to /dev/null? No, EclipseCon and now the endgame for Kepler. Since we need a CQ and the date for that passed in Feb, I won't be able to look at this until June. PW
OK. Thanks for the update. Will be waiting.
We would be able to get a CQ now, and you could sign your CLA (link beside your name in bugzilla) PW
Could you convert your change into a Gerrit review? The necessary process is described here: http://www.vogella.com/articles/Gerrit/article.html#eclipsegerritcontribution
Signed my CLA. Lars - is this a must? it's been like 8 months, and would require me to re-create my env. If there is no other way, I'll do it, but...
(In reply to Arieh Bibliowicz from comment #20) > Signed my CLA. > Lars - is this a must? it's been like 8 months, and would require me to > re-create my env. If there is no other way, I'll do it, but... You can hold off on that (I can push the initial change), but if we review it and send it back again for changes then you'll have to set up your env again. PW
(In reply to Arieh Bibliowicz from comment #20) > Signed my CLA. > Lars - is this a must? it's been like 8 months, and would require me to > re-create my env. If there is no other way, I'll do it, but... Not a must, but in my experience it speeds up the process of accepting patches. And of course it would be great to get more patches from you in the future, with Gerrit the feedback loop is relatively fast.
(In reply to Lars Vogel from comment #22) > (In reply to Arieh Bibliowicz from comment #20) > > Signed my CLA. > > Lars - is this a must? it's been like 8 months, and would require me to > > re-create my env. If there is no other way, I'll do it, but... > > Not a must, but in my experience it speeds up the process of accepting > patches. And of course it would be great to get more patches from you in the > future, with Gerrit the feedback loop is relatively fast. (In reply to Paul Webster from comment #21) > (In reply to Arieh Bibliowicz from comment #20) > > Signed my CLA. > > Lars - is this a must? it's been like 8 months, and would require me to > > re-create my env. If there is no other way, I'll do it, but... > > You can hold off on that (I can push the initial change), but if we review > it and send it back again for changes then you'll have to set up your env > again. > > PW Great guys. Thanks
(In reply to Arieh Bibliowicz from comment #23) > Great guys. Thanks If you need help with the setup, please feel free to contact me via email or ask in this bug report. Sorry for letting you that long waiting, we try to get better and faster in accepting contributions with Gerrit.
I've uploaded the latest from https://github.com/vainolo/eclipse.platform.ui/tree/bug400835 as a single Gerrit change set: https://git.eclipse.org/r/19982 We can review it there. Arieh, if you haven't yet you should log into Gerrit at least once: https://git.eclipse.org/r/ PW
(In reply to Paul Webster from comment #25) > I've uploaded the latest from > https://github.com/vainolo/eclipse.platform.ui/tree/bug400835 as a single > Gerrit change set: > > https://git.eclipse.org/r/19982 > > We can review it there. Arieh, if you haven't yet you should log into > Gerrit at least once: https://git.eclipse.org/r/ > > PW Did the review. I saw there is also an "IP-Clean" column. Is this relevant? if so, how can I fill this? thanks Paul for pushing the change for me! Arieh
Latest comments applied to Gerrit review. We have a couple of days of community contributions to get this in, then we're on to 4.3.2 building/testing. PW
Created attachment 239678 [details] PropertySheetViewer changes for gerrit CR Implemented desired changes from CR in change. * Didn't implement mouse listener because I don't handle default initial text therefore no code is needed there. * Didn't implement traverse listener because didn't understand what it did in the FilteredTree class. * Didn't change getControl method because it would disable the whole property sheet.
Simon, can we use the new FilteredTree implementation here?
