threads / patch / 13988

patchdiff: reset color before printing newline

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

## tl;dr

4 messages between Jun 16, 2008 and Jun 17, 2008. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

SZEDER Gábor· Jun 16, 2008, 22:00 UTC · lore

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(-)
Show changes to diff.c +4 −0
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
SZEDER Gábor· Jun 17, 2008, 00:22 UTC · re: SZEDER Gábor · lore

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

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(-)
Show changes to diff.c +7 −2
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)
Junio C Hamano· Jun 17, 2008, 00:59 UTC · re: SZEDER Gábor · lore

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

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.

← back to recent threads