Allow customizing command to start languagetool server

emacs-languagetool / flymake-languagetool

Flymake support for LanguageTool

GNU General Public License v3.0

33 stars 11 forks source link

Allow customizing command to start languagetool server #6

Closed ipvych closed 2 years ago

ipvych commented 2 years ago

Some linux distributions provide shell scripts to launch languagetool server so that users can launch it from CLI without knowing exact path to languagetool jar file. It would be nice to be able to set command to run that script instead of searching for location of jar file.

tpeacock19 commented 2 years ago

Can you give more information on the distro you that are using and what this command is?

ipvych commented 2 years ago

I use NixOS, the command is languagetool-http-server with this content:

#! /nix/store/...-bash-5.1-p16/bin/bash -e
exec "/nix/store/...-openjdk-17.0.1+12/bin/java" -cp /nix/store/...-LanguageTool-5.6/share/languagetool-server.jar org.languagetool.server.HTTPServer "$@"

Where ... is a hash I omitted to make it more readable, I can send command with hashes if you need it.

ipvych commented 2 years ago

I quickly looked at languagetool package in archlinux and it seems to provide similar script. The difference is that it wraps every languagetool command in single script, unlike on NixOS different scripts for different commands.

tpeacock19 commented 2 years ago

Okay, I do think it is a good idea to provide a server command. Additionally we should probably define the java executable using (executable-find "java"). I'll leave some comments on your PR.

tpeacock19 commented 2 years ago

can you test the feat/server-command branch to see if satisfies what you need. I have decided to go a slightly different route.

ipvych commented 2 years ago

Predicate in flymake-languagetool--start should be changed to

(when (or flymake-languagetool-server-command
          flymake-languagetool-server-jar))

Documentation needs to be updated in some way to inform user that flymake-languagetool expects languagetool server to be run on port flymake-languagetool-server-port and user should adjust flymake-languagetool-server-command so that it starts command on correct port. Or we can be just always append "--port" flag to anything user sets in flymake-languagetool-server-command. I think it will work well since most distributions should make their wrappers pass additional arguments directly to server, at least both nixos and arch does this.

Other than these two nitpicks I think it looks good

tpeacock19 commented 2 years ago

good catch on the predicate.

The server-port is the default value for languagetool. If a user defines a custom command and they don't change the port it will work as expected. However, the docstring should be updated to include its use in the api url requests. I don't think appending is the right approach.