Re: [PATCH 1/3] Move bundle specific stuff into bundle.[ch]

Hi,

On Tue, 17 Jul 2007, Daniel Barkalow wrote:

On Tue, 17 Jul 2007, Daniel Barkalow wrote:

quoted

On Tue, 17 Jul 2007, Johannes Schindelin wrote:

quoted

The transport specific stuff was moved into libgit.a, and the bundle 
specific stuff will not be left behind.

This is a big code move, with one exception: the function unbundle() 
no longer outputs the list of refs.  You have to call 
list_bundle_refs() yourself for that.

You should use -C on this sort of thing, so that the interesting 
aspects of the patch are easier to see. (It actually comes out longer 
in this case, but it's far easier to tell that the code in the new 
file is the same as the old code.) Can you tell I've been rearranging 
a lot of code lately and trying to make the patches not look really 
scary?

Actually, I ended up touching this up a tiny bit, too: I ordered the 
functions in bundle.c the way they were in builtin-bundle.c (so that the 
patch is more trivial) and removed the blank lines at the end of the file. 
This makes the "git diff -C" output really obvious. 

Makes sense.  Thanks.

(Someday, I'd like to have a diff that can show that a substantial block 
of '+' lines matches a block of lines from somewhere in the "before" 
content, so reviewers can verify that the patch reorders code but 
doesn't change it, or changes it in certain ways. But, of course, that's 
both hard to generate and hard to display usefully.)

AFAIR from the thread after

http://thread.gmane.org/gmane.linux.kernel/484924/focus=37498

the problem was not so much the displaying as the applying.  You have a 
diff for builtin-bundle.c.  You want to move code to bundle.c.  git-diff 
has to rearrange the diff so that bundle.c can copy the code from 
builtin-bundle.c, and then it is deleted from builtin-bundle.c.  If you 
move code criss-cross, it is not possible.

However, if you do something like

diff --git a/builtin-bundle.c:10--20 b/bundle.c:0--30

code move

--- a/builtin-bundle.c:10--20
+++ b/bundle.c:1--11
diff --git ...

it should be possible.  We could allow generation of such a diff with 
--code-moves, which would detect if there is substantial evidence for a 
code move, and produce such a diff.

Note: this kind of code movement diff has to come before _both_ the diffs 
for builtin-bundle.c and bundle.c, since chances are that the code had to 
be touched a little here and there.  And probably you would want a little 
context, just to make sure it is the correct code when applying the patch.

Ciao,
Dscho