Community
Participate
Working Groups
3.2 Before fixing bug 142530, resources in JAR files were shown in package fragments also for folders that weren't legal package fragments. We had discussion here about the fix, and found that rendering the resources as flat list is not a good solution. We suggest to turn back the fix and accept the problematic package fragments, or better, add a new element for folders in JARs. To be backward compatible I suggest to add a new interface that still implements IStorage: interface IJARFileResourceEntry extends IStorage { isFile(): boolean // true if file, false if folder getParent(): Object // either a IJARFileResourceEntry or a IJavaElement getChildren(): IJARFileResourceEntry[] // empty if entry is a file } or if you don't mind adding two interfaces: IJARFileResourceFile and IJARFileResourceFolder I think this the best solution and also fixes our problem of not being able to reveal such elements (bug 142048)
+1 to revert the fix for bug 142530 and bug 148657.
+1 for a clean fix: Invalid package names in jars should not be package fragments (causes problems such as bug 151112) but some other kind of containers.
Created attachment 60183 [details] Proposed fix
Martin, please let me know if this works for you. If it is ok, I'll ask the PMC for approval on this new API. Note we will have to coordinate the releasing of this patch, since without your changes, we will show only the first level directory in the Package Explorer.
I tried the patch and it works. In getParent, shouldn't it also state that it can return a IPackageFragmentRoot. Or is this not possible?
(In reply to comment #5) > In getParent, shouldn't it also state that it can return a > IPackageFragmentRoot. Or is this not possible? You're right. Thanks for noticing it. Please see new patch for a fix.
Created attachment 60250 [details] Fix with the possibility of the parent to be an IPackageFragmentRoot
+1 for API change
I think JarEntryDirectory#getContents() should throw a CoreException rather than returning an empty stream. IStorage#getContents() already tells that subtypes will refine the exception cases, so IJarEntryResource#getContents() could specify that an exception is thrown if isFile() is false. Anyway, all usage sites of IStorage (esp. 'instanceof IStorage') should be checked for problems like bug 176210 comment 2.
Unfortunately existing clients see the non-Java resources as IStorages, which doesn't refine the exception case. Thus they are not prepared for handling the CoreException. Returning an empty stream is not ideal, but I think it is more acceptable to see an empty editor rather than an internal error dialog.
> Thus they are not prepared for handling the CoreException. If they're no prepared, then it's a bug in their implementation. From IStorage: /** * [..] * @exception CoreException if the contents of this storage could * not be accessed. See any refinements for more information. */ public InputStream getContents() throws CoreException; E.g. the case from bug 176210 comment 2 is handled correctly because StorageDocumentProvider catches the CoreException and nicely shows the error message in the editor. I find that more informative and less confusing than an empty editor.
Although I see Markus point, I'd suggest to keep the behaviour as before (return empty stream). The correct solution would have been to not use IStorage to represent folders. But that's legacy and also not really a major issue.
This change will also affect PDE/UI, see bug 176635.
Patch released for 3.3M6 in HEAD.
*** Bug 22376 has been marked as a duplicate of this bug. ***
Verified for 3.3 M6 using v_743.
