From 331c6c92f3b54f7ee95ef5fe4db375b99b9be146 Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Mon, 15 Jun 2026 21:59:45 -0700 Subject: [PATCH 1/7] diff: rename line-range filter struct and clarify fields diff's line-range filtering logic uses the line_range_callback struct to represent filtering state. However, this name does not clearly reflect the role it plays. This is especially relevant as we expand diff's line-range filtering to work with more options, including --stat and -G. Also, line_range_callback's fields are terse, while the comment explaining line_range_callback is verbose and out of place compared to its surroundings. Rename line_range_callback to line_range_filter, and replace the verbose comment with a concise one, instead preferring descriptive field and variable names that are self-explanatory over comments. No logical behavior change. Some fields are grouped under a new struct in the newly renamed line_range_filter. Everything else is just a rename. Signed-off-by: Michael Montalbo --- diff.c | 270 ++++++++++++++++++++++++--------------------------------- 1 file changed, 114 insertions(+), 156 deletions(-) diff --git a/diff.c b/diff.c index 414532d09f5ba3..679a0e27d4d9c5 100644 --- a/diff.c +++ b/diff.c @@ -610,49 +610,31 @@ 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 - * 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). + * Filter the line ranges that are emitted by diff. */ -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) */ - unsigned int cur_range; /* index into the range_set */ - - /* Post/pre-image line counters (1-based, set from hunk headers) */ - long lno_post; - long lno_pre; + const struct range_set *range_sets_to_filter_by; + unsigned int range_set_idx; + + struct { + char func_name[80]; + long func_name_len; + long old_begin, old_count; + long new_begin, new_count; + long lno_in_preimage; + long lno_in_postimage; + struct strbuf lines; + int active; + int has_changes; + } accumulating_hunk; - /* - * Function name from most recent xdiff hunk header; - * size matches struct func_line.buf in xdiff/xemit.c. - */ - char func[80]; - long funclen; - - /* Range hunk being accumulated for the current range */ - struct strbuf rhunk; - long rhunk_old_begin, rhunk_old_count; - long rhunk_new_begin, rhunk_new_count; - 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 */ + long pending_rm_pre_begin; - int ret; /* latched error from orig_line_fn */ + int ret; }; static int count_lines(const char *data, int size) @@ -2540,69 +2522,59 @@ 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) +static void discard_pending_rm(struct line_range_filter *filter) { - strbuf_reset(&s->pending_rm); - s->pending_rm_count = 0; + strbuf_reset(&filter->pending_rm); + filter->pending_rm_count = 0; } -static void flush_rhunk(struct line_range_callback *s) +static void flush_range_hunk(struct line_range_filter *filter) { struct strbuf hdr = STRBUF_INIT; - const char *p, *end; + const char *line_buf, *line_buf_end; - if (!s->rhunk_active || s->ret) + if (!filter->accumulating_hunk.active || filter->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); + if (filter->pending_rm_count) { + strbuf_addbuf(&filter->accumulating_hunk.lines, &filter->pending_rm); + filter->accumulating_hunk.old_count += filter->pending_rm_count; + filter->accumulating_hunk.has_changes = 1; + discard_pending_rm(filter); } - /* - * Suppress context-only hunks: they contain no actual changes - * and would just be noise. This can happen when the inflated - * ctxlen causes xdiff to emit context covering a range that - * has no changes in this commit. - */ - if (!s->rhunk_has_changes) { - s->rhunk_active = 0; - strbuf_reset(&s->rhunk); + if (!filter->accumulating_hunk.has_changes) { + filter->accumulating_hunk.active = 0; + strbuf_reset(&filter->accumulating_hunk.lines); return; } strbuf_addf(&hdr, "@@ -%ld,%ld +%ld,%ld @@", - s->rhunk_old_begin, s->rhunk_old_count, - s->rhunk_new_begin, s->rhunk_new_count); - if (s->funclen > 0) { + filter->accumulating_hunk.old_begin, filter->accumulating_hunk.old_count, + filter->accumulating_hunk.new_begin, filter->accumulating_hunk.new_count); + if (filter->accumulating_hunk.func_name_len > 0) { strbuf_addch(&hdr, ' '); - strbuf_add(&hdr, s->func, s->funclen); + strbuf_add(&hdr, filter->accumulating_hunk.func_name, + filter->accumulating_hunk.func_name_len); } strbuf_addch(&hdr, '\n'); - s->ret = s->orig_line_fn(s->orig_cb_data, hdr.buf, hdr.len); + filter->ret = filter->orig_line_fn(filter->orig_cb_data, hdr.buf, hdr.len); strbuf_release(&hdr); - /* - * Replay buffered lines one at a time through fn_out_consume. - * The cast discards const because xdiff_emit_line_fn takes - * char *, though fn_out_consume does not modify the buffer. - */ - p = s->rhunk.buf; - end = p + s->rhunk.len; - while (!s->ret && p < end) { - const char *eol = memchr(p, '\n', end - p); - unsigned long line_len = eol ? (unsigned long)(eol - p + 1) - : (unsigned long)(end - p); - s->ret = s->orig_line_fn(s->orig_cb_data, (char *)p, line_len); - p += line_len; + line_buf = filter->accumulating_hunk.lines.buf; + line_buf_end = line_buf + filter->accumulating_hunk.lines.len; + while (!filter->ret && line_buf < line_buf_end) { + const char *eol = memchr(line_buf, '\n', line_buf_end - line_buf); + unsigned long line_len = eol ? (unsigned long)(eol - line_buf + 1) + : (unsigned long)(line_buf_end - line_buf); + filter->ret = filter->orig_line_fn(filter->orig_cb_data, + (char *)line_buf, line_len); + line_buf += line_len; } - s->rhunk_active = 0; - strbuf_reset(&s->rhunk); + filter->accumulating_hunk.active = 0; + strbuf_reset(&filter->accumulating_hunk.lines); } static void line_range_hunk_fn(void *data, @@ -2610,116 +2582,102 @@ 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 *filter = 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; + filter->accumulating_hunk.lno_in_postimage = new_begin; + filter->accumulating_hunk.lno_in_preimage = old_begin; if (funclen > 0) { - if (funclen > (long)sizeof(s->func)) - funclen = sizeof(s->func); - memcpy(s->func, func, funclen); + if (funclen > (long)sizeof(filter->accumulating_hunk.func_name)) + funclen = sizeof(filter->accumulating_hunk.func_name); + memcpy(filter->accumulating_hunk.func_name, func, funclen); } - s->funclen = funclen; + filter->accumulating_hunk.func_name_len = funclen; } static int line_range_line_fn(void *priv, char *line, unsigned long len) { - struct line_range_callback *s = priv; + struct line_range_filter *filter = priv; const struct range *cur; - long lno_0, cur_pre; + long idx_in_postimage, cur_pre; - if (s->ret) - return s->ret; + if (filter->ret) + return filter->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 (!filter->pending_rm_count) + filter->pending_rm_pre_begin = + filter->accumulating_hunk.lno_in_preimage; + filter->accumulating_hunk.lno_in_preimage++; + strbuf_add(&filter->pending_rm, line, len); + filter->pending_rm_count++; + return filter->ret; } if (line[0] == '\\') { - if (s->pending_rm_count) - strbuf_add(&s->pending_rm, line, len); - else if (s->rhunk_active) - strbuf_add(&s->rhunk, line, len); - /* otherwise outside tracked range; drop silently */ - return s->ret; + if (filter->pending_rm_count) + strbuf_add(&filter->pending_rm, line, len); + else if (filter->accumulating_hunk.active) + strbuf_add(&filter->accumulating_hunk.lines, line, len); + return filter->ret; } if (line[0] != '+' && line[0] != ' ') BUG("unexpected diff line type '%c'", line[0]); - lno_0 = s->lno_post - 1; - cur_pre = s->lno_pre; /* save before advancing for context lines */ - s->lno_post++; + idx_in_postimage = filter->accumulating_hunk.lno_in_postimage - 1; + cur_pre = filter->accumulating_hunk.lno_in_preimage; + filter->accumulating_hunk.lno_in_postimage++; if (line[0] == ' ') - s->lno_pre++; + filter->accumulating_hunk.lno_in_preimage++; - /* Advance past ranges we've passed */ - 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++; + while (filter->range_set_idx < filter->range_sets_to_filter_by->nr && + idx_in_postimage >= + filter->range_sets_to_filter_by->ranges[filter->range_set_idx].end) { + if (filter->accumulating_hunk.active) + flush_range_hunk(filter); + discard_pending_rm(filter); + filter->range_set_idx++; } - /* Past all ranges */ - if (s->cur_range >= s->ranges->nr) { - discard_pending_rm(s); - return s->ret; + if (filter->range_set_idx >= filter->range_sets_to_filter_by->nr) { + discard_pending_rm(filter); + return filter->ret; } - cur = &s->ranges->ranges[s->cur_range]; + cur = &filter->range_sets_to_filter_by->ranges[filter->range_set_idx]; - /* Before current range */ - if (lno_0 < cur->start) { - discard_pending_rm(s); - return s->ret; + if (idx_in_postimage < cur->start) { + discard_pending_rm(filter); + return filter->ret; } - /* In range so start a new range hunk if needed */ - 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_count = 0; - s->rhunk_new_count = 0; - strbuf_reset(&s->rhunk); + if (!filter->accumulating_hunk.active) { + filter->accumulating_hunk.active = 1; + filter->accumulating_hunk.has_changes = 0; + filter->accumulating_hunk.new_begin = idx_in_postimage + 1; + filter->accumulating_hunk.old_begin = filter->pending_rm_count + ? filter->pending_rm_pre_begin : cur_pre; + filter->accumulating_hunk.old_count = 0; + filter->accumulating_hunk.new_count = 0; + strbuf_reset(&filter->accumulating_hunk.lines); } - /* 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); + if (filter->pending_rm_count) { + strbuf_addbuf(&filter->accumulating_hunk.lines, &filter->pending_rm); + filter->accumulating_hunk.old_count += filter->pending_rm_count; + filter->accumulating_hunk.has_changes = 1; + discard_pending_rm(filter); } - strbuf_add(&s->rhunk, line, len); - s->rhunk_new_count++; + strbuf_add(&filter->accumulating_hunk.lines, line, len); + filter->accumulating_hunk.new_count++; if (line[0] == '+') - s->rhunk_has_changes = 1; + filter->accumulating_hunk.has_changes = 1; else - s->rhunk_old_count++; + filter->accumulating_hunk.old_count++; - return s->ret; + return filter->ret; } static void pprint_rename(struct strbuf *name, const char *a, const char *b) @@ -4066,15 +4024,15 @@ 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; 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); + lr_state.range_sets_to_filter_by = line_ranges; + strbuf_init(&lr_state.accumulating_hunk.lines, 0); strbuf_init(&lr_state.pending_rm, 0); /* @@ -4105,11 +4063,11 @@ static void builtin_diff(const char *name_a, die("unable to generate diff for %s", one->path); - flush_rhunk(&lr_state); + flush_range_hunk(&lr_state); if (lr_state.ret) die("unable to generate diff for %s", one->path); - strbuf_release(&lr_state.rhunk); + strbuf_release(&lr_state.accumulating_hunk.lines); strbuf_release(&lr_state.pending_rm); } else if (xdi_diff_outf(&mf1, &mf2, NULL, fn_out_consume, &ecbdata, &xpp, &xecfg)) From 020e07c0ea82c732bdb0702d2ec211503bd6f6e6 Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Mon, 15 Jun 2026 21:29:16 -0700 Subject: [PATCH 2/7] diff: simplify the line-range filter by classifying removals immediately Currently, the diff line-range filter buffers preimage removal lines until a postimage line arrives. That line's number confirms whether the preimage line falls in a relevant range. However, storing preimage lines in a separate buffer is unnecessary. Worse, the logic has a bug: a preimage line outside the target range is included when it immediately follows an in-range postimage line. Preimage lines will always precede their postimage counterpart both in content line number and emission order from xdiff's line callback function. So preimage lines can share the postimage buffer. The filter flushes them based on whether the postimage lines fall within the target range. Remove logic related to storing preimage lines in a separate "removal" buffer and prepending them to the accumulating_hunk's line buffer. Instead, store those lines in the accumulating_hunk's line_buffer immediately and flush everything as appropriate based on postimage line numbers that arrive. This resolves the bug by construction. Also, calculate the old and new line counts for the diff hunk header when flushing rather than storing counters in line_range_filter to simplify state management further. Add a test to t/t4211-line-log.sh that verifies the preimage line emission bug is fixed. Signed-off-by: Michael Montalbo --- diff.c | 121 +++++++++++++++++--------------------------- t/t4211-line-log.sh | 31 ++++++++++++ 2 files changed, 78 insertions(+), 74 deletions(-) diff --git a/diff.c b/diff.c index 679a0e27d4d9c5..c94ddbebe51be5 100644 --- a/diff.c +++ b/diff.c @@ -621,19 +621,14 @@ struct line_range_filter { struct { char func_name[80]; long func_name_len; - long old_begin, old_count; - long new_begin, new_count; + long old_begin; + long new_begin; long lno_in_preimage; long lno_in_postimage; struct strbuf lines; int active; - int has_changes; } accumulating_hunk; - struct strbuf pending_rm; - int pending_rm_count; - long pending_rm_pre_begin; - int ret; }; @@ -2522,40 +2517,56 @@ static int quick_consume(void *priv, char *line UNUSED, unsigned long len UNUSED return 1; } -static void discard_pending_rm(struct line_range_filter *filter) +static void begin_range_hunk(struct line_range_filter *filter) { - strbuf_reset(&filter->pending_rm); - filter->pending_rm_count = 0; + filter->accumulating_hunk.active = 1; + filter->accumulating_hunk.new_begin = filter->accumulating_hunk.lno_in_postimage; + filter->accumulating_hunk.old_begin = filter->accumulating_hunk.lno_in_preimage; + strbuf_reset(&filter->accumulating_hunk.lines); } static void flush_range_hunk(struct line_range_filter *filter) { struct strbuf hdr = STRBUF_INIT; const char *line_buf, *line_buf_end; + long old_count = 0, new_count = 0; + int has_changes = 0; if (!filter->accumulating_hunk.active || filter->ret) return; - if (filter->pending_rm_count) { - strbuf_addbuf(&filter->accumulating_hunk.lines, &filter->pending_rm); - filter->accumulating_hunk.old_count += filter->pending_rm_count; - filter->accumulating_hunk.has_changes = 1; - discard_pending_rm(filter); + line_buf = filter->accumulating_hunk.lines.buf; + line_buf_end = line_buf + filter->accumulating_hunk.lines.len; + while (line_buf < line_buf_end) { + const char *eol = memchr(line_buf, '\n', line_buf_end - line_buf); + if (*line_buf == ' ') { + old_count++; + new_count++; + } + else if (*line_buf == '-') { + old_count++; + has_changes = 1; + } + else if (*line_buf == '+') { + new_count++; + has_changes = 1; + } + line_buf = eol ? eol + 1 : line_buf_end; } - if (!filter->accumulating_hunk.has_changes) { + if (!has_changes) { filter->accumulating_hunk.active = 0; strbuf_reset(&filter->accumulating_hunk.lines); return; } strbuf_addf(&hdr, "@@ -%ld,%ld +%ld,%ld @@", - filter->accumulating_hunk.old_begin, filter->accumulating_hunk.old_count, - filter->accumulating_hunk.new_begin, filter->accumulating_hunk.new_count); + filter->accumulating_hunk.old_begin, old_count, + filter->accumulating_hunk.new_begin, new_count); if (filter->accumulating_hunk.func_name_len > 0) { strbuf_addch(&hdr, ' '); strbuf_add(&hdr, filter->accumulating_hunk.func_name, - filter->accumulating_hunk.func_name_len); + filter->accumulating_hunk.func_name_len); } strbuf_addch(&hdr, '\n'); @@ -2598,84 +2609,48 @@ static void line_range_hunk_fn(void *data, static int line_range_line_fn(void *priv, char *line, unsigned long len) { struct line_range_filter *filter = priv; - const struct range *cur; - long idx_in_postimage, cur_pre; + long idx_in_postimage; + int in_range; if (filter->ret) return filter->ret; - if (line[0] == '-') { - if (!filter->pending_rm_count) - filter->pending_rm_pre_begin = - filter->accumulating_hunk.lno_in_preimage; - filter->accumulating_hunk.lno_in_preimage++; - strbuf_add(&filter->pending_rm, line, len); - filter->pending_rm_count++; - return filter->ret; - } - if (line[0] == '\\') { - if (filter->pending_rm_count) - strbuf_add(&filter->pending_rm, line, len); - else if (filter->accumulating_hunk.active) + if (filter->accumulating_hunk.active) strbuf_add(&filter->accumulating_hunk.lines, line, len); return filter->ret; } - if (line[0] != '+' && line[0] != ' ') + if (line[0] != '+' && line[0] != ' ' && line[0] != '-') BUG("unexpected diff line type '%c'", line[0]); idx_in_postimage = filter->accumulating_hunk.lno_in_postimage - 1; - cur_pre = filter->accumulating_hunk.lno_in_preimage; - filter->accumulating_hunk.lno_in_postimage++; - if (line[0] == ' ') - filter->accumulating_hunk.lno_in_preimage++; while (filter->range_set_idx < filter->range_sets_to_filter_by->nr && idx_in_postimage >= filter->range_sets_to_filter_by->ranges[filter->range_set_idx].end) { if (filter->accumulating_hunk.active) flush_range_hunk(filter); - discard_pending_rm(filter); filter->range_set_idx++; } - if (filter->range_set_idx >= filter->range_sets_to_filter_by->nr) { - discard_pending_rm(filter); - return filter->ret; - } - - cur = &filter->range_sets_to_filter_by->ranges[filter->range_set_idx]; - - if (idx_in_postimage < cur->start) { - discard_pending_rm(filter); - return filter->ret; - } + in_range = filter->range_set_idx < filter->range_sets_to_filter_by->nr && + idx_in_postimage >= + filter->range_sets_to_filter_by->ranges[filter->range_set_idx].start && + idx_in_postimage < + filter->range_sets_to_filter_by->ranges[filter->range_set_idx].end; - if (!filter->accumulating_hunk.active) { - filter->accumulating_hunk.active = 1; - filter->accumulating_hunk.has_changes = 0; - filter->accumulating_hunk.new_begin = idx_in_postimage + 1; - filter->accumulating_hunk.old_begin = filter->pending_rm_count - ? filter->pending_rm_pre_begin : cur_pre; - filter->accumulating_hunk.old_count = 0; - filter->accumulating_hunk.new_count = 0; - strbuf_reset(&filter->accumulating_hunk.lines); - } + if (in_range) { + if (!filter->accumulating_hunk.active) + begin_range_hunk(filter); - if (filter->pending_rm_count) { - strbuf_addbuf(&filter->accumulating_hunk.lines, &filter->pending_rm); - filter->accumulating_hunk.old_count += filter->pending_rm_count; - filter->accumulating_hunk.has_changes = 1; - discard_pending_rm(filter); + strbuf_add(&filter->accumulating_hunk.lines, line, len); } - strbuf_add(&filter->accumulating_hunk.lines, line, len); - filter->accumulating_hunk.new_count++; - if (line[0] == '+') - filter->accumulating_hunk.has_changes = 1; - else - filter->accumulating_hunk.old_count++; + if (line[0] == ' ' || line[0] == '+') + filter->accumulating_hunk.lno_in_postimage++; + if (line[0] == ' ' || line[0] == '-') + filter->accumulating_hunk.lno_in_preimage++; return filter->ret; } @@ -4033,7 +4008,6 @@ static void builtin_diff(const char *name_a, lr_state.orig_cb_data = &ecbdata; lr_state.range_sets_to_filter_by = line_ranges; strbuf_init(&lr_state.accumulating_hunk.lines, 0); - strbuf_init(&lr_state.pending_rm, 0); /* * Inflate ctxlen so that all changes within @@ -4068,7 +4042,6 @@ static void builtin_diff(const char *name_a, die("unable to generate diff for %s", one->path); strbuf_release(&lr_state.accumulating_hunk.lines); - 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 d0a834ed8f5bba..233dc232e3150b 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -738,6 +738,37 @@ test_expect_success '-L with -G filters to diff-text matches' ' test_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 outside tracked range + EOF + git add file.c && + test_tick && + git commit -m "add file with trailing comment" && + # Remove the trailing comment AND modify tracked() so there + # is a modification to the line range we track and a + # modification to the following line, which we do not track. + 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 && + test_grep "return 2" actual && + test_grep ! "trailing comment" 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 2081d5d257d65b7b823fc46777e952a2f650e6dd Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Mon, 15 Jun 2026 21:33:25 -0700 Subject: [PATCH 3/7] diff: emit -L hunk headers via xdiff's formatter Currently, diff's line-range filter implements its own method for emitting diff hunk headers. This mostly matches what xdiff itself outputs, but there is a discrepancy for postimage or preimage sides with 0 line changes. For a side with no lines (count 0), the begin is the line before the change. The header omits the line count of 1. Rather than fix this case in the line-range implementation, expose the function xdiff uses to emit its headers. Reusing it keeps the header format consistent with and without -L. Update test scenarios and fixtures to reflect the now consistent header format. Signed-off-by: Michael Montalbo --- diff.c | 21 ++++++++------------- t/t4211/sha1/expect.no-assertion-error | 2 +- t/t4211/sha1/expect.vanishes-early | 6 +++--- t/t4211/sha256/expect.no-assertion-error | 2 +- t/t4211/sha256/expect.vanishes-early | 6 +++--- xdiff-interface.c | 19 +++++++++++++++++++ xdiff-interface.h | 13 +++++++++++++ 7 files changed, 48 insertions(+), 21 deletions(-) diff --git a/diff.c b/diff.c index c94ddbebe51be5..cb1a85c6247350 100644 --- a/diff.c +++ b/diff.c @@ -2560,15 +2560,10 @@ static void flush_range_hunk(struct line_range_filter *filter) return; } - strbuf_addf(&hdr, "@@ -%ld,%ld +%ld,%ld @@", - filter->accumulating_hunk.old_begin, old_count, - filter->accumulating_hunk.new_begin, new_count); - if (filter->accumulating_hunk.func_name_len > 0) { - strbuf_addch(&hdr, ' '); - strbuf_add(&hdr, filter->accumulating_hunk.func_name, - filter->accumulating_hunk.func_name_len); - } - strbuf_addch(&hdr, '\n'); + xdiff_emit_hunk_header(&hdr, filter->accumulating_hunk.old_begin, old_count, + filter->accumulating_hunk.new_begin, new_count, + filter->accumulating_hunk.func_name, + filter->accumulating_hunk.func_name_len); filter->ret = filter->orig_line_fn(filter->orig_cb_data, hdr.buf, hdr.len); strbuf_release(&hdr); @@ -2589,14 +2584,14 @@ static void flush_range_hunk(struct line_range_filter *filter) } static void line_range_hunk_fn(void *data, - long old_begin, long old_nr UNUSED, - long new_begin, long new_nr UNUSED, + long old_begin, long old_nr, + long new_begin, long new_nr, const char *func, long funclen) { struct line_range_filter *filter = data; - filter->accumulating_hunk.lno_in_postimage = new_begin; - filter->accumulating_hunk.lno_in_preimage = old_begin; + filter->accumulating_hunk.lno_in_postimage = new_nr ? new_begin : new_begin + 1; + filter->accumulating_hunk.lno_in_preimage = old_nr ? old_begin : old_begin + 1; if (funclen > 0) { if (funclen > (long)sizeof(filter->accumulating_hunk.func_name)) 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..e4b1a201d5d254 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 @@ int main () +/* incomplete lines are bad! */ commit 100b61a6f2f720f812620a9d10afb3a960ccb73c @@ -21,7 +21,7 @@ diff --git a/a.c b/a.c index 5e709a1..0b9cae5 100644 --- a/a.c +++ b/a.c -@@ -22,1 +22,1 @@ int main () +@@ -22 +22 @@ int main () -} +} \ No newline at end of file @@ -37,5 +37,5 @@ new file mode 100644 index 0000000..444e415 --- /dev/null +++ b/a.c -@@ -0,0 +20,1 @@ +@@ -0,0 +20 @@ +} 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..263fc9eaace442 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 @@ int main () +/* incomplete lines are bad! */ commit 29f32ac3141c48b22803e5c4127b719917b67d0f8ca8c5248bebfa2a19f7da10 @@ -21,7 +21,7 @@ diff --git a/a.c b/a.c index d325124..e4fa1d8 100644 --- a/a.c +++ b/a.c -@@ -22,1 +22,1 @@ int main () +@@ -22 +22 @@ int main () -} +} \ No newline at end of file @@ -37,5 +37,5 @@ new file mode 100644 index 0000000..9f550c3 --- /dev/null +++ b/a.c -@@ -0,0 +20,1 @@ +@@ -0,0 +20 @@ +} diff --git a/xdiff-interface.c b/xdiff-interface.c index db6938689f0a9e..686db8bb8b6710 100644 --- a/xdiff-interface.c +++ b/xdiff-interface.c @@ -91,6 +91,25 @@ static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf) return 0; } +static int strbuf_out_line(void *priv, mmbuffer_t *mb, int nbuf) +{ + struct strbuf *out = priv; + int i; + for (i = 0; i < nbuf; i++) + strbuf_add(out, mb[i].ptr, mb[i].size); + return 0; +} + +void xdiff_emit_hunk_header(struct strbuf *out, + long old_begin, long old_count, + long new_begin, long new_count, + const char *func, long funclen) +{ + xdemitcb_t ecb = { .priv = out, .out_line = strbuf_out_line }; + xdl_emit_hunk_hdr(old_begin, old_count, new_begin, new_count, + func, funclen, &ecb); +} + /* * Trim down common substring at the end of the buffers, * but end on a complete line. diff --git a/xdiff-interface.h b/xdiff-interface.h index ce54e1c0e002f8..24284566292c3d 100644 --- a/xdiff-interface.h +++ b/xdiff-interface.h @@ -76,4 +76,17 @@ int xdiff_compare_lines(const char *l1, long s1, */ unsigned long xdiff_hash_string(const char *s, size_t len, long flags); +struct strbuf; + +/* + * Append a unified-diff hunk header to `out`, e.g. + * "@@ - + @@ func\n". The header comes from wrapping xdiff's + * own hunk-header emitter, so it matches what a normal diff would + * produce for the given line number begins and line counts. + */ +void xdiff_emit_hunk_header(struct strbuf *out, + long old_begin, long old_nr, + long new_begin, long new_nr, + const char *func, long funclen); + #endif From 6b13c13ae72a24504aaa23be8d63571e6197aa1e Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Mon, 15 Jun 2026 21:35:31 -0700 Subject: [PATCH 4/7] diff: extract a line-range diff helper for reuse Extract logic for initializing the line-range filter and running a diff for a specific line range. This logic is needed for any diff that targets a line range independent of the current patch display path. The subsequent commits use this logic to enable additional line range targeted diff modes. No logical behavior change. Signed-off-by: Michael Montalbo --- diff.c | 87 ++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 48 insertions(+), 39 deletions(-) diff --git a/diff.c b/diff.c index cb1a85c6247350..a7604a773a38be 100644 --- a/diff.c +++ b/diff.c @@ -2517,6 +2517,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 *filter, + const struct range_set *ranges, + xdiff_emit_line_fn line_fn, + void *cb_data) +{ + memset(filter, 0, sizeof(*filter)); + filter->orig_line_fn = line_fn; + filter->orig_cb_data = cb_data; + filter->range_sets_to_filter_by = ranges; + strbuf_init(&filter->accumulating_hunk.lines, 0); +} + static void begin_range_hunk(struct line_range_filter *filter) { filter->accumulating_hunk.active = 1; @@ -2650,6 +2662,37 @@ static int line_range_line_fn(void *priv, char *line, unsigned long len) return filter->ret; } + +static int line_range_filter_diff(struct line_range_filter *filter, + mmfile_t *mf1, mmfile_t *mf2, + xpparam_t *xpp, xdemitconf_t *xecfg) +{ + const struct range_set *ranges = filter->range_sets_to_filter_by; + long max_span = 0; + unsigned int i; + int ret; + + for (i = 0; i < ranges->nr; i++) { + long span = ranges->ranges[i].end - ranges->ranges[i].start; + if (span > max_span) + max_span = span; + } + if (max_span > xecfg->ctxlen) + xecfg->ctxlen = max_span; + + /* the filter seeds its per-image position from hunk headers */ + xecfg->flags &= ~XDL_EMIT_NO_HUNK_HDR; + + ret = xdi_diff_outf(mf1, mf2, line_range_hunk_fn, + line_range_line_fn, filter, xpp, xecfg); + if (!ret) { + flush_range_hunk(filter); + ret = filter->ret; + } + strbuf_release(&filter->accumulating_hunk.lines); + return ret; +} + static void pprint_rename(struct strbuf *name, const char *a, const char *b) { const char *old_name = a; @@ -3994,49 +4037,15 @@ 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_filter lr_state; - unsigned int i; - long max_span = 0; + struct line_range_filter lr_filter; - memset(&lr_state, 0, sizeof(lr_state)); - lr_state.orig_line_fn = fn_out_consume; - lr_state.orig_cb_data = &ecbdata; - lr_state.range_sets_to_filter_by = line_ranges; - strbuf_init(&lr_state.accumulating_hunk.lines, 0); - - /* - * Inflate ctxlen so that all changes within - * any single range are merged into one xdiff - * hunk and the inter-change context is emitted. - * The callback clips back to range boundaries. - * - * The optimal ctxlen depends on where changes - * fall within the range, which is only known - * after xdiff runs; the max range span is the - * upper bound that guarantees correctness in a - * single pass. - */ - for (i = 0; i < line_ranges->nr; i++) { - long span = line_ranges->ranges[i].end - - line_ranges->ranges[i].start; - if (span > max_span) - max_span = span; - } - 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); + line_range_filter_init(&lr_filter, line_ranges, + fn_out_consume, &ecbdata); - flush_range_hunk(&lr_state); - if (lr_state.ret) + if (line_range_filter_diff(&lr_filter, &mf1, &mf2, + &xpp, &xecfg)) die("unable to generate diff for %s", one->path); - strbuf_release(&lr_state.accumulating_hunk.lines); } else if (xdi_diff_outf(&mf1, &mf2, NULL, fn_out_consume, &ecbdata, &xpp, &xecfg)) die("unable to generate diff for %s", one->path); From f12b61b9a408d23b8d8741571a9e9e030a69b6e0 Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Mon, 15 Jun 2026 21:37:10 -0700 Subject: [PATCH 5/7] diff: support stat formats with -L Reuse the line_range_filter in builtin_diffstat() so -L supports the stat formats and add tests verifying the new behavior. Ungate the newly enabled options and drop "yet" from the generic -L rejection message ("does not yet support the requested diff format"). Some rejected formats do not fit -L at all, so "yet" wrongly implies they are all awaiting support. Signed-off-by: Michael Montalbo --- Documentation/line-range-options.adoc | 10 +- diff.c | 13 ++- revision.c | 6 +- t/t4211-line-log.sh | 130 ++++++++++++++++++++++---- 4 files changed, 133 insertions(+), 26 deletions(-) diff --git a/Documentation/line-range-options.adoc b/Documentation/line-range-options.adoc index 72f639b5e79ea4..b3e8b5c62ce30a 100644 --- a/Documentation/line-range-options.adoc +++ b/Documentation/line-range-options.adoc @@ -9,10 +9,12 @@ __ 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. + The following non-patch diff formats are supported: `--raw`, + `--name-only`, `--name-status`, `--summary`, `--stat`, `--numstat`, + and `--shortstat`. The stat formats count only lines within the tracked + range. `--dirstat` is not supported with `-L`: it summarizes change as each + directory's share of the total churn, not as counts for the tracked lines. + 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 a7604a773a38be..4a30d7b6310fdb 100644 --- a/diff.c +++ b/diff.c @@ -4162,7 +4162,18 @@ 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); + + 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 35afe52208e710..4639c0df8e8501 100644 --- a/revision.c +++ b/revision.c @@ -3229,8 +3229,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 233dc232e3150b..4e8f71c28987be 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -176,24 +176,9 @@ 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 '--dirstat is not supported with -L' ' 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_expect_success 'setup for checking fancy rename following' ' @@ -793,9 +778,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' ' @@ -806,6 +791,113 @@ test_expect_success '-L --oneline has no extra blank line before diff' ' test_grep "^diff --git" line2 ' +test_expect_success 'setup for -L stat tests' ' + git checkout --orphan stat-range && + git reset --hard && + cat >file.c <<-\EOF && + int func1() + { + return F1; + } + + int tracked_fn() + { + return F2; + } + EOF + git add file.c && + test_tick && + git commit -m "Add func1() and tracked_fn()" && + + # Modify both functions so whole-file stats (2 added, 2 deleted) + # differ from the tracked range of tracked_fn (1 and 1). + 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 '-L --numstat limits counts to the tracked range' ' + git log -L:tracked_fn:file.c --numstat --format=%s >actual && + cat >expect <<-\EOF && + Modify both functions + + 1 1 file.c + Add func1() and tracked_fn() + + 4 0 file.c + EOF + test_cmp expect actual +' + +test_expect_success '-L --stat and --shortstat limit counts to the tracked range' ' + git log -L:tracked_fn:file.c --stat --format=%s -1 >actual && + cat >expect <<-\EOF && + Modify both functions + + file.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + EOF + test_cmp expect actual && + + git log -L:tracked_fn:file.c --shortstat --format=%s -1 >actual && + cat >expect <<-\EOF && + Modify both functions + + 1 file changed, 1 insertion(+), 1 deletion(-) + EOF + test_cmp expect actual +' + +test_expect_success '--numstat across renames and multiple commits' ' + # parallel-change carries the tracked function f across an a.c -> b.c + # rename and a merge of two parallel histories. + git checkout parallel-change && + git log -M -L ":f:b.c" --format= --numstat >actual && + cat >expect <<-\EOF && + 1 1 b.c + 1 1 a.c + 1 1 a.c + 1 1 a.c + 1 0 a.c + 13 0 a.c + EOF + test_cmp expect actual +' + +test_expect_success '-L multiple ranges with --numstat excludes untracked change' ' + git checkout --orphan multi-range && + git reset --hard && + cat >m.c <<-\EOF && + int tracked_func1() + { + return F1; + } + + int tracked_func2() + { + return F2; + } + + int func3() + { + return F3; + } + EOF + git add m.c && + test_tick && + git commit -m "add m.c" && + sed -e "s/F1/F1 + 1/" -e "s/F2/F2 + 2/" -e "s/F3/F3 + 3/" m.c >tmp && + mv tmp m.c && + git commit -a -m "Modify all three functions" && + git log -L:tracked_func1:m.c -L:tracked_func2:m.c --numstat --format=%s -1 >actual && + cat >expect <<-\EOF && + Modify all three functions + + 2 2 m.c + EOF + test_cmp expect 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 f1f5af8f77694ec4f775bcf33f17ed2cd00670e3 Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Tue, 12 May 2026 18:01:34 -0700 Subject: [PATCH 6/7] diff: support --check with -L line ranges Reuse the line_range_filter in builtin_checkdiff() so -L supports the --check option. Add orig_hunk_fn field similar to orig_line_fn that forwards xdiff_emit_hunk_fn calls when we flush filtered hunks. This is necessary because --check relies on receiving calls to its checkdiff_consume_hunk function for managing state. Document and ungate the newly enabled option, and add tests verifying the new behavior. Signed-off-by: Michael Montalbo --- Documentation/line-range-options.adoc | 11 ++-- diff.c | 43 +++++++++++++- revision.c | 2 +- t/t4211-line-log.sh | 83 +++++++++++++++++++++++++++ 4 files changed, 130 insertions(+), 9 deletions(-) diff --git a/Documentation/line-range-options.adoc b/Documentation/line-range-options.adoc index b3e8b5c62ce30a..4a7ab97d750068 100644 --- a/Documentation/line-range-options.adoc +++ b/Documentation/line-range-options.adoc @@ -10,11 +10,12 @@ You can specify this option more than once. Implies `--patch`. Patch output can be suppressed using `--no-patch`. The following non-patch diff formats are supported: `--raw`, - `--name-only`, `--name-status`, `--summary`, `--stat`, `--numstat`, - and `--shortstat`. The stat formats count only lines within the tracked - range. `--dirstat` is not supported with `-L`: it summarizes change as each - directory's share of the total churn, not as counts for the tracked lines. - Use `--numstat` for exact per-file counts within the range. + `--name-only`, `--name-status`, `--summary`, `--check`, `--stat`, + `--numstat`, and `--shortstat`. The stat formats count only lines + within the tracked range. `--dirstat` is not supported with `-L`: it + reports how change is distributed across directories over whole files, + which is not meaningful for line ranges within a file. 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 4a30d7b6310fdb..49b6732c817be6 100644 --- a/diff.c +++ b/diff.c @@ -614,6 +614,7 @@ struct emit_callback { */ struct line_range_filter { xdiff_emit_line_fn orig_line_fn; + xdiff_emit_hunk_fn orig_hunk_fn; void *orig_cb_data; const struct range_set *range_sets_to_filter_by; unsigned int range_set_idx; @@ -2577,6 +2578,13 @@ static void flush_range_hunk(struct line_range_filter *filter) filter->accumulating_hunk.func_name, filter->accumulating_hunk.func_name_len); + if (filter->orig_hunk_fn) + filter->orig_hunk_fn(filter->orig_cb_data, + filter->accumulating_hunk.old_begin, old_count, + filter->accumulating_hunk.new_begin, new_count, + filter->accumulating_hunk.func_name, + filter->accumulating_hunk.func_name_len); + filter->ret = filter->orig_line_fn(filter->orig_cb_data, hdr.buf, hdr.len); strbuf_release(&hdr); @@ -4203,11 +4211,23 @@ static void builtin_diffstat(const char *name_a, const char *name_b, diff_free_filespec_data(two); } +static int idx_in_ranges(const struct range_set *ranges, long idx) +{ + unsigned int i; + + for (i = 0; i < ranges->nr; i++) + if (idx >= ranges->ranges[i].start && + idx < ranges->ranges[i].end) + return 1; + return 0; +} + 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; @@ -4247,7 +4267,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); @@ -4260,6 +4292,10 @@ static void builtin_checkdiff(const char *name_a, const char *name_b, check_blank_at_eof(&mf1, &mf2, &ecbdata); blank_at_eof = ecbdata.blank_at_eof_in_postimage; + if (blank_at_eof && line_ranges && + !idx_in_ranges(line_ranges, blank_at_eof - 1)) + blank_at_eof = 0; + if (blank_at_eof) { static char *err; if (!err) @@ -5055,7 +5091,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 4639c0df8e8501..4cc0d032bcb05f 100644 --- a/revision.c +++ b/revision.c @@ -3231,7 +3231,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 4e8f71c28987be..2a542aa64363ad 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -924,4 +924,87 @@ test_expect_success 'get_commit_action() does not mutate a not-yet-walked commit ) ' +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" && + 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 is limited to tracked ranges and reports real file line numbers' ' + test_must_fail git log -L:tracked:check.c --check --format= >raw && + grep -E ":[0-9]+:" raw >actual && + echo "check.c:3: trailing whitespace." >expect && + test_cmp expect actual && + + test_must_fail git log -L:tracked:check.c -L:other:check.c \ + --check --format= >raw && + grep -E ":[0-9]+:" raw >actual && + cat >expect <<-\EOF && + check.c:3: trailing whitespace. + check.c:8: trailing whitespace. + EOF + test_cmp expect actual +' + +test_expect_success '--check reports each error at its real line across a gap in one range' ' + git checkout --orphan check-gap && + git reset --hard && + cat >gap.c <<-\EOF && + void tracked() + { + int a = 1; + int b = 2; + int c = 3; + int d = 4; + int e = 5; + int g = 7; + return; + } + EOF + git add gap.c && + test_tick && + git commit -m "add gap.c" && + sed -e "s/int a = 1;/int a = 1; /" -e "s/int g = 7;/int g = 7; /" gap.c >tmp && + mv tmp gap.c && + git commit -a -m "ws errors with a gap" && + test_must_fail git log -L:tracked:gap.c --check --format= >raw && + grep -E ":[0-9]+:" raw >actual && + cat >expect <<-\EOF && + gap.c:3: trailing whitespace. + gap.c:8: trailing whitespace. + EOF + test_cmp expect actual +' + +test_expect_success '--check does not report blank-at-eof outside the range' ' + git checkout --orphan check-eof && + git reset --hard && + printf "void tracked()\n{\n return;\n}\n\nint tail = 1;\n" >eof.c && + git add eof.c && + test_tick && + git commit -m "add eof.c" && + printf "void tracked()\n{\n return; \n}\n\nint tail = 1;\n\n" >eof.c && + git commit -a -m "ws in range, blank at eof out of range" && + test_must_fail git log -L:tracked:eof.c --check --format= >raw && + grep -E ":[0-9]+:" raw >actual && + echo "eof.c:3: trailing whitespace." >expect && + test_cmp expect actual +' + test_done From cff5c124ecd9e28ea66921cebfa356ca38aa7c1e Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Tue, 16 Jun 2026 15:24:36 -0700 Subject: [PATCH 7/7] diffcore-pickaxe: limit -G to the -L tracked range Teach -G to only search the line ranges specified by -L. Teaching -S is left as future work, so it still matches the entire file even if -L is specified. Rather than being part of diff.c's builtin implementations, the diffcore-pickaxe functionality interacts with xdiff-interface as a separate component. Add a sibling to xdi_diff_outf(), called diff_emit_line_ranges(), that limits emitted lines to the given line ranges. Use diff_emit_line_ranges() when searching text if line ranges have been specified. If textconv is enabled, use normal diffing instead of diff_emit_line_ranges() since line range tracking relies on the line coordinates of the original, pre-textconv file. Update documentation and add tests accordingly. Signed-off-by: Michael Montalbo --- Documentation/line-range-options.adoc | 4 +- diff.c | 11 ++++ diffcore-pickaxe.c | 30 ++++++++-- t/t4211-line-log.sh | 81 ++++++++++++++++++++++----- xdiff-interface.h | 10 ++++ 5 files changed, 115 insertions(+), 21 deletions(-) diff --git a/Documentation/line-range-options.adoc b/Documentation/line-range-options.adoc index 4a7ab97d750068..52e1262fd71fa1 100644 --- a/Documentation/line-range-options.adoc +++ b/Documentation/line-range-options.adoc @@ -19,6 +19,8 @@ + Patch formatting options such as `--word-diff`, `--color-moved`, `--no-prefix`, and whitespace options (`-w`, `-b`) are supported, -as are pickaxe options (`-S`, `-G`) and `--diff-filter`. +as are pickaxe options (`-S`, `-G`) and `--diff-filter`. `-G` is +limited to the tracked range. In contrast, `-S` is evaluated over the whole +file and may select a commit with a change outside the tracked range. + include::line-range-format.adoc[] diff --git a/diff.c b/diff.c index 49b6732c817be6..1a3571d229b5eb 100644 --- a/diff.c +++ b/diff.c @@ -2701,6 +2701,17 @@ static int line_range_filter_diff(struct line_range_filter *filter, return ret; } +int diff_emit_line_ranges(mmfile_t *one, mmfile_t *two, + const struct range_set *ranges, + xdiff_emit_line_fn line_fn, void *cb_data, + xpparam_t *xpp, xdemitconf_t *xecfg) +{ + struct line_range_filter filter; + + line_range_filter_init(&filter, ranges, line_fn, cb_data); + return line_range_filter_diff(&filter, one, two, xpp, xecfg); +} + static void pprint_rename(struct strbuf *name, const char *a, const char *b) { const char *old_name = a; diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c index b0915be86fc475..2425fe8101181f 100644 --- a/diffcore-pickaxe.c +++ b/diffcore-pickaxe.c @@ -16,7 +16,8 @@ typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two, struct diff_options *o, - regex_t *regexp, kwset_t kws); + regex_t *regexp, kwset_t kws, + const struct range_set *ranges); struct diffgrep_cb { regex_t *regexp; @@ -42,7 +43,8 @@ static int diffgrep_consume(void *priv, char *line, unsigned long len) static int diff_grep(mmfile_t *one, mmfile_t *two, struct diff_options *o, - regex_t *regexp, kwset_t kws UNUSED) + regex_t *regexp, kwset_t kws UNUSED, + const struct range_set *ranges) { struct diffgrep_cb ecbdata; xpparam_t xpp; @@ -65,8 +67,12 @@ static int diff_grep(mmfile_t *one, mmfile_t *two, * An xdiff error might be our "data->hit" from above. See the * comment for xdiff_emit_line_fn in xdiff-interface.h */ - ret = xdi_diff_outf(one, two, NULL, diffgrep_consume, - &ecbdata, &xpp, &xecfg); + if (ranges) + ret = diff_emit_line_ranges(one, two, ranges, diffgrep_consume, + &ecbdata, &xpp, &xecfg); + else + ret = xdi_diff_outf(one, two, NULL, diffgrep_consume, + &ecbdata, &xpp, &xecfg); if (ecbdata.hit) return 1; if (ret) @@ -119,8 +125,13 @@ static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws, static int has_changes(mmfile_t *one, mmfile_t *two, struct diff_options *o UNUSED, - regex_t *regexp, kwset_t kws) + regex_t *regexp, kwset_t kws, + const struct range_set *ranges UNUSED) { + /* + * -S counts needle occurrences in each whole blob. Limiting this to + * an -L range is left as a follow-up; for now -S ignores the range. + */ unsigned int c1 = one ? contains(one, regexp, kws, 0) : 0; unsigned int c2 = two ? contains(two, regexp, kws, c1 + 1) : 0; return c1 != c2; @@ -132,6 +143,7 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o, struct userdiff_driver *textconv_one = NULL; struct userdiff_driver *textconv_two = NULL; mmfile_t mf1, mf2; + const struct range_set *ranges; int ret; /* ignore unmerged */ @@ -169,7 +181,13 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o, mf1.size = fill_textconv(o->repo, textconv_one, p->one, &mf1.ptr); mf2.size = fill_textconv(o->repo, textconv_two, p->two, &mf2.ptr); - ret = fn(&mf1, &mf2, o, regexp, kws); + /* + * -L limits the search to the tracked range, but the range is in + * pre-textconv line coordinates that do not map onto textconv + * output, so search the whole file when textconv is enabled. + */ + ranges = (textconv_one || textconv_two) ? NULL : p->line_ranges; + ret = fn(&mf1, &mf2, o, regexp, kws, ranges); if (textconv_one) free(mf1.ptr); diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index 2a542aa64363ad..2354400d1cc16f 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -703,24 +703,18 @@ test_expect_success '-L suppresses deletions outside tracked range' ' test $(grep -c "^diff --git" actual) = 1 ' -test_expect_success '-L with -S filters to string-count changes' ' +test_expect_success '-L with -S selects only the matching commit' ' git checkout parent-oids && - git log -L:func2:file.c -S "F2 + 2" --format= >actual && - # -S searches the whole file, not just the tracked range; - # combined with the -L range walk, this selects commits that - # both touch func2 and change the count of "F2 + 2" in the file. - test $(grep -c "^diff --git" actual) = 1 && - test_grep "F2 + 2" actual + git log -L:func2:file.c -S "F2 + 2" --format=%s --no-patch >actual && + echo "Modify func2() in file.c" >expect && + test_cmp expect actual ' -test_expect_success '-L with -G filters to diff-text matches' ' +test_expect_success '-L with -G selects only the matching commit' ' git checkout parent-oids && - git log -L:func2:file.c -G "F2 [+] 2" --format= >actual && - # -G greps the whole-file diff text, not just the tracked range; - # combined with -L, this selects commits that both touch func2 - # and have "F2 + 2" in their diff. - test $(grep -c "^diff --git" actual) = 1 && - test_grep "F2 + 2" actual + git log -L:func2:file.c -G "F2 [+] 2" --format=%s --no-patch >actual && + echo "Modify func2() in file.c" >expect && + test_cmp expect actual ' test_expect_success 'setup for trailing deletion test' ' @@ -1007,4 +1001,63 @@ test_expect_success '--check does not report blank-at-eof outside the range' ' test_cmp expect actual ' +test_expect_success '-L -G is limited to the tracked range' ' + git checkout --orphan grep-range && + git reset --hard && + cat >gp.c <<-\EOF && + int func1() + { + return ALPHA; + } + + int func2() + { + return BETA; + } + EOF + git add gp.c && + test_tick && + git commit -m "add gp.c" && + sed -e "s/ALPHA/ALPHA2/" -e "s/BETA/BETA2/" gp.c >tmp && + mv tmp gp.c && + git commit -a -m "touch both functions" && + git log -L:func2:gp.c -G BETA --format=%s --no-patch >actual && + cat >expect <<-\EOF && + touch both functions + add gp.c + EOF + test_cmp expect actual && + git log -L:func2:gp.c -G ALPHA --format=%s --no-patch >actual && + test_must_be_empty actual +' + +test_expect_success '-L -G searches the whole file under textconv' ' + git checkout --orphan grep-textconv && + git reset --hard && + cat >tc.c <<-\EOF && + int func1() + { + return F1; + } + + int func2() + { + return F2; + } + EOF + git add tc.c && + test_tick && + git commit -m "add tc.c" && + sed -e "s/F1/F1 + 1/" -e "s/return F2/return FINDME/" tc.c >tmp && + mv tmp tc.c && + git commit -a -m "change both funcs" && + echo "tc.c diff=tc" >.gitattributes && + git log -L:func1:tc.c -G FINDME --format=%s --no-patch >actual && + test_must_be_empty actual && + git config diff.tc.textconv cat && + git log -L:func1:tc.c -G FINDME --format=%s --no-patch >actual && + echo "change both funcs" >expect && + test_cmp expect actual +' + test_done diff --git a/xdiff-interface.h b/xdiff-interface.h index 24284566292c3d..4151bc2097f95a 100644 --- a/xdiff-interface.h +++ b/xdiff-interface.h @@ -46,6 +46,16 @@ int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2, xdiff_emit_line_fn line_fn, void *consume_callback_data, xpparam_t const *xpp, xdemitconf_t const *xecfg); + +struct range_set; +/* + * Like xdi_diff_outf(), but forwards only the lines within the given + * postimage line ranges to line_fn. + */ +int diff_emit_line_ranges(mmfile_t *mf1, mmfile_t *mf2, + const struct range_set *ranges, + xdiff_emit_line_fn line_fn, void *cb_data, + xpparam_t *xpp, xdemitconf_t *xecfg); int read_mmfile(mmfile_t *ptr, const char *filename); void read_mmblob(mmfile_t *ptr, struct object_database *odb, const struct object_id *oid);