git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 2/7] diff: simplify the line-range filter by classifying removals immediately

From
Michael Montalbo via GitGitGadget <gitgitgadget@gmail.com>
Date
Sep 3, 2026, 05:05 UTC
Message-ID
<020e07c0ea82c732bdb0702d2ec211503bd6f6e6.1788411919.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.2152.v3.git.1788411919.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>

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 <mmontalbo@gmail.com>
---
 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 679a0e27d4..c94ddbebe5 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 d0a834ed8f..233dc232e3 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 &&

-- 
gitgitgadget
Previous: Michael Montalbo via GitGitGadgetNext: Michael Montalbo via GitGitGadget
Message 22 of 27 in “line-log: range-scope stat, check, and -G under -L”
  1. 0/7 line-log: range-scope stat, check, and -G under -LMichael Montalbo via GitGitGadget, Jun 18, 2026
  2. 1/7 diff: rename and group the line-range filter for clarityMichael Montalbo via GitGitGadget, Jun 18, 2026
  3. 2/7 diff: simplify the line-range filter by classifying removals immediatelyMichael Montalbo via GitGitGadget, Jun 18, 2026
  4. 3/7 diff: emit -L hunk headers via xdiff's formatterMichael Montalbo via GitGitGadget, Jun 18, 2026
  5. 4/7 diff: extract a line-range diff helper for reuseMichael Montalbo via GitGitGadget, Jun 18, 2026
  6. 5/7 line-log: support diff stat formats with -LMichael Montalbo via GitGitGadget, Jun 18, 2026
  7. Junio C HamanoJun 18, 2026
  8. Michael MontalboJun 23, 2026
  9. 6/7 diff: support --check with -L line rangesMichael Montalbo via GitGitGadget, Jun 18, 2026
  10. 7/7 diffcore-pickaxe: scope -G to the -L tracked rangeMichael Montalbo via GitGitGadget, Jun 18, 2026
  11. 0/7 line-log: scope stat, check, and -G to -L line rangesMichael Montalbo via GitGitGadget, Jun 27, 2026
  12. 1/7 diff: rename and group the line-range filter for clarityMichael Montalbo via GitGitGadget, Jun 27, 2026
  13. 2/7 diff: simplify the line-range filter by classifying removals immediatelyMichael Montalbo via GitGitGadget, Jun 27, 2026
  14. 3/7 diff: emit -L hunk headers via xdiff's formatterMichael Montalbo via GitGitGadget, Jun 27, 2026
  15. 4/7 diff: extract a line-range diff helper for reuseMichael Montalbo via GitGitGadget, Jun 27, 2026
  16. 5/7 line-log: support diff stat formats with -LMichael Montalbo via GitGitGadget, Jun 27, 2026
  17. 6/7 diff: support --check with -L line rangesMichael Montalbo via GitGitGadget, Jun 27, 2026
  18. 7/7 diffcore-pickaxe: scope -G to the -L tracked rangeMichael Montalbo via GitGitGadget, Jun 27, 2026
  19. Michael MontalboJul 15, 2026
  20. 0/7 line-log: scope stat, check, and -G to -L line rangesMichael Montalbo via GitGitGadget, Sep 3, 2026
  21. 1/7 diff: rename line-range filter struct and clarify fieldsMichael Montalbo via GitGitGadget, Sep 3, 2026
  22. 2/7 diff: simplify the line-range filter by classifying removals immediatelyMichael Montalbo via GitGitGadget, Sep 3, 2026
  23. 3/7 diff: emit -L hunk headers via xdiff's formatterMichael Montalbo via GitGitGadget, Sep 3, 2026
  24. 4/7 diff: extract a line-range diff helper for reuseMichael Montalbo via GitGitGadget, Sep 3, 2026
  25. 5/7 diff: support stat formats with -LMichael Montalbo via GitGitGadget, Sep 3, 2026
  26. 6/7 diff: support --check with -L line rangesMichael Montalbo via GitGitGadget, Sep 3, 2026
  27. 7/7 diffcore-pickaxe: limit -G to the -L tracked rangeMichael Montalbo via GitGitGadget, Sep 3, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.