llm.nvim icon indicating copy to clipboard operation
llm.nvim copied to clipboard

check for llm-ls in PATH ?

Open teto opened this issue 2 years ago • 5 comments

When developing locally, when using mason or if you built your own binary because your platform is not supported, you can set the lsp.bin_path setting to the path of the binary.

I am a bit perplexed by that, I had llm-ls in PATH and installing llm.nvim tried to install its own (Making TUI unresponsive for 4/5 seconds with no warning). Maybe just check with executable("llm-ls") if it's available ?

NB: I ma just starting but considering the relative complexity of the plugin, adding a health.lua to be able to run :checkhealth llm.nvim could help users

teto avatar Nov 04 '23 22:11 teto

Hello @teto, thanks for the feedback. I like the idea of a healthcheck. I'll see what I can do.

Not sure about checking the PATH for llm-ls though. It's not meant to be used as a CLI and for now there is no other usage outside the extensions. So I'm not sure it makes that much sense.

Making TUI unresponsive for 4/5 seconds with no warning

I'll add a notification warning that llm-ls is going to be downloaded.

McPatate avatar Nov 05 '23 17:11 McPatate

Not sure about checking the PATH for llm-ls though.

I believe most programs dont hardcode the paths to their software though, so they look in PATH, that's what it's for.

This might be a bit exceptional (but a common pattern with the nix package manager) but this is how nix configures neovim: It wrap the neovim binary in a shellscript that does:

export PATH=/path/to/lualanguage-server/bin:/path/to/yaml-language-server/bin nvim $@`

with all the tools it needs: lua-language-server, yaml-language-server etc. so neovim finds what it needs its in PATH but a user wont see it in its shell

teto avatar Nov 05 '23 18:11 teto

In that case let's add a check of PATH in the language server's setup function

McPatate avatar Nov 05 '23 18:11 McPatate

I feel like it could even deprecate the lsp.bin_path config option such that the change doesn't increase the code maintainance. Instead of setting lsp.bin_path, A user could just update the PATH environment variable in its neovim config https://vim.fandom.com/wiki/Environment_variables .

teto avatar Nov 05 '23 19:11 teto

I'd also note that the downloaded binary is dynamically linked, and on NixOS that means it can't find the necessary libraries.

❯ ldd /home/adam/.local/share/nvim/llm_nvim/bin/llm-ls-x86_64-unknown-linux-gnu-0.5.3
        linux-vdso.so.1 (0x00007ffc4d3f7000)
        libstdc++.so.6 => not found
        libgcc_s.so.1 => /nix/store/bihw7p4zdqwyxmnc8h67c06lnjkvdan8-xgcc-13.3.0-libgcc/lib/libgcc_s.so.1 (0x00007fb92a518000)
        libm.so.6 => /nix/store/m71p7f0nymb19yn1dascklyya2i96jfw-glibc-2.39-52/lib/libm.so.6 (0x00007fb92a432000)
        libc.so.6 => /nix/store/m71p7f0nymb19yn1dascklyya2i96jfw-glibc-2.39-52/lib/libc.so.6 (0x00007fb92a23b000)
        /lib64/ld-linux-x86-64.so.2 => /nix/store/m71p7f0nymb19yn1dascklyya2i96jfw-glibc-2.39-52/lib64/ld-linux-x86-64.so.2 (0x00007fb92d3f9000)

While I'd much prefer to read the path from the environment, it would probably be safer to download the musl llm-ls on Linux.

adamcstephens avatar Aug 02 '24 13:08 adamcstephens