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

Update default jdtls version from 1.12.0 -> 1.14.0 #405

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mzacho
Copy link
Contributor

@mzacho mzacho commented Aug 26, 2022

This PR updates the defaults value of lsp-java-jdt-download-url to point to the newest version of jdtls from https://download.eclipse.org/jdtls/milestones

@yyoncho
Copy link
Member

yyoncho commented Aug 26, 2022

Can you check if that version works fine with the java debug plugin we are downloading?

@mzacho
Copy link
Contributor Author

mzacho commented Aug 26, 2022

Yes, for sure, I'm doing some manual testing ATM, will get back after I've tried out the debugger

Due to eclipse-jdtls/eclipse.jdt.ls#2141, this update requires java 17 now. Which is probably a good thing due to a pretty severe CVE found in April 2022.

@yyoncho
Copy link
Member

yyoncho commented Aug 26, 2022

Yes, for sure, I'm doing some manual testing ATM, will get back after I've tried out the debugger

Due to eclipse/eclipse.jdt.ls#2141, this update requires java 17 now. Which is probably a good thing due to a pretty severe CVE found in April 2022.

Then I would like to have some kind of interactiveness here. Like: check if the user is on java 17. If they are not suggest they migrate to it, and if they don't: download a version that works with 11. If we don't do that lsp-java will get tons of bug reports, reddit posts, etc.

@yyoncho
Copy link
Member

yyoncho commented Aug 26, 2022

and if they don't: download a version that works with 11

and of course, warn them about that issue.

@mzacho
Copy link
Contributor Author

mzacho commented Aug 26, 2022

Hmm, yeah okay, that's one way to go down.. Another option is to update the README, so potential new users are aware that lsp-java currently needs Java 17 >=, however, if they really want to use a previous version of Java they can customize lsp-java-jdt-download-url before installing the language server.

This patch wouldn't affect current lsp-java users, unless they delete their jdtls installation.

@yyoncho
Copy link
Member

yyoncho commented Aug 26, 2022

IME that won't work and we will end up getting bug reports for years.

@yyoncho
Copy link
Member

yyoncho commented Aug 26, 2022

I mean I went through that, users are like +lsp +java in their doom config and if it doesn't work they go to redding/discord. This one got reported like 20 times even though there is pinned issue: emacs-lsp/lsp-mode#2615

@mzacho
Copy link
Contributor Author

mzacho commented Aug 26, 2022

Alright, I see what you mean.

I don't have time to look into a user-guided Java update/installation atm, hopefully I'll find the time soon

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

2 participants