sublimelsp / LSP-julia

Julia support for Sublime's LSP plugin using LanguageServer.jl
MIT License
23 stars 1 forks source link

It would be nice to see this package install the server for me #6

Closed rwols closed 3 years ago

rwols commented 4 years ago

I have to install the server manually apparently. Well, I suggest not doing this for now, until you can switch to the ST4 API.

jwortmann commented 4 years ago

Yes, I saw your implementation with the install_or_update method, but I chose to not install the server automatically for now, because there are basically two different ways to use the server:

  1. Add LanguageServer.jl into the default Julia environment. Julia is a "just in time" compiled language, and that means that the server and all dependencies are new compiled each time the server starts. This unfortunately leads to pretty long startup times (I would say the compiler is not particularly fast). I don't think it's a good idea to do that automatically when this LSP-julia package gets installed, because people might not want to have it in their default Julia environment.
  2. Build a precompiled "system image" including the server and its dependencies. I've tested this on Windows and added a command for it which is described in the README. Before doing that, users need to add PackageCompiler.jl into their default Julia environment:
    • start Julia REPL
      julia> ]
      pkg> add PackageCompiler

      After the system image has been created, this PackageCompiler package can be removed again. I refrained from doing that automatically as well, because it will take some time to compile (~ 3 minutes on my PC). Furthermore, it will create a relatively large file (200+ MB), because the precompilation process seems to work more or less on heuristics and additionally bundles the "stdlib" from a Julia installation, as well as the Julia compiler itself. Even if we decide to do that when LSP-julia is installed or updated, we would need to ensure/force that this PackageCompiler package is installed.

As a third alternative, I think VS Code bundles the language server in its Julia extension, but does not precompile it. But I haven't really figured out how that exactly works, or if the same is possible for us. So for now, in my opinion it is the best solution to let the users decide if they want to manually install the language server Julia package (in that case they need to ensure to use the correct version by theirselves - the server's API and configuration keys seem to be quite unstable), or use the precompile command from within ST.

rwols commented 4 years ago

Interesting problems for the language server maintainers. I can understand why we don't pre-install it now.

jwortmann commented 3 years ago

I have updated this package now on the main branch for compatibility with ST4, but it would be great if you could help me with a few questions.

  1. Downloading and installation of the language server is done by the Julia package manager, and the corresponding metadata-files ("Project.toml" and "Manifest.toml") to specify the exact version are provided in the "server" folder. These files will be copied into the Package Storage/LSP-julia folder then, and there is no need anymore to manually install the server and it also doesn't mess with user's default Julia environments. For the copying of the files to work, I currently have a .no-sublime-package file in this package. I'm not sure how the Node-based server packages handle this via the NpmLanguageHandler, but I guess there must be a better way to do it?

  2. What happens if the LSP-* package gets uninstalled, will the server files remain under that Package Storage directory, or will it automatically be removed? I'm asking because if I add the option to precompile the server (or maybe do it by default), the resulting file will be quite large (200+ MB).

  3. I was hoping to replace my hacky and error prone implementation to show a permanent message (with the currently active Julia environment) for all files in the status bar, by the Session.set_window_status_async() method. But unfortunately it doesn't work well (is this method not meant to be used by plugins yet?):

    • this function doesn't seem to work if called within __init__
    • it will remove the status text with the server name (controlled via LSP "show_view_status" setting). I'd ideally like to show both texts separately in the status bar instead.
    • it doesn't work for newly opened files

Maybe more questions soon... :wink:

Thanks in advance!

rwols commented 3 years ago

I'm not sure how the Node-based server packages handle this via the NpmLanguageHandler, but I guess there must be a better way to do it?

Those packages use ResourcePath from sublime_lib: https://github.com/sublimelsp/lsp_utils/blob/d19e6f558d62ceb829fda37af81d8f882e830932/st3/lsp_utils/server_npm_resource.py#L8

If the Julia package manager can take package metadata via stdin, you might be able to pull this off as well.

What happens if the LSP-* package gets uninstalled, will the server files remain under that Package Storage directory, or will it automatically be removed? I'm asking because if I add the option to precompile the server (or maybe do it by default), the resulting file will be quite large (200+ MB).

In the case of nodejs-based server files: the total size is not that large. So it's okay to remove it when the package is disabled and re-download when the package is enabled. For large files I'm personally somewhat hesitant to simply remove them. It also depends on the time it takes to precompile. I think if it takes a long time, keep it in the cache when the package is disabled as users don't like to wait. Everyone is always out of time.

I was hoping to replace my hacky and error prone implementation to show a permanent message

this function doesn't seem to work if called within init

This surprises me :) I'll have to investigate.

it will remove the status text with the server name (controlled via LSP "show_view_status" setting). I'd ideally like to show both texts separately in the status bar instead.

Yeah, good question. I think these kinds of messages are part of the status as well, and users who set "show_view_status" to false don't want to be disturbed. They apparently know what they're doing.

it doesn't work for newly opened files

Correct, feel free to make a pull request for this.

jwortmann commented 3 years ago

Those packages use ResourcePath from sublime_lib

Nice, this seems handy for copying files.

In the case of nodejs-based server files: the total size is not that large. So it's okay to remove it when the package is disabled and re-download when the package is enabled. For large files I'm personally somewhat hesitant to simply remove them. It also depends on the time it takes to precompile. I think if it takes a long time, keep it in the cache when the package is disabled as users don't like to wait. Everyone is always out of time.

Oh, I didn't even think about the disabling/enabling of the package. But yes, it seems better to not remove the server files in that case. What I was actually thinking was if the package gets completely removed via Package Control, then there are still the 200MB server files left which then waste SSD/HDD space somewhere under the "Package Storage" folder (which is even inside of a hidden folder on Windows). But I don't know if there is a good way how to notice that the package was uninstalled, for example it could just have been manually removed without using Package Control.

Yeah, good question. I think these kinds of messages are part of the status as well, and users who set "show_view_status" to false don't want to be disturbed. They apparently know what they're doing.

I realized I accidentally used the exact same status key ("lsp_julia" in this case), which was the reason why it would override the original text. I think Session.set_window_status_async currently ignores the "show_view_status" setting, though.

Correct, feel free to make a pull request for this.

Done :smiley:

jwortmann commented 3 years ago

I added some code a while ago with the ST4 version of this package, to automatically install and update the server. So I will close this issue now as resolved. Not sure how many other users than me are using this package, but since there were no issues created yet about it, I assume that it works fine on other platforms than Windows too.

The option to precompile the server was removed on the ST4 version, because it required some manual steps and maintenance, and the startup time should be reduced a bit with newer Julia versions (still quite long compared to other servers though).

I think I will make a PR for the Package Control repo, so that this package gains more visibility.