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

Add convenience wrappers for starting the server from the command line #748

Closed
joaotavora opened this issue Aug 9, 2018 · 5 comments · Fixed by #2005
Closed

Add convenience wrappers for starting the server from the command line #748

joaotavora opened this issue Aug 9, 2018 · 5 comments · Fixed by #2005

Comments

@joaotavora
Copy link

In joaotavora/eglot#63, we're adding built-int support for this LSP server. It's mostly working.

Though it's possible to start it from the command line (as seen in here), the number and shape of command line arguments that have to be given makes for a brittle implementation on our side.

To slightly complicate things, the invocation is different on linux/OSX/windows and also different between java versions. Our logic currently tests the current operating system and the java version, and tries to locate the correct jar.

Shouldn't these things be in a wrapper script in the LSP server? I'm thinking two scripts one .sh for linux/macosx and one .bat (shudder) for windows. Do you have any thoughts on this? Would you accept a pull request?

@joaotavora
Copy link
Author

ping?

@snjeza
Copy link
Contributor

snjeza commented Aug 17, 2018

@joaotavora If you create a PR we will review it.

@fbricon
Copy link
Contributor

fbricon commented Aug 22, 2018

@joaotavora sound like a nice enhancement, but I'm not sure it'll be easy to make a script that fits all sorts of JDK combinations. I absolutely suck at that sort of things so, feel free to provide a PR, as @snjeza said

@rgrunber
Copy link
Contributor

rgrunber commented Mar 2, 2022

See #2005. If there's anything missing or ways to improve it, feel free to open an issue for it.

@rgrunber rgrunber closed this as completed Mar 2, 2022
@joaotavora
Copy link
Author

@rgrunber thanks! This seems to be "just want the doctor ordered"! I'll open an issue on our side to see if we can integrate it soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants