Community
Participate
Working Groups
HEAD with bug 252431 fixed. IJavaElement#getAttachedJavadoc(IProgressMonitor) should support referenced entries. Currently, the implementation only looks at the raw classpath entry of a package fragment root.
Now with the provision for storing the javadoc location for referenced libraries, it opens more possibilities. 1) One can set the javadoc location for the individual referenced libraries or 2) Or one may decide to set the javadoc location at the referencing classpath entry (i.e. the raw classpath entry) itself To address the latter case, do you think we should look for the javadoc location in both the referenced entry and the referencing entry (in that order)? I think it will depend on the exact use case to decide on this.
It could be interesting to have an automatic fallback to the info from the referencing entry if the referenced entry does not have a Javadoc location. If you do this for Javadoc attachments, you should do the same for source attachments (such that e.g. ISourceReference#getSource() also works when only the referencing entry has source configured). However, this should not delay the fix for the primary issue in this bug. If it's easy to implement for source and doc, then go for it, but if that takes more time, please spawn it into a separate bug.
(In reply to comment #2) > It could be interesting to have an automatic fallback to the info from the > referencing entry if the referenced entry does not have a Javadoc location. Markus, we might need this behavior to support existing users - those who would have stored the javadoc location at the raw entry. If we just switch to use resolved entry, they will be affected. > If you do this for Javadoc attachments, you should do the same for source > attachments (such that e.g. ISourceReference#getSource() also works when only > the referencing entry has source configured). With the patch attached to bug 304083 comment 0, I do see the source attachment paths correctly picked up from the resolved entry's attributes and source content being shown correctly. Is this a wrong use case, perhaps?
> With the patch attached to bug 304083 comment 0, I do see the source attachment > paths correctly picked up from the resolved entry's attributes and source > content being shown correctly. Is this a wrong use case, perhaps? That scenario indeed works fine in HEAD. But when you have: - a.jar, with 'Class-Path: b.jar', with source and Javadoc attachments - b.jar, referenced from a.jar, no source nor Javadoc attachments ... , then you don't get source for a class from b.jar, but you do get Javadoc. > Markus, we might need this behavior to support existing users - those who would > have stored the javadoc location at the raw entry. If we just switch to use > resolved entry, they will be affected. I agree that would be nice, but using the attachment info of the referenced JAR in the first place has higher priority for me than implementing the fallback.
Created attachment 162633 [details] Proposed Patch Patch for Markus' review. This is not to be released and tests yet to be added. Markus, can you please tell me if this is what you had in mind? I think similar changed might be required for org.eclipse.jdt.internal.corext.javadoc.JavaDocLocations#getJavadocBaseLocation(IJavaElement) as well. Please confirm so that I will apply the changes there as well.
(In reply to comment #5) > Markus, can you please tell me if this is what you had in mind? Yes, looks good. > I think similar changed might be required for > org.eclipse.jdt.internal.corext.javadoc.JavaDocLocations#getJavadocBaseLocation(IJavaElement) > as well. Thanks for the heads up. I've released the fix in our JavaDocLocations.
(In reply to comment #6) My first fix in JavaDocLocations threw an IAE for libraries in a classpath container (e.g. for NumberFormat in the com.ibm.icu bundle). Fixed in HEAD. I think your patch has the same problem: entry= root.getRawClasspathEntry(); if (entry == null) { return null; } return getLibraryJavadocLocation(entry); => getLibraryJavadocLocation(..) will throw an IAE for a CPE_CONTAINER entry. BTW: The null test is unnecessary.
Created attachment 163254 [details] Updated patch Patch updated and regression tests added in AttachedJavadocTests#testBug304394 and testBug304394a.
Released in HEAD for 3.6M7.
Verified for 3.6M7 using build I20100424-2000.
Verified.
