git blame: Parse permalinks client side (#10714)

Thorsten Ball created

Release Notes:

- N/A

Change summary

crates/collab/src/tests/editor_tests.rs | 14 ++++----------
crates/editor/src/git/blame.rs          | 27 ++++++++++++++++++++++++---
crates/git/src/blame.rs                 |  4 ++++
crates/git/src/permalink.rs             | 10 +++++-----
crates/project/src/project.rs           |  2 ++
crates/rpc/proto/zed.proto              |  1 +
6 files changed, 40 insertions(+), 18 deletions(-)

Detailed changes

crates/collab/src/tests/editor_tests.rs 🔗

@@ -3,6 +3,7 @@ use crate::{
     tests::{rust_lang, TestServer},
 };
 use call::ActiveCall;
+use collections::HashMap;
 use editor::{
     actions::{
         ConfirmCodeAction, ConfirmCompletion, ConfirmRename, Redo, Rename, RevertSelectedHunks,
@@ -2041,15 +2042,7 @@ async fn test_git_blame_is_forwarded(cx_a: &mut TestAppContext, cx_b: &mut TestA
             blame_entry("3a3a3a", 2..3),
             blame_entry("4c4c4c", 3..4),
         ],
-        permalinks: [
-            ("1b1b1b", "http://example.com/codehost/idx-0"),
-            ("0d0d0d", "http://example.com/codehost/idx-1"),
-            ("3a3a3a", "http://example.com/codehost/idx-2"),
-            ("4c4c4c", "http://example.com/codehost/idx-3"),
-        ]
-        .into_iter()
-        .map(|(sha, url)| (sha.parse().unwrap(), url.parse().unwrap()))
-        .collect(),
+        permalinks: HashMap::default(), // This field is deprecrated
         messages: [
             ("1b1b1b", "message for idx-0"),
             ("0d0d0d", "message for idx-1"),
@@ -2059,6 +2052,7 @@ async fn test_git_blame_is_forwarded(cx_a: &mut TestAppContext, cx_b: &mut TestA
         .into_iter()
         .map(|(sha, message)| (sha.parse().unwrap(), message.into()))
         .collect(),
+        remote_url: Some("git@github.com:zed-industries/zed.git".to_string()),
     };
     client_a.fs().set_blame_for_repo(
         Path::new("/my-repo/.git"),
@@ -2127,7 +2121,7 @@ async fn test_git_blame_is_forwarded(cx_a: &mut TestAppContext, cx_b: &mut TestA
                 assert_eq!(details.message, format!("message for idx-{}", idx));
                 assert_eq!(
                     details.permalink.unwrap().to_string(),
-                    format!("http://example.com/codehost/idx-{}", idx)
+                    format!("https://github.com/zed-industries/zed/commit/{}", entry.sha)
                 );
             }
         });

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

@@ -4,6 +4,7 @@ use anyhow::Result;
 use collections::HashMap;
 use git::{
     blame::{Blame, BlameEntry},
+    permalink::{build_commit_permalink, parse_git_remote_url},
     Oid,
 };
 use gpui::{Model, ModelContext, Subscription, Task};
@@ -286,11 +287,13 @@ impl GitBlame {
                             entries,
                             permalinks,
                             messages,
+                            remote_url,
                         } = blame.await?;
 
                         let entries = build_blame_entry_sum_tree(entries, snapshot.max_point().row);
                         let commit_details =
-                            parse_commit_messages(messages, &permalinks, &languages).await;
+                            parse_commit_messages(messages, remote_url, &permalinks, &languages)
+                                .await;
 
                         anyhow::Ok((entries, commit_details))
                     }
@@ -379,13 +382,31 @@ fn build_blame_entry_sum_tree(entries: Vec<BlameEntry>, max_row: u32) -> SumTree
 
 async fn parse_commit_messages(
     messages: impl IntoIterator<Item = (Oid, String)>,
-    permalinks: &HashMap<Oid, Url>,
+    remote_url: Option<String>,
+    deprecated_permalinks: &HashMap<Oid, Url>,
     languages: &Arc<LanguageRegistry>,
 ) -> HashMap<Oid, CommitDetails> {
     let mut commit_details = HashMap::default();
+
+    let parsed_remote_url = remote_url.as_deref().and_then(parse_git_remote_url);
+
     for (oid, message) in messages {
         let parsed_message = parse_markdown(&message, &languages).await;
-        let permalink = permalinks.get(&oid).cloned();
+
+        let permalink = if let Some(git_remote) = parsed_remote_url.as_ref() {
+            Some(build_commit_permalink(
+                git::permalink::BuildCommitPermalinkParams {
+                    remote: git_remote,
+                    sha: oid.to_string().as_str(),
+                },
+            ))
+        } else {
+            // DEPRECATED (18 Apr 24): Sending permalinks over the wire is deprecated. Clients
+            // now do the parsing. This is here for backwards compatibility, so that
+            // when an old peer sends a client no `parsed_remote_url` but `deprecated_permalinks`,
+            // we fall back to that.
+            deprecated_permalinks.get(&oid).cloned()
+        };
 
         commit_details.insert(
             oid,

crates/git/src/blame.rs 🔗

@@ -21,6 +21,7 @@ pub struct Blame {
     pub entries: Vec<BlameEntry>,
     pub messages: HashMap<Oid, String>,
     pub permalinks: HashMap<Oid, Url>,
+    pub remote_url: Option<String>,
 }
 
 impl Blame {
@@ -41,6 +42,8 @@ impl Blame {
 
         for entry in entries.iter_mut() {
             unique_shas.insert(entry.sha);
+            // DEPRECATED (18 Apr 24): Sending permalinks over the wire is deprecated. Clients
+            // now do the parsing.
             if let Some(remote) = parsed_remote_url.as_ref() {
                 permalinks.entry(entry.sha).or_insert_with(|| {
                     build_commit_permalink(BuildCommitPermalinkParams {
@@ -59,6 +62,7 @@ impl Blame {
             entries,
             permalinks,
             messages,
+            remote_url,
         })
     }
 }

crates/git/src/permalink.rs 🔗

@@ -3,7 +3,7 @@ use std::ops::Range;
 use anyhow::{anyhow, Result};
 use url::Url;
 
-pub(crate) enum GitHostingProvider {
+pub enum GitHostingProvider {
     Github,
     Gitlab,
     Gitee,
@@ -90,18 +90,18 @@ pub fn build_permalink(params: BuildPermalinkParams) -> Result<Url> {
     Ok(permalink)
 }
 
-pub(crate) struct ParsedGitRemote<'a> {
+pub struct ParsedGitRemote<'a> {
     pub provider: GitHostingProvider,
     pub owner: &'a str,
     pub repo: &'a str,
 }
 
-pub(crate) struct BuildCommitPermalinkParams<'a> {
+pub struct BuildCommitPermalinkParams<'a> {
     pub remote: &'a ParsedGitRemote<'a>,
     pub sha: &'a str,
 }
 
-pub(crate) fn build_commit_permalink(params: BuildCommitPermalinkParams) -> Url {
+pub fn build_commit_permalink(params: BuildCommitPermalinkParams) -> Url {
     let BuildCommitPermalinkParams { sha, remote } = params;
 
     let ParsedGitRemote {
@@ -122,7 +122,7 @@ pub(crate) fn build_commit_permalink(params: BuildCommitPermalinkParams) -> Url
     provider.base_url().join(&path).unwrap()
 }
 
-pub(crate) fn parse_git_remote_url(url: &str) -> Option<ParsedGitRemote> {
+pub fn parse_git_remote_url(url: &str) -> Option<ParsedGitRemote> {
     if url.starts_with("git@github.com:") || url.starts_with("https://github.com/") {
         let repo_with_owner = url
             .trim_start_matches("git@github.com:")

crates/project/src/project.rs 🔗

@@ -10727,6 +10727,7 @@ fn serialize_blame_buffer_response(blame: git::blame::Blame) -> proto::BlameBuff
         entries,
         messages,
         permalinks,
+        remote_url: blame.remote_url,
     }
 }
 
@@ -10775,6 +10776,7 @@ fn deserialize_blame_buffer_response(response: proto::BlameBufferResponse) -> gi
         entries,
         permalinks,
         messages,
+        remote_url: response.remote_url,
     }
 }
 

crates/rpc/proto/zed.proto 🔗

@@ -1963,6 +1963,7 @@ message BlameBufferResponse {
     repeated BlameEntry entries = 1;
     repeated CommitMessage messages = 2;
     repeated CommitPermalink permalinks = 3;
+    optional string remote_url = 4;
 }
 
 message MultiLspQuery {