Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use already installed language server if in $PATH #4978

Closed
1 task done
diktomat opened this issue Mar 8, 2023 · 16 comments
Closed
1 task done

Use already installed language server if in $PATH #4978

diktomat opened this issue Mar 8, 2023 · 16 comments
Labels
enhancement [core label] language server An umbrella label for all language servers language An umbrella label for all programming languages syntax behaviors

Comments

@diktomat
Copy link

diktomat commented Mar 8, 2023

Check for existing issues

  • Completed

Describe the feature

Instead of downloading a language server within Zed, I propose it should first look in $PATH if it's already installed and use that one instead. Beyond saving some space, this would also enable using different ls versions, via rustup or direnv (#1025) for example.

@diktomat diktomat added enhancement [core label] triage labels Mar 8, 2023
@diktomat diktomat mentioned this issue Jan 24, 2024
1 task
@JosephTLyons JosephTLyons added language An umbrella label for all programming languages syntax behaviors and removed triage labels Mar 8, 2023
@JosephTLyons JosephTLyons transferred this issue from zed-industries/community Jan 24, 2024
@mrnugget mrnugget added the language server An umbrella label for all language servers label Feb 13, 2024
@mrnugget
Copy link
Member

Beyond saving some space, this would also enable using different ls versions, via rustup or direnv (#1025) for example.

Since you mention direnv: I assume the idea is to check $PATH in the scope of the currently open project, right?

@szlend
Copy link

szlend commented Feb 13, 2024

Since you mention direnv: I assume the idea is to check $PATH in the scope of the currently open project, right?

This is actually something I struggled with a lot in most GUI editors. If you have project A already open, and you open project B, that window will inherit the environment (including PATH) from the first project (A). This is because the second window usually reuses the same process, or forks off it.

I described some of these pitfalls in #4977 (comment)

@mrnugget
Copy link
Member

Yes, it's not that straightforward. We already do this:

zed/crates/zed/src/main.rs

Lines 99 to 106 in 98fff01

let login_shell_env_loaded = if stdout_is_a_pty() {
Task::ready(())
} else {
app.background_executor().spawn(async {
load_login_shell_environment().await.log_err();
})
};

What this does:

  • If zed was spawned on the CLI: nothing. Zed inherits env from parent shell process.
  • Otherwise: spawn a login shell (in home dir, I assume) and take its env.

What I thought we could do: when we weren't started by a ClI and you open a project, we spawn the login shell in that project's directory and use that env.

But that is also a bit tricky since you can add projects to a window, you have to remember your "original" env and reset it when switching windows/projects, ... and it's not that obvious to users what the env is.

@szlend
Copy link

szlend commented Feb 13, 2024

I think for this issue specifically, the way the env is loaded is fine, the language servers jut need to take PATH into account.

But for #4977 (direnv) support I would be happy to help figure out a good solution. Maybe worth moving the discussion there?

What I thought we could do: when we weren't started by a ClI and you open a project, we spawn the login shell in that project's directory and use that env.

I think that would probably cover most of the cases yeah. On the surface it sounds like a good idea to me.

But that is also a bit tricky since you can add projects to a window, you have to remember your "original" env and reset it when switching windows/projects, ... and it's not that obvious to users what the env is.

There's probably no good solution for that, unless you treat each project in the window kind of like its own window/sandbox. So each project runs its own language servers etc.

Though that still leaves the integrated terminal which is completely separate. If you use direnv, you would probably want the terminal to inherit the login environment, and let it apply its own environment overrides depending on where you cd to. If you don't use direnv, then I don't know what the appropriate environment should be.

@mrnugget
Copy link
Member

@szlend agree with you! I think the proposed solution in this ticket -- using a language server if it's in $PATH -- is good and we should implement it, but when @as-cii and I talked about it, we said that it probably only makes sense on a language-by-language basis. For gopls and zls it makes sense, but for the other servers powered by node it becomes a bit more tricky.

@szlend
Copy link

szlend commented Feb 13, 2024

For gopls and zls it makes sense, but for the other servers powered by node it becomes a bit more tricky.

Personally I would prefer if all language servers supported loading from PATH. Or at least have a configuration option to enable this if you think it's not a good default for zed. Though I imagine that's not something we can really enforce as language servers start moving outside the official repo.

@paperclover
Copy link

on top of supporting PATH, i think it would be nice if the exact path to the language server binary could be specified, and allow this configuration per project. this way, it can be made certain that the correct binary is loaded, especially in places where you want to work on the language server itself. trying to orchestrate PATH manually for these kinds of use cases is not fun.

@domenkozar
Copy link

Why this is important: Haskell needs LSP to be compiled against the same version of GHC that the tooling is built with.

@mrnugget
Copy link
Member

I wrote up some ideas and proposed solutions in #7902 — if you want, can you (whoever wants!) take a look and leave your thoughts and poke holes in it, if possible?

@domenkozar
Copy link

See also #4977

osiewicz pushed a commit that referenced this issue Aug 6, 2024
Related things:
#7902
#4978

Release Notes:
- Added: positibily to use locally installed vtsls
@JosephTLyons
Copy link
Collaborator

This is now included in today's v0.148.0-pre release, and will go out to stable next week.

@mrnugget
Copy link
Member

mrnugget commented Aug 8, 2024

Just to clarify: what landed in the release is support for running vtsls from $PATH. Not all language servers.

@mocenigo
Copy link

mocenigo commented Dec 7, 2024

Just to clarify: what landed in the release is support for running vtsls from $PATH. Not all language servers.

I sort of noticed. I cannot get digestif working :-)

