From 8e251e3507d54d1bb21f4b70a2b2b3875f6ec555 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Tue, 29 Oct 2024 13:02:21 -0700 Subject: [PATCH] Fix the log spam from the BlameBuffer request (#19921) Release Notes: - N/A --- 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(-) diff --git a/crates/editor/src/git/blame.rs b/crates/editor/src/git/blame.rs index 1ac134530532c0052652d5067963371360552593..9dfc379ae70edace4000fa18c9de464671518bef 100644 --- a/crates/editor/src/git/blame.rs +++ b/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::().is_none() { - log::error!("failed to get git blame data: {error:?}"); - } + log::error!("failed to get git blame data: {error:?}"); } }), }) diff --git a/crates/project/src/buffer_store.rs b/crates/project/src/buffer_store.rs index 8948ed6ee781f5e78bd8aeeca481ef72ec3d02c0..5d8fb3ab391d1d38e51096857b831e563c305598 100644 --- a/crates/project/src/buffer_store.rs +++ b/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, version: Option, cx: &AppContext, - ) -> Task> { + ) -> Task>> { 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) -> 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::>(); 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 { + 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::>(); - Blame { + Some(Blame { entries, permalinks, messages, remote_url: response.remote_url, - } + }) } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index f5a295a3a3fb6d214dd06351b30ab01066d7e18b..788de669963fc4885d756339d434224cdfeeb4f9 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -3420,7 +3420,7 @@ impl Project { buffer: &Model, version: Option, cx: &AppContext, - ) -> Task> { + ) -> Task>> { 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( diff --git a/crates/proto/proto/zed.proto b/crates/proto/proto/zed.proto index d78795eed935f1feb9f17fccd4b9d7fe009a5a89..439531ccb317ea52c46d918a05194331f210faaa 100644 --- a/crates/proto/proto/zed.proto +++ b/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 {