From 074a55c0c6022c50d61dffed3ec6b9b104ffed70 Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Thu, 23 Apr 2026 18:12:42 -0700 Subject: [PATCH 1/6] diff: simplify line-range filter by classifying removals immediately The line-range filter buffered '-' (removal) lines in a pending_rm strbuf, deferring their classification until a '+' or ' ' line arrived to reveal the post-image position. This buffering is unnecessary. Removal lines are pre-image content that occupies no post-image space, so they do not advance lno_post. Within a hunk, xdiff emits removals before additions for each change, so a '-' line always arrives while lno_post is at the same position that the following '+' or ' ' line will occupy. Each line can therefore be classified against the tracked ranges as it arrives, without waiting for a non-removal line to confirm the position. The buffering also had a bug: flush_rhunk() unconditionally drained the pending buffer when the range hunk was active, even if lno_post had advanced past the tracked range. This caused deletions immediately after the tracked function to be incorrectly included in patch output. Remove the pending_rm buffer and classify '-' lines using the same in_range check applied to '+' and ' ' lines. This simplifies the filter, removes three struct fields and a helper function, and makes the flush_rhunk() bug impossible by construction. Signed-off-by: Michael Montalbo --- diff.c | 114 ++++++++++++------------------------ t/t4211-line-log.sh | 137 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 174 insertions(+), 77 deletions(-) diff --git a/diff.c b/diff.c index 5a584fa1d569e7..81aa9ef3b42ec3 100644 --- a/diff.c +++ b/diff.c @@ -617,11 +617,6 @@ struct emit_callback { * the requested ranges. Contiguous in-range lines are collected into * range hunks and flushed with a synthetic @@ header so that * fn_out_consume() sees well-formed unified-diff fragments. - * - * Removal lines ('-') cannot be classified by post-image position, so - * they are buffered in pending_rm until the next '+' or ' ' line - * reveals whether they precede an in-range line (flush into range hunk) or - * an out-of-range line (discard). */ struct line_range_callback { xdiff_emit_line_fn orig_line_fn; @@ -647,11 +642,6 @@ struct line_range_callback { int rhunk_active; int rhunk_has_changes; /* any '+' or '-' lines? */ - /* Removal lines not yet known to be in-range */ - struct strbuf pending_rm; - int pending_rm_count; - long pending_rm_pre_begin; /* pre-image line of first pending */ - int ret; /* latched error from orig_line_fn */ }; @@ -2540,12 +2530,6 @@ static int quick_consume(void *priv, char *line UNUSED, unsigned long len UNUSED return 1; } -static void discard_pending_rm(struct line_range_callback *s) -{ - strbuf_reset(&s->pending_rm); - s->pending_rm_count = 0; -} - static void flush_rhunk(struct line_range_callback *s) { struct strbuf hdr = STRBUF_INIT; @@ -2554,14 +2538,6 @@ static void flush_rhunk(struct line_range_callback *s) if (!s->rhunk_active || s->ret) return; - /* Drain any pending removal lines into the range hunk */ - if (s->pending_rm_count) { - strbuf_addbuf(&s->rhunk, &s->pending_rm); - s->rhunk_old_count += s->pending_rm_count; - s->rhunk_has_changes = 1; - discard_pending_rm(s); - } - /* * Suppress context-only hunks: they contain no actual changes * and would just be noise. This can happen when the inflated @@ -2616,11 +2592,6 @@ static void line_range_hunk_fn(void *data, * When count > 0, begin is 1-based. When count == 0, begin is * adjusted down by 1 by xdl_emit_hunk_hdr(), but no lines of * that type will arrive, so the value is unused. - * - * Any pending removal lines from the previous xdiff hunk are - * intentionally left in pending_rm: the line callback will - * flush or discard them when the next content line reveals - * whether the removals precede in-range content. */ s->lno_post = new_begin; s->lno_pre = old_begin; @@ -2636,88 +2607,79 @@ static void line_range_hunk_fn(void *data, static int line_range_line_fn(void *priv, char *line, unsigned long len) { struct line_range_callback *s = priv; - const struct range *cur; - long lno_0, cur_pre; + long lno_0; + int in_range; if (s->ret) return s->ret; - if (line[0] == '-') { - if (!s->pending_rm_count) - s->pending_rm_pre_begin = s->lno_pre; - s->lno_pre++; - strbuf_add(&s->pending_rm, line, len); - s->pending_rm_count++; - return s->ret; - } - if (line[0] == '\\') { - if (s->pending_rm_count) - strbuf_add(&s->pending_rm, line, len); - else if (s->rhunk_active) + if (s->rhunk_active) strbuf_add(&s->rhunk, line, len); - /* otherwise outside tracked range; drop silently */ return s->ret; } - if (line[0] != '+' && line[0] != ' ') + if (line[0] != '+' && line[0] != ' ' && line[0] != '-') BUG("unexpected diff line type '%c'", line[0]); + /* + * Compute post-image position. '+' and ' ' lines advance + * lno_post; '-' lines do not (they occupy no post-image space). + */ lno_0 = s->lno_post - 1; - cur_pre = s->lno_pre; /* save before advancing for context lines */ - s->lno_post++; - if (line[0] == ' ') - s->lno_pre++; + if (line[0] != '-') + s->lno_post++; - /* Advance past ranges we've passed */ + /* + * Advance past any ranges we've moved beyond. Emit the + * accumulated range hunk for the range we're leaving. + */ while (s->cur_range < s->ranges->nr && lno_0 >= s->ranges->ranges[s->cur_range].end) { if (s->rhunk_active) flush_rhunk(s); - discard_pending_rm(s); s->cur_range++; } - /* Past all ranges */ - if (s->cur_range >= s->ranges->nr) { - discard_pending_rm(s); - return s->ret; - } + in_range = s->cur_range < s->ranges->nr && + lno_0 >= s->ranges->ranges[s->cur_range].start && + lno_0 < s->ranges->ranges[s->cur_range].end; - cur = &s->ranges->ranges[s->cur_range]; - - /* Before current range */ - if (lno_0 < cur->start) { - discard_pending_rm(s); + if (!in_range) { + if (line[0] != '+') + s->lno_pre++; return s->ret; } - /* In range so start a new range hunk if needed */ + /* + * Start a new range hunk if this is the first in-range line; + * its position fixes the hunk's old/new starts, captured here + * before the append below advances lno_pre. + */ if (!s->rhunk_active) { s->rhunk_active = 1; s->rhunk_has_changes = 0; s->rhunk_new_begin = lno_0 + 1; - s->rhunk_old_begin = s->pending_rm_count - ? s->pending_rm_pre_begin : cur_pre; + s->rhunk_old_begin = s->lno_pre; s->rhunk_old_count = 0; s->rhunk_new_count = 0; strbuf_reset(&s->rhunk); } - /* Flush pending removals into range hunk */ - if (s->pending_rm_count) { - strbuf_addbuf(&s->rhunk, &s->pending_rm); - s->rhunk_old_count += s->pending_rm_count; - s->rhunk_has_changes = 1; - discard_pending_rm(s); - } - + /* Append line to the range hunk */ strbuf_add(&s->rhunk, line, len); - s->rhunk_new_count++; - if (line[0] == '+') + if (line[0] == '-') { + s->rhunk_old_count++; s->rhunk_has_changes = 1; - else + s->lno_pre++; + } else if (line[0] == '+') { + s->rhunk_new_count++; + s->rhunk_has_changes = 1; + } else { s->rhunk_old_count++; + s->rhunk_new_count++; + s->lno_pre++; + } return s->ret; } @@ -4095,7 +4057,6 @@ static void builtin_diff(const char *name_a, lr_state.orig_cb_data = &ecbdata; lr_state.ranges = line_ranges; strbuf_init(&lr_state.rhunk, 0); - strbuf_init(&lr_state.pending_rm, 0); /* * Inflate ctxlen so that all changes within @@ -4130,7 +4091,6 @@ static void builtin_diff(const char *name_a, die("unable to generate diff for %s", one->path); strbuf_release(&lr_state.rhunk); - strbuf_release(&lr_state.pending_rm); } else if (xdi_diff_outf(&mf1, &mf2, NULL, fn_out_consume, &ecbdata, &xpp, &xecfg)) die("unable to generate diff for %s", one->path); diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index ca4eb7bbc713ef..6b5b846c0a5a8d 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -738,6 +738,143 @@ test_expect_success '-L with -G filters to diff-text matches' ' grep "F2 + 2" actual ' +test_expect_success 'setup for trailing deletion test' ' + git checkout --orphan trailing-del && + git reset --hard && + cat >file.c <<-\EOF && + void tracked() + { + return 1; + } + // trailing comment + EOF + git add file.c && + test_tick && + git commit -m "add file with trailing comment" && + # Modify tracked() AND delete the trailing comment in + # one commit, so the commit touches the tracked range + # and is not filtered out by the revision walker. + cat >file.c <<-\EOF && + void tracked() + { + return 2; + } + EOF + git commit -a -m "modify tracked and delete trailing comment" +' + +test_expect_success '-L does not include deletions past end of tracked range' ' + git log -L:tracked:file.c --format= -1 -p >actual && + # The trailing comment deletion is outside the tracked + # range and should not appear in the patch output. + test_grep "return 2" actual && + test_grep ! "trailing comment" actual +' + +test_expect_success '-L includes leading deletions resolved by in-range line' ' + git checkout --orphan leading-del && + git reset --hard && + cat >file.c <<-\EOF && + // leading comment + void tracked() + { + return 1; + } + EOF + git add file.c && + test_tick && + git commit -m "add file with leading comment" && + cat >file.c <<-\EOF && + void tracked() + { + return 2; + } + EOF + git commit -a -m "modify tracked and delete leading comment" && + git log -L:tracked:file.c --format= -1 -p >actual && + # The leading comment deletion is resolved by the next + # non-removal line (void tracked), which is in range: a + # removal is classified by the position of the following + # line, so it joins the range that line falls in. + test_grep "return 2" actual && + test_grep "leading comment" actual +' + +test_expect_success 'setup for line-range filter edge cases' ' + git checkout --orphan filter-edge && + git reset --hard && + cat >file.c <<-\EOF && + void before() + { + return 0; + } + + void tracked() + { + int a = 1; + int b = 2; + int c = 3; + return a + b + c; + } + + void after() + { + return 9; + } + EOF + git add file.c && + test_tick && + git commit -m "initial" +' + +test_expect_success '-L change at exact first line of range' ' + git checkout filter-edge && + # Change the function signature (first line of range) + sed "s/void tracked/int tracked/" file.c >tmp && + mv tmp file.c && + git commit -a -m "change first line" && + git log -L:tracked:file.c -p --format=%s -1 >actual && + test_grep "change first line" actual && + test_grep "+int tracked" actual && + test_grep "\\-void tracked" actual +' + +test_expect_success '-L change at exact last line of range' ' + git checkout filter-edge && + git reset --hard HEAD~1 && + # Change the closing brace line (last line of range) + sed "s/^}$/} \/\/ end tracked/" file.c >tmp && + mv tmp file.c && + git commit -a -m "change last line" && + git log -L:tracked:file.c -p --format=%s -1 >actual && + test_grep "change last line" actual && + test_grep "end tracked" actual +' + +test_expect_success '-L pure deletion in range (no additions)' ' + git checkout filter-edge && + git reset --hard HEAD~1 && + # Delete a line inside tracked() without adding anything + sed "/int c/d" file.c >tmp && + mv tmp file.c && + git commit -a -m "pure deletion" && + git log -L:tracked:file.c -p --format=%s -1 >actual && + test_grep "pure deletion" actual && + test_grep "\\-.*int c" actual +' + +test_expect_success '-L adjacent change outside range excluded' ' + git checkout filter-edge && + git reset --hard HEAD~1 && + # Change only before() - adjacent to tracked() but outside range + sed "s/return 0/return 42/" file.c >tmp && + mv tmp file.c && + git commit -a -m "change before only" && + # Commit should not appear since tracked() is unchanged + git log -L:tracked:file.c --format=%s --no-patch >actual && + test_grep ! "change before only" actual +' + test_expect_success '-L with --diff-filter=M excludes root commit' ' git checkout parent-oids && git log -L:func2:file.c --diff-filter=M --format=%s --no-patch >actual && From 08a53e4f90b5d124ef1f8efc3b04efbc27f2f7f0 Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Mon, 15 Jun 2026 18:58:24 -0700 Subject: [PATCH 2/6] diff: fix off-by-one in -L hunk header for pure insertions The line-range filter that scopes "git log -L" output builds a synthetic hunk header in flush_rhunk(). For a side with no lines it emitted the begin unadjusted, naming the line at the change instead of the line before it. In t4211 this produced @@ -25,0 +18,9 @@ whose old side names a nonexistent line 25 in a 24-line file. The unified-diff convention that git diff and xdl_emit_hunk_hdr() follow is to name the line just before the change (-24 here). The convention dates to 12da1d1f6f (Implement line-history search (git log -L), 2013-03-28), whose hand-rolled diff printer derived headers from range arithmetic without that adjustment. It special-cased only file creation, so a creation matched git diff but an interior insertion did not. 86e986f166 (line-log: route -L output through the standard diff pipeline, 2026-03-17) reimplemented header emission through the diff machinery but preserved this convention, so the off-by-one is inherited rather than new. Blaming a changed fixture line can mislead here: for t4211's vanishes-early that transition turned a "\ No newline at end of file" modification (@@ -22,1 +24,1 @@) into a pure insertion (@@ -23,0 +24,1 @@), so that zero-count header is newly shaped by xdiff even though the rule it follows predates it. Apply the convention in flush_rhunk(): when a side has no lines, name the preceding line. rhunk_*_begin is seeded from xdiff's hunk header, which already made this adjustment for a pure creation or deletion (begin 0), so do not decrement a begin that is already 0. Regenerate the two affected t4211 fixtures. Every other hunk, including file creations, is unchanged. Signed-off-by: Michael Montalbo --- diff.c | 19 +++++++++++++++++-- t/t4211/sha1/expect.no-assertion-error | 2 +- t/t4211/sha1/expect.vanishes-early | 2 +- t/t4211/sha256/expect.no-assertion-error | 2 +- t/t4211/sha256/expect.vanishes-early | 2 +- 5 files changed, 21 insertions(+), 6 deletions(-) diff --git a/diff.c b/diff.c index 81aa9ef3b42ec3..91bd3b5a55d633 100644 --- a/diff.c +++ b/diff.c @@ -2534,6 +2534,7 @@ static void flush_rhunk(struct line_range_callback *s) { struct strbuf hdr = STRBUF_INIT; const char *p, *end; + long old_begin, new_begin; if (!s->rhunk_active || s->ret) return; @@ -2550,9 +2551,23 @@ static void flush_rhunk(struct line_range_callback *s) return; } + /* + * Follow the unified-diff convention (see xdl_emit_hunk_hdr()): + * a side with no lines names the line just before the change. + * rhunk_*_begin is seeded from xdiff's hunk header, which already + * applied this for a pure creation or deletion (begin 0), so only + * adjust a begin that still points at a real line. + */ + old_begin = s->rhunk_old_begin; + if (!s->rhunk_old_count && old_begin > 0) + old_begin--; + new_begin = s->rhunk_new_begin; + if (!s->rhunk_new_count && new_begin > 0) + new_begin--; + strbuf_addf(&hdr, "@@ -%ld,%ld +%ld,%ld @@", - s->rhunk_old_begin, s->rhunk_old_count, - s->rhunk_new_begin, s->rhunk_new_count); + old_begin, s->rhunk_old_count, + new_begin, s->rhunk_new_count); if (s->funclen > 0) { strbuf_addch(&hdr, ' '); strbuf_add(&hdr, s->func, s->funclen); diff --git a/t/t4211/sha1/expect.no-assertion-error b/t/t4211/sha1/expect.no-assertion-error index 54c568f273a6d2..95faf51a7b46d9 100644 --- a/t/t4211/sha1/expect.no-assertion-error +++ b/t/t4211/sha1/expect.no-assertion-error @@ -8,7 +8,7 @@ diff --git a/b.c b/b.c index bf79c2f..27c829c 100644 --- a/b.c +++ b/b.c -@@ -25,0 +18,9 @@ +@@ -24,0 +18,9 @@ +long f(long x) +{ + int s = 0; diff --git a/t/t4211/sha1/expect.vanishes-early b/t/t4211/sha1/expect.vanishes-early index a413ad36598ddf..bf0fb6d858c8b2 100644 --- a/t/t4211/sha1/expect.vanishes-early +++ b/t/t4211/sha1/expect.vanishes-early @@ -8,7 +8,7 @@ diff --git a/a.c b/a.c index 0b9cae5..5de3ea4 100644 --- a/a.c +++ b/a.c -@@ -23,0 +24,1 @@ int main () +@@ -22,0 +24,1 @@ int main () +/* incomplete lines are bad! */ commit 100b61a6f2f720f812620a9d10afb3a960ccb73c diff --git a/t/t4211/sha256/expect.no-assertion-error b/t/t4211/sha256/expect.no-assertion-error index c25f2ce19c05d9..815d27f7f17b7e 100644 --- a/t/t4211/sha256/expect.no-assertion-error +++ b/t/t4211/sha256/expect.no-assertion-error @@ -8,7 +8,7 @@ diff --git a/b.c b/b.c index 69cb69c..a0d566e 100644 --- a/b.c +++ b/b.c -@@ -25,0 +18,9 @@ +@@ -24,0 +18,9 @@ +long f(long x) +{ + int s = 0; diff --git a/t/t4211/sha256/expect.vanishes-early b/t/t4211/sha256/expect.vanishes-early index bc33b963dc8570..fce3d7bd38af42 100644 --- a/t/t4211/sha256/expect.vanishes-early +++ b/t/t4211/sha256/expect.vanishes-early @@ -8,7 +8,7 @@ diff --git a/a.c b/a.c index e4fa1d8..62c1fc2 100644 --- a/a.c +++ b/a.c -@@ -23,0 +24,1 @@ int main () +@@ -22,0 +24,1 @@ int main () +/* incomplete lines are bad! */ commit 29f32ac3141c48b22803e5c4127b719917b67d0f8ca8c5248bebfa2a19f7da10 From a4055bb555f353879cac520fab3ddb25fdca0858 Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Tue, 12 May 2026 17:57:20 -0700 Subject: [PATCH 3/6] diff: rename line_range_callback to line_range_filter The struct carries filter state through xdiff callbacks, tracking post-image position and forwarding only in-range lines to the output callback. Rename to line_range_filter to reflect this role, and update the comment to describe the struct as a generic filter rather than a wrapper specific to fn_out_consume(). Signed-off-by: Michael Montalbo --- diff.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/diff.c b/diff.c index 91bd3b5a55d633..fb853b38ceed02 100644 --- a/diff.c +++ b/diff.c @@ -610,15 +610,15 @@ struct emit_callback { }; /* - * State for the line-range callback wrappers that sit between - * xdi_diff_outf() and fn_out_consume(). xdiff produces a normal, - * unfiltered diff; the wrappers intercept each hunk header and line, - * track post-image position, and forward only lines that fall within + * State for the line-range filter that sits between xdi_diff_outf() + * and an output callback such as fn_out_consume(). xdiff produces a + * normal, unfiltered diff; the filter intercepts each hunk header and line, + * tracks post-image position, and forwards only lines that fall within * the requested ranges. Contiguous in-range lines are collected into * range hunks and flushed with a synthetic @@ header so that * fn_out_consume() sees well-formed unified-diff fragments. */ -struct line_range_callback { +struct line_range_filter { xdiff_emit_line_fn orig_line_fn; void *orig_cb_data; const struct range_set *ranges; /* 0-based [start, end) */ @@ -2530,7 +2530,7 @@ static int quick_consume(void *priv, char *line UNUSED, unsigned long len UNUSED return 1; } -static void flush_rhunk(struct line_range_callback *s) +static void flush_rhunk(struct line_range_filter *s) { struct strbuf hdr = STRBUF_INIT; const char *p, *end; @@ -2601,7 +2601,7 @@ static void line_range_hunk_fn(void *data, long new_begin, long new_nr UNUSED, const char *func, long funclen) { - struct line_range_callback *s = data; + struct line_range_filter *s = data; /* * When count > 0, begin is 1-based. When count == 0, begin is @@ -2621,7 +2621,7 @@ static void line_range_hunk_fn(void *data, static int line_range_line_fn(void *priv, char *line, unsigned long len) { - struct line_range_callback *s = priv; + struct line_range_filter *s = priv; long lno_0; int in_range; @@ -4063,7 +4063,7 @@ static void builtin_diff(const char *name_a, xdi_diff_outf(&mf1, &mf2, NULL, quick_consume, &ecbdata, &xpp, &xecfg); } else if (line_ranges) { - struct line_range_callback lr_state; + struct line_range_filter lr_state; unsigned int i; long max_span = 0; From e64990b8163384ccaec84aca2c46af5cfb9ce3eb Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Mon, 15 Jun 2026 19:45:33 -0700 Subject: [PATCH 4/6] diff: extract a line-range diff helper for reuse builtin_diff() open-codes the line-range filter setup and teardown around its xdi_diff_outf() call: zero the struct, point it at the output callback, run the diff, flush the trailing range hunk, and release the buffer. The upcoming -L stat and check formats need the same sequence. Extract line_range_filter_init() for the setup and a line_range_filter_diff() helper that runs an initialized filter through xdi_diff_outf(), flushes the final range hunk, and releases it, returning the latched error. builtin_diff() now does init + line_range_filter_diff(); the next two patches reuse them in builtin_diffstat() and builtin_checkdiff() instead of repeating the boilerplate. No behavior change. Signed-off-by: Michael Montalbo --- diff.c | 50 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/diff.c b/diff.c index fb853b38ceed02..e1bed8d85aafa1 100644 --- a/diff.c +++ b/diff.c @@ -2530,6 +2530,18 @@ static int quick_consume(void *priv, char *line UNUSED, unsigned long len UNUSED return 1; } +static void line_range_filter_init(struct line_range_filter *f, + const struct range_set *ranges, + xdiff_emit_line_fn line_fn, + void *cb_data) +{ + memset(f, 0, sizeof(*f)); + f->orig_line_fn = line_fn; + f->orig_cb_data = cb_data; + f->ranges = ranges; + strbuf_init(&f->rhunk, 0); +} + static void flush_rhunk(struct line_range_filter *s) { struct strbuf hdr = STRBUF_INIT; @@ -2699,6 +2711,25 @@ static int line_range_line_fn(void *priv, char *line, unsigned long len) return s->ret; } +/* + * Run an xdiff pass through an initialized line-range filter, flush the + * final range hunk, and release the filter. Returns non-zero if xdiff or + * any forwarded callback failed. + */ +static int line_range_filter_diff(struct line_range_filter *f, + mmfile_t *mf1, mmfile_t *mf2, + xpparam_t *xpp, xdemitconf_t *xecfg) +{ + int ret = xdi_diff_outf(mf1, mf2, line_range_hunk_fn, + line_range_line_fn, f, xpp, xecfg); + if (!ret) { + flush_rhunk(f); + ret = f->ret; + } + strbuf_release(&f->rhunk); + return ret; +} + static void pprint_rename(struct strbuf *name, const char *a, const char *b) { const char *old_name = a; @@ -4067,11 +4098,8 @@ static void builtin_diff(const char *name_a, unsigned int i; long max_span = 0; - memset(&lr_state, 0, sizeof(lr_state)); - lr_state.orig_line_fn = fn_out_consume; - lr_state.orig_cb_data = &ecbdata; - lr_state.ranges = line_ranges; - strbuf_init(&lr_state.rhunk, 0); + line_range_filter_init(&lr_state, line_ranges, + fn_out_consume, &ecbdata); /* * Inflate ctxlen so that all changes within @@ -4094,18 +4122,10 @@ static void builtin_diff(const char *name_a, if (max_span > xecfg.ctxlen) xecfg.ctxlen = max_span; - if (xdi_diff_outf(&mf1, &mf2, - line_range_hunk_fn, - line_range_line_fn, - &lr_state, &xpp, &xecfg)) - die("unable to generate diff for %s", - one->path); - - flush_rhunk(&lr_state); - if (lr_state.ret) + if (line_range_filter_diff(&lr_state, &mf1, &mf2, + &xpp, &xecfg)) die("unable to generate diff for %s", one->path); - strbuf_release(&lr_state.rhunk); } else if (xdi_diff_outf(&mf1, &mf2, NULL, fn_out_consume, &ecbdata, &xpp, &xecfg)) die("unable to generate diff for %s", one->path); From eff02ac0c1dfeaf1657e75a1a49b345dbff87f98 Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Mon, 15 Jun 2026 19:46:50 -0700 Subject: [PATCH 5/6] line-log: support diff stat formats with -L Reuse the line_range_filter in builtin_diffstat() to produce range-scoped statistics. When a filepair carries line_ranges, the filter wraps diffstat_consume() as its output callback, forwarding only in-range lines for counting. flush_rhunk() replays buffered content through diffstat_consume(), which ignores synthetic @@ headers since it only counts '+' and '-' lines. Expand the output format allowlist in setup_revisions() to accept --stat, --numstat, and --shortstat with -L. Leave --dirstat out of the allowlist so it is rejected like any other unsupported format. It summarizes change as a per-directory percentage, which is degenerate for the usual single tracked file (always 100% of its directory) and, in its default changed-bytes mode, would measure whole-file damage that ignores the tracked range entirely. --numstat already reports the exact range-scoped per-file counts, so -L gains nothing from --dirstat. Signed-off-by: Michael Montalbo --- Documentation/line-range-options.adoc | 12 ++- diff.c | 16 +++- revision.c | 6 +- t/t4211-line-log.sh | 118 +++++++++++++++++++++----- 4 files changed, 126 insertions(+), 26 deletions(-) diff --git a/Documentation/line-range-options.adoc b/Documentation/line-range-options.adoc index 72f639b5e79ea4..753a54f486198d 100644 --- a/Documentation/line-range-options.adoc +++ b/Documentation/line-range-options.adoc @@ -9,10 +9,14 @@ __ and __ (or __) must exist in the starting revision. You can specify this option more than once. Implies `--patch`. Patch output can be suppressed using `--no-patch`. - Non-patch diff formats `--raw`, `--name-only`, `--name-status`, - and `--summary` are supported. Diff stat formats - (`--stat`, `--numstat`, `--shortstat`, `--dirstat`) are not - currently implemented. + Other diff formats are supported, including `--raw`, + `--name-only`, `--name-status`, `--summary`, + `--stat`, `--numstat`, and `--shortstat`. + The stat formats show range-scoped counts: only lines within + the tracked range are counted. `--dirstat` is not supported: + by default it measures whole-file change rather than the + line-level diff, so it cannot be confined to the tracked range. + Use `--numstat` for exact per-file counts within the range. + Patch formatting options such as `--word-diff`, `--color-moved`, `--no-prefix`, and whitespace options (`-w`, `-b`) are supported, diff --git a/diff.c b/diff.c index e1bed8d85aafa1..2a0abf9dd5cbbd 100644 --- a/diff.c +++ b/diff.c @@ -4242,7 +4242,21 @@ static void builtin_diffstat(const char *name_a, const char *name_b, xecfg.ctxlen = o->context; xecfg.interhunkctxlen = o->interhunkcontext; xecfg.flags = XDL_EMIT_NO_HUNK_HDR; - if (xdi_diff_outf(&mf1, &mf2, NULL, + + if (p->line_ranges) { + struct line_range_filter lr_filter; + + line_range_filter_init(&lr_filter, p->line_ranges, + diffstat_consume, diffstat); + + /* Need hunk headers for post-image position tracking */ + xecfg.flags &= ~XDL_EMIT_NO_HUNK_HDR; + + if (line_range_filter_diff(&lr_filter, &mf1, &mf2, + &xpp, &xecfg)) + die("unable to generate diffstat for %s", + one->path); + } else if (xdi_diff_outf(&mf1, &mf2, NULL, diffstat_consume, diffstat, &xpp, &xecfg)) die("unable to generate diffstat for %s", one->path); diff --git a/revision.c b/revision.c index 6a8101e8b7ef5f..2c76e15778de32 100644 --- a/revision.c +++ b/revision.c @@ -3193,8 +3193,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT | DIFF_FORMAT_RAW | DIFF_FORMAT_NAME | - DIFF_FORMAT_NAME_STATUS | DIFF_FORMAT_SUMMARY)))) - die(_("-L does not yet support the requested diff format")); + DIFF_FORMAT_NAME_STATUS | DIFF_FORMAT_SUMMARY | + DIFF_FORMAT_NUMSTAT | DIFF_FORMAT_DIFFSTAT | + DIFF_FORMAT_SHORTSTAT)))) + die(_("-L does not support the requested diff format")); if (revs->expand_tabs_in_log < 0) revs->expand_tabs_in_log = revs->expand_tabs_in_log_default; diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index 6b5b846c0a5a8d..98822cf4e14cb1 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -176,24 +176,22 @@ test_expect_success '--name-status shows status and path' ' test_grep ! "^@@" actual ' -test_expect_success '--stat is not yet supported with -L' ' - test_must_fail git log -L1,24:b.c --stat 2>err && - test_grep "does not yet support" err -' - -test_expect_success '--numstat is not yet supported with -L' ' - test_must_fail git log -L1,24:b.c --numstat 2>err && - test_grep "does not yet support" err -' - -test_expect_success '--shortstat is not yet supported with -L' ' - test_must_fail git log -L1,24:b.c --shortstat 2>err && - test_grep "does not yet support" err -' - -test_expect_success '--dirstat is not yet supported with -L' ' +test_expect_success '--stat works with -L' ' + git log -L1,24:b.c --stat --format= >actual && + test_grep "b.c |" actual && + test_grep "file changed" actual && + test_grep ! "^diff --git" actual +' + +test_expect_success '--dirstat is not supported with -L' ' + # --dirstat is degenerate for line ranges (a single tracked + # file is always 100% of its directory) and its default mode + # is not range-scoped, so it is rejected like any other + # unsupported diff format. test_must_fail git log -L1,24:b.c --dirstat 2>err && - test_grep "does not yet support" err + test_grep "does not support" err && + test_must_fail git log -L1,24:b.c --dirstat=lines 2>err && + test_grep "does not support" err ' test_expect_success 'setup for checking fancy rename following' ' @@ -899,9 +897,9 @@ test_expect_success '-L with -S suppresses non-matching commits' ' test_cmp expect actual ' -test_expect_success '--full-diff is not yet supported with -L' ' +test_expect_success '--full-diff is not supported with -L' ' test_must_fail git log -L1,24:b.c --full-diff 2>err && - test_grep "does not yet support" err + test_grep "does not support" err ' test_expect_success '-L --oneline has no extra blank line before diff' ' @@ -912,6 +910,88 @@ test_expect_success '-L --oneline has no extra blank line before diff' ' test_grep "^diff --git" line2 ' +test_expect_success 'setup for stat range-scoping tests' ' + git checkout --orphan stat-scoping && + git reset --hard && + cat >file.c <<-\EOF && + int func1() + { + return F1; + } + + int func2() + { + return F2; + } + EOF + git add file.c && + test_tick && + git commit -m "Add func1() and func2()" && + + # Modify both functions in a single commit so that + # whole-file stats differ from range-scoped stats. + sed -e "s/F1/F1 + 1/" -e "s/F2/F2 + 2/" file.c >tmp && + mv tmp file.c && + git commit -a -m "Modify both functions" +' + +test_expect_success '--numstat counts only lines in tracked range' ' + # "Modify both functions" changes one line in func1 and one in + # func2. Whole-file numstat would show 2 added, 2 deleted. + # Range-scoped numstat for func2 should show only 1 and 1. + git log -L:func2:file.c --numstat --format=%s -1 >actual && + test_grep "Modify both functions" actual && + test_grep "^1 1 file.c$" actual +' + +test_expect_success '--numstat counts only additions for root commit' ' + # Root commit creates both func1 (4 lines) and func2 (4 lines). + # Whole-file numstat would show 9 lines added. Range-scoped + # numstat for func2 should show only 4. + git log -L:func2:file.c --numstat --format=%s >actual && + test_grep "Add func1() and func2()" actual && + test_grep "^4 0 file.c$" actual +' + +test_expect_success '--stat counts only lines in tracked range' ' + git log -L:func2:file.c --stat --format=%s -1 >actual && + test_grep "Modify both functions" actual && + test_grep "1 insertion" actual && + test_grep "1 deletion" actual +' + +test_expect_success '--shortstat counts only lines in tracked range' ' + # Same range-scoped counting as --numstat above. + git log -L:func2:file.c --shortstat --format=%s -1 >actual && + test_grep "Modify both functions" actual && + test_grep "1 insertion" actual && + test_grep "1 deletion" actual +' + +test_expect_success '--numstat matches patch line counts' ' + # Cross-check: count +/- lines in patch output and verify + # --numstat reports the same numbers. Uses the parallel-change + # branch which has renames and multiple commits, exercising the + # filter across nontrivial history. + git checkout parallel-change && + git log -M -L ":f:b.c" --format= -p >patch_out && + adds=$(grep "^+" patch_out | grep -vc "^+++") && + dels=$(grep "^-" patch_out | grep -vc "^---") && + git log -M -L ":f:b.c" --numstat --format= >stat_out && + awk "{a+=\$1; d+=\$2} END {print a, d}" stat_out >stat_counts && + echo "$adds $dels" >patch_counts && + test_cmp patch_counts stat_counts +' + +test_expect_success '-L multiple ranges with --numstat' ' + git checkout stat-scoping && + # Both func1 and func2 were modified by "Modify both functions". + # Track both with separate -L flags; stats should sum. + git log -L:func1:file.c -L:func2:file.c --numstat --format=%s -1 >actual && + test_grep "Modify both functions" actual && + test_grep "^2 2 file.c$" actual +' + test_expect_success '--summary shows new file on root commit' ' git checkout parent-oids && git log -L:func2:file.c --summary --format= >actual && From 73980c72043d9997fdb9c35238868f391a802ceb Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Tue, 12 May 2026 18:01:34 -0700 Subject: [PATCH 6/6] diff: support --check with -L line ranges builtin_checkdiff() runs its own xdiff pass to detect whitespace errors in newly added lines. When -L is active, the check should be scoped to the tracked line ranges rather than the whole file. Reuse the line_range_filter to wrap checkdiff_consume(), the same pattern already used for patch output and diffstat. The filter forwards only in-range lines for whitespace checking. checkdiff reports the file line number of each error, which it normally learns from the hunk header via checkdiff_consume_hunk(). The filter synthesizes its own hunk headers, so give it an optional hunk callback and route checkdiff_consume_hunk() through it; this sets the post-image position before the in-range lines are replayed. Without it the reported line numbers would count from the start of the range hunk rather than the start of the file. Add DIFF_FORMAT_CHECKDIFF to the -L output format allowlist in setup_revisions() so that -L --check is accepted, and list --check among the supported formats in the documentation. Add tests covering that whitespace errors are reported, scoped to the tracked range, and labeled with the correct file line number. Signed-off-by: Michael Montalbo --- Documentation/line-range-options.adoc | 2 +- diff.c | 37 ++++++++++++++++++++-- revision.c | 2 +- t/t4211-line-log.sh | 44 +++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 5 deletions(-) diff --git a/Documentation/line-range-options.adoc b/Documentation/line-range-options.adoc index 753a54f486198d..744582185706eb 100644 --- a/Documentation/line-range-options.adoc +++ b/Documentation/line-range-options.adoc @@ -10,7 +10,7 @@ You can specify this option more than once. Implies `--patch`. Patch output can be suppressed using `--no-patch`. Other diff formats are supported, including `--raw`, - `--name-only`, `--name-status`, `--summary`, + `--name-only`, `--name-status`, `--summary`, `--check`, `--stat`, `--numstat`, and `--shortstat`. The stat formats show range-scoped counts: only lines within the tracked range are counted. `--dirstat` is not supported: diff --git a/diff.c b/diff.c index 2a0abf9dd5cbbd..72fb849f685877 100644 --- a/diff.c +++ b/diff.c @@ -620,6 +620,12 @@ struct emit_callback { */ struct line_range_filter { xdiff_emit_line_fn orig_line_fn; + /* + * Optional; consumers that report file line numbers (e.g. + * checkdiff) need the synthetic hunk header to set their + * post-image position before in-range lines are replayed. + */ + xdiff_emit_hunk_fn orig_hunk_fn; void *orig_cb_data; const struct range_set *ranges; /* 0-based [start, end) */ unsigned int cur_range; /* index into the range_set */ @@ -2586,6 +2592,17 @@ static void flush_rhunk(struct line_range_filter *s) } strbuf_addch(&hdr, '\n'); + /* + * Inform a line-numbering consumer of the post-image position + * before replaying lines, mirroring the hunk callback xdiff + * would have issued for a non-scoped diff. + */ + if (s->orig_hunk_fn) + s->orig_hunk_fn(s->orig_cb_data, + old_begin, s->rhunk_old_count, + new_begin, s->rhunk_new_count, + s->func, s->funclen); + s->ret = s->orig_line_fn(s->orig_cb_data, hdr.buf, hdr.len); strbuf_release(&hdr); @@ -4290,7 +4307,8 @@ static void builtin_checkdiff(const char *name_a, const char *name_b, const char *attr_path, struct diff_filespec *one, struct diff_filespec *two, - struct diff_options *o) + struct diff_options *o, + const struct range_set *line_ranges) { mmfile_t mf1, mf2; struct checkdiff_t data; @@ -4330,7 +4348,19 @@ static void builtin_checkdiff(const char *name_a, const char *name_b, memset(&xecfg, 0, sizeof(xecfg)); xecfg.ctxlen = 1; /* at least one context line */ xpp.flags = 0; - if (xdi_diff_outf(&mf1, &mf2, checkdiff_consume_hunk, + + if (line_ranges) { + struct line_range_filter lr_filter; + + line_range_filter_init(&lr_filter, line_ranges, + checkdiff_consume, &data); + lr_filter.orig_hunk_fn = checkdiff_consume_hunk; + + if (line_range_filter_diff(&lr_filter, &mf1, &mf2, + &xpp, &xecfg)) + die("unable to generate checkdiff for %s", + one->path); + } else if (xdi_diff_outf(&mf1, &mf2, checkdiff_consume_hunk, checkdiff_consume, &data, &xpp, &xecfg)) die("unable to generate checkdiff for %s", one->path); @@ -5135,7 +5165,8 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o) diff_fill_oid_info(p->one, o->repo->index); diff_fill_oid_info(p->two, o->repo->index); - builtin_checkdiff(name, other, attr_path, p->one, p->two, o); + builtin_checkdiff(name, other, attr_path, p->one, p->two, o, + p->line_ranges); } void repo_diff_setup(struct repository *r, struct diff_options *options) diff --git a/revision.c b/revision.c index 2c76e15778de32..7abb287451ba14 100644 --- a/revision.c +++ b/revision.c @@ -3195,7 +3195,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s DIFF_FORMAT_RAW | DIFF_FORMAT_NAME | DIFF_FORMAT_NAME_STATUS | DIFF_FORMAT_SUMMARY | DIFF_FORMAT_NUMSTAT | DIFF_FORMAT_DIFFSTAT | - DIFF_FORMAT_SHORTSTAT)))) + DIFF_FORMAT_SHORTSTAT | DIFF_FORMAT_CHECKDIFF)))) die(_("-L does not support the requested diff format")); if (revs->expand_tabs_in_log < 0) diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index 98822cf4e14cb1..70d297e8030fc0 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -998,4 +998,48 @@ test_expect_success '--summary shows new file on root commit' ' test_grep "create mode 100644 file.c" actual ' +test_expect_success 'setup for --check test' ' + git checkout --orphan check-test && + git reset --hard && + cat >check.c <<-\EOF && + void tracked() + { + return; + } + + void other() + { + return; + } + EOF + git add check.c && + test_tick && + git commit -m "add check.c" && + # Introduce trailing whitespace errors in both functions + sed "s/return;/return; /" check.c >check.c.tmp && + mv check.c.tmp check.c && + git commit -a -m "introduce trailing whitespace" +' + +test_expect_success '--check reports whitespace errors in tracked range' ' + test_must_fail git log -L:tracked:check.c --check --format= >actual && + test_grep "trailing whitespace" actual +' + +test_expect_success '--check scoped to tracked range' ' + test_must_fail git log -L:tracked:check.c --check --format= >actual && + # The error is within tracked(); other() errors are excluded + test_grep "trailing whitespace" actual && + # Only one error reported (tracked function only, not other()) + test $(grep -c "trailing whitespace" actual) = 1 +' + +test_expect_success '--check reports the file line number, not a range offset' ' + # The trailing whitespace is on the third line of check.c. + # Report the real file line number, not a count from the start + # of the range hunk. + test_must_fail git log -L:tracked:check.c --check --format= >actual && + test_grep "check.c:3: trailing whitespace" actual +' + test_done