[PATCH 0/7] Make diff3 the default conflict style

This patch series turned out much more complicated that simply flipping
the switch and dealing with the consequences. Apparently some commands
are completely ignoring the configuration (notes and merge-tree), and
others are handling it wrong (checkout).

So in preparation I created a new test to make sure these rowdy
commands handle the configuration correctly, and then step by step I fix
them.

Once all the commands are fixed I proceed to cleanup xdiff-interface in
preparation for the switch.

And finally once the switch is flipped the documnetation is updated, and
funch of test scripts receive a temporary configuration that returns
them to the old "merge" (diff2) behavior so they pass with minimum
changes.

I have already written patches to update the tests so no configuration
is needed and they parse the diff3 style directly, but the series is
already quite verbose as it is.

One salient thorn is that from my point of view merge_recursive_config()
is implemtend wrongly and thus can't be called as other configuration
functions, like git_diff_basic_config(). It seems there's a huge area of
opportunity there to clean all that up, but that's for another series.

Felipe Contreras (7):
  test: add merge style config test
  merge-tree: fix merge.conflictstyle handling
  notes: fix merge.conflictstyle handling
  checkout: fix merge.conflictstyle handling
  xdiff: rename XDL_MERGE_STYLE_DIFF3
  xdiff: simplify style assignments
  xdiff: make diff3 the default conflictStyle

 Documentation/config/merge.txt           |  12 +--
 Documentation/git-merge-file.txt         |   2 +
 Documentation/git-merge.txt              |  28 ++----
 Documentation/git-rerere.txt             |   2 +-
 Documentation/gitattributes.txt          |   6 +-
 Documentation/technical/rerere.txt       |   3 +-
 Documentation/user-manual.txt            |   6 +-
 builtin/merge-file.c                     |   5 +-
 builtin/merge-recursive.c                |   3 +
 builtin/merge-tree.c                     |   4 +
 builtin/merge.c                          |   4 +
 builtin/notes.c                          |   3 +-
 ll-merge.c                               |   3 +-
 merge-recursive.c                        |   2 +-
 sequencer.c                              |   5 +
 t/t2023-checkout-m.sh                    |   2 +
 t/t3310-notes-merge-manual-resolve.sh    |   2 +
 t/t3311-notes-merge-fanout.sh            |   2 +
 t/t3404-rebase-interactive.sh            |   2 +
 t/t3507-cherry-pick-conflict.sh          |   2 +
 t/t4017-diff-retval.sh                   |   2 +
 t/t4048-diff-combined-binary.sh          |   2 +
 t/t4200-rerere.sh                        |   2 +
 t/t4300-merge-tree.sh                    |   2 +
 t/t6402-merge-rename.sh                  |   2 +
 t/t6403-merge-file.sh                    |   2 +
 t/t6404-recursive-merge.sh               |   2 +
 t/t6416-recursive-corner-cases.sh        |   2 +
 t/t6417-merge-ours-theirs.sh             |   2 +
 t/t6418-merge-text-auto.sh               |   2 +
 t/t6422-merge-rename-corner-cases.sh     |   2 +
 t/t6423-merge-rename-directories.sh      |   1 +
 t/t6428-merge-conflicts-sparse.sh        |   1 +
 t/t6432-merge-recursive-space-options.sh |   2 +
 t/t6440-config-conflict-markers.sh       | 123 +++++++++++++++++++++++
 t/t7201-co.sh                            |   2 +
 t/t7506-status-submodule.sh              |   1 +
 xdiff-interface.c                        |   6 +-
 xdiff/xdiff.h                            |   3 +-
 xdiff/xmerge.c                           |   4 +-
 40 files changed, 217 insertions(+), 46 deletions(-)
 create mode 100755 t/t6440-config-conflict-markers.sh

-- 
2.32.0.2.g41be0a4e50

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 | 44 ++++++++++++++++++++++++++++++
 1 file changed, 44 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..6952552c58
--- /dev/null
+++ b/t/t6440-config-conflict-markers.sh

@@ -0,0 +1,44 @@
+#!/bin/sh
+
+test_description='merge style conflict markers configurations'
+
+. ./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 &&
+		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 -E "\|+" content &&
+
+		git reset --hard &&
+		test_must_fail git -c merge.conflictstyle=diff3 merge r &&
+		grep -E "\|+" content &&
+
+		git reset --hard &&
+		test_must_fail git -c merge.conflictstyle=merge merge r &&
+		! grep -E "\|+" content
+	)
+'
+
+test_done

-- 
2.32.0.2.g41be0a4e50

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 978f4e3e70..44f79ac91b 100755
--- a/t/t6440-config-conflict-markers.sh
+++ b/t/t6440-config-conflict-markers.sh

