[PATCH v2 0/4] Generic conflict style fixes

The switch to diff3 by default got stuck on a tar pit, however that's no
reason not to improve the merge.conflictStyle handling.

I dropped all the controversial changes and focused only on the obvious
improvements.

I switched to from the custom fill function to test_write_lines as Eric
Sunshine suggested, and used a simpler and stricter grep regex for diff3
markers as Phillip Wood prompted.

Felipe Contreras (4):
  test: add merge style config test
  merge-tree: fix merge.conflictstyle handling
  notes: fix merge.conflictstyle handling
  test: document broken merge.conflictStyle handling

 builtin/merge-tree.c               |   4 +
 builtin/notes.c                    |   3 +-
 t/t6440-config-conflict-markers.sh | 116 +++++++++++++++++++++++++++++
 3 files changed, 122 insertions(+), 1 deletion(-)
 create mode 100755 t/t6440-config-conflict-markers.sh

Range-diff against v1:
1:  118388c313 ! 1:  51351f1a77 test: add merge style config test
    @@ t/t6440-config-conflict-markers.sh (new)
     +
     +. ./test-lib.sh
     +
    -+fill () {
    -+	for i
    -+	do
    -+		echo "$i"
    -+	done
    -+}
    -+
     +test_expect_success 'merge' '
     +	test_create_repo merge &&
     +	(
     +		cd merge &&
     +
    -+		fill 1 2 3 >content &&
    ++		test_write_lines 1 2 3 >content &&
     +		git add content &&
     +		git commit -m base &&
     +
    @@ t/t6440-config-conflict-markers.sh (new)
     +		git commit -a -m left &&
     +
     +		test_must_fail git merge r &&
    -+		! grep -E "\|+" content &&
    ++		! grep "^|||||||" content &&
     +
     +		git reset --hard &&
     +		test_must_fail git -c merge.conflictstyle=diff3 merge r &&
    -+		grep -E "\|+" content &&
    ++		grep "^|||||||" content &&
     +
     +		git reset --hard &&
     +		test_must_fail git -c merge.conflictstyle=merge merge r &&
    -+		! grep -E "\|+" content
    ++		! grep "^|||||||" content
     +	)
     +'
     +
2:  697f2a2a5c ! 2:  1fccf561ed merge-tree: fix merge.conflictstyle handling
    @@ t/t6440-config-conflict-markers.sh: test_expect_success 'merge' '
     +		test_commit l content l &&
     +
     +		git merge-tree initial r l >actual &&
    -+		! grep -E "\|+" actual &&
    ++		! grep "^+|||||||" content &&
     +
     +		git -c merge.conflictstyle=diff3 merge-tree initial r l >actual &&
    -+		grep -E "\|+" actual &&
    ++		grep "^+|||||||" actual &&
     +
     +		git -c merge.conflictstyle=merge merge-tree initial r l >actual &&
    -+		! grep -E "\|+" actual
    ++		! grep "^+|||||||" content
     +	)
     +'
     +
