git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] merge-file: add option to specify the marker size
@ 2010-02-28 19:56 Bert Wesarg
  2010-02-28 19:56 ` [PATCH 2/3] make union merge an xdl merge favor Bert Wesarg
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Bert Wesarg @ 2010-02-28 19:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Bert Wesarg, git

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>

---

This can probably improved in a way, that the marker size will be taken
from attributes. This could be done by an explicit --marker-size-by-path
option or an option which names one of the three input files as a git
path.

For example:

 $ echo "foo conflict-marker-size=32" > .gitattributes
 $ git merge-file --marker-size-by-path=foo fileA fileB fileC

=> marker size would be 32

Or:

 $ echo "fileC conflict-marker-size=32" > .gitattributes
 $ git merge-file --marker-size-by-source=theirs fileA fileB fileC

=> marker size would be 32

 Documentation/git-merge-file.txt |    5 +++--
 builtin-merge-file.c             |    1 +
 t/t6023-merge-file.sh            |   37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt
index 234269a..a5b9c1f 100644
--- a/Documentation/git-merge-file.txt
+++ b/Documentation/git-merge-file.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]
-	[--ours|--theirs] [-p|--stdout] [-q|--quiet]
+	[--ours|--theirs] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]
 	<current-file> <base-file> <other-file>
 
 
@@ -37,7 +37,8 @@ normally outputs a warning and brackets the conflict with lines containing
 If there are conflicts, the user should edit the result and delete one of
 the alternatives.  When `--ours` or `--theirs` option is in effect, however,
 these conflicts are resolved favouring lines from `<current-file>` or
-lines from `<other-file>` respectively.
+lines from `<other-file>` respectively.  The length of the conflict markers
+can be given with the `--marker-size` option.
 
 The exit value of this program is negative on error, and the number of
 conflicts otherwise. If the merge was clean, the exit value is 0.
diff --git a/builtin-merge-file.c b/builtin-merge-file.c
index 1e70073..57d757c 100644
--- a/builtin-merge-file.c
+++ b/builtin-merge-file.c
@@ -39,6 +39,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 			    XDL_MERGE_FAVOR_OURS),
 		OPT_SET_INT(0, "theirs", &favor, "for conflicts, use their version",
 			    XDL_MERGE_FAVOR_THEIRS),
+		OPT_INTEGER(0, "marker-size", &xmp.marker_size, "for conflicts, use this marker size"),
 		OPT__QUIET(&quiet),
 		OPT_CALLBACK('L', NULL, names, "name",
 			     "set labels for file1/orig_file/file2", &label_cb),
diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh
index d605024..5034dd1 100755
--- a/t/t6023-merge-file.sh
+++ b/t/t6023-merge-file.sh
@@ -215,4 +215,41 @@ test_expect_success '"diff3 -m" style output (2)' '
 	test_cmp expect actual
 '
 
+cat >expect <<\EOF
+Dominus regit me,
+<<<<<<<<<< new8.txt
+et nihil mihi deerit;
+
+
+
+
+In loco pascuae ibi me collocavit;
+super aquam refectionis educavit me.
+||||||||||
+et nihil mihi deerit.
+In loco pascuae ibi me collocavit,
+super aquam refectionis educavit me;
+==========
+et nihil mihi deerit,
+
+
+
+
+In loco pascuae ibi me collocavit --
+super aquam refectionis educavit me,
+>>>>>>>>>> new9.txt
+animam meam convertit,
+deduxit me super semitas jusitiae,
+propter nomen suum.
+Nam et si ambulavero in medio umbrae mortis,
+non timebo mala, quoniam TU mecum es:
+virga tua et baculus tuus ipsa me consolata sunt.
+EOF
+
+test_expect_success 'marker size' '
+	test_must_fail git merge-file -p --marker-size=10 \
+		new8.txt new5.txt new9.txt >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.0.584.g2da2b

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] make union merge an xdl merge favor
  2010-02-28 19:56 [PATCH 1/3] merge-file: add option to specify the marker size Bert Wesarg
@ 2010-02-28 19:56 ` Bert Wesarg
  2010-02-28 20:15   ` Junio C Hamano
  2010-02-28 19:56 ` [PATCH 3/3] refactor merge flags into xmparam_t Bert Wesarg
  2010-02-28 20:13 ` [PATCH 1/3] merge-file: add option to specify the marker size Junio C Hamano
  2 siblings, 1 reply; 10+ messages in thread
From: Bert Wesarg @ 2010-02-28 19:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Bert Wesarg, Johannes Schindelin, git

The current union merge driver is implemented as an post process.  But the
xdl_merge code is quite capable to produce the result by itself.  Therefore
move to it there and teach git-merge-file a new --union option.

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 Documentation/git-merge-file.txt |    2 +-
 builtin-merge-file.c             |    2 +
 ll-merge.c                       |   43 ++++---------------------------------
 xdiff/xdiff.h                    |    1 +
 xdiff/xmerge.c                   |   20 +++++++++++------
 5 files changed, 22 insertions(+), 46 deletions(-)

diff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt
index a5b9c1f..11dff6f 100644
--- a/Documentation/git-merge-file.txt
+++ b/Documentation/git-merge-file.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]
-	[--ours|--theirs] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]
+	[--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]
 	<current-file> <base-file> <other-file>
 
 
diff --git a/builtin-merge-file.c b/builtin-merge-file.c
index 57d757c..f69dad6 100644
--- a/builtin-merge-file.c
+++ b/builtin-merge-file.c
@@ -39,6 +39,8 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 			    XDL_MERGE_FAVOR_OURS),
 		OPT_SET_INT(0, "theirs", &favor, "for conflicts, use their version",
 			    XDL_MERGE_FAVOR_THEIRS),
+		OPT_SET_INT(0, "union", &xmp.favor, "for conflicts, use a union version",
+			    XDL_MERGE_FAVOR_UNION),
 		OPT_INTEGER(0, "marker-size", &xmp.marker_size, "for conflicts, use this marker size"),
 		OPT__QUIET(&quiet),
 		OPT_CALLBACK('L', NULL, names, "name",
diff --git a/ll-merge.c b/ll-merge.c
index 4c7f11b..a4b2f4c 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -98,44 +98,11 @@ static int ll_union_merge(const struct ll_merge_driver *drv_unused,
 			  mmfile_t *src2, const char *name2,
 			  int flag, int marker_size)
 {
-	char *src, *dst;
-	long size;
-	int status, saved_style;
-
-	/* We have to force the RCS "merge" style */
-	saved_style = git_xmerge_style;
-	git_xmerge_style = 0;
-	status = ll_xdl_merge(drv_unused, result, path_unused,
-			      orig, src1, NULL, src2, NULL,
-			      flag, marker_size);
-	git_xmerge_style = saved_style;
-	if (status <= 0)
-		return status;
-	size = result->size;
-	src = dst = result->ptr;
-	while (size) {
-		char ch;
-		if ((marker_size < size) &&
-		    (*src == '<' || *src == '=' || *src == '>')) {
-			int i;
-			ch = *src;
-			for (i = 0; i < marker_size; i++)
-				if (src[i] != ch)
-					goto not_a_marker;
-			if (src[marker_size] != '\n')
-				goto not_a_marker;
-			src += marker_size + 1;
-			size -= marker_size + 1;
-			continue;
-		}
-	not_a_marker:
-		do {
-			ch = *src++;
-			*dst++ = ch;
-			size--;
-		} while (ch != '\n' && size);
-	}
-	result->size = dst - result->ptr;
+	/* Use union favor */
+	flag = (flag & 1) | (XDL_MERGE_FAVOR_UNION << 1);
+	return ll_xdl_merge(drv_unused, result, path_unused,
+			    orig, src1, NULL, src2, NULL,
+			    flag, marker_size);
 	return 0;
 }
 
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index 3f6229e..22614d5 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -61,6 +61,7 @@ extern "C" {
 /* merge favor modes */
 #define XDL_MERGE_FAVOR_OURS 1
 #define XDL_MERGE_FAVOR_THEIRS 2
+#define XDL_MERGE_FAVOR_UNION 3
 #define XDL_MERGE_FAVOR(flags) (((flags)>>4) & 3)
 #define XDL_MERGE_FLAGS(level, style, favor) ((level)|(style)|((favor)<<4))
 
diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
index 8cbe45e..c901ecb 100644
--- a/xdiff/xmerge.c
+++ b/xdiff/xmerge.c
@@ -28,6 +28,7 @@ typedef struct s_xdmerge {
 	 * 0 = conflict,
 	 * 1 = no conflict, take first,
 	 * 2 = no conflict, take second.
+	 * 3 = no conflict, take both.
 	 */
 	int mode;
 	/*
@@ -230,14 +231,19 @@ static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,
 			size = fill_conflict_hunk(xe1, name1, xe2, name2,
 						  size, i, style, m, dest,
 						  marker_size);
-		else if (m->mode == 1)
-			size += xdl_recs_copy(xe1, i, m->i1 + m->chg1 - i, 0,
+		else if (m->mode & 3) {
+			/* Before conflicting part */
+			size += xdl_recs_copy(xe1, i, m->i1 - i, 0,
 					      dest ? dest + size : NULL);
-		else if (m->mode == 2)
-			size += xdl_recs_copy(xe2, m->i2 - m->i1 + i,
-					      m->i1 + m->chg2 - i, 0,
-					      dest ? dest + size : NULL);
-		else
+			/* Postimage from side #1 */
+			if (m->mode & 1)
+				size += xdl_recs_copy(xe1, m->i1, m->chg1, 1,
+						      dest ? dest + size : NULL);
+			/* Postimage from side #2 */
+			if (m->mode & 2)
+				size += xdl_recs_copy(xe2, m->i2, m->chg2, 1,
+						      dest ? dest + size : NULL);
+		} else
 			continue;
 		i = m->i1 + m->chg1;
 	}
-- 
1.7.0.584.g2da2b

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] refactor merge flags into xmparam_t
  2010-02-28 19:56 [PATCH 1/3] merge-file: add option to specify the marker size Bert Wesarg
  2010-02-28 19:56 ` [PATCH 2/3] make union merge an xdl merge favor Bert Wesarg
