git_ui: Update project diff more aggressively (#39642)

Cole Miller created

This fixes a regression in #39557--for the project diff, we rely on
getting an event when a path inside a git repository changes, even if
the git state of the repository didn't change as a result (e.g. a new
modification to a file that already had the "modified" status).

I've also changed this code to send the `UpdateRepository` proto message
even when the git state didn't change, since otherwise we have the same
problem in SSH and collab projects.

Release Notes:

- N/A

Change summary

crates/git_ui/src/project_diff.rs         | 131 ++++++++++++++++++++++++
crates/project/src/git_store.rs           |  18 +-
crates/project/src/project_tests.rs       |  24 +++
crates/project_panel/src/project_panel.rs |   4 
4 files changed, 159 insertions(+), 18 deletions(-)

Detailed changes

crates/git_ui/src/project_diff.rs 🔗

@@ -27,7 +27,7 @@ use language::{Anchor, Buffer, Capability, OffsetRangeExt};
 use multi_buffer::{MultiBuffer, PathKey};
 use project::{
     Project, ProjectPath,
-    git_store::{GitStore, GitStoreEvent, RepositoryEvent},
+    git_store::{GitStore, GitStoreEvent},
 };
 use settings::{Settings, SettingsStore};
 use std::any::{Any, TypeId};
@@ -177,7 +177,7 @@ impl ProjectDiff {
             window,
             move |this, _git_store, event, _window, _cx| match event {
                 GitStoreEvent::ActiveRepositoryChanged(_)
-                | GitStoreEvent::RepositoryUpdated(_, RepositoryEvent::Updated { .. }, true)
+                | GitStoreEvent::RepositoryUpdated(_, _, true)
                 | GitStoreEvent::ConflictsUpdated => {
                     *this.update_needed.borrow_mut() = ();
                 }
@@ -1346,7 +1346,6 @@ fn merge_anchor_ranges<'a>(
     })
 }
 
-#[cfg(not(target_os = "windows"))]
 #[cfg(test)]
 mod tests {
     use db::indoc;
@@ -1624,6 +1623,7 @@ mod tests {
         project_diff::{self, ProjectDiff},
     };
 
+    #[cfg_attr(windows, ignore = "currently fails on windows")]
     #[gpui::test]
     async fn test_go_to_prev_hunk_multibuffer(cx: &mut TestAppContext) {
         init_test(cx);
@@ -1711,6 +1711,7 @@ mod tests {
         ));
     }
 
+    #[cfg_attr(windows, ignore = "currently fails on windows")]
     #[gpui::test]
     async fn test_excerpts_splitting_after_restoring_the_middle_excerpt(cx: &mut TestAppContext) {
         init_test(cx);
@@ -1869,4 +1870,128 @@ mod tests {
         let contents = String::from_utf8(contents).unwrap();
         assert_eq!(contents, "ours\n");
     }
+
+    #[gpui::test]
+    async fn test_new_hunk_in_modified_file(cx: &mut TestAppContext) {
+        init_test(cx);
+
+        let fs = FakeFs::new(cx.executor());
+        fs.insert_tree(
+            path!("/project"),
+            json!({
+                ".git": {},
+                "foo.txt": "
+                    one
+                    two
+                    three
+                    four
+                    five
+                    six
+                    seven
+                    eight
+                    nine
+                    ten
+                    ELEVEN
+                    twelve
+                ".unindent()
+            }),
+        )
+        .await;
+        let project = Project::test(fs.clone(), [path!("/project").as_ref()], cx).await;
+        let (workspace, cx) =
+            cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
+        let diff = cx.new_window_entity(|window, cx| {
+            ProjectDiff::new(project.clone(), workspace, window, cx)
+        });
+        cx.run_until_parked();
+
+        fs.set_head_and_index_for_repo(
+            Path::new(path!("/project/.git")),
+            &[(
+                "foo.txt",
+                "
+                    one
+                    two
+                    three
+                    four
+                    five
+                    six
+                    seven
+                    eight
+                    nine
+                    ten
+                    eleven
+                    twelve
+                "
+                .unindent(),
+            )],
+        );
+        cx.run_until_parked();
+
+        let editor = diff.read_with(cx, |diff, _| diff.editor.clone());
+        assert_state_with_diff(
+            &editor,
+            cx,
+            &"
+                  ˇnine
+                  ten
+                - eleven
+                + ELEVEN
+                  twelve
+            "
+            .unindent(),
+        );
+
+        let buffer = project
+            .update(cx, |project, cx| {
+                project.open_local_buffer(path!("/project/foo.txt"), cx)
+            })
+            .await
+            .unwrap();
+        buffer.update(cx, |buffer, cx| {
+            buffer.edit_via_marked_text(
+                &"
+                    one
+                    «TWO»
+                    three
+                    four
+                    five
+                    six
+                    seven
+                    eight
+                    nine
+                    ten
+                    ELEVEN
+                    twelve
+                "
+                .unindent(),
+                None,
+                cx,
+            );
+        });
+        project
+            .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx))
+            .await
+            .unwrap();
+        cx.run_until_parked();
+
+        assert_state_with_diff(
+            &editor,
+            cx,
+            &"
+                  one
+                - two
+                + TWO
+                  three
+                  four
+                  five
+                  ˇnine
+                  ten
+                - eleven
+                + ELEVEN
+                  twelve
+            "
+            .unindent(),
+        );
+    }
 }

