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

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

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 25/01/2026 17:34, Junio C Hamano wrote:
>> Phillip Wood <phillip.wood123@gmail.com> writes:
>> 
>>> On 21/01/2026 20:51, Junio C Hamano wrote:
>>>> "Yee Cheng Chin via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>>>
>>>>> @@ -915,6 +919,45 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
>>>>>    			}
>>>>>    		}
>>>>>    
>>>>> +		/*
>>>>> +		 * If this has a matching group from the other file, it could
>>>>> +		 * either be the original match from the diff algorithm, or
>>>>> +		 * arrived at by shifting and joining groups. When it's the
>>>>> +		 * latter, it's possible for the two newly joined sides to have
>>>>> +		 * matching lines. Re-diff the group to mark these matching
>>>>> +		 * lines as unchanged and remove from the diff output.
>> 
>> Also, after reading the first paragraph of the big comment again, it
>> makes me wonder if it is saying the same thing as "When histogram is
>> being used, we shouldn't bother shifting up and down to join groups,
>> as the result will always worse than the fallback", but is it that
>> bad?
>
> Looking at the example in the commit message the result of shifting up 
> and down and then calling the fallback is better than either the 
> unshifted diff or shifting without the fallback, so I don't think just 
> disabling shifting improves things. It would also stop us coalescing 
> changed lines, for example
>
> -A             A
>   A     ->     -A
> -B            -B
>
> The indent heuristic seems to assume that we've shifted down as far as 
> possible before trying it so that would probably get messed up as well. 

I see.  Thanks for a good explanation.

> To me the problem is that the histogram diff does not always generate 
> particularly good diffs (maybe I'm biased - whenever I've tried 
> switching the default to "histogram" I've always switched back 
> "patience" fairly quickly after being presented with a diff that I found 
> hard to comprehend)
>
> Thanks
>
> Phillip

Thanks.