Follow-up for #36093 and
https://github.com/zed-industries/zed/pull/36138
Since v1.355.0, `@github/copilot-language-server` has stopped responding
to `CheckStatus` requests if a `DidChangeConfiguration` notification
hasn’t been sent beforehand. This causes `CheckStatus` to remain in an
await state until it times out, leaving the connection stuck for a long
period before finally throwing a timeout error.
```rs
let status = server
.request::<request::CheckStatus>(request::CheckStatusParams {
local_checks_only: false,
})
.await
.into_response() // bails here with ConnectionResult::Timeout
.context("copilot: check status")?;
````
This PR fixes the issue by sending the `DidChangeConfiguration`
notification before making the `CheckStatus` request. It’s just an
ordering change i.e. no other LSP actions occur between these two calls.
Previously, we only updated our internal connection status and UI in
between.
Release Notes:
- Fixed an issue where GitHub Copilot could get stuck and fail to sign
in.
Closes#36093
Pin copilot version to 1.354 for now until further investigation.
Release Notes:
- Fixes issue where Copilot failed to sign in.
Co-authored-by: MrSubidubi <dev@bahn.sh>
The `async-watch` crate doesn't seem to be maintained and we noticed
several panics coming from it, such as:
```
[bug] failed to observe change after notificaton.
zed::reliability::init_panic_hook::{{closure}}::hea8cdcb6299fad6b+154543526
std::panicking::rust_panic_with_hook::h33b18b24045abff4+127578547
std::panicking::begin_panic_handler::{{closure}}::hf8313cc2fd0126bc+127577770
std::sys::backtrace::__rust_end_short_backtrace::h57fe07c8aea5c98a+127571385
__rustc[95feac21a9532783]::rust_begin_unwind+127576909
core::panicking::panic_fmt::hd54fb667be51beea+9433328
core::option::expect_failed::h8456634a3dada3e4+9433291
assistant_tools::edit_agent::EditAgent::apply_edit_chunks::{{closure}}::habe2e1a32b267fd4+26921553
gpui::app::async_context::AsyncApp::spawn::{{closure}}::h12f5f25757f572ea+25923441
async_task::raw::RawTask<F,T,S,M>::run::h3cca0d402690ccba+25186815
<gpui::platform::linux::x11::client::X11Client as gpui::platform::linux::platform::LinuxClient>::run::h26264aefbcfbc14b+73961666
gpui::platform::linux::platform::<impl gpui::platform::Platform for P>::run::hb12dcd4abad715b5+73562509
gpui::app::Application::run::h0f936a5f855a3f9f+150676820
zed::main::ha17f9a25fe257d35+154788471
std::sys::backtrace::__rust_begin_short_backtrace::h1edd02429370b2bd+154624579
std::rt::lang_start::{{closure}}::h3d2e300f10059b0a+154264777
std::rt::lang_start_internal::h418648f91f5be3a1+127502049
main+154806636
__libc_start_main+46051972301573
_start+12358494
```
I didn't find an executor-agnostic watch crate that was well maintained
(we already tried postage and async-watch), so decided to implement it
our own version.
Release Notes:
- Fixed a panic that could sometimes occur when the agent performed
edits.
* `state.last_options` was never being updated, so the caching wasn't
working.
* If node on the PATH was too old it was logging errors on every
invocation even though managed node is being used.
Release Notes:
- Fixed caching of Node.js runtime paths and improved error messages.
https://github.com/zed-industries/zed/issues/30972 brought up another
case where our context is not enough to track the actual source of the
issue: we get a general top-level error without inner error.
The reason for this was `.ok_or_else(|| anyhow!("failed to read HEAD
SHA"))?; ` on the top level.
The PR finally reworks the way we use anyhow to reduce such issues (or
at least make it simpler to bubble them up later in a fix).
On top of that, uses a few more anyhow methods for better readability.
* `.ok_or_else(|| anyhow!("..."))`, `map_err` and other similar error
conversion/option reporting cases are replaced with `context` and
`with_context` calls
* in addition to that, various `anyhow!("failed to do ...")` are
stripped with `.context("Doing ...")` messages instead to remove the
parasitic `failed to` text
* `anyhow::ensure!` is used instead of `if ... { return Err(...); }`
calls
* `anyhow::bail!` is used instead of `return Err(anyhow!(...));`
Release Notes:
- N/A
Release Notes:
- Fixed a race condition that sometimes prevented a system-installed
`node` binary from being detected.
- Fixed a bug where the `node.path` setting was not respected when
invoking npm.
Closes#27641
This PR fixes invalid proxy URIs being registered despite the URI not
being a valid proxy URI.
Whilst investigating #27641 , I noticed that currently any proxy URI
passed to `RequestClient::proxy_and_user_agent` will be assigned to the
created client, even if the URI is not a valid proxy URI. Given a test
as an example:
We create an URI here and pass it as a proxy to
`ReqwestClient::proxy_and_user_agent`:
https://github.com/zed-industries/zed/blob/main/crates/reqwest_client/src/reqwest_client.rs#L272-L273
In `ReqwestClient::proxy_and_user_agent`we take the proxy parameter here
9b40770e9f/crates/reqwest_client/src/reqwest_client.rs (L46)
and set it unconditionally here:
9b40770e9f/crates/reqwest_client/src/reqwest_client.rs (L62)
, not considering at all whether the proxy was successfully created
above. Concluding, we currently do not actually check whether a proxy
was successfully created, but rather whether an URI is equal to itself,
which trivially holds. The existing test for a malformed proxy URI
9b40770e9f/crates/reqwest_client/src/reqwest_client.rs (L293-L297)
does not check whether invalid proxies cause an error, but rather checks
whether `http::Uri::from_static` panics on an invalid URI, [which it
does as
documented](https://docs.rs/http/latest/http/uri/struct.Uri.html#panics).
Thus, the tests currently do not really check anything proxy-related and
invalid proxies are assigned as valid proxies.
---
This PR fixes the behaviour by considering whether the proxy was
actually properly parsed and only assigning it if that is the case.
Furthermore, it improves logging in case of errors so issues like the
linked one are easier to debug (for the linked issue, the log will now
include that the proxy schema is not supported in the logs).
Lastly, it also updates the test for a malformed proxy URI. The test now
actually checks that malformed proxy URIs are not registered for the
client rather than testing the `http` crate.
The update also initially caused the [test for a `socks4a`
proxy](9b40770e9f/crates/reqwest_client/src/reqwest_client.rs (L280C1-L282C50))
to fail. This happened because the reqwest-library introduced supports
for `socks4a` proxies in [version
0.12.13](https://github.com/seanmonstar/reqwest/blob/master/CHANGELOG.md#v01213).
Thus, this PR includes a bump of the reqwest library to add proper
support for socks4a proxies.
Release Notes:
- Added support for socks4a proxies.
---------
Co-authored-by: Peter Tripp <peter@zed.dev>
This PR introduces support for a `--user-data-dir` CLI flag to override
Zed's data directory and proposes renaming `support_dir` to `data_dir`
for better cross-platform clarity. It builds on the discussion in #25349
about custom data directories, aiming to provide a flexible
cross-platform solution.
### Changes
The PR is split into two commits:
1. **[feat(cli): add --user-data-dir to override data
directory](https://github.com/zed-industries/zed/pull/26886/commits/28e8889105847401e783d1739722d0998459fe5a)**
2. **[refactor(paths): rename support_dir to data_dir for cross-platform
clarity](https://github.com/zed-industries/zed/pull/26886/commits/affd2fc606b39af1b25432a688a9006229a8fc3a)**
### Context
Inspired by the need for custom data directories discussed in #25349,
this PR provides an immediate implementation in the first commit, while
the second commit suggests a naming improvement for broader appeal.
@mikayla-maki, I’d appreciate your feedback, especially on the rename
proposal, given your involvement in the original discussion!
### Testing
- `cargo build `
- `./target/debug/zed --user-data-dir ~/custom-data-dir`
Release Notes:
- Added --user-data-dir CLI flag
---------
Signed-off-by: Marko Kungla <marko.kungla@gmail.com>
Require a newer Node version to make Copilot work
Closes#27908
Release Notes:
- Breaking Change: If using system node Zed now requires Node >= v20.
Previously Node >= v18 was required. (Node v18 EOL date is 2025-04-30;
Node v19 EOL since 2023-06-01). Note: This does not change the Zed
bundled Node runtime version (still v23).
This adds a "workspace-hack" crate, see
[mozilla's](https://hg.mozilla.org/mozilla-central/file/3a265fdc9f33e5946f0ca0a04af73acd7e6d1a39/build/workspace-hack/Cargo.toml#l7)
for a concise explanation of why this is useful. For us in practice this
means that if I were to run all the tests (`cargo nextest r
--workspace`) and then `cargo r`, all the deps from the previous cargo
command will be reused. Before this PR it would rebuild many deps due to
resolving different sets of features for them. For me this frequently
caused long rebuilds when things "should" already be cached.
To avoid manually maintaining our workspace-hack crate, we will use
[cargo hakari](https://docs.rs/cargo-hakari) to update the build files
when there's a necessary change. I've added a step to CI that checks
whether the workspace-hack crate is up to date, and instructs you to
re-run `script/update-workspace-hack` when it fails.
Finally, to make sure that people can still depend on crates in our
workspace without pulling in all the workspace deps, we use a `[patch]`
section following [hakari's
instructions](https://docs.rs/cargo-hakari/0.9.36/cargo_hakari/patch_directive/index.html)
One possible followup task would be making guppy use our
`rust-toolchain.toml` instead of having to duplicate that list in its
config, I opened an issue for that upstream: guppy-rs/guppy#481.
TODO:
- [x] Fix the extension test failure
- [x] Ensure the dev dependencies aren't being unified by Hakari into
the main dependencies
- [x] Ensure that the remote-server binary continues to not depend on
LibSSL
Release Notes:
- N/A
---------
Co-authored-by: Mikayla <mikayla@zed.dev>
Co-authored-by: Mikayla Maki <mikayla.c.maki@gmail.com>
This partially reverts https://github.com/zed-industries/zed/pull/3324
We will still blank out user/global config when running managed NPM, to
keep to the spirit of #3324 (which was made at the time we did not allow
user-provided NPM builds - the intent of the change was to make the
behavior of NPM as consistent as possible).
I tested this change by:
1. Setting up a custom NPM registry via Versaccio
2. Adding this new registry to my .npmrc
3. Mirroring `vscode-langservers-extracted` to it
4. Blocking access to `registry.npmjs.org`
5. Opening up settings.json file in Zed Nightly
- Verifying that language server update fails for it
6. Opening up Zed Dev build of this branch
- Confirming that language server update check goes through for it
Closes#19806Closes#20749Closes#9422
Release Notes:
- User and global .npmrc configuration is now respected when running
user-provided NPM binary (which also happens automatically when `npm`
from PATH is newer than 18.0.0)
This PR improves the installation checks for `vtsls`.
Previously we were checking the installed version of TypeScript against
the latest available version to determine whether we needed to installed
the `vtsls` language server or not.
However, these are two independent concerns, so we should be checking
individually whether `typescript` or `@vtsls/language-server` need to be
installed/updated.
Closes https://github.com/zed-industries/zed/issues/18349.
Release Notes:
- typescript: Fixed an issue where `@vtsls/language-server` may not have
been updated to the latest version.
Release Notes:
- (Potentially breaking change) Zed will now use the node installed on
your $PATH (if it is more recent than v18) instead of downloading its
own. You can disable the new behavior with `{"node":
{"disable_path_lookup": true}}` in your settings. We do not yet use
system/project-local node_modules.
---------
Co-authored-by: Mikayla <mikayla@zed.dev>
Closes#15441 .
Fixed the issue where extensions couldn't start if the path contained
spaces. Additionally, this PR introduces the `node_environment_path`
function to obtain the PATH environment variable which includes the node
path.
Release Notes:
- N/A
Close#13786. To make `eslint` running on Windows, I made the following
changes:
1. Ensure that `zed` downloads the `.zip` file.
2. Handle the `$shared` symbolic link by copying files to the link
location.
3. In #13891, I mentioned that the `npm` `post-install` script was
always failing. After debugging, I found it was due to missing
environment variables. This has been fixed, and I will submit a new PR
to address the changes in #13891.
With this PR, `eslint` can now successfully run on Windows. Video:
https://github.com/user-attachments/assets/e85451b8-0388-490a-8a75-01c12d744f7c
Release Notes:
- Fixed `eslint` not running on Windows
([#13786](https://github.com/zed-industries/zed/issues/13786)).
---------
Co-authored-by: Marshall Bowers <elliott.codes@gmail.com>
This PR bumps the hard-coded Node.js version from v18.x (Hydrogen), which was LTS until October 2023, to v22.x (Jod) which will be the next LTS release in October 2024.
Release Notes:
- Updated Zed's node version (v18.x -> v22.x)
This PR replaces the `lazy_static!` usages in the `paths` crate with
`OnceLock` from the standard library.
This allows us to drop the `lazy_static` dependency from this crate.
The paths are now exposed as accessor functions that reference a private
static value.
Release Notes:
- N/A
This PR extracts the definition of the various Zed paths out of `util`
and into a new `paths` crate.
`util` is for generic utils, while these paths are Zed-specific. For
instance, `gpui` depends on `util`, and it shouldn't have knowledge of
these paths, since they are only used by Zed.
Release Notes:
- N/A
If you have already installed `node` using `brew install node`, you are
fine. If you did not install `node` on you local machine, it fails.
The `node_binary` path is actually not included in environment variable.
When run `npm install`, some extensions like `eslint`, may run some
commands like `sh -c node .....`. Since `node_binary` path is not
included in `PATH` variable, `sh -c node ...` will fail complaining that
"command not found". If you have installed `node` before, `node` is
already included in `PATH`, so you are fine. If not, it fails.
Closes#11890
Release Notes:
- Fixed Zed's internal Node runtime not being put in `$PATH` correctly
when running language servers and other commands with `node`.
([#11890](https://github.com/zed-industries/zed/issues/11890))
---------
Co-authored-by: Thorsten Ball <mrnugget@gmail.com>
Adding `proxy` keyword to configure proxy while using zed. After setting
the proxy, restart Zed to acctually use the proxy.
Example setting:
```rust
"proxy" = "socks5://localhost:10808"
"proxy" = "http://127.0.0.1:10809"
```
Closes#9424, closes#9422, closes#8650, closes#5032, closes#6701,
closes#11890
Release Notes:
- Added settings to configure proxy in Zed
---------
Co-authored-by: Jason Lee <huacnlee@gmail.com>
Our goal is to extract Svelte support into an extension, since we've
seen problems with the Tree-sitter Svelte parser crashing due to bugs in
the external scanner. In order to do this, we need a couple more
capabilities in LSP extensions:
* [x] `initialization_options` - programmatically controlling the JSON
initialization params sent to the language server
* [x] `prettier_plugins` - statically specifying a list of prettier
plugins that apply for a given language.
* [x] `npm_install_package`
Release Notes:
- N/A
---------
Co-authored-by: Marshall <marshall@zed.dev>
Co-authored-by: Marshall Bowers <elliott.codes@gmail.com>
Fixes: https://github.com/zed-industries/zed/issues/9234
This doesn't address `vue` as it has a slightly different install code,
but it should be fairly simple to add - I'll add it in in a follow-up.
This PR will allow all (except `vue`) node-based language servers to
update. It is mostly just throwing in a method into the `NodeRuntime`
trait that is used for checking if a package doesn't exist locally, or
is out of date, by checking the version against what's newest, and
installing. If any parsing of the `package.json` data fails along the
way, it assumes something has gone awry on the users system, logs the
error, and then proceeds with trying to install the package, so that
users don't get stuck on version if their package has some bad data.
Outside of adding this method, it just adds that check in all of the
language server's individual `fetch_server_binary` methods.
Release Notes:
- Added updating for node-based language servers
([#9234](https://github.com/zed-industries/zed/issues/9234)).
This PR moves the Clippy configuration up to the workspace level.
We're using the [`lints`
table](https://doc.rust-lang.org/cargo/reference/workspaces.html#the-lints-table)
to configure the Clippy ruleset in the workspace's `Cargo.toml`.
Each crate in the workspace now has the following in their own
`Cargo.toml` to inherit the lints from the workspace:
```toml
[lints]
workspace = true
```
This allows for configuring rust-analyzer to show Clippy lints in the
editor by using the following configuration in your Zed `settings.json`:
```json
{
"lsp": {
"rust-analyzer": {
"initialization_options": {
"check": {
"command": "clippy"
}
}
}
}
```
Release Notes:
- N/A
This PR sorts the dependency lists in our `Cargo.toml` files so that
they are in alphabetical order.
This should make them easier to visually scan when looking for a
dependency.
Apologies in advance for any merge conflicts 🙈
Release Notes:
- N/A
By default `npm install` will walk up the folder tree checking for a
folder that contains either a package.json file, or a node_modules
folder. If such a thing is found, then that is treated as the effective
"current directory" for the purpose of running npm commands.
This caused npm dependencies for language servers sometimes to be
installed in the wrong folder, described in #4628.
Adding `--prefix ./` to `npm install` forces
node_runtime.npm_install_packages to install packages in provided path
even if node_modules dir exists somewhere up the filesystem tree from
installation path.
- [x] Fill in GPL license text.
- [x] live_kit_client depends on live_kit_server as non-dev dependency,
even though it seems to only be used for tests. Is that an issue?
Release Notes:
- N/A