crates/project/src/git_store.rs 🔗

@@ -302,6 +302,7 @@ pub enum RepositoryState {
 pub enum RepositoryEvent {
     Updated { full_scan: bool, new_instance: bool },
     MergeHeadsChanged,
+    PathsChanged,
 }
 
 #[derive(Clone, Debug)]
@@ -4840,19 +4841,20 @@ impl Repository {
                     }
 
                     if needs_update {
-                        if let Some(updates_tx) = updates_tx {
-                            updates_tx
-                                .unbounded_send(DownstreamUpdate::UpdateRepository(
-                                    this.snapshot.clone(),
-                                ))
-                                .ok();
-                        }
-
                         cx.emit(RepositoryEvent::Updated {
                             full_scan: false,
                             new_instance: false,
                         });
                     }
+
+                    if let Some(updates_tx) = updates_tx {
+                        updates_tx
+                            .unbounded_send(DownstreamUpdate::UpdateRepository(
+                                this.snapshot.clone(),
+                            ))
+                            .ok();
+                    }
+                    cx.emit(RepositoryEvent::PathsChanged);
                 })
             },
         );

crates/project/src/project_tests.rs 🔗

@@ -8986,9 +8986,14 @@ async fn test_ignored_dirs_events(cx: &mut gpui::TestAppContext) {
     });
 
     assert_eq!(
-        repository_updates.lock().as_slice(),
+        repository_updates
+            .lock()
+            .iter()
+            .filter(|update| !matches!(update, RepositoryEvent::PathsChanged))
+            .cloned()
+            .collect::<Vec<_>>(),
         Vec::new(),
-        "No further repo events should happen, as only ignored dirs' contents was changed",
+        "No further RepositoryUpdated events should happen, as only ignored dirs' contents was changed",
     );
     assert_eq!(
         project_events.lock().as_slice(),
@@ -9088,7 +9093,11 @@ async fn test_odd_events_for_ignored_dirs(
     });
 
     assert_eq!(
-        repository_updates.lock().drain(..).collect::<Vec<_>>(),
+        repository_updates
+            .lock()
+            .drain(..)
+            .filter(|update| !matches!(update, RepositoryEvent::PathsChanged))
+            .collect::<Vec<_>>(),
         vec![
             RepositoryEvent::Updated {
                 full_scan: true,
@@ -9119,9 +9128,14 @@ async fn test_odd_events_for_ignored_dirs(
     cx.executor().run_until_parked();
 
     assert_eq!(
-        repository_updates.lock().as_slice(),
+        repository_updates
+            .lock()
+            .iter()
+            .filter(|update| !matches!(update, RepositoryEvent::PathsChanged))
+            .cloned()
+            .collect::<Vec<_>>(),
         Vec::new(),
-        "No further repo events should happen, as only ignored dirs received FS events",
+        "No further RepositoryUpdated events should happen, as only ignored dirs received FS events",
     );
     assert_eq!(
         project_events.lock().as_slice(),

crates/project_panel/src/project_panel.rs 🔗

@@ -30,7 +30,7 @@ use menu::{Confirm, SelectFirst, SelectLast, SelectNext, SelectPrevious};
 use project::{
     Entry, EntryKind, Fs, GitEntry, GitEntryRef, GitTraversal, Project, ProjectEntryId,
     ProjectPath, Worktree, WorktreeId,
-    git_store::{GitStoreEvent, git_traversal::ChildEntriesGitIter},
+    git_store::{GitStoreEvent, RepositoryEvent, git_traversal::ChildEntriesGitIter},
     project_settings::GoToDiagnosticSeverityFilter,
 };
 use project_panel_settings::ProjectPanelSettings;
@@ -500,7 +500,7 @@ impl ProjectPanel {
                 &git_store,
                 window,
                 |this, _, event, window, cx| match event {
-                    GitStoreEvent::RepositoryUpdated(_, _, _)
+                    GitStoreEvent::RepositoryUpdated(_, RepositoryEvent::Updated { .. }, _)
                     | GitStoreEvent::RepositoryAdded(_)
                     | GitStoreEvent::RepositoryRemoved(_) => {
                         this.update_visible_entries(None, false, false, window, cx);