From 29a50573a944dec9a83f7f0feb42c8678918af46 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mehmet=20Efe=20Ak=C3=A7a?=
 <13402668+mmtftr@users.noreply.github.com>
Date: Fri, 12 Apr 2024 08:20:34 +0300
Subject: [PATCH] Add git blame error reporting with notification (#10408)

<img width="1035" alt="Screenshot 2024-04-11 at 13 13 44"
src="https://github.com/zed-industries/zed/assets/13402668/cd0e96a0-41c6-4757-8840-97d15a75c511">

Release Notes:

- Added a notification to show possible `git blame` errors if it fails to run.

Caveats:
- ~git blame now executes in foreground
executor  (required since the Fut is !Send)~

TODOs:
- After a failed toggle, the app thinks the blame
is shown. This means toggling again will do nothing
instead of retrying. (Caused by editor.show_git_blame
being set to true before the git blame is generated)
- ~(Maybe) Trim error?~ Done

---------

Co-authored-by: Thorsten Ball <mrnugget@gmail.com>
---
 crates/editor/src/git/blame.rs | 75 +++++++++++++++++++++++++++++-----
 crates/git/src/blame.rs        |  1 +
 crates/project/src/project.rs  |  1 +
 3 files changed, 67 insertions(+), 10 deletions(-)

diff --git a/crates/editor/src/git/blame.rs b/crates/editor/src/git/blame.rs
index b0c25d73c4..8f753f7186 100644
--- a/crates/editor/src/git/blame.rs
+++ b/crates/editor/src/git/blame.rs
@@ -256,7 +256,7 @@ impl GitBlame {
         let blame = self.project.read(cx).blame_buffer(&self.buffer, None, cx);
 
         self.task = cx.spawn(|this, mut cx| async move {
-            let (entries, permalinks, messages) = cx
+            let result = cx
                 .background_executor()
                 .spawn({
                     let snapshot = snapshot.clone();
@@ -304,16 +304,23 @@ impl GitBlame {
                         anyhow::Ok((entries, permalinks, messages))
                     }
                 })
-                .await?;
+                .await;
 
-            this.update(&mut cx, |this, cx| {
-                this.buffer_edits = buffer_edits;
-                this.buffer_snapshot = snapshot;
-                this.entries = entries;
-                this.permalinks = permalinks;
-                this.messages = messages;
-                this.generated = true;
-                cx.notify();
+            this.update(&mut cx, |this, cx| match result {
+                Ok((entries, permalinks, messages)) => {
+                    this.buffer_edits = buffer_edits;
+                    this.buffer_snapshot = snapshot;
+                    this.entries = entries;
+                    this.permalinks = permalinks;
+                    this.messages = messages;
+                    this.generated = true;
+                    cx.notify();
+                }
+                Err(error) => this.project.update(cx, |_, cx| {
+                    log::error!("failed to get git blame data: {error:?}");
+                    let notification = format!("{:#}", error).trim().to_string();
+                    cx.emit(project::Event::Notification(notification));
+                }),
             })
         });
     }
@@ -359,6 +366,54 @@ mod tests {
         });
     }
 
+    #[gpui::test]
+    async fn test_blame_error_notifications(cx: &mut gpui::TestAppContext) {
+        init_test(cx);
+
+        let fs = FakeFs::new(cx.executor());
+        fs.insert_tree(
+            "/my-repo",
+            json!({
+                ".git": {},
+                "file.txt": r#"
+                    irrelevant contents
+                "#
+                .unindent()
+            }),
+        )
+        .await;
+
+        // Creating a GitBlame without a corresponding blame state
+        // will result in an error.
+
+        let project = Project::test(fs, ["/my-repo".as_ref()], cx).await;
+        let buffer = project
+            .update(cx, |project, cx| {
+                project.open_local_buffer("/my-repo/file.txt", cx)
+            })
+            .await
+            .unwrap();
+
+        let blame = cx.new_model(|cx| GitBlame::new(buffer.clone(), project.clone(), cx));
+
+        let event = project.next_event(cx);
+        assert_eq!(
+            event,
+            project::Event::Notification(
+                "Failed to blame \"file.txt\": failed to get blame for \"file.txt\"".to_string()
+            )
+        );
+
+        blame.update(cx, |blame, cx| {
+            assert_eq!(
+                blame
+                    .blame_for_rows((0..1).map(Some), cx)
+                    .collect::<Vec<_>>(),
+                vec![None]
+            );
+        });
+    }
+
     #[gpui::test]
     async fn test_blame_for_rows(cx: &mut gpui::TestAppContext) {
         init_test(cx);
diff --git a/crates/git/src/blame.rs b/crates/git/src/blame.rs
index 45b2e0c7ef..54ff7f664a 100644
--- a/crates/git/src/blame.rs
+++ b/crates/git/src/blame.rs
@@ -78,6 +78,7 @@ fn run_git_blame(
         .arg(path.as_os_str())
         .stdin(Stdio::piped())
         .stdout(Stdio::piped())
+        .stderr(Stdio::piped())
         .spawn()
         .map_err(|e| anyhow!("Failed to start git blame process: {}", e))?;
 
diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs
index 5e0aed2757..0571666cb7 100644
--- a/crates/project/src/project.rs
+++ b/crates/project/src/project.rs
@@ -7734,6 +7734,7 @@ impl Project {
                 let (repo, relative_path, content) = blame_params?;
                 let lock = repo.lock();
                 lock.blame(&relative_path, content)
+                    .with_context(|| format!("Failed to blame {relative_path:?}"))
             })
         } else {
             let project_id = self.remote_id();