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

Support python wrapper for Windows #2808

Merged
merged 2 commits into from
Aug 23, 2023
Merged

Support python wrapper for Windows #2808

merged 2 commits into from
Aug 23, 2023

Conversation

lesleyrs
Copy link
Contributor

Hi, after adding jdtls bin to PATH variable on windows you can't run the lsp server because they are python scripts. I know you can use the java command but then you would need to use a different command for Linux and Windows and it requires you to set the options manually.

Adding a simple batch file to the bin directory allows it to be run from path on Windows with just "jdtls". I think putting it in scripts directory would copy it there? Didn't build it myself.

This change is important for editors, particularly for https://github.com/helix-editor/helix/blob/master/languages.toml#L36 to be able to work out of the box.

Discussion about this here helix-editor/helix#5861.

@eclipse-ls-bot
Copy link

Can one of the admins verify this patch?

@lesleyrs
Copy link
Contributor Author

Maybe change it to python %~dp0/jdtls %1 %2 to be able to pass options? I'm not familiar with jdtls so not sure if that's useful with the scripts.

@rgrunber
Copy link
Contributor

Yes, putting it in the scripts directory would copy it to the final destination.

Maybe change it to python %~dp0/jdtls %1 %2 to be able to pass options? I'm not familiar with jdtls so not sure if that's useful with the scripts.

It would probably need to be %* to pass all other arguments.

@lesleyrs
Copy link
Contributor Author

Ok done, I don't have an eclipse account though. If there's a problem with that, somebody can just close this PR and create a new one.

@datho7561
Copy link
Contributor

I can take a look at this some time today, just setting up Helix on my Windows box...

Copy link
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

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

Looks good to me. I added eclipse.jdt.ls\org.eclipse.jdt.ls.product\target\repository\bin to PATH, and I can get validation and go-to-definition in Helix.

@rgrunber rgrunber added this to the End August 2023 milestone Aug 23, 2023
@rgrunber
Copy link
Contributor

Going by https://www.eclipse.org/projects/handbook/#contributing-eca maybe we can avoid re-creating this PR by just confirming what signing the ECA implies.

@lesleyrs :

  1. Is this contribution written by you ?
  2. Do you have the necessary permission to make this contribution ?
  3. Do you agree to make this contribution under the EPL-2.0 license that JDT-LS uses ?

@lesleyrs
Copy link
Contributor Author

Going by https://www.eclipse.org/projects/handbook/#contributing-eca maybe we can avoid re-creating this PR by just confirming what signing the ECA implies.

@lesleyrs :

  1. Is this contribution written by you ?

Yes it has been written by me.

  1. Do you have the necessary permission to make this contribution ?

Yes

  1. Do you agree to make this contribution under the EPL-2.0 license that JDT-LS uses ?

Sure no problem.

@rgrunber rgrunber merged commit eba4458 into eclipse-jdtls:master Aug 23, 2023
4 of 5 checks passed
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