[PATCH] diff: reset color before printing newline

[PATCH] diff: reset color before printing newline

[SZEDER Gábor]

It worked that way since commit 50f575fc (Tweak diff colors,
2006-06-22), but commit c1795bb0 (Unify whitespace checking, 2007-12-13)
changed it.  This patch restores the old behaviour.

Besides Linus' arguments in the log message of 50f575fc, resetting color
before printing newline is also important to keep 'git add --patch'
happy.  If the last line(s) of a file are removed, then that hunk will
end with a colored line.  However, if the newline comes before the color
reset, then the diff output will have an additional line at the end
containing only the reset sequence.  This causes trouble in
git-add--interactive.perl's parse_diff function, because @colored will
have one more element than @diff, and that last element will contain the
color reset.  The elements of these arrays will then be copied to @hunk,
but only as many as the number of elements in @diff.  As a result the
last color reset is lost and all subsequent terminal output will be
printed in color.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---

I'm not sure about putting a newline unconditionally at the end of the
line, but this was the status quo before c1795bb0.

 diff.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/diff.c b/diff.c
index 62fdc54..f23657b 100644
--- a/diff.c
+++ b/diff.c
@@ -514,9 +514,13 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix)
 
 static void emit_line(FILE *file, const char *set, const char *reset, const char *line, int len)
 {
+	if (len > 0 && line[len-1] == '\n')
+		len--;
+
 	fputs(set, file);
 	fwrite(line, len, 1, file);
 	fputs(reset, file);
+	fputc('\n', file);
 }
 
 static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)
-- 
1.5.6.rc2.55.g9b8c

Re: [PATCH] diff: reset color before printing newline

[Junio C Hamano]

Thanks; makes sense.

Re: [PATCH] diff: reset color before printing newline

[SZEDER Gábor]

Junio,

On Tue, Jun 17, 2008 at 12:00:02AM +0200, SZEDER Gábor wrote:
> I'm not sure about putting a newline unconditionally at the end of the
> line, but this was the status quo before c1795bb0.
we _must_ not print a newline when there was none, as it breaks
"normal" colored diffs very badly.

Since c1795bb0, for whatever reason, checkdiff_consume() first calls
emit_line() to print only the first '+' character of the line, without
trailing newline, and then calls check_and_emit_line() to print the
rest of the line.  Now if we restore the status quo prior to c1795bb0
regarding unconditional newlines in emit_line(), then there is always
a newline between the the '+' and the rest of the line, which is bad,
bad, bad.

So this should be squashed to the previous patch.

---
 diff.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index f23657b..722f577 100644
--- a/diff.c
+++ b/diff.c
@@ -514,13 +514,18 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix)
 
 static void emit_line(FILE *file, const char *set, const char *reset, const char *line, int len)
 {
-	if (len > 0 && line[len-1] == '\n')
+	int trailing_newline = 0;
+
+	if (len > 0 && line[len-1] == '\n') {
 		len--;
+		trailing_newline = 1;
+	}
 
 	fputs(set, file);
 	fwrite(line, len, 1, file);
 	fputs(reset, file);
-	fputc('\n', file);
+	if (trailing_newline)
+		fputc('\n', file);
 }
 
 static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)

Re: [PATCH] diff: reset color before printing newline

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> So this should be squashed to the previous patch.

Yeah, I noticed the breakage after I pushed the results out and made an
equivalent quickfix and re-pushed out.