From: Antonin Delpeuch Date: Sun, 19 Nov 2023 19:29:41 GMT Subject: Re: [PATCH] merge-file: add --diff-algorithm option Message-ID: <5da59ea3-f814-4345-8206-856ae68be1c9@delpeuch.eu> In-Reply-To: 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