From b43438d1a6446f5505c9e3f60e8b7f6871c960c6 Mon Sep 17 00:00:00 2001 From: damocles Date: Fri, 29 May 2026 00:41:40 +0200 Subject: [PATCH] hive-forge: collapse autogenerated diffs to a single one-line summary (#222) --- hive-forge/src/verbs/diff.rs | 99 ++++++++++++++++++++---------------- 1 file changed, 55 insertions(+), 44 deletions(-) diff --git a/hive-forge/src/verbs/diff.rs b/hive-forge/src/verbs/diff.rs index e35793b9..066e0dfc 100644 --- a/hive-forge/src/verbs/diff.rs +++ b/hive-forge/src/verbs/diff.rs @@ -1,14 +1,16 @@ //! `diff [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 `[: +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). +//! By default any per-file section whose target path matches a +//! known autogenerated lockfile (`flake.lock`, `Cargo.lock`, +//! `package-lock.json`, …) is collapsed to a single +//! `[: contents changed (+N -M, --full for content)]` +//! line so a `flake.lock` rev bump doesn't drown the human- +//! authored changes in 5 000 lines of lock churn (#222). The +//! per-file git headers (`diff --git`, `index`, `---`, `+++`, +//! and any rename / mode metadata) are suppressed alongside the +//! hunks since the placeholder already carries the file path and +//! the +/- magnitude — the headers add four lines of noise per +//! lockfile without any information the reader can act on. //! Pass `--full` to dump the unfiltered diff. use anyhow::Result; @@ -40,13 +42,12 @@ pub fn run(client: &Client, args: Args) -> Result<()> { 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. +/// Walk a unified diff line-by-line. When a `diff --git` line +/// names a known autogenerated file (`is_autogenerated`), suppress +/// every line of that per-file section (the git headers AND the +/// hunks) and emit a single `[file.lock: contents changed (+N -M, +/// --full for content)]` line in their place. Non-autogenerated +/// files pass through unchanged. fn collapse_autogenerated(diff: &str) -> String { let mut out = String::with_capacity(diff.len()); let mut state: Option = None; @@ -61,23 +62,23 @@ fn collapse_autogenerated(diff: &str) -> String { if let Some(p) = path.as_deref() && is_autogenerated(p) { + // Suppress the entire per-file block (headers and + // hunks alike). The placeholder we emit on flush + // carries the path, so the git headers are pure + // noise. state = Some(CollapseState::new(p.to_owned())); + continue; } 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("@@") { + // Inside an autogenerated section. Tally `+`/`-` body + // lines for the placeholder; drop everything else + // (headers, `@@` markers, context lines). + 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, @@ -85,8 +86,8 @@ fn collapse_autogenerated(diff: &str) -> String { _ => {} } } - continue; } + continue; } out.push_str(line); out.push('\n'); @@ -97,11 +98,11 @@ fn collapse_autogenerated(diff: &str) -> String { 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). +/// Per-file accumulator for the collapsed 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, @@ -121,7 +122,7 @@ impl CollapseState { fn placeholder(&self) -> String { format!( - "[{}: +{} -{} (autogenerated; pass --full for content)]\n", + "[{}: contents changed (+{} -{}, --full for content)]\n", self.path, self.added, self.removed ) } @@ -247,18 +248,23 @@ index 3333..4444 100644 +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")); + // Per-file git headers for the lockfile are suppressed — + // the placeholder already carries the path, so the four + // header lines are pure noise (4 lines per lockfile, stacks + // fast on a multi-lockfile PR). + 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}" + out.contains("[Cargo.lock: contents changed (+1 -1, --full for content)]"), + "expected one-line placeholder, got: {out}" ); // Non-lockfile file passes through untouched. + assert!(out.contains("diff --git a/src/main.rs")); assert!(out.contains("fn main() {}")); assert!(out.contains("println!(\"hi\")")); } @@ -300,13 +306,17 @@ index 1111..2222 100644 +lock-line-two-bumped "; let out = collapse_autogenerated(diff); + // README.md (non-lockfile) passes through with its full + // header + hunks intact. + assert!(out.contains("diff --git a/README.md")); assert!(out.contains("-old")); assert!(out.contains("+new")); - assert!(out.contains("diff --git a/flake.lock")); + // Lockfile is fully suppressed — no headers, no content. + 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}" + out.contains("[flake.lock: contents changed (+1 -1, --full for content)]"), + "expected one-line placeholder, got: {out}" ); } @@ -331,7 +341,7 @@ index 1111..2222 100644 "; let out = collapse_autogenerated(diff); assert!( - out.contains("[flake.lock: +4 -1"), + out.contains("[flake.lock: contents changed (+4 -1, --full for content)]"), "expected +4 -1, got: {out}" ); } @@ -401,13 +411,14 @@ index 1111..2222 100644 +new "; let out = collapse_autogenerated(diff); - // Content must be suppressed (lockfile detected). + // Entire per-file block suppressed (lockfile detected). + assert!(!out.contains("diff --git"), "headers leaked: {out}"); 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}" + out.contains("[odd dir/Cargo.lock: contents changed (+1 -1, --full for content)]"), + "expected one-line placeholder for quoted-path lockfile, got: {out}" ); } }