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

["Michael Montalbo via GitGitGadget" <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