Hi Antonin
On 17/11/2023 21:42, Antonin Delpeuch wrote:
Hi all,
Here a few more thoughts about this patch, to explain what brought me to
needing that. If this need is misguided, perhaps you could redirect me
to a better solution.
I am writing a custom merge driver for Java files. This merge driver
internally calls git-merge-file and then solves the merge conflicts
which only consist of import statements (there might be cases where it
gets it wrong, but I can then use other tools to cleanup those import
statements). When testing this, I noticed that the merge driver
performed more poorly on other sorts of conflicts, compared to the
standard "ort" merge strategy. This is because "ort" uses the
"histogram" diff algorithm, which gives better results than the "myers"
diff algorithm that merge-file uses.
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?
Intuitively, if "histogram" is the default diff algorithm used by "git
merge", then it would also make sense to have the same default for "git
merge-file", but I assume that changing this default could be considered
a bad breaking change. So I thought that making this diff algorithm
configurable would be an acceptable move, hence my patch.
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.
Of course, the diffing could be configured in other ways, for instance
with its handling of whitespace or EOL (similarly to what the "git-diff"
command offers). I think those options would definitely be worth
exposing in merge-file as well. If you think this makes sense, then I
would be happy to work on a new version of this patch which would
attempt to include all the relevant options. I could also try to add the
corresponding tests.
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.
Best Wishes
Phillip
But perhaps my need is misguided? Could it be that I should not be
writing a custom merge driver, but instead use another extension point
to only process the conflicting hunks after execution of the existing
merge driver? I couldn't find such an extension point, but it can well
be that I missed it.
Thank you,
Antonin
