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

Re: [PATCH] merge-file: add --diff-algorithm option

From
Antonin Delpeuch <antonin@delpeuch.eu>
Date
Nov 19, 2023, 19:29 UTC
Message-ID
<5da59ea3-f814-4345-8206-856ae68be1c9@delpeuch.eu>
In-Reply-To
<de04aec0-a195-45da-8951-bb30f2a629a3@gmail.com>
Hi Phillip,
Thank you so much for taking the time to review this!
On 19/11/2023 17:43, Phillip Wood wrote:
> I cannot comment on this particular use but I think in general calling 
> "git merge-file" from a custom merge driver is perfectly sensible. 
> Have you tested your driver with this patch to see if you get better 
> results with the histogram diff algorithm?
Yes, I can confirm that the results are better in my use case indeed.
Show 7 quoted lines
> I can see there's an argument for changing the default algorithm of 
> "git merge-file" to match what "ort" uses. I know Elijah found the 
> histogram algorithm gave better results in his testing when he was 
> developing "ort". While it would be a breaking change if on the 
> average the new default gives better conflicts it might be worth it. 
> This patch would mean that someone wanting to use the "myers" 
> algorithm could still do so.

Agreed. I would be happy to submit a follow-up patch to change the default. Or would you prefer to have it in the same patch (as a separate commit)? I was worried this would make my patch less likely to get merged.

> It would be nice to see some tests for this patch, ideally using a 
> test case that gives different conflicts for "myers" and "histogram". 
> We could add the other options later if there is a demand.
Will do.
> Perhaps we could list the available algorithms here so the user does 
> not have to go searching for them in another man page.

This part is copied from "Documentation/merge-strategies.txt", which redirects to the manual for git-diff in the same way. I assume it was done so that whenever a new diff algorithm is introduced, it only needs documenting in one place. But I agree it is definitely more user-friendly to list the algorithms directly. Should I change the documentation of merge strategies in the same way?

Best wishes,
Antonin
Previous: Phillip WoodNext: Junio C Hamano
Message 4 of 8 in “merge-file: add --diff-algorithm option”
  1. merge-file: add --diff-algorithm optionAntonin Delpeuch via GitGitGadget, Nov 8, 2023
  2. Antonin DelpeuchNov 17, 2023
  3. Phillip WoodNov 19, 2023
  4. Antonin DelpeuchNov 19, 2023
  5. Junio C HamanoNov 19, 2023
  6. Phillip WoodNov 19, 2023
  7. merge-file: add --diff-algorithm optionAntonin Delpeuch via GitGitGadget, Nov 20, 2023
  8. Phillip WoodNov 21, 2023

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.