palantir-java-format icon indicating copy to clipboard operation
palantir-java-format copied to clipboard

Is there interest in adding an Eclipse plugin?

Open ChristianCiach opened this issue 2 years ago • 13 comments

This repository contains the directory eclipse_plugin, but it is completely unmaintained and not part of the gradle build.

I've fixed this plugin for our company and I am willing to create a pull request to add my changes to this repository.

Before I do the work to create a pull-request I want to ask if there is even any interest to release a plugin for Eclipse.

I've not yet integrated the eclipse_plugin subproject with the parent Gradle build. The eclipse_plugin can currently (after applying my fixes) be built as a standalone maven project. If there is interest in actually releasing an Eclipse plugin, I will try to integrate it with Gradle before sending the PR.

ChristianCiach avatar Mar 23 '22 15:03 ChristianCiach

Internally we have pretty much standardized on using Intellij, which is why the eclipse plugin did not really get much attention unfortunately.

Generally happy to accept such a contribution! I have some small concerns that we'll probably not do a good job as steward, as we generally not use the plugin ourself which always makes things harder to maintain. However everything is better than the current state and would be great to revive the plugin if its helpful to others!

Also not super familiar with what the required steps are for getting an eclipse plugin published right now but I'm sure we can figure it out! :)

fawind avatar Mar 25 '22 21:03 fawind

Thanks for your response! For the reasons you stated I am considering if maybe maintaining the plugin by myself as a standalone project may the the better alternative. I am not really sure what's the best option here, but I will think about it.

Also not super familiar with what the required steps are for getting an eclipse plugin published right now

Me neither. But there is no need to "publish" it anyway. We just need to add the build artifact (a single zip file) as a github release artifact and users can easily install it by copying the zip file to the dropins directory of their eclipse installation.

ChristianCiach avatar Mar 25 '22 21:03 ChristianCiach

I am just noticing that you currently don't push any artifacts to the github release page of this project.

ChristianCiach avatar Mar 25 '22 21:03 ChristianCiach

Thanks for your response! For the reasons you stated I am considering if maybe maintaining the plugin by myself as a standalone project may the the better alternative. I am not really sure what's the best option here, but I will think about it.

Sure, let me know what you decide! Happy to accept the contribution. However also happy to include a link to your project if you decide that you want to keep ownership over it!

I am just noticing that you currently don't push any artifacts to the github release page of this project.

Yeah, we currently publish artifacts directly to maven-central and the gradle plugin portal instead of the github release entry. We might be able to change that or otherwise we can publish to eclipse artifacts to maven-central as well.

Me neither. But there is no need to "publish" it anyway. We just need to add the build artifact (a single zip file) as a github release artifact and users can easily install it by copying the zip file to the dropins directory of their eclipse installation.

Yep, that would also work. Reading a bit more into it, it looks like you just need to build and host a static "update site" somewhere (article). I think you should be able to host that using github-pages. This would allow users to directly download the plugin using the eclipse marketplace.

fawind avatar Mar 25 '22 21:03 fawind

I've forked this project to integrate the eclipse-plugin directory as a Gradle subproject. See my commits here: https://github.com/ChristianCiach/palantir-java-format/commits/eclipse-plugin

I could create a PR for this project, but for the reasons stated by @fawind this is probably not the best way to go forward. I will probably create a proper standalone project for this plugin including a proper eclipse "update site" to automate updates, but this will take a few days or weeks because I am rather busy at the moment.

Nevertheless, I just wanted to present the current state of my work. As you can see, integrating the plugin into the main gradle build as a subproject is not very hard at all.

My fork could also be of interest to other readers of this issue who may want to build the plugin themselves until I've extracted the plugin into a standalone github project.

The code of PalantirJavaFormatter.java is mostly copied from upstream google-java-format, because their eclipse plugin is better maintained than this one.

