Showing whitespace on minus lines of diff ouput

Showing whitespace on minus lines of diff ouput

[Jay Soffian]

The colored diff output is quite helpful to show introductions of whitespace.

However, if whitespace has been removed, it's impossible to see in the
diff output since whitespace is colorized on '+' lines, but not '-'
lines.

I'm looking at diff.c, but wow. Can someone more familiar with this
file point me toward how I'd make a patch to colorize whitespace on
'-' lines as well?

Thanks,

j.

Re: Showing whitespace on minus lines of diff ouput

[Jay Soffian]

On Thu, Feb 4, 2010 at 8:47 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> I'm looking at diff.c, but wow. Can someone more familiar with this
> file point me toward how I'd make a patch to colorize whitespace on
> '-' lines as well?

Ah, got it. I think. Patch shortly. :-)

j.

Re: Showing whitespace on minus lines of diff ouput

[Jay Soffian]

On Thu, Feb 4, 2010 at 9:08 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> Ah, got it. I think. Patch shortly. :-)

Bah, still need help. Here's what I tried (apologizes for the gmail
munging of long lines):

diff --git a/diff.c b/diff.c
index 381cc8d..17133cd 100644
--- a/diff.c
+++ b/diff.c
@@ -331,6 +331,23 @@ static int new_blank_line_at_eof(struct
emit_callback *ecbdata, const char *line
 	return ws_blank_line(line, len, ecbdata->ws_rule);
 }

+static void emit_sub_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_OLD);
+
+	if (!*ws)
+		emit_line_0(ecbdata->file, set, reset, '-', line, len);
+	else {
+		/* Emit just the prefix, then the rest. */
+		emit_line_0(ecbdata->file, set, reset, '-', "", 0);
+		ws_check_emit(line, len, ecbdata->ws_rule,
+			      ecbdata->file, set, reset, ws);
+	}
+}
+
 static void emit_add_line(const char *reset,
 			  struct emit_callback *ecbdata,
 			  const char *line, int len)
@@ -434,7 +451,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) {
@@ -444,8 +460,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->file, old, reset, '-',
-				    data, len);
+			emit_sub_line(reset, ecb, data, len);
 		} else {
 			ecb->lno_in_postimage++;
 			emit_add_line(reset, ecb, data, len);
@@ -862,9 +877,12 @@ static void fn_out_consume(void *priv, char
*line, unsigned long len)
 			diff_get_color(ecbdata->color_diff,
 				       line[0] == '-' ? DIFF_FILE_OLD : DIFF_PLAIN);
 		ecbdata->lno_in_preimage++;
-		if (line[0] == ' ')
+		if (line[0] == ' ') {
 			ecbdata->lno_in_postimage++;
-		emit_line(ecbdata->file, color, reset, line, len);
+			emit_line(ecbdata->file, color, reset, line, len);
+		} else {
+			emit_sub_line(reset, ecbdata, line + 1, len - 1);
+		}
 	} else {
 		ecbdata->lno_in_postimage++;
 		emit_add_line(reset, ecbdata, line + 1, len - 1);


But for reasons I don't understand this screws up some of the diff
test cases. It's emitting "index" lines where it shouldn't, and "- No
newline at end of file" instead of "\ No newline at end of file".

I guess something is post-processing the output of the emit functions
and doesn't like the change?

Help appreciated. :-)

j.

Re: Showing whitespace on minus lines of diff ouput

[Junio C Hamano]

Jay Soffian <jaysoffian@gmail.com> writes:

> The colored diff output is quite helpful to show introductions of whitespace.
>
> However, if whitespace has been removed, it's impossible to see in the
> diff output since whitespace is colorized on '+' lines, but not '-'
> lines.
>
> I'm looking at diff.c, but wow. Can someone more familiar with this
> file point me toward how I'd make a patch to colorize whitespace on
> '-' lines as well?

Totally uninterested.  "diff -R" would be enough, wouldn't it?

Re: Showing whitespace on minus lines of diff ouput

[Jay Soffian]

On Thu, Feb 4, 2010 at 10:03 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Totally uninterested.  "diff -R" would be enough, wouldn't it?

Duh, I hadn't thought of that.

j.

Re: Showing whitespace on minus lines of diff ouput

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Jay Soffian <jaysoffian@gmail.com> writes:
>
>> The colored diff output is quite helpful to show introductions of whitespace.
>>
>> However, if whitespace has been removed, it's impossible to see in the
>> diff output since whitespace is colorized on '+' lines, but not '-'
>> lines.
>>
>> I'm looking at diff.c, but wow. Can someone more familiar with this
>> file point me toward how I'd make a patch to colorize whitespace on
>> '-' lines as well?
>
> Totally uninterested.  "diff -R" would be enough, wouldn't it?

Actually the reason I don't like the idea to highlight corrected errors
was not just because "'diff -R' is enough".

The coloring of whitespace errors are like compiler warnings, at least to
me, and giving similar highlighting to corrected ones defeats the purpose
by making the output more distracting..

Re: Showing whitespace on minus lines of diff ouput

[Jay Soffian]

On Fri, Feb 5, 2010 at 1:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Actually the reason I don't like the idea to highlight corrected errors
> was not just because "'diff -R' is enough".
>
> The coloring of whitespace errors are like compiler warnings, at least to
> me, and giving similar highlighting to corrected ones defeats the purpose
> by making the output more distracting..

That makes perfect sense. I didn't think it through very well. :-)

j.