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

[PATCH 1/7] diff: rename and group the line-range filter for clarity

From
Michael Montalbo via GitGitGadget <gitgitgadget@gmail.com>
Date
Jun 18, 2026, 18:16 UTC
Message-ID
<6cfaccab923694fae4c1f607c459af52dce9a021.1781806593.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.2152.git.1781806593.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>

The line-range filter that mm/line-log-cleanup added uses names that obscure its model. The cursors lno_post/lno_pre and the index lno_0 share an lno_ prefix but conflate the pre/post-image axis with the 0-based/1-based axis, the hunk state is a flat set of rhunk_* fields, and the filter-state pointer is just s.

The filter bridges two layers of diff.c, and its fields already used each layer's vocabulary, but in cryptic abbreviations. Spell them out to the form the rest of the file uses, so that the patches that follow can simplify and fix it with those clearer names in place:

  - lno_post/lno_pre -> lno_in_postimage/lno_in_preimage, the
    line-number cursors, matching the counters in struct emit_callback
  - lno_0 -> idx_in_postimage, the 0-based range index
  - the hunk-header geometry stays old/new (old_begin, new_begin, and
    counts) to match the xdiff_emit_hunk_fn callback and the
    "@@ -<old> +<new> @@" header it feeds, but moves from flat rhunk_*
    fields into a "hunk" sub-struct, so accesses read
    filter->hunk.old_begin
  - flush_rhunk -> flush_range_hunk
  - the filter-state pointer in each callback: s -> filter

Also rename the struct line_range_callback to line_range_filter: it is a filter over xdiff output, not merely a callback.

No behavior change.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
 diff.c | 192 +++++++++++++++++++++++++++++----------------------------
 1 file changed, 97 insertions(+), 95 deletions(-)
diff --git a/diff.c b/diff.c
index 5a584fa1d5..1e043c959f 100644
--- a/diff.c
+++ b/diff.c
@@ -623,15 +623,15 @@ struct emit_callback {
  * reveals whether they precede an in-range line (flush into range hunk) or
  * an out-of-range line (discard).
  */
-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;
+	long lno_in_postimage;
+	long lno_in_preimage;
 
 	/*
 	 * Function name from most recent xdiff hunk header;
@@ -640,12 +640,14 @@ struct line_range_callback {
 	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? */
+	/* The range hunk being accumulated for the current range. */
+	struct {
+		struct strbuf lines;	/* buffered in-range diff lines */
+		long old_begin, old_count;
+		long new_begin, new_count;
+		int active;
+		int has_changes;	/* any '+' or '-' line? */
+	} hunk;
 
 	/* Removal lines not yet known to be in-range */
 	struct strbuf pending_rm;
@@ -2540,26 +2542,26 @@ 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;
 
-	if (!s->rhunk_active || s->ret)
+	if (!filter->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->hunk.lines, &filter->pending_rm);
+		filter->hunk.old_count += filter->pending_rm_count;
+		filter->hunk.has_changes = 1;
+		discard_pending_rm(filter);
 	}
 
 	/*
@@ -2568,22 +2570,22 @@ static void flush_rhunk(struct line_range_callback *s)
 	 * 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->hunk.has_changes) {
+		filter->hunk.active = 0;
+		strbuf_reset(&filter->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->hunk.old_begin, filter->hunk.old_count,
+		    filter->hunk.new_begin, filter->hunk.new_count);
+	if (filter->funclen > 0) {
 		strbuf_addch(&hdr, ' ');
-		strbuf_add(&hdr, s->func, s->funclen);
+		strbuf_add(&hdr, filter->func, filter->funclen);
 	}
 	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);
 
 	/*
@@ -2591,18 +2593,18 @@ static void flush_rhunk(struct line_range_callback *s)
 	 * 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) {
+	p = filter->hunk.lines.buf;
+	end = p + filter->hunk.lines.len;
+	while (!filter->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);
+		filter->ret = filter->orig_line_fn(filter->orig_cb_data, (char *)p, line_len);
 		p += line_len;
 	}
 
-	s->rhunk_active = 0;
-	strbuf_reset(&s->rhunk);
+	filter->hunk.active = 0;
+	strbuf_reset(&filter->hunk.lines);
 }
 
 static void line_range_hunk_fn(void *data,
@@ -2610,7 +2612,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 *filter = data;
 
 	/*
 	 * When count > 0, begin is 1-based.  When count == 0, begin is
@@ -2622,104 +2624,104 @@ static void line_range_hunk_fn(void *data,
 	 * 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->lno_in_postimage = new_begin;
+	filter->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->func))
+			funclen = sizeof(filter->func);
+		memcpy(filter->func, func, funclen);
 	}
-	s->funclen = funclen;
+	filter->funclen = 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->lno_in_preimage;
+		filter->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);
+		if (filter->pending_rm_count)
+			strbuf_add(&filter->pending_rm, line, len);
+		else if (filter->hunk.active)
+			strbuf_add(&filter->hunk.lines, line, len);
 		/* otherwise outside tracked range; drop silently */
