Re: [PATCH] xdiff: re-diff shifted change groups when using histogram algorithm

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

Yee Cheng Chin <ychin.git@gmail.com> writes:

> Thanks for the review and sorry for being a little late in replying.
> Aggregating all my inline replies in one email if that's ok.
>
> On Wed, Jan 21, 2026 at 12:51 PM Junio C Hamano <gitster@pobox.com> wrote:
>> So the idea is to remember the original values in g and go (the
>> location of the group in the file and the other file) and if
>> shifting up and down changed any one of the four ends from the
>> original locations, we always take the fall-back route (if we are
>> doing histogram)?
>>
>> By the way, this appears after the if/else if/ cascade that has:
>>
>>         if (g.end == earliest_end) {
>>                 ... do nothing case (case #1)
>>         } else if (end_matching_other != -1) {
>>                 ... do the slide-up thing (case #2)
>>         } else if (flags & XDF_INDENT_HEIRISTIC) {
>>                 ... do the indent heuristic thing (case #3)
>>         }
>>
>> Am I reading the code correctly that, even though this new block
>> appears as if it is a post-clean-up phase that is independent from
>> which one of the three choices are taken in the previous if/elseif
>> cascade, it only is relevant to the second case?  I am wondering if
>> it would make it easier to follow if the new code were made into a
>> small helper function that is called from the (case #2) arm of the
>> existing if/else if cascade.
>
> That's correct. This condition happens only in the 2nd case. The
> problematic scenario here only happens when the opposite side is
> non-empty. If the opposite is empty (case #3, where we run the indent
> heuristic algorithm), there's simply no need to re-diff anything
> because diff'ing against an empty hunk is pointless.

OK.  In the version posted, it appeard that it is possible, after
not doing the slide-up thing but using indent heuristic thing, to
fall into this compensation codepath because the new code was placed
after the above if-else-if cascade as if it is an independent
clean-up phase.  Encapsulating that new code in a helper function
and calling it at the end of "do the slide-up thing" block will make
the intent clearer.

Thanks.