[PATCH 0/4] Colouring whitespace errors in diff -B output

[PATCH 0/4] Colouring whitespace errors in diff -B output

[Junio C Hamano]

Here is a 4-patch miniseries to teach "diff -B" output routines to detect
and colour whitespace errors like we do for normal patches.

The first three patches are only about moving code around without changing
anything.

The last one hooks "diff -B" logic to the per-line output routines in a
way that mimicks how the normal patches are fed to them better, in order
to take advantage of all the existing whitespace error detection and
colouring logic.

Junio C Hamano (4):
      diff.c: shuffling code around
      diff.c: split emit_line() from the first char and the rest of the line
      diff.c: emit_add_line() takes only the rest of the line
      diff -B: colour whitespace errors

 diff.c |  327 +++++++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 180 insertions(+), 147 deletions(-)

[PATCH 1/4] diff.c: shuffling code around

[Junio C Hamano]

Move function, type, and structure definitions for fill_mmfile(),
count_trailing_blank(), check_blank_at_eof(), emit_line(),
new_blank_line_at_eof(), emit_add_line(), sane_truncate_fn, and
emit_callback up in the file, so that they can be refactored into helper
functions and reused by codepath for emitting rewrite patches.

This only moves the lines around to make the next two patches easier to
read.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This obviously comes on top of the earlier fix to the "blank lines at
   eof" breakage.

 diff.c |  250 ++++++++++++++++++++++++++++++++--------------------------------
 1 files changed, 125 insertions(+), 125 deletions(-)

diff --git a/diff.c b/diff.c
index 63a3bfc..7548966 100644
--- a/diff.c
+++ b/diff.c
@@ -241,6 +241,23 @@ static struct diff_tempfile {
 	char tmp_path[PATH_MAX];
 } diff_temp[2];
 
+typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);
+
+struct emit_callback {
+	struct xdiff_emit_state xm;
+	int color_diff;
+	unsigned ws_rule;
+	int blank_at_eof_in_preimage;
+	int blank_at_eof_in_postimage;
+	int lno_in_preimage;
+	int lno_in_postimage;
+	sane_truncate_fn truncate;
+	const char **label_path;
+	struct diff_words_data *diff_words;
+	int *found_changesp;
+	FILE *file;
+};
+
 static int count_lines(const char *data, int size)
 {
 	int count, ch, completely_empty = 1, nl_just_seen = 0;
@@ -301,6 +318,114 @@ static void copy_file_with_prefix(FILE *file,
 		fprintf(file, "%s\n\\ No newline at end of file\n", reset);
 }
 
+static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
+{
+	if (!DIFF_FILE_VALID(one)) {
+		mf->ptr = (char *)""; /* does not matter */
+		mf->size = 0;
+		return 0;
+	}
+	else if (diff_populate_filespec(one, 0))
+		return -1;
+	mf->ptr = one->data;
+	mf->size = one->size;
+	return 0;
+}
+
+static int count_trailing_blank(mmfile_t *mf, unsigned ws_rule)
+{
+	char *ptr = mf->ptr;
+	long size = mf->size;
+	int cnt = 0;
+
+	if (!size)
+		return cnt;
+	ptr += size - 1; /* pointing at the very end */
+	if (*ptr != '\n')
+		; /* incomplete line */
+	else
+		ptr--; /* skip the last LF */
+	while (mf->ptr < ptr) {
+		char *prev_eol;
+		for (prev_eol = ptr; mf->ptr <= prev_eol; prev_eol--)
+			if (*prev_eol == '\n')
+				break;
+		if (!ws_blank_line(prev_eol + 1, ptr - prev_eol, ws_rule))
+			break;
+		cnt++;
+		ptr = prev_eol - 1;
+	}
+	return cnt;
+}
+
+static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2,
+			       struct emit_callback *ecbdata)
+{
+	int l1, l2, at;
+	unsigned ws_rule = ecbdata->ws_rule;
+	l1 = count_trailing_blank(mf1, ws_rule);
+	l2 = count_trailing_blank(mf2, ws_rule);
+	if (l2 <= l1) {
+		ecbdata->blank_at_eof_in_preimage = 0;
+		ecbdata->blank_at_eof_in_postimage = 0;
+		return;
+	}
+	at = count_lines(mf1->ptr, mf1->size);
+	ecbdata->blank_at_eof_in_preimage = (at - l1) + 1;
+
+	at = count_lines(mf2->ptr, mf2->size);
+	ecbdata->blank_at_eof_in_postimage = (at - l2) + 1;
+}
+
+static void emit_line(FILE *file, const char *set, const char *reset, const char *line, int len)
+{
+	int has_trailing_newline, has_trailing_carriage_return;
+
+	has_trailing_newline = (len > 0 && line[len-1] == '\n');
+	if (has_trailing_newline)
+		len--;
+	has_trailing_carriage_return = (len > 0 && line[len-1] == '\r');
+	if (has_trailing_carriage_return)
+		len--;
+
+	fputs(set, file);
+	fwrite(line, len, 1, file);
+	fputs(reset, file);
+	if (has_trailing_carriage_return)
+		fputc('\r', file);
+	if (has_trailing_newline)
+		fputc('\n', file);
+}
+
+static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)
+{
+	if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
+	      ecbdata->blank_at_eof_in_preimage &&
+	      ecbdata->blank_at_eof_in_postimage &&
+	      ecbdata->blank_at_eof_in_preimage <= ecbdata->lno_in_preimage &&
+	      ecbdata->blank_at_eof_in_postimage <= ecbdata->lno_in_postimage))
+		return 0;
+	return ws_blank_line(line + 1, len - 1, ecbdata->ws_rule);
+}
+
+static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)
+{
+	const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE);
+	const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_NEW);
+
+	if (!*ws)
+		emit_line(ecbdata->file, set, reset, line, len);
+	else if (new_blank_line_at_eof(ecbdata, line, len))
+		/* Blank line at EOF - paint '+' as well */
+		emit_line(ecbdata->file, ws, reset, line, len);
+	else {
+		/* Emit just the prefix, then the rest. */
+		emit_line(ecbdata->file, set, reset, line, 1);
+		ws_check_emit(line + 1, len - 1, ecbdata->ws_rule,
+			      ecbdata->file, set, reset, ws);
+	}
+}
+
 static void emit_rewrite_diff(const char *name_a,
 			      const char *name_b,
 			      struct diff_filespec *one,
@@ -345,20 +470,6 @@ static void emit_rewrite_diff(const char *name_a,
 		copy_file_with_prefix(o->file, '+', two->data, two->size, new, reset);
 }
 
-static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
-{
-	if (!DIFF_FILE_VALID(one)) {
-		mf->ptr = (char *)""; /* does not matter */
-		mf->size = 0;
-		return 0;
-	}
-	else if (diff_populate_filespec(one, 0))
-		return -1;
-	mf->ptr = one->data;
-	mf->size = one->size;
-	return 0;
-}
-
 struct diff_words_buffer {
 	mmfile_t text;
 	long alloc;
@@ -485,23 +596,6 @@ static void diff_words_show(struct diff_words_data *diff_words)
 	}
 }
 
-typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);
-
-struct emit_callback {
-	struct xdiff_emit_state xm;
-	int color_diff;
-	unsigned ws_rule;
-	int blank_at_eof_in_preimage;
-	int blank_at_eof_in_postimage;
-	int lno_in_preimage;
-	int lno_in_postimage;
-	sane_truncate_fn truncate;
-	const char **label_path;
-	struct diff_words_data *diff_words;
-	int *found_changesp;
-	FILE *file;
-};
-
 static void free_diff_words_data(struct emit_callback *ecbdata)
 {
 	if (ecbdata->diff_words) {
@@ -524,55 +618,6 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix)
 	return "";
 }
 
-static void emit_line(FILE *file, const char *set, const char *reset, const char *line, int len)
-{
-	int has_trailing_newline, has_trailing_carriage_return;
-
-	has_trailing_newline = (len > 0 && line[len-1] == '\n');
-	if (has_trailing_newline)
-		len--;
-	has_trailing_carriage_return = (len > 0 && line[len-1] == '\r');
-	if (has_trailing_carriage_return)
-		len--;
-
-	fputs(set, file);
-	fwrite(line, len, 1, file);
-	fputs(reset, file);
-	if (has_trailing_carriage_return)
-		fputc('\r', file);
-	if (has_trailing_newline)
-		fputc('\n', file);
-}
-
-static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)
-{
-	if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
-	      ecbdata->blank_at_eof_in_preimage &&
-	      ecbdata->blank_at_eof_in_postimage &&
-	      ecbdata->blank_at_eof_in_preimage <= ecbdata->lno_in_preimage &&
-	      ecbdata->blank_at_eof_in_postimage <= ecbdata->lno_in_postimage))
-		return 0;
-	return ws_blank_line(line + 1, len - 1, ecbdata->ws_rule);
-}
-
-static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)
-{
-	const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE);
-	const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_NEW);
-
-	if (!*ws)
-		emit_line(ecbdata->file, set, reset, line, len);
-	else if (new_blank_line_at_eof(ecbdata, line, len))
-		/* Blank line at EOF - paint '+' as well */
-		emit_line(ecbdata->file, ws, reset, line, len);
-	else {
-		/* Emit just the prefix, then the rest. */
-		emit_line(ecbdata->file, set, reset, line, 1);
-		ws_check_emit(line + 1, len - 1, ecbdata->ws_rule,
-			      ecbdata->file, set, reset, ws);
-	}
-}
-
 static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, unsigned long len)
 {
 	const char *cp;
@@ -1464,51 +1509,6 @@ static const struct funcname_pattern_entry *diff_funcname_pattern(struct diff_fi
 	return NULL;
 }
 
-static int count_trailing_blank(mmfile_t *mf, unsigned ws_rule)
-{
-	char *ptr = mf->ptr;
-	long size = mf->size;
-	int cnt = 0;
-
-	if (!size)
-		return cnt;
-	ptr += size - 1; /* pointing at the very end */
-	if (*ptr != '\n')
-		; /* incomplete line */
-	else
-		ptr--; /* skip the last LF */
-	while (mf->ptr < ptr) {
-		char *prev_eol;
-		for (prev_eol = ptr; mf->ptr <= prev_eol; prev_eol--)
-			if (*prev_eol == '\n')
-				break;
-		if (!ws_blank_line(prev_eol + 1, ptr - prev_eol, ws_rule))
-			break;
-		cnt++;
-		ptr = prev_eol - 1;
-	}
-	return cnt;
-}
-
-static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2,
-			       struct emit_callback *ecbdata)
-{
-	int l1, l2, at;
-	unsigned ws_rule = ecbdata->ws_rule;
-	l1 = count_trailing_blank(mf1, ws_rule);
-	l2 = count_trailing_blank(mf2, ws_rule);
-	if (l2 <= l1) {
-		ecbdata->blank_at_eof_in_preimage = 0;
-		ecbdata->blank_at_eof_in_postimage = 0;
-		return;
-	}
-	at = count_lines(mf1->ptr, mf1->size);
-	ecbdata->blank_at_eof_in_preimage = (at - l1) + 1;
-
-	at = count_lines(mf2->ptr, mf2->size);
-	ecbdata->blank_at_eof_in_postimage = (at - l2) + 1;
-}
-
 static void builtin_diff(const char *name_a,
 			 const char *name_b,
 			 struct diff_filespec *one,
-- 
1.6.5.rc1.54.g4aad

[PATCH 2/4] diff.c: split emit_line() from the first char and the rest of the line

[Junio C Hamano]

A new helper function emit_line_0() takes the first line of diff output
(typically "-", " ", or "+") separately from the remainder of the line.
No other functional changes.

This change will make it easier to reuse the logic when emitting the
rewrite diff, as we do not want to copy a line only to add "+"/"-"/" "
immediately before its first character when we produce rewrite diff
output.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/diff.c b/diff.c
index 7548966..b5c2574 100644
--- a/diff.c
+++ b/diff.c
@@ -377,7 +377,8 @@ static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2,
 	ecbdata->blank_at_eof_in_postimage = (at - l2) + 1;
 }
 
