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

Detect and possibly use user-installed gopls / zls language servers #8188

Merged
merged 14 commits into from
Feb 23, 2024

Conversation

mrnugget
Copy link
Member

@mrnugget mrnugget commented Feb 22, 2024

After a lot of back-and-forth, this is a small attempt to implement solutions (1) and (3) in #7902. The goal is to have a minimal change that helps users get started with Zed, until we have extensions ready.

Release Notes:

  • Added detection of user-installed gopls to Go language server adapter. If a user has gopls in $PATH when opening a worktree, it will be used.
  • Added detection of user-installed zls to Zig language server adapter. If a user has zls in $PATH when opening a worktree, it will be used.

Example:

I don't have go installed globally, but I do have gopls:

~ $ which go
go not found
~ $ which gopls
/Users/thorstenball/code/go/bin/gopls

But I do have go in a project's directory:

~/tmp/go-testing φ which go
/Users/thorstenball/.local/share/mise/installs/go/1.21.5/go/bin/go
~/tmp/go-testing φ which gopls
/Users/thorstenball/code/go/bin/gopls

With current Zed when I run zed ~/tmp/go-testing, I'd get the dreaded error:

screenshot-2024-02-23-11 14 08@2x

But with the changes in this PR, it works:

[2024-02-23T11:14:42+01:00 INFO  language::language_registry] starting language server "gopls", path: "/Users/thorstenball/tmp/go-testing", id: 1
[2024-02-23T11:14:42+01:00 INFO  language::language_registry] found user-installed language server for Go. path: "/Users/thorstenball/code/go/bin/gopls", arguments: ["-mode=stdio"]
[2024-02-23T11:14:42+01:00 INFO  lsp] starting language server. binary path: "/Users/thorstenball/code/go/bin/gopls", working directory: "/Users/thorstenball/tmp/go-testing", args: ["-mode=stdio"]

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Feb 22, 2024
@ConradIrwin
Copy link
Member

@mrnugget As gopls is particularly finicky; should we bite the bullet and use gopls -v version to verify that it was compiled with the correct version of go before chosing to use it?

@mrnugget
Copy link
Member Author

@mrnugget As gopls is particularly finicky; should we bite the bullet and use gopls -v version to verify that it was compiled with the correct version of go before chosing to use it?

Yeah, but I'd prefer to do that as a follow-up. Right now I want to make the interface changes as small as possible.

@mrnugget mrnugget changed the title WIP: Detect and possibly use user-installed gopls language server Detect and possibly use user-installed gopls language server Feb 23, 2024
@mrnugget mrnugget marked this pull request as ready for review February 23, 2024 10:42
@mrnugget mrnugget changed the title Detect and possibly use user-installed gopls language server Detect and possibly use user-installed gopls / zls language servers Feb 23, 2024
@mrnugget mrnugget merged commit 42ac988 into main Feb 23, 2024
10 checks passed
@mrnugget mrnugget deleted the path-handling branch February 23, 2024 12:39
mrnugget added a commit that referenced this pull request Feb 23, 2024
…rs (#8188)

After a lot of back-and-forth, this is a small attempt to implement
solutions (1) and (3) in
#7902. The goal is to have a
minimal change that helps users get started with Zed, until we have
extensions ready.

Release Notes:

- Added detection of user-installed `gopls` to Go language server
adapter. If a user has `gopls` in `$PATH` when opening a worktree, it
will be used.
- Added detection of user-installed `zls` to Zig language server
adapter. If a user has `zls` in `$PATH` when opening a worktree, it will
be used.

Example:

I don't have `go` installed globally, but I do have `gopls`:

```
~ $ which go
go not found
~ $ which gopls
/Users/thorstenball/code/go/bin/gopls
```

But I do have `go` in a project's directory:

```
~/tmp/go-testing φ which go
/Users/thorstenball/.local/share/mise/installs/go/1.21.5/go/bin/go
~/tmp/go-testing φ which gopls
/Users/thorstenball/code/go/bin/gopls
```

With current Zed when I run `zed ~/tmp/go-testing`, I'd get the dreaded
error:

![screenshot-2024-02-23-11 14
08@2x](https://github.com/zed-industries/zed/assets/1185253/822ea59b-c63e-4102-a50e-75501cc4e0e3)

But with the changes in this PR, it works:

```
[2024-02-23T11:14:42+01:00 INFO  language::language_registry] starting language server "gopls", path: "/Users/thorstenball/tmp/go-testing", id: 1
[2024-02-23T11:14:42+01:00 INFO  language::language_registry] found user-installed language server for Go. path: "/Users/thorstenball/code/go/bin/gopls", arguments: ["-mode=stdio"]
[2024-02-23T11:14:42+01:00 INFO  lsp] starting language server. binary path: "/Users/thorstenball/code/go/bin/gopls", working directory: "/Users/thorstenball/tmp/go-testing", args: ["-mode=stdio"]
```

---------

Co-authored-by: Antonio <[email protected]>
mrnugget added a commit that referenced this pull request Feb 23, 2024
This is related to #8188 and should fix two things:

1. It spawns a login shell that `cd`s into a users home directory, which
   causes tools like `asdf`, `mise`, `direnv`, ... to fire. That means
   that if I don't have `gopls` installed, for example, but `go` (via
   `mise`) that Zed will then use my `go` when launched from `Zed.app`.
2. It appends `exit 0;` to the login shell command. It's the same
   mysterious fix that we had in #8188 for the login shell. We thought
   that this shell here was immune but turns out it's not when launched
   via `cli` (`cargo build && cargo run -p cli --bin cli -- -b
   target/debug/Zed`).
@mrnugget mrnugget mentioned this pull request Feb 27, 2024
1 task
mrnugget pushed a commit that referenced this pull request Dec 9, 2024
Closes #8071

Release Notes:

- Changed the Go integration to check whether an existing `gopls` was compiled for the current `go` version.

Previously we cached gopls (the go language server) as a file called
`gopls_{GOPLS_VERSION}`. The go version that gopls was built with is
crucial, so we need to cache the go version as well.

It's actually super interesting and very clever; gopls uses go to parse
the AST and do all the analyzation etc. Go exposes its internals in its
standard lib (`go/parser`, `go/types`, ...), which gopls uses to analyze
the user code. So if there is a new go release that contains new
syntax/features/etc. (the libraries `go/parser`, `go/types`, ...
change), we can rebuild the same version of `gopls` with the new version
of go (with the updated `go/xxx` libraries) to support the new language
features.

We had some issues around that (e.g., range over integers introduced in
go1.22, or custom iterators in go1.23) where we never updated gopls,
because we were on the latest gopls version, but built with an old go
version.

After this PR gopls will be cached under the name
`gopls_{GOPLS_VERSION}_go_{GO_VERSION}`.

Most users do not see this issue anymore, because after
#8188 we first check if we can
find gopls in the PATH before downloading and caching gopls, but the
issue still exists.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants