Re: [BUG] diff algorithm selection issue
> When the config has diff.algorithm=patience set, "git diff --minimal" seems to
> be ignored, and does not give the same output as "git diff --diff-algorithm=minimal",
> but really the same as "git diff --diff-algorithm=patience".
Thanks for reporting. The document on --diff-algorithm does make it sound as if the --diff-algorithm=minimal option must operate as if myers algorithm is used with the minumalization tweak, but that is wrong from the point of view of the intent of the "minimal" option, which was meant to be a secondary option that tweaks the base algorithm (be it myers or patience or any other new algorithm we might introduce in the future) by allowing it to spend more cycles to come up with a smaller diff.
At least, it is what the "--minimal" option (not the value "minimal" given to the "--diff-algorithm=<algo>" option), and the underlying mechanism that supports the option, meant to do.
But the way the option is surfaced to the end-user facing UI (and the documentation) with "--diff-algorithm=minimal", it does look like
git -c diff.algorithm=patience cmd --diff-algorithm=minimal
git -c diff.algorithm=patience cmd --diff-algorithm=myers --minimal
ought to be equivalent.
Also, I suspect that
git -c diff.algorithm=patience cmd --diff-algorithm=myers
does not do what we expect, either.
I have not convinced myself that the attached is the best way to fix the issue, but anyway, somebody seems to be OR'ing in diff_algorithm to xdl_opts field after we see --diff-algorithm=minimal and replaced XDF_DIFF_ALGORITHM_MASK bits in xdl_opts field in this function, so the attached patch may defeat that code---the real bug is probably in that code, but I haven't figured out where it is X-<.
diff.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/diff.c b/diff.c
index 863da896c0..a6dba45cd6 100644
--- a/diff.c
+++ b/diff.c
@@ -5026,6 +5026,10 @@ static int diff_opt_diff_algorithm(const struct option *opt,
return error(_("option diff-algorithm accepts \"myers\", "
"\"minimal\", \"patience\" and \"histogram\""));
+ /* we shouldn't have to do this... */
+ if (!(value & XDF_DIFF_ALGORITHM_MASK))
+ diff_algorithm &= ~XDF_DIFF_ALGORITHM_MASK;
+
/* clear out previous settings */
DIFF_XDL_CLR(options, NEED_MINIMAL);
options->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;
