Re: [PATCH 2/3] Introduce rename factorization in diffcore.

From: Yann Dirson <ydirson@altern.org>
Date: Nov 7, 2008, 22:12 UTC
Message-ID: <20081107221224.GK5158@nan92-1-81-57-214-146.fbx.proxad.net>
In-Reply-To: <7vskq3fdgz.fsf@gitster.siamese.dyndns.org>

On Fri, Nov 07, 2008 at 01:11:40PM -0800, Junio C Hamano wrote:

Show 14 quoted lines

> Yann Dirson <ydirson@altern.org> writes:
> 
> > On Fri, Nov 07, 2008 at 12:19:14PM -0800, Junio C Hamano wrote:
> >> I am afraid that this is totally unacceptable, as you yourself mentioned,
> >> the end result is unapplicable with any existing tool and would confuse
> >> viewers like gitk and gitweb.
> >
> > Well, other tools will still have to be taught about a new syntax, if
> > they're going to use the new flag - just like it was for --rename.
> 
> You are mistaken.  For a patch, you are dealing with two different
> parties: producer and consumer.  If you are adding new feature to the
> producer, the output format should be desigend to allow the consumer tell
> that it is something it does not know how to handle.

Agreed.

> Marking a non patch with "diff --git" to trigger the logic to signal the
> beginning of a patch to git-apply (and perhaps other tools) is a non
> starter.

This is not what I meant.

Show 5 quoted lines

> And for this "we are giving a patch that your git-apply can apply and gitk
> can show, but by the way we also think the whole directory foo moved to
> new location bar" is merely an additional information.  You should still
> be able to apply the patch with tools that are unaware of this new
> directory move detection feature.

I hope I just miss your point. Letting unaware tools handle such a patch "the right way" would imply just adding the information "dir foo moved to bar", and not removing the individual file moves, which goes in the way of the exact reason why I have started to work on this.

Compare this to the addition of the "file rename" feature (correct me if I'm wrong): it was added without bothering whether plain old "patch" can deal with it, and when feeding a git diff to patch(1) I cannot expect it to DTRT (indeed it applies the content change but not the rename part, without complaining, and an unsuspecting user would just be shot in the foot): we are precisely *not* able to apply the patch with tools that are unaware of this new file rename feature.

This is precisely to limit this problem in the future that I proposed this "diff extension" stuff in my last mail: limit how the "backward compatibility" argument can cripple innovation.