Represent relative paths using a dedicated, separator-agnostic type (#38744)

Closes https://github.com/zed-industries/zed/issues/38690
Closes #37353

### Background

On Windows, paths are normally separated by `\`, unlike mac and linux
where they are separated by `/`. When editing code in a project that
uses a different path style than your local system (e.g. remoting from
Windows to Linux, using WSL, and collaboration between windows and unix
users), the correct separator for a path may differ from the "native"
separator.

Previously, to work around this, Zed converted paths' separators in
numerous places. This was applied to both absolute and relative paths,
leading to incorrect conversions in some cases.

### Solution

Many code paths in Zed use paths that are *relative* to either a
worktree root or a git repository. This PR introduces a dedicated type
for these paths called `RelPath`, which stores the path in the same way
regardless of host platform, and offers `Path`-like manipulation APIs.
RelPath supports *displaying* the path using either separator, so that
we can display paths in a style that is determined at runtime based on
the current project.

The representation of absolute paths is left untouched, for now.
Absolute paths are different from relative paths because (except in
contexts where we know that the path refers to the local filesystem)
they should generally be treated as opaque strings. Currently we use a
mix of types for these paths (std::path::Path, String, SanitizedPath).

Release Notes:

- N/A

---------

Co-authored-by: Cole Miller <cole@zed.dev>
Co-authored-by: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com>
Co-authored-by: Peter Tripp <petertripp@gmail.com>
Co-authored-by: Smit Barmase <heysmitbarmase@gmail.com>
Co-authored-by: Lukas Wirth <me@lukaswirth.dev>

crates/git/src/blame.rs

@@ -1,4 +1,5 @@
 use crate::commit::get_messages;
+use crate::repository::RepoPath;
 use crate::{GitRemote, Oid};
 use anyhow::{Context as _, Result};
 use collections::{HashMap, HashSet};
@@ -33,7 +34,7 @@ impl Blame {
     pub async fn for_path(
         git_binary: &Path,
         working_directory: &Path,
-        path: &Path,
+        path: &RepoPath,
         content: &Rope,
         remote_url: Option<String>,
     ) -> Result<Self> {
@@ -66,7 +67,7 @@ const GIT_BLAME_NO_PATH: &str = "fatal: no such path";
 async fn run_git_blame(
     git_binary: &Path,
     working_directory: &Path,
-    path: &Path,
+    path: &RepoPath,
     contents: &Rope,
 ) -> Result<String> {
     let mut child = util::command::new_smol_command(git_binary)
@@ -76,7 +77,7 @@ async fn run_git_blame(
         .arg("-w")
         .arg("--contents")
         .arg("-")
-        .arg(path.as_os_str())
+        .arg(path.as_str())
         .stdin(Stdio::piped())
         .stdout(Stdio::piped())
         .stderr(Stdio::piped())

crates/git/src/commit.rs

@@ -39,7 +39,7 @@ pub async fn get_messages(working_directory: &Path, shas: &[Oid]) -> Result<Hash
 }
 
 /// Parse the output of `git diff --name-status -z`
-pub fn parse_git_diff_name_status(content: &str) -> impl Iterator<Item = (&Path, StatusCode)> {
+pub fn parse_git_diff_name_status(content: &str) -> impl Iterator<Item = (&str, StatusCode)> {
     let mut parts = content.split('\0');
     std::iter::from_fn(move || {
         loop {
@@ -51,13 +51,14 @@ pub fn parse_git_diff_name_status(content: &str) -> impl Iterator<Item = (&Path,
                 "D" => StatusCode::Deleted,
                 _ => continue,
             };
-            return Some((Path::new(path), status));
+            return Some((path, status));
         }
     })
 }
 
 #[cfg(test)]
 mod tests {
+
     use super::*;
 
     #[test]
@@ -78,31 +79,19 @@ mod tests {
         assert_eq!(
             output,
             &[
-                (Path::new("Cargo.lock"), StatusCode::Modified),
-                (Path::new("crates/project/Cargo.toml"), StatusCode::Modified),
+                ("Cargo.lock", StatusCode::Modified),
+                ("crates/project/Cargo.toml", StatusCode::Modified),
+                ("crates/project/src/buffer_store.rs", StatusCode::Modified),
+                ("crates/project/src/git.rs", StatusCode::Deleted),
+                ("crates/project/src/git_store.rs", StatusCode::Added),
                 (
-                    Path::new("crates/project/src/buffer_store.rs"),
-                    StatusCode::Modified
-                ),
-                (Path::new("crates/project/src/git.rs"), StatusCode::Deleted),
-                (
-                    Path::new("crates/project/src/git_store.rs"),
-                    StatusCode::Added
-                ),
-                (
-                    Path::new("crates/project/src/git_store/git_traversal.rs"),
+                    "crates/project/src/git_store/git_traversal.rs",
                     StatusCode::Added,
                 ),
+                ("crates/project/src/project.rs", StatusCode::Modified),
+                ("crates/project/src/worktree_store.rs", StatusCode::Modified),
                 (
-                    Path::new("crates/project/src/project.rs"),
-                    StatusCode::Modified
-                ),
-                (
-                    Path::new("crates/project/src/worktree_store.rs"),
-                    StatusCode::Modified
-                ),
-                (
-                    Path::new("crates/project_panel/src/project_panel.rs"),
+                    "crates/project_panel/src/project_panel.rs",
                     StatusCode::Modified
                 ),
             ]

crates/git/src/git.rs

@@ -12,22 +12,17 @@ use anyhow::{Context as _, Result};
 pub use git2 as libgit;
 use gpui::{Action, actions};
 pub use repository::RemoteCommandOutput;
-pub use repository::WORK_DIRECTORY_REPO_PATH;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
-use std::ffi::OsStr;
 use std::fmt;
 use std::str::FromStr;
-use std::sync::LazyLock;
 
-pub static DOT_GIT: LazyLock<&'static OsStr> = LazyLock::new(|| OsStr::new(".git"));
-pub static GITIGNORE: LazyLock<&'static OsStr> = LazyLock::new(|| OsStr::new(".gitignore"));
-pub static FSMONITOR_DAEMON: LazyLock<&'static OsStr> =
-    LazyLock::new(|| OsStr::new("fsmonitor--daemon"));
-pub static LFS_DIR: LazyLock<&'static OsStr> = LazyLock::new(|| OsStr::new("lfs"));
-pub static COMMIT_MESSAGE: LazyLock<&'static OsStr> =
-    LazyLock::new(|| OsStr::new("COMMIT_EDITMSG"));
-pub static INDEX_LOCK: LazyLock<&'static OsStr> = LazyLock::new(|| OsStr::new("index.lock"));
+pub const DOT_GIT: &str = ".git";
+pub const GITIGNORE: &str = ".gitignore";
+pub const FSMONITOR_DAEMON: &str = "fsmonitor--daemon";
+pub const LFS_DIR: &str = "lfs";
+pub const COMMIT_MESSAGE: &str = "COMMIT_EDITMSG";
+pub const INDEX_LOCK: &str = "index.lock";
 
 actions!(
     git,

crates/git/src/repository.rs

@@ -12,12 +12,9 @@ use parking_lot::Mutex;
 use rope::Rope;
 use schemars::JsonSchema;
 use serde::Deserialize;
-use std::borrow::{Borrow, Cow};
 use std::ffi::{OsStr, OsString};
 use std::io::prelude::*;
-use std::path::Component;
 use std::process::{ExitStatus, Stdio};
-use std::sync::LazyLock;
 use std::{
     cmp::Ordering,
     future,
@@ -28,6 +25,8 @@ use std::{
 use sum_tree::MapSeekTarget;
 use thiserror::Error;
 use util::command::{new_smol_command, new_std_command};
+use util::paths::PathStyle;
+use util::rel_path::RelPath;
 use util::{ResultExt, paths};
 use uuid::Uuid;
 
@@ -719,16 +718,21 @@ impl GitRepository for RealGitRepository {
             let mut info_line = String::new();
             let mut newline = [b'\0'];
             for (path, status_code) in changes {
+                // git-show outputs `/`-delimited paths even on Windows.
+                let Ok(rel_path) = RelPath::new(path) else {
+                    continue;
+                };
+
                 match status_code {
                     StatusCode::Modified => {
-                        writeln!(&mut stdin, "{commit}:{}", path.display())?;
-                        writeln!(&mut stdin, "{parent_sha}:{}", path.display())?;
+                        writeln!(&mut stdin, "{commit}:{path}")?;
+                        writeln!(&mut stdin, "{parent_sha}:{path}")?;
                     }
                     StatusCode::Added => {
-                        writeln!(&mut stdin, "{commit}:{}", path.display())?;
+                        writeln!(&mut stdin, "{commit}:{path}")?;
                     }
                     StatusCode::Deleted => {
-                        writeln!(&mut stdin, "{parent_sha}:{}", path.display())?;
+                        writeln!(&mut stdin, "{parent_sha}:{path}")?;
                     }
                     _ => continue,
                 }
@@ -766,7 +770,7 @@ impl GitRepository for RealGitRepository {
                 }
 
                 files.push(CommitFile {
-                    path: path.into(),
+                    path: rel_path.into(),
                     old_text,
                     new_text,
                 })
@@ -824,7 +828,7 @@ impl GitRepository for RealGitRepository {
                 .current_dir(&working_directory?)
                 .envs(env.iter())
                 .args(["checkout", &commit, "--"])
-                .args(paths.iter().map(|path| path.as_ref()))
+                .args(paths.iter().map(|path| path.as_str()))
                 .output()
                 .await?;
             anyhow::ensure!(
@@ -846,13 +850,11 @@ impl GitRepository for RealGitRepository {
             .spawn(async move {
                 fn logic(repo: &git2::Repository, path: &RepoPath) -> Result<Option<String>> {
                     // This check is required because index.get_path() unwraps internally :(
-                    check_path_to_repo_path_errors(path)?;
-
                     let mut index = repo.index()?;
                     index.read(false)?;
 
                     const STAGE_NORMAL: i32 = 0;
-                    let oid = match index.get_path(path, STAGE_NORMAL) {
+                    let oid = match index.get_path(path.as_std_path(), STAGE_NORMAL) {
                         Some(entry) if entry.mode != GIT_MODE_SYMLINK => entry.id,
                         _ => return Ok(None),
                     };
@@ -876,7 +878,7 @@ impl GitRepository for RealGitRepository {
             .spawn(async move {
                 let repo = repo.lock();
                 let head = repo.head().ok()?.peel_to_tree().log_err()?;
-                let entry = head.get_path(&path).ok()?;
+                let entry = head.get_path(path.as_std_path()).ok()?;
                 if entry.filemode() == i32::from(git2::FileMode::Link) {
                     return None;
                 }
@@ -918,7 +920,7 @@ impl GitRepository for RealGitRepository {
                         .current_dir(&working_directory)
                         .envs(env.iter())
                         .args(["update-index", "--add", "--cacheinfo", "100644", sha])
-                        .arg(path.to_unix_style())
+                        .arg(path.as_str())
                         .output()
                         .await?;
 
@@ -933,7 +935,7 @@ impl GitRepository for RealGitRepository {
                         .current_dir(&working_directory)
                         .envs(env.iter())
                         .args(["update-index", "--force-remove"])
-                        .arg(path.to_unix_style())
+                        .arg(path.as_str())
                         .output()
                         .await?;
                     anyhow::ensure!(
@@ -1251,7 +1253,7 @@ impl GitRepository for RealGitRepository {
                         .current_dir(&working_directory?)
                         .envs(env.iter())
                         .args(["update-index", "--add", "--remove", "--"])
-                        .args(paths.iter().map(|p| p.to_unix_style()))
+                        .args(paths.iter().map(|p| p.as_str()))
                         .output()
                         .await?;
                     anyhow::ensure!(
@@ -1812,7 +1814,7 @@ fn git_status_args(path_prefixes: &[RepoPath]) -> Vec<OsString> {
         OsString::from("-z"),
     ];
     args.extend(path_prefixes.iter().map(|path_prefix| {
-        if path_prefix.0.as_ref() == Path::new("") {
+        if path_prefix.is_empty() {
             Path::new(".").into()
         } else {
             path_prefix.as_os_str().into()
@@ -2066,99 +2068,65 @@ async fn run_askpass_command(
     }
 }
 
-pub static WORK_DIRECTORY_REPO_PATH: LazyLock<RepoPath> =
-    LazyLock::new(|| RepoPath(Path::new("").into()));
-
 #[derive(Clone, Debug, Ord, Hash, PartialOrd, Eq, PartialEq)]
-pub struct RepoPath(pub Arc<Path>);
+pub struct RepoPath(pub Arc<RelPath>);
 
 impl RepoPath {
-    pub fn new(path: PathBuf) -> Self {
-        debug_assert!(path.is_relative(), "Repo paths must be relative");
-
-        RepoPath(path.into())
+    pub fn new<S: AsRef<str> + ?Sized>(s: &S) -> Result<Self> {
+        let rel_path = RelPath::new(s)?;
+        Ok(rel_path.into())
     }
 
-    pub fn from_str(path: &str) -> Self {
-        let path = Path::new(path);
-        debug_assert!(path.is_relative(), "Repo paths must be relative");
-
-        RepoPath(path.into())
+    pub fn from_proto(proto: &str) -> Result<Self> {
+        let rel_path = RelPath::from_proto(proto)?;
+        Ok(rel_path.into())
     }
 
-    pub fn to_unix_style(&self) -> Cow<'_, OsStr> {
-        #[cfg(target_os = "windows")]
-        {
-            use std::ffi::OsString;
-
-            let path = self.0.as_os_str().to_string_lossy().replace("\\", "/");
-            Cow::Owned(OsString::from(path))
-        }
-        #[cfg(not(target_os = "windows"))]
-        {
-            Cow::Borrowed(self.0.as_os_str())
-        }
+    pub fn from_std_path(path: &Path, path_style: PathStyle) -> Result<Self> {
+        let rel_path = RelPath::from_std_path(path, path_style)?;
+        Ok(rel_path.into())
     }
 }
 
-impl std::fmt::Display for RepoPath {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        self.0.to_string_lossy().fmt(f)
+#[cfg(any(test, feature = "test-support"))]
+pub fn repo_path<S: AsRef<str> + ?Sized>(s: &S) -> RepoPath {
+    RepoPath(RelPath::new(s).unwrap().into())
+}
+
+impl From<&RelPath> for RepoPath {
+    fn from(value: &RelPath) -> Self {
+        RepoPath(value.into())
     }
 }
 
-impl From<&Path> for RepoPath {
-    fn from(value: &Path) -> Self {
-        RepoPath::new(value.into())
-    }
-}
-
-impl From<Arc<Path>> for RepoPath {
-    fn from(value: Arc<Path>) -> Self {
+impl From<Arc<RelPath>> for RepoPath {
+    fn from(value: Arc<RelPath>) -> Self {
         RepoPath(value)
     }
 }
 
-impl From<PathBuf> for RepoPath {
-    fn from(value: PathBuf) -> Self {
-        RepoPath::new(value)
-    }
-}
-
-impl From<&str> for RepoPath {
-    fn from(value: &str) -> Self {
-        Self::from_str(value)
-    }
-}
-
 impl Default for RepoPath {
     fn default() -> Self {
-        RepoPath(Path::new("").into())
-    }
-}
-
-impl AsRef<Path> for RepoPath {
-    fn as_ref(&self) -> &Path {
-        self.0.as_ref()
+        RepoPath(RelPath::empty().into())
     }
 }
 
 impl std::ops::Deref for RepoPath {
-    type Target = Path;
+    type Target = RelPath;
 
     fn deref(&self) -> &Self::Target {
         &self.0
     }
 }
 
-impl Borrow<Path> for RepoPath {
-    fn borrow(&self) -> &Path {
-        self.0.as_ref()
+impl AsRef<Path> for RepoPath {
+    fn as_ref(&self) -> &Path {
+        RelPath::as_ref(&self.0)
     }
 }
 
 #[derive(Debug)]
-pub struct RepoPathDescendants<'a>(pub &'a Path);
+pub struct RepoPathDescendants<'a>(pub &'a RepoPath);
 
 impl MapSeekTarget<RepoPath> for RepoPathDescendants<'_> {
     fn cmp_cursor(&self, key: &RepoPath) -> Ordering {
@@ -2244,35 +2212,6 @@ fn parse_upstream_track(upstream_track: &str) -> Result<UpstreamTracking> {
     }))
 }
 
-fn check_path_to_repo_path_errors(relative_file_path: &Path) -> Result<()> {
-    match relative_file_path.components().next() {
-        None => anyhow::bail!("repo path should not be empty"),
-        Some(Component::Prefix(_)) => anyhow::bail!(
-            "repo path `{}` should be relative, not a windows prefix",
-            relative_file_path.to_string_lossy()
-        ),
-        Some(Component::RootDir) => {
-            anyhow::bail!(
-                "repo path `{}` should be relative",
-                relative_file_path.to_string_lossy()
-            )
-        }
-        Some(Component::CurDir) => {
-            anyhow::bail!(
-                "repo path `{}` should not start with `.`",
-                relative_file_path.to_string_lossy()
-            )
-        }
-        Some(Component::ParentDir) => {
-            anyhow::bail!(
-                "repo path `{}` should not start with `..`",
-                relative_file_path.to_string_lossy()
-            )
-        }
-        _ => Ok(()),
-    }
-}
-
 fn checkpoint_author_envs() -> HashMap<String, String> {
     HashMap::from_iter([
         ("GIT_AUTHOR_NAME".to_string(), "Zed".to_string()),
@@ -2299,12 +2238,9 @@ mod tests {
 
         let repo =
             RealGitRepository::new(&repo_dir.path().join(".git"), None, cx.executor()).unwrap();
-        repo.stage_paths(
-            vec![RepoPath::from_str("file")],
-            Arc::new(HashMap::default()),
-        )
-        .await
-        .unwrap();
+        repo.stage_paths(vec![repo_path("file")], Arc::new(HashMap::default()))
+            .await
+            .unwrap();
         repo.commit(
             "Initial commit".into(),
             None,
@@ -2328,12 +2264,9 @@ mod tests {
         smol::fs::write(&file_path, "modified after checkpoint")
             .await
             .unwrap();
-        repo.stage_paths(
-            vec![RepoPath::from_str("file")],
-            Arc::new(HashMap::default()),
-        )
-        .await
-        .unwrap();
+        repo.stage_paths(vec![repo_path("file")], Arc::new(HashMap::default()))
+            .await
+            .unwrap();
         repo.commit(
             "Commit after checkpoint".into(),
             None,
@@ -2466,12 +2399,9 @@ mod tests {
             RealGitRepository::new(&repo_dir.path().join(".git"), None, cx.executor()).unwrap();
 
         // initial commit
-        repo.stage_paths(
-            vec![RepoPath::from_str("main.rs")],
-            Arc::new(HashMap::default()),
-        )
-        .await
-        .unwrap();
+        repo.stage_paths(vec![repo_path("main.rs")], Arc::new(HashMap::default()))
+            .await
+            .unwrap();
         repo.commit(
             "Initial commit".into(),
             None,

crates/git/src/status.rs

@@ -1,8 +1,8 @@
 use crate::repository::RepoPath;
 use anyhow::Result;
 use serde::{Deserialize, Serialize};
-use std::{path::Path, str::FromStr, sync::Arc};
-use util::ResultExt;
+use std::{str::FromStr, sync::Arc};
+use util::{ResultExt, rel_path::RelPath};
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
 pub enum FileStatus {
@@ -447,7 +447,8 @@ impl FromStr for GitStatus {
                 }
                 let status = entry.as_bytes()[0..2].try_into().unwrap();
                 let status = FileStatus::from_bytes(status).log_err()?;
-                let path = RepoPath(Path::new(path).into());
+                // git-status outputs `/`-delimited repo paths, even on Windows.
+                let path = RepoPath(RelPath::new(path).log_err()?.into());
                 Some((path, status))
             })
             .collect::<Vec<_>>();