Fix file git status refresh on .gitignore update (#9466)

Andrew Lygin created

This PR fixes file name coloring in the project panel and tabs when
.gitignore file is updated. It's intended to fix #7831.

There's another, less vivid, problem with git-aware labels coloring.
It's about files that are both ignored and contained in the git index.
I'll file a separate issue for it to keep this fix focused.

Release Notes:

- Fixed file Git status refreshing on .gitignore update (#7831).

Change summary

crates/editor/src/items.rs                |  46 ++++++---
crates/project_panel/src/project_panel.rs |  23 ----
crates/worktree/src/worktree.rs           |  12 ++
crates/worktree/src/worktree_tests.rs     | 116 ++++++++++++++++++------
4 files changed, 132 insertions(+), 65 deletions(-)

Detailed changes

crates/editor/src/items.rs 🔗

@@ -595,27 +595,18 @@ impl Item for Editor {
     }
 
     fn tab_content(&self, detail: Option<usize>, selected: bool, cx: &WindowContext) -> AnyElement {
-        let git_status = if ItemSettings::get_global(cx).git_status {
+        let label_color = if ItemSettings::get_global(cx).git_status {
             self.buffer()
                 .read(cx)
                 .as_singleton()
                 .and_then(|buffer| buffer.read(cx).project_path(cx))
                 .and_then(|path| self.project.as_ref()?.read(cx).entry_for_path(&path, cx))
-                .and_then(|entry| entry.git_status())
+                .map(|entry| {
+                    entry_git_aware_label_color(entry.git_status, entry.is_ignored, selected)
+                })
+                .unwrap_or_else(|| entry_label_color(selected))
         } else {
-            None
-        };
-        let label_color = match git_status {
-            Some(GitFileStatus::Added) => Color::Created,
-            Some(GitFileStatus::Modified) => Color::Modified,
-            Some(GitFileStatus::Conflict) => Color::Conflict,
-            None => {
-                if selected {
-                    Color::Default
-                } else {
-                    Color::Muted
-                }
-            }
+            entry_label_color(selected)
         };
 
         let description = detail.and_then(|detail| {
@@ -1195,6 +1186,31 @@ pub fn active_match_index(
     }
 }
 
+pub fn entry_label_color(selected: bool) -> Color {
+    if selected {
+        Color::Default
+    } else {
+        Color::Muted
+    }
+}
+
+pub fn entry_git_aware_label_color(
+    git_status: Option<GitFileStatus>,
+    ignored: bool,
+    selected: bool,
+) -> Color {
+    if ignored {
+        Color::Disabled
+    } else {
+        match git_status {
+            Some(GitFileStatus::Added) => Color::Created,
+            Some(GitFileStatus::Modified) => Color::Modified,
+            Some(GitFileStatus::Conflict) => Color::Conflict,
+            None => entry_label_color(selected),
+        }
+    }
+}
+
 fn path_for_buffer<'a>(
     buffer: &Model<MultiBuffer>,
     height: usize,

crates/project_panel/src/project_panel.rs 🔗

@@ -4,7 +4,7 @@ use client::{ErrorCode, ErrorExt};
 use settings::Settings;
 
 use db::kvp::KEY_VALUE_STORE;
-use editor::{actions::Cancel, scroll::Autoscroll, Editor};
+use editor::{actions::Cancel, items::entry_git_aware_label_color, scroll::Autoscroll, Editor};
 use file_associations::FileAssociations;
 
 use anyhow::{anyhow, Result};
@@ -1180,7 +1180,7 @@ impl ProjectPanel {
                         inode: 0,
                         mtime: entry.mtime,
                         is_symlink: false,
-                        is_ignored: false,
+                        is_ignored: entry.is_ignored,
                         is_external: false,
                         is_private: false,
                         git_status: entry.git_status,
@@ -1408,23 +1408,8 @@ impl ProjectPanel {
             .selection
             .map_or(false, |selection| selection.entry_id == entry_id);
         let width = self.size(cx);
-
-        let filename_text_color = details
-            .git_status
-            .as_ref()
-            .map(|status| match status {
-                GitFileStatus::Added => Color::Created,
-                GitFileStatus::Modified => Color::Modified,
-                GitFileStatus::Conflict => Color::Conflict,
-            })
-            .unwrap_or(if is_selected {
-                Color::Default
-            } else if details.is_ignored {
-                Color::Disabled
-            } else {
-                Color::Muted
-            });
-
+        let filename_text_color =
+            entry_git_aware_label_color(details.git_status, details.is_ignored, is_selected);
         let file_name = details.filename.clone();
         let icon = details.icon.clone();
         let depth = details.depth;

crates/worktree/src/worktree.rs 🔗

@@ -4225,6 +4225,9 @@ impl BackgroundScanner {
         let mut entries_by_id_edits = Vec::new();
         let mut entries_by_path_edits = Vec::new();
         let path = job.abs_path.strip_prefix(&snapshot.abs_path).unwrap();
+        let repo = snapshot
+            .local_repo_for_path(path)
+            .map_or(None, |local_repo| Some(local_repo.1));
         for mut entry in snapshot.child_entries(path).cloned() {
             let was_ignored = entry.is_ignored;
             let abs_path: Arc<Path> = snapshot.abs_path().join(&entry.path).into();
@@ -4259,6 +4262,15 @@ impl BackgroundScanner {
                 let mut path_entry = snapshot.entries_by_id.get(&entry.id, &()).unwrap().clone();
                 path_entry.scan_id = snapshot.scan_id;
                 path_entry.is_ignored = entry.is_ignored;
+                if !entry.is_dir() && !entry.is_ignored && !entry.is_external {
+                    if let Some(repo) = repo {
+                        if let Some(mtime) = &entry.mtime {
+                            let repo_path = RepoPath(entry.path.to_path_buf());
+                            let repo = repo.repo_ptr.lock();
+                            entry.git_status = repo.status(&repo_path, *mtime);
+                        }
+                    }
+                }
                 entries_by_id_edits.push(Edit::Insert(path_entry));
                 entries_by_path_edits.push(Edit::Insert(entry));
             }

crates/worktree/src/worktree_tests.rs 🔗

@@ -851,24 +851,16 @@ async fn test_rescan_with_gitignore(cx: &mut TestAppContext) {
 
     cx.read(|cx| {
         let tree = tree.read(cx);
-        assert!(
-            !tree
-                .entry_for_path("tracked-dir/tracked-file1")
-                .unwrap()
-                .is_ignored
-        );
-        assert!(
-            tree.entry_for_path("tracked-dir/ancestor-ignored-file1")
-                .unwrap()
-                .is_ignored
-        );
-        assert!(
-            tree.entry_for_path("ignored-dir/ignored-file1")
-                .unwrap()
-                .is_ignored
-        );
+        assert_entry_git_state(tree, "tracked-dir/tracked-file1", None, false);
+        assert_entry_git_state(tree, "tracked-dir/ancestor-ignored-file1", None, true);
+        assert_entry_git_state(tree, "ignored-dir/ignored-file1", None, true);
     });
 
+    fs.set_status_for_repo_via_working_copy_change(
+        &Path::new("/root/tree/.git"),
+        &[(Path::new("tracked-dir/tracked-file2"), GitFileStatus::Added)],
+    );
+
     fs.create_file(
         "/root/tree/tracked-dir/tracked-file2".as_ref(),
         Default::default(),
@@ -891,26 +883,77 @@ async fn test_rescan_with_gitignore(cx: &mut TestAppContext) {
     cx.executor().run_until_parked();
     cx.read(|cx| {
         let tree = tree.read(cx);
-        assert!(
-            !tree
-                .entry_for_path("tracked-dir/tracked-file2")
-                .unwrap()
-                .is_ignored
-        );
-        assert!(
-            tree.entry_for_path("tracked-dir/ancestor-ignored-file2")
-                .unwrap()
-                .is_ignored
-        );
-        assert!(
-            tree.entry_for_path("ignored-dir/ignored-file2")
-                .unwrap()
-                .is_ignored
+        assert_entry_git_state(
+            tree,
+            "tracked-dir/tracked-file2",
+            Some(GitFileStatus::Added),
+            false,
         );
+        assert_entry_git_state(tree, "tracked-dir/ancestor-ignored-file2", None, true);
+        assert_entry_git_state(tree, "ignored-dir/ignored-file2", None, true);
         assert!(tree.entry_for_path(".git").unwrap().is_ignored);
     });
 }
 
+#[gpui::test]
+async fn test_update_gitignore(cx: &mut TestAppContext) {
+    init_test(cx);
+    let fs = FakeFs::new(cx.background_executor.clone());
+    fs.insert_tree(
+        "/root",
+        json!({
+            ".git": {},
+            ".gitignore": "*.txt\n",
+            "a.xml": "<a></a>",
+            "b.txt": "Some text"
+        }),
+    )
+    .await;
+
+    let tree = Worktree::local(
+        build_client(cx),
+        "/root".as_ref(),
+        true,
+        fs.clone(),
+        Default::default(),
+        &mut cx.to_async(),
+    )
+    .await
+    .unwrap();
+    cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete())
+        .await;
+
+    tree.read_with(cx, |tree, _| {
+        tree.as_local()
+            .unwrap()
+            .refresh_entries_for_paths(vec![Path::new("").into()])
+    })
+    .recv()
+    .await;
+
+    cx.read(|cx| {
+        let tree = tree.read(cx);
+        assert_entry_git_state(tree, "a.xml", None, false);
+        assert_entry_git_state(tree, "b.txt", None, true);
+    });
+
+    fs.atomic_write("/root/.gitignore".into(), "*.xml".into())
+        .await
+        .unwrap();
+
+    fs.set_status_for_repo_via_working_copy_change(
+        &Path::new("/root/.git"),
+        &[(Path::new("b.txt"), GitFileStatus::Added)],
+    );
+
+    cx.executor().run_until_parked();
+    cx.read(|cx| {
+        let tree = tree.read(cx);
+        assert_entry_git_state(tree, "a.xml", None, true);
+        assert_entry_git_state(tree, "b.txt", Some(GitFileStatus::Added), false);
+    });
+}
+
 #[gpui::test]
 async fn test_write_file(cx: &mut TestAppContext) {
     init_test(cx);
@@ -2580,3 +2623,14 @@ fn init_test(cx: &mut gpui::TestAppContext) {
         WorktreeSettings::register(cx);
     });
 }
+
+fn assert_entry_git_state(
+    tree: &Worktree,
+    path: &str,
+    git_status: Option<GitFileStatus>,
+    is_ignored: bool,
+) {
+    let entry = tree.entry_for_path(path).expect("entry {path} not found");
+    assert_eq!(entry.git_status, git_status);
+    assert_eq!(entry.is_ignored, is_ignored);
+}