diff --git a/Cargo.lock b/Cargo.lock index 8aeffc53c8f28d..a5e27f772fd76a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7382,6 +7382,7 @@ dependencies = [ "itertools 0.14.0", "language", "language_model", + "linkify", "log", "markdown", "menu", diff --git a/crates/git_ui/Cargo.toml b/crates/git_ui/Cargo.toml index 4b6fc85041b346..e65265264b82cf 100644 --- a/crates/git_ui/Cargo.toml +++ b/crates/git_ui/Cargo.toml @@ -38,6 +38,7 @@ gpui.workspace = true itertools.workspace = true language.workspace = true language_model.workspace = true +linkify.workspace = true log.workspace = true markdown.workspace = true menu.workspace = true diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index d22220af2a28de..15ceaf98b7715b 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -4443,26 +4443,27 @@ impl GitPanel { .size(IconSize::Small) .color(Color::Muted), ); - match (style, is_push) { - (Toast | ToastWithLog { .. }, true) => { + match style { + PushPrLink { link } => this.action("Create Pull Request", move |_, cx| { + cx.open_url(&link); + }), + Toast | ToastWithLog { .. } if is_push => { this.action("Create Pull Request", move |window, cx| { window .dispatch_action(Box::new(zed_actions::git::CreatePullRequest), cx); }) } - (Toast, false) => this, - (ToastWithLog { output }, false) => { - this.action("View Log", move |window, cx| { - let output = output.clone(); - let output = - format!("stdout:\n{}\nstderr:\n{}", output.stdout, output.stderr); - workspace_weak - .update(cx, move |workspace, cx| { - open_output(operation, workspace, &output, window, cx) - }) - .ok(); - }) - } + Toast => this, + ToastWithLog { output } => this.action("View Log", move |window, cx| { + let output = output.clone(); + let output = + format!("stdout:\n{}\nstderr:\n{}", output.stdout, output.stderr); + workspace_weak + .update(cx, move |workspace, cx| { + open_output(operation, workspace, &output, window, cx) + }) + .ok(); + }), } .dismiss_button(true) }); diff --git a/crates/git_ui/src/remote_output.rs b/crates/git_ui/src/remote_output.rs index 157ce8316775d9..5ddc6459baad50 100644 --- a/crates/git_ui/src/remote_output.rs +++ b/crates/git_ui/src/remote_output.rs @@ -1,6 +1,7 @@ use anyhow::Context as _; use git::repository::{Remote, RemoteCommandOutput}; +use linkify::{LinkFinder, LinkKind}; use ui::SharedString; use util::ResultExt as _; @@ -21,9 +22,19 @@ impl RemoteAction { } } +#[derive(Debug)] pub enum SuccessStyle { Toast, - ToastWithLog { output: RemoteCommandOutput }, + ToastWithLog { + output: RemoteCommandOutput, + }, + /// A push whose stderr contained a link to create or view a pull/merge + /// request. Opening this URL directly avoids relying on a hosting provider + /// being registered for the remote, which is not the case for every host + /// (e.g. self-hosted GitLab instances behind a private domain). + PushPrLink { + link: String, + }, } pub struct SuccessMessage { @@ -123,15 +134,46 @@ pub fn format_output(action: &RemoteAction, output: RemoteCommandOutput) -> Succ style: SuccessStyle::Toast, } } else { - SuccessMessage { - message: format!("Pushed {} to {}", branch_name, remote_ref.name), - style: SuccessStyle::ToastWithLog { output }, - } + // Many hosting providers print a link to create or view a pull/merge + // request in the push output (prefixed with `remote:`). Prefer that + // link when present: it is produced by the server itself, so it works + // for any host regardless of whether Zed has a matching provider. + let link = extract_pull_request_link(&output.stderr); + let message = format!("Pushed {} to {}", branch_name, remote_ref.name); + let style = match link { + Some(link) => SuccessStyle::PushPrLink { link }, + None => SuccessStyle::ToastWithLog { output }, + }; + SuccessMessage { message, style } } } } } +/// Extracts a pull/merge request link from a push command's stderr, if any. +/// +/// Hosting providers surface these links on lines prefixed with `remote:` +/// (e.g. GitHub's "Create a pull request for ... on GitHub by visiting:" +/// followed by the URL, or GitLab's "To create a merge request for ..., visit:"). +/// We only inspect `remote:` lines so that unrelated URLs printed earlier in +/// the output (such as OpenSSH's post-quantum warning linking to openssh.com) +/// are not picked up. +fn extract_pull_request_link(stderr: &str) -> Option { + let finder = LinkFinder::new(); + stderr.lines().find_map(|line| { + let trimmed = line.trim_start(); + trimmed + .strip_prefix("remote:") + .and_then(|rest| { + finder + .links(rest) + .find(|link| *link.kind() == LinkKind::Url) + .map(|link| link.as_str().trim().to_string()) + }) + .filter(|link| !link.is_empty()) + }) +} + #[cfg(test)] mod tests { use super::*; @@ -162,7 +204,10 @@ mod tests { let msg = format_output(&action, output); - assert!(matches!(msg.style, SuccessStyle::ToastWithLog { .. })); + let SuccessStyle::PushPrLink { link } = &msg.style else { + panic!("Expected PushPrLink variant, got {:?}", msg.style); + }; + assert_eq!(link, "https://example.com/test/test/pull/new/test"); assert_eq!(msg.message, "Pushed test_branch to test_remote"); } @@ -191,7 +236,13 @@ mod tests { let msg = format_output(&action, output); - assert!(matches!(msg.style, SuccessStyle::ToastWithLog { .. })); + let SuccessStyle::PushPrLink { link } = &msg.style else { + panic!("Expected PushPrLink variant, got {:?}", msg.style); + }; + assert_eq!( + link, + "https://example.com/test/test/-/merge_requests/new?merge_request%5Bsource_branch%5D=test" + ); assert_eq!(msg.message, "Pushed test_branch to test_remote"); } @@ -224,7 +275,11 @@ mod tests { let msg = format_output(&action, output); - assert!(matches!(msg.style, SuccessStyle::ToastWithLog { .. })); + let SuccessStyle::PushPrLink { link } = &msg.style else { + panic!("Expected PushPrLink variant, got {:?}", msg.style); + }; + // The openssh.com URL on a non-`remote:` line must be ignored. + assert_eq!(link, "https://example.com/test/test/-/merge_requests/99999"); assert_eq!(msg.message, "Pushed test_branch to test_remote"); } @@ -258,4 +313,42 @@ mod tests { panic!("Expected ToastWithLog variant"); } } + + /// Regression test for an internal GitLab host with no registered provider: + /// the create-merge-request URL printed by the server must still be picked + /// up from stderr so the toast can open it directly, instead of failing + /// with "Unsupported remote URL". + #[test] + fn test_push_internal_host_merge_request_link() { + let action = RemoteAction::Push( + SharedString::new_static("dtm-harness"), + Remote { + name: SharedString::new_static("origin"), + }, + ); + + let output = RemoteCommandOutput { + stdout: String::new(), + stderr: indoc! {" + remote: + remote: To create a merge request for dtm-harness, visit: + remote: https://git.woa.com/ybtm-client/dtm-harness/-/merge_requests/new?merge_request%5Bsource_branch%5D=dtm-harness + remote: + To git.woa.com:ybtm-client/dtm-harness.git + * [new branch] dtm-harness -> dtm-harness + "} + .to_string(), + }; + + let msg = format_output(&action, output); + + let SuccessStyle::PushPrLink { link } = &msg.style else { + panic!("Expected PushPrLink variant, got {:?}", msg.style); + }; + assert_eq!( + link, + "https://git.woa.com/ybtm-client/dtm-harness/-/merge_requests/new?merge_request%5Bsource_branch%5D=dtm-harness" + ); + assert_eq!(msg.message, "Pushed dtm-harness to origin"); + } }