Community
Participate
Working Groups
Created attachment 219303 [details] PLug-in projects I developed a small plug-in that allows to browse for existing icons within an eclipse installation. For detected images this plugin provides easy access to eiter extract the platform URI (like platform:/plugin/org.eclipse.pde.ds.ui/icons/obj16/ds_wizard_obj.gif) or the image itself. Might come in handy for PDE tooling to allow better reuse of images. Find more info in the associated blog entry: http://codeandme.blogspot.co.at/2012/07/reusing-platform-images.html or just check out the code for yourself.
The problem is well known: which icons are available, can I reuse one or do I have to add it? This plug-in offers a sound solution. I'd like to see this integrated into PDE. Maybe drag'n'drop of the icon into the extensions form page of the plug-in editor would be nice.
I have tested out the code and made a few changes and agree that this is worth adding to the platform. Before this can be done we will have to file a CQ for the contribution. Can you please comment on this bug asserting the following: I authored 100% of the content I have the right to contribute the content to Eclipse I contribute the content under the EPL So the steps to completing this are: 1) Christian comments stating the above information 2) I will start a contribution questionnaire (CQ) 3) Decide what the final bundle will be called (org.eclipse.ui.views.imagebroswer or something in PDE namespace). Fix any licensing information. 4) After approval, add the project to the repository. 5) Polish the view, add documentation (help here is appreciated but should wait until after approval)
For the record: I authored 100% of the content I have the right to contribute the content to Eclipse I contribute the content under the EPL I know this plug-in needs some polishing. Not only the UI (where IMHO forms UI would look better), but the (currently non-existent) caching mechanism needs some rework. When you hit the "next" button now, I will rescan all jars, simply ignoring images I already displayed. Not very efficient. If you need help here I will gladly do my best
Thanks Christian, I have filed the CQ. Once the view has been added to PDE, I would certainly appreciate help improving it!
Hi Christian, the IP legal team has asked that you resubmit the fix with all files containing the proper EPL copyright. The about.html file I will attach to this bug should be added to the plug-in. I was planning to do this for you before committing, but you fixing it in the original submission makes is much easier for the IP process. For reference, each source file should start with the following: /******************************************************************************* * Copyright (c) 2012 Christian Pontesegger and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at * http://www.eclipse.org/legal/epl-v10.html * * Contributors: * Christian Pontesegger - initial API and implementation *******************************************************************************/ If you are able, it would simplify my life if you were able to combine your tools plug-in into the view plug-in so there is only one bundle we are managing.
Created attachment 220902 [details] About.html
Christian, is it possible for you to update the copyrights?
(In reply to comment #7) > Christian, is it possible for you to update the copyrights? This hit me during vacation time, hence the delay. I added the comments & the about file as requested.
Created attachment 221377 [details] single project source with epl comments
The CQ was approved. I am recommending that the code get added to the pde.ui bundle. Unfortunately, the bundle has a BREE of Java 1.4. I think this justifies moving our bundles up to 1.5, so I am looking at Bug 319744. I will need to give the PMC notice. This contribution still looks good for M3 though.
Perfect, let me know once it is available in git for improvements. It is still kind of "rough"
I fixed up most of it and pushed it to master: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=d403ab0bc12c1f565a4e6bfd9873e374ade44432 There are a few things I want fixed before closing this bug: 1) Add F1 help doc (or at least the contexts) 2) The scrollbars won't show up until the view is resized 3) Add Mnemonics After that there are many ways to improve which are probably better handled in separate bugs.
Some feedback on initial testing besides those mentioned in comment 12: 1) 'Show More' is not intuitive. Especially, I was not expecting to "loose" the already shown icons. A better approach might be to show all plug-ins collapsed in a tree and I could then expand one to see its icons. 2) The drop down shows a very strange entry which has to be removed: 'org.eclipse.pde.internal.ui.views.imagebrowser.repositories.TargetPlatform@.. 3) It should probably open on the 'Workspace' by default, so that the user sees something useful when opening the view for the first time. 4) If the tree approach is not chosen (see 1) then 'Link with Editor and Selection' should be added, so that easily look at the stuff in the plug-in that I just selected in the 'Package Explorer'. 5) The whole thing is not accessible via keyboard. 6) It is quite slow. So, either this needs a progress bar or some tuning.
(In reply to comment #13) > Some feedback on initial testing besides those mentioned in comment 12: > > 1) 'Show More' is not intuitive. Especially, I was not expecting to "loose" > the already shown icons. A better approach might be to show all plug-ins > collapsed in a tree and I could then expand one to see its icons. I agree that 'show more' isn't the most intuitive workflow. However, I'm not a fan of the tree concept as having to expand a lot of entries makes it a lot slower to browse. The show more button does work quite well once you figure it out. > 2) The drop down shows a very strange entry which has to be removed: > > 'org.eclipse.pde.internal.ui.views.imagebrowser.repositories.TargetPlatform@. > . That is an interesting entry. What build was this on? The drop down should show Workspace/Running Application/Targe Platform: <name>. The labels do use toString(), so maybe there is a problem in one of the builds. > 3) It should probably open on the 'Workspace' by default, so that the user > sees something useful when opening the view for the first time. I'm on the fence about this. When the view is opened, it would be nice to start with more data. However, if the view was left open after a restart, the workspace could have a lot of images that we would load into memory for no benefit. Just trying it in my main workspace I have 500+ images which can take 45seconds+ of hard drive crunching to load in the view. > 4) If the tree approach is not chosen (see 1) then 'Link with Editor and > Selection' should be added, so that easily look at the stuff in the > plug-in that I just selected in the 'Package Explorer'. This would be a great addition and probably pretty easy to implement. > 5) The whole thing is not accessible via keyboard. Mnemonics is on my list of must fixes, but it would be better to have keyboard functionality in the image list as well. > 6) It is quite slow. So, either this needs a progress bar or some tuning.
(In reply to comment #14) > (In reply to comment #13) > > Some feedback on initial testing besides those mentioned in comment 12: > > > > 1) 'Show More' is not intuitive. Especially, I was not expecting to "loose" > > the already shown icons. A better approach might be to show all plug-ins > > collapsed in a tree and I could then expand one to see its icons. > > I agree that 'show more' isn't the most intuitive workflow. However, I'm > not a fan of the tree concept as having to expand a lot of entries makes it > a lot slower to browse. The show more button does work quite well once you > figure it out. Not it does not work well. Besides what I mentioned it is currently a pain to figure out when you've seen all items. And that it deletes the previous stuff is also not good. > > 2) The drop down shows a very strange entry which has to be removed: > > > > 'org.eclipse.pde.internal.ui.views.imagebrowser.repositories.TargetPlatform@. > > . > > That is an interesting entry. What build was this on? Latest N-build. > > 3) It should probably open on the 'Workspace' by default, so that the user > > sees something useful when opening the view for the first time. > > I'm on the fence about this. When the view is opened, it would be nice to > start with more data. However, if the view was left open after a restart, > the workspace could have a lot of images that we would load into memory for > no benefit. > > Just trying it in my main workspace I have 500+ images which can take > 45seconds+ of hard drive crunching to load in the view. Sure, see 6) plus if it loads 500 images then the limit (250) seems not to work. > > 5) The whole thing is not accessible via keyboard. > > Mnemonics is on my list of must fixes, but it would be better to have > keyboard functionality in the image list as well. Not just better but a must to pass accessibility testing ;-)
The simple fix is to use buttons instead of labels for the images. They are easily selected, keyboard accessible, have tooltips, etc. Obviously it doesn't fix the performance issues.
(In reply to comment #13) > 1) 'Show More' is not intuitive. Especially, I was not expecting to "loose" > the already shown icons. A better approach might be to show all plug-ins > collapsed in a tree and I could then expand one to see its icons. You are right. What I wanted was some kind of a pager. Could not find such a widget in SWT, so this was my quick-and-dirty hack. Besides clicking on "show more" will restart the parser which browses through all the files again, filtering out the already shown images by their names. This is damn slow and it gets slower on each klick as we are parsing the same jar files over and over again. It needs some caching mechanism, a better background job to scan for images and so on. Displaying too much images within the view makes scolling damn slow. When experiencing with 500 images my (definitely old) pc started to stutter. > 3) It should probably open on the 'Workspace' by default, so that the user > sees something useful when opening the view for the first time. I agree with Curtis, this would slow down startup and would not be all too useful. > 4) If the tree approach is not chosen (see 1) then 'Link with Editor and > Selection' should be added, so that easily look at the stuff in the > plug-in that I just selected in the 'Package Explorer'. Could be enabled as long the target is set to "Workbench" > 6) It is quite slow. So, either this needs a progress bar or some tuning. See my response to 1) Additionally there is the question of how to filter entries. Currently I allow to search for active/disabled icons or wizards. I use some filters internally. It would be great to expose this to the user somehow, so that a search could be limited to a subset of plugins maybe via regexp. I was wondering if it would be possible to use the expressions framework for this...
I've pushed a change to use buttons instead of labels which allows keyboard traversal. http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=d5ea43c8c644777e20db29ca51dcba45b663034f Christian, if you can look at patches for some of the other outstanding issues that would be very helpful.
Created attachment 222398 [details] Patch parsing image repositories Added a patch that fixes repository parsing. Now repositories use an internal cache for ImageData objects. It works this way: the repository keeps a cache. When depleted it populates the cache with images from one plugin (jar file/directory/ workspace project) and continues delivering from cache. Thus each location should only be visited once. Added parsing of target platform directories too. Before only jar files were recognized. The cache would support going back and forth, so we could have a "show prev images" button. But due to filtering it would be tough to set the index position in the cache. If we want this feature we could move filtering to the repositories. Please do me a favor and externalize the Job name from AbstractRepository(IImageTarget target) constructor. When testing I saw that the vertical scrollbar is not displayed anymore when needed. Seems that one of the previous commits changed that. Regarding the buttons: personally I would prefer to remove the borders around it, what do you think?
> When testing I saw that the vertical scrollbar is not displayed anymore when > needed. Seems that one of the previous commits changed that. If the view is resized, the scrollbars show, otherwise they don't. Haven't figured out why. > > Regarding the buttons: personally I would prefer to remove the borders > around it, what do you think? The only way to get no borders is to use an invisible coolbar. SWT.FLAT leaves borders on some OS. In Linux the flat hint is ignored entirely. If it looks better on Windows I would be fine with the flag though.
Pushed the latest patch (with externalized string) to master http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=54f28d1df708c084068cfe9dfec61c9a30ef0fcb
There is a recent post on PDE Tools http://jeeeyul.wordpress.com/2012/10/16/make-you-ide-more-awesome-with-pde-tools/ with a similar feature. I especially like the idea of using a pshelf widget to display the results. As this seems to be taken from Nebula I'm not sure if we can use these widgets in PDE. Are we allowed to use it?
(In reply to comment #22) > There is a recent post on PDE Tools > http://jeeeyul.wordpress.com/2012/10/16/make-you-ide-more-awesome-with-pde- > tools/ > > with a similar feature. I especially like the idea of using a pshelf widget > to display the results. As this seems to be taken from Nebula I'm not sure > if we can use these widgets in PDE. Are we allowed to use it? I added a comment to his post asking for his input on this view. No, Nebula widgets are not available to PDE.
I tried to give it a try again but ran into several issues: 1) !ENTRY org.eclipse.pde.ui 4 4 2012-10-19 13:41:52.743 !MESSAGE Could not find target platform service for image browser view. 2) !ENTRY org.eclipse.core.jobs 4 2 2012-10-19 13:41:52.747 !MESSAGE An internal error occurred during: "Scan for UI images". !STACK 0 java.lang.NullPointerException at org.eclipse.pde.internal.ui.views.imagebrowser.repositories.TargetPlatformRepository.populateCache(TargetPlatformRepository.java:49) at org.eclipse.pde.internal.ui.views.imagebrowser.repositories.AbstractRepository.run(AbstractRepository.java:49) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:53) 3) when using a new workspace with N20121018-2330 I get this: !ENTRY org.eclipse.pde.ui 4 0 2012-10-19 13:40:30.846 !MESSAGE Invalid image !STACK 0 org.eclipse.swt.SWTException: Invalid image at org.eclipse.swt.SWT.error(SWT.java:4361) at org.eclipse.swt.SWT.error(SWT.java:4276) at org.eclipse.swt.SWT.error(SWT.java:4247) at org.eclipse.swt.internal.image.GIFFileFormat.loadFromByteStream(GIFFileFormat.java:126) at org.eclipse.swt.internal.image.FileFormat.loadFromStream(FileFormat.java:55) at org.eclipse.swt.internal.image.FileFormat.load(FileFormat.java:86) at org.eclipse.swt.graphics.ImageLoader.load(ImageLoader.java:147) at org.eclipse.swt.graphics.ImageDataLoader.load(ImageDataLoader.java:22) at org.eclipse.swt.graphics.ImageData.<init>(ImageData.java:331) at org.eclipse.pde.internal.ui.views.imagebrowser.repositories.AbstractRepository.searchJarFile(AbstractRepository.java:109) at org.eclipse.pde.internal.ui.views.imagebrowser.repositories.TargetPlatformRepository.populateCache(TargetPlatformRepository.java:54) at org.eclipse.pde.internal.ui.views.imagebrowser.repositories.AbstractRepository.run(AbstractRepository.java:49) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:53) !ENTRY org.eclipse.core.jobs 4 2 2012-10-19 13:40:46.222 !MESSAGE An internal error occurred during: "Scan for UI images". !STACK 0 java.lang.OutOfMemoryError: Java heap space at org.eclipse.swt.graphics.ImageData.<init>(ImageData.java:448) at org.eclipse.swt.graphics.ImageData.internal_new(ImageData.java:517) at org.eclipse.swt.internal.image.GIFFileFormat.readImageBlock(GIFFileFormat.java:367) at org.eclipse.swt.internal.image.GIFFileFormat.loadFromByteStream(GIFFileFormat.java:106) at org.eclipse.swt.internal.image.FileFormat.loadFromStream(FileFormat.java:55) at org.eclipse.swt.internal.image.FileFormat.load(FileFormat.java:86) at org.eclipse.swt.graphics.ImageLoader.load(ImageLoader.java:147) at org.eclipse.swt.graphics.ImageDataLoader.load(ImageDataLoader.java:22) at org.eclipse.swt.graphics.ImageData.<init>(ImageData.java:331) at org.eclipse.pde.internal.ui.views.imagebrowser.repositories.AbstractRepository.searchJarFile(AbstractRepository.java:109) at org.eclipse.pde.internal.ui.views.imagebrowser.repositories.TargetPlatformRepository.populateCache(TargetPlatformRepository.java:54) at org.eclipse.pde.internal.ui.views.imagebrowser.repositories.AbstractRepository.run(AbstractRepository.java:49) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:53)
See bug 303321 why you get an OOME.
Christian, any time you can put into this during the week would be appreciated as next week is M3.
(In reply to comment #24) > 1) > !ENTRY org.eclipse.pde.ui 4 4 2012-10-19 13:41:52.743 > !MESSAGE Could not find target platform service for image browser view. This happens when the target platform service cannot be obtained in TargetPlatformRepository:98 Currently we log this. We could alternately remove the repository from the dropdown. Strange that the service cannot be resolved on your setup. > 2) > !ENTRY org.eclipse.core.jobs 4 2 2012-10-19 13:41:52.747 > !MESSAGE An internal error occurred during: "Scan for UI images". > !STACK 0 > java.lang.NullPointerException Happens when the target platform service is not there, so see 1) I could easily catch this case to avoid the NPE, but cannot resolve any images then > 3) when using a new workspace with N20121018-2330 > I get this: > !ENTRY org.eclipse.pde.ui 4 0 2012-10-19 13:40:30.846 > !MESSAGE Invalid image > [...] > java.lang.OutOfMemoryError: Java heap space Cannot do anything on invalid images except ignoring them. Currently we log an SWTException. What to do with the OOME? Catch it? Not sure whether this makes sense once the heap is depleted. Input how to react here is appreciated
(In reply to comment #27) > (In reply to comment #24) > What to do with the OOME? Catch it? Not sure whether this makes sense once > the heap is depleted. When loading the image we could catch the OOME and then free the space again. We could also remember the memory before we load the image, so that we can guess whether the loading caused the OOME. And we should write the full file name which caused the OOME to the .log.
Created attachment 222700 [details] fix for OOME (In reply to comment #28) > When loading the image we could catch the OOME and then free the space > again. We could also remember the memory before we load the image, so that > we can guess whether the loading caused the OOME. And we should write the > full file name which caused the OOME to the .log. Cannot recover from an OOME. Once the heap is full the application is dead. Fixed two things to make it work: * removed caching of already displayed images as this used up a lot of space when browsing lots of images. * fixed some resource leaks with unclosed streams * added an additional log entry when an invalid image is detected to log the full image path (please localize the string!). Seems PDEPlugin does not provide a method to log a message along with a throwable. Therefore I create 2 entries right now. Maybe we should stick to the error message only * additionally changed the listener on the buttons to focuslistener as key traversal is not tracked by selection listener. * had a look into the missing scroll bar issue too. This seems to be related to the creation of the scrollcomposite which is done differently to the original contribution. Could not find a fix for this yet.
(In reply to comment #29) I applied the patch along with a few of my own fixes. I think that we'll be in good enough shape for M3 inclusion. There is nothing stopping from us making changes in M4. http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=1fb80e95640ecda9a7ea5bc23e5d0517f765af82 > * added an additional log entry when an invalid image is detected to log the > full image path (please localize the string!). Seems PDEPlugin does not > provide a method to log a message along with a throwable. Therefore I create > 2 entries right now. Maybe we should stick to the error message only I NLS'd the string and combined the entries. To do this, you create a new status object containing both the message and the exception. > * had a look into the missing scroll bar issue too. This seems to be related > to the creation of the scrollcomposite which is done differently to the > original contribution. Could not find a fix for this yet. I switched back to the setup you original submitted and made some tweaks. Scrollbar works correctly AFAICT. I also added code to reveal the button with focus for easier tab traversal.
Marking as fixed for M3. We can open new bugs for additional improvements.
Note that this fix causes early loading of PDE UI, see bug 477578 comment 1 for details.
