Community
Participate
Working Groups
see bug 204404 for some more information
This is a bit of work, since there is no filtered checkbox tree. The naive solution doesn't work, since you must maintain the checkbox state whenever a refresh happens (and in our case we don't actually have a model that maintains check state). As well, tree/table viewers don't have refresh listeners. Currently I am addressing this issue by overriding all the refresh operations and saving the check state, and then putting them back. There must be a better way. Anyone have any pointers.
ok, if this is too tricky, I would worry less about it as the other bug does quite a bit for us.
Created attachment 81280 [details] FilteredCheckboxTreeViewer Here is the first crack at a filtered checkboxtreeviewer. This viewer saves all the items which have a check before a refresh, and then after the refresh it puts the check back. This viewer will be needed for this bug, so I am putting it here so I don't loose the code :-)
Created attachment 81502 [details] Initial version of a filtered tree for the launch configuration This is an initial version of the launch configuration. The patch still needs some TLC, but it outlines the basic approach. I think we will need some people to checkout the behaviour since a filtered checkbox tree is not common in Eclipse, so the interactions may not be right. Maybe we can get this in early in the M4 release cycle so we have lots to time to test it out.
Created attachment 81503 [details] mylyn/context/zip
targetting to 3.4M4
Created attachment 81545 [details] Updated patch for filtered checkbox tree Here is an updated patch with some comments and TLC. Chris / Brian, you can probably start to review this (when you get a chance). We may also want to get some of the equinox guys to try it out and see if it makes sense or not.
Created attachment 81546 [details] mylyn/context/zip
We will start making the Equinox guys too productive with all your good work Ian :)
When you actually review this, let me know. We can hit IRC if you have questions. There were a lot of changes in the OSGiBundleBlock because the existing version assumed that all checked bundles have a TreeItem. However, if you filter some nodes, they are still considered "checked". The basic approach I took was to create a FilteredCheckboxTree which contains a cache of all elements in the tree. This way, even when an item is filtered, you can still get / set its checked state.
*** Bug 154592 has been marked as a duplicate of this bug. ***
Almost there Ian, it even works well on a large set of bundles (RSA's ~1000 bundles) however a few issues need to be addressed to get us to the finish line: 1) The buttons like 'Select All', etc... should be aligned with the jface viewer so users can associate that the buttons act on the actual plug-ins within the viewer 2) The semantics of 'Select All' and 'Deselect All' should apply to whatever is filtered / displayed. It shouldn't apply to everything. This is a similar approach we take in the Import Plug-ins and Fragments wizard. 3) I notice some flicker and lag. The refresh should happen in a WorkbenchJob with a delay of about 200 ms. Take inspiration from PluginImportWizardDetailedPage and its clever way of filtering things in a responsive way. 4) I also got an NPE while toying with the code: java.lang.NullPointerException at org.eclipse.pde.internal.ui.launcher.OSGiBundleBlock.handleGroupStateChanged(OSGiBundleBlock.java:297) at org.eclipse.pde.internal.ui.launcher.AbstractPluginBlock.toggleGroups(AbstractPluginBlock.java:444) at org.eclipse.pde.internal.ui.launcher.AbstractPluginBlock$Listener.filterAffectingControl(AbstractPluginBlock.java:128) at org.eclipse.pde.internal.ui.launcher.AbstractPluginBlock$Listener.widgetSelected(AbstractPluginBlock.java:156) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:227) This is great work Ian. I'm excited about this stuff as much as I am about the Plug-in Spy at the moment :)
(In reply to comment #12) > 1) The buttons like 'Select All', etc... should be aligned with the jface > viewer so users can associate that the buttons act on the actual plug-ins > within the viewer Ok, I will fix this. > > 2) The semantics of 'Select All' and 'Deselect All' should apply to whatever is > filtered / displayed. It shouldn't apply to everything. This is a similar > approach we take in the Import Plug-ins and Fragments wizard. Hum... the reason I went with this was because of the "add required bundles". How should this button act? Does it only add the required bundles that are currently visible? Should this act differently from select all? Chris suggested that we disable the "Add Required", "Working Set" and "Restore Defaults" when filtering, since their behaviour is not clear. > > 3) I notice some flicker and lag. The refresh should happen in a WorkbenchJob > with a delay of about 200 ms. Take inspiration from > PluginImportWizardDetailedPage and its clever way of filtering things in a > responsive way. > I'm pretty sure this is what happens in the FilteredTree (which I extend). If you look at the FilteredTree::textChange() method this seems to do this. I will keep an eye out for this. > 4) I also got an NPE while toying with the code: Yikes... ok, I will look into this.
For #3, use the Import Plug-ins wizard as a litmus test... there's no flicker there. Thanks again!
Ian, any time to get this in for M4?
Let's move this to M5
Thanks Chris, I haven't forgotten about this :-). I spent part of the weekend getting this to work again (a lot has changed in eclipse core since M3), but I was able to get it working with last weeks I build. I will work on this early in the M5 cycle to *make sure* it gets in. (and doesn't become the reason for an M5a) :-)
I have some time this week while my supervisor reads my thesis :-), so I thought I would return to this bug. I am having a hard time applying my patch since a bunch of formatting was done to both the AbstractPluginBlock and OSGiBundleBlock on Jan16. Are these formatting rules published somewhere? Was there a reason for this? Also, I noticed we are now using import java.util.* instead of java.util.Set, etc... Is this a new standard?
ya, each PDE project has project-level settings along with formatting options. Sorry that this affected the patch :(
Created attachment 90017 [details] Patch No worries, I have updated my original patch so it can now be applied again. This is still the same patch (with the same problems), but at least I can start working on this bug again :-).
Created attachment 90018 [details] mylyn/context/zip
Created attachment 90264 [details] Patch (Feb 20) This is an updated patch. It fixes most of Chris's comments, but there is still one issue remaning. I will work on it tonight.
Created attachment 90265 [details] mylyn/context/zip
Created attachment 90297 [details] Patch (Feb21) This patch addresses the comments Chris had. If someone would like to take it for a spin and post comments, it would be much appreciated.
Created attachment 90298 [details] mylyn/context/zip
Created attachment 90433 [details] Another Patch I have fixed up the code a bit, added some documentation and made a few tweaks. This patch should be good to go. Any takers :-)
Created attachment 90434 [details] mylyn/context/zip
reviewing...
Created attachment 90468 [details] org.eclipse.pde.ui.patch I updated the patch to add a separator between the pluginviewer and everything else. This makes it look a bit cleaner with the new filtered text there. I also did some code cleanup and fixed some copyrights. This should make its way into the next I-build and hopefully please people that deal with complex launch configurations.
Created attachment 90469 [details] mylyn/context/zip
Thanks for the good work Ian! Simon Archer has one less thing to complain about :)
Thanks Chris, The separator looks good! I will grab the next I-Build when it comes out and continue to test this. There are still a few open issues I discovered, and I have opened bugs for these.