@ 2010-02-28 19:56 ` Bert Wesarg
  2010-02-28 20:13 ` [PATCH 1/3] merge-file: add option to specify the marker size Junio C Hamano
  2 siblings, 0 replies; 10+ messages in thread
From: Bert Wesarg @ 2010-02-28 19:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Bert Wesarg, git

Include the merge level, favor, and style flags into the xmparam_t struct.
This removes the bit twiddling with these three values into the one flags
parameter.

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 builtin-merge-file.c |   18 ++++++++++--------
 ll-merge.c           |   12 ++++--------
 xdiff/xdiff.h        |   11 +++++------
 xdiff/xmerge.c       |   12 ++++++------
 4 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/builtin-merge-file.c b/builtin-merge-file.c
index f69dad6..8a70d0e 100644
--- a/builtin-merge-file.c
+++ b/builtin-merge-file.c
@@ -27,17 +27,19 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 	mmbuffer_t result = {NULL, 0};
 	xmparam_t xmp = {{XDF_NEED_MINIMAL}};
 	int ret = 0, i = 0, to_stdout = 0;
-	int level = XDL_MERGE_ZEALOUS_ALNUM;
-	int style = 0, quiet = 0;
-	int favor = 0;
+	int quiet = 0;
 	int nongit;
 
+	xmp.level = XDL_MERGE_ZEALOUS_ALNUM;
+	xmp.style = 0;
+	xmp.favor = 0;
+
 	struct option options[] = {
 		OPT_BOOLEAN('p', "stdout", &to_stdout, "send results to standard output"),
-		OPT_SET_INT(0, "diff3", &style, "use a diff3 based merge", XDL_MERGE_DIFF3),
-		OPT_SET_INT(0, "ours", &favor, "for conflicts, use our version",
+		OPT_SET_INT(0, "diff3", &xmp.style, "use a diff3 based merge", XDL_MERGE_DIFF3),
+		OPT_SET_INT(0, "ours", &xmp.favor, "for conflicts, use our version",
 			    XDL_MERGE_FAVOR_OURS),
-		OPT_SET_INT(0, "theirs", &favor, "for conflicts, use their version",
+		OPT_SET_INT(0, "theirs", &xmp.favor, "for conflicts, use their version",
 			    XDL_MERGE_FAVOR_THEIRS),
 		OPT_SET_INT(0, "union", &xmp.favor, "for conflicts, use a union version",
 			    XDL_MERGE_FAVOR_UNION),
@@ -53,7 +55,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 		/* Read the configuration file */
 		git_config(git_xmerge_config, NULL);
 		if (0 <= git_xmerge_style)
-			style = git_xmerge_style;
+			xmp.style = git_xmerge_style;
 	}
 
 	argc = parse_options(argc, argv, prefix, options, merge_file_usage, 0);
@@ -76,7 +78,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 	}
 
 	ret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],
-			&xmp, XDL_MERGE_FLAGS(level, style, favor), &result);
+			&xmp, &result);
 
 	for (i = 0; i < 3; i++)
 		free(mmfs[i].ptr);
diff --git a/ll-merge.c b/ll-merge.c
index a4b2f4c..82c7742 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -63,8 +63,6 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 			int flag, int marker_size)
 {
 	xmparam_t xmp;
-	int style = 0;
-	int favor = (flag >> 1) & 03;
 
 	if (buffer_is_binary(orig->ptr, orig->size) ||
 	    buffer_is_binary(src1->ptr, src1->size) ||
@@ -79,15 +77,13 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 	}
 
 	memset(&xmp, 0, sizeof(xmp));
+	xmp.level = XDL_MERGE_ZEALOUS;
+	xmp.favor= (flag >> 1) & 03;
 	if (git_xmerge_style >= 0)
-		style = git_xmerge_style;
+		xmp.style = git_xmerge_style;
 	if (marker_size > 0)
 		xmp.marker_size = marker_size;
-	return xdl_merge(orig,
-			 src1, name1,
-			 src2, name2,
-			 &xmp, XDL_MERGE_FLAGS(XDL_MERGE_ZEALOUS, style, favor),
-			 result);
+	return xdl_merge(orig, src1, name1, src2, name2, &xmp, result);
 }
 
 static int ll_union_merge(const struct ll_merge_driver *drv_unused,
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index 22614d5..a71763a 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -56,18 +56,14 @@ extern "C" {
 #define XDL_MERGE_EAGER 1
 #define XDL_MERGE_ZEALOUS 2
 #define XDL_MERGE_ZEALOUS_ALNUM 3
-#define XDL_MERGE_LEVEL_MASK 0x0f
 
 /* merge favor modes */
 #define XDL_MERGE_FAVOR_OURS 1
 #define XDL_MERGE_FAVOR_THEIRS 2
 #define XDL_MERGE_FAVOR_UNION 3
-#define XDL_MERGE_FAVOR(flags) (((flags)>>4) & 3)
-#define XDL_MERGE_FLAGS(level, style, favor) ((level)|(style)|((favor)<<4))
 
 /* merge output styles */
-#define XDL_MERGE_DIFF3 0x8000
-#define XDL_MERGE_STYLE_MASK 0x8000
+#define XDL_MERGE_DIFF3 1
 
 typedef struct s_mmfile {
 	char *ptr;
@@ -118,13 +114,16 @@ int xdl_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
 typedef struct s_xmparam {
 	xpparam_t xpp;
 	int marker_size;
+	int level;
+	int favor;
+	int style;
 } xmparam_t;
 
 #define DEFAULT_CONFLICT_MARKER_SIZE 7
 
 int xdl_merge(mmfile_t *orig, mmfile_t *mf1, const char *name1,
 		mmfile_t *mf2, const char *name2,
-		xmparam_t const *xmp, int flags, mmbuffer_t *result);
+		xmparam_t const *xmp, mmbuffer_t *result);
 
 #ifdef __cplusplus
 }
diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
index c901ecb..87cafa7 100644
--- a/xdiff/xmerge.c
+++ b/xdiff/xmerge.c
@@ -400,13 +400,13 @@ static int xdl_simplify_non_conflicts(xdfenv_t *xe1, xdmerge_t *m,
  */
 static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 		xdfenv_t *xe2, xdchange_t *xscr2, const char *name2,
-		int flags, xmparam_t const *xmp, mmbuffer_t *result) {
+		xmparam_t const *xmp, mmbuffer_t *result) {
 	xdmerge_t *changes, *c;
 	xpparam_t const *xpp = &xmp->xpp;
 	int i0, i1, i2, chg0, chg1, chg2;
-	int level = flags & XDL_MERGE_LEVEL_MASK;
-	int style = flags & XDL_MERGE_STYLE_MASK;
-	int favor = XDL_MERGE_FAVOR(flags);
+	int level = xmp->level;
+	int style = xmp->style;
+	int favor = xmp->favor;
 
 	if (style == XDL_MERGE_DIFF3) {
 		/*
@@ -556,7 +556,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 
 int xdl_merge(mmfile_t *orig, mmfile_t *mf1, const char *name1,
 		mmfile_t *mf2, const char *name2,
-		xmparam_t const *xmp, int flags, mmbuffer_t *result) {
+		xmparam_t const *xmp, mmbuffer_t *result) {
 	xdchange_t *xscr1, *xscr2;
 	xdfenv_t xe1, xe2;
 	int status;
@@ -593,7 +593,7 @@ int xdl_merge(mmfile_t *orig, mmfile_t *mf1, const char *name1,
 	} else {
 		status = xdl_do_merge(&xe1, xscr1, name1,
 				      &xe2, xscr2, name2,
-				      flags, xmp, result);
+				      xmp, result);
 	}
 	xdl_free_script(xscr1);
 	xdl_free_script(xscr2);
-- 
1.7.0.584.g2da2b

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] merge-file: add option to specify the marker size
  2010-02-28 19:56 [PATCH 1/3] merge-file: add option to specify the marker size Bert Wesarg
  2010-02-28 19:56 ` [PATCH 2/3] make union merge an xdl merge favor Bert Wesarg
  2010-02-28 19:56 ` [PATCH 3/3] refactor merge flags into xmparam_t Bert Wesarg
@ 2010-02-28 20:13 ` Junio C Hamano
       [not found]   ` <36ca99e91002282315m6c3caf1el61755c0d96d21cf2@mail.gmail.com>
  2 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2010-02-28 20:13 UTC (permalink / raw)
  To: Bert Wesarg; +Cc: git

Bert Wesarg <bert.wesarg@googlemail.com> writes:

> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>
> ---
>
> This can probably improved in a way, that the marker size will be taken
> from attributes. This could be done by an explicit --marker-size-by-path
> option or an option which names one of the three input files as a git
> path.

You don't want any option that is specifically about "marker size".  An
option to specify the path to take attributes for would be the right way
to go.  You might want to see "hash-object --path" for an inspiration.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] make union merge an xdl merge favor
  2010-02-28 19:56 ` [PATCH 2/3] make union merge an xdl merge favor Bert Wesarg
@ 2010-02-28 20:15   ` Junio C Hamano
  2010-03-01  6:57     ` Bert Wesarg
  0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2010-02-28 20:15 UTC (permalink / raw)
  To: Bert Wesarg; +Cc: Junio C Hamano, Johannes Schindelin, git

Bert Wesarg <bert.wesarg@googlemail.com> writes:

> The current union merge driver is implemented as an post process.  But the
> xdl_merge code is quite capable to produce the result by itself.  Therefore
> move to it there and teach git-merge-file a new --union option.

I like the idea of patch 2 and 3 but they are independent of what we do
(or don't do) to merge-file.  Could you flip the order of the patches so
that 2 and 3 can go first?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] make union merge an xdl merge favor
  2010-02-28 20:15   ` Junio C Hamano
@ 2010-03-01  6:57     ` Bert Wesarg
  2010-03-01  9:01       ` Junio C Hamano
  0 siblings, 1 reply; 10+ messages in thread
From: Bert Wesarg @ 2010-03-01  6:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git

On Sun, Feb 28, 2010 at 21:15, Junio C Hamano <gitster@pobox.com> wrote:
> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>
>> The current union merge driver is implemented as an post process.  But the
>> xdl_merge code is quite capable to produce the result by itself.  Therefore
>> move to it there and teach git-merge-file a new --union option.
>
> I like the idea of patch 2 and 3 but they are independent of what we do
> (or don't do) to merge-file.  Could you flip the order of the patches so
> that 2 and 3 can go first?

WIll do. On a side note: I plan to support the --union option also for
git-checkout. Would that be a good idea?

Bert

>
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] make union merge an xdl merge favor
  2010-03-01  6:57     ` Bert Wesarg
@ 2010-03-01  9:01       ` Junio C Hamano
  2010-03-01 10:55         ` Bert Wesarg
  2010-03-01 17:28         ` Junio C Hamano
  0 siblings, 2 replies; 10+ messages in thread
From: Junio C Hamano @ 2010-03-01  9:01 UTC (permalink / raw)
  To: Bert Wesarg; +Cc: Johannes Schindelin, git

Bert Wesarg <bert.wesarg@googlemail.com> writes:

> WIll do. On a side note: I plan to support the --union option also for
> git-checkout. Would that be a good idea?

What does it do?  Resolve conflicted paths using union merge at checkout
time?  That somehow feels like a dirty "because we can, and I only care
about union" hack to me.

I am mildly against "merge-file --union" in the first place.  The part I
actually liked in [2/3] was the removal of postprocessing.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] merge-file: add option to specify the marker size
       [not found]   ` <36ca99e91002282315m6c3caf1el61755c0d96d21cf2@mail.gmail.com>
@ 2010-03-01  9:18     ` Bert Wesarg
  0 siblings, 0 replies; 10+ messages in thread
From: Bert Wesarg @ 2010-03-01  9:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

[ Sorry again, I removed the list again ]

On Mon, Mar 1, 2010 at 08:15, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> On Sun, Feb 28, 2010 at 21:13, Junio C Hamano <gitster@pobox.com> wrote:
>> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>>
>>> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>>>
>>> ---
>>>
>>> This can probably improved in a way, that the marker size will be taken
>>> from attributes. This could be done by an explicit --marker-size-by-path
>>> option or an option which names one of the three input files as a git
>>> path.
>>
>> You don't want any option that is specifically about "marker size".  An
>> option to specify the path to take attributes for would be the right way
>> to go.  You might want to see "hash-object --path" for an inspiration.
>>
> So you suggest that it is only possible to set the conflict marker
> size for merges inside a git directory? I don't think this is a good
> idea, for a tool which is designed to work also outside of a git
> directory.
>
> In any case, if we add a --path option, merge-file should use ll_merge
> in this case, right?
>
> Bert

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] make union merge an xdl merge favor
  2010-03-01  9:01       ` Junio C Hamano
@ 2010-03-01 10:55         ` Bert Wesarg
  2010-03-01 17:28         ` Junio C Hamano
  1 sibling, 0 replies; 10+ messages in thread
From: Bert Wesarg @ 2010-03-01 10:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git

On Mon, Mar 1, 2010 at 10:01, Junio C Hamano <gitster@pobox.com> wrote:
> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>
>> WIll do. On a side note: I plan to support the --union option also for
>> git-checkout. Would that be a good idea?
>
> What does it do?  Resolve conflicted paths using union merge at checkout
> time?  That somehow feels like a dirty "because we can, and I only care
> about union" hack to me.

Point taken, I does not have a usecase by myself.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] make union merge an xdl merge favor
  2010-03-01  9:01       ` Junio C Hamano
  2010-03-01 10:55         ` Bert Wesarg
@ 2010-03-01 17:28         ` Junio C Hamano
  1 sibling, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2010-03-01 17:28 UTC (permalink / raw)
  To: Bert Wesarg; +Cc: Johannes Schindelin, git

Junio C Hamano <gitster@pobox.com> writes:

> I am mildly against "merge-file --union" in the first place.  The part I
> actually liked in [2/3] was the removal of postprocessing.

Actually I take the first sentence back.  The --union option given to
merge-file is comparable to --ours/--theirs that let you (a Porcelain
writer using the command) access the ll-merge machinery to resolve
conflicts in favor of one side on contents stored in an arbitrary
temporary filename (or even outside a git repository).  So as a feature
I think addition of "union" is in line with what we already have.

But I at the same time I think that we could add the feature while making
existing --ours/--theirs options redundant (and deprecatable if we want to
in the longer term), if the attributes mechanism is made available to us
(e.g. add "--attribute-path=<use attributes for this path>" option, and/or
perhaps add "--attributes=<use this set of attributes>" option) directly
from the command line.

I doubt "checkout --union" is a good idea, though.  Its --ours/--theirs
options do not have anything to do with merge (it specifies _what_ to
checkout, not _how_), and the comparison I made in the previous paragraph
for merge-file does not apply.

It however is plausible that in a scenario where:

 (1) you ran "git merge" (or anything that results in conflicts) and got a
     conflict; and

 (2) you know using other ll-merge backend, perhaps a custom one you have
     that is accessible via ll-merge, would have produced a better result.

it might be useful if you can say "forget this failed auto-merge; attempt
the same merge for the path but using this different ll-merge backend".
I don't think that feature belongs to "checkout", though.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2010-03-01 17:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-28 19:56 [PATCH 1/3] merge-file: add option to specify the marker size Bert Wesarg
2010-02-28 19:56 ` [PATCH 2/3] make union merge an xdl merge favor Bert Wesarg
2010-02-28 20:15   ` Junio C Hamano
2010-03-01  6:57     ` Bert Wesarg
2010-03-01  9:01       ` Junio C Hamano
2010-03-01 10:55         ` Bert Wesarg
2010-03-01 17:28         ` Junio C Hamano
2010-02-28 19:56 ` [PATCH 3/3] refactor merge flags into xmparam_t Bert Wesarg
2010-02-28 20:13 ` [PATCH 1/3] merge-file: add option to specify the marker size Junio C Hamano
     [not found]   ` <36ca99e91002282315m6c3caf1el61755c0d96d21cf2@mail.gmail.com>
2010-03-01  9:18     ` Bert Wesarg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).