Noteworthy:

  • The gradle task publish will publish the artifact com.palantir.javaformat:eclipse-plugin to a maven repository, similar to the existing idea-plugin.
  • The jar-file will contain all the jar files that the plugin depends on as an OSGI bundle.
  • When executing the jar task directly, the produced jar file will be named com.palantir.javaformat.eclipse-${version}.jar to conform to the eclipse plugin naming conventions.
  • The produced jar file can be installed by copying it into the dropins folder of the eclipse installation.
  • The plugin must be build with Java 14 or later, otherwise the plugin will crash due to the missing class com.palantir.javaformat.java.java14.Java14InputAstVisitor. This has actually nothing to do with this plugin, because this class is included conditionally by the palantir-java-format subproject.
  • When running Eclipse on Java 16 or newer, Eclipse must be started with multiple --add-exports parameters (see https://github.com/google/google-java-format#as-a-library). These parameters can be set in the eclipse.ini as -vmargs. The idea-plugin workarounds this issue by starting the formatter in its own process, but this is not possible here because eclipse has no API to retrieve project information (like the JVM path) from within a CodeFormatter implementation.
  • I am currently building the plugin against org.eclipse.jdt.core version 3.29.0, which is the version used by the current Eclipse release 2022-03. This means that the plugin will probably only work with the current Eclipse release, but not with older releases. I could build the plugin against an older version of jdt.core without issues.

ChristianCiach avatar Mar 28 '22 16:03 ChristianCiach

hey @ChristianCiach,

I am trying to add the eclipse plugin by cloning your project, and running ./gradlew :eclipse-plugin:jar. After the task is run succesfully, it will generate com.palantir.javaformat.eclipse-2.20.0-43-g3c2f193.jar in the build directory which I copied to eclipse/dropins directory. I also added the exports to the ini file but I don't see the plugin being installed/activated. There is no error that I can see.

Is there something that I did wrongly? Thanks!

bwgjoseph avatar Jul 28 '22 18:07 bwgjoseph

@bwgjoseph

It should work exactly like that. I assume you are using at least Eclipse 2022-03 or newer? It worked for me and many of my coworkers were able to install the jar this way.

It's not easy to debug your issue just like that :/

I am sorry that I didn't yet create a proper eclipse update site for this plugin. I have been very busy and I will be on vacation for the next three weeks, so please be patient for a while longer. As soon as I've created the update site, I will announce it here and will close the issue.

ChristianCiach avatar Jul 28 '22 18:07 ChristianCiach

I'm actually using sts-4 which is based off the latest eclipse

image

Am I supposed to see a new profile like palantir over here?

I'm not sure what I should be expecting after dropping the jar to the dropins directory. I did restart the IDE though.

bwgjoseph avatar Jul 28 '22 18:07 bwgjoseph

Am I supposed to see a new profile like palantir over here?

Actually, no. You are supposed to see a new Formatter implementation below the profiles. The selected profile actually doesn't matter when selecting the palantir-java-format ~~profile~~ implementation. The google-java-format plugin works the same way.

ChristianCiach avatar Jul 28 '22 18:07 ChristianCiach

According to https://github.com/spring-projects/sts4/blob/4.15.1.RELEASE/eclipse-distribution/pom.xml this version of STS seems to be based on Eclipse 2020-09. This is probably the issue. But this should be easy to fix. You have to build my plugin against an older version of org.eclipse.jdt:org.eclipse.jdt.core. You can downgrade the version in versions.props and then have to recreate the versions.lock file. I am on mobile right now, so I am not able to tell you the exact version of eclipse.jdt.core to use.

ChristianCiach avatar Jul 28 '22 18:07 ChristianCiach

ah! Thanks, didn't see that.

Only thing was that I encountered this exception (https://github.com/palantir/palantir-java-format/issues/779) when it tries to format and managed to resolve it by following the comment in https://github.com/google/google-java-format/issues/787#issuecomment-1198389887 to export those args

Now it seems to be working. Well, the only thing is that your fork is now currently behind, and I'm not exactly sure what changes, and if the rules remains the same when using your version in eclipse, and intellij-java-palantir-format plugin

bwgjoseph avatar Jul 28 '22 18:07 bwgjoseph

According to https://github.com/spring-projects/sts4/blob/4.15.1.RELEASE/eclipse-distribution/pom.xml this version of STS seems to be based on Eclipse 2020-09. This is probably the issue. But this should be easy to fix. You have to build my plugin against an older version of org.eclipse.jdt:org.eclipse.jdt.core. You can downgrade the version on version.props and then have to recreate the versions.lock file. I am on mobile right now, so I am not able to tell you the exact version of eclipse.jdt.core to use.

It's not actually, it might be wrong/outdated info? I'm not quite sure. See https://twitter.com/springtools4/status/1538049958376615936. It's based off 2022-06

bwgjoseph avatar Jul 28 '22 18:07 bwgjoseph

It's not actually, it might be wrong/outdated info? I'm not quite sure. See https://twitter.com/springtools4/status/1538049958376615936. It's based off 2022-06

Alright, thanks! I see now that I misinterpreted this part of the pom.xml. Again, I am on mobile, so this stuff is hard to make sense of on a small screen :)

ChristianCiach avatar Jul 28 '22 18:07 ChristianCiach

Hello, what's the status on this? Is the implementation ready? Will it be merged in this repository or kept in a fork? Thanks

vtintillier avatar Dec 03 '22 21:12 vtintillier

Sorry, I had no time to create a proper update site. You are welcome to compile my fork by yourself :)

Since this won't be merged here, there is no point in keeping this issue open.

ChristianCiach avatar Dec 03 '22 22:12 ChristianCiach