Re: [PATCH v8 1/5] Introduce bulk-move detection in diffcore.

[Jonathan Nieder <jrnieder@gmail.com>]

Yann Dirson wrote:

> Hm, I fear too much granularity would become meaningless :)
[...]
>                 If the code needs to be easier to understand, I'd
> rather add some more doc, than added commits that are basically
> "useless for bisect".

Yes, the example list was probably a little overboard.  Even so, the
idea was to maintain bisectability by introducing a few tests at a
time.  That way, the fallout of a subfeature can easily be found with
"git bisect", among other benefits[1].

I might try it out some time soon; don't worry about it if you don't
like the idea. :)

> On Thu, Oct 28, 2010 at 08:45:40PM -0500, Jonathan Nieder wrote:

>> 1. introduce DETECT_DIRECTORY_RENAMES flag and hidden UI for it.
>
> What do you mean by "hidden UI" ?

PARSE_OPT_HIDDEN (i.e., a commandline option not tempting readers by
listing in -h).

>> Is the debugging output infrequent enough to just use a function
>> unconditionally?
>
> You mean, keep funccalls even with DEBUG_BULKMOVE is not set ?  No,
> there are too many traces for that.

Yep, that's what I meant.  Alas.

Will the debugging output still be useful once the code is known
to work reliably?  If not, I guess we can remove it at that point
(avoiding the need to worry about patches that may introduce

	debug_bulkmove(expression that does not even
		       typecheck);

down the line).

> Ah, sure the dst==src case can be improved.  But I'm not sure
> factorizing writing NUL is worth the cost of re-computing where to put
> it when using mempcpy would avoid.  Wouldn't the following be more
> adequate ?
> 
> 	if (dst != src) {
> 		end = mempcpy(dst, src, slash - src + 1);
> 		*end = '\0';
> 	} else
> 		dst[slash - src + 1] = '\0';
> 	return dst;

I doubt the difference is measurable.  But that looks fine (micronit:
I suppose one should put the dst == src case first in that case).

Jonathan

[1] The example that set me on this path to madness:
http://thread.gmane.org/gmane.comp.version-control.git/151086/focus=158913