Re: [PATCH] merge-file: add --diff-algorithm option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Antonin

On 08/11/2023 21:54, Antonin Delpeuch via GitGitGadget wrote:
From: Antonin Delpeuch <antonin@xxxxxxxxxxx>

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 <antonin@xxxxxxxxxxx>
---
     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.

  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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux