lite-xl / lite-xl-lsp-servers

Public repo of easily usable lsp servers, that interface with the lite-xl plugin system.
MIT License
8 stars 10 forks source link

Add java support via jdtls. #12

Closed PerilousBooklet closed 3 months ago

PerilousBooklet commented 4 months ago

This PR is incomplete, as it lacks the bundled jdk.

PerilousBooklet commented 4 months ago

How should we approach the bundling of the jdk? Do we download the jre only or the jdk? Also, from what I found it seems that lpm could just download the jdk files into a subfolder of USERDATA/plugins/lsp_java and then we change the command path in the plugin's init.lua. Am I missing something?

adamharrison commented 4 months ago

Hrm. I'd say minimum required to run. So, if that's just the JRE; then I'd say just use that. Maybe even flag it as optional; as many people who're devleoping java will have a system-wide JRE anyway.

Guldoman commented 4 months ago

Ideally it shouldn't be bound to lsp_java as it could be used for other things. So yeah, an optional dependency to a java_jre library sounds ideal.

PerilousBooklet commented 4 months ago

The xml lsp lemminx would also need it.

adamharrison commented 4 months ago

lpm will cache downloads, so not a huge deal to have both, really. If it makes it simpler.

PerilousBooklet commented 4 months ago

Should I add something like this:

{
  "id": "jdk",
    "description": "Production and Early-Access OpenJDK Builds, from Oracle.",
    "version": "21.0.2",
    "type": "library",
    "mod_version": "3",
    "files": [
      {
        "url": "https://download.java.net/java/GA/jdk21.0.2/f2283984656d49d69e91c558476027ac/13/GPL/openjdk-21.0.2_linux-x64_bin.tar.gz",
        "arch": ["x86_64-linux"],
        "checksum": ""
      },
      {
        "url": "https://download.java.net/java/GA/jdk21.0.2/f2283984656d49d69e91c558476027ac/13/GPL/openjdk-21.0.2_macos-aarch64_bin.tar.gz",
        "arch": ["aarch64-darwin"],
        "checksum": ""
      },
      {
        "url": "https://download.java.net/java/GA/jdk21.0.2/f2283984656d49d69e91c558476027ac/13/GPL/openjdk-21.0.2_macos-x64_bin.tar.gz",
        "arch": ["x86_64-darwin"],
        "checksum": ""
      },
      {
        "url": "https://download.java.net/java/GA/jdk21.0.2/f2283984656d49d69e91c558476027ac/13/GPL/openjdk-21.0.2_windows-x64_bin.zip",
        "arch": ["x86_64-windows"],
        "checksum": ""
      }
    ]
}

to make lpm download all the jdk versions needed?

PerilousBooklet commented 4 months ago

This is going to be my first solution to a conflict, so I'd rather ask to be sure: what I think I should do is the following:

git checkout java
git rebase master
git checkout master
git merge java

Am I missing something?

PerilousBooklet commented 4 months ago

Currently there are two problems:

PerilousBooklet commented 4 months ago

I'm fine with it.

PerilousBooklet commented 4 months ago

Ok, the current problems are:

PerilousBooklet commented 4 months ago

I should be able to tell lpm to put the extracted jdk archive inside USERDIR/libraries/jdk, but at the moment I'm not sure if it's possible.

Guldoman commented 4 months ago

The current version of the lsp plugin doesn't support setting env variables, so this is trying to execute an executable named JAVA_HOME=.... See lite-xl/lite-xl-lsp#84.

For now you could try wrapping the command in a shell, so { "bash", "-c", "JAVA_HOME=... jdk..." }.

PerilousBooklet commented 4 months ago

I think the only problem remaining is finding the right command syntax to run jdtls.

PerilousBooklet commented 4 months ago

The last thing remaining is writing the java_command for Windows.

Guldoman commented 4 months ago

Looks like this is broken on Windows because of lite-xl/lite-xl-plugin-manager#74.

Guldoman commented 4 months ago

Is this still trying to execute the python script? This should not depend on the user having python installed and working, so we should run jdtls directly using java.

PerilousBooklet commented 4 months ago

It's still using the script I think.

Guldoman commented 4 months ago

Btw lite-xl/lite-xl-lsp#97 adds env support to the LSP plugin. If you try using it, let me know how it goes.

PerilousBooklet commented 4 months ago

I'll try right away. Who needs to sleep anyway.

PerilousBooklet commented 4 months ago

Seems to be working.

Guldoman commented 4 months ago

Awesome! Thanks for testing!

PerilousBooklet commented 4 months ago

Now we have to wait until the env var PR is merged in lsp. And the windows command check.

Guldoman commented 4 months ago

And to avoid using the python script.

PerilousBooklet commented 3 months ago

I think we're ready to merge.

Guldoman commented 3 months ago

Thank you!