@@ -62,4 +62,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 -E "\|+" .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/* &&
+
+		git notes merge --abort &&
+		test_must_fail git -c merge.conflictstyle=merge notes merge r &&
+		! grep -E "\|+" .git/NOTES_MERGE_WORKTREE/*
+	)
+'
+
 test_done

-- 
2.32.0.2.g41be0a4e50

Virtually everyone is using it, and it's one of the first things we
teach newcomers in order to resolve conflicts efficiently.

Let's make it the default.

This generates a ton of changes in the tests. Although we probably will
want to update them to use th new default, override the configuration so
we use the old one for now.

Signed-off-by: Felipe Contreras <redacted>
---
 Documentation/config/merge.txt           | 12 +++++-----
 Documentation/git-merge-file.txt         |  2 ++
 Documentation/git-merge.txt              | 28 +++++++-----------------
 Documentation/git-rerere.txt             |  2 +-
 Documentation/gitattributes.txt          |  6 ++---
 Documentation/technical/rerere.txt       |  3 +--
 Documentation/user-manual.txt            |  6 ++++-
 t/t2023-checkout-m.sh                    |  2 ++
 t/t3310-notes-merge-manual-resolve.sh    |  2 ++
 t/t3311-notes-merge-fanout.sh            |  2 ++
 t/t3404-rebase-interactive.sh            |  2 ++
 t/t3507-cherry-pick-conflict.sh          |  2 ++
 t/t4017-diff-retval.sh                   |  2 ++
 t/t4048-diff-combined-binary.sh          |  2 ++
 t/t4200-rerere.sh                        |  2 ++
 t/t4300-merge-tree.sh                    |  2 ++
 t/t6402-merge-rename.sh                  |  2 ++
 t/t6403-merge-file.sh                    |  2 ++
 t/t6404-recursive-merge.sh               |  2 ++
 t/t6416-recursive-corner-cases.sh        |  2 ++
 t/t6417-merge-ours-theirs.sh             |  2 ++
 t/t6418-merge-text-auto.sh               |  2 ++
 t/t6422-merge-rename-corner-cases.sh     |  2 ++
 t/t6423-merge-rename-directories.sh      |  1 +
 t/t6428-merge-conflicts-sparse.sh        |  1 +
 t/t6432-merge-recursive-space-options.sh |  2 ++
 t/t6440-config-conflict-markers.sh       |  8 +++----
 t/t7201-co.sh                            |  2 ++
 t/t7506-status-submodule.sh              |  1 +
 xdiff-interface.c                        |  2 +-
 30 files changed, 70 insertions(+), 38 deletions(-)

diff --git a/Documentation/config/merge.txt b/Documentation/config/merge.txt
index cb2ed58907..2dba937dd0 100644
--- a/Documentation/config/merge.txt
+++ b/Documentation/config/merge.txt

@@ -1,10 +1,10 @@
 merge.conflictStyle::
-	Specify the style in which conflicted hunks are written out to
-	working tree files upon merge.  The default is "merge", which
-	shows a `<<<<<<<` conflict marker, changes made by one side,
-	a `=======` marker, changes made by the other side, and then
-	a `>>>>>>>` marker.  An alternate style, "diff3", adds a `|||||||`
-	marker and the original text before the `=======` marker.
+	Specify the style in which conflicted hunks are written out to working
+	tree files upon merge. The default is "diff3", which shows a `<<<<<<<`
+	conflict marker, changes made by one side, a `|||||||` marker, the
+	original text, a `=======` marker, changes made by the other side, and
+	then a `>>>>>>>` marker. A simpler mode "merge" omits the `|||||||`
+	marker and the original text.
 
 merge.defaultToUpstream::
 	If merge is called without any commit argument, merge the upstream

diff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt
index f856032613..7d8e74c872 100644
--- a/Documentation/git-merge-file.txt
+++ b/Documentation/git-merge-file.txt

@@ -30,6 +30,8 @@ normally outputs a warning and brackets the conflict with lines containing
 
 	<<<<<<< A
 	lines in file A
+	|||||||
+	lines in merge base
 	=======
 	lines in file B
 	>>>>>>> B

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 3819fadac1..14dadf2e16 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt

@@ -233,7 +233,7 @@ final result verbatim.  When both sides made changes to the same area,
 however, Git cannot randomly pick one side over the other, and asks you to
 resolve it by leaving what both sides did to that area.
 
-By default, Git uses the same style as the one used by the "merge" program
+By default, Git uses a similar style to the one used by the "merge" program
 from the RCS suite to present such a conflicted hunk, like this:
 
 ------------

@@ -242,6 +242,8 @@ ancestor, or cleanly resolved because only one side changed.
 <<<<<<< yours:sample.txt
 Conflict resolution is hard;
 let's go shopping.
+|||||||
+Originally there's no conflict.
 =======
 Git makes conflict resolution easy.
 >>>>>>> theirs:sample.txt

@@ -249,17 +251,12 @@ And here is another line that is cleanly resolved or unmodified.
 ------------
 
 The area where a pair of conflicting changes happened is marked with markers
-`<<<<<<<`, `=======`, and `>>>>>>>`.  The part before the `=======`
-is typically your side, and the part afterwards is typically their side.
-
-The default format does not show what the original said in the conflicting
-area.  You cannot tell how many lines are deleted and replaced with
-Barbie's remark on your side.  The only thing you can tell is that your
-side wants to say it is hard and you'd prefer to go shopping, while the
-other side wants to claim it is easy.
+`<<<<<<<`, `=======`, and `>>>>>>>`.  The part before the `|||||||`
+is typically your side, and the part after `=======` is typically their side.
+In-between is the original code.
 
-An alternative style can be used by setting the "merge.conflictStyle"
-configuration variable to "diff3".  In "diff3" style, the above conflict
+An more basic style can be used by setting the "merge.conflictStyle"
+configuration variable to "merge".  In "merge" style, the above conflict
 may look like this:
 
 ------------

@@ -268,21 +265,12 @@ ancestor, or cleanly resolved because only one side changed.
 <<<<<<< yours:sample.txt
 Conflict resolution is hard;
 let's go shopping.
-|||||||
-Conflict resolution is hard.
 =======
 Git makes conflict resolution easy.
 >>>>>>> theirs:sample.txt
 And here is another line that is cleanly resolved or unmodified.
 ------------
 
-In addition to the `<<<<<<<`, `=======`, and `>>>>>>>` markers, it uses
-another `|||||||` marker that is followed by the original text.  You can
-tell that the original just stated a fact, and your side simply gave in to
-that statement and gave up, while the other side tried to have a more
-positive attitude.  You can sometimes come up with a better resolution by
-viewing the original.
-
 
 HOW TO RESOLVE CONFLICTS
 ------------------------

diff --git a/Documentation/git-rerere.txt b/Documentation/git-rerere.txt
index 4cfc883378..89b0820995 100644
--- a/Documentation/git-rerere.txt
+++ b/Documentation/git-rerere.txt

@@ -159,7 +159,7 @@ resolve.
 
 Running the 'git rerere' command immediately after a conflicted
 automerge records the conflicted working tree files, with the
-usual conflict markers `<<<<<<<`, `=======`, and `>>>>>>>` in
+usual conflict markers `<<<<<<<`, `|||||||`, `=======`, and `>>>>>>>` in
 them.  Later, after you are done resolving the conflicts,
 running 'git rerere' again will record the resolved state of these
 files.  Suppose you did this when you created the test merge of

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 83fd4e19a4..b767215ac2 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt

@@ -1042,10 +1042,10 @@ text::
 
 	Usual 3-way file level merge for text files.  Conflicted
 	regions are marked with conflict markers `<<<<<<<`,
-	`=======` and `>>>>>>>`.  The version from your branch
-	appears before the `=======` marker, and the version
+	`|||||||`, `=======` and `>>>>>>>`.  The version from your branch
+	appears before the `|||||||` marker, and the version
 	from the merged branch appears after the `=======`
-	marker.
+	marker. In-between is the original.
 
 binary::
 

diff --git a/Documentation/technical/rerere.txt b/Documentation/technical/rerere.txt
index af5f9fc24f..38b44f4430 100644
--- a/Documentation/technical/rerere.txt
+++ b/Documentation/technical/rerere.txt

@@ -42,8 +42,7 @@ get a conflict like the following:
     >>>>>>> AC
 
 Doing the analogous with AC2 (forking a branch ABAC2 off of branch AB
-and then merging branch AC2 into it), using the diff3 conflict style,
-we get a conflict like the following:
+and then merging branch AC2 into it), we get a conflict like the following:
 
     <<<<<<< HEAD
     B

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index f9e54b8674..3ddde87482 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt

@@ -1243,6 +1243,8 @@ files with conflicts will have conflict markers added, like this:
 -------------------------------------------------
 <<<<<<< HEAD:file.txt
 Hello world
+|||||||
+Original
 =======
 Goodbye
 >>>>>>> 77976da35a11db4580b80ae27e8d65caf5208086:file.txt

@@ -1276,9 +1278,11 @@ diff --cc file.txt
 index 802992c,2b60207..0000000
 --- a/file.txt
 +++ b/file.txt
-@@@ -1,1 -1,1 +1,5 @@@
+@@@ -1,1 -1,1 +1,7 @@@
 ++<<<<<<< HEAD:file.txt
  +Hello world
+++|||||||
+++Original
 ++=======
 + Goodbye
 ++>>>>>>> 77976da35a11db4580b80ae27e8d65caf5208086:file.txt

diff --git a/t/t2023-checkout-m.sh b/t/t2023-checkout-m.sh
index 7b327b7544..219c82532a 100755
--- a/t/t2023-checkout-m.sh
+++ b/t/t2023-checkout-m.sh

@@ -9,6 +9,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+git config merge.conflictstyle merge # TODO: use the default
+
 test_expect_success setup '
 	test_tick &&
 	test_commit both.txt both.txt initial &&

diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
index d3d72e25fe..cbd5d8302e 100755
--- a/t/t3310-notes-merge-manual-resolve.sh
+++ b/t/t3310-notes-merge-manual-resolve.sh

@@ -7,6 +7,8 @@ test_description='Test notes merging with manual conflict resolution'
 
 . ./test-lib.sh
 
+git config --global merge.conflictstyle merge # TODO: use the default
+
 # Set up a notes merge scenario with different kinds of conflicts
 test_expect_success 'setup commits' '
 	test_commit 1st &&

diff --git a/t/t3311-notes-merge-fanout.sh b/t/t3311-notes-merge-fanout.sh
index 5b675417e9..4aeaa05c15 100755
--- a/t/t3311-notes-merge-fanout.sh
+++ b/t/t3311-notes-merge-fanout.sh

@@ -7,6 +7,8 @@ test_description='Test notes merging at various fanout levels'
 
 . ./test-lib.sh
 
+git config merge.conflictstyle merge # TODO: use the default
+
 verify_notes () {
 	notes_ref="$1"
 	commit="$2"

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 66bcbbf952..769079a71c 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh

@@ -32,6 +32,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . "$TEST_DIRECTORY"/lib-rebase.sh
 
+git config merge.conflictstyle merge # TODO: use the default
+
 test_expect_success 'setup' '
 	git switch -C primary &&
 	test_commit A file1 &&

diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 014001b8f3..647a40f314 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh

@@ -281,6 +281,7 @@ test_expect_success \
 
 test_expect_success 'failed cherry-pick describes conflict in work tree' '
 	pristine_detach initial &&
+	git config merge.conflictstyle merge && # TODO: use the default
 	cat <<-EOF >expected &&
 	<<<<<<< HEAD
 	a

@@ -316,6 +317,7 @@ test_expect_success 'diff3 -m style' '
 
 test_expect_success 'revert also handles conflicts sanely' '
 	git config --unset merge.conflictstyle &&
+	git config merge.conflictstyle merge && # TODO: use the default
 	pristine_detach initial &&
 	cat <<-EOF >expected &&
 	<<<<<<< HEAD

diff --git a/t/t4017-diff-retval.sh b/t/t4017-diff-retval.sh
index ed461f481e..04b77af2a4 100755
--- a/t/t4017-diff-retval.sh
+++ b/t/t4017-diff-retval.sh

@@ -7,6 +7,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+git config merge.conflictstyle merge # TODO: use the default
+
 test_expect_success 'setup' '
 	echo "1 " >a &&
 	git add . &&

diff --git a/t/t4048-diff-combined-binary.sh b/t/t4048-diff-combined-binary.sh
index 0260cf64f5..49a56731dd 100755
--- a/t/t4048-diff-combined-binary.sh
+++ b/t/t4048-diff-combined-binary.sh

@@ -6,6 +6,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+git config merge.conflictstyle merge # TODO: use the default
+
 test_expect_success 'setup binary merge conflict' '
 	echo oneQ1 | q_to_nul >binary &&
 	git add binary &&

diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 9f8c76dffb..e9ae3d6fde 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh

@@ -27,6 +27,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+git config merge.conflictstyle merge # TODO: use the default
+
 test_expect_success 'setup' '
 	cat >a1 <<-\EOF &&
 	Some title

diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh
index e59601e5fe..f21ccaf0a6 100755
--- a/t/t4300-merge-tree.sh
+++ b/t/t4300-merge-tree.sh

@@ -6,6 +6,8 @@
 test_description='git merge-tree'
 . ./test-lib.sh
 
+git config merge.conflictstyle merge # TODO: use the default
+
 test_expect_success setup '
 	test_commit "initial" "initial-file" "initial"
 '

diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
index 425dad97d5..4f01bbc451 100755
--- a/t/t6402-merge-rename.sh
+++ b/t/t6402-merge-rename.sh

@@ -270,6 +270,7 @@ test_expect_success 'setup for rename + d/f conflicts' '
 	git checkout --orphan dir-in-way &&
 	git rm -rf . &&
 	git clean -fdqx &&
+	git config merge.conflictstyle merge && # TODO: use the default
 
 	mkdir sub &&
 	mkdir dir &&

@@ -871,6 +872,7 @@ test_expect_success 'setup for use of extended merge markers' '
 	git clean -fdqx &&
 	rm -rf .git &&
 	git init &&
+	git config merge.conflictstyle merge && # TODO: use the default
 
 	printf "1\n2\n3\n4\n5\n6\n7\n8\n" >original_file &&
 	git add original_file &&

diff --git a/t/t6403-merge-file.sh b/t/t6403-merge-file.sh
index 2f421d967a..1428dfb5c6 100755
--- a/t/t6403-merge-file.sh
+++ b/t/t6403-merge-file.sh

@@ -3,6 +3,8 @@
 test_description='RCS merge replacement: merge-file'
 . ./test-lib.sh
 
+git config merge.conflictstyle merge # TODO: use the default
+
 test_expect_success 'setup' '
 	cat >orig.txt <<-\EOF &&
 	Dominus regit me,

diff --git a/t/t6404-recursive-merge.sh b/t/t6404-recursive-merge.sh
index eaf48e941e..a3354b8f9a 100755
--- a/t/t6404-recursive-merge.sh
+++ b/t/t6404-recursive-merge.sh

@@ -6,6 +6,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+git config merge.conflictstyle merge # TODO: use the default
+
 # This scenario is based on a real-world repository of Shawn Pearce.
 
 # 1 - A - D - F

diff --git a/t/t6416-recursive-corner-cases.sh b/t/t6416-recursive-corner-cases.sh
index 84f5082366..ac4e69a325 100755
--- a/t/t6416-recursive-corner-cases.sh
+++ b/t/t6416-recursive-corner-cases.sh

@@ -8,6 +8,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-merge.sh
 
+git config --global merge.conflictstyle merge # TODO: use the default
+
 #
 #  L1  L2
 #   o---o

diff --git a/t/t6417-merge-ours-theirs.sh b/t/t6417-merge-ours-theirs.sh
index ac9aee9a66..b8208a383b 100755
--- a/t/t6417-merge-ours-theirs.sh
+++ b/t/t6417-merge-ours-theirs.sh

@@ -6,6 +6,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+git config merge.conflictstyle merge # TODO: use the default
+
 test_expect_success setup '
 	for i in 1 2 3 4 5 6 7 8 9
 	do

diff --git a/t/t6418-merge-text-auto.sh b/t/t6418-merge-text-auto.sh
index 1e0296dd17..e18f67776c 100755
--- a/t/t6418-merge-text-auto.sh
+++ b/t/t6418-merge-text-auto.sh

@@ -17,6 +17,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+git config merge.conflictstyle merge # TODO: use the default
+
 test_have_prereq SED_STRIPS_CR && SED_OPTIONS=-b
 
 compare_files () {

diff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh
index bf4ce3c63d..6bb4b6d968 100755
--- a/t/t6422-merge-rename-corner-cases.sh
+++ b/t/t6422-merge-rename-corner-cases.sh

@@ -9,6 +9,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-merge.sh
 
+git config --global merge.conflictstyle merge # TODO: use the default
+
 test_setup_rename_delete_untracked () {
 	test_create_repo rename-delete-untracked &&
 	(

diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index 7134769149..5f6cacd064 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh

@@ -28,6 +28,7 @@ test_description="recursive merge with directory renames"
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-merge.sh
 
+git config --global merge.conflictstyle merge # TODO: use the default
 
 ###########################################################################
 # SECTION 1: Basic cases we should be able to handle

diff --git a/t/t6428-merge-conflicts-sparse.sh b/t/t6428-merge-conflicts-sparse.sh
index 7e8bf497f8..18975801db 100755
--- a/t/t6428-merge-conflicts-sparse.sh
+++ b/t/t6428-merge-conflicts-sparse.sh

@@ -25,6 +25,7 @@ test_description="merge cases"
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-merge.sh
 
+git config --global merge.conflictstyle merge # TODO: use the default
 
 # Testcase basic, conflicting changes in 'numerals'
 

diff --git a/t/t6432-merge-recursive-space-options.sh b/t/t6432-merge-recursive-space-options.sh
index db4b77e63d..5cfe8a4fbd 100755
--- a/t/t6432-merge-recursive-space-options.sh
+++ b/t/t6432-merge-recursive-space-options.sh

@@ -16,6 +16,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+git config merge.conflictstyle merge # TODO: use the default
+
 test_have_prereq SED_STRIPS_CR && SED_OPTIONS=-b
 if test_have_prereq GREP_STRIPS_CR
 then

diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh
index 485ad0eee0..ae0bab37ad 100755
--- a/t/t6440-config-conflict-markers.sh
+++ b/t/t6440-config-conflict-markers.sh

@@ -29,7 +29,7 @@ test_expect_success 'merge' '
 		git commit -a -m left &&
 
 		test_must_fail git merge r &&
-		! grep -E "\|+" content &&
+		grep -E "\|+" content &&
 
 		git reset --hard &&
 		test_must_fail git -c merge.conflictstyle=diff3 merge r &&

@@ -52,7 +52,7 @@ test_expect_success 'merge-tree' '
 		test_commit l content l &&
 
 		git merge-tree initial r l >actual &&
-		! grep -E "\|+" actual &&
+		grep -E "\|+" actual &&
 
 		git -c merge.conflictstyle=diff3 merge-tree initial r l >actual &&
 		grep -E "\|+" actual &&

@@ -77,7 +77,7 @@ test_expect_success 'notes' '
 		git notes add -f -m l initial &&
 
 		test_must_fail git notes merge r &&
-		! grep -E "\|+" .git/NOTES_MERGE_WORKTREE/* &&
+		grep -E "\|+" .git/NOTES_MERGE_WORKTREE/* &&
 
 		git notes merge --abort &&
 		test_must_fail git -c merge.conflictstyle=diff3 notes merge r &&

@@ -104,7 +104,7 @@ test_expect_success 'checkout' '
 
 		fill b d >content &&
 		git checkout --merge master &&
-		! grep -E "\|+" content &&
+		grep -E "\|+" content &&
 
 		git config merge.conflictstyle merge &&
 

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 7f6e23a4bb..11444d5360 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh

@@ -25,6 +25,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+git config merge.conflictstyle merge # TODO: use the default
+
 test_tick
 
 fill () {

diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
index 3fcb44767f..90449e80c3 100755
--- a/t/t7506-status-submodule.sh
+++ b/t/t7506-status-submodule.sh

@@ -253,6 +253,7 @@ test_expect_success 'status with merge conflict in .gitmodules' '
 	test_create_repo_with_commit sub2 &&
 	(
 		cd super &&
+		git config merge.conflictstyle merge && # TODO: use the default
 		prev=$(git rev-parse HEAD) &&
 		git checkout -b add_sub1 &&
 		git submodule add ../sub1 &&

diff --git a/xdiff-interface.c b/xdiff-interface.c
index 19a030fbe2..1447771724 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c

@@ -299,7 +299,7 @@ int xdiff_compare_lines(const char *l1, long s1,
 	return xdl_recmatch(l1, s1, l2, s2, flags);
 }
 
-int git_xmerge_style = XDL_MERGE_STYLE_MERGE;
+int git_xmerge_style = XDL_MERGE_STYLE_DIFF3;
 
 int git_xmerge_config(const char *var, const char *value, void *cb)
 {

-- 
2.32.0.2.g41be0a4e50

If we don't specify we are talking about a style, XDL_MERGE_MINIMAL
could be confused with a valid value instead of XDL_MERGE_DIFF3, which
it isn't.

Signed-off-by: Felipe Contreras <redacted>
---
 builtin/merge-file.c | 2 +-
 xdiff-interface.c    | 2 +-
 xdiff/xdiff.h        | 2 +-
 xdiff/xmerge.c       | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index 06a2f90c48..a4097a596f 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c

@@ -33,7 +33,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 	int quiet = 0;
 	struct option options[] = {
 		OPT_BOOL('p', "stdout", &to_stdout, N_("send results to standard output")),
-		OPT_SET_INT(0, "diff3", &xmp.style, N_("use a diff3 based merge"), XDL_MERGE_DIFF3),
+		OPT_SET_INT(0, "diff3", &xmp.style, N_("use a diff3 based merge"), XDL_MERGE_STYLE_DIFF3),
 		OPT_SET_INT(0, "ours", &xmp.favor, N_("for conflicts, use our version"),
 			    XDL_MERGE_FAVOR_OURS),
 		OPT_SET_INT(0, "theirs", &xmp.favor, N_("for conflicts, use their version"),

diff --git a/xdiff-interface.c b/xdiff-interface.c
index 609615db2c..64e2c4e301 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c

@@ -307,7 +307,7 @@ int git_xmerge_config(const char *var, const char *value, void *cb)
 		if (!value)
 			die("'%s' is not a boolean", var);
 		if (!strcmp(value, "diff3"))
-			git_xmerge_style = XDL_MERGE_DIFF3;
+			git_xmerge_style = XDL_MERGE_STYLE_DIFF3;
 		else if (!strcmp(value, "merge"))
 			git_xmerge_style = 0;
 		/*

diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index 7a04605146..45883f5eb3 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h

@@ -64,7 +64,7 @@ extern "C" {
 #define XDL_MERGE_FAVOR_UNION 3
 
 /* merge output styles */