3:  56a20f41a4 ! 3:  3bb872e3cd notes: fix merge.conflictstyle handling
    @@ t/t6440-config-conflict-markers.sh: test_expect_success 'merge-tree' '
     +		git notes add -f -m l initial &&
     +
     +		test_must_fail git notes merge r &&
    -+		! grep -E "\|+" .git/NOTES_MERGE_WORKTREE/* &&
    ++		! grep "^|||||||" .git/NOTES_MERGE_WORKTREE/* &&
     +
     +		git notes merge --abort &&
     +		test_must_fail git -c merge.conflictstyle=diff3 notes merge r &&
    -+		grep -E "\|+" .git/NOTES_MERGE_WORKTREE/* &&
    ++		grep "^|||||||" .git/NOTES_MERGE_WORKTREE/* &&
     +
     +		git notes merge --abort &&
     +		test_must_fail git -c merge.conflictstyle=merge notes merge r &&
    -+		! grep -E "\|+" .git/NOTES_MERGE_WORKTREE/*
    ++		! grep "^|||||||" .git/NOTES_MERGE_WORKTREE/*
     +	)
     +'
     +
4:  adce598754 < -:  ---------- checkout: fix merge.conflictstyle handling
5:  a4ddbbc44a < -:  ---------- xdiff: rename XDL_MERGE_STYLE_DIFF3
6:  c3e0940dab < -:  ---------- xdiff: simplify style assignments
7:  6415e53345 < -:  ---------- xdiff: make diff3 the default conflictStyle
-:  ---------- > 4:  a767bc68e6 test: document broken merge.conflictStyle handling
-- 
2.32.0

We want to test different combinations of merge.conflictstyle, and a new
file is the best place to do that.

Signed-off-by: Felipe Contreras <redacted>
---
 t/t6440-config-conflict-markers.sh | 37 ++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100755 t/t6440-config-conflict-markers.sh

diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh
new file mode 100755
index 0000000000..813d7dda9a
--- /dev/null
+++ b/t/t6440-config-conflict-markers.sh

@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description='merge style conflict markers configurations'
+
+. ./test-lib.sh
+
+test_expect_success 'merge' '
+	test_create_repo merge &&
+	(
+		cd merge &&
+
+		test_write_lines 1 2 3 >content &&
+		git add content &&
+		git commit -m base &&
+
+		git checkout -b r &&
+		echo six >>content &&
+		git commit -a -m right &&
+
+		git checkout master &&
+		echo 7 >>content &&
+		git commit -a -m left &&
+
+		test_must_fail git merge r &&
+		! grep "^|||||||" content &&
+
+		git reset --hard &&
+		test_must_fail git -c merge.conflictstyle=diff3 merge r &&
+		grep "^|||||||" content &&
+
+		git reset --hard &&
+		test_must_fail git -c merge.conflictstyle=merge merge r &&
+		! grep "^|||||||" content
+	)
+'
+
+test_done

-- 
2.32.0

Currently it's completely ignored.

Signed-off-by: Felipe Contreras <redacted>
---
 builtin/merge-tree.c               |  4 ++++
 t/t6440-config-conflict-markers.sh | 21 +++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index de8520778d..7d677bd75c 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c

@@ -7,6 +7,8 @@
 #include "blob.h"
 #include "exec-cmd.h"
 #include "merge-blobs.h"
+#include "config.h"
+#include "xdiff-interface.h"
 
 static const char merge_tree_usage[] = "git merge-tree <base-tree> <branch1> <branch2>";
 

@@ -378,6 +380,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
 	if (argc != 4)
 		usage(merge_tree_usage);
 
+	git_config(git_xmerge_config, NULL);
+
 	buf1 = get_tree_descriptor(r, t+0, argv[1]);
 	buf2 = get_tree_descriptor(r, t+1, argv[2]);
 	buf3 = get_tree_descriptor(r, t+2, argv[3]);

diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh
index 813d7dda9a..cb2ee3ad0a 100755
--- a/t/t6440-config-conflict-markers.sh
+++ b/t/t6440-config-conflict-markers.sh

@@ -34,4 +34,25 @@ test_expect_success 'merge' '
 	)
 '
 
+test_expect_success 'merge-tree' '
+	test_create_repo merge-tree &&
+	(
+		cd merge-tree &&
+
+		test_commit initial initial-file initial &&
+		test_commit r content r &&
+		git reset --hard initial &&
+		test_commit l content l &&
+
+		git merge-tree initial r l >actual &&
+		! grep "^+|||||||" content &&
+
+		git -c merge.conflictstyle=diff3 merge-tree initial r l >actual &&
+		grep "^+|||||||" actual &&
+
+		git -c merge.conflictstyle=merge merge-tree initial r l >actual &&
+		! grep "^+|||||||" content
+	)
+'
+
 test_done

-- 
2.32.0

Currently both merge.conflictStyle and --conflict=diff3 don't work
together for `git commit --merge`, since the former wrongly overrides
the later.

There is no easy way to fix this, so mark it as broken for now.

Signee-off-by: Felipe Contreras [off-list ref]
---
 t/t6440-config-conflict-markers.sh | 31 ++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh
index c51512ced6..3ba993a6a8 100755
--- a/t/t6440-config-conflict-markers.sh
+++ b/t/t6440-config-conflict-markers.sh

@@ -82,4 +82,35 @@ test_expect_success 'notes' '
 	)
 '
 
+test_expect_failure 'checkout' '
+	test_create_repo checkout &&
+	(
+		test_commit checkout &&
+
+		test_write_lines a b c d e >content &&
+		git add content &&
+		git commit -m initial &&
+
+		git checkout -b simple master &&
+		test_write_lines a c e >content &&
+		git commit -a -m simple &&
+
+		test_write_lines b d >content &&
+		git checkout --merge master &&
+		! grep "^|||||||" content &&
+
+		git config merge.conflictstyle merge &&
+
+		git checkout -f simple &&
+		test_write_lines b d >content &&
+		git checkout --merge --conflict=diff3 master &&
+		grep "^|||||||" content &&
+
+		git checkout -f simple &&
+		test_write_lines b d >content &&
+		git checkout --merge --conflict=merge master &&
+		! grep "^|||||||" content
+	)
+'
+
 test_done

-- 
2.32.0

Currently it's completely ignored.

Signed-off-by: Felipe Contreras <redacted>
---
 builtin/notes.c                    |  3 ++-
 t/t6440-config-conflict-markers.sh | 27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 74bba39ca8..a333cc68ec 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c

@@ -23,6 +23,7 @@
 #include "notes-merge.h"
 #include "notes-utils.h"
 #include "worktree.h"
+#include "xdiff-interface.h"
 
 static const char * const git_notes_usage[] = {
 	N_("git notes [--ref <notes-ref>] [list [<object>]]"),

@@ -1000,7 +1001,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	git_config(git_default_config, NULL);
+	git_config(git_xmerge_config, NULL);
 	argc = parse_options(argc, argv, prefix, options, git_notes_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 

diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh
index cb2ee3ad0a..c51512ced6 100755
--- a/t/t6440-config-conflict-markers.sh
+++ b/t/t6440-config-conflict-markers.sh

@@ -55,4 +55,31 @@ test_expect_success 'merge-tree' '
 	)
 '
 
+test_expect_success 'notes' '
+	test_create_repo notes &&
+	(
+		test_commit initial &&
+
+		git -c core.notesRef=refs/notes/b notes add -m b initial &&
+
+		git update-ref refs/notes/r refs/notes/b &&
+		git -c core.notesRef=refs/notes/r notes add -f -m r initial &&
+
+		git update-ref refs/notes/l refs/notes/b &&
+		git config core.notesRef refs/notes/l &&
+		git notes add -f -m l initial &&
+
+		test_must_fail git notes merge r &&
+		! grep "^|||||||" .git/NOTES_MERGE_WORKTREE/* &&
+
+		git notes merge --abort &&
+		test_must_fail git -c merge.conflictstyle=diff3 notes merge r &&
+		grep "^|||||||" .git/NOTES_MERGE_WORKTREE/* &&
+
+		git notes merge --abort &&
+		test_must_fail git -c merge.conflictstyle=merge notes merge r &&
+		! grep "^|||||||" .git/NOTES_MERGE_WORKTREE/*
+	)
+'
+
 test_done

-- 
2.32.0