git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 29, 2026, 20:58 UTC
Message-ID
<xmqqsebo9lv6.fsf@gitster.g>
In-Reply-To
<CAHTeOx8SOZmqvi0pkcheSjFpbEALmOwaUiX0tKLmNP7fqvjMXA@mail.gmail.com>
Yee Cheng Chin <ychin.git@gmail.com> writes:
Show 33 quoted lines
> 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.
Previous: Yee Cheng ChinNext: Yee Cheng Chin
Message 8 of 17 in “xdiff: re-diff shifted change groups when using histogram algorithm”
  1. xdiff: re-diff shifted change groups when using histogram algorithmYee Cheng Chin via GitGitGadget, Dec 6, 2025
  2. Junio C HamanoJan 21, 2026
  3. Phillip WoodJan 24, 2026
  4. Junio C HamanoJan 25, 2026
  5. Phillip WoodJan 26, 2026
  6. Junio C HamanoJan 26, 2026
  7. Yee Cheng ChinJan 29, 2026
  8. Junio C HamanoJan 29, 2026
  9. Yee Cheng ChinJan 30, 2026
  10. Junio C HamanoJan 30, 2026
  11. Phillip WoodJan 30, 2026
  12. Junio C HamanoFeb 20, 2026
  13. Yee Cheng ChinFeb 21, 2026
  14. xdiff: re-diff shifted change groups when using histogram algorithmYee Cheng Chin via GitGitGadget, Mar 2, 2026
  15. Junio C HamanoMar 13, 2026
  16. Phillip WoodMar 13, 2026
  17. Yee Cheng ChinMar 19, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.