-#define XDL_MERGE_DIFF3 1
+#define XDL_MERGE_STYLE_DIFF3 1
 
 typedef struct s_mmfile {
 	char *ptr;

diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
index 1659edb453..f6916a4ba4 100644
--- a/xdiff/xmerge.c
+++ b/xdiff/xmerge.c

@@ -230,7 +230,7 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,
 	size += xdl_recs_copy(xe1, m->i1, m->chg1, needs_cr, 1,
 			      dest ? dest + size : NULL);
 
-	if (style == XDL_MERGE_DIFF3) {
+	if (style == XDL_MERGE_STYLE_DIFF3) {
 		/* Shared preimage */
 		if (!dest) {
 			size += marker_size + 1 + needs_cr + marker3_size;

@@ -482,7 +482,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1,
 	int style = xmp->style;
 	int favor = xmp->favor;
 
-	if (style == XDL_MERGE_DIFF3) {
+	if (style == XDL_MERGE_STYLE_DIFF3) {
 		/*
 		 * "diff3 -m" output does not make sense for anything
 		 * more aggressive than XDL_MERGE_EAGER.

-- 
2.32.0.2.g41be0a4e50

There is little value in checking that git_xmerge_style isn't 0 before
changing it's default value.

Most of the time it isn't 0 anyway, so just assign the value directly.

Also, add the missing constant for the default value: XDL_MERGE_STYLE_MERGE.

Additionally this change has the benefit that it gets rid of a Yoda
condition.

No functional changes.

Signed-off-by: Felipe Contreras <redacted>
---
 builtin/merge-file.c | 3 +--
 ll-merge.c           | 3 +--
 xdiff-interface.c    | 4 ++--
 xdiff/xdiff.h        | 1 +
 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index a4097a596f..01951762ec 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c

@@ -55,8 +55,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 	if (startup_info->have_repository) {
 		/* Read the configuration file */
 		git_config(git_xmerge_config, NULL);
-		if (0 <= git_xmerge_style)
-			xmp.style = git_xmerge_style;
+		xmp.style = git_xmerge_style;
 	}
 
 	argc = parse_options(argc, argv, prefix, options, merge_file_usage, 0);

diff --git a/ll-merge.c b/ll-merge.c
index 9a8a2c365c..4ce8d3f9cc 100644
--- a/ll-merge.c
+++ b/ll-merge.c

@@ -124,8 +124,7 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 	xmp.level = XDL_MERGE_ZEALOUS;
 	xmp.favor = opts->variant;
 	xmp.xpp.flags = opts->xdl_opts;
-	if (git_xmerge_style >= 0)
-		xmp.style = git_xmerge_style;
+	xmp.style = git_xmerge_style;
 	if (marker_size > 0)
 		xmp.marker_size = marker_size;
 	xmp.ancestor = orig_name;

diff --git a/xdiff-interface.c b/xdiff-interface.c
index 64e2c4e301..19a030fbe2 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c

@@ -299,7 +299,7 @@ int xdiff_compare_lines(const char *l1, long s1,
 	return xdl_recmatch(l1, s1, l2, s2, flags);
 }
 
-int git_xmerge_style = -1;
+int git_xmerge_style = XDL_MERGE_STYLE_MERGE;
 
 int git_xmerge_config(const char *var, const char *value, void *cb)
 {

@@ -309,7 +309,7 @@ int git_xmerge_config(const char *var, const char *value, void *cb)
 		if (!strcmp(value, "diff3"))
 			git_xmerge_style = XDL_MERGE_STYLE_DIFF3;
 		else if (!strcmp(value, "merge"))
-			git_xmerge_style = 0;
+			git_xmerge_style = XDL_MERGE_STYLE_MERGE;
 		/*
 		 * Please update _git_checkout() in
 		 * git-completion.bash when you add new merge config

diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index 45883f5eb3..d24cd9f6ae 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h

@@ -64,6 +64,7 @@ extern "C" {
 #define XDL_MERGE_FAVOR_UNION 3
 
 /* merge output styles */
+#define XDL_MERGE_STYLE_MERGE 0
 #define XDL_MERGE_STYLE_DIFF3 1
 
 typedef struct s_mmfile {

-- 
2.32.0.2.g41be0a4e50

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 6952552c58..978f4e3e70 100755
--- a/t/t6440-config-conflict-markers.sh
+++ b/t/t6440-config-conflict-markers.sh

@@ -41,4 +41,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 -E "\|+" actual &&
+
+		git -c merge.conflictstyle=diff3 merge-tree initial r l >actual &&
+		grep -E "\|+" actual &&
+
+		git -c merge.conflictstyle=merge merge-tree initial r l >actual &&
+		! grep -E "\|+" actual
+	)
+'
+
 test_done

-- 
2.32.0.2.g41be0a4e50

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

The way merge configurations are handled is not correct.
It should be possible to do git_config(merge_recursive_config, ...) just
like we can with git_diff_basic_config and others.

Therefore builtins like `git merge` can't call this function at the
right time.

We shuffle the functions a little bit so at least merge_recursive_config
doesn't call git_xmerge_config directly and thus override previous
configurations.

Signed-off-by: Felipe Contreras <redacted>
---
 builtin/merge-recursive.c          |  3 +++
 builtin/merge.c                    |  4 ++++
 merge-recursive.c                  |  2 +-
 sequencer.c                        |  5 +++++
 t/t6440-config-conflict-markers.sh | 31 ++++++++++++++++++++++++++++++
 5 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c
index a4bfd8fc51..80f9279b4c 100644
--- a/builtin/merge-recursive.c
+++ b/builtin/merge-recursive.c

@@ -4,6 +4,7 @@
 #include "tag.h"
 #include "merge-recursive.h"
 #include "xdiff-interface.h"
+#include "config.h"
 
 static const char builtin_merge_recursive_usage[] =
 	"git %s <base>... -- <head> <remote> ...";

@@ -30,6 +31,8 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
 	char *better1, *better2;
 	struct commit *result;
 
+	git_config(git_xmerge_config, NULL);
+
 	init_merge_options(&o, the_repository);
 	if (argv[0] && ends_with(argv[0], "-subtree"))
 		o.subtree_shift = "";

diff --git a/builtin/merge.c b/builtin/merge.c
index eddb8ae70d..7aa3dbb111 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c

@@ -43,6 +43,7 @@
 #include "commit-reach.h"
 #include "wt-status.h"
 #include "commit-graph.h"
+#include "xdiff-interface.h"
 
 #define DEFAULT_TWOHEAD (1<<0)
 #define DEFAULT_OCTOPUS (1<<1)

@@ -659,6 +660,9 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 	if (status)
 		return status;
 	status = git_gpg_config(k, v, NULL);
+	if (status)
+		return status;
+	status = git_xmerge_config(k, v, NULL);
 	if (status)
 		return status;
 	return git_diff_ui_config(k, v, cb);

diff --git a/merge-recursive.c b/merge-recursive.c
index d146bb116f..10e6e1e4d1 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c

@@ -3845,7 +3845,7 @@ static void merge_recursive_config(struct merge_options *opt)
 		} /* avoid erroring on values from future versions of git */
 		free(value);
 	}
-	git_config(git_xmerge_config, NULL);
+	git_config(git_default_config, NULL);
 }
 
 void init_merge_options(struct merge_options *opt,

diff --git a/sequencer.c b/sequencer.c
index 0bec01cf38..9e2bdca0f6 100644
--- a/sequencer.c
+++ b/sequencer.c

@@ -34,6 +34,7 @@
 #include "commit-reach.h"
 #include "rebase-interactive.h"
 #include "reset.h"
+#include "xdiff-interface.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 

@@ -224,6 +225,10 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)
 	if (status)
 		return status;
 
+	status = git_xmerge_config(k, v, NULL);
+	if (status)
+		return status;
+
 	return git_diff_basic_config(k, v, NULL);
 }
 

diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh
index 44f79ac91b..485ad0eee0 100755
--- a/t/t6440-config-conflict-markers.sh
+++ b/t/t6440-config-conflict-markers.sh

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

-- 
2.32.0.2.g41be0a4e50

On Wed, Jun 9, 2021 at 3:29 PM Felipe Contreras
[off-list ref] wrote:

This seems to duplicate the behavior of test_write_lines()...

+test_expect_success 'merge' '
+       test_create_repo merge &&
+       (
+               cd merge &&
+               fill 1 2 3 >content &&

... which could be used here instead:

    test_write_lines 1 2 3 >content &&

Eric Sunshine wrote:

On Wed, Jun 9, 2021 at 3:29 PM Felipe Contreras
[off-list ref] wrote:

quoted

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>
---

diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh

@@ -0,0 +1,44 @@
+fill () {
+       for i
+       do
+               echo "$i"
+       done
+}

This seems to duplicate the behavior of test_write_lines()...

Right, I'll update the patch.

The above function is used in:

  t6440-config-conflict-markers.sh
  t7201-co.sh

So those two probably should be updated.

Cheers.

-- 
Felipe Contreras

Am 09.06.21 um 21:28 schrieb Felipe Contreras:

Virtually everyone is using it, and it's one of the first things we
teach newcomers in order to resolve conflicts efficiently.

Let's make it the default.

I tested diff3 style the VERY FIRST TIME the other day and was greated
with the below. Needless to say that this change is a no-go from my POV.

Without diff3:

<<<<<<< HEAD
    CClustering ComputeSSLClusters(double threshPercent, const CDataInfo* scale) const override;
    void ComputeDist(DistFunc distFunc, CDoubleArray& dist,
        double& minDist, double& maxDist) const;
    double EstimateNodeDist2() const override;
    std::vector<double> EstimateNeighborMinDist() const override;
=======
    CClustering ComputeSSLClusters(double threshPercent,
        const CDoubleArray& compWeights, const CDataInfo* scale) const override;
    static void ComputeDist(const CNetNodeHolder& vecs, CDoubleArray& dist,
        double& minDist, double& maxDist);

quoted

quoted

quoted

quoted

quoted

quoted

no-compweights-in-cnet 

With diff3:

<<<<<<< HEAD
    CClustering ComputeSSLClusters(double threshPercent, const CDataInfo* scale) const override;
    void ComputeDist(DistFunc distFunc, CDoubleArray& dist,
        double& minDist, double& maxDist) const;
    double EstimateNodeDist2() const override;
    std::vector<double> EstimateNeighborMinDist() const override;
||||||| merged common ancestors
<<<<<<<<< Temporary merge branch 1
    CClustering ComputeSSLClusters(double threshPercent, const CDataInfo* scale) const override;
    void ComputeDist(DistFunc distFunc, CDoubleArray& dist,
        double& minDist, double& maxDist) const;
    virtual void ComputeKNearest(int K, const double*,
        Neighborhood& result) const;
||||||||| d261d9944
    CClustering ComputeClusters(const double* dist, double threshold,
        const CDataInfo* scale) const override;
    virtual void ComputeDist(DistFunc distFunc, CDoubleArray& dist,
        double& minDist, double& maxDist);
    virtual void ComputeUMatrix();
    virtual void ComputeKNearest(int K, const double*,
        Neighborhood& result) const;
=========
    CClustering ComputeClusters(const double* dist, double threshold,
        const CDataInfo* scale) const override;
    virtual void ComputeDist(DistFunc distFunc, CDoubleArray& dist,
        double& minDist, double& maxDist);
    virtual void ComputeUMatrix();
=======
    CClustering ComputeSSLClusters(double threshPercent,
        const CDoubleArray& compWeights, const CDataInfo* scale) const override;
    static void ComputeDist(const CNetNodeHolder& vecs, CDoubleArray& dist,
        double& minDist, double& maxDist);

quoted

quoted

quoted

quoted

quoted

quoted

no-compweights-in-cnet 

-- Hannes

On 2021-06-10 08:41:21+0200, Johannes Sixt [off-list ref] wrote:

Am 09.06.21 um 21:28 schrieb Felipe Contreras:

quoted

Virtually everyone is using it, and it's one of the first things we
teach newcomers in order to resolve conflicts efficiently.

Let's make it the default.

I tested diff3 style the VERY FIRST TIME the other day and was greated
with the below. Needless to say that this change is a no-go from my POV.

I agree, despite using 3-way merging (with external tools) to resolve conflicts.

I prefer the current conflict style, aka no-diff3 conflict style.

-- Danh

Without diff3:

<<<<<<< HEAD
    CClustering ComputeSSLClusters(double threshPercent, const CDataInfo* scale) const override;
    void ComputeDist(DistFunc distFunc, CDoubleArray& dist,
        double& minDist, double& maxDist) const;
    double EstimateNodeDist2() const override;
    std::vector<double> EstimateNeighborMinDist() const override;
=======
    CClustering ComputeSSLClusters(double threshPercent,
        const CDoubleArray& compWeights, const CDataInfo* scale) const override;
    static void ComputeDist(const CNetNodeHolder& vecs, CDoubleArray& dist,
        double& minDist, double& maxDist);

quoted

quoted

quoted

quoted

quoted

quoted

quoted

no-compweights-in-cnet 

With diff3:

<<<<<<< HEAD
    CClustering ComputeSSLClusters(double threshPercent, const CDataInfo* scale) const override;
    void ComputeDist(DistFunc distFunc, CDoubleArray& dist,
        double& minDist, double& maxDist) const;
    double EstimateNodeDist2() const override;
    std::vector<double> EstimateNeighborMinDist() const override;
||||||| merged common ancestors
<<<<<<<<< Temporary merge branch 1
    CClustering ComputeSSLClusters(double threshPercent, const CDataInfo* scale) const override;
    void ComputeDist(DistFunc distFunc, CDoubleArray& dist,
        double& minDist, double& maxDist) const;
    virtual void ComputeKNearest(int K, const double*,
        Neighborhood& result) const;
||||||||| d261d9944
    CClustering ComputeClusters(const double* dist, double threshold,
        const CDataInfo* scale) const override;
    virtual void ComputeDist(DistFunc distFunc, CDoubleArray& dist,
        double& minDist, double& maxDist);
    virtual void ComputeUMatrix();
    virtual void ComputeKNearest(int K, const double*,
        Neighborhood& result) const;
=========
    CClustering ComputeClusters(const double* dist, double threshold,
        const CDataInfo* scale) const override;
    virtual void ComputeDist(DistFunc distFunc, CDoubleArray& dist,
        double& minDist, double& maxDist);
    virtual void ComputeUMatrix();
=======
    CClustering ComputeSSLClusters(double threshPercent,
        const CDoubleArray& compWeights, const CDataInfo* scale) const override;
    static void ComputeDist(const CNetNodeHolder& vecs, CDoubleArray& dist,
        double& minDist, double& maxDist);

quoted

quoted

quoted

quoted

quoted

quoted

quoted

no-compweights-in-cnet 

-- Hannes

-- 
Danh

On 09/06/2021 20:28, Felipe Contreras wrote:

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

I'm not sure what this particular tests adds over the existing ones in 
t6427-diff3-conflict-markers.sh. The commit message does not explain why 
a new file is better than adding this test to that file. There are 
already diff3 tests for checkout so it ends up being confusing when a 
checkout test gets added to this file rather than with those tests later 
in the series because there is no longer a single location for diff3 
checkout tests.

! grep "|"  would be simpler and just as effective. This is quite a weak 
test, something like "^|||||| " would be a stronger test for conflict 
markers

Best Wishes

Phillip

+		git reset --hard &&
+		test_must_fail git -c merge.conflictstyle=diff3 merge r &&
+		grep -E "\|+" content &&
+
+		git reset --hard &&
+		test_must_fail git -c merge.conflictstyle=merge merge r &&
+		! grep -E "\|+" content
+	)
+'
+
+test_done

On 09/06/2021 20:28, Felipe Contreras wrote:

The subject would make more sense as 'xdiff: rename XDL_MERGE_DIFF3 to 
XDL_MERGE_STYLE_DIFF3' rather than using the new name of the constant alone.

If we don't specify we are talking about a style, XDL_MERGE_MINIMAL
could be confused with a valid value instead of XDL_MERGE_DIFF3, which
it isn't.

I don't object to the rename but what is the source of the confusion 
with XDL_MERGE_MINIMAL?

Best Wishes

Phillip

On 09/06/2021 20:28, Felipe Contreras wrote:

I don't find the commit message explains this change very well

There is little value in checking that git_xmerge_style isn't 0 before
changing it's default value.

I think the check is actually that git_xmerge_style isn't -1. Why is 
there little value in the check?

Most of the time it isn't 0 anyway, so just assign the value directly.

Why to the times when it is zero (or -1) not matter?

Also, add the missing constant for the default value: XDL_MERGE_STYLE_MERGE.

Additionally this change has the benefit that it gets rid of a Yoda
condition.

No functional changes.

I think that is probably correct but it would be helpful if the commit 
message offered a bit more explanation.

Best Wishes

Phillip

On 09/06/2021 20:28, Felipe Contreras wrote:

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

The way merge configurations are handled is not correct.
It should be possible to do git_config(merge_recursive_config, ...) just
like we can with git_diff_basic_config and others.

It would be helpful to explain what the problem with 
merge_recursive_config() actually is rather than just saying "it should 
be possible ..."

Therefore builtins like `git merge` can't call this function at the
right time.

 >

We shuffle the functions a little bit so at least merge_recursive_config
doesn't call git_xmerge_config directly and thus override previous
configurations.

Rather than papering of the problem, how difficult would it be to add a 
field to ll_merge_options and pass the conflict style with that rather 
than fiddling with the order that we set a global variable.

Does this change affect 'am/apply -3'? - Do they still read the config 
setting properly?

Best Wishes

Phillip

On 09/06/2021 20:28, Felipe Contreras wrote:

Virtually everyone is using it, and it's one of the first things we
teach newcomers in order to resolve conflicts efficiently.

Given there are millions of users I'm not sure how you established that 
virtually everyone is using it. I think that while this change would be 
useful to some users (though not many if virtually everyone already has 
it set) it has the potential to annoy a lot of users who are happy with 
the existing default. I do not think that it is a positive change over all.

Had the default been diff3 from early on in git's history then I would 
not advocate changing it to the current default but I think the time has 
passed when it can be changed without inconveniencing existing users.

The patches up to this point have useful fixes in them which would 
improve git, thanks for working on them.

Best Wishes

Phillip

Johannes Sixt wrote:

Am 09.06.21 um 21:28 schrieb Felipe Contreras:

quoted

Virtually everyone is using it, and it's one of the first things we
teach newcomers in order to resolve conflicts efficiently.

Let's make it the default.

I tested diff3 style the VERY FIRST TIME the other day and was greated
with the below. Needless to say that this change is a no-go from my POV.

"I found a bug" is not a valid reason not to do approach X.

The bug gets fixed and the approach continues.

Cheers.

-- 
Felipe Contreras

Đoàn Trần Công Danh wrote:

On 2021-06-10 08:41:21+0200, Johannes Sixt [off-list ref] wrote:

quoted

Am 09.06.21 um 21:28 schrieb Felipe Contreras:

quoted

Virtually everyone is using it, and it's one of the first things we
teach newcomers in order to resolve conflicts efficiently.

Let's make it the default.

I tested diff3 style the VERY FIRST TIME the other day and was greated
with the below. Needless to say that this change is a no-go from my POV.

I agree, despite using 3-way merging (with external tools) to resolve conflicts.

I prefer the current conflict style, aka no-diff3 conflict style.

Defaults are not for you, they are for the majority of users.

-- 
Felipe Contreras

Phillip Wood wrote:

On 09/06/2021 20:28, Felipe Contreras wrote:

quoted

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

I'm not sure what this particular tests adds over the existing ones in 
t6427-diff3-conflict-markers.sh.

That file is for diff3 conflict markers. The tests in this file are not.

The commit message does not explain why a new file is better than
adding this test to that file.

Because there's no file that is testing for this.

There are already diff3 tests for checkout

This file is not doing diff3 tests.


As stated above, it's testing different *combinations* of
merge.conflictstyle, diff3 is only *one* of the possibilities, another
possibility is:

  git -c merge.conflictstyle=diff3 checkout -m --conflict=merge

That is *not* a diff3 test.

quoted

diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh
new file mode 100755

quoted

+test_expect_success 'merge' '
+	test_create_repo merge &&
+	(
+		cd merge &&
+
+		fill 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 -E "\|+" content &&

! grep "|"  would be simpler and just as effective.

But that would fail if there's a "command1 | command2".

This is quite a weak 
test, something like "^|||||| " would be a stronger test for conflict 
markers

But that doesn't work in all the tests.

Cheers.

-- 
Felipe Contreras

Phillip Wood wrote:

On 09/06/2021 20:28, Felipe Contreras wrote:

The subject would make more sense as 'xdiff: rename XDL_MERGE_DIFF3 to 
XDL_MERGE_STYLE_DIFF3' rather than using the new name of the constant alone.

That is 55 characters, more than the recommended commit title length.

quoted

If we don't specify we are talking about a style, XDL_MERGE_MINIMAL
could be confused with a valid value instead of XDL_MERGE_DIFF3, which
it isn't.

I don't object to the rename but what is the source of the confusion 
with XDL_MERGE_MINIMAL?

XDL_MERGE_MINIMAL and other XDL_MERGE_FOO constants go into xmparam_t.level,
XDL_MERGE_DIFF3 does not.

As stated in the commit message, the name XDL_MERGE_DIFF3 doesn't
distinguish it as a style.

Chers.

-- 
Felipe Contreras

On Thu, Jun 10, 2021 at 08:41:21AM +0200, Johannes Sixt wrote:

Am 09.06.21 um 21:28 schrieb Felipe Contreras:

quoted

Virtually everyone is using it, and it's one of the first things we
teach newcomers in order to resolve conflicts efficiently.

Let's make it the default.

I tested diff3 style the VERY FIRST TIME the other day and was greated
with the below. Needless to say that this change is a no-go from my POV.
[...]

I didn't look too deeply at your example, but I suspect it may be
related to the fact that diff3 does not try to minimize the conflicts as
much (and then the recursive merge on top of that piles on extra layers
of confusion).

There's a lot more discussion in this old thread:

  https://lore.kernel.org/git/20130306150548.GC15375@pengutronix.de/

-Peff

Phillip Wood wrote:

On 09/06/2021 20:28, Felipe Contreras wrote:

I don't find the commit message explains this change very well

quoted

There is little value in checking that git_xmerge_style isn't 0 before
changing it's default value.

I think the check is actually that git_xmerge_style isn't -1.

Actually I meant "< 0", but yeah, that's mainly to check the -1 case.

Why is there little value in the check?

It's explained in the very next sentence.

quoted

Most of the time it isn't 0 anyway, so just assign the value directly.

Why to the times when it is zero (or -1) not matter?

When it's 0 it's a no-op, and now it can't be -1.

By default structures are zeroed in git, so the defaults of integers
are 0, and in the case of xmparam_t.style that is no exception.

These are all the same:

	static xmparam_t xmp;

	static xmparam_t xmp;
	xmp.style = 0;

	static xmparam_t xmp;
	if (1)
		xmp.style = 0;

But of course as it's explained most of the time that's not what
happens, what happens is:

  if (1)
    xml.style = 1;

Perhaps this is clearer:

  There is little value in checking that git_xmerge_style isn't < 0
  before changing its value to xmp.style. If it's 0 then assigning 0 to
  xmp.style is a no-op, and if it's 1 (as it usually is), we are going
  to assign the value anyway.

  The only exception is when git_xmerge_style is -1, but there is no
  value in having that as default, so we just don't, and set the default
  to 0.

Cheers.

-- 
Felipe Contreras

Phillip Wood wrote:

On 09/06/2021 20:28, Felipe Contreras wrote:

quoted

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

The way merge configurations are handled is not correct.
It should be possible to do git_config(merge_recursive_config, ...) just
like we can with git_diff_basic_config and others.

It would be helpful to explain what the problem with 
merge_recursive_config() actually is rather than just saying "it should 
be possible ..."

The problem is that you can't do this:

  git_config(merge_recursive_config, NULL);

As it was explained.

That is the problem. I don't know how that's not clear.

quoted

Therefore builtins like `git merge` can't call this function at the
right time.

 >

quoted

We shuffle the functions a little bit so at least merge_recursive_config
doesn't call git_xmerge_config directly and thus override previous
configurations.

Rather than papering of the problem, how difficult would it be to add a 
field to ll_merge_options and pass the conflict style with that rather 
than fiddling with the order that we set a global variable.

Probably not that difficult, but then we also need a parser that
converts from "diff3" to whatever values we decide in that new field. We
would need a new parse_config_conflict_style() function.

And that function will be only used by `git checkout` and nothing else.
So I don't think there's much value in it.

That problem whoever, is orthogonal to this series.

Does this change affect 'am/apply -3'? - Do they still read the config 
setting properly?

Good question. I'll have to add more tests to make sure that works
properly.

Cheers.

-- 
Felipe Contreras

Phillip Wood wrote:

On 09/06/2021 20:28, Felipe Contreras wrote:

quoted

Virtually everyone is using it, and it's one of the first things we
teach newcomers in order to resolve conflicts efficiently.

Given there are millions of users I'm not sure how you established that 
virtually everyone is using it.

Because it's the stablished consensus that resolving conflicts with
merge.conflictstyle=merge is suboptimal.

Even if it's not the majority using it (say 49%), the majority would
benefit from using it.

I think that while this change would be useful to some users (though
not many if virtually everyone already has it set) it has the
potential to annoy a lot of users who are happy with the existing
default.

Every change has the potential to annoy some users.

That's an argument against further development of git, not this
particular patch.

I do not think that it is a positive change over all.

OK. Others disagree.

Had the default been diff3 from early on in git's history then I would 
not advocate changing it to the current default but I think the time has 
passed when it can be changed without inconveniencing existing users.

git has changed defaults in the past, and it will change defaults in the
future.

There will always be people pushing back against progress, and that is
good. But progress happens regardless.

Cheers.

-- 
Felipe Contreras

On 10/06/2021 15:11, Felipe Contreras wrote:

Phillip Wood wrote:

quoted

On 09/06/2021 20:28, Felipe Contreras wrote:

quoted

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

The way merge configurations are handled is not correct.
It should be possible to do git_config(merge_recursive_config, ...) just
like we can with git_diff_basic_config and others.

It would be helpful to explain what the problem with
merge_recursive_config() actually is rather than just saying "it should
be possible ..."

The problem is that you can't do this:

   git_config(merge_recursive_config, NULL);

As it was explained.

You do not explain why you cannot do that

That is the problem. I don't know how that's not clear.

quoted

quoted

Therefore builtins like `git merge` can't call this function at the
right time.

  >

quoted

We shuffle the functions a little bit so at least merge_recursive_config
doesn't call git_xmerge_config directly and thus override previous
configurations.

Rather than papering of the problem, how difficult would it be to add a
field to ll_merge_options and pass the conflict style with that rather
than fiddling with the order that we set a global variable.

Probably not that difficult, but then we also need a parser that
converts from "diff3" to whatever values we decide in that new field. We
would need a new parse_config_conflict_style() function.
And that function will be only used by `git checkout` and nothing else.
So I don't think there's much value in it.

It would allow us to add a --conflict option to all the mergey commands 
in the future and would be much easier to reason about than the approach 
of juggling where we call git_xmerge_config(). This patch requires us to 
audit all the code paths that end up in merge_recursive_config() to make 
sure they now call git_xmerge_config() themselves. You don't seem to 
have done that as you don't know if am/apply are affected or not.


Best Wishes

Phillip

That problem whoever, is orthogonal to this series.

quoted

Does this change affect 'am/apply -3'? - Do they still read the config
setting properly?

Good question. I'll have to add more tests to make sure that works
properly.

Cheers.

On 10/06/2021 14:26, Felipe Contreras wrote:

Phillip Wood wrote:

quoted

On 09/06/2021 20:28, Felipe Contreras wrote:

quoted

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

I'm not sure what this particular tests adds over the existing ones in
t6427-diff3-conflict-markers.sh.

That file is for diff3 conflict markers. The tests in this file are not.

quoted

The commit message does not explain why a new file is better than
adding this test to that file.

Because there's no file that is testing for this.

quoted

There are already diff3 tests for checkout

This file is not doing diff3 tests.


As stated above, it's testing different *combinations* of
merge.conflictstyle, diff3 is only *one* of the possibilities, another
possibility is:

   git -c merge.conflictstyle=diff3 checkout -m --conflict=merge

That is *not* a diff3 test.

I think that is an artificial distinction, it is testing the behavior of 
checkout when merge.conflictStyle=diff3 just like the other tests, it 
just happens to be checking that the config option can be combined with 
a command line option.

Best Wishes

Phillip

quoted

quoted

diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh
new file mode 100755

quoted

quoted

+test_expect_success 'merge' '
+	test_create_repo merge &&
+	(
+		cd merge &&
+
+		fill 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 -E "\|+" content &&

! grep "|"  would be simpler and just as effective.

But that would fail if there's a "command1 | command2".

quoted

This is quite a weak
test, something like "^|||||| " would be a stronger test for conflict
markers

But that doesn't work in all the tests.

Cheers.

On 10/06/2021 14:26, Felipe Contreras wrote:

Phillip Wood wrote:

quoted

On 09/06/2021 20:28, Felipe Contreras wrote:

quoted

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

[...]

quoted

diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh
new file mode 100755

quoted

quoted

+test_expect_success 'merge' '
+	test_create_repo merge &&
+	(
+		cd merge &&
+
+		fill 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 -E "\|+" content &&

! grep "|"  would be simpler and just as effective.

But that would fail if there's a "command1 | command2".

I don't understand. What are you expecting content to contain? Why 
doesn't "\|+" fail in that case?

quoted

This is quite a weak
test, something like "^|||||| " would be a stronger test for conflict
markers

But that doesn't work in all the tests.

So test for what you actually expect, you don't need to have the same 
check in all the tests if the expected output is different.

Best Wishes

Phillip

Cheers.

Jeff King wrote:

On Thu, Jun 10, 2021 at 08:41:21AM +0200, Johannes Sixt wrote:

quoted

Am 09.06.21 um 21:28 schrieb Felipe Contreras:

quoted

Virtually everyone is using it, and it's one of the first things we
teach newcomers in order to resolve conflicts efficiently.

Let's make it the default.

I tested diff3 style the VERY FIRST TIME the other day and was greated
with the below. Needless to say that this change is a no-go from my POV.
[...]

I didn't look too deeply at your example, but I suspect it may be
related to the fact that diff3 does not try to minimize the conflicts as
much (and then the recursive merge on top of that piles on extra layers
of confusion).

There's a lot more discussion in this old thread:

  https://lore.kernel.org/git/20130306150548.GC15375@pengutronix.de/

Geezus. My patches always end up kicking the hornest nest don't they?

Maybe it would make sense to revive the zdiff3 patch and attempt to make
that the default. That would take a lot of time though, so it wasn't as
easy as just flipping a switch from "merge" to "diff3".

But there is value in attempting to make the default merge.conflictstyle
work for everyone (or last least as many people as possible). It's
shinning a light on issues that are already present now.

For reference--and since gmane links don't work any more--here are the
relevant links for the past discussions:

https://lore.kernel.org/git/20130306150548.GC15375@pengutronix.de/
https://lore.kernel.org/git/alpine.LFD.1.10.0808311021120.12958@nehalem.linux-foundation.org/
https://lore.kernel.org/git/1220056963-2352-5-git-send-email-gitster@pobox.com/

Cheers.

-- 
Felipe Contreras

On Thu, Jun 10, 2021 at 11:00:59AM -0500, Felipe Contreras wrote:

quoted

I didn't look too deeply at your example, but I suspect it may be
related to the fact that diff3 does not try to minimize the conflicts as
much (and then the recursive merge on top of that piles on extra layers
of confusion).

There's a lot more discussion in this old thread:

  https://lore.kernel.org/git/20130306150548.GC15375@pengutronix.de/

Geezus. My patches always end up kicking the hornest nest don't they?

Maybe it would make sense to revive the zdiff3 patch and attempt to make
that the default. That would take a lot of time though, so it wasn't as
easy as just flipping a switch from "merge" to "diff3".

I had that patch in my daily build for several years, and I would
occasionally trigger it when seeing an ugly conflict. IIRC, it
segfaulted on me a few times, but I never tracked down the bug. Just a
caution in case anybody wants to resurrect it.

-Peff

Phillip Wood wrote:

On 10/06/2021 15:11, Felipe Contreras wrote:

quoted

Phillip Wood wrote:

quoted

On 09/06/2021 20:28, Felipe Contreras wrote:

quoted

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

The way merge configurations are handled is not correct.
It should be possible to do git_config(merge_recursive_config, ...) just
like we can with git_diff_basic_config and others.

It would be helpful to explain what the problem with
merge_recursive_config() actually is rather than just saying "it should
be possible ..."

The problem is that you can't do this:

   git_config(merge_recursive_config, NULL);

As it was explained.

You do not explain why you cannot do that

  % git grep merge_recursive_config
  static void merge_recursive_config(struct merge_options *opt)

For starters it's a static function.

Second, clearly the type of functions git_config() receives are not
`void (*)(struct merge_options *)`.

I mean, I do see value in explaning as much detail as needed in the
commit message, but it shouldn't be a lesson on git's codebase:
git_config() is a standard thing, and it's even mentioned in the user
manual.

https://git-scm.com/docs/user-manual#birdview-on-the-source-code

quoted

That is the problem. I don't know how that's not clear.

quoted

quoted

Therefore builtins like `git merge` can't call this function at the
right time.

  >

quoted

We shuffle the functions a little bit so at least merge_recursive_config
doesn't call git_xmerge_config directly and thus override previous
configurations.

Rather than papering of the problem, how difficult would it be to add a
field to ll_merge_options and pass the conflict style with that rather
than fiddling with the order that we set a global variable.

Probably not that difficult, but then we also need a parser that
converts from "diff3" to whatever values we decide in that new field. We
would need a new parse_config_conflict_style() function.
And that function will be only used by `git checkout` and nothing else.
So I don't think there's much value in it.

It would allow us to add a --conflict option to all the mergey commands 
in the future and would be much easier to reason about than the approach 
of juggling where we call git_xmerge_config().