git: Unify commit popups (#38749)

LoricAndre , Cole Miller , and Zed Zippy created

Closes #26424
Supersedes #35328

Originally, `git::blame` uses its own `ParsedCommitMessage` as the
source for the commit information, including the PR section. This
changes unifies this with `git::repository` and `git_ui::git_panel` by
moving this and some other commit-related structs to `git::commit`
instead, and making both `git_ui::blame_ui` and `git_ui::git_panel` pull
their information from these structs.

Release notes :

- (Let's Git Together) Fixed the commit tooltip in the git panel not
showing information like avatars.

---------

Co-authored-by: Cole Miller <cole@zed.dev>
Co-authored-by: Zed Zippy <234243425+zed-zippy[bot]@users.noreply.github.com>

Change summary

crates/editor/src/editor.rs         |  6 -
crates/editor/src/element.rs        |  6 -
crates/editor/src/git/blame.rs      | 82 ++++++------------------------
crates/git/src/blame.rs             | 11 ---
crates/git/src/commit.rs            | 49 +++++++++++++++++
crates/git_ui/src/blame_ui.rs       |  5 -
crates/git_ui/src/commit_tooltip.rs |  2 
crates/git_ui/src/git_panel.rs      | 20 ++++--
crates/project/src/git_store.rs     |  5 +
9 files changed, 87 insertions(+), 99 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -73,11 +73,7 @@ pub use multi_buffer::{
 pub use split::SplittableEditor;
 pub use text::Bias;
 
-use ::git::{
-    Restore,
-    blame::{BlameEntry, ParsedCommitMessage},
-    status::FileStatus,
-};
+use ::git::{Restore, blame::BlameEntry, commit::ParsedCommitMessage, status::FileStatus};
 use aho_corasick::{AhoCorasick, AhoCorasickBuilder, BuildError};
 use anyhow::{Context as _, Result, anyhow, bail};
 use blink_manager::BlinkManager;

crates/editor/src/element.rs 🔗

@@ -37,11 +37,7 @@ use crate::{
 use buffer_diff::{DiffHunkStatus, DiffHunkStatusKind};
 use collections::{BTreeMap, HashMap};
 use file_icons::FileIcons;
-use git::{
-    Oid,
-    blame::{BlameEntry, ParsedCommitMessage},
-    status::FileStatus,
-};
+use git::{Oid, blame::BlameEntry, commit::ParsedCommitMessage, status::FileStatus};
 use gpui::{
     Action, Along, AnyElement, App, AppContext, AvailableSpace, Axis as ScrollbarAxis, BorderStyle,
     Bounds, ClickEvent, ClipboardItem, ContentMask, Context, Corner, Corners, CursorStyle,

crates/editor/src/git/blame.rs 🔗

@@ -3,9 +3,9 @@ use anyhow::{Context as _, Result};
 use collections::HashMap;
 
 use git::{
-    GitHostingProviderRegistry, GitRemote, Oid,
-    blame::{Blame, BlameEntry, ParsedCommitMessage},
-    parse_git_remote_url,
+    GitHostingProviderRegistry, Oid,
+    blame::{Blame, BlameEntry},
+    commit::ParsedCommitMessage,
 };
 use gpui::{
     AnyElement, App, AppContext as _, Context, Entity, Hsla, ScrollHandle, Subscription, Task,
@@ -525,12 +525,7 @@ impl GitBlame {
                                 .git_store()
                                 .read(cx)
                                 .repository_and_path_for_buffer_id(buffer.read(cx).remote_id(), cx)
-                                .and_then(|(repo, _)| {
-                                    repo.read(cx)
-                                        .remote_upstream_url
-                                        .clone()
-                                        .or(repo.read(cx).remote_origin_url.clone())
-                                });
+                                .and_then(|(repo, _)| repo.read(cx).default_remote_url());
                             let blame_buffer = project
                                 .update(cx, |project, cx| project.blame_buffer(&buffer, None, cx));
                             Ok(async move {
@@ -554,13 +549,19 @@ impl GitBlame {
                                             entries,
                                             snapshot.max_point().row,
                                         );
-                                        let commit_details = parse_commit_messages(
-                                            messages,
-                                            remote_url,
-                                            provider_registry.clone(),
-                                        )
-                                        .await;
-
+                                        let commit_details = messages
+                                            .into_iter()
+                                            .map(|(oid, message)| {
+                                                let parsed_commit_message =
+                                                    ParsedCommitMessage::parse(
+                                                        oid.to_string(),
+                                                        message,
+                                                        remote_url.as_deref(),
+                                                        Some(provider_registry.clone()),
+                                                    );
+                                                (oid, parsed_commit_message)
+                                            })
+                                            .collect();
                                         res.push((
                                             id,
                                             snapshot,
@@ -680,55 +681,6 @@ fn build_blame_entry_sum_tree(entries: Vec<BlameEntry>, max_row: u32) -> SumTree
     entries
 }
 
-async fn parse_commit_messages(
-    messages: impl IntoIterator<Item = (Oid, String)>,
-    remote_url: Option<String>,
-    provider_registry: Arc<GitHostingProviderRegistry>,
-) -> HashMap<Oid, ParsedCommitMessage> {
-    let mut commit_details = HashMap::default();
-
-    let parsed_remote_url = remote_url
-        .as_deref()
-        .and_then(|remote_url| parse_git_remote_url(provider_registry, remote_url));
-
-    for (oid, message) in messages {
-        let permalink = if let Some((provider, git_remote)) = parsed_remote_url.as_ref() {
-            Some(provider.build_commit_permalink(
-                git_remote,
-                git::BuildCommitPermalinkParams {
-                    sha: oid.to_string().as_str(),
-                },
-            ))
-        } else {
-            None
-        };
-
-        let remote = parsed_remote_url
-            .as_ref()
-            .map(|(provider, remote)| GitRemote {
-                host: provider.clone(),
-                owner: remote.owner.clone().into(),
-                repo: remote.repo.clone().into(),
-            });
-
-        let pull_request = parsed_remote_url
-            .as_ref()
-            .and_then(|(provider, remote)| provider.extract_pull_request(remote, &message));
-
-        commit_details.insert(
-            oid,
-            ParsedCommitMessage {
-                message: message.into(),
-                permalink,
-                remote,
-                pull_request,
-            },
-        );
-    }
-
-    commit_details
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

crates/git/src/blame.rs 🔗

@@ -1,10 +1,9 @@
+use crate::Oid;
 use crate::commit::get_messages;
 use crate::repository::RepoPath;
-use crate::{GitRemote, Oid};
 use anyhow::{Context as _, Result};
 use collections::{HashMap, HashSet};
 use futures::AsyncWriteExt;
-use gpui::SharedString;
 use serde::{Deserialize, Serialize};
 use std::process::Stdio;
 use std::{ops::Range, path::Path};
@@ -21,14 +20,6 @@ pub struct Blame {
     pub messages: HashMap<Oid, String>,
 }
 
-#[derive(Clone, Debug, Default)]
-pub struct ParsedCommitMessage {
-    pub message: SharedString,
-    pub permalink: Option<url::Url>,
-    pub pull_request: Option<crate::hosting_provider::PullRequest>,
-    pub remote: Option<GitRemote>,
-}
-
 impl Blame {
     pub async fn for_path(
         git_binary: &Path,

crates/git/src/commit.rs 🔗

@@ -1,7 +1,52 @@
-use crate::{Oid, status::StatusCode};
+use crate::{
+    BuildCommitPermalinkParams, GitHostingProviderRegistry, GitRemote, Oid, parse_git_remote_url,
+    status::StatusCode,
+};
 use anyhow::{Context as _, Result};
 use collections::HashMap;
-use std::path::Path;
+use gpui::SharedString;
+use std::{path::Path, sync::Arc};
+
+#[derive(Clone, Debug, Default)]
+pub struct ParsedCommitMessage {
+    pub message: SharedString,
+    pub permalink: Option<url::Url>,
+    pub pull_request: Option<crate::hosting_provider::PullRequest>,
+    pub remote: Option<GitRemote>,
+}
+
+impl ParsedCommitMessage {
+    pub fn parse(
+        sha: String,
+        message: String,
+        remote_url: Option<&str>,
+        provider_registry: Option<Arc<GitHostingProviderRegistry>>,
+    ) -> Self {
+        if let Some((hosting_provider, remote)) = provider_registry
+            .and_then(|reg| remote_url.and_then(|url| parse_git_remote_url(reg, url)))
+        {
+            let pull_request = hosting_provider.extract_pull_request(&remote, &message);
+            Self {
+                message: message.into(),
+                permalink: Some(
+                    hosting_provider
+                        .build_commit_permalink(&remote, BuildCommitPermalinkParams { sha: &sha }),
+                ),
+                pull_request,
+                remote: Some(GitRemote {
+                    host: hosting_provider,
+                    owner: remote.owner.into(),
+                    repo: remote.repo.into(),
+                }),
+            }
+        } else {
+            Self {
+                message: message.into(),
+                ..Default::default()
+            }
+        }
+    }
+}
 
 pub async fn get_messages(working_directory: &Path, shas: &[Oid]) -> Result<HashMap<Oid, String>> {
     if shas.is_empty() {

crates/git_ui/src/blame_ui.rs 🔗

@@ -3,10 +3,7 @@ use crate::{
     commit_view::CommitView,
 };
 use editor::{BlameRenderer, Editor, hover_markdown_style};
-use git::{
-    blame::{BlameEntry, ParsedCommitMessage},
-    repository::CommitSummary,
-};
+use git::{blame::BlameEntry, commit::ParsedCommitMessage, repository::CommitSummary};
 use gpui::{
     ClipboardItem, Entity, Hsla, MouseButton, ScrollHandle, Subscription, TextStyle,
     TextStyleRefinement, UnderlineStyle, WeakEntity, prelude::*,

crates/git_ui/src/commit_tooltip.rs 🔗

@@ -3,7 +3,7 @@ use editor::hover_markdown_style;
 use futures::Future;
 use git::blame::BlameEntry;
 use git::repository::CommitSummary;
-use git::{GitRemote, blame::ParsedCommitMessage};
+use git::{GitRemote, commit::ParsedCommitMessage};
 use gpui::{
     App, Asset, ClipboardItem, Element, Entity, MouseButton, ParentElement, Render, ScrollHandle,
     StatefulInteractiveElement, WeakEntity, prelude::*,

crates/git_ui/src/git_panel.rs 🔗

@@ -20,7 +20,7 @@ use editor::{
     actions::ExpandAllDiffHunks,
 };
 use futures::StreamExt as _;
-use git::blame::ParsedCommitMessage;
+use git::commit::ParsedCommitMessage;
 use git::repository::{
     Branch, CommitDetails, CommitOptions, CommitSummary, DiffType, FetchOptions, GitCommitter,
     PushOptions, Remote, RemoteCommandOutput, ResetMode, Upstream, UpstreamTracking,
@@ -30,8 +30,8 @@ use git::stash::GitStash;
 use git::status::StageStatus;
 use git::{Amend, Signoff, ToggleStaged, repository::RepoPath, status::FileStatus};
 use git::{
-    ExpandCommitEditor, RestoreTrackedFiles, StageAll, StashAll, StashApply, StashPop,
-    TrashUntrackedFiles, UnstageAll,
+    ExpandCommitEditor, GitHostingProviderRegistry, RestoreTrackedFiles, StageAll, StashAll,
+    StashApply, StashPop, TrashUntrackedFiles, UnstageAll,
 };
 use gpui::{
     Action, AsyncApp, AsyncWindowContext, Bounds, ClickEvent, Corner, DismissEvent, Entity,
@@ -5613,6 +5613,7 @@ impl GitPanelMessageTooltip {
         window: &mut Window,
         cx: &mut App,
     ) -> Entity<Self> {
+        let remote_url = repository.read(cx).default_remote_url();
         cx.new(|cx| {
             cx.spawn_in(window, async move |this, cx| {
                 let (details, workspace) = git_panel.update(cx, |git_panel, cx| {
@@ -5622,16 +5623,21 @@ impl GitPanelMessageTooltip {
                     )
                 })?;
                 let details = details.await?;
+                let provider_registry = cx
+                    .update(|_, app| GitHostingProviderRegistry::default_global(app))
+                    .ok();
 
                 let commit_details = crate::commit_tooltip::CommitDetails {
                     sha: details.sha.clone(),
                     author_name: details.author_name.clone(),
                     author_email: details.author_email.clone(),
                     commit_time: OffsetDateTime::from_unix_timestamp(details.commit_timestamp)?,
-                    message: Some(ParsedCommitMessage {
-                        message: details.message,
-                        ..Default::default()
-                    }),
+                    message: Some(ParsedCommitMessage::parse(
+                        details.sha.to_string(),
+                        details.message.to_string(),
+                        remote_url.as_deref(),
+                        provider_registry,
+                    )),
                 };
 
                 this.update(cx, |this: &mut GitPanelMessageTooltip, cx| {

crates/project/src/git_store.rs 🔗

@@ -5948,6 +5948,11 @@ impl Repository {
         self.pending_ops.edit(edits, ());
         ids
     }
+    pub fn default_remote_url(&self) -> Option<String> {
+        self.remote_upstream_url
+            .clone()
+            .or(self.remote_origin_url.clone())
+    }
 }
 
 fn get_permalink_in_rust_registry_src(