[BUG] diff -B does not (always?) use colors

[BUG] diff -B does not (always?) use colors

[Matthias Lederhofer]

I found that git diff -B does not always use colors and I don't have
time to figure out what it is atm.

The bug can be triggered by this two files and running git diff with
-B.  You can also just clone the repository from
git://igit.ath.cx/~matled/tmp/break/

% git cat-file -p HEAD~1:test
def test(p)
    if p
        Array.new
        Array.new(2)
        Array.new(5, "A")

        # only one copy of the object is created
        a = Array.new(2, Hash.new)
        a[0]['cat'] = 'feline'
        a
        a[1]['cat'] = 'Felix'
        a

        # here multiple copies are created
        a = Array.new(2) { Hash.new }
        a[0]['cat'] = 'feline'
        a

        squares = Array.new(5) {|i| i*i}
        squares

        copy = Array.new(squares)
    end
end
% git cat-file -p HEAD:test  
def test(p)
    test_bla if p
end

def test_bla
    Array.new
    Array.new(2)
    Array.new(5, "A")

    # only one copy of the object is created
    a = Array.new(2, Hash.new)
    a[0]['cat'] = 'feline'
    a
    a[1]['cat'] = 'Felix'
    a

    # here multiple copies are created
    a = Array.new(2) { Hash.new }
    a[0]['cat'] = 'feline'
    a

    squares = Array.new(5) {|i| i*i}
    squares

    copy = Array.new(squares)
end

[PATCH] Teach diff -B about colours

[Johannes Schindelin]

Matthias Lederhofer noticed that `diff -B` did not pick up on diff 
colournig.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---

	On Tue, 20 Feb 2007, Matthias Lederhofer wrote:

	> I found that git diff -B does not always use colors and I don't 
	> have time to figure out what it is atm.
	> 
	> The bug can be triggered by this two files and running git diff 
	> with -B.

	Thanks to this excellent bug report, it was very easy to find and 
	fix that bug.

 diff.c |   39 +++++++++++++++++++++++++--------------
 1 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/diff.c b/diff.c
index 6f8b742..5094dcd 100644
--- a/diff.c
+++ b/diff.c
@@ -184,30 +184,40 @@ static void print_line_count(int count)
 	}
 }
 
-static void copy_file(int prefix, const char *data, int size)
+static void copy_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)
+		if (nl_just_seen) {
+			fputs(set, stdout);
 			putchar(prefix);
-		putchar(ch);
-		if (ch == '\n')
+		}
+		if (ch == '\n') {
 			nl_just_seen = 1;
-		else
+			fputs(reset, stdout);
+		} else
 			nl_just_seen = 0;
+		putchar(ch);
 	}
 	if (!nl_just_seen)
-		printf("\n\\ No newline at end of file\n");
+		printf("%s\n\\ No newline at end of file\n", reset);
 }
 
 static void emit_rewrite_diff(const char *name_a,
 			      const char *name_b,
 			      struct diff_filespec *one,
-			      struct diff_filespec *two)
+			      struct diff_filespec *two,
+			      int color_diff)
 {
 	int lc_a, lc_b;
 	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);
 
 	name_a_tab = strchr(name_a, ' ') ? "\t" : "";
 	name_b_tab = strchr(name_b, ' ') ? "\t" : "";
@@ -216,17 +226,17 @@ static void emit_rewrite_diff(const char *name_a,
 	diff_populate_filespec(two, 0);
 	lc_a = count_lines(one->data, one->size);
 	lc_b = count_lines(two->data, two->size);
-	printf("--- a/%s%s\n+++ b/%s%s\n@@ -",
-	       name_a, name_a_tab,
-	       name_b, name_b_tab);
+	printf("%s--- a/%s%s%s\n%s+++ b/%s%s%s\n%s@@ -",
+	       metainfo, name_a, name_a_tab, reset,
+	       metainfo, name_b, name_b_tab, reset, fraginfo);
 	print_line_count(lc_a);
 	printf(" +");
 	print_line_count(lc_b);
-	printf(" @@\n");
+	printf(" @@%s\n", reset);
 	if (lc_a)
-		copy_file('-', one->data, one->size);
+		copy_file('-', one->data, one->size, old, reset);
 	if (lc_b)
-		copy_file('+', two->data, two->size);
+		copy_file('+', two->data, two->size, new, reset);
 }
 
 static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
@@ -1084,7 +1094,8 @@ static void builtin_diff(const char *name_a,
 		if ((one->mode ^ two->mode) & S_IFMT)
 			goto free_ab_and_return;
 		if (complete_rewrite) {
-			emit_rewrite_diff(name_a, name_b, one, two);
+			emit_rewrite_diff(name_a, name_b, one, two,
+					o->color_diff);
 			goto free_ab_and_return;
 		}
 	}
-- 
1.5.0.1.2160.g5bd9-dirty

Re: [PATCH] Teach diff -B about colours

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Matthias Lederhofer noticed that `diff -B` did not pick up on diff 
> colournig.

sp.?

> -static void copy_file(int prefix, const char *data, int size)
> +static void copy_file(int prefix, const char *data, int size,
> +		const char *set, const char *reset)
> ...
>  	if (!nl_just_seen)
> -		printf("\n\\ No newline at end of file\n");
> +		printf("%s\n\\ No newline at end of file\n", reset);
>  }

Are you sure about this one?  If preimage lacked terminating LF and
the postimage has it, then don't you want to see "\ No..." as an
addition?

Re: [PATCH] Teach diff -B about colours

[Johannes Schindelin]

Hi,

On Tue, 20 Feb 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Matthias Lederhofer noticed that `diff -B` did not pick up on diff 
> > colournig.
> 
> sp.?

s/colournig/colouring/

> > -static void copy_file(int prefix, const char *data, int size)
> > +static void copy_file(int prefix, const char *data, int size,
> > +		const char *set, const char *reset)
> > ...
> >  	if (!nl_just_seen)
> > -		printf("\n\\ No newline at end of file\n");
> > +		printf("%s\n\\ No newline at end of file\n", reset);
> >  }
> 
> Are you sure about this one?  If preimage lacked terminating LF and the 
> postimage has it, then don't you want to see "\ No..." as an addition?

Yes, you're right. But I tried to imitate what git-diff does without -B.

Ciao,
Dscho