vscode-languagetool-linter icon indicating copy to clipboard operation
vscode-languagetool-linter copied to clipboard

Configure managed instance via environment

Open t-gebauer opened this issue 5 years ago • 4 comments

Is your feature request related to a problem? Please describe.

I install languagetool with the nix package manager.
This will install it in the /nix/store/ with a hashed path, e.g. /nix/store/gj9a5ksrgmrmqmw2rwgd38c04xvj4dck-LanguageTool-5.0/.

I can use your extension by specifying this as the class path: /nix/store/gj9a5ksrgmrmqmw2rwgd38c04xvj4dck-LanguageTool-5.0/share/*

But then I would have to change this path every time I update language tool and the hash changes.

Describe the solution you'd like

Allow specifying the path of a language tool executable.

"languageToolLinter.managed.executable": "languagetool-http-server"
// Or maybe
"languageToolLinter.managed.startupScript": "~/start-language-tool-server.sh"

Additional context

The executable might just be a bash script containing something like this:

#! bash -e
java  -cp /nix/store/...-5.0/share/ -jar /nix/store/...-LanguageTool-5.0/share/languagetool-server.jar "$@"

This is actually a shortened version of what the nix package already contains: /nix/store/.../bin/languagetool-server

By installing languagetool with nix, this executable is already in my PATH. So on my system(s) you could just start the server by executing languagetool-http-server. No configuration required.

t-gebauer avatar Oct 17 '20 11:10 t-gebauer

@t-gebauer - I have some concerns around launching with a script vs. pointing to the jar. Via the script, I'm not sure I could capture the pid and shut the process down when VS Code shuts down. And if I can't both start and stop the service alongside the editor, I hesitate to call it "managed". I can look into it more, but I want the extension to stay focused on integrating with LT and not have a bunch of extra code in there to try and manage the LT processes across various platforms.

Might there be an alternative, such as starting the process during login/logout? I'm not familiar with Nix or NixOS. I run Ubuntu, and I have the languagetool server script in my Startup Applications, which does exactly that - start LT when I log in, and shut it down when I log out. Is there something similar you could do in NixOS?

EDIT:

And if I can both start and stop the service alongside the editor, I hesitate to call it "managed".

Should have been: And if I can't both start and stop the service alongside the editor, I hesitate to call it "managed".

davidlday avatar Oct 28 '20 01:10 davidlday

Via the script, I'm not sure I could capture the pid and shut the process down when VS Code shuts down.

That would not be different to managing java directly. Just assume, that the executable behaves the same. If a user is so mad, as to detach the process in their script, or even supply a totally different executable, then it's their own fault.

Might there there be an alternative, such as starting the process during login/logout?

Yes, that would work too. But currently I am only using LT in combination with your extension and not very often. Furthermore, this would require even more configuration from a user, which I would like to minimize.

I might not think of all edge cases right now, but basically the only change I am proposing to your code is something like the following. Process management could stay the same as before.

if (has classpath configured) {
  this.process = execa("java", ["-cp", ..., "-port", port]))
}
else if (has executable configured) {
  this.process = execa(executable, ["-port", port]))
}

I could try to implement this myself if I find some time. Then you can judge whether it could be a worthy addition ;)

t-gebauer avatar Oct 28 '20 08:10 t-gebauer

The problem isn't starting the process. The problem is stopping the process, and knowing if/when to start the process again. Unless I have the pid of the java process (JVM) itself, I can't guarantee that I can stop the server from inside the extension. Based on your snippet above, this.process is the shell script, not the JVM. The JVM is really resilient. If I just kill the shell script that launched LT, the JVM does not die, but becomes an orphan and is adopted by pid 1.

Try this experiment:

  1. Open a terminal and launch the service using the shell script provided by the nix package.
  2. Get the pid of the shell script and the pid of the child java process. For me, I just did ps -ef|grep languagetool in a new terminal.
  3. In a new terminal, kill the shell script using kill (not ctrl-c on the command line).
  4. Check for the pid of the java process.

To be fair, I haven't actually tried to implement this in the extension to prove the problem exists there as well, but I don't see why execa would behave differently.

PRs are always welcome, but the criteria for adding an alternative for launching the managed service is that any method of starting the service within the extension must also guarantee the extension can stop the service, otherwise it isn't truly being managed.

This would definitely be a worthy addition, which is why I'm leaving the issue open. I just haven't been able to solve the second part so far.

davidlday avatar Oct 28 '20 11:10 davidlday

I do not currently use language tool. So I am not very invested in finding a solution right now. But I just found this in my GitHub inbox, and can't stop thinking about it...

I think, the trick to a simple solution, is to shift the responsibility to the user. If the user supplies the path to a custom executable, then it is on them to clean up all child processes. (And to ensure that all arguments are passed to the java process.) For example, the shell script that the nix package manager generates is a good example:

#! /nix/store/6ffp6xbr6j2ryqw2dafbbvv55ggq16iv-bash-4.4-p23/bin/bash -e
exec "/nix/store/7mkpcz3l651khminb41gvlgi73g56x3a-openjdk-8u265-ga-jre/bin/java"  -cp /nix/store/gj9a5ksrgmrmqmw2rwgd38c04xvj4dck-LanguageTool-5.0/share/languagetool-server.jar org.languagetool.server.HTTPServer "$@"

The magic ingredient here is exec which causes java to take over the process. No new process is spawned. The PID remains the same.

(I wish I had included the full script in the original post. I am pretty sure it already included exec then...)

t-gebauer avatar Jun 15 '24 13:06 t-gebauer