From 8f211e085d3f4a5dc106bfdea36db9b8d944cf76 Mon Sep 17 00:00:00 2001 From: Urgau Date: Tue, 15 Oct 2024 12:14:57 +0200 Subject: [PATCH] Improve rendered link handler with proper edit and sha-based link --- src/handlers/rendered_link.rs | 89 ++++++++++++++++++++++++++--------- 1 file changed, 66 insertions(+), 23 deletions(-) diff --git a/src/handlers/rendered_link.rs b/src/handlers/rendered_link.rs index 81547701..037f0607 100644 --- a/src/handlers/rendered_link.rs +++ b/src/handlers/rendered_link.rs @@ -1,3 +1,5 @@ +use anyhow::bail; + use crate::{ github::{Event, IssuesAction, IssuesEvent}, handlers::Context, @@ -10,6 +12,10 @@ pub async fn handle(ctx: &Context, event: &Event) -> anyhow::Result<()> { return Ok(()); }; + if !e.issue.is_pr() { + return Ok(()); + } + let repo = e.issue.repository(); let prefix = match (&*repo.organization, &*repo.repository) { ("rust-lang", "rfcs") => "text/", @@ -25,32 +31,69 @@ pub async fn handle(ctx: &Context, event: &Event) -> anyhow::Result<()> { } async fn add_rendered_link(ctx: &Context, e: &IssuesEvent, prefix: &str) -> anyhow::Result<()> { - if e.action == IssuesAction::Opened { + if e.action == IssuesAction::Opened + || e.action == IssuesAction::Closed + || e.action == IssuesAction::Reopened + { let files = e.issue.files(&ctx.github).await?; if let Some(file) = files.iter().find(|f| f.filename.starts_with(prefix)) { - if !e.issue.body.contains("[Rendered]") { - // This URL should be stable while the PR is open, even if the - // user pushes new commits. - // - // It will go away if the user deletes their branch, or if - // they reset it (such as if they created a PR from master). - // That should usually only happen after the PR is closed. - // During the closing process, the closer should update the - // Rendered link to the new location (which we should - // automate!). - let head = e.issue.head.as_ref().unwrap(); - let url = format!( - "https://github.com/{}/blob/{}/{}", - head.repo.full_name, head.git_ref, file.filename - ); - e.issue - .edit_body( - &ctx.github, - &format!("{}\n\n[Rendered]({})", e.issue.body, url), - ) - .await?; - } + let head = e.issue.head.as_ref().unwrap(); + let base = e.issue.base.as_ref().unwrap(); + + // This URL should be stable while the PR is open, even if the + // user pushes new commits. + // + // It will go away if the user deletes their branch, or if + // they reset it (such as if they created a PR from master). + // That should usually only happen after the PR is closed + // a which point we switch to a SHA-based url. + // + // If the PR is merged we use a URL that points to the actual + // repository, as to be resilient to branch deletion, as well + // be in sync with current "master" branch. + // + // For a PR "octocat:master" <- "Bob:patch-1", we generate, + // - if merged: `https://github.com/octocat/REPO/blob/master/FILEPATH` + // - if open: `https://github.com/Bob/REPO/blob/patch-1/FILEPATH` + // - if closed: `https://github.com/octocat/REPO/blob/SHA/FILEPATH` + let rendered_link = format!( + "[Rendered](https://github.com/{}/blob/{}/{})", + if e.issue.merged || e.action == IssuesAction::Closed { + &e.repository.full_name + } else { + &head.repo.full_name + }, + if e.issue.merged { + &base.git_ref + } else if e.action == IssuesAction::Closed { + &head.sha + } else { + &head.git_ref + }, + file.filename + ); + + let new_body = if !e.issue.body.contains("[Rendered]") { + // add rendered link to the end of the body + format!("{}\n\n{rendered_link}", e.issue.body) + } else if let Some(start_pos) = e.issue.body.find("[Rendered](") { + let Some(end_offset) = &e.issue.body[start_pos..].find(')') else { + bail!("no `)` after `[Rendered]` found") + }; + + // replace the current rendered link with the new one + e.issue.body.replace( + &e.issue.body[start_pos..=(start_pos + end_offset)], + &rendered_link, + ) + } else { + bail!( + "found `[Rendered]` but not it's associated link, can't replace it, bailing out" + ) + }; + + e.issue.edit_body(&ctx.github, &new_body).await?; } }