Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions src/gh_range_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}:");
Expand Down Expand Up @@ -346,6 +348,7 @@ fn process_old_new(
html,
r#"<details open=""><summary>{filename} <a href="{before_href}">before</a> <a href="{after_href}">after</a></summary><pre class="diff-content">{diff}</pre></details>"#
)?;
diff_displayed += 1;
}
Ok(())
};
Expand Down Expand Up @@ -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, "<p>No differences</p>")?;
}

writeln!(
html,
r#"
Expand Down
191 changes: 174 additions & 17 deletions src/handlers/check_commits/force_push_range_diff.rs
Original file line number Diff line number Diff line change
@@ -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<String>,
}

pub(super) async fn handle_event(
ctx: &Context,
host: &str,
Expand All @@ -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<String> {
// 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:") {
Expand All @@ -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(
Copy link
Member Author

@Urgau Urgau Aug 16, 2025

Choose a reason for hiding this comment

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

I don't like that this logic is here.

I would prefer if it was shared with the other check-commits sub-handlers, but given the requirement of only hiding the previous comment when posting a new one, which is different from all the others sub-handler, it's was simpler to do it here rather than increasing the complexity in check-commits.

Copy link
Member

Choose a reason for hiding this comment

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

I guess that in theory we could add the logic to the shared "check commits result" comment, but for now this is fine.

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()
)
);
}
}