-static void emit_line(FILE *file, const char *set, const char *reset, const char *line, int len)
+static void emit_line_0(FILE *file, const char *set, const char *reset,
+			int first, const char *line, int len)
 {
 	int has_trailing_newline, has_trailing_carriage_return;
 
@@ -389,6 +390,7 @@ static void emit_line(FILE *file, const char *set, const char *reset, const char
 		len--;
 
 	fputs(set, file);
+	fputc(first, file);
 	fwrite(line, len, 1, file);
 	fputs(reset, file);
 	if (has_trailing_carriage_return)
@@ -397,6 +399,12 @@ static void emit_line(FILE *file, const char *set, const char *reset, const char
 		fputc('\n', file);
 }
 
+static void emit_line(FILE *file, const char *set, const char *reset,
+		      const char *line, int len)
+{
+	emit_line_0(file, set, reset, line[0], line+1, len-1);
+}
+
 static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)
 {
 	if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
-- 
1.6.5.rc1.54.g4aad

[PATCH 3/4] diff.c: emit_add_line() takes only the rest of the line

[Junio C Hamano]

As the first character on the line that is fed to this function is always
"+", it is pointless to send that along with the rest of the line.

This change will make it easier to reuse the logic when emitting the
rewrite diff, as we do not want to copy a line only to add "+"/"-"/" "
immediately before its first character when we produce rewrite diff
output.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/diff.c b/diff.c
index b5c2574..baf46ab 100644
--- a/diff.c
+++ b/diff.c
@@ -416,20 +416,22 @@ static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line
 	return ws_blank_line(line + 1, len - 1, ecbdata->ws_rule);
 }
 
-static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)
+static void emit_add_line(const char *reset,
+			  struct emit_callback *ecbdata,
+			  const char *line, int len)
 {
 	const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE);
 	const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_NEW);
 
 	if (!*ws)
