alexaandru / nvim-lspupdate

Updates installed LSP servers, automatically
MIT License
91 stars 5 forks source link

Texlab, cargo builds from git #1

Closed skyegecko closed 3 years ago

skyegecko commented 3 years ago

Hi,

First of all, nice project :)

I'm a part-time Texlab user, and normally update by building directly from their git repo using

cargo install --git https://github.com/latex-lsp/texlab.git --locked

I looked at integrating this into nvim-lspupdate but quickly noticed that this would be incompatible with the default behaviour of combining multiple installs into a single command with the %s expansion.

Based on this, I think it would be nice to have an alternative way to run install commands that can install only one package at a time. I can think of two ways of doing this:

  1. Make separate commands (e.g. cargo vs cargogit), and for each command register whether it has the ability to concatenate multiple packages into a single command line.
  2. Register for each package whether it may be installed in the same command line as other packages or not. This would allow any extra flags such as --git in my example to be added to the package config.

What are your thoughts on this, would you be interested in supporting this approach?

alexaandru commented 3 years ago

Hello,

I think the biggest problem right now, is not the command itself (which you already can override and/or create additional commands), but the fact that you cannot override the config. Whatever is the config for that LSP (and right now, for texlab is the empty string), you are stuck with it.

I think I should just allow end users to override that as well, and then they can update the config on their end. Combined with adding and/or overriding commands, this should give all the freedom needed to experiment on their own.

The problem of concatenating packages still remains, but I don't see how that would affect you in this particular case, since you only have one package. Oh, maybe you use multiple cargo based LSPs? Otherwise, if I open up config, then you could override texlab to say cargo|https://github.com/latex-lsp/texlab.git and the cargo command to say cargo install --git %s --locked and that should do it (or introduce a new, cargogit command as you suggested).

Until then, I'm gonna add the package name for texlab right now, it's a tiny change anyway.

alexaandru commented 3 years ago

Done, hope this helps: https://github.com/alexaandru/nvim-lspupdate/commit/d450e9cc9d1f5df486c4482b169a80d6faf84cad

alexaandru commented 3 years ago

There's one problem though with the above commit: it only does an install, failing to do what the package name implies: update :)

Looking at the manual https://doc.rust-lang.org/cargo/commands/cargo-install.html I see there is an -f flag to force a reinstall. So perhaps, that is the way to "install or update" with a single command, achieving this project's goals? Also, it seems lke the --locked flag works against that, so I'd remove that. Of course, end users can override and add it if they need it, but for the purpose of this package (lsp update), the goal is to update stuff, not to keep it locked to a particular version. Thoughts?

skyegecko commented 3 years ago

cargo install will update the package if an update is available; if the latest version is already installed then it's a noop. So just using cargo install should be sufficient. The only time this might not be enough is if a user has updated their rust toolchain and really wants their packages to be rebuilt with the newer version, but tbh I don't think this is a likely use case.

alexaandru commented 3 years ago

Excellent, thank you very much. Then it is a perfect fit :)

alexaandru commented 3 years ago

Please let me know if the solution implemented works for you and if it's ok to close this ticket. Thank you :)

skyegecko commented 3 years ago

Oops sorry! Yes, this is working for me. Thanks!

alexaandru commented 3 years ago

Awesome, thank you very much! :) Cheers!