From c2428f9f5da75dbb26e31945ecd7adb3a9b82c06 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Thu, 18 Apr 2024 12:36:22 +0200 Subject: [PATCH] git blame: Parse permalinks client side (#10714) Release Notes: - N/A --- 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(-) diff --git a/crates/collab/src/tests/editor_tests.rs b/crates/collab/src/tests/editor_tests.rs index 1706cb4a8b..3e541da2c6 100644 --- a/crates/collab/src/tests/editor_tests.rs +++ b/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) ); } }); diff --git a/crates/editor/src/git/blame.rs b/crates/editor/src/git/blame.rs index 3bf56b3fc9..bef72e0817 100644 --- a/crates/editor/src/git/blame.rs +++ b/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, max_row: u32) -> SumTree async fn parse_commit_messages( messages: impl IntoIterator, - permalinks: &HashMap, + remote_url: Option, + deprecated_permalinks: &HashMap, languages: &Arc, ) -> HashMap { 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, diff --git a/crates/git/src/blame.rs b/crates/git/src/blame.rs index 8eedefc6fe..d16915174d 100644 --- a/crates/git/src/blame.rs +++ b/crates/git/src/blame.rs @@ -21,6 +21,7 @@ pub struct Blame { pub entries: Vec, pub messages: HashMap, pub permalinks: HashMap, + pub remote_url: Option, } 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, }) } } diff --git a/crates/git/src/permalink.rs b/crates/git/src/permalink.rs index c69ce2f893..8ec2d86426 100644 --- a/crates/git/src/permalink.rs +++ b/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 { 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 { +pub fn parse_git_remote_url(url: &str) -> Option { if url.starts_with("git@github.com:") || url.starts_with("https://github.com/") { let repo_with_owner = url .trim_start_matches("git@github.com:") diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index f0f400198a..04af41ba71 100644 --- a/crates/project/src/project.rs +++ b/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, } } diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 606c0bb101..c2c0363efa 100644 --- a/crates/rpc/proto/zed.proto +++ b/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 {