From 67a3c9e2da8edcfa80d535eaa7ecfe4e4a0b821b Mon Sep 17 00:00:00 2001 From: damocles Date: Tue, 26 May 2026 17:59:59 +0200 Subject: [PATCH] hive-forge: stat-style +N -M counts in collapsed-lockfile placeholder (#222) --- hive-forge/src/verbs/diff.rs | 139 +++++++++++++++++++++++++++-------- 1 file changed, 107 insertions(+), 32 deletions(-) diff --git a/hive-forge/src/verbs/diff.rs b/hive-forge/src/verbs/diff.rs index f9e4b18a..a9575ae3 100644 --- a/hive-forge/src/verbs/diff.rs +++ b/hive-forge/src/verbs/diff.rs @@ -2,12 +2,14 @@ //! //! By default the hunks for known autogenerated lockfiles //! (`flake.lock`, `Cargo.lock`, `package-lock.json`, …) are -//! collapsed to a one-line 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 — only the per-hunk content -//! is omitted. Pass `--full` to dump the unfiltered diff. +//! 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). +//! Pass `--full` to dump the unfiltered diff. use anyhow::Result; use clap::Args as ClapArgs; @@ -41,50 +43,88 @@ pub fn run(client: &Client, args: Args) -> Result<()> { /// 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 -/// `[N lines of … omitted]` placeholder before the next file. -/// Non-autogenerated files pass through unchanged. +/// 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 collapse = false; - let mut in_body = false; - let mut omitted = 0usize; + let mut state: Option = None; for line in diff.lines() { if let Some(rest) = line.strip_prefix("diff --git ") { // New file section — flush prior collapse counter. - if collapse && omitted > 0 { - out.push_str(&placeholder(omitted)); + if let Some(s) = state.take() { + out.push_str(&s.placeholder()); } let path = parse_diff_git_path(rest); - collapse = path.as_deref().is_some_and(is_autogenerated); - in_body = false; - omitted = 0; + 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; } - // First `@@` marks the boundary between file header and - // hunk content; everything from this point is suppressed - // when `collapse` is on. - if !in_body && line.starts_with("@@") { - in_body = true; - } - if collapse && in_body { - omitted += 1; - 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 collapse && omitted > 0 { - out.push_str(&placeholder(omitted)); + if let Some(s) = state.take() { + out.push_str(&s.placeholder()); } out } -fn placeholder(n: usize) -> String { - format!("[{n} lines of autogenerated content omitted; pass --full to view]\n") +/// 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` @@ -159,7 +199,13 @@ index 3333..4444 100644 assert!(out.contains("--- a/Cargo.lock")); assert!(out.contains("+++ b/Cargo.lock")); assert!(!out.contains("[[package]]")); - assert!(out.contains("4 lines of autogenerated content omitted")); + // 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\")")); @@ -206,7 +252,36 @@ index 1111..2222 100644 assert!(out.contains("+new")); assert!(out.contains("diff --git a/flake.lock")); assert!(!out.contains("lock-line-one")); - assert!(out.contains("4 lines of autogenerated content omitted")); + 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]