Re: [PATCH v7 1/3] Introduce bulk-move detection in diffcore.

[Junio C Hamano <gitster@pobox.com>]

Yann Dirson <ydirson@altern.org> writes:

> The output of raw diff is displayed as "Rnnn a/* b/".  Those cannot be
> confused with renames of files named "whatever/*" with a literal star
> character, from the full-zero SHA1's.

I do not particularly like this asterisk here.  It is ambiguous with the
case where you renamed a directory whose name is '*' below a/ to a new
directory whose name is 'b', isn't it?  Using 0{40} as a differentiator
like you did is probably a good idea, but then I do not see a sound reason
you would need nor want that asterisk.

> +Bulk move of all files of a directory into a different one can get
> +detected using the `\--detect-bulk-moves` option.  This adds an
> +additional pass on top of the results of per-file rename detection.
> +They are reported with NULL SHA1 id, in addition to the file renames:
> +
> +------------------------------------------------
> +:040000 040000 0000000... 0000000... R100	a/*	b/
> +:100644 100644 0123456... 1234567... R90	a/file0	b/file3
> +:100644 100644 0123456... 1234567... R100	a/file1	b/file1
> +:100644 100644 0123456... 1234567... R100	a/file2	b/file2

This is kind of interesting.  Let's address two issues that should be
uncontroversial:

 (1) please do not use a/ and b/, as the reader can easily be confused and
     mistakenly think you are referring to the pre-/post- indicator we use
     when writing textual diffs (e.g. "diff --git a/frotz b/frotz").  You
     are illustrating rename between a directory to another directory,
     both of which are tracked paths.

 (2) The last two lines are R100 but with different object names between
     pre- and post-images, which is wrong and unnecessarily puzzling.
     Please make them match.

These obvious two complaints behind us, there is one more interesting
thing in the above, which is _not_ a complaint.

What about renaming of a/file0 to b/file3?  Is this part of "all files
from directory A moved to directory B"?  IOW, is the goal of this series
to use the "A/* -> B/" to label the change as bulk directory rename, if
the preimage has A/{1,2,3} and the postimage has their moved contents in
B/{one,two,three}?

I am wondering about the utility of such an extra information.  If there
were no "a/file0 -> b/file3" entry in the example, I would imagine that we
could use this "a/* -> b/" information to move "a/file5" to "b/file5" when
rebasing this patch to apply to a different preimage that had files other
than file{1,2} in directory "a", and I would further imagine that might be
a wonderful thing.

But if the new "a/* -> b/" hint only gives "things from A/ have migrated
to B/ but I can give no information on what name they took under their new
home", that may probably reduce the utility of this feature.

I dunno.