Fix the log spam from the BlameBuffer request (#19921)

Mikayla Maki created 1 year ago

Release Notes:

- N/A

Change summary

crates/editor/src/git/blame.rs     | 20 ++++++++-------
crates/project/src/buffer_store.rs | 40 +++++++++++++++++++++----------
crates/project/src/project.rs      | 13 ---------
crates/proto/proto/zed.proto       | 14 ++++++++---
4 files changed, 49 insertions(+), 38 deletions(-)

Detailed changes

crates/editor/src/git/blame.rs

@@ -368,12 +368,15 @@ impl GitBlame {
                 .spawn({
                     let snapshot = snapshot.clone();
                     async move {
-                        let Blame {
+                        let Some(Blame {
                             entries,
                             permalinks,
                             messages,
                             remote_url,
-                        } = blame.await?;
+                        }) = blame.await?
+                        else {
+                            return Ok(None);
+                        };
 
                         let entries = build_blame_entry_sum_tree(entries, snapshot.max_point().row);
                         let commit_details = parse_commit_messages(
@@ -385,13 +388,16 @@ impl GitBlame {
                         )
                         .await;
 
-                        anyhow::Ok((entries, commit_details))
+                        anyhow::Ok(Some((entries, commit_details)))
                     }
                 })
                 .await;
 
             this.update(&mut cx, |this, cx| match result {
-                Ok((entries, commit_details)) => {
+                Ok(None) => {
+                    // Nothing to do, e.g. no repository found
+                }
+                Ok(Some((entries, commit_details))) => {
                     this.buffer_edits = buffer_edits;
                     this.buffer_snapshot = snapshot;
                     this.entries = entries;
@@ -410,11 +416,7 @@ impl GitBlame {
                     } else {
                         // If we weren't triggered by a user, we just log errors in the background, instead of sending
                         // notifications.
-                        // Except for `NoRepositoryError`, which can  happen often if a user has inline-blame turned on
-                        // and opens a non-git file.
-                        if error.downcast_ref::<project::NoRepositoryError>().is_none() {
-                            log::error!("failed to get git blame data: {error:?}");
-                        }
+                        log::error!("failed to get git blame data: {error:?}");
                     }
                 }),
             })

crates/project/src/buffer_store.rs

@@ -1,7 +1,7 @@
 use crate::{
     search::SearchQuery,
     worktree_store::{WorktreeStore, WorktreeStoreEvent},
-    Item, NoRepositoryError, ProjectPath,
+    Item, ProjectPath,
 };
 use ::git::{parse_git_remote_url, BuildPermalinkParams, GitHostingProviderRegistry};
 use anyhow::{anyhow, Context as _, Result};
@@ -1118,7 +1118,7 @@ impl BufferStore {
         buffer: &Model<Buffer>,
         version: Option<clock::Global>,
         cx: &AppContext,
-    ) -> Task<Result<Blame>> {
+    ) -> Task<Result<Option<Blame>>> {
         let buffer = buffer.read(cx);
         let Some(file) = File::from_dyn(buffer.file()) else {
             return Task::ready(Err(anyhow!("buffer has no file")));
@@ -1130,7 +1130,7 @@ impl BufferStore {
                 let blame_params = maybe!({
                     let (repo_entry, local_repo_entry) = match worktree.repo_for_path(&file.path) {
                         Some(repo_for_path) => repo_for_path,
-                        None => anyhow::bail!(NoRepositoryError {}),
+                        None => return Ok(None),
                     };
 
                     let relative_path = repo_entry
@@ -1144,13 +1144,16 @@ impl BufferStore {
                         None => buffer.as_rope().clone(),
                     };
 
-                    anyhow::Ok((repo, relative_path, content))
+                    anyhow::Ok(Some((repo, relative_path, content)))
                 });
 
                 cx.background_executor().spawn(async move {
-                    let (repo, relative_path, content) = blame_params?;
+                    let Some((repo, relative_path, content)) = blame_params? else {
+                        return Ok(None);
+                    };
                     repo.blame(&relative_path, content)
                         .with_context(|| format!("Failed to blame {:?}", relative_path.0))
+                        .map(Some)
                 })
             }
             Worktree::Remote(worktree) => {
@@ -2112,7 +2115,13 @@ fn is_not_found_error(error: &anyhow::Error) -> bool {
         .is_some_and(|err| err.kind() == io::ErrorKind::NotFound)
 }
 
-fn serialize_blame_buffer_response(blame: git::blame::Blame) -> proto::BlameBufferResponse {
+fn serialize_blame_buffer_response(blame: Option<git::blame::Blame>) -> proto::BlameBufferResponse {
+    let Some(blame) = blame else {
+        return proto::BlameBufferResponse {
+            blame_response: None,
+        };
+    };
+
     let entries = blame
         .entries
         .into_iter()
@@ -2154,14 +2163,19 @@ fn serialize_blame_buffer_response(blame: git::blame::Blame) -> proto::BlameBuff
         .collect::<Vec<_>>();
 
     proto::BlameBufferResponse {
-        entries,
-        messages,
-        permalinks,
-        remote_url: blame.remote_url,
+        blame_response: Some(proto::blame_buffer_response::BlameResponse {
+            entries,
+            messages,
+            permalinks,
+            remote_url: blame.remote_url,
+        }),
     }
 }
 
-fn deserialize_blame_buffer_response(response: proto::BlameBufferResponse) -> git::blame::Blame {
+fn deserialize_blame_buffer_response(
+    response: proto::BlameBufferResponse,
+) -> Option<git::blame::Blame> {
+    let response = response.blame_response?;
     let entries = response
         .entries
         .into_iter()
@@ -2202,10 +2216,10 @@ fn deserialize_blame_buffer_response(response: proto::BlameBufferResponse) -> gi
         })
         .collect::<HashMap<_, _>>();
 
-    Blame {
+    Some(Blame {
         entries,
         permalinks,
         messages,
         remote_url: response.remote_url,
-    }
+    })
 }

crates/project/src/project.rs

@@ -3420,7 +3420,7 @@ impl Project {
         buffer: &Model<Buffer>,
         version: Option<clock::Global>,
         cx: &AppContext,
-    ) -> Task<Result<Blame>> {
+    ) -> Task<Result<Option<Blame>>> {
         self.buffer_store.read(cx).blame_buffer(buffer, version, cx)
     }
 
@@ -4273,17 +4273,6 @@ impl Completion {
     }
 }
 
-#[derive(Debug)]
-pub struct NoRepositoryError {}
-
-impl std::fmt::Display for NoRepositoryError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "no git repository for worktree found")
-    }
-}
-
-impl std::error::Error for NoRepositoryError {}
-
 pub fn sort_worktree_entries(entries: &mut [Entry]) {
     entries.sort_by(|entry_a, entry_b| {
         compare_paths(

crates/proto/proto/zed.proto

@@ -2117,10 +2117,16 @@ message CommitPermalink {
 }
 
 message BlameBufferResponse {
-    repeated BlameEntry entries = 1;
-    repeated CommitMessage messages = 2;
-    repeated CommitPermalink permalinks = 3;
-    optional string remote_url = 4;
+    message BlameResponse {
+        repeated BlameEntry entries = 1;
+        repeated CommitMessage messages = 2;
+        repeated CommitPermalink permalinks = 3;
+        optional string remote_url = 4;
+    }
+
+    optional BlameResponse blame_response = 5;
+
+    reserved 1 to 4;
 }
 
 message MultiLspQuery {