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.
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
