diff --git a/src/gh_range_diff.rs b/src/gh_range_diff.rs index 6e3f56e5..551ab99e 100644 --- a/src/gh_range_diff.rs +++ b/src/gh_range_diff.rs @@ -315,6 +315,8 @@ fn process_old_new( "# )?; + let mut diff_displayed = 0; + let mut process_diffs = |filename, old_patch, new_patch| -> anyhow::Result<()> { // Removes diff markers to avoid false-positives let new_marker = format!("@@ {filename}:"); @@ -346,6 +348,7 @@ fn process_old_new( html, r#"
{filename} before after
{diff}
"# )?; + diff_displayed += 1; } Ok(()) }; @@ -379,6 +382,11 @@ fn process_old_new( process_diffs(filename, "", &*new_file.patch)?; } + // Print message when there aren't any differences + if diff_displayed == 0 { + writeln!(html, "

No differences

")?; + } + writeln!( html, r#" diff --git a/src/handlers/check_commits/force_push_range_diff.rs b/src/handlers/check_commits/force_push_range_diff.rs index 1917c350..57c734da 100644 --- a/src/handlers/check_commits/force_push_range_diff.rs +++ b/src/handlers/check_commits/force_push_range_diff.rs @@ -1,11 +1,24 @@ use anyhow::Context as _; use crate::config::RangeDiffConfig; +use crate::db::issue_data::IssueData; +use crate::github::CommitBase; use crate::github::GithubCompare; use crate::github::IssueRepository; use crate::github::IssuesEvent; +use crate::github::ReportedContentClassifiers; use crate::handlers::Context; +/// Key for the state in the database +const RANGE_DIFF_LINK_KEY: &str = "range-diff-link"; + +/// State stored in the database +#[derive(Debug, Default, serde::Deserialize, serde::Serialize, Clone, PartialEq)] +struct RangeDiffLinkState { + /// ID of the most recent range-diff comment. + last_comment: Option, +} + pub(super) async fn handle_event( ctx: &Context, host: &str, @@ -23,25 +36,45 @@ pub(super) async fn handle_event( }; let base = event.issue.base.as_ref().context("no base ref")?; - let org = event.repository.owner(); - let repo = event.repository.name(); + let issue_repo = IssueRepository { + organization: event.repository.owner().to_string(), + repository: event.repository.name().to_string(), + }; let compare_before = ctx .github - .compare( - &IssueRepository { - organization: org.to_string(), - repository: repo.to_string(), - }, - &base.sha, - &before, - ) + .compare(&issue_repo, &base.sha, &before) .await .context("failed to get the before compare")?; + if let Some(message) = changed_base_commit( + host, + &issue_repo, + base, + &compare_before, + compare_after, + before, + after, + ) { + // Rebase detected, post a comment linking to our range-diff. + post_new_comment(ctx, event, message).await?; + } + + Ok(()) +} + +fn changed_base_commit( + host: &str, + issue_repo: &IssueRepository, + base: &CommitBase, + compare_before: &GithubCompare, + compare_after: &GithubCompare, + oldhead: &str, + newhead: &str, +) -> Option { // Does the merge_base_commits differs? No, not a force-push with rebase. if compare_before.merge_base_commit.sha == compare_after.merge_base_commit.sha { - return Ok(()); + return None; } let protocol = if host.starts_with("localhost:") { @@ -51,13 +84,137 @@ pub(super) async fn handle_event( }; let branch = &base.git_ref; - let (oldbase, oldhead) = (&compare_before.merge_base_commit.sha, before); - let (newbase, newhead) = (&compare_after.merge_base_commit.sha, after); + let oldbase = &compare_before.merge_base_commit.sha; + let newbase = &compare_after.merge_base_commit.sha; - // Rebase detected, post a comment to our range-diff. - event.issue.post_comment(&ctx.github, - &format!(r#"This PR was rebased onto a different {branch} commit! Check out the changes with our [`range-diff`]({protocol}://{host}/gh-range-diff/{org}/{repo}/{oldbase}..{oldhead}/{newbase}..{newhead})."#) - ).await.context("failed to post range-diff comment")?; + let message = format!( + r#"This PR was rebased onto a different {branch} commit. Here's a [range-diff]({protocol}://{host}/gh-range-diff/{issue_repo}/{oldbase}..{oldhead}/{newbase}..{newhead}) highlighting what actually changed. + +*Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.*"# + ); + + Some(message) +} + +async fn post_new_comment( + ctx: &Context, + event: &IssuesEvent, + message: String, +) -> anyhow::Result<()> { + let mut db = ctx.db.get().await; + let mut state: IssueData<'_, RangeDiffLinkState> = + IssueData::load(&mut db, &event.issue, RANGE_DIFF_LINK_KEY).await?; + + // Hide previous range-diff comment. + if let Some(last_comment) = state.data.last_comment { + event + .issue + .hide_comment( + &ctx.github, + &last_comment, + ReportedContentClassifiers::Outdated, + ) + .await + .context("failed to hide previous range-diff comment")?; + } + + // Post new range-diff comment and remember it. + state.data.last_comment = Some( + event + .issue + .post_comment(&ctx.github, &message) + .await + .context("failed to post range-diff comment")? + .node_id, + ); + + state.save().await?; Ok(()) } + +#[cfg(test)] +mod tests { + use crate::handlers::check_commits::dummy_commit_from_body; + + use super::changed_base_commit; + + #[test] + fn unchanged_base_commit() { + assert_eq!( + changed_base_commit( + "mytriagebot.com", + &crate::github::IssueRepository { + organization: "rust-lang".to_string(), + repository: "rust".to_string() + }, + &crate::github::CommitBase { + sha: "sha-base-commit".to_string(), + git_ref: "master".to_string(), + repo: None + }, + &crate::github::GithubCompare { + base_commit: dummy_commit_from_body("base-commit-sha", "base-commit-body"), + merge_base_commit: dummy_commit_from_body( + "same-merge-commit", + "merge-commit-body" + ), + files: vec![] + }, + &crate::github::GithubCompare { + base_commit: dummy_commit_from_body("base-commit-sha", "base-commit-body"), + merge_base_commit: dummy_commit_from_body( + "same-merge-commit", + "merge-commit-body" + ), + files: vec![] + }, + "oldhead", + "newhead" + ), + None + ); + } + + #[test] + fn changed_base_commit_() { + assert_eq!( + changed_base_commit( + "mytriagebot.com", + &crate::github::IssueRepository { + organization: "rust-lang".to_string(), + repository: "rust".to_string() + }, + &crate::github::CommitBase { + sha: "sha-base-commit".to_string(), + git_ref: "master".to_string(), + repo: None + }, + &crate::github::GithubCompare { + base_commit: dummy_commit_from_body("base-commit-sha", "base-commit-body"), + merge_base_commit: dummy_commit_from_body( + "before-merge-commit", + "merge-commit-body" + ), + files: vec![] + }, + &crate::github::GithubCompare { + base_commit: dummy_commit_from_body("base-commit-sha", "base-commit-body"), + merge_base_commit: dummy_commit_from_body( + "after-merge-commit", + "merge-commit-body" + ), + files: vec![] + }, + "oldhead", + "newhead" + ), + Some( + r#"This PR was rebased onto a different master commit. Here's a [range-diff](https://mytriagebot.com/gh-range-diff/rust-lang/rust/before-merge-commit..oldhead/after-merge-commit..newhead) highlighting what actually changed. + +*Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.*"# + .to_string() + ) + ); + } +}