From f7fe6181cde07b1bf9e5e27ed11badc1ab647fb6 Mon Sep 17 00:00:00 2001 From: littleKitchen <55852668+littleKitchen@users.noreply.github.com> Date: Sat, 31 Jan 2026 13:56:34 -0800 Subject: [PATCH] Implement `extract_pull_request` for GitLab provider (#47973) Fixes #38709 ## Summary This implements the `extract_pull_request` method for GitLab's `GitHostingProvider` trait, enabling merge request links to be shown in the git blame hover popover. ## Implementation The implementation parses GitLab MR references from commit messages using a regex that matches two common patterns: 1. **Squash merge pattern**: `message (!123)` - When GitLab squash-merges, it appends the MR number in parentheses 2. **Standard merge commit**: `See merge request group/project!123` - The default merge commit message ## Tests Added - `test_extract_merge_request_from_squash_commit` - Validates the `(!123)` pattern - `test_extract_merge_request_from_merge_commit` - Validates the `See merge request` pattern - `test_extract_merge_request_self_hosted` - Ensures it works with self-hosted GitLab instances - `test_extract_merge_request_no_match` - Confirms None is returned for non-matching messages ## Related This follows the same pattern as the GitHub provider implementation. ## Release Notes - Added support for GitLab merge request links in git blame hover popover --- .../src/providers/gitlab.rs | 114 +++++++++++++++++- 1 file changed, 112 insertions(+), 2 deletions(-) diff --git a/crates/git_hosting_providers/src/providers/gitlab.rs b/crates/git_hosting_providers/src/providers/gitlab.rs index ec81d6d7f61bddb287e935374e5154487818b240..2d4e71aace01cc9f434daa637497fa345798e88a 100644 --- a/crates/git_hosting_providers/src/providers/gitlab.rs +++ b/crates/git_hosting_providers/src/providers/gitlab.rs @@ -1,19 +1,31 @@ -use std::{str::FromStr, sync::Arc}; +use std::str::FromStr; +use std::sync::{Arc, LazyLock}; use anyhow::{Context as _, Result, bail}; use async_trait::async_trait; use futures::AsyncReadExt; use gpui::SharedString; use http_client::{AsyncBody, HttpClient, HttpRequestExt, Request}; +use regex::Regex; use serde::Deserialize; use url::Url; use urlencoding::encode; use git::{ BuildCommitPermalinkParams, BuildPermalinkParams, GitHostingProvider, ParsedGitRemote, - RemoteUrl, + PullRequest, RemoteUrl, }; +fn merge_request_number_regex() -> &'static Regex { + static MERGE_REQUEST_NUMBER_REGEX: LazyLock = LazyLock::new(|| { + // Matches GitLab MR references: + // - "(!123)" at the end of line (squash merge pattern) + // - "See merge request group/project!123" (standard merge commit) + Regex::new(r"(?:\(!(\d+)\)$|See merge request [^\s]+!(\d+))").unwrap() + }); + &MERGE_REQUEST_NUMBER_REGEX +} + use crate::get_host_from_git_remote_url; #[derive(Debug, Deserialize)] @@ -229,6 +241,27 @@ impl GitHostingProvider for Gitlab { Some(url) } + fn extract_pull_request(&self, remote: &ParsedGitRemote, message: &str) -> Option { + // Check commit message for GitLab MR references + let capture = merge_request_number_regex().captures(message)?; + // The regex has two capture groups - one for "(!123)" pattern, one for "See merge request" pattern + let number = capture + .get(1) + .or_else(|| capture.get(2))? + .as_str() + .parse::() + .ok()?; + + let mut url = self.base_url(); + let path = format!( + "{}/{}/-/merge_requests/{}", + remote.owner, remote.repo, number + ); + url.set_path(&path); + + Some(PullRequest { number, url }) + } + async fn commit_author_avatar_url( &self, repo_owner: &str, @@ -486,4 +519,81 @@ mod tests { "https://gitlab.zed.com/zed-industries/zed/-/merge_requests/new?merge_request%5Bsource_branch%5D=feature%2Fnew-feature" ); } + + #[test] + fn test_extract_merge_request_from_squash_commit() { + let remote = ParsedGitRemote { + owner: "zed-industries".into(), + repo: "zed".into(), + }; + + let provider = Gitlab::public_instance(); + + // Test squash merge pattern: "commit message (!123)" + let message = "Add new feature (!456)"; + let pull_request = provider.extract_pull_request(&remote, message).unwrap(); + + assert_eq!(pull_request.number, 456); + assert_eq!( + pull_request.url.as_str(), + "https://gitlab.com/zed-industries/zed/-/merge_requests/456" + ); + } + + #[test] + fn test_extract_merge_request_from_merge_commit() { + let remote = ParsedGitRemote { + owner: "zed-industries".into(), + repo: "zed".into(), + }; + + let provider = Gitlab::public_instance(); + + // Test standard merge commit pattern: "See merge request group/project!123" + let message = + "Merge branch 'feature' into 'main'\n\nSee merge request zed-industries/zed!789"; + let pull_request = provider.extract_pull_request(&remote, message).unwrap(); + + assert_eq!(pull_request.number, 789); + assert_eq!( + pull_request.url.as_str(), + "https://gitlab.com/zed-industries/zed/-/merge_requests/789" + ); + } + + #[test] + fn test_extract_merge_request_self_hosted() { + let base_url = Url::parse("https://gitlab.my-company.com").unwrap(); + let provider = Gitlab::new("GitLab Self-Hosted", base_url); + + let remote = ParsedGitRemote { + owner: "team".into(), + repo: "project".into(), + }; + + let message = "Fix bug (!42)"; + let pull_request = provider.extract_pull_request(&remote, message).unwrap(); + + assert_eq!(pull_request.number, 42); + assert_eq!( + pull_request.url.as_str(), + "https://gitlab.my-company.com/team/project/-/merge_requests/42" + ); + } + + #[test] + fn test_extract_merge_request_no_match() { + let remote = ParsedGitRemote { + owner: "zed-industries".into(), + repo: "zed".into(), + }; + + let provider = Gitlab::public_instance(); + + // No MR reference in message + let message = "Just a regular commit message"; + let pull_request = provider.extract_pull_request(&remote, message); + + assert!(pull_request.is_none()); + } }