Re: Mark trailing whitespace error in del lines of diff

[Junio C Hamano <gitster@pobox.com>]

Christian Brabandt <cblists@256bit.org> writes:

> It was the one I am interesting in and also the one that I usually try 
> to avoid ;)
>
> (In fact, I thought if the other options would be needed, one could add 
> additional suboptions for core.whitespace as well,...

That road leads to madness.  Why should we add 2*N options in the
first place?  What valid reason are there, other than "because we
could", to have "diff --ws-check-delelted" and "diff -R" paint
whitespace breakages differently?

I personally do not think I'd use such an option often, but I do
recall running "diff -R" and realized that we fixed whitespace
breakages in the past, which made me feel good (the reason why I
gave "-R" was not to see whitespace breakages in the preimage,
though; it merely was a side effect).

I'll send out two patch series to do the painting part (I didn't
want to touch "--check", as its utility is even more dubious
compared to painting, at least to me).

Here is the first one, a preliminary refactoring.

-- >8 --
Subject: [PATCH 1/2] diff.c: add emit_del_line() and restructure callers of emit_line_0()

Traditionally, we only had emit_add_line() helper, which knows how
to find and paint whitespace breakages on the given line, because we
only care about whitespace breakages introduced in new lines.  The
context lines and old (i.e. deleted) lines are emitted with a
simpler emit_line_0() that paints the entire line in plain or old
colors.

Some people may want to paint whitespace breakages on old lines;
when they see a whitespace breakage on a new line, they can spot
the same kind of whitespace breakage on the corresponding old line
and want to say "Ah, that breakage is there but was inherited from
the original, so let's not fix that for now."

To make such a future enhancement easier, introduce emit_del_line()
and replace direct calls to emit_line_0() with it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/diff.c b/diff.c
index 7500c55..93c1eb4 100644
--- a/diff.c
+++ b/diff.c
@@ -498,6 +498,15 @@ static void emit_add_line(const char *reset,
 	}
 }
 
+static void emit_del_line(const char *reset,
+			  struct emit_callback *ecbdata,
+			  const char *line, int len)
+{
+	const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_OLD);
+
+	emit_line_0(ecbdata->opt, set, reset, '-', line, len);
+}
+
 static void emit_hunk_header(struct emit_callback *ecbdata,
 			     const char *line, int len)
 {
@@ -603,7 +612,6 @@ static void emit_rewrite_lines(struct emit_callback *ecb,
 {
 	const char *endp = NULL;
 	static const char *nneof = " No newline at end of file\n";
-	const char *old = diff_get_color(ecb->color_diff, DIFF_FILE_OLD);
 	const char *reset = diff_get_color(ecb->color_diff, DIFF_RESET);
 
 	while (0 < size) {
@@ -613,8 +621,7 @@ static void emit_rewrite_lines(struct emit_callback *ecb,
 		len = endp ? (endp - data + 1) : size;
 		if (prefix != '+') {
 			ecb->lno_in_preimage++;
-			emit_line_0(ecb->opt, old, reset, '-',
-				    data, len);
+			emit_del_line(reset, ecb, data, len);
 		} else {
 			ecb->lno_in_postimage++;
 			emit_add_line(reset, ecb, data, len);
@@ -1250,17 +1257,22 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 		return;
 	}
 
-	if (line[0] != '+') {
-		const char *color =
-			diff_get_color(ecbdata->color_diff,
-				       line[0] == '-' ? DIFF_FILE_OLD : DIFF_PLAIN);
-		ecbdata->lno_in_preimage++;
-		if (line[0] == ' ')
-			ecbdata->lno_in_postimage++;
-		emit_line(ecbdata->opt, color, reset, line, len);
-	} else {
+	switch (line[0]) {
+	case '+':
 		ecbdata->lno_in_postimage++;
 		emit_add_line(reset, ecbdata, line + 1, len - 1);
+		break;
+	case '-':
+		ecbdata->lno_in_preimage++;
+		emit_del_line(reset, ecbdata, line + 1, len - 1);
+		break;
+	default: /* both ' ' and incomplete line */
+		ecbdata->lno_in_preimage++;
+		ecbdata->lno_in_postimage++;
+		emit_line(ecbdata->opt,
+			  diff_get_color(ecbdata->color_diff, DIFF_PLAIN),
+			  reset, line, len);
+		break;
 	}
 }
 
-- 
2.4.1-511-gc1146d5