From 15521179fcf0fabba508e64f6a9231b88190202d Mon Sep 17 00:00:00 2001 From: damocles Date: Tue, 26 May 2026 16:39:36 +0200 Subject: [PATCH] hive-forge: collapse autogenerated lockfile hunks in diff (#222) --- hive-forge/src/verbs/diff.rs | 210 ++++++++++++++++++++++++++++++++++- 1 file changed, 209 insertions(+), 1 deletion(-) diff --git a/hive-forge/src/verbs/diff.rs b/hive-forge/src/verbs/diff.rs index aa64c5cd..f9e4b18a 100644 --- a/hive-forge/src/verbs/diff.rs +++ b/hive-forge/src/verbs/diff.rs @@ -1,4 +1,13 @@ //! `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 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. use anyhow::Result; use clap::Args as ClapArgs; @@ -9,11 +18,210 @@ use crate::client::Client; 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")?; - print!("{diff}"); + 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 +/// `[N lines of … omitted]` 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; + + 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)); + } + let path = parse_diff_git_path(rest); + collapse = path.as_deref().is_some_and(is_autogenerated); + in_body = false; + omitted = 0; + 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; + } + out.push_str(line); + out.push('\n'); + } + if collapse && omitted > 0 { + out.push_str(&placeholder(omitted)); + } + out +} + +fn placeholder(n: usize) -> String { + format!("[{n} lines of autogenerated content omitted; pass --full to view]\n") +} + +/// `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. +fn parse_diff_git_path(rest: &str) -> Option { + let token = rest.split_whitespace().nth(1)?; + let unquoted = token.trim_matches('"'); + Some(unquoted.strip_prefix("b/").unwrap_or(unquoted).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]]")); + assert!(out.contains("4 lines of autogenerated content omitted")); + // 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("4 lines of autogenerated content omitted")); + } + + #[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()) + ); + } +}