On Thu, Apr 14, 2016 at 7:09 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>>
>> +static int starts_with_emptyline(const char *recs)
>> +{
>> + return recs[0] == '\n'; /* CRLF not covered here */
>> +}
>> +
>> +
>
> That's "is-empty-line", not "starts-with" ;-)
heh, ok.
To understand the code, I was debugging it and looking at the
pointers `recs[ix - 1]->ptr` which is pointing into the text file,
i.e. when printing it in the debugger it would read
'\n\tbla\n\nfoo\n ...'
so I found that a proper description at the time.
>
>> +
>> + /*
>> + * If a group can be moved back and forth, see if there is an
>> + * empty line in the moving space. If there is an empty line,
>> + * make sure the last empty line is the end of the group.
>> + *
>> + * As we shifted the group forward as far as possible, we only
>> + * need to shift it back if at all.
>> + */
>
> Sounds sensible.
>
>> + if (has_emptyline) {
>> + while (ixs > 0 && recs[ixs - 1]->ha == recs[ix - 1]->ha &&
>> + xdl_recmatch(recs[ixs - 1]->ptr, recs[ixs - 1]->size, recs[ix - 1]->ptr, recs[ix - 1]->size, flags) &&
>> + !starts_with_emptyline(recs[ix - 1]->ptr)) {
>
> You probably want to wrap the "hash compares equal and recmatch does
> say they are the same" into a helper function (to be automatically
> inlined by the compiler) to make it more readable here. I think
> is-empty is a lot cheaper than the recmatch so that should probably
> be done earlier in the && chain.
ok, will do. Given that xdiff upstream and our code diverged over the years,
I could apply this helper function at other places in the code as well.
>
>> + rchg[--ixs] = 1;
>> + rchg[--ix] = 0;
>> +
>> + /*
>> + * This change did not join two change groups,
>> + * as we did that before already, so there is no
>
> Sorry, cannot quite parse the part before "already".
I think to drop this comment in the final version of this patch.
As I `wrote` this loop by copying it from above, I tried justifying
each change to it. (More to prove to myself I understood the code)
will drop this comment.
>
>> + * need to adapt the other-file, i.e.
>> + * running
>> + * for (; rchg[ixs - 1]; ixs--);
>> + * while (rchgo[--ixo]);
>> + */
>> + }
>> + }
>> }
>>
>> return 0;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html