threads / patch / 379

patchBuilt-in diff driver shows Index: line.

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

## tl;dr

5 messages between Apr 28, 2005 and Apr 28, 2005. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Junio C Hamano· Apr 28, 2005, 19:14 UTC · lore

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.

Show changes to diff +2 −0
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);
 }
Linus Torvalds· Apr 28, 2005, 20:05 UTC · re: Junio C Hamano · lore

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

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
Junio C Hamano· Apr 28, 2005, 22:09 UTC · re: Linus Torvalds · lore

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

>>>>> "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.

Linus Torvalds· Apr 28, 2005, 22:30 UTC · re: Junio C Hamano · lore

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

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
Junio C Hamano· Apr 28, 2005, 22:41 UTC · re: Linus Torvalds · lore

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

>>>>> "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 ;-).

← back to recent threads