git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Yann Dirson <ydirson@altern.org>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/3] Introduce rename factorization in diffcore.
Date: Fri, 07 Nov 2008 15:43:19 -0800	[thread overview]
Message-ID: <7vhc6jdrvs.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: 20081107221224.GK5158@nan92-1-81-57-214-146.fbx.proxad.net

Yann Dirson <ydirson@altern.org> writes:

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

If your change is to move a/{1,2,3} to b/{1,2,3} and without content
change to a/{1,2} to b/{1,2}, then do you want to say "a/ moved to b/
and by the way here is the content change from a/3 to b/3" without saying
anything about a/{1,2} and b/{1,2}?

Two points.

 * I do not think it is a good idea to begin with.  If you are to apply
   such a patch (with git-apply that is updated with your patch to
   understand that notation) to the exact tree that has only {1,2,3} under
   a/, you would get an expected result.  But if the recipient of your
   patch has a/4 (or lacks a/2), there is no cue in the patch that
   automatically moving a/4 to b/4 may or may not be what is sane (or the
   patch is unapplicable in general).

 * If you give at least the names of paths that were moved without any
   content changes as I suggested, at least the recipient of your patch
   can catch the case where his tree is structurally different from what
   you used to prepare the patch for by noticing the a/2 in the patch that
   he does not have.

In addition, if you keep the movements for the paths whose contents did
not change, existing tools are perfectly capable of applying (or showing)
the output.  I seriously doubt that keeping 4 lines per perfectly moved
paths is too much a price to pay to keep backward compatibility.

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

Sorry, but that's an old history whie git-diff output format was rapidly
being developed, when we did not have that many users, and when we did not
have an old version of git-apply that did not understand the new feature
in majority of user's hands.

We do not have that kind of luxury anymore.  git is much more widespread
now and the majority of people use pre-1.6.1 git now (including me ;-)).

  reply	other threads:[~2008-11-07 23:44 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-01 22:03 [PATCH v3 0/3] Detection of directory renames Yann Dirson
2008-11-01 22:03 ` [PATCH 1/3] Fix error in diff_filepair::status documentation Yann Dirson
2008-11-02  6:31   ` Junio C Hamano
2008-11-01 22:03 ` [PATCH 2/3] Introduce rename factorization in diffcore Yann Dirson
2008-11-07  1:10   ` Junio C Hamano
2008-11-07 19:39     ` Yann Dirson
2008-11-07 20:19       ` Junio C Hamano
2008-11-07 20:39         ` Yann Dirson
2008-11-07 21:11           ` Junio C Hamano
2008-11-07 22:12             ` Yann Dirson
2008-11-07 23:43               ` Junio C Hamano [this message]
2008-11-08  0:29                 ` Yann Dirson
2008-11-08  0:47                   ` Junio C Hamano
2008-11-08  0:50                     ` Yann Dirson
2008-11-01 22:03 ` [PATCH 3/3] Add testcases for the --factorize-renames diffcore flag Yann Dirson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vhc6jdrvs.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=ydirson@altern.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).