413 lines
14 KiB
Rust
413 lines
14 KiB
Rust
//! `diff <pr> [repo]` — print the unified diff for a PR.
|
||
//!
|
||
//! By default the hunks for known autogenerated lockfiles
|
||
//! (`flake.lock`, `Cargo.lock`, `package-lock.json`, …) are
|
||
//! collapsed to a `[<path>: +N -M (autogenerated; pass --full for
|
||
//! content)]` placeholder so a `flake.lock` rev bump doesn't drown
|
||
//! the human-authored changes in 5 000 lines of lock churn
|
||
//! (#222). The header (`diff --git`, `index`, `---`, `+++`,
|
||
//! rename / mode metadata) is preserved so the reader can still
|
||
//! see WHICH lockfiles changed; the +/- counts give a `diff
|
||
//! --stat`-style magnitude (excluding the `@@` hunk header).
|
||
//! Pass `--full` to dump the unfiltered diff.
|
||
|
||
use anyhow::Result;
|
||
use clap::Args as ClapArgs;
|
||
|
||
use crate::client::Client;
|
||
|
||
#[derive(ClapArgs)]
|
||
pub struct Args {
|
||
/// PR number.
|
||
number: u64,
|
||
/// Print the unfiltered diff including autogenerated-file
|
||
/// hunks (`flake.lock`, `Cargo.lock`, etc.). Default is to
|
||
/// collapse those hunks to a placeholder so the human-authored
|
||
/// changes aren't drowned in lock churn.
|
||
#[arg(long)]
|
||
full: bool,
|
||
}
|
||
|
||
pub fn run(client: &Client, args: Args) -> Result<()> {
|
||
let repo = client.repo();
|
||
let diff = client.get_text(&format!("/repos/{repo}/pulls/{}.diff", args.number), "text/plain")?;
|
||
let out = if args.full {
|
||
diff
|
||
} else {
|
||
collapse_autogenerated(&diff)
|
||
};
|
||
print!("{out}");
|
||
Ok(())
|
||
}
|
||
|
||
/// Walk a unified diff line-by-line. For each per-file section
|
||
/// whose target path matches a known autogenerated file
|
||
/// (`is_autogenerated`), drop every line from the first hunk
|
||
/// header (`@@`) onward and emit a single `diff --stat`-style
|
||
/// `[file.lock: +N -M (autogenerated, --full for content)]`
|
||
/// placeholder before the next file. Non-autogenerated files
|
||
/// pass through unchanged.
|
||
fn collapse_autogenerated(diff: &str) -> String {
|
||
let mut out = String::with_capacity(diff.len());
|
||
let mut state: Option<CollapseState> = None;
|
||
|
||
for line in diff.lines() {
|
||
if let Some(rest) = line.strip_prefix("diff --git ") {
|
||
// New file section — flush prior collapse counter.
|
||
if let Some(s) = state.take() {
|
||
out.push_str(&s.placeholder());
|
||
}
|
||
let path = parse_diff_git_path(rest);
|
||
if let Some(p) = path.as_deref()
|
||
&& is_autogenerated(p)
|
||
{
|
||
state = Some(CollapseState::new(p.to_owned()));
|
||
}
|
||
out.push_str(line);
|
||
out.push('\n');
|
||
continue;
|
||
}
|
||
if let Some(s) = state.as_mut() {
|
||
// First `@@` marks the boundary between file header and
|
||
// hunk content; everything from this point is
|
||
// suppressed (and tallied) while `state` is Some.
|
||
if !s.in_body && line.starts_with("@@") {
|
||
s.in_body = true;
|
||
}
|
||
if s.in_body {
|
||
// The hunk header `@@` itself counts as a body line
|
||
// for the +/− tally only via its descendant content
|
||
// lines; skip it for the counters.
|
||
if !line.starts_with("@@") {
|
||
match line.as_bytes().first() {
|
||
Some(b'+') => s.added += 1,
|
||
Some(b'-') => s.removed += 1,
|
||
_ => {}
|
||
}
|
||
}
|
||
continue;
|
||
}
|
||
}
|
||
out.push_str(line);
|
||
out.push('\n');
|
||
}
|
||
if let Some(s) = state.take() {
|
||
out.push_str(&s.placeholder());
|
||
}
|
||
out
|
||
}
|
||
|
||
/// Per-file accumulator for the collapsed-hunk placeholder. Tracks
|
||
/// the file's display name plus `+`/`−` line counts (excluding the
|
||
/// `@@` hunk header), so the placeholder shows operator-meaningful
|
||
/// magnitude instead of "N lines omitted" (which mixed context +
|
||
/// added + removed indistinguishably).
|
||
struct CollapseState {
|
||
path: String,
|
||
in_body: bool,
|
||
added: u32,
|
||
removed: u32,
|
||
}
|
||
|
||
impl CollapseState {
|
||
fn new(path: String) -> Self {
|
||
Self {
|
||
path,
|
||
in_body: false,
|
||
added: 0,
|
||
removed: 0,
|
||
}
|
||
}
|
||
|
||
fn placeholder(&self) -> String {
|
||
format!(
|
||
"[{}: +{} -{} (autogenerated; pass --full for content)]\n",
|
||
self.path, self.added, self.removed
|
||
)
|
||
}
|
||
}
|
||
|
||
/// `rest` is everything after `diff --git `, e.g. `a/foo b/foo`
|
||
/// or `"a/path with space" "b/path with space"`. Return the
|
||
/// post-rename (`b/`-side) path so renames report the new path.
|
||
///
|
||
/// Git uses C-style quoting (`\"`, `\\`, octal escapes) for paths
|
||
/// with spaces or unusual bytes — we don't unescape because we
|
||
/// only need the bytewise file name for the lockfile allowlist
|
||
/// match. We do parse the quoted-vs-unquoted form correctly so a
|
||
/// whitespace-containing path doesn't break `split_whitespace`.
|
||
fn parse_diff_git_path(rest: &str) -> Option<String> {
|
||
if let Some(after_open) = rest.strip_prefix('"') {
|
||
// Quoted form: `"a/<path>" "b/<path>"`. Find the closing
|
||
// quote of the a-side (skipping `\"` escapes so a path
|
||
// containing `"` doesn't terminate early).
|
||
let mut iter = after_open.char_indices();
|
||
let mut a_close = None;
|
||
loop {
|
||
let Some((i, c)) = iter.next() else { break };
|
||
if c == '\\' {
|
||
// Skip the next char — it's part of the escape.
|
||
iter.next();
|
||
continue;
|
||
}
|
||
if c == '"' {
|
||
a_close = Some(i);
|
||
break;
|
||
}
|
||
}
|
||
let a_close = a_close?;
|
||
// After the a-side closing quote, we expect `" "b/...`.
|
||
// `after_open` skipped the leading `"`, so the b-side
|
||
// starts in `after_open[a_close + 1..]` — strip leading
|
||
// space + opening quote + `b/`, then strip trailing `"`.
|
||
let after_a = after_open.get(a_close + 1..)?;
|
||
let after_a = after_a.strip_prefix(' ')?;
|
||
let b_inside = after_a.strip_prefix('"')?;
|
||
// Find b-side's closing quote with the same escape rule.
|
||
let mut iter = b_inside.char_indices();
|
||
let mut b_close = None;
|
||
loop {
|
||
let Some((i, c)) = iter.next() else { break };
|
||
if c == '\\' {
|
||
iter.next();
|
||
continue;
|
||
}
|
||
if c == '"' {
|
||
b_close = Some(i);
|
||
break;
|
||
}
|
||
}
|
||
let b_close = b_close?;
|
||
let b_path = b_inside.get(..b_close)?;
|
||
Some(b_path.strip_prefix("b/").unwrap_or(b_path).to_owned())
|
||
} else {
|
||
// Unquoted: paths have no whitespace, so `split_whitespace`
|
||
// gives exactly two tokens.
|
||
let token = rest.split_whitespace().nth(1)?;
|
||
Some(token.strip_prefix("b/").unwrap_or(token).to_owned())
|
||
}
|
||
}
|
||
|
||
/// File-name match against a small whitelist of well-known
|
||
/// machine-generated lockfiles. We deliberately don't pattern-match
|
||
/// extensions like `*.lock` because some real human-authored files
|
||
/// use that suffix (e.g. `keep.lock` config markers); explicit
|
||
/// listing avoids surprising the user.
|
||
fn is_autogenerated(path: &str) -> bool {
|
||
let name = path.rsplit('/').next().unwrap_or(path);
|
||
matches!(
|
||
name,
|
||
"flake.lock"
|
||
| "Cargo.lock"
|
||
| "package-lock.json"
|
||
| "pnpm-lock.yaml"
|
||
| "yarn.lock"
|
||
| "Gemfile.lock"
|
||
| "poetry.lock"
|
||
| "Pipfile.lock"
|
||
| "composer.lock"
|
||
| "go.sum"
|
||
)
|
||
}
|
||
|
||
#[cfg(test)]
|
||
mod tests {
|
||
use super::*;
|
||
|
||
#[test]
|
||
fn detects_known_lockfiles() {
|
||
assert!(is_autogenerated("flake.lock"));
|
||
assert!(is_autogenerated("a/flake.lock"));
|
||
assert!(is_autogenerated("hive-c0re/Cargo.lock"));
|
||
assert!(is_autogenerated("frontend/packages/dashboard/package-lock.json"));
|
||
assert!(!is_autogenerated("src/main.rs"));
|
||
assert!(!is_autogenerated("Cargo.toml"));
|
||
// Suffix-only files we deliberately don't match — keep
|
||
// `is_autogenerated` allowlist-driven, not pattern-driven.
|
||
assert!(!is_autogenerated("config/keep.lock"));
|
||
}
|
||
|
||
#[test]
|
||
fn collapse_replaces_lockfile_hunks_with_placeholder() {
|
||
let diff = "\
|
||
diff --git a/Cargo.lock b/Cargo.lock
|
||
index 1111..2222 100644
|
||
--- a/Cargo.lock
|
||
+++ b/Cargo.lock
|
||
@@ -1,3 +1,3 @@
|
||
[[package]]
|
||
-name = \"old\"
|
||
+name = \"new\"
|
||
diff --git a/src/main.rs b/src/main.rs
|
||
index 3333..4444 100644
|
||
--- a/src/main.rs
|
||
+++ b/src/main.rs
|
||
@@ -1,2 +1,2 @@
|
||
-fn main() {}
|
||
+fn main() { println!(\"hi\"); }
|
||
";
|
||
let out = collapse_autogenerated(diff);
|
||
assert!(out.contains("diff --git a/Cargo.lock"));
|
||
assert!(out.contains("--- a/Cargo.lock"));
|
||
assert!(out.contains("+++ b/Cargo.lock"));
|
||
assert!(!out.contains("[[package]]"));
|
||
// Stat-style placeholder: 1 added (`+name = "new"`), 1
|
||
// removed (`-name = "old"`); context line and `@@` header
|
||
// don't count.
|
||
assert!(
|
||
out.contains("[Cargo.lock: +1 -1"),
|
||
"expected stat placeholder, got: {out}"
|
||
);
|
||
// Non-lockfile file passes through untouched.
|
||
assert!(out.contains("fn main() {}"));
|
||
assert!(out.contains("println!(\"hi\")"));
|
||
}
|
||
|
||
#[test]
|
||
fn collapse_passes_normal_diff_through() {
|
||
let diff = "\
|
||
diff --git a/src/lib.rs b/src/lib.rs
|
||
index aaaa..bbbb 100644
|
||
--- a/src/lib.rs
|
||
+++ b/src/lib.rs
|
||
@@ -1,1 +1,1 @@
|
||
-fn old() {}
|
||
+fn new() {}
|
||
";
|
||
let out = collapse_autogenerated(diff);
|
||
assert_eq!(out, diff);
|
||
}
|
||
|
||
#[test]
|
||
fn collapse_handles_trailing_lockfile() {
|
||
// Lockfile is the LAST file in the diff — exercises the
|
||
// post-loop flush path.
|
||
let diff = "\
|
||
diff --git a/README.md b/README.md
|
||
index aaaa..bbbb 100644
|
||
--- a/README.md
|
||
+++ b/README.md
|
||
@@ -1,1 +1,1 @@
|
||
-old
|
||
+new
|
||
diff --git a/flake.lock b/flake.lock
|
||
index 1111..2222 100644
|
||
--- a/flake.lock
|
||
+++ b/flake.lock
|
||
@@ -1,2 +1,2 @@
|
||
lock-line-one
|
||
-lock-line-two
|
||
+lock-line-two-bumped
|
||
";
|
||
let out = collapse_autogenerated(diff);
|
||
assert!(out.contains("-old"));
|
||
assert!(out.contains("+new"));
|
||
assert!(out.contains("diff --git a/flake.lock"));
|
||
assert!(!out.contains("lock-line-one"));
|
||
assert!(
|
||
out.contains("[flake.lock: +1 -1"),
|
||
"expected stat placeholder, got: {out}"
|
||
);
|
||
}
|
||
|
||
#[test]
|
||
fn collapse_counts_distinguish_added_and_removed() {
|
||
// Big asymmetric churn — flake.lock rev bump with several
|
||
// adds and one removal in the worker fields. Verifies
|
||
// we don't conflate `+`/`-` totals.
|
||
let diff = "\
|
||
diff --git a/flake.lock b/flake.lock
|
||
index 1111..2222 100644
|
||
--- a/flake.lock
|
||
+++ b/flake.lock
|
||
@@ -1,5 +1,8 @@
|
||
{
|
||
- \"old\": 1
|
||
+ \"new\": 1,
|
||
+ \"another\": 2,
|
||
+ \"and\": 3,
|
||
+ \"more\": 4
|
||
}
|
||
";
|
||
let out = collapse_autogenerated(diff);
|
||
assert!(
|
||
out.contains("[flake.lock: +4 -1"),
|
||
"expected +4 -1, got: {out}"
|
||
);
|
||
}
|
||
|
||
#[test]
|
||
fn parse_diff_git_path_picks_b_side() {
|
||
assert_eq!(
|
||
parse_diff_git_path("a/foo b/foo"),
|
||
Some("foo".to_owned())
|
||
);
|
||
assert_eq!(
|
||
parse_diff_git_path("a/old.txt b/new.txt"),
|
||
Some("new.txt".to_owned())
|
||
);
|
||
assert_eq!(
|
||
parse_diff_git_path("a/dir/Cargo.lock b/dir/Cargo.lock"),
|
||
Some("dir/Cargo.lock".to_owned())
|
||
);
|
||
}
|
||
|
||
#[test]
|
||
fn parse_diff_git_path_handles_quoted_paths_with_spaces() {
|
||
// Git quotes paths with spaces / unusual chars per
|
||
// `core.quotePath`. The b-side path must survive intact.
|
||
assert_eq!(
|
||
parse_diff_git_path("\"a/foo bar\" \"b/foo bar\""),
|
||
Some("foo bar".to_owned())
|
||
);
|
||
// Rename with quoted sides.
|
||
assert_eq!(
|
||
parse_diff_git_path("\"a/old name\" \"b/new name\""),
|
||
Some("new name".to_owned())
|
||
);
|
||
// Lockfile inside a directory whose name has a space.
|
||
assert_eq!(
|
||
parse_diff_git_path("\"a/dir with space/Cargo.lock\" \"b/dir with space/Cargo.lock\""),
|
||
Some("dir with space/Cargo.lock".to_owned())
|
||
);
|
||
}
|
||
|
||
#[test]
|
||
fn parse_diff_git_path_handles_escaped_quote_inside_path() {
|
||
// Git escapes embedded `"` as `\"`. The closing-quote
|
||
// search must skip these so it doesn't terminate early.
|
||
// Path is literally `a/has"quote` / `b/has"quote`.
|
||
let rest = r#""a/has\"quote" "b/has\"quote""#;
|
||
assert_eq!(
|
||
parse_diff_git_path(rest),
|
||
// Bytewise: backslash + quote stay in the result
|
||
// because we don't unescape (allowlist match is by
|
||
// file name, never contains escapes).
|
||
Some(r#"has\"quote"#.to_owned())
|
||
);
|
||
}
|
||
|
||
#[test]
|
||
fn collapse_recognises_lockfile_in_quoted_path() {
|
||
// Path "a/odd dir/Cargo.lock" forces git to use the
|
||
// quoted form. We must still detect it as a lockfile.
|
||
let diff = "\
|
||
diff --git \"a/odd dir/Cargo.lock\" \"b/odd dir/Cargo.lock\"
|
||
index 1111..2222 100644
|
||
--- \"a/odd dir/Cargo.lock\"
|
||
+++ \"b/odd dir/Cargo.lock\"
|
||
@@ -1,1 +1,1 @@
|
||
-old
|
||
+new
|
||
";
|
||
let out = collapse_autogenerated(diff);
|
||
// Content must be suppressed (lockfile detected).
|
||
assert!(!out.contains("old\n"), "lock content leaked: {out}");
|
||
assert!(!out.contains("+new"), "lock content leaked: {out}");
|
||
// Placeholder uses the parsed b-side path (without quotes).
|
||
assert!(
|
||
out.contains("[odd dir/Cargo.lock: +1 -1"),
|
||
"expected stat placeholder for quoted-path lockfile, got: {out}"
|
||
);
|
||
}
|
||
}
|