From 731096c09a28beb2927be50482ffca77427ee3b0 Mon Sep 17 00:00:00 2001 From: Xin Zhao Date: Wed, 19 Aug 2026 12:16:37 +0000 Subject: [PATCH] Fix project path handling when connecting from Unix to Windows remotes (#62038) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Objective Follow-up of #61374. Zed now supports Windows as a remote target, but when connecting from a Unix platform to Windows, some path handling still uses the native client's path style (Unix) to construct paths, which causes weird path displays in different areas. One of them is the project path stored in the `settings.json` file, which is related to the open path picker in the codebase: https://github.com/zed-industries/zed/blob/0500fd9ee99a4a98b36e339f3674e391697690fd/crates/open_path_prompt/src/open_path_prompt.rs#L668-L679 For example, if I have a remote project at `D:\code\test_python` and want to open it in remote development, I usually use path completions, with `D:\code\` as the parent path and `test_python` as the selected candidate. Zed directly joins them using `Path::join` on the Unix platform, which results in `D:\code\/test_python`. A second thing I found is the displayed name for the git repo. The related source code is: https://github.com/zed-industries/zed/blob/0500fd9ee99a4a98b36e339f3674e391697690fd/crates/title_bar/src/title_bar.rs#L262-L268 Also taking `D:\code\test_python` as an example: the passed-in `common_dir_abs_path` is `D:\code\test_python\.git`, and `repo_identity_path()` directly uses `Path::file_name()` and `Path::parent()` from the standard library to handle this: https://github.com/zed-industries/zed/blob/0500fd9ee99a4a98b36e339f3674e391697690fd/crates/project/src/git_store.rs#L9956-L9965 Ideally, this function should return `D:\code\test_python`. But due to the platform mismatch, `D:\code\test_python\.git` is returned; after further processing in the title bar, we get `D:\code\test_python\` as the displayed name, while the expected display name is `test_python`. In the past, only Unix-like systems could serve as remote servers, and their path separator (`/`) is valid on Windows, so everything looked fine. But Unix does not support `\` as a valid separator — that's the root cause. We need to use `PathStyle`, which is designed for processing paths across platforms, to deal with these cases. ## Solution - Added new APIs `PathStyle::parent()` and `PathStyle::file_name()`, which serve as replacements for `Path::parent()` and `Path::file_name()` to process paths cross-platform. - Adopted the new APIs in `repo_identity_path()`, and updated the relevant call sites. - For the open path picker, use `PathStyle::join_path()` instead of `Path::join`. ## Testing The added `PathStyle::parent()` and `PathStyle::file_name()` are covered by detailed unit tests. These tests verify that the behavior matches the corresponding methods in `Path`, just independent of the host platform. For the path display issues, I built and tested manually; a comparison is attached in the Showcase section. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [ ] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable ## Showcase
Click to view showcase | Content | Before | After | |:--:|:--:|:--:| |title bar|title_bar_before | title_bar_after | |path storage| project_path_before| project_path_after| |open remote| open_remote_before|open_remote_after |
## Other things to note This PR also did a little refactoring: it moved the `PathStyle`-related tests from the `util` crate to the `path` crate, and updated the documentation to reflect that Windows can serve as a remote platform. The recent project picker also suffers from the same cross-platform bug, but it is not fixed here, because a clean fix requires dealing with database storage, unlike the direct API changes made here. I will address it in a follow-up PR. This PR looks very large, but most of the changes are the test migration and the new API implementation. I hope the unit tests and comments can offload some of the burden for reviewers. --- Release Notes: - Fixed project paths being built incorrectly when connecting from Unix machines to Windows remote servers. --- Cargo.lock | 2 +- .../open_path_prompt/src/open_path_prompt.rs | 28 +- crates/path/Cargo.toml | 1 + crates/path/src/path.rs | 536 ++++++++++++++++++ crates/project/src/git_store.rs | 12 +- crates/project/src/worktree_store.rs | 2 +- crates/project/tests/integration/git_store.rs | 20 +- crates/title_bar/src/title_bar.rs | 8 +- crates/util/Cargo.toml | 1 - crates/util/src/paths.rs | 161 ------ crates/workspace/src/workspace.rs | 2 +- docs/src/remote-development.md | 2 +- 12 files changed, 593 insertions(+), 182 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1a5464c797..97f935001c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12878,6 +12878,7 @@ version = "0.1.0" dependencies = [ "anyhow", "dunce", + "pretty_assertions", "serde", "tempfile", ] @@ -20192,7 +20193,6 @@ dependencies = [ "nix 0.29.0", "path", "percent-encoding", - "pretty_assertions", "rand 0.9.4", "regex", "rust-embed", diff --git a/crates/open_path_prompt/src/open_path_prompt.rs b/crates/open_path_prompt/src/open_path_prompt.rs index 239edb676a..ae043324bd 100644 --- a/crates/open_path_prompt/src/open_path_prompt.rs +++ b/crates/open_path_prompt/src/open_path_prompt.rs @@ -22,7 +22,7 @@ use std::{ use ui::{Context, LabelLike, ListItem, Window}; use ui::{HighlightedLabel, ListItemSpacing, prelude::*}; use util::{ - maybe, + ResultExt, maybe, paths::{PathStyle, compare_paths}, }; use workspace::Workspace; @@ -671,8 +671,17 @@ impl PickerDelegate for OpenPathDelegate { if parent_path == &self.prompt_root && candidate.path.string.is_empty() { PathBuf::from(&self.prompt_root) } else { - Path::new(self.lister.resolve_tilde(parent_path, cx).as_ref()) - .join(&candidate.path.string) + let Some(confirmed_path) = self + .path_style + .join_path_preserving_components( + Path::new(self.lister.resolve_tilde(parent_path, cx).as_ref()), + &candidate.path.string, + ) + .log_err() + else { + return; + }; + confirmed_path }; if let Some(tx) = self.tx.take() { tx.send(Some(vec![confirmed_path])).ok(); @@ -692,8 +701,17 @@ impl PickerDelegate for OpenPathDelegate { if parent_path == &self.prompt_root && user_input.file.string.is_empty() { PathBuf::from(&self.prompt_root) } else { - Path::new(self.lister.resolve_tilde(parent_path, cx).as_ref()) - .join(&user_input.file.string) + let Some(prompted_path) = self + .path_style + .join_path_preserving_components( + Path::new(self.lister.resolve_tilde(parent_path, cx).as_ref()), + &user_input.file.string, + ) + .log_err() + else { + return; + }; + prompted_path }; if user_input.exists { self.should_dismiss = false; diff --git a/crates/path/Cargo.toml b/crates/path/Cargo.toml index e3a1c33df0..c50e6ef97c 100644 --- a/crates/path/Cargo.toml +++ b/crates/path/Cargo.toml @@ -18,6 +18,7 @@ serde = { workspace = true, optional = true } [dev-dependencies] tempfile.workspace = true +pretty_assertions.workspace = true [lints] workspace = true diff --git a/crates/path/src/path.rs b/crates/path/src/path.rs index 0b942c2e10..83f386c756 100644 --- a/crates/path/src/path.rs +++ b/crates/path/src/path.rs @@ -125,6 +125,25 @@ impl PathStyle { Ok(PathBuf::from(self.normalize(&joined))) } + pub fn join_path_preserving_components( + self, + left: impl AsRef, + right: impl AsRef, + ) -> anyhow::Result { + let left = left + .as_ref() + .to_str() + .ok_or_else(|| anyhow::anyhow!("Path contains invalid UTF-8"))?; + let right = right.as_ref(); + let right_string = right + .to_str() + .ok_or_else(|| anyhow::anyhow!("Path contains invalid UTF-8"))?; + let joined = self + .join(left, right_string) + .ok_or_else(|| anyhow::anyhow!("Path must be relative: {right:?}"))?; + Ok(PathBuf::from(&joined)) + } + pub fn normalize(self, path_like: &str) -> String { match self { PathStyle::Windows => { @@ -238,6 +257,177 @@ impl PathStyle { ) } + pub fn file_name(self, path: &Path) -> Option<&str> { + if self == PathStyle::local() { + return path.file_name().and_then(|n| n.to_str()); + } + let path_string = path.to_str()?; + let parent_length = self.parent(path)?.to_str()?.len(); + let remainder = path_string.get(parent_length..)?; + let is_verbatim = self.is_windows() && path_string.as_bytes().starts_with(br"\\?\"); + let is_body_separator = |character: char| { + if is_verbatim { + character == '\\' + } else { + self.separators_ch().contains(&character) + } + }; + + let component = remainder + .rsplit(is_body_separator) + .find(|component| !component.is_empty() && (is_verbatim || *component != "."))?; + if matches!(component, "." | "..") { + None + } else { + Some(component) + } + } + + pub fn parent(self, path: &Path) -> Option<&Path> { + if self == PathStyle::local() { + return path.parent(); + } + let path = path.to_str()?; + let path_bytes = path.as_bytes(); + let is_windows = self.is_windows(); + + const DRIVE_PREFIX_LENGTH: usize = 2; + const UNC_PREFIX_LENGTH: usize = 2; + const VERBATIM_PREFIX: &[u8] = br"\\?\"; + const VERBATIM_UNC_PREFIX: &[u8] = br"\\?\UNC\"; + const DEVICE_PREFIX: &[u8] = br"\\.\"; + + let is_separator = |byte: u8| byte == b'/' || is_windows && byte == b'\\'; + let is_verbatim = is_windows && path_bytes.starts_with(VERBATIM_PREFIX); + let has_unc_prefix = path_bytes + .get(..UNC_PREFIX_LENGTH) + .is_some_and(|prefix| prefix.iter().all(|byte| is_separator(*byte))); + // Verbatim paths treat '/' as a literal character, so only '\' separates body components. + let is_body_separator = |byte: u8| { + if is_verbatim { + byte == b'\\' + } else { + is_separator(byte) + } + }; + + let component_end = |component_start: usize| { + path_bytes + .get(component_start..) + .and_then(|remainder| remainder.iter().position(|byte| is_body_separator(*byte))) + .map_or(path_bytes.len(), |position| component_start + position) + }; + + // Windows prefixes, in precedence order: \\?\UNC\, \\?\ (verbatim), \\.\ (device), + // \\ (UNC), drive letter. The device prefix must be matched before the generic UNC + // check because it also starts with two separators. + let prefix_end = { + if !is_windows { + 0 + } else if path_bytes.starts_with(VERBATIM_UNC_PREFIX) { + let server_end = component_end(VERBATIM_UNC_PREFIX.len()); + let share_start = server_end.saturating_add(1).min(path_bytes.len()); + if share_start < path_bytes.len() { + component_end(share_start) + } else { + server_end + } + } else if is_verbatim { + let drive_start = VERBATIM_PREFIX.len(); + let drive_end = drive_start + DRIVE_PREFIX_LENGTH; + let has_drive_prefix = path_bytes + .get(drive_start) + .is_some_and(u8::is_ascii_alphabetic) + && path_bytes.get(drive_start + 1) == Some(&b':'); + + // A drive letter only counts as a prefix when followed by a separator or end + // of path; otherwise (e.g. `\\?\C:foo`) the whole first component is the prefix. + if has_drive_prefix + && path_bytes + .get(drive_end) + .is_none_or(|byte| is_separator(*byte)) + { + drive_end + } else { + component_end(drive_start) + } + } else if path_bytes.starts_with(DEVICE_PREFIX) { + component_end(DEVICE_PREFIX.len()) + } else if has_unc_prefix { + let server_end = component_end(UNC_PREFIX_LENGTH); + let share_start = server_end.saturating_add(1).min(path_bytes.len()); + let share_end = component_end(share_start); + // A UNC prefix requires a share name; `\\server` without one is treated as a + // relative path, so its prefix is empty. + if server_end > UNC_PREFIX_LENGTH && share_end > share_start { + share_end + } else { + 0 + } + } else if path_bytes.first().is_some_and(u8::is_ascii_alphabetic) + && path_bytes.get(1) == Some(&b':') + { + DRIVE_PREFIX_LENGTH + } else { + 0 + } + }; + + // Uses is_separator rather than is_body_separator: after a verbatim drive prefix, + // '/' still counts as the root separator (e.g. `\\?\C:/foo`) even though '/' does + // not separate body components. + let has_root = path_bytes + .get(prefix_end) + .is_some_and(|byte| is_separator(*byte)); + // The leading `.`/`./` is absorbed into the body (e.g. `./a` → parent `.`). + let starts_with_current_directory = prefix_end == 0 + && !has_root + && path_bytes.first() == Some(&b'.') + && (path_bytes.len() == 1 || path_bytes.get(1).is_some_and(|byte| is_separator(*byte))); + let body_start = + prefix_end + usize::from(has_root) + usize::from(starts_with_current_directory); + + // Loop because trimming a trailing '.' (e.g. `foo/.`) can expose a separator to trim, + // and vice versa (`foo/./`). Verbatim paths never treat '.' as a current-directory + // component, hence the !is_verbatim check below. + let trim_trailing = |mut end: usize| { + loop { + while end > body_start && is_body_separator(path_bytes[end - 1]) { + end -= 1; + } + + let is_trailing_current_directory = !is_verbatim + && end > body_start + && path_bytes[end - 1] == b'.' + && (end - 1 == body_start + || end + .checked_sub(2) + .and_then(|index| path_bytes.get(index)) + .is_some_and(|byte| is_body_separator(*byte))); + if is_trailing_current_directory { + end -= 1; + } else { + return end; + } + } + }; + + let body_end = trim_trailing(path_bytes.len()); + // An empty body means the path is root- or prefix-only and has no parent; only a + // bare current-directory component (`.`/`./`) has `""` as its parent. + if body_end == body_start { + return starts_with_current_directory.then_some(Path::new("")); + } + + let parent_end = path_bytes + .get(body_start..body_end)? + .iter() + .rposition(|byte| is_body_separator(*byte)) + .map_or(body_start, |position| trim_trailing(body_start + position)); + + Some(Path::new(path.get(..parent_end)?)) + } + pub fn strip_prefix<'a>( &self, child: &'a Path, @@ -320,3 +510,349 @@ pub fn normalize_path(path: &Path) -> PathBuf { } ret } + +#[cfg(test)] +mod tests { + use super::*; + use crate::rel_path::rel_path; + + #[test] + fn test_join_path_uses_path_style_separator() { + let posix_path = PathStyle::Unix + .join_path(Path::new("/home/user/dev"), "worktrees") + .unwrap(); + let windows_path = PathStyle::Windows + .join_path(Path::new("C:\\Users\\user\\dev"), "worktrees") + .unwrap(); + + assert_eq!(posix_path, PathBuf::from("/home/user/dev/worktrees")); + assert_eq!( + windows_path.to_string_lossy(), + "C:\\Users\\user\\dev\\worktrees" + ); + } + + #[test] + fn test_join_path_preserving_components() { + let posix_path = PathStyle::Unix + .join_path_preserving_components(Path::new("/home/user/symlink"), "../worktrees") + .unwrap(); + let windows_path = PathStyle::Windows + .join_path_preserving_components(Path::new(r"C:\Users\user\symlink"), r"..\worktrees") + .unwrap(); + + assert_eq!(posix_path, PathBuf::from("/home/user/symlink/../worktrees")); + assert_eq!( + windows_path.to_string_lossy(), + r"C:\Users\user\symlink\..\worktrees" + ); + } + + #[test] + fn test_normalize_uses_path_style_separator() { + assert_eq!( + PathStyle::Unix.normalize("/home/user/dev/../worktrees/./zed"), + "/home/user/worktrees/zed" + ); + assert_eq!( + PathStyle::Windows.normalize("C:\\Users\\user\\dev\\worktrees"), + "C:\\Users\\user\\dev\\worktrees" + ); + } + + #[test] + fn test_normalize_windows_path_regardless_of_host_platform() { + assert_eq!( + PathStyle::Windows.normalize(r"C:\Users\user\dev\..\worktrees"), + r"C:\Users\user\worktrees" + ); + assert_eq!( + PathStyle::Windows.normalize(r"C:\Users\.\worktrees"), + r"C:\Users\worktrees" + ); + assert_eq!( + PathStyle::Windows.normalize(r"C:\Users\user\dev\sub\..\..\worktrees"), + r"C:\Users\user\worktrees" + ); + assert_eq!( + PathStyle::Windows.normalize("C:/Users/user/dev/../worktrees"), + r"C:\Users\user\worktrees" + ); + assert_eq!( + PathStyle::Windows.normalize(r"C:/Users\user/dev\..\worktrees"), + r"C:\Users\user\worktrees" + ); + assert_eq!( + PathStyle::Windows.normalize(r"C:\Users/user\.\worktrees"), + r"C:\Users\user\worktrees" + ); + assert_eq!( + PathStyle::Windows.normalize(r"\\server\share\dev\..\worktrees"), + r"\\server\share\worktrees" + ); + assert_eq!( + PathStyle::Windows.normalize(r"//server\share/dev\..\worktrees"), + r"\\server\share\worktrees" + ); + assert_eq!( + PathStyle::Windows.normalize(r"\dev\..\worktrees"), + r"\worktrees" + ); + assert_eq!( + PathStyle::Windows.normalize(r"dev\..\worktrees"), + r"worktrees" + ); + assert_eq!( + PathStyle::Windows.normalize(r"C:\..\worktrees"), + r"C:\worktrees" + ); + } + + #[test] + fn test_strip_prefix() { + let expected = [ + ( + PathStyle::Unix, + "/a/b/c", + "/a/b", + Some(rel_path("c").into_arc()), + ), + ( + PathStyle::Unix, + "/a/b/c", + "/a/b/", + Some(rel_path("c").into_arc()), + ), + ( + PathStyle::Unix, + "/a/b/c", + "/", + Some(rel_path("a/b/c").into_arc()), + ), + (PathStyle::Unix, "/a/b/c", "", None), + (PathStyle::Unix, "/a/b//c", "/a/b/", None), + (PathStyle::Unix, "/a/bc", "/a/b", None), + ( + PathStyle::Unix, + "/a/b/c", + "/a/b/c", + Some(rel_path("").into_arc()), + ), + ( + PathStyle::Windows, + "C:\\a\\b\\c", + "C:\\a\\b", + Some(rel_path("c").into_arc()), + ), + ( + PathStyle::Windows, + "C:\\a\\b\\c", + "C:\\a\\b\\", + Some(rel_path("c").into_arc()), + ), + ( + PathStyle::Windows, + "C:\\a\\b\\c", + "C:\\", + Some(rel_path("a/b/c").into_arc()), + ), + (PathStyle::Windows, "C:\\a\\b\\c", "", None), + (PathStyle::Windows, "C:\\a\\b\\\\c", "C:\\a\\b\\", None), + (PathStyle::Windows, "C:\\a\\bc", "C:\\a\\b", None), + ( + PathStyle::Windows, + "C:\\a\\b/c", + "C:\\a\\b", + Some(rel_path("c").into_arc()), + ), + ( + PathStyle::Windows, + "C:\\a\\b/c", + "C:\\a\\b\\", + Some(rel_path("c").into_arc()), + ), + ( + PathStyle::Windows, + "C:\\a\\b/c", + "C:\\a\\b/", + Some(rel_path("c").into_arc()), + ), + ]; + let actual = expected.clone().map(|(style, child, parent, _)| { + ( + style, + child, + parent, + style + .strip_prefix(child.as_ref(), parent.as_ref()) + .map(|rel_path| rel_path.into_arc()), + ) + }); + pretty_assertions::assert_eq!(actual, expected); + } + + #[test] + fn test_unix_path_style_file_name() { + let unix_paths_and_filenames = [ + (Path::new(""), None), + (Path::new("/usr/bin/"), Some("bin")), + (Path::new("tmp/foo.txt"), Some("foo.txt")), + (Path::new("."), None), + (Path::new("./"), None), + (Path::new("foo.txt/."), Some("foo.txt")), + (Path::new("foo.txt/.//"), Some("foo.txt")), + (Path::new("foo/./bar"), Some("bar")), + (Path::new("foo.txt/.."), None), + (Path::new("/.."), None), + (Path::new("/"), None), + ]; + + for (path, filename) in unix_paths_and_filenames { + assert_eq!(PathStyle::Unix.file_name(path), filename); + } + } + + #[test] + fn test_windows_path_style_file_name() { + let windows_paths_and_filenames = [ + (Path::new(""), None), + (Path::new("."), None), + (Path::new(r"C:\usr\bin\"), Some("bin")), + (Path::new(r"C:"), None), + (Path::new(r"tmp\foo.txt"), Some("foo.txt")), + (Path::new("tmp/foo.txt"), Some("foo.txt")), + (Path::new(r"foo.txt\."), Some("foo.txt")), + (Path::new(r"foo.txt\.\"), Some("foo.txt")), + (Path::new(r"foo.txt\.."), None), + (Path::new(r"C:\"), None), + (Path::new(r"\\server\share"), None), + (Path::new(r"\\server\share\foo.txt"), Some("foo.txt")), + (Path::new("//server/share"), None), + (Path::new("//server/share/foo.txt"), Some("foo.txt")), + (Path::new(r"\\?\bar"), None), + (Path::new(r"\\?\bar\foo.txt"), Some("foo.txt")), + (Path::new(r"\\?\C:/foo/bar"), Some("foo/bar")), + (Path::new(r"\\.\device"), None), + (Path::new(r"\\.\device\foo.txt"), Some("foo.txt")), + ]; + + for (path, filename) in windows_paths_and_filenames { + assert_eq!(PathStyle::Windows.file_name(path), filename); + } + } + + #[test] + fn test_unix_path_style_parent() { + let unix_paths_and_parents = [ + ("", None), + ("/foo/bar", Some("/foo")), + ("/foo", Some("/")), + ("/", None), + ("///foo///", Some("/")), + ("///foo///bar", Some("///foo")), + ("foo/bar", Some("foo")), + ("foo", Some("")), + ("foo/.", Some("")), + ("foo/./bar", Some("foo")), + ("foo/../bar", Some("foo/..")), + ("./a", Some(".")), + ("./.", Some("")), + ("/..", Some("/")), + (".", Some("")), + ("..", Some("")), + ]; + + for (path, parent) in unix_paths_and_parents { + assert_eq!( + PathStyle::Unix.parent(Path::new(path)), + parent.map(Path::new) + ); + } + } + + #[test] + fn test_windows_path_style_parent() { + let windows_paths_and_parents = [ + ("", None), + (r"C:\foo\bar", Some(r"C:\foo")), + (r"C:\foo", Some(r"C:\")), + (r"C:\", None), + (r"C:foo", Some("C:")), + (r"C:", None), + (r"\foo", Some(r"\")), + (r"\", None), + (r"foo\bar", Some("foo")), + (r"foo\\bar", Some("foo")), + ("foo/bar", Some("foo")), + ("foo", Some("")), + (r"foo\.", Some("")), + (r"foo\.\bar", Some("foo")), + (r"foo\..\bar", Some(r"foo\..")), + (r".\a", Some(".")), + (r".\.", Some("")), + (r"\\server\share", None), + (r"\\server\share\foo.txt", Some(r"\\server\share\")), + ("//server/share", None), + ("//server/share/foo.txt", Some("//server/share/")), + (r"\\?\bar", None), + (r"\\?\bar\foo.txt", Some(r"\\?\bar\")), + ( + r"\\?\UNC\server\share\foo.txt", + Some(r"\\?\UNC\server\share\"), + ), + (r"\\?\C:\foo.txt", Some(r"\\?\C:\")), + (r"\\?\C:/foo/bar", Some(r"\\?\C:/")), + (r"\\.\device", None), + (r"\\.\device\foo.txt", Some(r"\\.\device\")), + (".", Some("")), + ("..", Some("")), + ]; + + for (path, parent) in windows_paths_and_parents { + assert_eq!( + PathStyle::Windows.parent(Path::new(path)), + parent.map(Path::new) + ); + } + } + + #[cfg(unix)] + #[test] + fn test_local_path_style_non_utf8() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + let path = Path::new(OsStr::from_bytes(b"/home/user/\xff\xfe/repo/file.txt")); + assert_eq!(PathStyle::Unix.file_name(path), Some("file.txt")); + assert_eq!( + PathStyle::Unix.parent(path), + Some(Path::new(OsStr::from_bytes(b"/home/user/\xff\xfe/repo"))) + ); + + let invalid_filename_path = Path::new(OsStr::from_bytes(b"/home/user/repo/\xff\xfe.txt")); + assert_eq!(PathStyle::Unix.file_name(invalid_filename_path), None); + assert_eq!( + PathStyle::Unix.parent(invalid_filename_path), + Some(Path::new("/home/user/repo")) + ); + } + + #[cfg(windows)] + #[test] + fn test_local_path_style_non_utf8() { + use std::ffi::OsString; + use std::os::windows::ffi::OsStringExt; + + let wide: Vec = "C:\\invalid_" + .encode_utf16() + .chain(Some(0xD800)) + .chain(r"\repo\file.txt".encode_utf16()) + .collect(); + let os_str = OsString::from_wide(&wide); + let path = Path::new(&os_str); + + assert_eq!(PathStyle::Windows.file_name(path), Some("file.txt")); + let parent = PathStyle::Windows.parent(path).unwrap(); + assert_eq!(PathStyle::Windows.file_name(parent), Some("repo")); + } +} diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 32656c527c..a6c44c7901 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -10467,7 +10467,7 @@ pub async fn resolve_git_worktree_to_main_repo(fs: &dyn Fs, path: &Path) -> Opti .canonicalize(&gitdir_abs.join(commondir_content.trim())) .await .ok()?; - Some(repo_identity_path(&common_dir).to_path_buf()) + Some(repo_identity_path(&common_dir, PathStyle::local()).to_path_buf()) } /// Validates that the resolved worktree directory is acceptable: @@ -10587,12 +10587,12 @@ async fn remove_empty_managed_worktree_ancestors(fs: &dyn Fs, child_path: &Path, /// meaningful project root. /// - Otherwise (e.g. `zed.git` for a bare clone), `common_dir` itself is /// returned — it is already a meaningful on-disk path. -pub fn repo_identity_path(common_dir: &Path) -> &Path { - let is_dot_entry = common_dir - .file_name() - .is_some_and(|n| n.to_string_lossy().starts_with('.')); +pub fn repo_identity_path(common_dir: &Path, path_style: PathStyle) -> &Path { + let is_dot_entry = path_style + .file_name(common_dir) + .is_some_and(|n| n.starts_with('.')); if is_dot_entry { - common_dir.parent().unwrap_or(common_dir) + path_style.parent(common_dir).unwrap_or(common_dir) } else { common_dir } diff --git a/crates/project/src/worktree_store.rs b/crates/project/src/worktree_store.rs index 2445ad57b0..300d9cf448 100644 --- a/crates/project/src/worktree_store.rs +++ b/crates/project/src/worktree_store.rs @@ -1487,7 +1487,7 @@ impl WorktreeStore { let main_path = snapshot .root_repo_common_dir() .filter(|dir| !crate::git_store::is_submodule_git_dir(dir)) - .map(|dir| crate::git_store::repo_identity_path(dir)) + .map(|dir| crate::git_store::repo_identity_path(dir, snapshot.path_style())) .filter(|repo_path| { snapshot.root_repo_is_linked_worktree() || *repo_path == folder_path.as_path() diff --git a/crates/project/tests/integration/git_store.rs b/crates/project/tests/integration/git_store.rs index 4557a788bd..fb2db48fef 100644 --- a/crates/project/tests/integration/git_store.rs +++ b/crates/project/tests/integration/git_store.rs @@ -1637,6 +1637,7 @@ mod trust_tests { mod resolve_worktree_tests { use fs::FakeFs; use gpui::TestAppContext; + use path::PathStyle; use project::{ git_store::resolve_git_worktree_to_main_repo, linked_worktree_short_name, repo_identity_path, @@ -1761,7 +1762,24 @@ mod resolve_worktree_tests { ]; for (common_dir, expected) in examples { assert_eq!( - repo_identity_path(Path::new(common_dir)), + repo_identity_path(Path::new(common_dir), PathStyle::local()), + Path::new(expected), + "identity path for common_dir {common_dir:?} should be {expected:?}" + ); + } + } + + #[test] + fn test_windows_remote_identity_path() { + let examples = [ + (r"C:\Users\zed\.git", r"C:\Users\zed"), + (r"C:\Users\project\.bare", r"C:\Users\project"), + (r"C:\Users\zed.git", r"C:\Users\zed.git"), + (r"C:\Users\zed", r"C:\Users\zed"), + ]; + for (common_dir, expected) in examples { + assert_eq!( + repo_identity_path(Path::new(common_dir), PathStyle::Windows), Path::new(expected), "identity path for common_dir {common_dir:?} should be {expected:?}" ); diff --git a/crates/title_bar/src/title_bar.rs b/crates/title_bar/src/title_bar.rs index f50a762649..dbc398e228 100644 --- a/crates/title_bar/src/title_bar.rs +++ b/crates/title_bar/src/title_bar.rs @@ -259,15 +259,15 @@ impl Render for TitleBar { .flatten() }); - let identity = repo_identity_path(&repo.common_dir_abs_path); + let identity = repo_identity_path(&repo.common_dir_abs_path, repo.path_style); let display_name = if identity.extension() == Some(std::ffi::OsStr::new("git")) { - identity.file_stem() + identity.file_stem().and_then(|n| n.to_str()) } else { - identity.file_name() + repo.path_style.file_name(identity) }; - if let Some(repo_name) = display_name.and_then(|n| n.to_str()) { + if let Some(repo_name) = display_name { let visible_worktrees_in_repo = self.visible_worktrees_in_repository(repo, cx); let name = if visible_worktrees_in_repo == 1 { if let Ok(relative) = diff --git a/crates/util/Cargo.toml b/crates/util/Cargo.toml index 125ff0389a..0ab698b84d 100644 --- a/crates/util/Cargo.toml +++ b/crates/util/Cargo.toml @@ -66,5 +66,4 @@ windows.workspace = true [dev-dependencies] rand.workspace = true util_macros.workspace = true -pretty_assertions.workspace = true path = { workspace = true, features = ["test-support"] } diff --git a/crates/util/src/paths.rs b/crates/util/src/paths.rs index dec52c65d3..c2c3268714 100644 --- a/crates/util/src/paths.rs +++ b/crates/util/src/paths.rs @@ -1468,8 +1468,6 @@ impl UrlExt for url::Url { #[cfg(test)] mod tests { - use path::rel_path::rel_path; - use super::*; use util_macros::perf; @@ -1483,82 +1481,6 @@ mod tests { assert_eq!(parsed.row, Some(3)); } - #[test] - fn test_join_path_uses_path_style_separator() { - let posix_path = PathStyle::Unix - .join_path(Path::new("/home/user/dev"), "worktrees") - .unwrap(); - let windows_path = PathStyle::Windows - .join_path(Path::new("C:\\Users\\user\\dev"), "worktrees") - .unwrap(); - - assert_eq!(posix_path, PathBuf::from("/home/user/dev/worktrees")); - assert_eq!( - windows_path.to_string_lossy(), - "C:\\Users\\user\\dev\\worktrees" - ); - } - - #[test] - fn test_normalize_uses_path_style_separator() { - assert_eq!( - PathStyle::Unix.normalize("/home/user/dev/../worktrees/./zed"), - "/home/user/worktrees/zed" - ); - assert_eq!( - PathStyle::Windows.normalize("C:\\Users\\user\\dev\\worktrees"), - "C:\\Users\\user\\dev\\worktrees" - ); - } - - #[test] - fn test_normalize_windows_path_regardless_of_host_platform() { - assert_eq!( - PathStyle::Windows.normalize(r"C:\Users\user\dev\..\worktrees"), - r"C:\Users\user\worktrees" - ); - assert_eq!( - PathStyle::Windows.normalize(r"C:\Users\.\worktrees"), - r"C:\Users\worktrees" - ); - assert_eq!( - PathStyle::Windows.normalize(r"C:\Users\user\dev\sub\..\..\worktrees"), - r"C:\Users\user\worktrees" - ); - assert_eq!( - PathStyle::Windows.normalize("C:/Users/user/dev/../worktrees"), - r"C:\Users\user\worktrees" - ); - assert_eq!( - PathStyle::Windows.normalize(r"C:/Users\user/dev\..\worktrees"), - r"C:\Users\user\worktrees" - ); - assert_eq!( - PathStyle::Windows.normalize(r"C:\Users/user\.\worktrees"), - r"C:\Users\user\worktrees" - ); - assert_eq!( - PathStyle::Windows.normalize(r"\\server\share\dev\..\worktrees"), - r"\\server\share\worktrees" - ); - assert_eq!( - PathStyle::Windows.normalize(r"//server\share/dev\..\worktrees"), - r"\\server\share\worktrees" - ); - assert_eq!( - PathStyle::Windows.normalize(r"\dev\..\worktrees"), - r"\worktrees" - ); - assert_eq!( - PathStyle::Windows.normalize(r"dev\..\worktrees"), - r"worktrees" - ); - assert_eq!( - PathStyle::Windows.normalize(r"C:\..\worktrees"), - r"C:\worktrees" - ); - } - fn rel_path_entry(path: &'static str, is_file: bool) -> (&'static RelPath, bool) { (RelPath::from_unix_str(path).unwrap(), is_file) } @@ -3117,89 +3039,6 @@ mod tests { assert_eq!(strip_path_suffix(base, suffix), None); } - #[test] - fn test_strip_prefix() { - let expected = [ - ( - PathStyle::Unix, - "/a/b/c", - "/a/b", - Some(rel_path("c").into_arc()), - ), - ( - PathStyle::Unix, - "/a/b/c", - "/a/b/", - Some(rel_path("c").into_arc()), - ), - ( - PathStyle::Unix, - "/a/b/c", - "/", - Some(rel_path("a/b/c").into_arc()), - ), - (PathStyle::Unix, "/a/b/c", "", None), - (PathStyle::Unix, "/a/b//c", "/a/b/", None), - (PathStyle::Unix, "/a/bc", "/a/b", None), - ( - PathStyle::Unix, - "/a/b/c", - "/a/b/c", - Some(rel_path("").into_arc()), - ), - ( - PathStyle::Windows, - "C:\\a\\b\\c", - "C:\\a\\b", - Some(rel_path("c").into_arc()), - ), - ( - PathStyle::Windows, - "C:\\a\\b\\c", - "C:\\a\\b\\", - Some(rel_path("c").into_arc()), - ), - ( - PathStyle::Windows, - "C:\\a\\b\\c", - "C:\\", - Some(rel_path("a/b/c").into_arc()), - ), - (PathStyle::Windows, "C:\\a\\b\\c", "", None), - (PathStyle::Windows, "C:\\a\\b\\\\c", "C:\\a\\b\\", None), - (PathStyle::Windows, "C:\\a\\bc", "C:\\a\\b", None), - ( - PathStyle::Windows, - "C:\\a\\b/c", - "C:\\a\\b", - Some(rel_path("c").into_arc()), - ), - ( - PathStyle::Windows, - "C:\\a\\b/c", - "C:\\a\\b\\", - Some(rel_path("c").into_arc()), - ), - ( - PathStyle::Windows, - "C:\\a\\b/c", - "C:\\a\\b/", - Some(rel_path("c").into_arc()), - ), - ]; - let actual = expected.clone().map(|(style, child, parent, _)| { - ( - style, - child, - parent, - style - .strip_prefix(child.as_ref(), parent.as_ref()) - .map(|rel_path| rel_path.into_arc()), - ) - }); - pretty_assertions::assert_eq!(actual, expected); - } - #[cfg(target_os = "windows")] #[test] fn test_wsl_path() { diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index d6c959d781..c9942cebbb 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -9714,7 +9714,7 @@ pub async fn apply_restored_multiworkspace_state( project::discover_root_repo_common_dir(path, fs.as_ref()).await && !project::is_submodule_git_dir(&common_dir) { - let main_path = project::repo_identity_path(&common_dir); + let main_path = project::repo_identity_path(&common_dir, PathStyle::local()); resolved_paths.push(main_path.to_path_buf()); } else { resolved_paths.push(path.to_path_buf()); diff --git a/docs/src/remote-development.md b/docs/src/remote-development.md index d23f9520aa..768e2d2d7d 100644 --- a/docs/src/remote-development.md +++ b/docs/src/remote-development.md @@ -37,7 +37,7 @@ The remote machine must be able to run Zed's server. The following platforms sho - macOS Catalina or later (Intel or Apple Silicon) - Linux (x86_64 or arm64, we do not yet support 32-bit platforms) -- Windows is not yet supported as a remote server, but Windows can be used as a local machine to connect to remote servers. +- Windows (x86_64 or arm64) ## Configuration