Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle pushes and rename for rendered links #1850

Merged
merged 2 commits into from
Oct 31, 2024
Merged

Handle pushes and rename for rendered links #1850

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb766af
Also react rendered link handler on synchronize (new commits)
Urgau Oct 30, 2024
18de61a
Remove link if no file and optimize rendered link api calls
Urgau Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 69 additions & 49 deletions src/handlers/rendered_link.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::borrow::Cow;

use anyhow::bail;

use crate::{
Expand Down Expand Up @@ -34,65 +36,83 @@ async fn add_rendered_link(ctx: &Context, e: &IssuesEvent, prefix: &str) -> anyh
if e.action == IssuesAction::Opened
|| e.action == IssuesAction::Closed
|| e.action == IssuesAction::Reopened
|| e.action == IssuesAction::Synchronize
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally though I would have to go through the PushEvent instead of the Issue/PushRequestEvent, but it seems to be as simple as checking the synchronize action, yay for simplicity.

{
let files = e.issue.files(&ctx.github).await?;

if let Some(file) = files.iter().find(|f| f.filename.starts_with(prefix)) {
let head = e.issue.head.as_ref().unwrap();
let base = e.issue.base.as_ref().unwrap();
let rendered_link = files
.iter()
.find(|f| f.filename.starts_with(prefix))
.map(|file| {
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
);
// 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`
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]") {
let new_body: Cow<'_, str> = if !e.issue.body.contains("[Rendered]") {
if let Some(rendered_link) = rendered_link {
// 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")
};
format!("{}\n\n{rendered_link}", e.issue.body).into()
} else {
// or return the original body since we don't have
// a rendered link to add
e.issue.body.as_str().into()
}
} 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(
// replace the current rendered link with the new one or replace
// it with an empty string if we don't have one
e.issue
.body
.replace(
&e.issue.body[start_pos..=(start_pos + end_offset)],
&rendered_link,
rendered_link.as_deref().unwrap_or(""),
)
} else {
bail!(
"found `[Rendered]` but not it's associated link, can't replace it, bailing out"
)
};
.into()
} else {
bail!(
"found `[Rendered]` but not it's associated link, can't replace it or remove it, bailing out"
)
};

// avoid an expensive GitHub api call by first checking if we actually
// edited the pull request body
if e.issue.body != new_body {
e.issue.edit_body(&ctx.github, &new_body).await?;
}
}
Expand Down
Loading