Merge pull request #1682 from zed-industries/load-diff-base-from-correct-relative-path

Julia created 3 years ago

Fix some git gutter bugs

Change summary

crates/collab/src/integration_tests.rs | 142 +++++++++++++++++++++++++--
crates/editor/src/element.rs           |   4 
crates/git/src/repository.rs           |  16 ++
crates/project/src/fs.rs               |   3 
crates/project/src/project.rs          |   7 +
crates/project/src/worktree.rs         |  17 +-
6 files changed, 160 insertions(+), 29 deletions(-)

Detailed changes

crates/collab/src/integration_tests.rs

@@ -966,7 +966,14 @@ async fn test_git_diff_base_change(
         .insert_tree(
             "/dir",
             json!({
-            ".git": {
+            ".git": {},
+            "sub": {
+                ".git": {},
+                "b.txt": "
+                    one
+                    two
+                    three
+                ".unindent(),
             },
             "a.txt": "
                     one
@@ -977,6 +984,11 @@ async fn test_git_diff_base_change(
         )
         .await;
 
+    let (project_local, worktree_id) = client_a.build_local_project("/dir", cx_a).await;
+    let project_remote = client_b
+        .build_remote_project(&project_local, cx_a, cx_b)
+        .await;
+
     let diff_base = "
         one
         three
@@ -998,12 +1010,9 @@ async fn test_git_diff_base_change(
         )
         .await;
 
-    let (project_a, worktree_id) = client_a.build_local_project("/dir", cx_a).await;
-    let project_b = client_b.build_remote_project(&project_a, cx_a, cx_b).await;
-
     // Create the buffer
-    let buffer_a = project_a
-        .update(cx_a, |p, cx| p.open_buffer((worktree_id, "/dir/a.txt"), cx))
+    let buffer_local_a = project_local
+        .update(cx_a, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx))
         .await
         .unwrap();
 
@@ -1011,7 +1020,7 @@ async fn test_git_diff_base_change(
     executor.run_until_parked();
 
     // Smoke test diffing
-    buffer_a.read_with(cx_a, |buffer, _| {
+    buffer_local_a.read_with(cx_a, |buffer, _| {
         assert_eq!(buffer.diff_base(), Some(diff_base.as_ref()));
         git::diff::assert_hunks(
             buffer.snapshot().git_diff_hunks_in_range(0..4),
@@ -1022,8 +1031,8 @@ async fn test_git_diff_base_change(
     });
 
     // Create remote buffer
-    let buffer_b = project_b
-        .update(cx_b, |p, cx| p.open_buffer((worktree_id, "/dir/a.txt"), cx))
+    let buffer_remote_a = project_remote
+        .update(cx_b, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx))
         .await
         .unwrap();
 
@@ -1031,7 +1040,7 @@ async fn test_git_diff_base_change(
     executor.run_until_parked();
 
     // Smoke test diffing
-    buffer_b.read_with(cx_b, |buffer, _| {
+    buffer_remote_a.read_with(cx_b, |buffer, _| {
         assert_eq!(buffer.diff_base(), Some(diff_base.as_ref()));
         git::diff::assert_hunks(
             buffer.snapshot().git_diff_hunks_in_range(0..4),
@@ -1050,11 +1059,11 @@ async fn test_git_diff_base_change(
         )
         .await;
 
-    // Wait for buffer_a to receive it
+    // Wait for buffer_local_a to receive it
     executor.run_until_parked();
 
     // Smoke test new diffing
-    buffer_a.read_with(cx_a, |buffer, _| {
+    buffer_local_a.read_with(cx_a, |buffer, _| {
         assert_eq!(buffer.diff_base(), Some(new_diff_base.as_ref()));
 
         git::diff::assert_hunks(
@@ -1066,7 +1075,114 @@ async fn test_git_diff_base_change(
     });
 
     // Smoke test B
-    buffer_b.read_with(cx_b, |buffer, _| {
+    buffer_remote_a.read_with(cx_b, |buffer, _| {
+        assert_eq!(buffer.diff_base(), Some(new_diff_base.as_ref()));
+        git::diff::assert_hunks(
+            buffer.snapshot().git_diff_hunks_in_range(0..4),
+            &buffer,
+            &diff_base,
+            &[(2..3, "", "three\n")],
+        );
+    });
+
+    //Nested git dir
+
+    let diff_base = "
+        one
+        three
+    "
+    .unindent();
+
+    let new_diff_base = "
+        one
+        two
+    "
+    .unindent();
+
+    client_a
+        .fs
+        .as_fake()
+        .set_index_for_repo(
+            Path::new("/dir/sub/.git"),
+            &[(Path::new("b.txt"), diff_base.clone())],
+        )
+        .await;
+
+    // Create the buffer
+    let buffer_local_b = project_local
+        .update(cx_a, |p, cx| p.open_buffer((worktree_id, "sub/b.txt"), cx))
+        .await
+        .unwrap();
+
+    // Wait for it to catch up to the new diff
+    executor.run_until_parked();
+
+    // Smoke test diffing
+    buffer_local_b.read_with(cx_a, |buffer, _| {
+        assert_eq!(buffer.diff_base(), Some(diff_base.as_ref()));
+        git::diff::assert_hunks(
+            buffer.snapshot().git_diff_hunks_in_range(0..4),
+            &buffer,
+            &diff_base,
+            &[(1..2, "", "two\n")],
+        );
+    });
+
+    // Create remote buffer
+    let buffer_remote_b = project_remote
+        .update(cx_b, |p, cx| p.open_buffer((worktree_id, "sub/b.txt"), cx))
+        .await
+        .unwrap();
+
+    // Wait remote buffer to catch up to the new diff
+    executor.run_until_parked();
+
+    // Smoke test diffing
+    buffer_remote_b.read_with(cx_b, |buffer, _| {
+        assert_eq!(buffer.diff_base(), Some(diff_base.as_ref()));
+        git::diff::assert_hunks(
+            buffer.snapshot().git_diff_hunks_in_range(0..4),
+            &buffer,
+            &diff_base,
+            &[(1..2, "", "two\n")],
+        );
+    });
+
+    client_a
+        .fs
+        .as_fake()
+        .set_index_for_repo(
+            Path::new("/dir/sub/.git"),
+            &[(Path::new("b.txt"), new_diff_base.clone())],
+        )
+        .await;
+
+    // Wait for buffer_local_b to receive it
+    executor.run_until_parked();
+
+    // Smoke test new diffing
+    buffer_local_b.read_with(cx_a, |buffer, _| {
+        assert_eq!(buffer.diff_base(), Some(new_diff_base.as_ref()));
+        println!("{:?}", buffer.as_rope().to_string());
+        println!("{:?}", buffer.diff_base());
+        println!(
+            "{:?}",
+            buffer
+                .snapshot()
+                .git_diff_hunks_in_range(0..4)
+                .collect::<Vec<_>>()
+        );
+
+        git::diff::assert_hunks(
+            buffer.snapshot().git_diff_hunks_in_range(0..4),
+            &buffer,
+            &diff_base,
+            &[(2..3, "", "three\n")],
+        );
+    });
+
+    // Smoke test B
+    buffer_remote_b.read_with(cx_b, |buffer, _| {
         assert_eq!(buffer.diff_base(), Some(new_diff_base.as_ref()));
         git::diff::assert_hunks(
             buffer.snapshot().git_diff_hunks_in_range(0..4),

crates/editor/src/element.rs

@@ -630,9 +630,7 @@ impl EditorElement {
                     let new_hunk = get_hunk(diff_layout.buffer_row, &layout.diff_hunks);
 
                     let (is_ending, is_starting) = match (diff_layout.last_diff, new_hunk) {
-                        (Some(old_hunk), Some(new_hunk)) if new_hunk == old_hunk => {
-                            (false, false)
-                        }
+                        (Some(old_hunk), Some(new_hunk)) if new_hunk == old_hunk => (false, false),
                         (a, b) => (a.is_some(), b.is_some()),
                     };
 

crates/git/src/repository.rs

@@ -10,12 +10,20 @@ pub use git2::Repository as LibGitRepository;
 
 #[async_trait::async_trait]
 pub trait GitRepository: Send {
-    fn load_index(&self, relative_file_path: &Path) -> Option<String>;
+    fn reload_index(&self);
+
+    fn load_index_text(&self, relative_file_path: &Path) -> Option<String>;
 }
 
 #[async_trait::async_trait]
 impl GitRepository for LibGitRepository {
-    fn load_index(&self, relative_file_path: &Path) -> Option<String> {
+    fn reload_index(&self) {
+        if let Ok(mut index) = self.index() {
+            _ = index.read(false);
+        }
+    }
+
+    fn load_index_text(&self, relative_file_path: &Path) -> Option<String> {
         fn logic(repo: &LibGitRepository, relative_file_path: &Path) -> Result<Option<String>> {
             const STAGE_NORMAL: i32 = 0;
             let index = repo.index()?;
@@ -54,7 +62,9 @@ impl FakeGitRepository {
 
 #[async_trait::async_trait]
 impl GitRepository for FakeGitRepository {
-    fn load_index(&self, path: &Path) -> Option<String> {
+    fn reload_index(&self) {}
+
+    fn load_index_text(&self, path: &Path) -> Option<String> {
         let state = self.state.lock();
         state.index_contents.get(path).cloned()
     }

crates/project/src/fs.rs

@@ -491,7 +491,6 @@ impl FakeFs {
     }
 
     pub async fn set_index_for_repo(&self, dot_git: &Path, head_state: &[(&Path, String)]) {
-        let content_path = dot_git.parent().unwrap();
         let mut state = self.state.lock().await;
         let entry = state.read_path(dot_git).await.unwrap();
         let mut entry = entry.lock().await;
@@ -504,7 +503,7 @@ impl FakeFs {
             repo_state.index_contents.extend(
                 head_state
                     .iter()
-                    .map(|(path, content)| (content_path.join(path), content.clone())),
+                    .map(|(path, content)| (path.to_path_buf(), content.clone())),
             );
 
             state.emit_event([dot_git]);

crates/project/src/project.rs

@@ -4673,13 +4673,18 @@ impl Project {
                     None => return,
                 };
 
+                let relative_repo = match path.strip_prefix(repo.content_path) {
+                    Ok(relative_repo) => relative_repo.to_owned(),
+                    Err(_) => return,
+                };
+
                 let shared_remote_id = self.shared_remote_id();
                 let client = self.client.clone();
 
                 cx.spawn(|_, mut cx| async move {
                     let diff_base = cx
                         .background()
-                        .spawn(async move { repo.repo.lock().load_index(&path) })
+                        .spawn(async move { repo.repo.lock().load_index_text(&relative_repo) })
                         .await;
 
                     let buffer_id = buffer.update(&mut cx, |buffer, cx| {

crates/project/src/worktree.rs

@@ -667,13 +667,15 @@ impl LocalWorktree {
         cx.spawn(|this, mut cx| async move {
             let text = fs.load(&abs_path).await?;
 
-            let diff_base = if let Some(repo) = snapshot.repo_for(&abs_path) {
-                cx.background()
-                    .spawn({
-                        let path = path.clone();
-                        async move { repo.repo.lock().load_index(&path) }
-                    })
-                    .await
+            let diff_base = if let Some(repo) = snapshot.repo_for(&path) {
+                if let Ok(repo_relative) = path.strip_prefix(repo.content_path) {
+                    let repo_relative = repo_relative.to_owned();
+                    cx.background()
+                        .spawn(async move { repo.repo.lock().load_index_text(&repo_relative) })
+                        .await
+                } else {
+                    None
+                }
             } else {
                 None
             };
@@ -2503,6 +2505,7 @@ impl BackgroundScanner {
 
                         let scan_id = snapshot.scan_id;
                         if let Some(repo) = snapshot.in_dot_git(&path) {
+                            repo.repo.lock().reload_index();
                             repo.scan_id = scan_id;
                         }