Community
Participate
Working Groups
Could we support $nl$ (and possibly) others in our platform: scheme? $nl$/icons/full/eview16/prop_ps.gif works but I get an error for: platform:/plugin/org.eclipse.ui.views/$nl$/icons/full/eview16/prop_ps.gif eclipse.buildId=I20081007-0922 java.fullversion=J2RE 1.5.0 IBM J9 2.3 Linux x86-32 j9vmxi3223-20080315 (JIT enabled) J9VM - 20080314_17962_lHdSMr JIT - 20080130_0718ifx2_r8 GC - 200802_08 BootLoader constants: OS=linux, ARCH=x86, WS=gtk, NL=en_US Framework arguments: -product org.eclipse.sdk.ide Command-line arguments: -product org.eclipse.sdk.ide -data /opt/pwebster/workspaces/build350/../runtime-B350 -dev file:/opt/pwebster/workspaces/build350/.metadata/.plugins/org.eclipse.pde.core/B350/dev.properties -os linux -ws gtk -arch x86 Error Fri Oct 10 08:36:17 EDT 2008 /$nl$/icons/full/eview16/prop_ps.gif java.io.FileNotFoundException: /$nl$/icons/full/eview16/prop_ps.gif at org.eclipse.osgi.framework.internal.protocol.bundleentry.Handler.findBundleEntry(Handler.java:44) at org.eclipse.osgi.framework.internal.core.BundleResourceHandler.openConnection(BundleResourceHandler.java:169) at java.net.URL.openConnection(URL.java:978) at org.eclipse.core.internal.boot.PlatformURLConnection.connect(PlatformURLConnection.java:110) at org.eclipse.core.internal.boot.PlatformURLConnection.getURLAsLocal(PlatformURLConnection.java:238) at org.eclipse.core.internal.runtime.PlatformURLConverter.toFileURL(PlatformURLConverter.java:36) at org.eclipse.core.runtime.FileLocator.toFileURL(FileLocator.java:165) at org.eclipse.jface.resource.URLImageDescriptor.getFilePath(URLImageDescriptor.java:137) at org.eclipse.jface.resource.URLImageDescriptor.createImage(URLImageDescriptor.java:157) at org.eclipse.jface.resource.ImageDescriptor.createResource(ImageDescriptor.java:165) at org.eclipse.jface.resource.DeviceResourceManager.allocate(DeviceResourceManager.java:56) at org.eclipse.jface.resource.AbstractResourceManager.create(AbstractResourceManager.java:88) at org.eclipse.jface.resource.LocalResourceManager.allocate(LocalResourceManager.java:82) at org.eclipse.jface.resource.AbstractResourceManager.create(AbstractResourceManager.java:88) at org.eclipse.jface.resource.ResourceManager.createImage(ResourceManager.java:172) at org.eclipse.ui.menus.CommandContributionItem.updateIcons(CommandContributionItem.java:799) at org.eclipse.ui.menus.CommandContributionItem.fill(CommandContributionItem.java:405) at org.eclipse.ui.internal.ShowInMenu.fill(ShowInMenu.java:133) at org.eclipse.jface.action.MenuManager.doItemFill(MenuManager.java:731) at org.eclipse.jface.action.MenuManager.update(MenuManager.java:812) at org.eclipse.jface.action.MenuManager.handleAboutToShow(MenuManager.java:464) at org.eclipse.jface.action.MenuManager.access$1(MenuManager.java:459) at org.eclipse.jface.action.MenuManager$2.menuShown(MenuManager.java:485) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:235) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1158) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1182) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1163) at org.eclipse.swt.widgets.Menu.gtk_show(Menu.java:623) at org.eclipse.swt.widgets.Widget.windowProc(Widget.java:1508) at org.eclipse.swt.widgets.Display.windowProc(Display.java:4105) at org.eclipse.swt.internal.gtk.OS._g_main_context_iteration(Native Method) at org.eclipse.swt.internal.gtk.OS.g_main_context_iteration(OS.java:1830) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3043) at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2382) at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2346) at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2198) at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:493) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:333) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:488) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:113) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:193) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:370) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:79) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:618) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:549) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:504) at org.eclipse.equinox.launcher.Main.run(Main.java:1236) at org.eclipse.equinox.launcher.Main.main(Main.java:1212)
DJ, is this reasonable? Can we support the same variable substitution as FileLocator.find does?
Most of Paul's requests are completely unreasonable but I think this one is ok. :-)
Created attachment 125432 [details] patch Patch to handle $nl$ as a segment in a path in a platform:/plugin/ URL. The $nl$ can occur at any depth in the path.
John, do you mind taking a look at this? Also, can you think of a good way of writing automated tests for this without assuming a particular NL?
This seems reasonable, but there are some concerns: - '$' is a valid character in URLs, so $nl$ could actually be a valid path segment - Would we also support $os$ and $ws$ to be consistent with FileLocator - It seems inconsistent for some platform: URLs to support this and not others - We would need to specify this behaviour somewhere in our doc I think these above issues could all be addressed by only supporting the resolution of these variables via FileLocator. I.e., just creating a URL and calling openStream() would not magically resolve the variable (since maybe it is not a variable). But, someone could call FileLocator#resolve(URL) or perhaps a new method on FileLocator to get the variable resolution behaviour. This removes the magic, the compatibility problem, and gives us somewhere to clearly specify the variable substitution behaviour. We could then also support the Map argument for overriding variable values that we provide on the find* methods. I'm also a bit concerned about the motivating usecase, which is the desire to reach into other plugins to access their files. This raises issues about whether resource files in other people's plugins can/should be considered as unchanging API. I'm talking offline with the UI team about this to see what other alternatives there are.
John talked to the UI team and we've decided to go ahead with the approach of taking advantage of the FileLocator. Basically the new API will take in a URL which may contain a var in the path and we will split off the path, pass it to FileLocator, then hand back the URL.
Created attachment 127905 [details] patch Here is another patch. John, comments and help with the API would be appreciated. A couple of questions: - in the case of failure, should we return null or the original URL? (I choose null) - should the method throw any exceptions? (I say yes, otherwise we should at least add some sort of logging) - should the resulting URL already be passed to FileLocator#resolve before being handed back to the user? (I think this should probably be added... note that the patch doesn't do this) fyi here is a doit that I was playing around with to debug some of the behaviour. System.out.println(FileLocator.resolve(new URL("platform:/plugin/anApp"))); System.out.println(FileLocator.resolve(new URL("platform:/plugin/anApp/plugin.xml"))); URL[] urls = new URL[] { // new URL("platform:/plugin/anApp"), // new URL("platform:/plugin/anApp/plugin.xml"), // new URL("platform:/plugin/anApp/$nl$/a.txt"), // }; for (int i = 0; i < urls.length; i++) System.out.println(FileLocator.resolve(FindSupport.find(urls[i])));
Created attachment 127956 [details] Updated patch with javadoc I added javadoc for the API method. My suggestion on those questions is to be consistent with the other find* methods on FileLocator. I believe this would mean: - Return null on failure - Don't declare an exception (log if needed) - Don't resolve the returned URL (actual spec of other find* methods says format of returned URL is unspecified, which gives us flexibility for the future)
Created attachment 127959 [details] Remove throws clause This patch removes the "throws IOException" from the API. I think this didn't make sense because it was only thrown when the argument didn't have the right syntax (which isn't really an IO issue). If we want to throw an exception when the input URL is invalid we could throw IllegalArgumentException, but I'm leaning towards just logging it and returning null.
I have committed this latest patch to HEAD so that we have the API in place for the M6 warm-up build. I'll leave this open to iron out remaining details.
Fixed for M6.
To summarize the chosen solution and to save others a day of investigations: **************************************************************** *** Support for $nl$ in platform:/ URLs was NOT IMPLEMENTED. *** **************************************************************** Instead, method URL org.eclipse.core.runtime.FileLocator#find(URL) was added, which converts an invalid platform:/ URL that contains $nl$ into a URL that can be handled by the PlatformURLHandler, e.g. on URL#openConnection(). This solution is error-prone, and the concerns about "$" being a valid character are hard to justify, given Equinox' mediocre support for other "special" characters like space in URLs (bug 3109, bug 145096).
