Re: [PATCH] array index mixup

Re: [PATCH] array index mixup

[linux]

> *1* If somebody wants to do this, one thing to watch out for is
> matching up of broken pairs.  If a pair originally broken by
> diffcore-break (because they were dissimilar enough according to
> the option given to -B flag) are merged into one by
> diffcore-rename (because they were similar enough according to
> the option given to -M flag), we should _not_ say the resulting
> pair is renamed.  In general, the threashold for breaking should
> be lower than diffcore-rename to merge them for a sane use, so
> this might be a non-issue in practice, though.

Er... no.  You want to be fairly aggressive when doing both things.
That is, you want to break aggressively so you can look for a better
match elsewhere, but once you've found the best match, you don't want to
be shy about accepting it.

Pulling numbers out of thin air, say break if 1/3 of a file has
changed (66% common), and merge if you have 33% common.  Or maybe
even less.  The point of break then merge is to give you a chance
to find the 90% common file that has a new name.

I always understood that the reason for having two thresholds
is exactly so they can have this relationship, not the opposite
one as you suggest.

Strange output of 'git diff <revision1>:<file> <revision2>:<file>'

[Jakub Narebski]

git diff output for files specified by revision is somewhat unexpected. 

  $ git diff <revision_1>:<file> <revision_2>:<file>

outputs the following diff metainfo

  diff --git a/<revision_2>:<file> b/<revision_2>:<file>
  index 5eabe06..2e87de4 100644
  --- a/<revision_2>:<file>
  +++ b/<revision_2>:<file>

Is it intended, or is it a bug? Looks like a bug to me...

git 1.4.0 (git-core-1.4.0-1.fc4 by Fedora Project).
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

[PATCH] array index mixup

[Matthias Lederhofer]

---
Jakub Narebski <jnareb@gmail.com> wrote:
> git diff output for files specified by revision is somewhat unexpected. 
> 
>   $ git diff <revision_1>:<file> <revision_2>:<file>
> 
> outputs the following diff metainfo
> 
>   diff --git a/<revision_2>:<file> b/<revision_2>:<file>
>   index 5eabe06..2e87de4 100644
>   --- a/<revision_2>:<file>
>   +++ b/<revision_2>:<file>
> 
> Is it intended, or is it a bug? Looks like a bug to me...
I dunno if this is really an index mixup or was intended.  If this is
intended please add a comment what it's for.  (Without this you get
rename information, perhaps this is the reason.)
---
 builtin-diff.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-diff.c b/builtin-diff.c
index cb38f44..4d43a5c 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -136,7 +136,7 @@ static int builtin_diff_blobs(struct rev
 	stuff_change(&revs->diffopt,
 		     mode, mode,
 		     blob[1].sha1, blob[0].sha1,
-		     blob[0].name, blob[0].name);
+		     blob[1].name, blob[0].name);
 	diffcore_std(&revs->diffopt);
 	diff_flush(&revs->diffopt);
 	return 0;
-- 
1.4.1.ga3e6

Re: [PATCH] array index mixup

[Junio C Hamano]

Matthias Lederhofer <matled@gmx.net> writes:

> I dunno if this is really an index mixup or was intended.  If this is
> intended please add a comment what it's for.  (Without this you get
> rename information, perhaps this is the reason.)

That is exactly the reason -- it was a temporary workaround
which nobody noticed so far.

The right fix would involve updating diff_resolve_rename_copy so
that it does not rely on the comparison of path names (that
means DIFF_PAIR_RENAME() macro needs to change), and instead
mark the pairs synthesized in diffcore-rename as such, and use
that to tell if a pair is a result of rename/copy [*1*].

Your other patch (not the one to change the index of the array
used for labels, but the one that extracts the pathname out of
the syntax to name a blob by path in an arbitrary tree object)
could be safely applied when that happens.


[Footnote]

*1* If somebody wants to do this, one thing to watch out for is
matching up of broken pairs.  If a pair originally broken by
diffcore-break (because they were dissimilar enough according to
the option given to -B flag) are merged into one by
diffcore-rename (because they were similar enough according to
the option given to -M flag), we should _not_ say the resulting
pair is renamed.  In general, the threashold for breaking should
be lower than diffcore-rename to merge them for a sane use, so
this might be a non-issue in practice, though.