Community
Participate
Working Groups
Open Declaration doesn't work in Javadoc hover on IPackageFragment. This action is available in the toolbar of the Javadoc view and in rich Javadoc hovers. It's also available via the element icon in the Javadoc header (the icon is a clickable link). If there's a package-info.(java|class) or package.html file, then we should open that one in an editor. Otherwise, we should open the Package Explorer and select/reveal the package fragment.
Open Declaration (F3) and hyperlinking also doesn't work on package declarations or references. I think it would make sense to enable Open Declaration on packages in general.
(In reply to comment #1) > Open Declaration (F3) and hyperlinking also doesn't work on package > declarations or references. I think it would make sense to enable Open > Declaration on packages in general. I disagree. I would not want the editor to open when I hit F3 on a package in the Package Explorer. And adding this inside the editor doesn't make much sense either for me.
> Otherwise, we should open the Package Explorer and select/reveal the package > fragment. So far, 'Open Declaration' always opened the editor (and the icon indicates this too), but I'm fine to stretch the concept here a bit.
I agree that selecting the package in the Package Explorer is stretching the concept. However, when there's no package-info, it's really the package directory that declares the package. That's similar to a Java class that lacks an explicit constructor declaration: There, F3 on a class instance creation also jumps to the enclosing element -- the class declaration. (In reply to comment #2) > And adding this inside the editor doesn't make much sense either for me. Jumping to a package declaration can be handy if you need to add an annotation there (e.g. @NonNullByDefault). I don't see a big difference between jumping to a package declaration and jumping to any other element.
(In reply to comment #4) > I agree that selecting the package in the Package Explorer is stretching the > concept. However, when there's no package-info, it's really the package > directory that declares the package. Open [*] is the concept that opens editors. I want to keep this clean. As discussed, we could alternatively open an editor with a template that can then be stored. Of course this only works for source. Seems like a bit overkill and/or a separate feature request. > That's similar to a Java class that lacks an explicit constructor > declaration: There, F3 on a class instance creation also jumps to the > enclosing element -- the class declaration. But it stays in the editor. > (In reply to comment #2) > > And adding this inside the editor doesn't make much sense either for me. > > Jumping to a package declaration can be handy if you need to add an > annotation there (e.g. @NonNullByDefault). I don't see a big difference > between jumping to a package declaration and jumping to any other element. Agreed.
OK, to summarize: In the Javadoc view and in the Javadoc hover, the Open Declaration button should open the package info file if one exists, and fall back to selecting the package in the Package Explorer if no file is available. In the editor, Open Declaration should only open the package info file if one exists, and otherwise show the error message like today.
Markus, below are the points taken care during fixing this bug. 1. Open declaration will open package-info.java, package-info.class or package.html files if it is present in source or binary container. 2. An exception to the above is for package.html present in a Jar file. I could not find a EditorUtilty that opens a normal file (i.e. not an IFile). Hence in this case the Javadoc Hover silently fails(nothing happens when the user clicks on Open Declaration), Javadoc View pops an error dialog. 3. The case when the package javadoc is retrieved from JavaElement.getAttachedJavadoc(), then open declaration has no file input to open the editor. The default behaviour in JavadocView is to pop an error dialog as in the above case. JavadocHover silently fails again. 4. If there is no package Javadoc file for a package present in source container, then Open Declaration selects the corresponding package in PackageExplorer View. 5. Regarding Junit testcases for these cases, since all the scenario is about opening the underlying resource associated and also an action is performed so as to activate the scenario, i am not able to figure out what kind of JUnits to write for them. Let me know what were the expected behavior in the above cases/ have i handled them properly? Also what other cases needs to be handled?
> 1., 4. Sounds good. > 2., 3. See comment 6: we should not show an error dialog, but: > fall back to selecting the package in the Package Explorer if no file is > available. I.e., in case the package file is not available as a file (e.g. it's in a Jar or an external URL), we also fall back to selecting the package in the PE. If there are multiple IPackageFragments that could be selected, use the first one on the build path. > 5. Junit testcases That's OK. I think this is one of these "end of the food chain" areas, where writing and maintaining tests is more work than we want to spend, since the expected gains are not high enough.
Created attachment 225927 [details] Proposed Fix. In this fix 1. If user invoke Open Declaration from Javadoc View or Javadoc Hover or from the Java editor, then the available package-info.java or package-info.class or package.html file of that package is opened in the corresponding editor. 2. If the file is present in an archive, or if there is no Javadoc file associated to that package, then the corresponding package is highlighted in the package explorer view. 3. If there are multiple IPackageFragments that could be selected, then the first one from the build path is selected.
(In reply to comment #9) > Created attachment 225927 [details] [diff] I like the behavior, but it doesn't implement what comment 6 summarized: > In the editor, Open Declaration should only open the package info file if one > exists, and otherwise show the error message like today. Implementation comments: 1. JavaModelUtil#isBinary(IJavaElement) doesn't work for all arguments. Utility methods must not leave room for interpretation/surprises. 2. Code flow in JavadocHover.OpenDeclarationAction#openInEditor(IPackageFragment) is overly complicated. foundPackageJavadoc is not necessary. Use return statements. 3. OpenAction#getPackageFragmentObjectToOpen(IPackageFragment) is supposed to be a pure function that doesn't have side effects. The "view.selectAndReveal..." must be moved out. And you should use "view.tryToReveal...", so that it works more like Show In > Package Explorer (e.g. asks to remove filters).
(In reply to comment #10) The "package info file" here means all 3 variants. The important point is not to select the package fragment in the Package Explorer when Open Declaration is executed in the editor (but behave like before in this case).
Created attachment 226214 [details] Proposed Fix. Thanks Markus for the clarification. Now Open declaration from editor will show error message if the corresponding package Javadoc does not exist. Took care of the code review comments. 1. Removed the utility method. 2. Refactored JavadocHover.OpenDeclarationAction#openInEditor(IPackageFragment). 3. used view.tryToReveal(element) and is moved out of the getPackageFragmentObjectToOpen method.
(In reply to comment #12) Changes in JavadocHover look mostly good, but the package should also be opened when I click the package icon in the hover. Please rename "iFile" to "file" and make format all comments the same way, e.g.: // select the package in the Package Explorer ... In the editor, F3 shouldn't open an error dialog, but show the error in the status line like before.
Fixed with http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=e775f37ffdd45a30062233ba3bde2bd8c413bbd4 , so that we can test it tomorrow.
Verified in I20130129-2000.
