Re: [PATCH] merge-file: add --diff-algorithm option
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Nov 19, 2023, 16:42 UTC
- Message-ID
- <a83321f0-7184-4779-82d2-854a1e324f92@gmail.com>
- In-Reply-To
- <pull.1606.git.git.1699480494355.gitgitgadget@gmail.com>
Hi Antonin
On 08/11/2023 21:54, Antonin Delpeuch via GitGitGadget wrote:
> From: Antonin Delpeuch <antonin@delpeuch.eu> > > 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.
Show 25 quoted lines
> Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu> > --- > 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 <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.
Show 54 quoted lines
> 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_("<algorithm>"),
> + 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