wip

Julia Ryan created

Change summary

crates/fs/src/fake_git_repo.rs  |  6 ++++
crates/git/src/blame.rs         |  5 ++-
crates/git/src/repository.rs    | 42 +++++-----------------------------
crates/git/src/status.rs        |  6 ++--
crates/util/src/rel_path.rs     | 39 ++++++++++++++++++++++++++++++-
crates/worktree/src/worktree.rs |  2 
6 files changed, 56 insertions(+), 44 deletions(-)

Detailed changes

crates/fs/src/fake_git_repo.rs 🔗

@@ -15,6 +15,7 @@ use ignore::gitignore::GitignoreBuilder;
 use rope::Rope;
 use smol::future::FutureExt as _;
 use std::{path::PathBuf, sync::Arc};
+use util::rel_path::RelPath;
 
 #[derive(Clone)]
 pub struct FakeGitRepository {
@@ -222,7 +223,10 @@ impl GitRepository for FakeGitRepository {
                     .read_file_sync(path)
                     .ok()
                     .map(|content| String::from_utf8(content).unwrap())?;
-                Some((repo_path.into(), (content, is_ignored)))
+                Some((
+                    RepoPath::from(&RelPath::new(repo_path)),
+                    (content, is_ignored),
+                ))
             })
             .collect();
 

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)

crates/git/src/repository.rs 🔗

@@ -774,7 +774,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.to_unix_style()))
                 .output()
                 .await?;
             anyhow::ensure!(
@@ -796,13 +796,14 @@ 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)?;
+                    // TODO: move this function to where we instantiate the repopaths
+                    // 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::new(&path.to_unix_style()), STAGE_NORMAL) {
                         Some(entry) if entry.mode != GIT_MODE_SYMLINK => entry.id,
                         _ => return Ok(None),
                     };
@@ -826,7 +827,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::new(&path.as_os_str())).ok()?;
                 if entry.filemode() == i32::from(git2::FileMode::Link) {
                     return None;
                 }
@@ -1193,7 +1194,7 @@ impl GitRepository for RealGitRepository {
                         .current_dir(&working_directory?)
                         .envs(env.iter())
                         .args(["reset", "--quiet", "--"])
-                        .args(paths.iter().map(|p| p.as_ref()))
+                        .args(paths.iter().map(|p| p.to_unix_style()))
                         .output()
                         .await?;
 
@@ -1222,7 +1223,7 @@ impl GitRepository for RealGitRepository {
                     .args(["stash", "push", "--quiet"])
                     .arg("--include-untracked");
 
-                cmd.args(paths.iter().map(|p| p.as_ref()));
+                cmd.args(paths.iter().map(|p| p.to_unix_style()));
 
                 let output = cmd.output().await?;
 
@@ -2058,35 +2059,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()),

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 {
@@ -464,7 +464,7 @@ 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());
+                let path = RepoPath(RelPath::new(path.as_bytes()).into());
                 Some((path, status))
             })
             .collect::<Vec<_>>();

crates/util/src/rel_path.rs 🔗

@@ -1,10 +1,13 @@
 use std::{
     borrow::Cow,
     ffi::OsStr,
-    path::{Display, Path, PathBuf},
+    os::unix::ffi::OsStrExt,
+    path::{Path, PathBuf},
     sync::Arc,
 };
 
+use anyhow::{Result, bail};
+
 #[repr(transparent)]
 #[derive(PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct RelPath([u8]);
@@ -50,6 +53,36 @@ impl RelPath {
             .ok_or_else(|| ())
     }
 
+    pub fn from_path(relative_path: &Path) -> Result<&Self> {
+        use std::path::Component;
+        match relative_path.components().next() {
+            Some(Component::Prefix(_)) => bail!(
+                "path `{}` should be relative, not a windows prefix",
+                relative_path.to_string_lossy()
+            ),
+            Some(Component::RootDir) => {
+                bail!(
+                    "path `{}` should be relative",
+                    relative_path.to_string_lossy()
+                )
+            }
+            Some(Component::CurDir) => {
+                bail!(
+                    "path `{}` should not start with `.`",
+                    relative_path.to_string_lossy()
+                )
+            }
+            Some(Component::ParentDir) => {
+                bail!(
+                    "path `{}` should not start with `..`",
+                    relative_path.to_string_lossy()
+                )
+            }
+            None => bail!("relative path should not be empty"),
+            _ => Ok(Self::new(relative_path.as_os_str().as_bytes())),
+        }
+    }
+
     pub fn append_to_abs_path(&self, abs_path: &Path) -> PathBuf {
         // TODO: implement this differently
         let mut result = abs_path.to_path_buf();
@@ -79,7 +112,9 @@ impl RelPath {
         }
         #[cfg(not(target_os = "windows"))]
         {
-            Cow::Borrowed(self.0.as_os_str())
+            use std::os::unix::ffi::OsStrExt;
+
+            Cow::Borrowed(OsStr::from_bytes(&self.0))
         }
     }
 }

crates/worktree/src/worktree.rs 🔗

@@ -5217,7 +5217,7 @@ impl<'a> sum_tree::Dimension<'a, EntrySummary> for TraversalProgress<'a> {
 impl Default for TraversalProgress<'_> {
     fn default() -> Self {
         Self {
-            max_path: Path::new(""),
+            max_path: RelPath::new(""),
             count: 0,
             non_ignored_count: 0,
             file_count: 0,