Re: updated design for the diff-raw format.

From: Junio C Hamano <junkio@cox.net>
Date: May 21, 2005, 23:19 UTC
Message-ID: <7vd5rkdv5p.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <7vwtpsdvgm.fsf@assigned-by-dhcp.cox.net>

(fourth of the replayed messages)

Date: Sat, 21 May 2005 15:03:06 -0700 (PDT)
From: Linus Torvalds <torvalds@osdl.org>
To: Junio C Hamano <junkio@cox.net>
Subject: Re: [PATCH 3/3] Diff overhaul, adding the other half of copy detection.
Message-ID: <Pine.LNX.4.58.0505211452180.2206@ppc970.osdl.org>

On Sat, 21 May 2005, Junio C Hamano wrote:

> 
>      - omit the inter_name_termination and second path if both
>        paths are the same, only when doing human-readable
>        (i.e. inter_name_termination != line_termination).

Hmm. I guess that's fair enough, since it's still easily parseable even if you don't want to use the -z version in scripts (as many tools are better at handling line-terminated stuff than handling zero-terminations).

It might become another flag too, if somebody ends up caring.

Show 7 quoted lines

> Somehow I failed to CC the GIT list the message you are
> responding to.  Discussing a change with an impact of this scale
> needs to be taken public before going further, so with your
> permission I would like to repost both my original ("Once we
> start to think of it this way...")  and your response to the GIT
> list first.  At least I feel that Petr needs to be in the loop
> about this one.

Sure. Although I doube people use the raw diff output except to (a) feed to diff-helper or (b) check that it's non-empty.

But absolutely, post the previous (and this) one.

Show 8 quoted lines

> Another reason is that, as I said, I still have problems about the
> diffcore interface, namely the lack of interface for the applications to
> ask diffcore what the final outcome is.  The "diff-tree not being to
> omit its header output when pickaxe says the result is empty" problem is
> primarily what bothers me, but I think we want a more generic interface
> for the application to inspect the result (not just emptiness check),
> probably before starting to feed the resulting diff list to the external
> diff.

Why not just have a "is there anything pending" query before doing the flush? And always put _everything_ in the pending category, regardless of whether detect_rename/copy is in effect (but if it's not in effect, then flushing the pending queue is obviously just a "go through it and flush it" without any other complexity).

In other words, there would be four clear stages to this:

 1) diff_setup()
	remove "detect_rename" and "diff_score_opt" and "reverse_diff" 
	from this, since they are irrelevant until you _show_ the diffs

 2) diff_queue() *n
	tell diff-core about the files we are going to diff

 3) diff_detect_rename()
	this is what takes the "detect_rename" flag and "diff_score_opt", 
	and walks the list of diffs and potentially changes them into 
	"rename" and "copy" diffs.

 4) diff_flush()
	this just prints out the result (either as a raw diff or as 
	patches). This takes the "reverse_diff" flag that was removed from 
	diff_setup().

and then you can always query the state of the diff tree before stage 3 or before stage 4.

In fact, there's no reason not to even _change_ the diff-queue in magic ways before (3) or (4) depending on what you want to do. For example, your "-S" thing might want to do it's thing between stages (3) and (4).

		Linus