-		emit_line(ecbdata->file, set, reset, line, len);
+		emit_line_0(ecbdata->file, set, reset, '+', line, len);
 	else if (new_blank_line_at_eof(ecbdata, line, len))
 		/* Blank line at EOF - paint '+' as well */
-		emit_line(ecbdata->file, ws, reset, line, len);
+		emit_line_0(ecbdata->file, ws, reset, '+', line, len);
 	else {
 		/* Emit just the prefix, then the rest. */
-		emit_line(ecbdata->file, set, reset, line, 1);
-		ws_check_emit(line + 1, len - 1, ecbdata->ws_rule,
+		emit_line_0(ecbdata->file, set, reset, '+', "", 0);
+		ws_check_emit(line, len, ecbdata->ws_rule,
 			      ecbdata->file, set, reset, ws);
 	}
 }
@@ -726,7 +728,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 		emit_line(ecbdata->file, color, reset, line, len);
 	} else {
 		ecbdata->lno_in_postimage++;
-		emit_add_line(reset, ecbdata, line, len);
+		emit_add_line(reset, ecbdata, line + 1, len - 1);
 	}
 }
 
-- 
1.6.5.rc1.54.g4aad

[PATCH 4/4] diff -B: colour whitespace errors

[Junio C Hamano]

We used to send the old and new contents more or less straight out to the
output with only the original "old is red, new is green" colouring.  Now
all the necessary support routines have been prepared, call them with a
line of data at a time from the output code and have them check and color
whitespace errors in exactly the same way as they are called from the low
level diff callback routines.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c |   75 +++++++++++++++++++++++++++++++++++++++++----------------------
 1 files changed, 49 insertions(+), 26 deletions(-)

diff --git a/diff.c b/diff.c
index baf46ab..b6d40d7 100644
--- a/diff.c
+++ b/diff.c
@@ -296,28 +296,6 @@ static void print_line_count(FILE *file, int count)
 	}
 }
 
