mirror of
https://github.com/zed-industries/zed.git
synced 2025-02-03 08:54:04 +00:00
7302be8ebd
Some checks are pending
CI / Check Postgres and Protobuf migrations, mergability (push) Waiting to run
CI / Check formatting and spelling (push) Waiting to run
CI / (macOS) Run Clippy and tests (push) Waiting to run
CI / (Linux) Run Clippy and tests (push) Waiting to run
CI / (Linux) Build Remote Server (push) Waiting to run
CI / (Windows) Run Clippy and tests (push) Waiting to run
CI / Create a macOS bundle (push) Blocked by required conditions
CI / Linux x86_x64 release bundle (push) Blocked by required conditions
CI / Linux arm64 release bundle (push) Blocked by required conditions
CI / Auto release preview (push) Blocked by required conditions
During my work on PR #22616, while trying to fix the `test_reporting_fs_changes_to_language_servers` test case, I noticed that we are currently handling paths using `String` in some places. However, this approach causes issues on Windows. This draft PR modifies `rebuild_watched_paths_inner` and `glob_literal_prefix`. For example, take the `glob_literal_prefix` function modified in this PR: ```rust assert_eq!( glob_literal_prefix("node_modules/**/*.js"), "node_modules" ); // This works on Unix, fails on Windows assert_eq!( glob_literal_prefix("node_modules\\**\\*.js"), "node_modules" ); // This works on Windows assert_eq!( glob_literal_prefix("node_modules\\**/*.js"), "node_modules" ); // This fails on Windows ``` The current implementation treats path as `String` and relies on `\` as the path separator on Windows, but on Windows, both `/` and `\` can be used as separators. This means that `node_modules\**/*.js` is also a valid path representation. There are two potential solutions to this issue: 1. **Continue handling paths with `String`**, and on Windows, replace all `/` with `\`. 2. **Use `Path` for path handling**, which is the solution implemented in this PR. ### Advantages of Solution 1: - Simple and direct. ### Advantages of Solution 2: - More robust, especially in handling `strip_prefix`. Currently, the logic for removing a path prefix looks like this: ```rust let path = "/some/path/to/file.rs"; let parent = "/some/path/to"; // remove prefix let file = path.strip_prefix(parent).unwrap(); // which is `/file.rs` let file = file.strip_prefix("/").unwrap(); ``` However, using `Path` simplifies this process and makes it more robust: ```rust let path = Path::new("C:/path/to/src/main.rs"); let parent = Path::new("C:/path/to/src"); let file = path.strip_prefix(&parent).unwrap(); // which is `main.rs` let path = Path::new("C:\\path\\to/src/main.rs"); let parent = Path::new("C:/path/to\\src\\"); let file = path.strip_prefix(&parent).unwrap(); // which is `main.rs` ``` Release Notes: - N/A --------- Co-authored-by: Kirill Bulatov <mail4score@gmail.com> |
||
---|---|---|
.. | ||
src | ||
Cargo.toml | ||
LICENSE-GPL |