Skip to content

Add hover for package fragments#84

Merged
fbricon merged 4 commits into
eclipse-jdtls:masterfrom
othomann:hover-package-fragments
Oct 7, 2016
Merged

Add hover for package fragments#84
fbricon merged 4 commits into
eclipse-jdtls:masterfrom
othomann:hover-package-fragments

Conversation

@othomann

@othomann othomann commented Oct 7, 2016

Copy link
Copy Markdown

I propose these changes to add some hover contents for a package fragment. This should work for the actual package declaration of the current unit if there is a package-info.java file in the same package or for import statements when you hover on the package part of the import.
Let me know what you think.

This also fixes the broken test on Windows.

I integrated all proposals from the previous PR (#83) that I closed by mistake.

Signed-off-by: Olivier Thomann <Olivier_Thomann@ca.ibm.com>
// return computeSourceHover(curr);
}
IJavaElement curr = null;
if (elements.length != 1) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

potential IIOOBE if length == 0, L54 and L58

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll check that.

* IBM Corporation - initial API and implementation
*******************************************************************************/
package copied.org.eclipse.jdt.ui;
package org.jboss.tools.vscode.java.internal;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

org.jboss.tools.vscode.java.internal.javadoc?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, doable.

* IBM Corporation - initial API and implementation
*******************************************************************************/
package copied.org.eclipse.jdt.internal.corext.javadoc;
package org.jboss.tools.vscode.java.internal.hover;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

org.jboss.tools.vscode.java.internal.javadoc?

package org.jboss.tools.vscode.java.internal;

public class Util {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add private constructor if it's supposed to only contain static methods


public static String convertToIndependentLineDelimiter(String source) {
if (source.indexOf('\n') == -1 && source.indexOf('\r') == -1) return source;
StringBuffer buffer = new StringBuffer();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use a StringBuilder

Signed-off-by: Olivier Thomann <Olivier_Thomann@ca.ibm.com>
Fix test Util class

Signed-off-by: Olivier Thomann <Olivier_Thomann@ca.ibm.com>
.filter(e -> e.equals(packageFragment))
.findFirst()
.orElse(null);
if (found == null) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not imply test packageFragment.getKind() == IPackageFragmentRoot.K_BINARY?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I think this is too early to check that. It can be a package from source as well if you import a type from another package. This is a case where it doesn't match the package from the current compilation unit.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough

Signed-off-by: Olivier Thomann <Olivier_Thomann@ca.ibm.com>
@fbricon fbricon merged commit 41a327c into eclipse-jdtls:master Oct 7, 2016
@gorkem gorkem added this to the Mid October 2016 milestone Oct 8, 2016
yaohaizh pushed a commit to yaohaizh/eclipse.jdt.ls that referenced this pull request Jul 4, 2017
adds pom.xml as workspaceContains activation
Fixes eclipse-jdtls#84
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants