Re: [PATCH 02/12] xdiff-merge: optionally show conflicts in "diff3 -m" style
From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:45:15
Hi, On Fri, 29 Aug 2008, Junio C Hamano wrote:
quoted hunk ↗ jump to hunk
diff --git a/builtin-merge-file.c b/builtin-merge-file.c index 3605960..5b4f020 100644 --- a/builtin-merge-file.c +++ b/builtin-merge-file.c@@ -25,6 +27,10 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix) else if (!strcmp(argv[1], "-q") || !strcmp(argv[1], "--quiet")) freopen("/dev/null", "w", stderr); + else if (!strcmp(argv[1], "--diff3")) { + merge_style = XDL_MERGE_DIFF3; + merge_level = XDL_MERGE_EAGER; + }
FWIW I do not follow your reasoning why --diff3 does not make sense for anything more eager than MERGE_EAGER. All that ZEALOUS and ZEALOUS_ALNUM (the latter of which is useless at the moment, since it is not enabled for git-merge) do is change the way the conflicting regions are displayed, but they do not leave out conflicting regions. So I actually suspect that ZEALOUS_ALNUM will be _especially_ useful with --diff3, since it is designed to skip the single curly brackets that would disrupt the reading pleasure otherwise.
quoted hunk ↗ jump to hunk
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h index 413082e..deebe02 100644 --- a/xdiff/xdiff.h +++ b/xdiff/xdiff.h@@ -50,10 +50,16 @@ extern "C" { #define XDL_BDOP_CPY 2 #define XDL_BDOP_INSB 3 +/* merge simplification levels */ #define XDL_MERGE_MINIMAL 0 #define XDL_MERGE_EAGER 1 #define XDL_MERGE_ZEALOUS 2 #define XDL_MERGE_ZEALOUS_ALNUM 3 +#define XDL_MERGE_LEVEL_MASK 0x0f + +/* merge output styles */ +#define XDL_MERGE_DIFF3 0x8000 +#define XDL_MERGE_STYLE_MASK 0x8000
Hmm. This is not the Linux kernel, I think we could safely pass around two integers instead of one.
quoted hunk ↗ jump to hunk
@@ -91,11 +108,13 @@ static int xdl_merge_cmp_lines(xdfenv_t *xe1, int i1, xdfenv_t *xe2, int i2, return 0; } -static int xdl_recs_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest) +static int xdl_recs_copy_0(int use_orig, xdfenv_t *xe, int i, int count, int add_nl, char *dest)
You rewrapped many function headers already; I wonder why this one was left out. The rest looks pretty much obviously correct to me; I was too lazy/ran out of time to apply the patch and look through the resulting code, though, but I guess that you searched for "i1" and "chg1" and added the code for i0 and chg0 where necessary. So: ACK. Ciao, Dscho