-static void copy_file_with_prefix(FILE *file,
-				  int prefix, const char *data, int size,
-				  const char *set, const char *reset)
-{
-	int ch, nl_just_seen = 1;
-	while (0 < size--) {
-		ch = *data++;
-		if (nl_just_seen) {
-			fputs(set, file);
-			putc(prefix, file);
-		}
-		if (ch == '\n') {
-			nl_just_seen = 1;
-			fputs(reset, file);
-		} else
-			nl_just_seen = 0;
-		putc(ch, file);
-	}
-	if (!nl_just_seen)
-		fprintf(file, "%s\n\\ No newline at end of file\n", reset);
-}
-
 static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
 {
 	if (!DIFF_FILE_VALID(one)) {
@@ -436,6 +414,38 @@ static void emit_add_line(const char *reset,
 	}
 }
 
+static void emit_rewrite_lines(struct emit_callback *ecb,
+			       int prefix, const char *data, int size)
+{
+	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) {
+		int len;
+
+		endp = memchr(data, '\n', size);
+		len = endp ? (endp - data + 1) : size;
+		if (prefix != '+') {
+			ecb->lno_in_preimage++;
+			emit_line_0(ecb->file, old, reset, '-',
+				    data, len);
+		} else {
+			ecb->lno_in_postimage++;
+			emit_add_line(reset, ecb, data, len);
+		}
+		size -= len;
+		data += len;
+	}
+	if (!endp) {
+		const char *plain = diff_get_color(ecb->color_diff,
+						   DIFF_PLAIN);
+		emit_line_0(ecb->file, plain, reset, '\\',
+			    nneof, strlen(nneof));
+	}
+}
+
 static void emit_rewrite_diff(const char *name_a,
 			      const char *name_b,
 			      struct diff_filespec *one,
@@ -447,10 +457,23 @@ static void emit_rewrite_diff(const char *name_a,
 	const char *name_a_tab, *name_b_tab;
 	const char *metainfo = diff_get_color(color_diff, DIFF_METAINFO);
 	const char *fraginfo = diff_get_color(color_diff, DIFF_FRAGINFO);
-	const char *old = diff_get_color(color_diff, DIFF_FILE_OLD);
-	const char *new = diff_get_color(color_diff, DIFF_FILE_NEW);
 	const char *reset = diff_get_color(color_diff, DIFF_RESET);
 	static struct strbuf a_name = STRBUF_INIT, b_name = STRBUF_INIT;
+	struct emit_callback ecbdata;
+
+	memset(&ecbdata, 0, sizeof(ecbdata));
+	ecbdata.color_diff = color_diff;
+	ecbdata.found_changesp = &o->found_changes;
+	ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
+	ecbdata.file = o->file;
+	if (ecbdata.ws_rule & WS_BLANK_AT_EOF) {
+		mmfile_t mf1, mf2;
+		fill_mmfile(&mf1, one);
+		fill_mmfile(&mf2, two);
+		check_blank_at_eof(&mf1, &mf2, &ecbdata);
+	}
+	ecbdata.lno_in_preimage = 1;
+	ecbdata.lno_in_postimage = 1;
 
 	name_a += (*name_a == '/');
 	name_b += (*name_b == '/');
@@ -475,9 +498,9 @@ static void emit_rewrite_diff(const char *name_a,
 	print_line_count(o->file, lc_b);
 	fprintf(o->file, " @@%s\n", reset);
 	if (lc_a)
-		copy_file_with_prefix(o->file, '-', one->data, one->size, old, reset);
+		emit_rewrite_lines(&ecbdata, '-', one->data, one->size);
 	if (lc_b)
-		copy_file_with_prefix(o->file, '+', two->data, two->size, new, reset);
+		emit_rewrite_lines(&ecbdata, '+', two->data, two->size);
 }
 
 struct diff_words_buffer {
-- 
1.6.5.rc1.54.g4aad

Re: [PATCH 0/4] Colouring whitespace errors in diff -B output

[Nanako Shiraishi]

Quoting Junio C Hamano <gitster@pobox.com>

> Here is a 4-patch miniseries to teach "diff -B" output routines to detect
> and colour whitespace errors like we do for normal patches.
>
> The first three patches are only about moving code around without changing
> anything.
>
> The last one hooks "diff -B" logic to the per-line output routines in a
> way that mimicks how the normal patches are fed to them better, in order
> to take advantage of all the existing whitespace error detection and
> colouring logic.
>
> Junio C Hamano (4):
>       diff.c: shuffling code around
>       diff.c: split emit_line() from the first char and the rest of the line
>       diff.c: emit_add_line() takes only the rest of the line
>       diff -B: colour whitespace errors
>
>  diff.c |  327 +++++++++++++++++++++++++++++++++++-----------------------------
>  1 files changed, 180 insertions(+), 147 deletions(-)

Sorry, but I don't seem to be able to apply these patches anywhere.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: [PATCH 0/4] Colouring whitespace errors in diff -B output

[Junio C Hamano]

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Quoting Junio C Hamano <gitster@pobox.com>
>
>> The last one hooks "diff -B" logic to the per-line output routines in a
>> way that mimicks how the normal patches are fed to them better, in order
>> to take advantage of all the existing whitespace error detection and
>> colouring logic.
>>
>> Junio C Hamano (4):
>>       diff.c: shuffling code around
>>       diff.c: split emit_line() from the first char and the rest of the line
>>       diff.c: emit_add_line() takes only the rest of the line
>>       diff -B: colour whitespace errors
>>
>>  diff.c |  327 +++++++++++++++++++++++++++++++++++-----------------------------
>>  1 files changed, 180 insertions(+), 147 deletions(-)
>
> Sorry, but I don't seem to be able to apply these patches anywhere.

There is a subtle bug in "split emit_line()" patch I sent out, but later I
noticed the problem and fixed it in my tree, so please use the updated one
instead from my tree.

The series applies cleanly at the tip of jc/maint-1.6.0-blank-at-eof, but
the result of applying them will have large conflicts whether you are
merging that into 'maint', 'master', or 'next'.  It's probably easier to
use the merge I've prepared to resolve them in my tree.

I have these two topics:

 - jc/maint-1.6.0-blank-at-eof forks from old 1.6.0 codebase to contain
   the above fixes (and the ones that are already in 'next'); and

 - jc/maint-blank-at-eof that forks from 1.6.4 codebase and merges the
   above branch.  This branch does not have any commit on its own, but
   does a rather nasty conflict resolution.

I'll push out the result, merging the latter to 'next' (and 'pu'),
sometime in the next few hours.