hive-forge: collapse autogenerated lockfile hunks in diff (#222)
This commit is contained in:
parent
cfef348a7f
commit
15521179fc
1 changed files with 209 additions and 1 deletions
|
|
@ -1,4 +1,13 @@
|
||||||
//! `diff <pr> [repo]` — print the unified diff for a PR.
|
//! `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 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 anyhow::Result;
|
||||||
use clap::Args as ClapArgs;
|
use clap::Args as ClapArgs;
|
||||||
|
|
@ -9,11 +18,210 @@ use crate::client::Client;
|
||||||
pub struct Args {
|
pub struct Args {
|
||||||
/// PR number.
|
/// PR number.
|
||||||
number: u64,
|
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<()> {
|
pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
let repo = client.repo();
|
let repo = client.repo();
|
||||||
let diff = client.get_text(&format!("/repos/{repo}/pulls/{}.diff", args.number), "text/plain")?;
|
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(())
|
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<String> {
|
||||||
|
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())
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue