Make lsp server installation optional

[Tanish2002]

⚠️ Please verify that this feature request has NOT been suggested before.

• I checked and didn't find a similar feature request

🏷️ Feature Type

API Additions

🔖 Feature description

As previously discussed on discord:

• Lsp servers shouldn't be installed when enabling lsp for a language like vim.lsp.rust.enable = true;.
• I myself use flake devshells on per project basis to get lsp servers and having them installed by neovim even when I don't need them will waste space.
• Another common scenario for this will be, A user uses 2 editors say emacs and neovim and they want the lsp server to be available to both. This flake will install the server and it will be exclusively available to only neovim. Which would result the user to install another copy of the same server system wide so it's available to emacs.

✔️ Solution

I believe one possible solution is:

• provide a config option for every language the flake supports.
• The option can be something like vim.lsp.rust.lspServerPackage
• This option could have 2 values either a string or a package.
• That way users can use the same package for both neovim and emacs(as under the hood nix will just symlink the package to make it available to both)
• The string option will be helpful for my usecase. That is I can just set it to "rust-analyzer" so in lsp config it'll be cmd = {"rust-analyzer"}, Instead of cmd = {"${pkgs.rust-analyzer}/bin/rust-analyzer"}, (link to what I'm talking about)
• So when I'm in my devshell it'll just search for rust-analyzer in the path which should be available to it.

The same can be done for formatters as well.

❓ Alternatives

No response

📝 Additional Context

The default value for this option can be left same to what is used now. So for rust it'll be pkgs.rust-analyzer. So nothing breaks for people who are using this already

[NotAShelf]

I have a general idea of how I might add this change. Will take a look as soon as I find some extra time.

[Amanse]

If i remember correctly the cmd in lspconfig doesn't need absolute path to the binary, just giving "rust-analyzer" should make it search in PATH, the current thing can be the default string value

[Amanse]

cmd = mkOption {
        description = "lsp command to run";
        type = types.str;
        default = "${lib.getExe pkgs.rust-analyzer}";
       };

and then in config

server = {
             capabilities = capabilities,
             on_attach = rust_on_attach,
             cmd = {"${cfg.lsp.cmd}"},
             settings = {
               ${cfg.lsp.opts}
             }

This is working for me with rust-analyzer(I installed rust-analyzer with rustup)
with config like

rust.lsp.cmd = "rust-analyzer";

[NotAShelf]

If i remember correctly the cmd in lspconfig doesn't need absolute path to the binary, just giving "rust-analyzer" should make it search in PATH, the current thing can be the default string value

That is correct, but for maximum purity we prefer passing the full path to the executable instead of trying to read from PATH. I'll tweak the system slightly to accept a package or a string, and if it is a package the mainProgram will be executed. If it is a string, then the value will be parsed as is.

[MonaMayrhofer]

A workaround - especially for the rust use case - is to use a custom derivation as the lsp package that just calls rust-analyzer on the path.

        rust = {
          enable = true;
          crates.enable = true;
          lsp = {
            enable = true;
            package = writeShellApplication {
              name = "rust-analyzer";
              text = ''
                rust-analyzer "$@"
              '';
            };
          };
        };

[Amanse]

      package = mkOption {
        description = "rust-analyzer package";
        type = with types; oneOf [string package];
        default = pkgs.rust-analyzer;
      };

and then

          server = {
            capabilities = capabilities,
            on_attach = rust_on_attach,
            cmd = {"${
          if builtins.isString cfg.lsp.package
          then cfg.lsp.package
          else lib.getExe cfg.lsp.package
        }"},
            settings = {

mainProgram doesn't seem to exist on multiple packages, rust-analyzer included, replacing lib.getExe with that should be close to what you were talking about. (idk why lib.types.isType always returns false lol, could use that with lib.types.package otherwise)

[NotAShelf]

mainProgram doesn't seem to exist on multiple packages, rust-analyzer included
the new nixpkgs convention is that mainProgram needs to be explicitly defined as it will no longer be defaulted to `pname - I'll open a tracking issue, so we are aware which lsp packages need manual intervention from us (I might even go PR each package that doesn't have a mainProgram)

Since we'll be checking if it's a package or a string we should probably add a function to the shared library, that goes something like

pkgOrStr = v: if builtins.isString v then v else lib.getExe v;

[Amanse]

(I might even go PR each package that doesn't have a mainProgram)

the hero we need, not the hero we deserve

mainProgram does return not found on rust-analyzer, I'm using pkgs.rust-analyzer.mainProgram, is that the wrong way to write?

pkgOrStr is nice, I'll try with that, lesgoo

[NotAShelf]

mainProgram does return not found on rust-analyzer, I'm using pkgs.rust-analyzer.mainProgram

lib.getExe (unless they changed it as well) should return the meta.mainProgram attribute of a package.

[Amanse]

ohh it's under meta, that makes sense, that's why

[NotAShelf]

Should be done as of #134