From: Phillip Wood Date: Sun, 19 Nov 2023 16:42:53 GMT Subject: Re: [PATCH] merge-file: add --diff-algorithm option Message-ID: In-Reply-To: Hi Antonin On 08/11/2023 21:54, Antonin Delpeuch via GitGitGadget wrote: > From: Antonin Delpeuch > > This makes it possible to use other diff algorithms than the 'myers' > default algorithm, when using the 'git merge-file' command. I think being able to select the diff algorithm is reasonable. I might be nice to mention the use of "git merge-file" in custom merge drivers as a motivation in the commit message. > Signed-off-by: Antonin Delpeuch > --- > merge-file: add --diff-algorithm option > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1606%2Fwetneb%2Fmerge_file_configurable_diff_algorithm-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1606/wetneb/merge_file_configurable_diff_algorithm-v1 > Pull-Request: https://github.com/git/git/pull/1606 > > Documentation/git-merge-file.txt | 5 +++++ > builtin/merge-file.c | 28 ++++++++++++++++++++++++++++ > 2 files changed, 33 insertions(+) > > diff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt > index 6a081eacb72..917535217c1 100644 > --- a/Documentation/git-merge-file.txt > +++ b/Documentation/git-merge-file.txt > @@ -92,6 +92,11 @@ object store and the object ID of its blob is written to standard output. > Instead of leaving conflicts in the file, resolve conflicts > favouring our (or their or both) side of the lines. > > +--diff-algorithm :: > + Use a different diff algorithm while merging, which can help > + avoid mismerges that occur due to unimportant matching lines > + (such as braces from distinct functions). See also > + linkgit:git-diff[1] `--diff-algorithm`. Perhaps we could list the available algorithms here so the user does not have to go searching for them in another man page. > EXAMPLES > -------- > diff --git a/builtin/merge-file.c b/builtin/merge-file.c > index 832c93d8d54..1f987334a31 100644 > --- a/builtin/merge-file.c > +++ b/builtin/merge-file.c > @@ -1,5 +1,6 @@ > #include "builtin.h" > #include "abspath.h" > +#include "diff.h" > #include "hex.h" > #include "object-name.h" > #include "object-store.h" > @@ -28,6 +29,30 @@ static int label_cb(const struct option *opt, const char *arg, int unset) > return 0; > } > > +static int set_diff_algorithm(xpparam_t *xpp, > + const char *alg) > +{ > + long diff_algorithm = parse_algorithm_value(alg); > + if (diff_algorithm < 0) > + return -1; > + xpp->flags = (xpp->flags & ~XDF_DIFF_ALGORITHM_MASK) | diff_algorithm; > + return 0; > +} > + > +static int diff_algorithm_cb(const struct option *opt, > + const char *arg, int unset) > +{ > + xpparam_t *xpp = opt->value; > + > + BUG_ON_OPT_NEG(unset); > + > + if (set_diff_algorithm(xpp, arg)) > + return error(_("option diff-algorithm accepts \"myers\", " > + "\"minimal\", \"patience\" and \"histogram\"")); > + > + return 0; > +} > + > int cmd_merge_file(int argc, const char **argv, const char *prefix) > { > const char *names[3] = { 0 }; > @@ -48,6 +73,9 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix) > XDL_MERGE_FAVOR_THEIRS), > OPT_SET_INT(0, "union", &xmp.favor, N_("for conflicts, use a union version"), > XDL_MERGE_FAVOR_UNION), > + OPT_CALLBACK_F(0, "diff-algorithm", &xmp.xpp, N_(""), > + N_("choose a diff algorithm"), > + PARSE_OPT_NONEG, diff_algorithm_cb), > OPT_INTEGER(0, "marker-size", &xmp.marker_size, > N_("for conflicts, use this marker size")), > OPT__QUIET(&quiet, N_("do not warn about conflicts")), This patch looks sensible to me, it would be nice to have some tests though. Best Wishes Phillip > base-commit: 98009afd24e2304bf923a64750340423473809ff