[PATCH] Built-in diff driver shows Index: line.

[PATCH] Built-in diff driver shows Index: line.

[Junio C Hamano]

As suggested by Linus as a workaround to unconfuse diffstat,
this patch adds Index: line before the diff output the built-in
driver produces.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff.c |    2 ++
1 files changed, 2 insertions(+)

# - 04/28 11:20 No need to say diff-tree -p -r in git-export
# + 04/28 11:25 Show Index: line from built-in diff driver.
Index: diff.c
--- k/diff.c  (mode:100644)
+++ l/diff.c  (mode:100644)
@@ -125,6 +125,8 @@ static void builtin_diff(const char *nam
 	next_at += snprintf(cmd+next_at, cmd_size-next_at,
 			    diff_arg, input_name_sq[0], input_name_sq[1]);
 
+	printf("Index: %s\n", name);
+	fflush(NULL);
 	execlp("/bin/sh","sh", "-c", cmd, NULL);
 }
 

Re: [PATCH] Built-in diff driver shows Index: line.

[Linus Torvalds]

On Thu, 28 Apr 2005, Junio C Hamano wrote:
>
> As suggested by Linus as a workaround to unconfuse diffstat,
> this patch adds Index: line before the diff output the built-in
> driver produces.

Actually, I do dislike the Index: line, and think this is a pretty
intrusive work-around for a problem with diffstat.

There are other work-arounds for diffstat. In particular, diffstat has 
various heuristics for finding the filename from the +++/--- files, and 
the main one is

	"*** %[^\t]\t%[^ ] %[^ ] %d %d:%d:%d %d"

(where "***" can be either +++ or ---). If if you match that one, diffstat
will pick up the name (first match) on its own.

Oh, actually maybe the better pattern to use is the one that GNU diff 
itself ends up matching:

	"*** %[^\t ]%[\t ]%d%c%d%c%d %d:%d:%d"

where the "%c" has to be either '-' or '/' (ie it ends up matching as 
"numeric date" + "numeric time").

You can put the "mode" thing at the end, and diffstat won't care about it.

		Linus

Re: [PATCH] Built-in diff driver shows Index: line.

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> Actually, I do dislike the Index: line, and think this is a pretty
LT> intrusive work-around for a problem with diffstat.

Alright.  Please consider the patch retracted.

LT> Oh, actually maybe the better pattern to use is the one that GNU diff 
LT> itself ends up matching:
LT> 	"*** %[^\t ]%[\t ]%d%c%d%c%d %d:%d:%d"
LT> where the "%c" has to be either '-' or '/' (ie it ends up matching as 
LT> "numeric date" + "numeric time").

LT> You can put the "mode" thing at the end, and diffstat won't care about it.

Hmph.  Timestamps do not mean anything in most of the intended
use of diff-* family, since they are meant to operate on trees,
except:

 - comparing against the working tree --- show-diff's <new> and
   diff-cache's <new>; we can take the timestamp from the
   filesystem.

 - comparing against a tree that comes from a known commit ---
   we can take the timestamp of the commit that contains the
   file.

If we want to show the timestamp of the latter, diff-tree and
diff-cache need to be taught to take notice if their
tree-or-commit parameter is actually a commit and if so needs to
pass the timestamp in the committer field down the path for the
diff driver.  There is no way for diff-tree-helper to do this
because the origin information is already stripped out when it
sees a valid SHA1.

So I'd say we'd punt this one for now, unless somebody else has
a better idea.

Re: [PATCH] Built-in diff driver shows Index: line.

[Linus Torvalds]

On Thu, 28 Apr 2005, Junio C Hamano wrote:
> 
> So I'd say we'd punt this one for now, unless somebody else has
> a better idea.

It's trivially easy to do it with a external diff helper.

So you can do it with a few lines of GIT_EXTERNAL_DIFF, and the hardest 
part is showing it in a nice format (ie do the normalization of the 
results that diffstat does).

The external diff program can _literally_ just do something like

	#!/bin/sh
	name="$1"
	src="$2"
	dst="$5"
	diff "$src" "$dst" | cut -c1 | grep '[<>]' | sort | uniq -c

and the output should be something like

    458 <
    104 >

which means "458 lines removed, 104 lines added". Pretty-print it some, 
and you're done.

Hacky hacky,

		Linus

Re: [PATCH] Built-in diff driver shows Index: line.

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> On Thu, 28 Apr 2005, Junio C Hamano wrote:
>> 
>> So I'd say we'd punt this one for now, unless somebody else has
>> a better idea.

LT> It's trivially easy to do it with a external diff helper.

What I meant to _punt_ is trying to show meaningful dates, not
satisfying the diffstat.

LT> So you can do it with a few lines of GIT_EXTERNAL_DIFF ...

Heh.  I am the one who wrote that stuff and you already know how
to use that effectively far better than I do ;-).