@mocenigo
Copy link

mocenigo commented Dec 8, 2024

So, let us speak about digestif. It is a great language server and with SublimeText it is sufficient to add lines like the following in the setings file.

{
    "clients": {
        "digestif": {
            "enabled": true,
            "command": ["digestif"],
            "selector": "text.tex.latex"
        }
    }
}

I have tried everything that comes to mind, for instance

"lsp": {
    "digestif": {
      "binary": {
        "path_lookup": true,
        "path": "digestif",
        "arguments": []
      },
    },

(where digestif is in the $PATH, and I also gave the explicit path, but it does not work) and of course I also added it to the list of language servers for LaTeX. Nothing. Why is this not possible with Zed?

I only get warnings like the following in the log

2024-12-09T03:49:01.893886+05:30 [WARN] no language server found matching 'digestif'

Why is it not possible? What should I do? The documentation is not very helpful.

@mrnugget
Copy link
Member

mrnugget commented Dec 9, 2024

digestif is not supported by default and would need a Zed extension to work. Adding one that just runs the language server doesn't seem to be that hard: https://zed.dev/docs/extensions/developing-extensions

There's also zed-latex that supports texlab as the language server: rzukic/zed-latex#2

@notpeter
Copy link
Member

I'm going to go ahead and close this as resolved.

Beginning with extension API v0.1.0 (2024-08-14) the [Worktree.which(https://docs.rs/zed_extension_api/latest/zed_extension_api/struct.Worktree.html#method.which) function is available:

pub fn which(&self, binary_name: &str) -> Option<String>
// Returns the path to the given binary name, if one is present on the $PATH.

I believe all the built in languages (C/C++, Go, Python, Rust, VTSLS) and official Zed Extensions (Clojure, CSharp, Deno, Elixir, Erlang, GLSL, Haskell, Lua, PHP, Protobuf, Terraform, Zig, etc) which don't use NPM packages for LSP installation properly support which for hunting in user's paths for pre-existing LSP binaries. Similarly many (but not all) 3rd party extensions support it too. If you run into cases where language servers installed in your Path are not supported please open new issues on the repositories for those projects. It's usually a pretty easy fix.

Thanks all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement [core label] language server An umbrella label for all language servers language An umbrella label for all programming languages syntax behaviors
Projects
None yet
Development

No branches or pull requests

8 participants