-		return s->ret;
+		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->lno_in_postimage - 1;
+	cur_pre = filter->lno_in_preimage;	/* save before advancing for context lines */
+	filter->lno_in_postimage++;
 	if (line[0] == ' ')
-		s->lno_pre++;
+		filter->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->cur_range < filter->ranges->nr &&
+	       idx_in_postimage >= filter->ranges->ranges[filter->cur_range].end) {
+		if (filter->hunk.active)
+			flush_range_hunk(filter);
+		discard_pending_rm(filter);
+		filter->cur_range++;
 	}
 
 	/* Past all ranges */
-	if (s->cur_range >= s->ranges->nr) {
-		discard_pending_rm(s);
-		return s->ret;
+	if (filter->cur_range >= filter->ranges->nr) {
+		discard_pending_rm(filter);
+		return filter->ret;
 	}
 
-	cur = &s->ranges->ranges[s->cur_range];
+	cur = &filter->ranges->ranges[filter->cur_range];
 
 	/* 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->hunk.active) {
+		filter->hunk.active = 1;
+		filter->hunk.has_changes = 0;
+		filter->hunk.new_begin = idx_in_postimage + 1;
+		filter->hunk.old_begin = filter->pending_rm_count
+			? filter->pending_rm_pre_begin : cur_pre;
+		filter->hunk.old_count = 0;
+		filter->hunk.new_count = 0;
+		strbuf_reset(&filter->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->hunk.lines, &filter->pending_rm);
+		filter->hunk.old_count += filter->pending_rm_count;
+		filter->hunk.has_changes = 1;
+		discard_pending_rm(filter);
 	}
 
-	strbuf_add(&s->rhunk, line, len);
-	s->rhunk_new_count++;
+	strbuf_add(&filter->hunk.lines, line, len);
+	filter->hunk.new_count++;
 	if (line[0] == '+')
-		s->rhunk_has_changes = 1;
+		filter->hunk.has_changes = 1;
 	else
-		s->rhunk_old_count++;
+		filter->hunk.old_count++;
 
-	return s->ret;
+	return filter->ret;
 }
 
 static void pprint_rename(struct strbuf *name, const char *a, const char *b)
@@ -4086,7 +4088,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;
 
@@ -4094,7 +4096,7 @@ static void builtin_diff(const char *name_a,
 			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);
+			strbuf_init(&lr_state.hunk.lines, 0);
 			strbuf_init(&lr_state.pending_rm, 0);
 
 			/*
@@ -4125,11 +4127,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.hunk.lines);
 			strbuf_release(&lr_state.pending_rm);
 		} else if (xdi_diff_outf(&mf1, &mf2, NULL, fn_out_consume,
 					 &ecbdata, &xpp, &xecfg))

-- 
gitgitgadget
Previous: Michael Montalbo via GitGitGadgetNext: Michael Montalbo via GitGitGadget
Message 2 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.