Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a Java process command builder. #833

Closed

Conversation

angelozerr
Copy link
Contributor

@angelozerr angelozerr commented Oct 3, 2023

This PR provides the capability to build a Java process command with a builder to start a language server written in Java. The advantage of this builder is:

  • share same code for any language server which need to start a Java process
  • in the future, LSP4E could provide an UI (for language server written in Java) to fill debug port. In LSP4IJ we provide this feature and debug info are stored in the language server settings (so no need to implement a custom UI for a language server)

image

  • in the future LSP4E could provide a JRE combo to choose which JRE to use to start the language server
  • M2E provide an extension point to override the classpath (to add lemminx-maven JAR in XMLLanguageServer). This extension point could be hosted in LSP4E (we have for instance a MicroProfile language server which add extra JARs to provide Quarkus support into MicroProfile LS)

This builder could simplify code:

As you can notice the code is the same (ex : computeJavaPath), etc

Signed-off-by: azerr <azerr@redhat.com>
@mickaelistria
Copy link
Contributor

I find there is almost always a few things, like a system property, a Java version, some classpath, some other configuration file... to look at that make that there is no strong benefit in having such rationalization here. For instance, inLemMinX, we wouldn't use it as you mention it has some extension to contribute to classpath and those extensions wouldn't necessary make sense for other Java-process-based language servers.

@angelozerr
Copy link
Contributor Author

For instance, inLemMinX, we wouldn't use it as you mention it has some extension to contribute to classpath and those extensions wouldn't necessary make sense for other Java-process-based language servers.

You will use it https://github.com/eclipse-wildwebdeveloper/wildwebdeveloper/blob/92ece9fc35e4e028eb249cacf445512bbafac835/org.eclipse.wildwebdeveloper.xml/src/org/eclipse/wildwebdeveloper/xml/internal/XMLLanguageServer.java#L91

And as I tried to explain, for override classpath, I think LSP4E should integrate the M2E extension point which override classpaths and builder could use it, It will avoid coding https://github.com/eclipse-wildwebdeveloper/wildwebdeveloper/blob/92ece9fc35e4e028eb249cacf445512bbafac835/org.eclipse.wildwebdeveloper.xml/src/org/eclipse/wildwebdeveloper/xml/internal/XMLLanguageServer.java#L169 and another LS (like Quarkus could benefit with this extension point) I mean that builder will use this extension point to prefill classpath.

The builder create a standard command list but you can add another command if you wish.

@mickaelistria
Copy link
Contributor

And as I tried to explain, for override classpath, I think LSP4E should integrate the M2E extension point which override classpaths and builder could use it,

I disagree here: the way to extend a Java-based process can differ from a Java process to another. The problem of how to best start a Java LS is not so generic, and the how to extend it is even less. I'm not under the impression that the problems are really generic and hard to solve with existing API, so I'm not convinced LSP4E should try to go further/more specific here.

@akurtakov
Copy link
Contributor

What is the status of this one ?

@angelozerr
Copy link
Contributor Author

What is the status of this one ?

IMHO I think it can be a good idea to have that. We provide that in LSP4IJ an dit is very helpfull. For instance you don't need to take care of generation of debug string command. In LSP4IJ we have a generic debug port and suspend fields and the Java builder generate the proper command (no need to write a custom UI to fill debugging info for Java process).

The Java builder from LSP4IJ will provide an extension point to add some JAR in classpath which is very helpfull too. I tried to convince LSP4E to adopt this idea by giving concrete sample like WWD for staring lemminx, Qute LS, MicroProfile LS, etc

All those language server have the same code.

@eclipsewebmaster eclipsewebmaster deleted the branch eclipse:master May 21, 2024 14:09
@mickaelistria
Copy link
Contributor

The initial target branch master was deleted and replaced by main, so this PR got closed automatically. If this is still relevant, please re-create this PR targetting the main branch.

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.

None yet

4 participants