All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC/PATCH] mergetool: use resolved conflicts in all the views
@ 2020-12-16 17:43 Felipe Contreras
  2020-12-16 22:24 ` Junio C Hamano
  2020-12-17  2:35 ` [RFC/PATCH v2] " Felipe Contreras
  0 siblings, 2 replies; 70+ messages in thread
From: Felipe Contreras @ 2020-12-16 17:43 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, David Aguilar, Johannes Sixt, Felipe Contreras,
	Seth House

It doesn't make sense to display already-resolved conflicts in the
different views of all mergetools.

We already have the best version in MERGED, with annotations that can
be used to extract a pruned version of LOCAL and REMOTE. If we are using
the diff3 conflict-style, we can even extract BASE.

Let's use these annotations instead of using the original files before
the conflict resolution.

TODO: There may be a better way to extract these files that doesn't rely
on the user's conflict-style configuration.

See Seth House's blog post [1] for the idea and the rationale.

[1] https://www.eseth.org/2020/mergetools.html

Cc: Seth House <seth@eseth.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-mergetool.sh | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index e3f6d543fb..4759433d46 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -227,18 +227,6 @@ stage_submodule () {
 	git update-index --add --replace --cacheinfo 160000 "$submodule_sha1" "${work_rel_path%/}" || die
 }
 
-checkout_staged_file () {
-	tmpfile="$(git checkout-index --temp --stage="$1" "$2" 2>/dev/null)" &&
-	tmpfile=${tmpfile%%'	'*}
-
-	if test $? -eq 0 && test -n "$tmpfile"
-	then
-		mv -- "$(git rev-parse --show-cdup)$tmpfile" "$3"
-	else
-		>"$3"
-	fi
-}
-
 merge_file () {
 	MERGED="$1"
 
@@ -318,9 +306,10 @@ merge_file () {
 	# where the base's directory no longer exists.
 	mkdir -p "$(dirname "$MERGED")"
 
-	checkout_staged_file 1 "$MERGED" "$BASE"
-	checkout_staged_file 2 "$MERGED" "$LOCAL"
-	checkout_staged_file 3 "$MERGED" "$REMOTE"
+	# TODO: How do we get $MERGED always with diff3?
+	sed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======$/,/^>>>>>>> /d' "$MERGED" > "$BASE"
+	sed -e '/^<<<<<<< /,/^=======$/d' -e '/^>>>>>>> /d' "$MERGED" > "$LOCAL"
+	sed -e '/^||||||| /,/^>>>>>>> /d' -e '/^<<<<<<< /d' "$MERGED" > "$REMOTE"
 
 	if test -z "$local_mode" || test -z "$remote_mode"
 	then
-- 
2.30.0.rc0


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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-16 17:43 [RFC/PATCH] mergetool: use resolved conflicts in all the views Felipe Contreras
@ 2020-12-16 22:24 ` Junio C Hamano
  2020-12-16 22:53   ` Seth House
  2020-12-16 23:41   ` Felipe Contreras
  2020-12-17  2:35 ` [RFC/PATCH v2] " Felipe Contreras
  1 sibling, 2 replies; 70+ messages in thread
From: Junio C Hamano @ 2020-12-16 22:24 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, David Aguilar, Johannes Sixt, Seth House

Felipe Contreras <felipe.contreras@gmail.com> writes:

> It doesn't make sense to display already-resolved conflicts in the
> different views of all mergetools.
>
> We already have the best version in MERGED, with annotations that can
> be used to extract a pruned version of LOCAL and REMOTE. If we are using
> the diff3 conflict-style, we can even extract BASE.
>
> Let's use these annotations instead of using the original files before
> the conflict resolution.
>
> TODO: There may be a better way to extract these files that doesn't rely
> on the user's conflict-style configuration.
>
> See Seth House's blog post [1] for the idea and the rationale.
>
> [1] https://www.eseth.org/2020/mergetools.html
>
> Cc: Seth House <seth@eseth.com>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>

Hmph, I got what Seth showed, but I do not quite see how the ideas
in the post relate to what this patch does.  The patch just avoids
grabbing the contents of each stage out to a file for three stages
using "git checkout-index" and instead does the same by munging the
diff3 output, which ought to have the same information at least for
text files, using "sed", or is there something I am not seeing?

If there is none, then what is the benefit of doing the same thing
without running 3 checkout-index?  Performance?

Also using checkout-index to grab the raw contents without relying
on the textual context marker would mean that a custom mergetool
backend could be used to merge non-text files.  So if this
optimization (I am still assuming this is "doing the same without
running three checkout-index to gain performance" I read out of the
patch text) is worth doing, there needs a knob to allow users to opt
out of it somehow to avoid regressing on the feature front.

If the mergetool were in control to (re)start the merge process that
would result in the conflicted state, we could override the end-user
preference on the conflict style (either 'GNU merge'-style or
'diff3'-style) and conflict-marker-length to be used in showing
textual conflicts, but as I understand "mergetool" is handed an
already conflicted state and asked to resolve it, it would not be
possible without at least looking at the stage #1 to recover the
base for folks who do not use diff3 style.

> ---
>  git-mergetool.sh | 19 ++++---------------
>  1 file changed, 4 insertions(+), 15 deletions(-)
>
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index e3f6d543fb..4759433d46 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -227,18 +227,6 @@ stage_submodule () {
>  	git update-index --add --replace --cacheinfo 160000 "$submodule_sha1" "${work_rel_path%/}" || die
>  }
>  
> -checkout_staged_file () {
> -	tmpfile="$(git checkout-index --temp --stage="$1" "$2" 2>/dev/null)" &&
> -	tmpfile=${tmpfile%%'	'*}
> -
> -	if test $? -eq 0 && test -n "$tmpfile"
> -	then
> -		mv -- "$(git rev-parse --show-cdup)$tmpfile" "$3"
> -	else
> -		>"$3"
> -	fi
> -}
> -
>  merge_file () {
>  	MERGED="$1"
>  
> @@ -318,9 +306,10 @@ merge_file () {
>  	# where the base's directory no longer exists.
>  	mkdir -p "$(dirname "$MERGED")"
>  
> -	checkout_staged_file 1 "$MERGED" "$BASE"
> -	checkout_staged_file 2 "$MERGED" "$LOCAL"
> -	checkout_staged_file 3 "$MERGED" "$REMOTE"
> +	# TODO: How do we get $MERGED always with diff3?
> +	sed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======$/,/^>>>>>>> /d' "$MERGED" > "$BASE"
> +	sed -e '/^<<<<<<< /,/^=======$/d' -e '/^>>>>>>> /d' "$MERGED" > "$LOCAL"
> +	sed -e '/^||||||| /,/^>>>>>>> /d' -e '/^<<<<<<< /d' "$MERGED" > "$REMOTE"
>  

Style (lose SP after '>' redirection; I looked at the existing code
and spotted one existing violation but that is not an excuse to make
things even less consistent).

Also, conflict-marker-size=<N> attributes can change the length, so
hardcoding these patterns would not work for everybody.

>  	if test -z "$local_mode" || test -z "$remote_mode"
>  	then

Thanks.

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-16 22:24 ` Junio C Hamano
@ 2020-12-16 22:53   ` Seth House
  2020-12-17  5:18     ` Junio C Hamano
  2020-12-17  5:41     ` Felipe Contreras
  2020-12-16 23:41   ` Felipe Contreras
  1 sibling, 2 replies; 70+ messages in thread
From: Seth House @ 2020-12-16 22:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Felipe Contreras, git, David Aguilar, Johannes Sixt

I appreciate Felipe getting the discussion started.

On Wed, Dec 16, 2020 at 02:24:23PM -0800, Junio C Hamano wrote:
> If there is none, then what is the benefit of doing the same thing
> without running 3 checkout-index?

I wasn't aware of this plubming when I wrote the initial shell-script
version of the technique. This is a much better approach (even *if*
there's a negligible performance penalty). This nicely avoids
UNIX/Windows line-ending surprises, and instead leans on
already-configured Git defaults for those. Plus the non-text files
benefit you mentioned is also huge.

> as I understand "mergetool" is handed an
> already conflicted state and asked to resolve it, it would not be
> possible without at least looking at the stage #1 to recover the
> base for folks who do not use diff3 style.

I feel strongly that LOCAL, REMOTE, and BASE should be left intact for
this reason, Also because they aid readers in understanding the
pre-conflicts versions of the file.

Rather mergetools (that support it) should be given the stage 1-3
versions of the file in addition to the usual, unmodified, above three.
Then each tool can decide whether or how to show each. Some graphical
tools might be able to make effective use of all five (six?).

(Feedback & other ideas are very welcome.)


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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-16 22:24 ` Junio C Hamano
  2020-12-16 22:53   ` Seth House
@ 2020-12-16 23:41   ` Felipe Contreras
  2020-12-17  5:19     ` Junio C Hamano
  1 sibling, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-16 23:41 UTC (permalink / raw)
  To: Junio C Hamano, Felipe Contreras
  Cc: git, David Aguilar, Johannes Sixt, Seth House

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > It doesn't make sense to display already-resolved conflicts in the
> > different views of all mergetools.
> >
> > We already have the best version in MERGED, with annotations that can
> > be used to extract a pruned version of LOCAL and REMOTE. If we are using
> > the diff3 conflict-style, we can even extract BASE.
> >
> > Let's use these annotations instead of using the original files before
> > the conflict resolution.
> >
> > TODO: There may be a better way to extract these files that doesn't rely
> > on the user's conflict-style configuration.
> >
> > See Seth House's blog post [1] for the idea and the rationale.
> >
> > [1] https://www.eseth.org/2020/mergetools.html
> >
> > Cc: Seth House <seth@eseth.com>
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> 
> Hmph, I got what Seth showed, but I do not quite see how the ideas
> in the post relate to what this patch does.  The patch just avoids
> grabbing the contents of each stage out to a file for three stages
> using "git checkout-index" and instead does the same by munging the
> diff3 output, which ought to have the same information at least for
> text files, using "sed", or is there something I am not seeing?

It's not quite the same information.

Take the following script that uses Seth's example:

----------------------------------------
cat > BASE <<EOF
A

"Beware the Jabberwock, my son!
The jaws that bite, the claws that catch!
Beware the Jub jub bird, and shun
The frumious bandersnatch!"
EOF

cat > LOCAL <<EOF
B

"Beware the Jabberwock, my son!
The jaws that bite, the claws that catch!
Beware the Jub jub bird, and shun
The frumious bandersnatch!"
EOF

cat > REMOTE <<EOF
C

"Beware the Jabberwock, my son!
The jaws that bite, the claws that catch!
Beware the Jubjub bird, and shun
The frumious Bandersnatch!"
EOF

git merge-file "$@" --diff3 -p LOCAL BASE REMOTE
----------------------------------------

Notice how git is smart enough to resolve the conflicts of the second
paragraph, so the user doesn't have to do anything.

LOCAL is the equivalent of "git checkout-index --stage 2", but that
doesn't have the resolved conflict.

We could call "git merge-file --ours" and overwrite $LOCAL; that way the
user is not presented with any diff for the second paragraph. The same
with --theirs and $REMOTE, but there's no "git merge-file --base".


The implementation details of the proposed patch are not relevant at
this point; it was just to show an example of what Seth's diffconflicts
vim plugin does.

Cheers.

-- 
Felipe Contreras

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

* [RFC/PATCH v2] mergetool: use resolved conflicts in all the views
  2020-12-16 17:43 [RFC/PATCH] mergetool: use resolved conflicts in all the views Felipe Contreras
  2020-12-16 22:24 ` Junio C Hamano
@ 2020-12-17  2:35 ` Felipe Contreras
  1 sibling, 0 replies; 70+ messages in thread
From: Felipe Contreras @ 2020-12-17  2:35 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, David Aguilar, Johannes Sixt, Felipe Contreras,
	Seth House

It doesn't make sense to display easily-solvable conflicts in the
different views of all mergetools.

Only the chunks that warrant conflict markers should be displayed.

TODO: There should be a better way to get the BASE version (maybe add
--base to git mergetool). Or maybe a way to generate the three files in
one go.

See Seth House's blog post [1] for the idea and the rationale.

[1] https://www.eseth.org/2020/mergetools.html

Cc: Seth House <seth@eseth.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-mergetool.sh     | 11 +++++++++++
 t/t7610-mergetool.sh | 17 +++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index e3f6d543fb..2f71da4574 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -322,6 +322,17 @@ merge_file () {
 	checkout_staged_file 2 "$MERGED" "$LOCAL"
 	checkout_staged_file 3 "$MERGED" "$REMOTE"
 
+	# TODO Shouldn't merge-file have a --base option?
+	git merge-file --diff3 -q -p "$LOCAL" "$BASE" "$REMOTE" |
+		sed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======$/,/^>>>>>>> /d' \
+			>"${BASE}_resolved"
+	git merge-file --ours -q -p "$LOCAL" "$BASE" "$REMOTE" >"${LOCAL}_resolved"
+	git merge-file --theirs -q -p "$LOCAL" "$BASE" "$REMOTE" >"${REMOTE}_resolved"
+
+	mv -f "${BASE}_resolved" "$BASE"
+	mv -f "${LOCAL}_resolved" "$LOCAL"
+	mv -f "${REMOTE}_resolved" "$REMOTE"
+
 	if test -z "$local_mode" || test -z "$remote_mode"
 	then
 		echo "Deleted merge conflict for '$MERGED':"
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 70afdd06fa..69260c4a46 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -828,4 +828,21 @@ test_expect_success 'mergetool -Oorder-file is honored' '
 	test_cmp expect actual
 '
 
+test_expect_success 'skip unnecessary chunks' '
+	test_when_finished "git reset --hard" &&
+	git checkout -b test${test_count}_b master &&
+	echo -e "base\n\na" >file1 &&
+	git commit -a -m "base" &&
+	echo -e "base\n\nc" >file1 &&
+	git commit -a -m "remote update" &&
+	git checkout -b test${test_count}_a HEAD~ &&
+	echo -e "local\n\nb" >file1 &&
+	git commit -a -m "local update" &&
+	test_must_fail git merge test${test_count}_b &&
+	yes "" | git mergetool file1 &&
+	echo -e "local\n\nc" >expect &&
+	test_cmp expect file1 &&
+	git commit -m "test resolved with mergetool"
+'
+
 test_done
-- 
2.30.0.rc0


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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-16 22:53   ` Seth House
@ 2020-12-17  5:18     ` Junio C Hamano
  2020-12-17  5:41     ` Felipe Contreras
  1 sibling, 0 replies; 70+ messages in thread
From: Junio C Hamano @ 2020-12-17  5:18 UTC (permalink / raw)
  To: Seth House; +Cc: Felipe Contreras, git, David Aguilar, Johannes Sixt

Seth House <seth@eseth.com> writes:

> I wasn't aware of this plubming when I wrote the initial shell-script
> version of the technique. This is a much better approach (even *if*
> there's a negligible performance penalty). This nicely avoids
> UNIX/Windows line-ending surprises, and instead leans on
> already-configured Git defaults for those. Plus the non-text files
> benefit you mentioned is also huge.
>
>> as I understand "mergetool" is handed an
>> already conflicted state and asked to resolve it, it would not be
>> possible without at least looking at the stage #1 to recover the
>> base for folks who do not use diff3 style.
>
> I feel strongly that LOCAL, REMOTE, and BASE should be left intact for
> this reason, Also because they aid readers in understanding the
> pre-conflicts versions of the file.

Well, good to see that we helped somebody who we do not see
regularly around here.  You should hang around here more often ;-)


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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-16 23:41   ` Felipe Contreras
@ 2020-12-17  5:19     ` Junio C Hamano
  2020-12-17  5:43       ` Felipe Contreras
  0 siblings, 1 reply; 70+ messages in thread
From: Junio C Hamano @ 2020-12-17  5:19 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, David Aguilar, Johannes Sixt, Seth House

Felipe Contreras <felipe.contreras@gmail.com> writes:

> The implementation details of the proposed patch are not relevant at
> this point; it was just to show an example of what Seth's diffconflicts
> vim plugin does.

Sorry for commenting on the "irrelevant" part; as the patch was
marked with RFC label, I bothered to read it and tried to give
comments, but it wasn't clear which part was welcoming comments
and which parts were off limits ;-)


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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-16 22:53   ` Seth House
  2020-12-17  5:18     ` Junio C Hamano
@ 2020-12-17  5:41     ` Felipe Contreras
  2020-12-17  7:35       ` Johannes Sixt
  1 sibling, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-17  5:41 UTC (permalink / raw)
  To: Seth House, Junio C Hamano
  Cc: Felipe Contreras, git, David Aguilar, Johannes Sixt

Seth House wrote:
> I appreciate Felipe getting the discussion started.
> 
> On Wed, Dec 16, 2020 at 02:24:23PM -0800, Junio C Hamano wrote:
> > If there is none, then what is the benefit of doing the same thing
> > without running 3 checkout-index?
> 
> I wasn't aware of this plubming when I wrote the initial shell-script
> version of the technique. This is a much better approach (even *if*
> there's a negligible performance penalty). This nicely avoids
> UNIX/Windows line-ending surprises, and instead leans on
> already-configured Git defaults for those. Plus the non-text files
> benefit you mentioned is also huge.

I think you misunderstood.

This command:

  git checkout-index --stage 2 --temp -- poem.txt

Will give you *exactly* the same output as LOCAL.

The context is "git mergetool", not the mergetool itself.

> > as I understand "mergetool" is handed an
> > already conflicted state and asked to resolve it, it would not be
> > possible without at least looking at the stage #1 to recover the
> > base for folks who do not use diff3 style.
> 
> I feel strongly that LOCAL, REMOTE, and BASE should be left intact for
> this reason, Also because they aid readers in understanding the
> pre-conflicts versions of the file.
> 
> Rather mergetools (that support it) should be given the stage 1-3
> versions of the file in addition to the usual, unmodified, above three.
> Then each tool can decide whether or how to show each. Some graphical
> tools might be able to make effective use of all five (six?).

Except as you stated in your blog post, not a *single* tool does this
correctly using LOCAL, REMOTE, and BASE.

 * Araxis: a mess of changes
 * Beyond Compare: a mess of changes
 * DiffMerge: a mess of changes
 * kdiff3: a mess of changes
 * Meld: a mess of changes
 * Sublime Merge: displays unnecessary changes
 * SmartGit: ignores the other files
 * Fork: displays unnecessary changes
 * P4Merge: displays unnecessary changes
 * IntelliJ: a mess of changes
 * Tortoise Merge: uncertain
 * tkdiff: displays unnecessary changes
 * vimdiff: so, so wrong
 * vimdiff2: displays unnecessary changes
 * diffconflicts: RIGHT!

So all tools would benefit from the patch (except yours).

Which tool would be negatively affected?

Cheers.

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-17  5:19     ` Junio C Hamano
@ 2020-12-17  5:43       ` Felipe Contreras
  0 siblings, 0 replies; 70+ messages in thread
From: Felipe Contreras @ 2020-12-17  5:43 UTC (permalink / raw)
  To: Junio C Hamano, Felipe Contreras
  Cc: git, David Aguilar, Johannes Sixt, Seth House

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > The implementation details of the proposed patch are not relevant at
> > this point; it was just to show an example of what Seth's diffconflicts
> > vim plugin does.
> 
> Sorry for commenting on the "irrelevant" part; as the patch was
> marked with RFC label, I bothered to read it and tried to give
> comments, but it wasn't clear which part was welcoming comments
> and which parts were off limits ;-)

All parts welcomed comments.

I'm just saying the idea is what's important in my opinion.

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-17  5:41     ` Felipe Contreras
@ 2020-12-17  7:35       ` Johannes Sixt
  2020-12-17  8:27         ` Felipe Contreras
  2020-12-17  9:44         ` Seth House
  0 siblings, 2 replies; 70+ messages in thread
From: Johannes Sixt @ 2020-12-17  7:35 UTC (permalink / raw)
  To: Felipe Contreras, Seth House, Junio C Hamano; +Cc: git, David Aguilar

Am 17.12.20 um 06:41 schrieb Felipe Contreras:
> Seth House wrote:
>> I appreciate Felipe getting the discussion started.
>>
>> On Wed, Dec 16, 2020 at 02:24:23PM -0800, Junio C Hamano wrote:
>>> If there is none, then what is the benefit of doing the same thing
>>> without running 3 checkout-index?
>>
>> I wasn't aware of this plubming when I wrote the initial shell-script
>> version of the technique. This is a much better approach (even *if*
>> there's a negligible performance penalty). This nicely avoids
>> UNIX/Windows line-ending surprises, and instead leans on
>> already-configured Git defaults for those. Plus the non-text files
>> benefit you mentioned is also huge.
> 
> I think you misunderstood.
> 
> This command:
> 
>   git checkout-index --stage 2 --temp -- poem.txt
> 
> Will give you *exactly* the same output as LOCAL.
> 
> The context is "git mergetool", not the mergetool itself.
> 
>>> as I understand "mergetool" is handed an
>>> already conflicted state and asked to resolve it, it would not be
>>> possible without at least looking at the stage #1 to recover the
>>> base for folks who do not use diff3 style.
>>
>> I feel strongly that LOCAL, REMOTE, and BASE should be left intact for
>> this reason, Also because they aid readers in understanding the
>> pre-conflicts versions of the file.
>>
>> Rather mergetools (that support it) should be given the stage 1-3
>> versions of the file in addition to the usual, unmodified, above three.
>> Then each tool can decide whether or how to show each. Some graphical
>> tools might be able to make effective use of all five (six?).
> 
> Except as you stated in your blog post, not a *single* tool does this
> correctly using LOCAL, REMOTE, and BASE.
> 
>  * Araxis: a mess of changes
>  * Beyond Compare: a mess of changes
>  * DiffMerge: a mess of changes
>  * kdiff3: a mess of changes
>  * Meld: a mess of changes
>  * Sublime Merge: displays unnecessary changes
>  * SmartGit: ignores the other files
>  * Fork: displays unnecessary changes
>  * P4Merge: displays unnecessary changes
>  * IntelliJ: a mess of changes
>  * Tortoise Merge: uncertain
>  * tkdiff: displays unnecessary changes
>  * vimdiff: so, so wrong
>  * vimdiff2: displays unnecessary changes
>  * diffconflicts: RIGHT!
> 
> So all tools would benefit from the patch (except yours).
> 
> Which tool would be negatively affected?

Where's WinMerge in your list? I'm mostly using WinMerge these days, and
it can do what your patch does all by itself. It does not require the
proposed post-processing of stages in order to show only the real
conflicts. I would say that this is a hint that post-processing should
be moved to the tool drivers, and should not be done at the proposed level.

I don't know, though, whether your patch would have a negative effect
for WinMerge.

-- Hannes

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-17  7:35       ` Johannes Sixt
@ 2020-12-17  8:27         ` Felipe Contreras
  2020-12-17 19:23           ` Johannes Sixt
  2020-12-17  9:44         ` Seth House
  1 sibling, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-17  8:27 UTC (permalink / raw)
  To: Johannes Sixt, Felipe Contreras, Seth House, Junio C Hamano
  Cc: git, David Aguilar

Johannes Sixt wrote:
> Am 17.12.20 um 06:41 schrieb Felipe Contreras:
> > Seth House wrote:
> >> I appreciate Felipe getting the discussion started.
> >>
> >> On Wed, Dec 16, 2020 at 02:24:23PM -0800, Junio C Hamano wrote:
> >>> If there is none, then what is the benefit of doing the same thing
> >>> without running 3 checkout-index?
> >>
> >> I wasn't aware of this plubming when I wrote the initial shell-script
> >> version of the technique. This is a much better approach (even *if*
> >> there's a negligible performance penalty). This nicely avoids
> >> UNIX/Windows line-ending surprises, and instead leans on
> >> already-configured Git defaults for those. Plus the non-text files
> >> benefit you mentioned is also huge.
> > 
> > I think you misunderstood.
> > 
> > This command:
> > 
> >   git checkout-index --stage 2 --temp -- poem.txt
> > 
> > Will give you *exactly* the same output as LOCAL.
> > 
> > The context is "git mergetool", not the mergetool itself.
> > 
> >>> as I understand "mergetool" is handed an
> >>> already conflicted state and asked to resolve it, it would not be
> >>> possible without at least looking at the stage #1 to recover the
> >>> base for folks who do not use diff3 style.
> >>
> >> I feel strongly that LOCAL, REMOTE, and BASE should be left intact for
> >> this reason, Also because they aid readers in understanding the
> >> pre-conflicts versions of the file.
> >>
> >> Rather mergetools (that support it) should be given the stage 1-3
> >> versions of the file in addition to the usual, unmodified, above three.
> >> Then each tool can decide whether or how to show each. Some graphical
> >> tools might be able to make effective use of all five (six?).
> > 
> > Except as you stated in your blog post, not a *single* tool does this
> > correctly using LOCAL, REMOTE, and BASE.
> > 
> >  * Araxis: a mess of changes
> >  * Beyond Compare: a mess of changes
> >  * DiffMerge: a mess of changes
> >  * kdiff3: a mess of changes
> >  * Meld: a mess of changes
> >  * Sublime Merge: displays unnecessary changes
> >  * SmartGit: ignores the other files
> >  * Fork: displays unnecessary changes
> >  * P4Merge: displays unnecessary changes
> >  * IntelliJ: a mess of changes
> >  * Tortoise Merge: uncertain
> >  * tkdiff: displays unnecessary changes
> >  * vimdiff: so, so wrong
> >  * vimdiff2: displays unnecessary changes
> >  * diffconflicts: RIGHT!
> > 
> > So all tools would benefit from the patch (except yours).
> > 
> > Which tool would be negatively affected?
> 
> Where's WinMerge in your list?

It's not my list; it's Seth's list.

> I'm mostly using WinMerge these days, and it can do what your patch
> does all by itself.

Really? Because under Wine it doesn't look like it:

 1. Before: https://snipboard.io/8JA5Oz.jpg
 2. After: https://snipboard.io/HUXnOg.jpg

> I don't know, though, whether your patch would have a negative effect
> for WinMerge.

Seems like it has a *positive* effect. Like in all mergetools.

Cheers.

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-17  7:35       ` Johannes Sixt
  2020-12-17  8:27         ` Felipe Contreras
@ 2020-12-17  9:44         ` Seth House
  2020-12-17 10:35           ` Felipe Contreras
  1 sibling, 1 reply; 70+ messages in thread
From: Seth House @ 2020-12-17  9:44 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Felipe Contreras, Junio C Hamano, git, David Aguilar

On Thu, Dec 17, 2020 at 08:35:21AM +0100, Johannes Sixt wrote:
> Where's WinMerge in your list?

Added. Thanks for the suggestion; I wasn't familiar with it. Let me know
if I missed anything in my (quick) assessment. The "auto-merge" function
does indeed produce similar results to Felipe's patch.

> I would say that this is a hint that post-processing should
> be moved to the tool drivers, and should not be done at the proposed level.

That was my thought as well and it didn't occur to me to raise
a discussion here. However Felipe made the good point that adding this
functionality in upstream Git would have immediate downstream effects
for most tools. Almost every mergetool I've surveyed so far simply
blindly shows a diff betweeen LOCAL and REMOTE (and sometimes BASE and
sometimes MERGED) and Felipe's patch would have an immediate benefit for
all those tools.

There are a few notable exceptions that have their own, internal
conflict resolution logic -- tkdiff, WinMerge, and (I think) IntelliJ.
And only two tools make direct use of the conflict resolution that Git
already performed to produce MERGED -- Emacs+Magit, and diffconflicts.
Those tools would have to make a decision whether to opt-in to the
autoMerge flag or not.

I did not initially like Felipe's patch because I personally want my
mergetool to use all five of LOCAL, REMOTE, BASE, _and_ the two split
halves of MERGED. However the pragmatism of it is growing on me. Because
so many mergetools do simply diff LOCAL and REMOTE this one, simple
opt-in would positively benefit all of them.


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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-17  9:44         ` Seth House
@ 2020-12-17 10:35           ` Felipe Contreras
  2020-12-17 17:50             ` Seth House
  0 siblings, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-17 10:35 UTC (permalink / raw)
  To: Seth House, Johannes Sixt
  Cc: Felipe Contreras, Junio C Hamano, git, David Aguilar

Seth House wrote:
> On Thu, Dec 17, 2020 at 08:35:21AM +0100, Johannes Sixt wrote:
> > Where's WinMerge in your list?
> 
> Added. Thanks for the suggestion; I wasn't familiar with it. Let me know
> if I missed anything in my (quick) assessment. The "auto-merge" function
> does indeed produce similar results to Felipe's patch.
> 
> > I would say that this is a hint that post-processing should
> > be moved to the tool drivers, and should not be done at the proposed level.
> 
> That was my thought as well and it didn't occur to me to raise
> a discussion here. However Felipe made the good point that adding this
> functionality in upstream Git would have immediate downstream effects
> for most tools. Almost every mergetool I've surveyed so far simply
> blindly shows a diff betweeen LOCAL and REMOTE (and sometimes BASE and
> sometimes MERGED) and Felipe's patch would have an immediate benefit for
> all those tools.
> 
> There are a few notable exceptions that have their own, internal
> conflict resolution logic -- tkdiff, WinMerge, and (I think) IntelliJ.
> And only two tools make direct use of the conflict resolution that Git
> already performed to produce MERGED -- Emacs+Magit, and diffconflicts.
> Those tools would have to make a decision whether to opt-in to the
> autoMerge flag or not.

Each tool would have to be evaluated individually, but at least from
what I can see your tool--diffconflicts--shows exactly the same diff
with or without my patch, and the `mergetool.automerge` configuration.
It's only the DiffConflictsShowHistory command (which is secondary
functionality, that you didn't even mention in your blog) that gets
affected.

So, yes; *some* behavior is affected, but in my opinion not in a
negative way.

In my view the whole point of "git mergetool" is to resolve conflicts,
so if git can resolve conflicts *before* launching the mergetool, it
should (whether it's rerere or automerge).

> I did not initially like Felipe's patch because I personally want my
> mergetool to use all five of LOCAL, REMOTE, BASE, _and_ the two split
> halves of MERGED. However the pragmatism of it is growing on me. Because
> so many mergetools do simply diff LOCAL and REMOTE this one, simple
> opt-in would positively benefit all of them.

Yes. But other people may want to see the secondary functionality of
your DiffConflictsShowHistory command with simplified conflicts, and
those people can enable the option.

If you want to see the original LOCAL, REMOTE, and BASE, you can turn
off that option (or simply never turn it on).

With the option everyone can have what they want.

I for example can keep using `gvimdiff3`, except with simpler diff
chunks, thanks to your idea.

And for the record; "git mergetool" is one of of those things that you
setup once, and forget about it. My fingers have been trained for more
than a decade (maybe two) on exactly the same configuration, and I
initially didn't think you were onto something.

It's really good that you did setup a test repository with a merge
conflict that showcased the different situations one might encounter. It
immediately became clear to me that you were indeed onto something.

So I give you full credit for the idea, and explaining it succinctly,
that's in the commit message. I just found a way to shove it directly
into "git mergetool".

Cheers.

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-17 10:35           ` Felipe Contreras
@ 2020-12-17 17:50             ` Seth House
  2020-12-17 19:28               ` Junio C Hamano
  2020-12-18  2:05               ` Felipe Contreras
  0 siblings, 2 replies; 70+ messages in thread
From: Seth House @ 2020-12-17 17:50 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Johannes Sixt, Junio C Hamano, git, David Aguilar

On Thu, Dec 17, 2020 at 04:35:29AM -0600, Felipe Contreras wrote:
> If you want to see the original LOCAL, REMOTE, and BASE, you can turn
> off that option (or simply never turn it on).

Agreed.

Felipe, the functionality in the v3 version of your patch looks good to
me and I think it's well worth including in upstream Git.

Would you mind switching the autoMerge option to be per-tool rather than
under the main mergetool config section?

You're right that it will likely not cause any downstream errors; it's
just a difference in preference. The tools that perform their own
conflict resolution will likely want it off and most of the other tools
will likely want it on. I could envision wanting to configure multiple
mergetools -- some with and some without.

After work today I can go back through the list of mergetools reviewed
in my post and grab screenshots of each with this option enabled so that
everyone in this thread can quickly compare the before/after results.


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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-17  8:27         ` Felipe Contreras
@ 2020-12-17 19:23           ` Johannes Sixt
  2020-12-18  2:30             ` Felipe Contreras
  0 siblings, 1 reply; 70+ messages in thread
From: Johannes Sixt @ 2020-12-17 19:23 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, David Aguilar, Junio C Hamano, Seth House

Am 17.12.20 um 09:27 schrieb Felipe Contreras:
> Johannes Sixt wrote:
>> I'm mostly using WinMerge these days, and it can do what your patch
>> does all by itself.
> 
> Really? Because under Wine it doesn't look like it:
> 
>  1. Before: https://snipboard.io/8JA5Oz.jpg
>  2. After: https://snipboard.io/HUXnOg.jpg

These show 3 panes, while mine shows only 2. And I think I know why:

First, I seem to use an older version of WinMerge that does not support
3-way diffs.

Second, I only run the merge tool via Git GUI, and it has its own tool
drivers and completely bypasses git-mergetool. In particular, it invokes
WinMerge in a way that it auto-merges the non-conflicted parts, whereas
(the unpatched) git-mergetool does it differently such that it shows
many more differences in the same merge situation.

-- Hannes

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-17 17:50             ` Seth House
@ 2020-12-17 19:28               ` Junio C Hamano
  2020-12-18  2:34                 ` Felipe Contreras
  2020-12-18  2:05               ` Felipe Contreras
  1 sibling, 1 reply; 70+ messages in thread
From: Junio C Hamano @ 2020-12-17 19:28 UTC (permalink / raw)
  To: Seth House; +Cc: Felipe Contreras, Johannes Sixt, git, David Aguilar

Seth House <seth@eseth.com> writes:

> Would you mind switching the autoMerge option to be per-tool rather than
> under the main mergetool config section?
>
> You're right that it will likely not cause any downstream errors; it's
> just a difference in preference. The tools that perform their own
> conflict resolution will likely want it off and most of the other tools
> will likely want it on. I could envision wanting to configure multiple
> mergetools -- some with and some without.

Thanks for a concise summary.

Many people may stick to just a single tool and may find a single
mergetool.autoMerge knob convenient (and it is OK for them if the
new behaviour broke a tool they do not use), but for those who use
more than one, being able to configure them differently would be
valuable.  So I agree that making it a per-tool option would be a
good thing to do.  Extra bonus for "this covers all" fallback default
perhaps like so:

	if test "$(git config --get --bool "mergetool.$merge_tool.automerge" ||
	           git config --get --bool "mergetool.autoMerge" ||
	           echo true)" = true
	then
		... do the new "reduce conflicts" thing ...
	else
		... do the "grab the original" thing ...
	fi

There is another thing that the final version may want to consider.

Would there be a reason for a tool to actively want to refuse the
end-user request to use the new autoMerge behaviour?  A tool that
can merge binary files may want to make sure it gets the original
three blobs that got involved in the conflict, and the above
would allow users to break such a tool.  We could say "don't do
that then" to users, or we can use the same mechanism to set up
diff_cmd() etc. per tool [*1*].  A tool that always wants to use
the autoMerge without letting end-user to opt out can be handled
with the same mechanism, but I think that is less likely.

Also, just for completeness (this is a comment made after seeing v3),
unlike the previous rounds, we do not have to worry about conflict
marker length attribute user may have and rely on the default
marker-size hardcoded at the xdl_merge() layer.  If we wanted to
make it even safer, "git merge-file --diff3" invocation can also
use a hardcoded --marker-size=7 option to protect from changes in
the underlying xdl_merge()'s default.

Unfortunately, it however also means that we can be confused when we
are seeing a conflicted outer merge of a recursive merge.  In such a
case, the virtual ancestor version in stage #1 can have conflict
markers from the inner merge that also conflicted, which uses marker
size a bit longer than what the user gave via the attribute
machinery.  If that "a bit longer" length happens to be the same as
the hardcoded assumption of 7 the sed script in v3 makes, the sed
script would not be able to tell between the existing markers in the
virtual ancestor version and the markers "git merge-file --diff3"
creates.

It is unlikely that the end-user sets conflict marker length to
anything shorter than the default 7, so I think it is OK to punt
this potential problem, and assume we are OK.  The limitation may
need to be documented, though.

> After work today I can go back through the list of mergetools reviewed
> in my post and grab screenshots of each with this option enabled so that
> everyone in this thread can quickly compare the before/after results.

Thanks.


[Footnote]

*1* If we were to go that extra mile for completeness, the result
    would roughly look like the following outline:

 1. git-mergetool--lib.sh::setup_tool() would define a new helper
    function 

	automerge_enabled () {
		true
	}

    to be used as the fallback

 2. a tool that wants to use the original without allowing the user
    to opt into automerge would add

	automerge_enabled () {
		false
	}

    in mergetools/$toolname (where toolname is like 'tkdiff',
    'vimdiff', etc.)

 3. then the "consult mergetool.<tool>.automerge and then
    mergetool.automerge in this order" if statement we saw above
    would first ask the new helper function, i.e.

	if automerge_enabled &&
	   test "$(git config --get --bool "mergetool.$merge_tool.automerge" ||
                   git config --get --bool "mergetool.automerge" ||
	           echo true)" = true
	then
		...


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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-17 17:50             ` Seth House
  2020-12-17 19:28               ` Junio C Hamano
@ 2020-12-18  2:05               ` Felipe Contreras
  2020-12-18  2:35                 ` Seth House
  1 sibling, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-18  2:05 UTC (permalink / raw)
  To: Seth House, Felipe Contreras
  Cc: Johannes Sixt, Junio C Hamano, git, David Aguilar

Seth House wrote:
> On Thu, Dec 17, 2020 at 04:35:29AM -0600, Felipe Contreras wrote:
> > If you want to see the original LOCAL, REMOTE, and BASE, you can turn
> > off that option (or simply never turn it on).
> 
> Agreed.
> 
> Felipe, the functionality in the v3 version of your patch looks good to
> me and I think it's well worth including in upstream Git.
> 
> Would you mind switching the autoMerge option to be per-tool rather than
> under the main mergetool config section?

As I said; I don't see the point.

Even if I were using diffconflict, I would want this option on.

So it's not a tool configuration; it's a user preference.

> You're right that it will likely not cause any downstream errors; it's
> just a difference in preference. The tools that perform their own
> conflict resolution will likely want it off

What tools would that be?

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-17 19:23           ` Johannes Sixt
@ 2020-12-18  2:30             ` Felipe Contreras
  0 siblings, 0 replies; 70+ messages in thread
From: Felipe Contreras @ 2020-12-18  2:30 UTC (permalink / raw)
  To: Johannes Sixt, Felipe Contreras
  Cc: git, David Aguilar, Junio C Hamano, Seth House

Johannes Sixt wrote:
> Am 17.12.20 um 09:27 schrieb Felipe Contreras:
> > Johannes Sixt wrote:
> >> I'm mostly using WinMerge these days, and it can do what your patch
> >> does all by itself.
> > 
> > Really? Because under Wine it doesn't look like it:
> > 
> >  1. Before: https://snipboard.io/8JA5Oz.jpg
> >  2. After: https://snipboard.io/HUXnOg.jpg
> 
> These show 3 panes, while mine shows only 2. And I think I know why:
> 
> First, I seem to use an older version of WinMerge that does not support
> 3-way diffs.
> 
> Second, I only run the merge tool via Git GUI, and it has its own tool
> drivers and completely bypasses git-mergetool. In particular, it invokes
> WinMerge in a way that it auto-merges the non-conflicted parts, whereas
> (the unpatched) git-mergetool does it differently such that it shows
> many more differences in the same merge situation.

So it makes sense that the users of "git mergetool" get the same
benefits of the users of git-gui (with the proposed patch).

However, I'm trying to parse what git-gui is doing in
git-gui/lib/mergetool.tcl, and I see nothing similar to automerging.

Plus, the arguments passed to the tool are completely different:

  "$merge_tool_path" -e -ub -wl \
    -dl "Theirs File" -dr "Mine File" "$REMOTE" "$LOCAL" "$MERGED"]

	"$merge_tool_path" -u -e -dl Local -dr Remote \
		"$LOCAL" "$REMOTE" "$MERGED"

It's not even gramatically correct.

Cheers.

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-17 19:28               ` Junio C Hamano
@ 2020-12-18  2:34                 ` Felipe Contreras
       [not found]                   ` <CANiSa6jMXTyfo43bUdC8601BvYKiF67HXo+QaiTh_-8KWyBsLg@mail.gmail.com>
  0 siblings, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-18  2:34 UTC (permalink / raw)
  To: Junio C Hamano, Seth House
  Cc: Felipe Contreras, Johannes Sixt, git, David Aguilar

Junio C Hamano wrote:
> Seth House <seth@eseth.com> writes:
> 
> > Would you mind switching the autoMerge option to be per-tool rather than
> > under the main mergetool config section?
> >
> > You're right that it will likely not cause any downstream errors; it's
> > just a difference in preference. The tools that perform their own
> > conflict resolution will likely want it off and most of the other tools
> > will likely want it on. I could envision wanting to configure multiple
> > mergetools -- some with and some without.
> 
> Thanks for a concise summary.
> 
> Many people may stick to just a single tool and may find a single
> mergetool.autoMerge knob convenient (and it is OK for them if the
> new behaviour broke a tool they do not use), but for those who use
> more than one, being able to configure them differently would be
> valuable.

On what tool would you turn this on, and what tool would you turn this
off?

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-18  2:05               ` Felipe Contreras
@ 2020-12-18  2:35                 ` Seth House
  2020-12-18  2:49                   ` Felipe Contreras
  0 siblings, 1 reply; 70+ messages in thread
From: Seth House @ 2020-12-18  2:35 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Johannes Sixt, Junio C Hamano, git, David Aguilar

On Thu, Dec 17, 2020 at 08:05:33PM -0600, Felipe Contreras wrote:
> > Would you mind switching the autoMerge option to be per-tool rather than
> > under the main mergetool config section?
> 
> As I said; I don't see the point.

I know you've said that. But I would prefer that, and Junio requested
that, and other people may prefer that as well. Since it's not too much
trouble to make the option per-tool with a global fallback, and since
Junio already provided example code to do that, do you mind adding it
anyway even though you don't see the point?


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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-18  2:35                 ` Seth House
@ 2020-12-18  2:49                   ` Felipe Contreras
  2020-12-18  5:49                     ` Seth House
  0 siblings, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-18  2:49 UTC (permalink / raw)
  To: Seth House, Felipe Contreras
  Cc: Johannes Sixt, Junio C Hamano, git, David Aguilar

Seth House wrote:
> On Thu, Dec 17, 2020 at 08:05:33PM -0600, Felipe Contreras wrote:
> > > Would you mind switching the autoMerge option to be per-tool rather than
> > > under the main mergetool config section?
> > 
> > As I said; I don't see the point.
> 
> I know you've said that. But I would prefer that, and Junio requested
> that, and other people may prefer that as well. Since it's not too much
> trouble to make the option per-tool with a global fallback, and since
> Junio already provided example code to do that, do you mind adding it
> anyway even though you don't see the point?

It's not about the level of effort; it's about doing what makes sense.

And no person is the sole arbiter of truth, in this list, or anywhere.
People have managed to change Junio's mind.

You said you would provide a list of tools that would be affected. Do you
have the list of tools who would want this flag off regardless of user
preference?

Cheers.

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-18  2:49                   ` Felipe Contreras
@ 2020-12-18  5:49                     ` Seth House
  2020-12-18  9:46                       ` Felipe Contreras
  2020-12-18 10:04                       ` [RFC/PATCH] mergetool: use resolved conflicts in all the views Junio C Hamano
  0 siblings, 2 replies; 70+ messages in thread
From: Seth House @ 2020-12-18  5:49 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Johannes Sixt, Junio C Hamano, git, David Aguilar

On Thu, Dec 17, 2020 at 08:49:13PM -0600, Felipe Contreras wrote:
> And no person is the sole arbiter of truth, in this list, or anywhere.
> People have managed to change Junio's mind.

I'm not worried about Junio but I am wondering if anyone has managed to
change your mind. You and I have been going back and forth on this list
and on Reddit for two solid days and, to be frank, I'm running out of
steam.

> You said you would provide a list of tools that would be affected.

No, I said I would provide a list of before/after screenshots so people
in this thread that haven't been following the entire discussion can
more easily see the differences and benefits of this change.

I can't speak for the preferences of all the mergetool authors out there
but I will try to convince you of the merit of adding a configuration
flag in two places instead of just one. This took me a long time to
write; I would appreciate it if you read it slowly and carefully.

There are three broad classifications of mergetools that I've seen so
far. I've only collected notes for seventeen of them but there are many,
many others. I mentioned those classifications earlier in this thread
but to repeat them here:

1.  Tools that ignore conflict resolution and simply diff LOCAL and
    REMOTE. This accounts for most of them.
2.  Tools that perform their own, internal conflict resolution. Examples
    are tkdiff, WinMerge, IntelliJ.
3.  Tools that reuse Git's conflict resolution by splitting MERGED.
    Examples are Emacs + Magit, vim-mergetool, diffconflicts.

This patch *will* (probably) always benefit the first group. Because
this patch *overwrites* LOCAL and REMOTE those tools will see immediate
benefit without having to make any changes at all.

This patch *may* benefit the second group, or it may not affect them at
all, or they may want it off. If they keep it on they will run conflict
resolution against files that have already undergone that process in Git
and likely turn up no additional resolutions since Git is quite good at
doing that. It is also possible that some of them may do a better job
than Git, or that some of them may want Git to tackle some of the
conflict resolution (things that require access to the file history such
as renames or recursive parents), but also want to perform even more
sophisticated conflict resolution on top of that. Some of those tools
are very clever and innovation in this space is still happening.

This patch *may* benefit the third group or it may make them no longer
necessary. Speaking as the author of one of them: I see that as a good
thing.

This patch *may* prevent tools in the second and third groups from
having all the information they desire to show users without also
running additional Git commands. Because this patch *overwrites* LOCAL
and REMOTE any tool that wishes to show the state of the file before the
merge was started will need to use Git to generate a new copy of those
files. Although this is a negative I think benefiting the large number
of tools in the first group outweighs this negative. Because the second
and third groups of tools are more sophisticated, I think the authors of
those tools are better suited to navigating these pros and cons.

In closing, below are several use-cases for having both a tool-level
flag and a global flag:

I, as the author of diffconflicts, *will* want to disable this flag for
diffconflicts because that tool already does what this flag does *and*
because that tool makes use of the original versions of LOCAL and
REMOTE which this patch destroys.

I, as a user of diffconflicts, *may* want to disable this flag because
in practice I don't actually reference LOCAL and REMOTE very often.
Other users may reference LOCAL and REMOTE every time. You yourself
said, "Even if I were using diffconflict, I would want this option on."
Other users may choose differently than you because you are also not the
"sole arbiter of truth" and because this is entirely about personal
preference and is decidedly not about objective truth. If this patch
always benefited everyone then we would make it default and not put it
behind an opt-in flag.

I, as a user of diffconflicts, may want to both disable this flag for
diffconflicts but enable this flag for VS Code and kdiff3. It is not
unusual for people to use more than one mergetool. Some of them are
better or worse at visualizing different kinds of conflicts. Sometimes
a conflict is small and straightforward; othertimes a conflict is
complicated and requires deep knowledge of the history of both branches.
If we force this to be a global flag only then users will not be able to
make different choices for different tools.

Someone who does use multiple mergetools but only uses tools from group
one may appreciate a single global flag so s/he doesn't need to set it
for each tool.

Someone who is actively developing a new mergetool may want to enable or
disable this flag for that tool while still keeping the flag enabled or
disabled for tools that s/he uses for work or school. Someone who is
comparing and contrasting multiple mergetools for work or school may
want to selectively enable or disable the flag for one tool or another
in order to do the comparison. Someone may want the same mergetool
configured twice, once with this flag and once without, in order to
contrast the difference.

An end-user may want to use a mergetool differently than the author
intended. You said, "This should be a setting in the tool driver, not
a user-visible setting," and then you said, "Even if I were using
diffconflict, I would want this option on." But I said, "I, as the
author of diffconflicts, *will* want to disable this flag." and there's
no reason we can't have both. I can't presume what preference other
mergetool authors will have. We can make some educated guesses and even
default the option to true for some mergetool wrappers that ship with
Git but there's every possibility that a user will prefer it a different
way or that a mergetool author will. And there's every possibility that
there will be differing opinions between users and authors like there is
between you and me. But that's ok! Because it's just a configuration
option.


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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-18  5:49                     ` Seth House
@ 2020-12-18  9:46                       ` Felipe Contreras
  2020-12-19  0:13                         ` Seth House
  2020-12-18 10:04                       ` [RFC/PATCH] mergetool: use resolved conflicts in all the views Junio C Hamano
  1 sibling, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-18  9:46 UTC (permalink / raw)
  To: Seth House, Felipe Contreras
  Cc: Johannes Sixt, Junio C Hamano, git, David Aguilar

Seth House wrote:
> On Thu, Dec 17, 2020 at 08:49:13PM -0600, Felipe Contreras wrote:
> > And no person is the sole arbiter of truth, in this list, or anywhere.
> > People have managed to change Junio's mind.
> 
> I'm not worried about Junio but I am wondering if anyone has managed to
> change your mind.

Yes, with evidence and reasoning.

> You and I have been going back and forth on this list
> and on Reddit for two solid days and, to be frank, I'm running out of
> steam.

That doesn't mean you are right.

> > You said you would provide a list of tools that would be affected.
> 
> No, I said I would provide a list of before/after screenshots so people
> in this thread that haven't been following the entire discussion can
> more easily see the differences and benefits of this change.

We can run the simulation in our mind; if uncontested chunks are the
same in all files, then *all* tools would present them with no diff.

No tool would present a diff in the second paragraph. We don't need to
see the screenshots to know that.

I don't see how that effort advances this discussion, at this point
nobody is against this change. The discussion now is about what kind of
configuration it should be: user or tool.

> I can't speak for the preferences of all the mergetool authors out there
> but I will try to convince you of the merit of adding a configuration
> flag in two places instead of just one.

The preferences of mergetool authors do not matter, what matters is how
the users of those tools use them.

We don't need to hear from the author of WinMerge, we need to hear from
Johannes, who uses WinMerge.

Authors are fallible--like everyone else--and they can be wrong about
their own inventions. Play-Doh for example was invented to remove
wallpapers.

It's perfectly fine for a user to use a tool in ways the author did not
intend.

> This took me a long time to write; I would appreciate it if you read
> it slowly and carefully.

Sure.

> There are three broad classifications of mergetools that I've seen so
> far. I've only collected notes for seventeen of them but there are many,
> many others. I mentioned those classifications earlier in this thread
> but to repeat them here:
> 
> 1.  Tools that ignore conflict resolution and simply diff LOCAL and
>     REMOTE. This accounts for most of them.

This is not true.

I already told you this, but *all* tools have MERGED in one of their
panes.

If a tool doesn't have MERGED, then it can't work.

> 2.  Tools that perform their own, internal conflict resolution. Examples
>     are tkdiff, WinMerge, IntelliJ.

OK. I don't see how that is relevant.

> 3.  Tools that reuse Git's conflict resolution by splitting MERGED.
>     Examples are Emacs + Magit, vim-mergetool, diffconflicts.

Interesting. I know 2/3 of the people that are the original authors of
these tools; I know Marius Vollmer (the original author of magit).

However, I don't think magit is a mergetool; you can't do "git mergetool
-t magit".

> This patch *will* (probably) always benefit the first group. Because
> this patch *overwrites* LOCAL and REMOTE those tools will see immediate
> benefit without having to make any changes at all.

Yes.

> This patch *may* benefit the second group, or it may not affect them at
> all, or they may want it off.

Why might they want it off? Let's read...

> If they keep it on they will run conflict resolution against files
> that have already undergone that process in Git and likely turn up no
> additional resolutions since Git is quite good at doing that. It is
> also possible that some of them may do a better job than Git, or that
> some of them may want Git to tackle some of the conflict resolution
> (things that require access to the file history such as renames or
> recursive parents), but also want to perform even more sophisticated
> conflict resolution on top of that. Some of those tools are very
> clever and innovation in this space is still happening.

You did not explain why they would want it off.

In your mergetool rant [2] you wrote this point:

 4. All conflicts in the second stanza were automatically resolved. They
    should not be shown to the user.

So it doesn't matter if git does it, or the mergetool does it, you said
the conflicts in the second paragraph can be solved, so they should not
be shown to the user.

If git does it, that's one less thing the mergetool has to do.

Even if the mergetool is smarter than git, in fact, so smart it's run by
a superintelligent AI; it still would resolve the conflict in precisely
the same way as git, and it still should not present that conflict to
the user.

So the end result is the same. Correct?

> This patch *may* benefit the third group or it may make them no longer
> necessary. Speaking as the author of one of them: I see that as a good
> thing.

Yes, at least the core of what they do.

> This patch *may* prevent tools in the second and third groups from
> having all the information they desire to show users without also
> running additional Git commands.

Explain how.

-------------------------------------------------
cat > BASE <<EOF
= tite =

sentence
EOF

cat > LOCAL <<EOF
= title =

sentence
EOF

cat > REMOTE <<EOF
= tite =

sentence.
EOF

git merge-file "$@" --diff3 -p LOCAL BASE REMOTE
-------------------------------------------------

In this example the conflicts are so trivial that git doesn't even
bother to stop the merge; it doesn't even think there's a need to launch
the mergetool.

If instead in REMOTE you have this:

-------------------------------------------------
cat > REMOTE <<EOF
= temporary tite =

sentence.
EOF
-------------------------------------------------

Now there's a *true* conflict, and git requires user-intervention to fix
it, it has placed markers where the user should focus her attention, and
that's where the mergetool should focus the user attention too.

It's very clear where the *true* conflict is:

-------------------------------------------------
<<<<<<< LOCAL
= title =
||||||| BASE
= tite =
=======
= temporary tite =
>>>>>>> REMOTE

sentence.
-------------------------------------------------

If there was no conflict in the first paragraph git wouldn't have
bothered the user with the second paragraph.

Why should the mergetool?

> Because this patch *overwrites* LOCAL and REMOTE any tool that wishes
> to show the state of the file before the merge was started will need
> to use Git to generate a new copy of those files.

Why would they want to do that? Git already has determined there's no
valuable information there.

> Although this is a negative I think benefiting the large number of
> tools in the first group outweighs this negative. Because the second
> and third groups of tools are more sophisticated, I think the authors
> of those tools are better suited to navigating these pros and cons.

I'm sorry, but you still haven't determined that it's a negative, and
why.

> In closing, below are several use-cases for having both a tool-level
> flag and a global flag:
> 
> I, as the author of diffconflicts, *will* want to disable this flag for
> diffconflicts because that tool already does what this flag does *and*
> because that tool makes use of the original versions of LOCAL and
> REMOTE which this patch destroys.

Yes, but the author of diffconflicts is not infallible.

Explain why the *users* of the diffconclits tool would be affected
negatively by not being shown a conflict that git resolved
automatically, and if it were not for other *real* conflicts in the file
wouldn't have bothered to stop the merge.

Furthermore, explain why a user in vimdiff2 wouldn't want to see the
trivial conflict, but a user in diffconflicts would.

> I, as a user of diffconflicts, *may* want to disable this flag because
> in practice I don't actually reference LOCAL and REMOTE very often.

It doesn't change your experience at all.

> Other users may reference LOCAL and REMOTE every time.

Yes, and they would prefer the trivial conflicts that git already
resolved *not there*.

> You yourself said, "Even if I were using diffconflict, I would want
> this option on." Other users may choose differently than you because
> you are also not the "sole arbiter of truth" and because this is
> entirely about personal preference and is decidedly not about
> objective truth.

And that's why it is an *OPTION*.

Didn't I introduce an option for those other users?

> If this patch always benefited everyone then we would make it default
> and not put it behind an opt-in flag.

That's not how git works.

10 years ago doing "git merge" (without arguments) used to fail, then
after I suggested a sensible default, the merge.defaultToUpstream option
was introduced, and turned off by default, even though it was clear 100%
of users would want this.

3 years after the introduction of that flag it was turned on, and
literally not one person complained.

Today you can't find a single person that has
"merge.defaulttoupstream=false" in their configuration, and yet, the
option is still there.

In git even if 100% of users we know would benefit, as long as there
exists one hypothetical user that might not, the option is there.

> I, as a user of diffconflicts, may want to both disable this flag for
> diffconflicts but enable this flag for VS Code and kdiff3.

Why? You still have not explained.

> It is not unusual for people to use more than one mergetool. Some of
> them are better or worse at visualizing different kinds of conflicts.
> Sometimes a conflict is small and straightforward; othertimes a
> conflict is complicated and requires deep knowledge of the history of
> both branches.  If we force this to be a global flag only then users
> will not be able to make different choices for different tools.

They would still not be able to make different choices.

If you read Junio's proposal, if kdiff3 returns false in
automerge_enabled, then the user can *not* turn this on.

> Someone who does use multiple mergetools but only uses tools from group
> one may appreciate a single global flag so s/he doesn't need to set it
> for each tool.

You still have not explained why a user might want different
configuration per tool.

> Someone who is actively developing a new mergetool may want to enable or
> disable this flag for that tool while still keeping the flag enabled or
> disabled for tools that s/he uses for work or school. Someone who is
> comparing and contrasting multiple mergetools for work or school may
> want to selectively enable or disable the flag for one tool or another
> in order to do the comparison. Someone may want the same mergetool
> configured twice, once with this flag and once without, in order to
> contrast the difference.

Yes, I've been doing precisely that without problems.

> An end-user may want to use a mergetool differently than the author
> intended. You said, "This should be a setting in the tool driver, not
> a user-visible setting,"

That was Johannes Sixt, not me.

> and then you said, "Even if I were using diffconflict, I would want
> this option on." But I said, "I, as the author of diffconflicts,
> *will* want to disable this flag." and there's no reason we can't have
> both.

In Junio's proposal if the author of diffconflicts disables the flag, the
user cannot activate it.

But even if the user could, there's no reason not to do X is not a valid
argument in favor of doing X.

You still need a valid reason to have both.

> I can't presume what preference other mergetool authors will have.

As I said; this is not relevant.

Why would a *user* of diffconflict want this off?

You still haven't given a reason.

> We can make some educated guesses and even default the option to true
> for some mergetool wrappers that ship with Git but there's every
> possibility that a user will prefer it a different way or that a
> mergetool author will. And there's every possibility that there will
> be differing opinions between users and authors like there is between
> you and me. But that's ok! Because it's just a configuration option.

At the end of the day it's not opinions that matter, it's facts.


You asked me to read slow and carefully a long explanation, which I did.
Now allow me to ask something of you. Please answer the following question.

Do you have any concrete evidence of a single instance where the user
will be negatively affected by turning on this flag in one mergetool,
but not so in another mergetool?

Cheers.

[1] https://www.reddit.com/r/kde/comments/7y6eds/kompare_is_a_great_difftool_why_doesnt_it_work_as/
[2] https://github.com/whiteinge/diffconflicts/blob/master/_utils/README.md

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-18  5:49                     ` Seth House
  2020-12-18  9:46                       ` Felipe Contreras
@ 2020-12-18 10:04                       ` Junio C Hamano
  2020-12-18 11:58                         ` Felipe Contreras
  2020-12-19  0:18                         ` Seth House
  1 sibling, 2 replies; 70+ messages in thread
From: Junio C Hamano @ 2020-12-18 10:04 UTC (permalink / raw)
  To: Seth House; +Cc: Felipe Contreras, Johannes Sixt, git, David Aguilar

Seth House <seth@eseth.com> writes:

> On Thu, Dec 17, 2020 at 08:49:13PM -0600, Felipe Contreras wrote:
>> And no person is the sole arbiter of truth, in this list, or anywhere.
>> People have managed to change Junio's mind.
>
> I'm not worried about Junio but I am wondering if anyone has managed to
> change your mind. You and I have been going back and forth on this list
> and on Reddit for two solid days and, to be frank, I'm running out of
> steam.
> ...
> I, as a user of diffconflicts, may want to both disable this flag for
> diffconflicts but enable this flag for VS Code and kdiff3. It is not
> unusual for people to use more than one mergetool. Some of them are
> better or worse at visualizing different kinds of conflicts. Sometimes
> a conflict is small and straightforward; othertimes a conflict is
> complicated and requires deep knowledge of the history of both branches.
> If we force this to be a global flag only then users will not be able to
> make different choices for different tools.
>
> Someone who does use multiple mergetools but only uses tools from group
> one may appreciate a single global flag so s/he doesn't need to set it
> for each tool.
> ... there's every possibility that a user will prefer it a different
> way or that a mergetool author will. And there's every possibility that
> there will be differing opinions between users and authors like there is
> between you and me. But that's ok! Because it's just a configuration
> option.

Well explained.  I do not think I need to add much.

It makes sense to at least allow people to enable/disable the
behaviour independently for different tools.  When unconfigured, I
would say we should enable the feature by default to give it wider
exposure.

Because what I care is not about the set of tools we happen to have
right now, but is about leaving users access to an escape hatch in
case things go wrong.  If it turns out that all the tools we happen
to have do not seem to break with this new option with just a few
days' survey, it does not mean we do not need a per-tool escape
hatch they can use until the next release either fixes the feature
or makes it disabled by default, when there is unexpected breakages
discovered later.  The time between a release and the next is a long
time that the users cannot keep the tool they have learned to rely
on broken.  And that's a conservative maintainer's view.

When a tool that never wants its input munged appears, we might
further want to have the mechanism to give different default per
each tool for users who have no configuration, so that such a tool
can be disabled while other tools are enabled by default while
allowing users to choose.  But such a code to set different
enabled/disabled default per tool (the one I outlined in the
footnote of the other message) won't be exercised in practice with
the set of tools we have (hence a bug in such a code would go
unnoticed for a long time), so I tend to think we might be better
off to wait until the need arises before implementing per-tool
fallback default for users who do not configure at all.

Another reason why allowing users to disable the feature per tool is
important is because as far as I know we have kept the mergetool
framework to allow adding a tool that can merge binary data, and
leaving these three files pristine was one ingredient for that.
With only a single knob, we would be making a decision to declare
that such a tool is unwelcome, which is not quite acceptable.  I
expect that users would want the new feature most of the time
because they would be managing text files more of the time, and
having only a single knob would force an unnecessary choice on those
who want to use such a binary-capable tool as well.

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-18 10:04                       ` [RFC/PATCH] mergetool: use resolved conflicts in all the views Junio C Hamano
@ 2020-12-18 11:58                         ` Felipe Contreras
  2020-12-19 18:52                           ` Junio C Hamano
  2020-12-19  0:18                         ` Seth House
  1 sibling, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-18 11:58 UTC (permalink / raw)
  To: Junio C Hamano, Seth House
  Cc: Felipe Contreras, Johannes Sixt, git, David Aguilar

Junio C Hamano wrote:
> Seth House <seth@eseth.com> writes:
> 
> > On Thu, Dec 17, 2020 at 08:49:13PM -0600, Felipe Contreras wrote:
> >> And no person is the sole arbiter of truth, in this list, or anywhere.
> >> People have managed to change Junio's mind.
> >
> > I'm not worried about Junio but I am wondering if anyone has managed to
> > change your mind. You and I have been going back and forth on this list
> > and on Reddit for two solid days and, to be frank, I'm running out of
> > steam.
> > ...
> > I, as a user of diffconflicts, may want to both disable this flag for
> > diffconflicts but enable this flag for VS Code and kdiff3. It is not
> > unusual for people to use more than one mergetool. Some of them are
> > better or worse at visualizing different kinds of conflicts. Sometimes
> > a conflict is small and straightforward; othertimes a conflict is
> > complicated and requires deep knowledge of the history of both branches.
> > If we force this to be a global flag only then users will not be able to
> > make different choices for different tools.
> >
> > Someone who does use multiple mergetools but only uses tools from group
> > one may appreciate a single global flag so s/he doesn't need to set it
> > for each tool.
> > ... there's every possibility that a user will prefer it a different
> > way or that a mergetool author will. And there's every possibility that
> > there will be differing opinions between users and authors like there is
> > between you and me. But that's ok! Because it's just a configuration
> > option.
> 
> Well explained.  I do not think I need to add much.
> 
> It makes sense to at least allow people to enable/disable the
> behaviour independently for different tools.  When unconfigured, I
> would say we should enable the feature by default to give it wider
> exposure.
> 
> Because what I care is not about the set of tools we happen to have
> right now, but is about leaving users access to an escape hatch in
> case things go wrong.

They do have an escape hatch.

> If it turns out that all the tools we happen to have do not seem to
> break with this new option with just a few days' survey,

Seth's blog post was ten days ago, and I presume it took him more than
one day to write, so I'm not sure it's a "few days' survey".

> it does not mean we do not need a per-tool escape
> hatch they can use until the next release either fixes the feature
> or makes it disabled by default, when there is unexpected breakages
> discovered later.

They don't need to wait for the next release, they can just disable the
flag.

These hypothetical users of multiple mergetools that are using a
hypothetical mergetool that doesn't seem to exist yet can just disable
the flag globally.

These users have managed to survive many years without this feature
enabled in all the usual mergetools, I think they would survive until
the next release.

> The time between a release and the next is a long
> time that the users cannot keep the tool they have learned to rely
> on broken.  And that's a conservative maintainer's view.

They don't have to; they can just disable the flag.

> When a tool that never wants its input munged appears, we might
> further want to have the mechanism to give different default per
> each tool for users who have no configuration, so that such a tool
> can be disabled while other tools are enabled by default while
> allowing users to choose.  But such a code to set different
> enabled/disabled default per tool (the one I outlined in the
> footnote of the other message) won't be exercised in practice with
> the set of tools we have (hence a bug in such a code would go
> unnoticed for a long time), so I tend to think we might be better
> off to wait until the need arises before implementing per-tool
> fallback default for users who do not configure at all.

Agreed.

> Another reason why allowing users to disable the feature per tool is
> important is because as far as I know we have kept the mergetool
> framework to allow adding a tool that can merge binary data, and
> leaving these three files pristine was one ingredient for that.
> With only a single knob, we would be making a decision to declare
> that such a tool is unwelcome, which is not quite acceptable.  I
> expect that users would want the new feature most of the time
> because they would be managing text files more of the time, and
> having only a single knob would force an unnecessary choice on those
> who want to use such a binary-capable tool as well.

I can't imagine what that binary data could look like, and how any tool
could represent that to the user.

But either way "git merge-file" fails on those cases, so we can just
check if the file is empty, and bail out.

I've implemented that in the next version.

Cheers.

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-18  9:46                       ` Felipe Contreras
@ 2020-12-19  0:13                         ` Seth House
  2020-12-19  0:53                           ` Felipe Contreras
  2020-12-19 11:14                           ` Junio C Hamano
  0 siblings, 2 replies; 70+ messages in thread
From: Seth House @ 2020-12-19  0:13 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Johannes Sixt, Junio C Hamano, git, David Aguilar

On Fri, Dec 18, 2020 at 03:46:37AM -0600, Felipe Contreras wrote:
> Yes, with evidence and reasoning.

[Citation needed]

> Yes, but the author of diffconflicts is not infallible.
> 
> Explain why the *users* of the diffconclits tool would be affected
> negatively

You've got that right -- I'm definitely not infallible. My point isn't
that I'm right; my point is that is my *preference*. Other mergetool
authors may have different preferences.

I think where we're not seeing eye-to-eye is that you're focusing on
potential "negative" consequences whereas I'm talking about having more
information about the merge rather than less.

There is very likely no negative consequences for most, if not all,
mergetools. I wrote the initial version of diffconflicts ten years ago
and I've been using it nearly every day since. I'm fairly confident in
the end result. What is a fact is there is undisputedly less information
about the merge if we overwrite LOCAL and REMOTE; as I've written,
I think the tradeoff is worthwhile for most tools but a per-tool
configuration will allow people that feel differently to choose
differently.

This is where I will part this particular debate. If you are not arguing
for the sake of arguing and if you are genuinely willing to have your
mind changed then I invite you to reread my blog post, rewatch my
YouTube video, and reread parts this thread -- watch out for where
I talk about why LOCAL and REMOTE (and BASE) are useful.

Cheers.


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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-18 10:04                       ` [RFC/PATCH] mergetool: use resolved conflicts in all the views Junio C Hamano
  2020-12-18 11:58                         ` Felipe Contreras
@ 2020-12-19  0:18                         ` Seth House
  1 sibling, 0 replies; 70+ messages in thread
From: Seth House @ 2020-12-19  0:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Felipe Contreras, Johannes Sixt, git, David Aguilar

On Fri, Dec 18, 2020 at 02:04:23AM -0800, Junio C Hamano wrote:
> When unconfigured, I
> would say we should enable the feature by default to give it wider
> exposure.

I am unreasonably excited about that. :D

> If it turns out that all the tools we happen
> to have do not seem to break with this new option with just a few
> days' survey, it does not mean we do not need a per-tool escape
> hatch they can use until the next release either fixes the feature
> or makes it disabled by default, when there is unexpected breakages
> discovered later.

Well said. I will try to complete that screenshot comparison part of the
survey that I promised by tonight, or by this weekend at the latest.


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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-19  0:13                         ` Seth House
@ 2020-12-19  0:53                           ` Felipe Contreras
  2020-12-19 11:14                           ` Junio C Hamano
  1 sibling, 0 replies; 70+ messages in thread
From: Felipe Contreras @ 2020-12-19  0:53 UTC (permalink / raw)
  To: Seth House, Felipe Contreras
  Cc: Johannes Sixt, Junio C Hamano, git, David Aguilar

Seth House wrote:
> On Fri, Dec 18, 2020 at 03:46:37AM -0600, Felipe Contreras wrote:
> > Yes, but the author of diffconflicts is not infallible.
> > 
> > Explain why the *users* of the diffconclits tool would be affected
> > negatively
> 
> You've got that right -- I'm definitely not infallible. My point isn't
> that I'm right; my point is that is my *preference*. Other mergetool
> authors may have different preferences.
> 
> I think where we're not seeing eye-to-eye is that you're focusing on
> potential "negative" consequences whereas I'm talking about having more
> information about the merge rather than less.

Yes, but it's not due to some unreasonable hankering; it comes from a
deep philosophical reason, which is Karl Popper's falsifiability principle
[1] that solves both the problems of induction and demarcation.

To put it plainly; if we want to know if all swans are white, where you claim
the negative, and I the positive; it's much easier for you to prove the
negative. All you need is *one* black swan.

Analogously in our case; all you need is *one* negative consequence to
prove your point, while me providing one hundred success cases does not
prove my point.

> There is very likely no negative consequences for most, if not all,
> mergetools.

Again: do you have *one* negative consequence that is present in tool a,
but not in tool b?

You say there is "very likely no negative consequences", but do you have
evidence of *any* negative consequence?

> I wrote the initial version of diffconflicts ten years ago and I've
> been using it nearly every day since. I'm fairly confident in the end
> result. What is a fact is there is undisputedly less information about
> the merge if we overwrite LOCAL and REMOTE;

But it's objectively not useful information.

Edit your make-conflicts.sh script, and remove the first paragraph from
poem.txt.

What happens when you run "git merge"?

Does it not complete the merge without *any* user interaction?

Doesn't that mean that git considers the changes in the second paragraph
to be non-conflicts?

> This is where I will part this particular debate.

All right.

I'm still waiting for anyone to provide *one* example of a negative
consequence.

Cheers.

[1] https://en.wikipedia.org/wiki/Falsifiability

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-19  0:13                         ` Seth House
  2020-12-19  0:53                           ` Felipe Contreras
@ 2020-12-19 11:14                           ` Junio C Hamano
  2020-12-19 12:08                             ` Felipe Contreras
  2020-12-21  4:25                             ` Seth House
  1 sibling, 2 replies; 70+ messages in thread
From: Junio C Hamano @ 2020-12-19 11:14 UTC (permalink / raw)
  To: Seth House; +Cc: Felipe Contreras, Johannes Sixt, git, David Aguilar

Seth House <seth@eseth.com> writes:

> I think where we're not seeing eye-to-eye is that you're focusing on
> potential "negative" consequences whereas I'm talking about having more
> information about the merge rather than less.
>
> There is very likely no negative consequences for most, if not all,
> mergetools. I wrote the initial version of diffconflicts ten years ago
> and I've been using it nearly every day since. I'm fairly confident in
> the end result. What is a fact is there is undisputedly less information
> about the merge if we overwrite LOCAL and REMOTE; as I've written,
> I think the tradeoff is worthwhile for most tools but a per-tool
> configuration will allow people that feel differently to choose
> differently.

Thanks for stressing this point.

When a user or developer asks for a reasonable feature (e.g.
configurability to suit personal taste), especially when there is no
obvious downside for adding it, the burden of proof is on the party
who refuses to add it---they are the ones who have to adequately
explain why adding it is actively harmful, not just the proposed
addition is not necessary [*1*].

There is no need for any "evidence" of "negative consequence" at all
to ask for a way to selectively enable or disable a new feature.  A
new feature tends to trigger unexpected bugs in unseen corner cases
more than an older feature, even without any concrete numbers, and
that is good enough reason to insist an escape hatch that is easy to
access by users to exist.

This is especially true for a software with wide userbase, most of
which do not run the bleeding edge version.  That is how we get
users unstuck after releasing our software with a new feature with
unforeseen consequences, before we can deliver fixed version in
their hands.

> This is where I will part this particular debate. If you are not arguing
> for the sake of arguing and if you are genuinely willing to have your
> mind changed then I invite you to reread my blog post, rewatch my
> YouTube video, and reread parts this thread -- watch out for where
> I talk about why LOCAL and REMOTE (and BASE) are useful.

Thanks for your contribution so far.


[Footnote]

*1* They are, however, not obligated to add the feature themselves.
They can just honestly say "I did not understand the help offered to
make use of it", or "I am too busy, so I am not doing it", or give
any other reason or excuse for not doing so.  And the rest of us can
take it from there by building on top.

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-19 11:14                           ` Junio C Hamano
@ 2020-12-19 12:08                             ` Felipe Contreras
  2020-12-19 18:26                               ` Junio C Hamano
  2020-12-21  4:25                             ` Seth House
  1 sibling, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-19 12:08 UTC (permalink / raw)
  To: Junio C Hamano, Seth House
  Cc: Felipe Contreras, Johannes Sixt, git, David Aguilar

Junio C Hamano wrote:
> Seth House <seth@eseth.com> writes:
> 
> > I think where we're not seeing eye-to-eye is that you're focusing on
> > potential "negative" consequences whereas I'm talking about having more
> > information about the merge rather than less.
> >
> > There is very likely no negative consequences for most, if not all,
> > mergetools. I wrote the initial version of diffconflicts ten years ago
> > and I've been using it nearly every day since. I'm fairly confident in
> > the end result. What is a fact is there is undisputedly less information
> > about the merge if we overwrite LOCAL and REMOTE; as I've written,
> > I think the tradeoff is worthwhile for most tools but a per-tool
> > configuration will allow people that feel differently to choose
> > differently.
> 
> Thanks for stressing this point.
> 
> When a user or developer asks for a reasonable feature (e.g.
> configurability to suit personal taste), especially when there is no
> obvious downside for adding it, the burden of proof is on the party
> who refuses to add it

Sorry, but no.

You may be the final word in the git project, but the burden of proof is
an essential part of logic, not project-dependent, and that's just not
the case.

*Anyone* that makes any claim has the burden of proof.

That is enshrined in the latin maxim:

  Semper necessitas probandi incumbit ei qui agit.

  The necessity of proof always lies with the person who lays charges.

There is a reason why in the US justice system defendants are declared
"not guilty", and not "inocent".

I have written extensively about the subject, for example: First
principles of logic [1].

If a woman is not wearing a wedding ring, the person that claims she is
married has the burden of proof, but also the person that claims she is
single. *Both* have the burden of proof. It's only the person that says
"we don't know" that doesn't.


Is the feature reasonable? If I claim that it's not reasonable, I have
he burden of proof, but if you claim that it is reasonable, so do you.

That's just a fact of logic.

> ---they are the ones who have to adequately explain why adding it is
> actively harmful, not just the proposed addition is not necessary
> [*1*].

No. Both have the burden of proof.

Only the people in the default position (we don't know if the feature is
reasonable or not) don't require burden of proof.

> There is no need for any "evidence" of "negative consequence" at all
> to ask for a way to selectively enable or disable a new feature.

Yes there is. Not to ask for a feature, but *demand* a feature.

But such burden has been met, which is why users can turn off
mergetool.autoMerge, so they can already selectively disable the new
feature.

> A new feature tends to trigger unexpected bugs in unseen corner cases
> more than an older feature, even without any concrete numbers, and
> that is good enough reason to insist an escape hatch that is easy to
> access by users to exist.

That rationale is meeting its burden of proof.

But users do already have an escape hatch: "mergetool.automerge=false".


On the other hand the burden of proof for the claim that some tools
might want to disable this flag and not other tools has not been met.


Moreover, ignoring arguments doesn't magically make the burden of proof
from your side dissappear.

Is there a conflict in this example?

  echo Hello > BASE
  echo Hello > LOCAL
  echo Hello. > REMOTE
  git merge-file -p LOCAL BASE REMOTE

Cheers.

[1] https://felipec.substack.com/p/first-principles-of-logic

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-19 12:08                             ` Felipe Contreras
@ 2020-12-19 18:26                               ` Junio C Hamano
  2020-12-19 20:18                                 ` Felipe Contreras
  0 siblings, 1 reply; 70+ messages in thread
From: Junio C Hamano @ 2020-12-19 18:26 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Seth House, Johannes Sixt, git, David Aguilar

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Junio C Hamano wrote:
>> Seth House <seth@eseth.com> writes:
>> 
>> > I think where we're not seeing eye-to-eye is that you're focusing on
>> > potential "negative" consequences whereas I'm talking about having more
>> > information about the merge rather than less.
>> >
>> > There is very likely no negative consequences for most, if not all,
>> > mergetools. I wrote the initial version of diffconflicts ten years ago
>> > and I've been using it nearly every day since. I'm fairly confident in
>> > the end result. What is a fact is there is undisputedly less information
>> > about the merge if we overwrite LOCAL and REMOTE; as I've written,
>> > I think the tradeoff is worthwhile for most tools but a per-tool
>> > configuration will allow people that feel differently to choose
>> > differently.
>> 
>> Thanks for stressing this point.
>> 
>> When a user or developer asks for a reasonable feature (e.g.
>> configurability to suit personal taste), especially when there is no
>> obvious downside for adding it, the burden of proof is on the party
>> who refuses to add it
>
> Sorry, but no.
>
> You may be the final word in the git project, but the burden of proof is
> an essential part of logic, not project-dependent, and that's just not
> the case.
>
> *Anyone* that makes any claim has the burden of proof.

Yes, and in this case, Seth already said he prefers to be able to
see the original, and not necessarily all the users of his mergetool
backend prefer the same thing.  That is enough "proof" to me that
the need exists.  It is your turn to prove your (implicit) claim
that it does more harm than it helps to allow such a preference
expressed by end users.

> Is there a conflict in this example?
>
>   echo Hello > BASE
>   echo Hello > LOCAL
>   echo Hello. > REMOTE
>   git merge-file -p LOCAL BASE REMOTE

Sorry, but I do not see why that example matters.  Would such a case
even come into the picture to be resolved by "git mergetool" in the
first place?


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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-18 11:58                         ` Felipe Contreras
@ 2020-12-19 18:52                           ` Junio C Hamano
  2020-12-19 20:59                             ` Felipe Contreras
  0 siblings, 1 reply; 70+ messages in thread
From: Junio C Hamano @ 2020-12-19 18:52 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Seth House, Johannes Sixt, git, David Aguilar

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> Another reason why allowing users to disable the feature per tool is
>> important is because as far as I know we have kept the mergetool
>> framework to allow adding a tool that can merge binary data, and
>> leaving these three files pristine was one ingredient for that.
>> With only a single knob, we would be making a decision to declare
>> that such a tool is unwelcome, which is not quite acceptable.  I
>> expect that users would want the new feature most of the time
>> because they would be managing text files more of the time, and
>> having only a single knob would force an unnecessary choice on those
>> who want to use such a binary-capable tool as well.
>
> I can't imagine what that binary data could look like, and how any tool
> could represent that to the user.

What I had in mind are use cases like merging "comment"-ish part of
media files (e.g. exif in jpeg, id3 in mp3---things like that), as
I've heard some people do use Git to manage their photo collection.

Of course, I can imagine that a cartoonist first draws a background
picture, cop es it twice, and then draws a dog on top of the
background in one copy while drawing a cat in the other.  You should
be able to merge the resulting two pictures cleanly by following the
three-way merge idea (take what got changed on only one side plus
what did not change--anything else is a conflict) as long as these
animals do not overlap.  You probably can even do an equivalent of
-Xours (not --ours) to essentially say which object is closer to the
viewer in a conflicting case.

> But either way "git merge-file" fails on those cases, so we can just
> check if the file is empty, and bail out.

Catching failures from merge-file and reverting back to the original
behaviour would be an improvement, if the code in the earlier
iteration was not checking errors.  But I would prefer not count on
the tool always to fail, as there are image file formats that appear
to be text that are unreadable to humans (like pnm), and my primary
reason for configurability is as an escape hatch to be used in cases
where we do not anticipate here.  Listing "what about this case, it
does not break" million times would not help us here.

With per-tool enable/disable option, the users do not have to rely
on failure from merge-file anyway.

Thanks.

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-19 18:26                               ` Junio C Hamano
@ 2020-12-19 20:18                                 ` Felipe Contreras
  0 siblings, 0 replies; 70+ messages in thread
From: Felipe Contreras @ 2020-12-19 20:18 UTC (permalink / raw)
  To: Junio C Hamano, Felipe Contreras
  Cc: Seth House, Johannes Sixt, git, David Aguilar

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > Junio C Hamano wrote:
> >> Seth House <seth@eseth.com> writes:
> >> 
> >> > I think where we're not seeing eye-to-eye is that you're focusing on
> >> > potential "negative" consequences whereas I'm talking about having more
> >> > information about the merge rather than less.
> >> >
> >> > There is very likely no negative consequences for most, if not all,
> >> > mergetools. I wrote the initial version of diffconflicts ten years ago
> >> > and I've been using it nearly every day since. I'm fairly confident in
> >> > the end result. What is a fact is there is undisputedly less information
> >> > about the merge if we overwrite LOCAL and REMOTE; as I've written,
> >> > I think the tradeoff is worthwhile for most tools but a per-tool
> >> > configuration will allow people that feel differently to choose
> >> > differently.
> >> 
> >> Thanks for stressing this point.
> >> 
> >> When a user or developer asks for a reasonable feature (e.g.
> >> configurability to suit personal taste), especially when there is no
> >> obvious downside for adding it, the burden of proof is on the party
> >> who refuses to add it
> >
> > Sorry, but no.
> >
> > You may be the final word in the git project, but the burden of proof is
> > an essential part of logic, not project-dependent, and that's just not
> > the case.
> >
> > *Anyone* that makes any claim has the burden of proof.
> 
> Yes, and in this case, Seth already said he prefers to be able to
> see the original, and not necessarily all the users of his mergetool
> backend prefer the same thing.  That is enough "proof" to me that
> the need exists.

That's not proof. That's one person stating his opinion.

He did not present evidence of *why* one of his users would prefer to
turn this flag off in his tool, but not others.

Here, allow me to show what diffconflicts does:

diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index abc8ce4ec4..a8fd8e4a84 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -29,6 +29,10 @@ merge_cmd () {
 				"$LOCAL" "$REMOTE" "$MERGED"
 		fi
 		;;
+	*vimdiff4)
+		cp "$LOCAL" "$MERGED"
+		"$merge_tool_path" -f -d "$MERGED" "$REMOTE"
+		;;
 	esac
 }
 

"mergetool.automerge=true" plus the code above replicates the main
functionality of the diffconflicts plugin.

Why would anyone want to turn off automerge for vimdiff4, but not for
vimdiff2?

Nobody has explained that.

> It is your turn to prove your (implicit) claim that it does more harm
> than it helps to allow such a preference expressed by end users.

No. The burden of proof is still on the side that claims such preference
is desirable.

"I think X" is not evidence of X.

> > Is there a conflict in this example?
> >
> >   echo Hello > BASE
> >   echo Hello > LOCAL
> >   echo Hello. > REMOTE
> >   git merge-file -p LOCAL BASE REMOTE
> 
> Sorry, but I do not see why that example matters.

But I do. That's why I asked the question.

> Would such a case even come into the picture to be resolved by "git
> mergetool" in the first place?

That is precisely the point.

It is not a conflict because that line only has changes in REMOTE, but
not in LOCAL. Correct?

git doesn't bother the user because there's no merge conflict.

But if there is an *actual conflict* some lines below, why should git,
or git-mergetool, or the mergetool now bother the user presenting it the
diff of the "Hello." line?

Why does that line that according to you "wouldn't not even come into
the picture" becomes suddenly relevant when there's an actual conflict
below?

Either the "Hello." change is relevant or it isn't.

But more importantly; its relevance doesn't depend on the mergetool.

Choosing vimdiff4 doesn't magically make it a conflict.

Cheer.

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-19 18:52                           ` Junio C Hamano
@ 2020-12-19 20:59                             ` Felipe Contreras
  2020-12-20  6:44                               ` David Aguilar
  0 siblings, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-19 20:59 UTC (permalink / raw)
  To: Junio C Hamano, Felipe Contreras
  Cc: Seth House, Johannes Sixt, git, David Aguilar

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> >> Another reason why allowing users to disable the feature per tool is
> >> important is because as far as I know we have kept the mergetool
> >> framework to allow adding a tool that can merge binary data, and
> >> leaving these three files pristine was one ingredient for that.
> >> With only a single knob, we would be making a decision to declare
> >> that such a tool is unwelcome, which is not quite acceptable.  I
> >> expect that users would want the new feature most of the time
> >> because they would be managing text files more of the time, and
> >> having only a single knob would force an unnecessary choice on those
> >> who want to use such a binary-capable tool as well.
> >
> > I can't imagine what that binary data could look like, and how any tool
> > could represent that to the user.
> 
> What I had in mind are use cases like merging "comment"-ish part of
> media files (e.g. exif in jpeg, id3 in mp3---things like that), as
> I've heard some people do use Git to manage their photo collection.

Right. They can do that with a text editor.

> Of course, I can imagine that a cartoonist first draws a background
> picture, cop es it twice, and then draws a dog on top of the
> background in one copy while drawing a cat in the other.  You should
> be able to merge the resulting two pictures cleanly by following the
> three-way merge idea (take what got changed on only one side plus
> what did not change--anything else is a conflict) as long as these
> animals do not overlap.  You probably can even do an equivalent of
> -Xours (not --ours) to essentially say which object is closer to the
> viewer in a conflicting case.

The whole point of separating the background from the foreground is that
the foreground can be animated on top of the background, so they would
always be two different files.

Even if we force the issue and make two graphic artists work on two
different branches, what they would inevitably end up doing is work on
different layers, which for all intents and purposes are like two files.
No mergetool is going to help them integrate their changes.

> > But either way "git merge-file" fails on those cases, so we can just
> > check if the file is empty, and bail out.
> 
> Catching failures from merge-file and reverting back to the original
> behaviour would be an improvement, if the code in the earlier
> iteration was not checking errors.  But I would prefer not count on
> the tool always to fail, as there are image file formats that appear
> to be text that are unreadable to humans (like pnm),

git would not add conflict markers on the part of a pnm file that did
not change, so in fact, a person merging pnm files might in fact desire
automerge.

> and my primary reason for configurability is as an escape hatch to be
> used in cases where we do not anticipate here.

Once again: "mergetool.automerge=false" is a thing.

> Listing "what about this case, it does not break" million times would
> not help us here.

This is the philosophical problem of induction: a million white swans
doesn't prove all swans are white.

The only thing we know for certain is that there is no known problem.
And that if and when such a problem occurs, we would need to think about
the proper solution.

> With per-tool enable/disable option, the users do not have to rely
> on failure from merge-file anyway.

They don't have to rely on that failure, they can just turn off
mergetool.automerge.


But fine. Let's the perfect be the enemy of the good. That seems wise.

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-19 20:59                             ` Felipe Contreras
@ 2020-12-20  6:44                               ` David Aguilar
  2020-12-20  7:53                                 ` Felipe Contreras
  0 siblings, 1 reply; 70+ messages in thread
From: David Aguilar @ 2020-12-20  6:44 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Junio C Hamano, Seth House, Johannes Sixt, Git Mailing List

On Sat, Dec 19, 2020 at 12:59 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> Junio C Hamano wrote:
> > Felipe Contreras <felipe.contreras@gmail.com> writes:
> >
> > >> Another reason why allowing users to disable the feature per tool is
> > >> important is because as far as I know we have kept the mergetool
> > >> framework to allow adding a tool that can merge binary data, and
> > >> leaving these three files pristine was one ingredient for that.
> > >> With only a single knob, we would be making a decision to declare
> > >> that such a tool is unwelcome, which is not quite acceptable.  I
> > >> expect that users would want the new feature most of the time
> > >> because they would be managing text files more of the time, and
> > >> having only a single knob would force an unnecessary choice on those
> > >> who want to use such a binary-capable tool as well.
> > >
> > > I can't imagine what that binary data could look like, and how any tool
> > > could represent that to the user.
> >
> > What I had in mind are use cases like merging "comment"-ish part of
> > media files (e.g. exif in jpeg, id3 in mp3---things like that), as
> > I've heard some people do use Git to manage their photo collection.
>
> Right. They can do that with a text editor.
>
> > Of course, I can imagine that a cartoonist first draws a background
> > picture, cop es it twice, and then draws a dog on top of the
> > background in one copy while drawing a cat in the other.  You should
> > be able to merge the resulting two pictures cleanly by following the
> > three-way merge idea (take what got changed on only one side plus
> > what did not change--anything else is a conflict) as long as these
> > animals do not overlap.  You probably can even do an equivalent of
> > -Xours (not --ours) to essentially say which object is closer to the
> > viewer in a conflicting case.
>
> The whole point of separating the background from the foreground is that
> the foreground can be animated on top of the background, so they would
> always be two different files.
>
> Even if we force the issue and make two graphic artists work on two
> different branches, what they would inevitably end up doing is work on
> different layers, which for all intents and purposes are like two files.
> No mergetool is going to help them integrate their changes.
>
> > > But either way "git merge-file" fails on those cases, so we can just
> > > check if the file is empty, and bail out.
> >
> > Catching failures from merge-file and reverting back to the original
> > behaviour would be an improvement, if the code in the earlier
> > iteration was not checking errors.  But I would prefer not count on
> > the tool always to fail, as there are image file formats that appear
> > to be text that are unreadable to humans (like pnm),
>
> git would not add conflict markers on the part of a pnm file that did
> not change, so in fact, a person merging pnm files might in fact desire
> automerge.
>
> > and my primary reason for configurability is as an escape hatch to be
> > used in cases where we do not anticipate here.
>
> Once again: "mergetool.automerge=false" is a thing.
>
> > Listing "what about this case, it does not break" million times would
> > not help us here.
>
> This is the philosophical problem of induction: a million white swans
> doesn't prove all swans are white.
>
> The only thing we know for certain is that there is no known problem.
> And that if and when such a problem occurs, we would need to think about
> the proper solution.
>
> > With per-tool enable/disable option, the users do not have to rely
> > on failure from merge-file anyway.
>
> They don't have to rely on that failure, they can just turn off
> mergetool.automerge.
>
>
> But fine. Let's the perfect be the enemy of the good. That seems wise.


FWIW I'm in favor of having per-tool configuration precisely for
custom mergetools that do things with custom file formats and benefit
from having all of LOCAL REMOTE and BASE.

I don't have to imagine these use cases, they are very real. No survey
can be exhaustive so being flexible and allowing for a mixed tool
ecosystem is the right choice.

This design choice is also in alignment with the general
mergetool/difftool per-tool configuration paradigm.  If we didn't
support per-tool, then it would be inconsistent.
-- 
David

(sorry, posting from gmail's web interface so this probably won't hit
the public lists, but I probably won't reply beyond this email stating
my preference)

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-20  6:44                               ` David Aguilar
@ 2020-12-20  7:53                                 ` Felipe Contreras
  2020-12-20 22:22                                   ` David Aguilar
  0 siblings, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-20  7:53 UTC (permalink / raw)
  To: David Aguilar, Felipe Contreras
  Cc: Junio C Hamano, Seth House, Johannes Sixt, Git Mailing List

David Aguilar wrote:
> On Sat, Dec 19, 2020 at 12:59 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> > They don't have to rely on that failure, they can just turn off
> > mergetool.automerge.
> >
> >
> > But fine. Let's the perfect be the enemy of the good. That seems wise.
> 
> FWIW I'm in favor of having per-tool configuration precisely for
> custom mergetools that do things with custom file formats and benefit
> from having all of LOCAL REMOTE and BASE.

That's a preference, not an argument.

> I don't have to imagine these use cases, they are very real.

Show one.

> This design choice is also in alignment with the general
> mergetool/difftool per-tool configuration paradigm.  If we didn't
> support per-tool, then it would be inconsistent.

Can you do

 1. mergetool.vimdiff3.keepBackup
 2. mergetool.vimdiff3.keepTemporaries
 3. mergetool.vimdiff3.writeToTemp
 4. mergetool.vimdiff3.prompt

?

In fact the opposite is the case; not a *single* `mergetool.foo`
configuration can be done with `mergetool.<tool>.foo`.

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-20  7:53                                 ` Felipe Contreras
@ 2020-12-20 22:22                                   ` David Aguilar
  2020-12-21  1:46                                     ` Felipe Contreras
  0 siblings, 1 reply; 70+ messages in thread
From: David Aguilar @ 2020-12-20 22:22 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Junio C Hamano, Seth House, Johannes Sixt, Git Mailing List

On Sat, Dec 19, 2020 at 11:53 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> David Aguilar wrote:
> > On Sat, Dec 19, 2020 at 12:59 PM Felipe Contreras
> > <felipe.contreras@gmail.com> wrote:
> > > They don't have to rely on that failure, they can just turn off
> > > mergetool.automerge.
> > >
> > >
> > > But fine. Let's the perfect be the enemy of the good. That seems wise.
> >
> > FWIW I'm in favor of having per-tool configuration precisely for
> > custom mergetools that do things with custom file formats and benefit
> > from having all of LOCAL REMOTE and BASE.
>
> That's a preference, not an argument.

Lol, that's why I said "in favor".  But, since you asked, it turns out
that my preferences have a stronger weight than yours.

"git shortlog -n git-difftool* git-mergetool*" shows that my
preferences and opinions are 44 times more important than yours in
this area, whether you like it or not.  ;-p

Poking fun is my answer to such misguided seriousness.


> > I don't have to imagine these use cases, they are very real.
>
> Show one.

Heh, I don't have to show you anything.  If you *really* want to see
one then I would be super happy to get your resume, but I doubt I can
convince you to go that far!

Barring that, please use your imagination.  Imagine a custom file
format for a graph-like data structure (both binary and text-like)
that is able to use the full set of information for the purposes of
resolving conflicts.  Private tools exist.  It's impossible to prove
that they don't so I won't ask you or anyone else to do so.


> > This design choice is also in alignment with the general
> > mergetool/difftool per-tool configuration paradigm.  If we didn't
> > support per-tool, then it would be inconsistent.
>
> Can you do
>
>  1. mergetool.vimdiff3.keepBackup
>  2. mergetool.vimdiff3.keepTemporaries
>  3. mergetool.vimdiff3.writeToTemp
>  4. mergetool.vimdiff3.prompt

can_merge()
diff_cmd()
merge_cmd()
translate_merge_tool_path()
list_tool_variants()
exit_code_trustable()

mergetool.<tool>.cmd
mergetool.<tool>.path
mergetool.<tool>.trustExitCode

We even have mergetool.meld.hasOutput which is completely tool-specific.

I'm not asking, I'm telling you: the tool was designed around per-tool
hooks.  This is per-tool behavior.  End of story.


> ?
>
> In fact the opposite is the case; not a *single* `mergetool.foo`
> configuration can be done with `mergetool.<tool>.foo`.

That's not the right question.  This a behavior that can differ
per-tool and thus this feature must reflect that.

There is a difference between top-level and per-tool behavior.  This
is a per-tool feature and we want a way for users to be able to
enable/disable it globally while still enabling/disabling for an
individual tool.  We must allow individual tools to expose their
enhanced capabilities, otherwise the community tools have no reason to
improve, and private tools would be harmed.

The way to think about it is that it's a per-tool feature with a
top-level default.  The important implementation detail is that tools
have plenty of hooks for per-tool flexibility; the configuration is
just one way that we allow for it.  The scriplet for something like
"diffconflicts" could even override should_automerge() to return false
since it might want to handle it itself, for example.

We are not designing just for today; we must keep our eye on
longer-term goals where the community tools can be improved.

I'm in favor of the auto-merge feature in general, and think it should
be made the default behavior (in a future version?).  That said, it
entails a change in behavior and having a granular mechanism to both
allow enhanced tools to do their own thing, and to allow the original
behavior to be retained, is the most sensible approach.

Here is the "right" question to ask: Why shouldn't we have this
flexibility?  The implementation is pretty darn simple considering
that the toolset is already designed with per-tool affordances in
mind, so why not?

What I am 100% certain about is that someone is going to notice the
change in behavior, and whether or not it's "better" is completely
irrelevant.  The heavy hammer of completely disabling the feature
means that they will have crippled all of the other mergetools just
because one of them couldn't cope, or because for whatever reason they
want the old behavior, so we should turn that heavy hammer into a
fine-grained chisel.

Are you willing to compromise on the small concession that we should
allow per-tool overrides so that we can move forward and get this
integrated?


To be extremely clear -- is this discussion arguing between the
following two strategies?

# Per-tool override + Global default

should_automerge () {
    git config mergetool.$tool.automerge || git config
mergetool.automerge || echo true
}

# vs. Global default only

should_automerge () {
    git config mergetool.automerge || echo true
}

, or are we discussing a different aspect?  When spelled out in code,
we're discussing whether or not we should have
"mergetool.$tool.automerge ||" in there, and the argument for why not
is pretty thin considering that use cases that would be harmed by not
doing so exist.

Such a trivial difference is not a hill worth dying on so please route
this energy into a patch so that we can get this integrated.

I suggest this compromise -- if adding "mergetool.$tool.automerge ||"
to the logic is something you don't want to do then submit the patch
without it and I'll submit a follow-up patch that adds the per-tool
override.

I'd personally prefer to just have the patch include this from the
get-go, though, so if we've managed to convince you then please take
the shorter path to success.
-- 
David

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
       [not found]                   ` <CANiSa6jMXTyfo43bUdC8601BvYKiF67HXo+QaiTh_-8KWyBsLg@mail.gmail.com>
@ 2020-12-21  0:31                     ` Felipe Contreras
  0 siblings, 0 replies; 70+ messages in thread
From: Felipe Contreras @ 2020-12-21  0:31 UTC (permalink / raw)
  To: Martin von Zweigbergk, Felipe Contreras
  Cc: Junio C Hamano, Seth House, Johannes Sixt, git, David Aguilar

Martin von Zweigbergk wrote:
> On Thu, Dec 17, 2020, 16:52 Felipe Contreras <felipe.contreras@gmail.com>
> wrote:
> 
> > Junio C Hamano wrote:
> > > Many people may stick to just a single tool and may find a single
> > > mergetool.autoMerge knob convenient (and it is OK for them if the
> > > new behaviour broke a tool they do not use), but for those who use
> > > more than one, being able to configure them differently would be
> > > valuable.
> >
> > On what tool would you turn this on, and what tool would you turn this
> > off?
> 
> Maybe turn it off for a mergetool smart enough to understand the semantics
> of the change?
> 
> Let's say BASE contains a function foo(). LOCAL renames foo() to bar() and
> OTHER adds a call to foo(). The tool would need to know that the call to
> foo() was added in order to know that it should be renamed in the output.

That's true, but that's something we would want in git itself, not the
mergetool.

If there are no conflicts, "git merge" would simply do the automatic
merge, and this magical semantic mergetool would never have an
opportunity to run.

> I've only skimmed this thread, so I apologize if I've misunderstood the
> quotation or if my point has already been brought up.

Nobody has made this point. And it's the kind of rationale I was looking
for.

Cheers.

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-20 22:22                                   ` David Aguilar
@ 2020-12-21  1:46                                     ` Felipe Contreras
  0 siblings, 0 replies; 70+ messages in thread
From: Felipe Contreras @ 2020-12-21  1:46 UTC (permalink / raw)
  To: David Aguilar, Felipe Contreras
  Cc: Junio C Hamano, Seth House, Johannes Sixt, Git Mailing List

David Aguilar wrote:
> On Sat, Dec 19, 2020 at 11:53 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > David Aguilar wrote:
> > > On Sat, Dec 19, 2020 at 12:59 PM Felipe Contreras
> > > <felipe.contreras@gmail.com> wrote:
> > > > They don't have to rely on that failure, they can just turn off
> > > > mergetool.automerge.
> > > >
> > > >
> > > > But fine. Let's the perfect be the enemy of the good. That seems wise.
> > >
> > > FWIW I'm in favor of having per-tool configuration precisely for
> > > custom mergetools that do things with custom file formats and benefit
> > > from having all of LOCAL REMOTE and BASE.
> >
> > That's a preference, not an argument.
> 
> Lol, that's why I said "in favor".  But, since you asked, it turns out
> that my preferences have a stronger weight than yours.
> 
> "git shortlog -n git-difftool* git-mergetool*" shows that my
> preferences and opinions are 44 times more important than yours in
> this area, whether you like it or not.  ;-p

That doesn't mean you are right.

This is an appeal to authority fallacy.

> Poking fun is my answer to such misguided seriousness.

I did not poke fun. I was just pointing out the fact that it wasn't an
argument.

> > > I don't have to imagine these use cases, they are very real.
> >
> > Show one.
> 
> Heh, I don't have to show you anything.

No you don't, but in a discussion with other people if you don't
substantiate your claims, then others have no rational reason to
consider them.

> Barring that, please use your imagination.  Imagine a custom file
> format for a graph-like data structure (both binary and text-like)
> that is able to use the full set of information for the purposes of
> resolving conflicts.  Private tools exist.  It's impossible to prove
> that they don't so I won't ask you or anyone else to do so.

So it's not a real use-case, it's an imaginary one.

Even if we do consider this imaginary use-case, you just swept the
problem under the carpet by saying "is able to use the full set of
information".

How?

Explain how this tool would use this information.

> > > This design choice is also in alignment with the general
> > > mergetool/difftool per-tool configuration paradigm.  If we didn't
> > > support per-tool, then it would be inconsistent.
> >
> > Can you do
> >
> >  1. mergetool.vimdiff3.keepBackup
> >  2. mergetool.vimdiff3.keepTemporaries
> >  3. mergetool.vimdiff3.writeToTemp
> >  4. mergetool.vimdiff3.prompt
> 
> can_merge()
> diff_cmd()
> merge_cmd()
> translate_merge_tool_path()
> list_tool_variants()
> exit_code_trustable()
> 
> mergetool.<tool>.cmd
> mergetool.<tool>.path
> mergetool.<tool>.trustExitCode

None of those are user preferences.

> We even have mergetool.meld.hasOutput which is completely tool-specific.

We don't have a user preference `mergetool.hasOutput`, or
`mergetool.vimdiff3.hasOutput` for that matter.
 
> I'm not asking, I'm telling you: the tool was designed around per-tool
> hooks.  This is per-tool behavior.  End of story.

And yet a tool cannot override `mergetool.keepBackup`.

Correct?

> > ?
> >
> > In fact the opposite is the case; not a *single* `mergetool.foo`
> > configuration can be done with `mergetool.<tool>.foo`.
> 
> That's not the right question.  This a behavior that can differ
> per-tool and thus this feature must reflect that.

Can it?

I am waiting for somebody to show *how*.

> There is a difference between top-level and per-tool behavior.

> This is a per-tool feature

No, that's your opinion. You haven't explained why.

> The way to think about it is that it's a per-tool feature

Ditto.

> We are not designing just for today; we must keep our eye on
> longer-term goals where the community tools can be improved.

Yes, but an actual future that might actually happen.

> Here is the "right" question to ask: Why shouldn't we have this
> flexibility?

Why shouldn't we have `mergetool.vimdiff3.keepBackup`?

> The heavy hammer of completely disabling the feature means that they
> will have crippled all of the other mergetools just because one of
> them couldn't cope,

Precisely *how* a tool couldn't cope?

> Are you willing to compromise on the small concession that we should
> allow per-tool overrides so that we can move forward and get this
> integrated?

We can move forward right now.

But no, I am 99.99% certain *nobody* will ever implement
should_automerge (), so I won't waste my time with it.

If you think otherwise, you spend your time implementing this on top of
my patch. That way it's clear who made the mistake.

> To be extremely clear -- is this discussion arguing between the
> following two strategies?
> 
> # Per-tool override + Global default
> 
> should_automerge () {
>     git config mergetool.$tool.automerge || git config
> mergetool.automerge || echo true
> }
> 
> # vs. Global default only
> 
> should_automerge () {
>     git config mergetool.automerge || echo true
> }
> 
> , or are we discussing a different aspect?  When spelled out in code,
> we're discussing whether or not we should have
> "mergetool.$tool.automerge ||" in there, and the argument for why not
> is pretty thin considering that use cases that would be harmed by not
> doing so exist.

I don't think mergetool.autoMerge should be any different from
mergetool.keepBackup; a single global user configuration suffices.

You say the argument against such per-tool configuration is thin, I say
the argument in favor is non-existent.

> Such a trivial difference is not a hill worth dying on so please route
> this energy into a patch so that we can get this integrated.

Nobody can tell me how I chose to spend my free time that I'm choosing to
contribute to this project altruistically.

> I suggest this compromise -- if adding "mergetool.$tool.automerge ||"
> to the logic is something you don't want to do then submit the patch
> without it and I'll submit a follow-up patch that adds the per-tool
> override.

That's not a compromise. That's a difference of opinion stated in the
code.

I don't have to convince you. And you don't have to convince me.

> I'd personally prefer to just have the patch include this from the
> get-go, though, so if we've managed to convince you then please take
> the shorter path to success.

And I would prefer that we don't waste time with hypothetical use-cases
that will never materialize.

We can add such configuration when the need actually arises.

But we can't always have what we prefer.

Cheers.

[1] https://en.wikipedia.org/wiki/Argument_from_authority

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-19 11:14                           ` Junio C Hamano
  2020-12-19 12:08                             ` Felipe Contreras
@ 2020-12-21  4:25                             ` Seth House
  2020-12-21  5:34                               ` Felipe Contreras
  1 sibling, 1 reply; 70+ messages in thread
From: Seth House @ 2020-12-21  4:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Felipe Contreras, Johannes Sixt, git, David Aguilar

On Sat, Dec 19, 2020 at 03:14:23AM -0800, Junio C Hamano wrote:
> Seth House <seth@eseth.com> writes:
> > This is where I will part this particular debate.
> 
> Thanks for your contribution so far.

Re-reading my message now, I sounded overly final. I appreciated your
initial warm welcome and I'm still very much present on this list and
still very much invested in seeing this patch through to consensus and
completion -- and that includes writing code if needed.

To rephrase: I don't wish to spend any more of my time debating
`mergetool.autoMerge`
vs.
`mergetool.autoMerge || mergetool.$tool.autoMerge`
and I appreciate that others on this list have joined that debate.
I think all the angles and opinions have been covered at this point so
perhaps the time has come for a vote or for an executive decision.

---

The task of re-reviewing all the mergetools surveyed in the original
blog post is now complete. I took the opportunity to update most of the
original post to reflect the discussion and audience here.

https://www.eseth.org/2020/mergetools.html

The "Mergetools Comparison" section is long and is not very easy to read
with the current layout. Sorry. I wanted to get this published quickly
and I'll try to clean it up and add a proper TOC. For now, watch out for
the "Summary" under each tool.

tl;dr: I didn't see any noteworthy problems with any tool. Mostly
positive results and some no-impact results. The two minor impacts were
from the two tools that make use of LOCAL and REMOTE as historical
references; I think those are safe to ignore because one is mine and the
other builds on mine. All the other surveyed tools that reference older
versions of the conflicted file appear to actively query the Git
repository to obtain that info.

I'd still like to add more tools and do deeper dives with some of the
tools already surveyed so suggestions, feedback, and criticisms are very
welcome. That said, I am now feeling comfortable about adding this to
Git and defaulting it to enabled. :D


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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-21  4:25                             ` Seth House
@ 2020-12-21  5:34                               ` Felipe Contreras
  2020-12-21  7:36                                 ` Seth House
  0 siblings, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-21  5:34 UTC (permalink / raw)
  To: Seth House, Junio C Hamano
  Cc: Felipe Contreras, Johannes Sixt, git, David Aguilar

Seth House wrote:
> To rephrase: I don't wish to spend any more of my time debating
> `mergetool.autoMerge`
> vs.
> `mergetool.autoMerge || mergetool.$tool.autoMerge`
> and I appreciate that others on this list have joined that debate.

> I think all the angles and opinions have been covered at this point so
> perhaps the time has come for a vote or for an executive decision.

I disagree. It's fine if you don't want to participate, but the fact
remains that the position that some tools would want to turn this off
hasn't been properly defended.

> The task of re-reviewing all the mergetools surveyed in the original
> blog post is now complete. I took the opportunity to update most of the
> original post to reflect the discussion and audience here.
> 
> https://www.eseth.org/2020/mergetools.html

Thanks for doing this, but unfortunately one of the most popular isn't
listed there: vimdiff2.

> The "Mergetools Comparison" section is long and is not very easy to read
> with the current layout. Sorry. I wanted to get this published quickly
> and I'll try to clean it up and add a proper TOC. For now, watch out for
> the "Summary" under each tool.
> 
> tl;dr: I didn't see any noteworthy problems with any tool. Mostly
> positive results and some no-impact results.

That's what I expected.

> The two minor impacts were from the two tools that make use of LOCAL
> and REMOTE as historical references; I think those are safe to ignore
> because one is mine and the other builds on mine. All the other
> surveyed tools that reference older versions of the conflicted file
> appear to actively query the Git repository to obtain that info.

In both you say "identical output". The only "adverse effects" are in
the secondary view, which you didn't present.

Can you show these "adverse effects"?

> I'd still like to add more tools and do deeper dives with some of the
> tools already surveyed so suggestions, feedback, and criticisms are very
> welcome. That said, I am now feeling comfortable about adding this to
> Git and defaulting it to enabled. :D

Cool.

Fortunately in the unlikely situation that somebody manages to find a
tool with adverse effects they can just disable the flag.

Cheers.

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-21  5:34                               ` Felipe Contreras
@ 2020-12-21  7:36                                 ` Seth House
  2020-12-21 11:17                                   ` Felipe Contreras
  2020-12-21 22:15                                   ` David Aguilar
  0 siblings, 2 replies; 70+ messages in thread
From: Seth House @ 2020-12-21  7:36 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Junio C Hamano, Johannes Sixt, git, David Aguilar

On Sun, Dec 20, 2020 at 11:34:24PM -0600, Felipe Contreras wrote:
> I disagree. It's fine if you don't want to participate, but the fact
> remains that the position that some tools would want to turn this off
> hasn't been properly defended.

If you are _genuinely_ interested in the answer to this question, please
read the section in my post titled "Conflict Resolution" followed by the
sub-section "Custom Merge Algorithm", and finally "Merge algorithms" [1]
on Wikipedia. Then pretend you want to write your own conflict
resolution algorithm for a new mergetool you've been dreaming up and ask
yourself what versions of the conflicted file your tool will need.

[1] https://en.wikipedia.org/wiki/Merge_(version_control)#Merge_algorithms

Right now the algorithm Git uses is pretty best-in-class so it might
seem unlikely that someone would want to write one of those. However
a whopping *seven* of the tools surveyed do just that. Some of them even
do a pretty good job (I've tried to point those out in the reviews).
You're preoccupied with identifying a specific "adverse effect" but this
debate isn't about that -- it's about giving individual tools the option
to choose how they are used. If people out there want to try and write
a better algorithm than Git, I want to see them try.

That's the point I've been trying to drive home and that's the point
that David also made in his last reply to you.

On that note: you replied to David and said:

> [Y]ou spend your time implementing this on top of my patch. That way
> it's clear who made the mistake.

I plan to start work on exactly that tomorrow. You made the initial
patch so if you'd prefer to take it over the finish line yourself I'll
defer. But if you're not interested then I would be happy to credit you
and finish it.

> Thanks for doing this, but unfortunately one of the most popular isn't
> listed there: vimdiff2.

It's under the Vim section. Use the page search in your browser for
"vimdiff2".

> That's what I expected.

That's what we all expected. The purpose of this follow-up survey was to
validate that expectation.


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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-21  7:36                                 ` Seth House
@ 2020-12-21 11:17                                   ` Felipe Contreras
  2020-12-21 22:15                                   ` David Aguilar
  1 sibling, 0 replies; 70+ messages in thread
From: Felipe Contreras @ 2020-12-21 11:17 UTC (permalink / raw)
  To: Seth House, Felipe Contreras
  Cc: Junio C Hamano, Johannes Sixt, git, David Aguilar

Seth House wrote:
> On Sun, Dec 20, 2020 at 11:34:24PM -0600, Felipe Contreras wrote:
> > I disagree. It's fine if you don't want to participate, but the fact
> > remains that the position that some tools would want to turn this off
> > hasn't been properly defended.
> 
> If you are _genuinely_ interested in the answer to this question,

It's not about an answer to any specific question, defending a position
requires for you to answer any subsequent questions, like the one I made
here [1], which you never answered.

> please read the section in my post titled "Conflict Resolution"
> followed by the sub-section "Custom Merge Algorithm", and finally
> "Merge algorithms" [1] on Wikipedia.
> [1] https://en.wikipedia.org/wiki/Merge_(version_control)#Merge_algorithms

I had already read your blog post, and OK; I read that section in
Wikipedia.

> Then pretend you want to write your own conflict resolution algorithm
> for a new mergetool you've been dreaming up and ask yourself what
> versions of the conflicted file your tool will need.

I don't dream of writing my own conflict resolution algorithm, but if I
did it, would be to be used *before* the mergetool, before git makes the
decision to bother the user to resolve the conflicts manually, before it
decides there are actual conflicts, and before it writes the conflict
markers in the file.

In fact, it would work without any mergetool, that way the people that
like to edit the file directly (which you mentioned in your blog post
are actually the majority of your coworkers) can benefit from such
an algorithm.


I answered yours, now you answer mine [1]. What happens if you remove
the first paragraph of your make-conflicts.sh script?

Would a user have the chance to run "git mergetool" in those situations?

  echo "The frumious bandersnatch!" > BASE
  echo "The frumious bandersnatch!" > LOCAL
  echo "The frumious Bandersnatch!" > REMOTE
  git merge-file -p LOCAL BASE REMOTE

> Right now the algorithm Git uses is pretty best-in-class so it might
> seem unlikely that someone would want to write one of those.

And yet that's what Elijah Newren is doing with his merge-ort proposal
which is flying right now.

You want these algorithms to run in "git merge", not "git mergetool".

> However a whopping *seven* of the tools surveyed do just that.

Because they are used in other situations, like working with subversion,
not just in git merge conflicts.

> Some of them even do a pretty good job (I've tried to point those out
> in the reviews).

Better than git?

> You're preoccupied with identifying a specific "adverse effect" but this
> debate isn't about that -- it's about giving individual tools the option
> to choose how they are used.

Yes, individual *real* tools that either exist, or could actually exist.

> If people out there want to try and write a better algorithm than Git,
> I want to see them try.

Yes, and we want that algorithm to be used *before* the mergetool.

Moreover, if any merge algorithm hopes to do better than git, it would
require more than just 3 versions of the file; it would require to look
at the commit graph.

If you somehow managed to create such an algorithm that is better than
git, you certainly wouldn't give it out for free, and it wouldn't be on
a puny mergetool to be used only when git thinks there are conflicts; it
would be a solution to be used *at every merge*.

And that's what a *real* tool, Plastic SCM [2], does; it's a complete
solution that claims to have a better merge algorithm than Git.

> That's the point I've been trying to drive home and that's the point
> that David also made in his last reply to you.
> 
> On that note: you replied to David and said:
> 
> > [Y]ou spend your time implementing this on top of my patch. That way
> > it's clear who made the mistake.
> 
> I plan to start work on exactly that tomorrow. You made the initial
> patch so if you'd prefer to take it over the finish line yourself I'll
> defer. But if you're not interested then I would be happy to credit you
> and finish it.

It is already at the finish line (I just need to update the commit
message).

If you take my patch and add a bunch of unnecessary code, then remove my
'Signed-off-by' line, because I don't approve of those changes.

Better yet; add *your* changes on a separate commit on top of my patch.
That way it's clear who did the meat of the changes, and who did the
unnecessary ones.

> > Thanks for doing this, but unfortunately one of the most popular isn't
> > listed there: vimdiff2.
> 
> It's under the Vim section. Use the page search in your browser for
> "vimdiff2".

It says "The vimdiff, vimdiff3, and vimdiff2 mergetools are not
individually detailed because they're all variations on the same theme".
So yeah; no screenshots.

And I disagree; vimdiff2 looks completely different from vimdiff. If you
have to choose one, you should choose vimdiff2, which is what everyone
in the reddit thread you posted on r/git told you they use, not vimdiff.

You shouldn't deliberately chose the worst option that nobody uses.

Moreover, the main view of your mergetool can be considered a variation
also, I can replicate the behavior with the following vimdiff4:

  "$merge_tool_path" -f -d -c 'wincmd l | %d | read #1 | 1d | bd 1' \
    "$LOCAL" "$MERGED" "$REMOTE"

Does that mean diffconflicts shouldn't be listed either?

Of course not; just add vimdiff2.

Cheers.

[1] https://lore.kernel.org/git/5fdd4f22c473b_1d952220844@natae.notmuch/
[2] http://blog.plasticscm.com/2011/09/merge-recursive-strategy.html

-- 
Felipe Contreras

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

* Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
  2020-12-21  7:36                                 ` Seth House
  2020-12-21 11:17                                   ` Felipe Contreras
@ 2020-12-21 22:15                                   ` David Aguilar
  2020-12-21 23:51                                     ` Code of conduct violation? Felipe Contreras
  1 sibling, 1 reply; 70+ messages in thread
From: David Aguilar @ 2020-12-21 22:15 UTC (permalink / raw)
  To: Seth House
  Cc: Felipe Contreras, Junio C Hamano, Johannes Sixt, Git Mailing List

On Sun, Dec 20, 2020 at 11:36 PM Seth House <seth@eseth.com> wrote:
>
> On Sun, Dec 20, 2020 at 11:34:24PM -0600, Felipe Contreras wrote:
> > I disagree. It's fine if you don't want to participate, but the fact
> > remains that the position that some tools would want to turn this off
> > hasn't been properly defended.
>
> If you are _genuinely_ interested in the answer to this question, please
> read the section in my post titled "Conflict Resolution" followed by the
> sub-section "Custom Merge Algorithm", and finally "Merge algorithms" [1]
> on Wikipedia. Then pretend you want to write your own conflict
> resolution algorithm for a new mergetool you've been dreaming up and ask
> yourself what versions of the conflicted file your tool will need.
>
> [1] https://en.wikipedia.org/wiki/Merge_(version_control)#Merge_algorithms
>
> Right now the algorithm Git uses is pretty best-in-class so it might
> seem unlikely that someone would want to write one of those. However
> a whopping *seven* of the tools surveyed do just that. Some of them even
> do a pretty good job (I've tried to point those out in the reviews).
> You're preoccupied with identifying a specific "adverse effect" but this
> debate isn't about that -- it's about giving individual tools the option
> to choose how they are used. If people out there want to try and write
> a better algorithm than Git, I want to see them try.
>
> That's the point I've been trying to drive home and that's the point
> that David also made in his last reply to you.
>
> On that note: you replied to David and said:
>
> > [Y]ou spend your time implementing this on top of my patch. That way
> > it's clear who made the mistake.
>
> I plan to start work on exactly that tomorrow. You made the initial
> patch so if you'd prefer to take it over the finish line yourself I'll
> defer. But if you're not interested then I would be happy to credit you
> and finish it.

Thanks Seth, I think your plan of action sounds pretty solid to me.

I'll be happy to review your patch.  Just a heads-up that Felipe has a
history on this list of creating long, never-ending, pointless rant
threads so sometimes the best course of action is to ignore him.

ciao,
David

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

* Code of conduct violation?
  2020-12-21 22:15                                   ` David Aguilar
@ 2020-12-21 23:51                                     ` Felipe Contreras
  2020-12-22  7:13                                       ` Junio C Hamano
                                                         ` (2 more replies)
  0 siblings, 3 replies; 70+ messages in thread
From: Felipe Contreras @ 2020-12-21 23:51 UTC (permalink / raw)
  To: David Aguilar, Seth House
  Cc: Felipe Contreras, Junio C Hamano, Git Mailing List,
	Christian Couder, git

David Aguilar wrote:
> Just a heads-up that Felipe has a history on this list of creating
> long, never-ending, pointless rant threads so sometimes the best
> course of action is to ignore him.

I think this can be considered a personal attack, which goes against the
code of conduct.

The code of conduct suggests behavior like:

 * Being respectful of differing viewpoints and experiences
 * Showing empathy towards other community members

It seems to me the above comment is an example of the opposite.

If you feel that my feedback is pointless, then don't engage with it.
There's no need to put aggravating labels, especially on a public
setting. Even if most people agree with you.

I tolerate your opinion, but it's just that; an opinion.

I think you should treat it as such (an opinion), and not blatantly
disregard the viewpoints of people you disagree with, and worse;
publicly suggest others do the same.

The Git project engages in outreach programs precisely to increase the
diversity of the project, and this interest should not exclude diversity
of thought. Seems counterproductive to allow these kinds of attacks
against a minority opinion. It is in fact the minority opinion that
needs protection, the majority opinion can defend itself.

Cheers.

-- 
Felipe Contreras

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

* Re: Code of conduct violation?
  2020-12-21 23:51                                     ` Code of conduct violation? Felipe Contreras
@ 2020-12-22  7:13                                       ` Junio C Hamano
  2020-12-22  9:58                                         ` Felipe Contreras
  2020-12-22 15:01                                       ` Pratyush Yadav
  2020-12-24 21:00                                       ` Code of conduct violation? David Aguilar
  2 siblings, 1 reply; 70+ messages in thread
From: Junio C Hamano @ 2020-12-22  7:13 UTC (permalink / raw)
  To: Git Mailing List
  Cc: David Aguilar, Seth House, Christian Couder, git, Felipe Contreras

Felipe Contreras <felipe.contreras@gmail.com> writes:

> David Aguilar wrote:
>> Just a heads-up that Felipe has a history on this list of creating
>> long, never-ending, pointless rant threads so sometimes the best
>> course of action is to ignore him.
>
> I think this can be considered a personal attack, which goes against the
> code of conduct.

A PLC member here.

As CoC document outlines in its Enforcement section, the project
leadership committee will review complaints of this kind, and may
issue a statement later, but we do not encourage list participants
to hold People's court publicly on the matter.

Thanks.


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

* Re: Code of conduct violation?
  2020-12-22  7:13                                       ` Junio C Hamano
@ 2020-12-22  9:58                                         ` Felipe Contreras
  0 siblings, 0 replies; 70+ messages in thread
From: Felipe Contreras @ 2020-12-22  9:58 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List
  Cc: David Aguilar, Seth House, Christian Couder, git, Felipe Contreras

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > David Aguilar wrote:
> >> Just a heads-up that Felipe has a history on this list of creating
> >> long, never-ending, pointless rant threads so sometimes the best
> >> course of action is to ignore him.
> >
> > I think this can be considered a personal attack, which goes against the
> > code of conduct.
> 
> A PLC member here.
> 
> As CoC document outlines in its Enforcement section, the project
> leadership committee will review complaints of this kind, and may
> issue a statement later, but we do not encourage list participants
> to hold People's court publicly on the matter.

That's fine.

In real life courts don't have to be open, but accusations and decisions
do.

The public is supposed to learn which sort of behavior is acceptable, and
which not, from the behavior of others, and past decisions. In fact it's
from examples that most people learn the rules, including future judges.

-- 
Felipe Contreras

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

* Re: Code of conduct violation?
  2020-12-21 23:51                                     ` Code of conduct violation? Felipe Contreras
  2020-12-22  7:13                                       ` Junio C Hamano
@ 2020-12-22 15:01                                       ` Pratyush Yadav
  2020-12-23  4:23                                         ` Felipe Contreras
  2020-12-24 21:00                                       ` Code of conduct violation? David Aguilar
  2 siblings, 1 reply; 70+ messages in thread
From: Pratyush Yadav @ 2020-12-22 15:01 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: David Aguilar, Seth House, Junio C Hamano, Git Mailing List,
	Christian Couder, git

Hi Felipe,

On 21/12/20 05:51PM, Felipe Contreras wrote:
> David Aguilar wrote:
> > Just a heads-up that Felipe has a history on this list of creating
> > long, never-ending, pointless rant threads so sometimes the best
> > course of action is to ignore him.

While calling someone out like this is not the right thing at all, it is 
a good opportunity for some introspection. I do not participate in the 
threads you do so I can't say much about these "never-ending, pointless 
rant threads". But if enough people feel this way, maybe you need to 
dial it down a bit. The community needs to do its part in making you and 
everyone else feel welcome. At the same time you need to do your part in 
making contributors, especially the new ones, feel welcome and 
appreciated. Being overly critical can turn developers away from the 
project.

Anyway, those are my 2 cents. Do with them what you will.
 
> I think this can be considered a personal attack, which goes against the
> code of conduct.
> 
> The code of conduct suggests behavior like:
> 
>  * Being respectful of differing viewpoints and experiences
>  * Showing empathy towards other community members
> 
> It seems to me the above comment is an example of the opposite.
> 
> If you feel that my feedback is pointless, then don't engage with it.
> There's no need to put aggravating labels, especially on a public
> setting. Even if most people agree with you.
> 
> I tolerate your opinion, but it's just that; an opinion.
> 
> I think you should treat it as such (an opinion), and not blatantly
> disregard the viewpoints of people you disagree with, and worse;
> publicly suggest others do the same.
> 
> The Git project engages in outreach programs precisely to increase the
> diversity of the project, and this interest should not exclude diversity
> of thought. Seems counterproductive to allow these kinds of attacks
> against a minority opinion. It is in fact the minority opinion that
> needs protection, the majority opinion can defend itself.
> 

-- 
Regards,
Pratyush Yadav

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

* Re: Code of conduct violation?
  2020-12-22 15:01                                       ` Pratyush Yadav
@ 2020-12-23  4:23                                         ` Felipe Contreras
  2020-12-23  5:02                                           ` Junio C Hamano
  0 siblings, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-23  4:23 UTC (permalink / raw)
  To: Pratyush Yadav, Felipe Contreras
  Cc: David Aguilar, Seth House, Junio C Hamano, Git Mailing List,
	Christian Couder, git

Pratyush Yadav wrote:
> While calling someone out like this is not the right thing at all, it is 
> a good opportunity for some introspection. I do not participate in the 
> threads you do so I can't say much about these "never-ending, pointless 
> rant threads". But if enough people feel this way, maybe you need to 
> dial it down a bit.

Dial what down a bit?

My opinion is my opinion.

If they want to discuss my opinion, then we can do so.

If they don't want to discuss my opinion, they can agree to disagree.

But I'm not going to pretend I'm fine with a change I disagree with; I'm
not. Especially when nobody is paying me to do this.

> The community needs to do its part in making you and everyone else
> feel welcome. At the same time you need to do your part in making
> contributors, especially the new ones, feel welcome and appreciated.
> Being overly critical can turn developers away from the project.

Who are you talking about? I'm the one who made the contribution.

Cheers.

-- 
Felipe Contreras

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

* Re: Code of conduct violation?
  2020-12-23  4:23                                         ` Felipe Contreras
@ 2020-12-23  5:02                                           ` Junio C Hamano
  2020-12-23  5:41                                             ` Felipe Contreras
  0 siblings, 1 reply; 70+ messages in thread
From: Junio C Hamano @ 2020-12-23  5:02 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Pratyush Yadav, David Aguilar, Seth House, Git Mailing List,
	Christian Couder, git

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Pratyush Yadav wrote:
> ...
> But I'm not going to pretend I'm fine with a change I disagree with; I'm
> not. Especially when nobody is paying me to do this.

I presume that at this point you are no longer talking about the
inappropriateness of the phrase "never-ending pointless rant".
Perhaps it is time to change the subject line.

If you still are, then please disregard the rest of the message.

>> The community needs to do its part in making you and everyone else
>> feel welcome. At the same time you need to do your part in making
>> contributors, especially the new ones, feel welcome and appreciated.
>> Being overly critical can turn developers away from the project.
>
> Who are you talking about? I'm the one who made the contribution.

What does the "change you disagree with" you mention above refer to?
Changes suggested by reviewers to add per-tool knob?

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

* Re: Code of conduct violation?
  2020-12-23  5:02                                           ` Junio C Hamano
@ 2020-12-23  5:41                                             ` Felipe Contreras
  2020-12-23 15:04                                               ` Nobody is THE one making contribution Junio C Hamano
  0 siblings, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-23  5:41 UTC (permalink / raw)
  To: Junio C Hamano, Felipe Contreras
  Cc: Pratyush Yadav, David Aguilar, Seth House, Git Mailing List,
	Christian Couder, git

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > Pratyush Yadav wrote:
> > ...
> > But I'm not going to pretend I'm fine with a change I disagree with; I'm
> > not. Especially when nobody is paying me to do this.
> 
> I presume that at this point you are no longer talking about the
> inappropriateness of the phrase "never-ending pointless rant".
> Perhaps it is time to change the subject line.

I am responding to what Pratyush said, and he didn't change the subject
line.

I don't know what he meant by "it". I'm just guessing.

> If you still are, then please disregard the rest of the message.
> 
> >> The community needs to do its part in making you and everyone else
> >> feel welcome. At the same time you need to do your part in making
> >> contributors, especially the new ones, feel welcome and appreciated.
> >> Being overly critical can turn developers away from the project.
> >
> > Who are you talking about? I'm the one who made the contribution.
> 
> What does the "change you disagree with" you mention above refer to?
> Changes suggested by reviewers to add per-tool knob?

Yes.

-- 
Felipe Contreras

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

* Nobody is THE one making contribution
  2020-12-23  5:41                                             ` Felipe Contreras
@ 2020-12-23 15:04                                               ` Junio C Hamano
  2020-12-23 15:51                                                 ` Felipe Contreras
  0 siblings, 1 reply; 70+ messages in thread
From: Junio C Hamano @ 2020-12-23 15:04 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Pratyush Yadav, David Aguilar, Seth House, Git Mailing List,
	Christian Couder, git

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> > But I'm not going to pretend I'm fine with a change I disagree with; I'm
>> > not. Especially when nobody is paying me to do this.
>> ...
>> >> The community needs to do its part in making you and everyone else
>> >> feel welcome. At the same time you need to do your part in making
>> >> contributors, especially the new ones, feel welcome and appreciated.
>> >> Being overly critical can turn developers away from the project.
>> >
>> > Who are you talking about? I'm the one who made the contribution.
>> 
>> What does the "change you disagree with" you mention above refer to?
>> Changes suggested by reviewers to add per-tool knob?
>
> Yes.

If so, please realize that this is a team effort.  You are not THE
one making contribution.  Everybody else also is.  You are building
on top of others' code, others are helping you to improve your
patches with their input, and others can and will later build on top
of what you have done.

If you are not fine with a change others will make on top of what
you did, well, tough.  It's not like by sending a patch you lick a
corner of the cake and make it untouchable by others.

Just as you said, you can agree to disagree and move on.  Once a
rough concensus is reached that the work on top of what you did is a
good direction to go, it would not get us anywhere to repeat the
same opinion over and over again to block it.


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

* RE: Nobody is THE one making contribution
  2020-12-23 15:04                                               ` Nobody is THE one making contribution Junio C Hamano
@ 2020-12-23 15:51                                                 ` Felipe Contreras
  2020-12-23 20:56                                                   ` Junio C Hamano
  2020-12-24  2:01                                                   ` Ævar Arnfjörð Bjarmason
  0 siblings, 2 replies; 70+ messages in thread
From: Felipe Contreras @ 2020-12-23 15:51 UTC (permalink / raw)
  To: Junio C Hamano, Felipe Contreras
  Cc: Pratyush Yadav, David Aguilar, Seth House, Git Mailing List,
	Christian Couder, git

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> >> > But I'm not going to pretend I'm fine with a change I disagree with; I'm
> >> > not. Especially when nobody is paying me to do this.
> >> ...
> >> >> The community needs to do its part in making you and everyone else
> >> >> feel welcome. At the same time you need to do your part in making
> >> >> contributors, especially the new ones, feel welcome and appreciated.
> >> >> Being overly critical can turn developers away from the project.
> >> >
> >> > Who are you talking about? I'm the one who made the contribution.
> >> 
> >> What does the "change you disagree with" you mention above refer to?
> >> Changes suggested by reviewers to add per-tool knob?
> >
> > Yes.
> 
> If so, please realize that this is a team effort.

Yes, but different people in a team have different roles. There's the
maintainers, the contributors, the reviewers, the translators, etc.

> You are not THE one making contribution.

When I'm sending a patch, I have the role of "contributor".

That doesn't mean the people revieweing the patch aren't making a
contribution as well, but traditionally they are not the ones being
assigned the role of "contributor".

In your own release notes [1] you say:

  New contributors whose contributions weren't in v2.29.0 are as
  follows.

Presumably these are the people who contributed patches, not reviews.

> If you are not fine with a change others will make on top of what
> you did, well, tough.

Yes, I know.

I don't know why you feel the need to explain that to me. I have been
contributing to open source projects for more than 20 years.

> It's not like by sending a patch you lick a corner of the cake and
> make it untouchable by others.

No, but just because others have an opinion that doesn't mean I must
have the same opinion as others.

Take judicial panels as an analogy. A group of judges must arrive at an
opinion [2], and sometimes they do without problems; all of them agree,
which is called an "unanimous opinion". Sometimes not everyone agrees,
in which case it's called a "majority opinion". When not everyone
agrees, a judge is perfectly entitled to write his or her "dissenting
opinion", which is a disagreement with the "majority opinion".

When there's a dissenting opinion, the court still moves forward with
the majority opinion, the dissenting opinion is written down for the
record.

When I express my dissenting opinion I'm not saying nobody should write
a patch on top of mine. Of course they can. Anybody can take my code and
do whatever they want with it (as long as they don't violate the license
of the project).

What they cannot do is add my Signed-off-by line to code I don't agree
with.

> Just as you said, you can agree to disagree and move on.

Yes, with my disagreement stated for the record. Just like dissenting
opinions in judicial panels.

> Once a rough concensus is reached that the work on top of what you did
> is a good direction to go, it would not get us anywhere to repeat the
> same opinion over and over again to block it.

I am not repeating the same opinion over and over, I am responding to
the comments of others, with an argument they fail to address.

If person A doesn't address my argument, and person B doesn't address my
argument, and person C comes along with comments that warrant the repeat
of the argument; I'm going to repeat the argument.

Nobody has addressed the argument.

It is the fault of person A that didn't address my argument that I had
to repeat it to person B, and so on. Not mine.

Cheers.

[1] https://lore.kernel.org/git/xmqqsg82qur4.fsf@gitster.c.googlers.com/
[2] https://en.wikipedia.org/wiki/Judicial_opinion#Kinds_of_judicial_opinions

-- 
Felipe Contreras

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

* Re: Nobody is THE one making contribution
  2020-12-23 15:51                                                 ` Felipe Contreras
@ 2020-12-23 20:56                                                   ` Junio C Hamano
  2020-12-24  1:09                                                     ` Felipe Contreras
  2020-12-24  2:01                                                   ` Ævar Arnfjörð Bjarmason
  1 sibling, 1 reply; 70+ messages in thread
From: Junio C Hamano @ 2020-12-23 20:56 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Pratyush Yadav, David Aguilar, Seth House, Git Mailing List,
	Christian Couder, git

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> You are not THE one making contribution.
>
> When I'm sending a patch, I have the role of "contributor".

Yes, you are a contributor, but there are other contributors to the
change under discussion.

> In your own release notes [1] you say:
>
>   New contributors whose contributions weren't in v2.29.0 are as
>   follows.
>
> Presumably these are the people who contributed patches, not reviews.

If I said "These community members have their name as an author of a
patch for the first time since v2.29", would that mean those who do
not have any commit under their name are not community members?

> I don't know why you feel the need to explain that to me. I have been
> contributing to open source projects for more than 20 years.

Because you are acting as if you don't know and have to always be
the right one no matter what.  You may not mean to do so, but that
is how your behaviour appears to me (note that I did not know say
"to others").

I won't have time to respond to your word games after I send this
message.

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

* Re: Nobody is THE one making contribution
  2020-12-23 20:56                                                   ` Junio C Hamano
@ 2020-12-24  1:09                                                     ` Felipe Contreras
  0 siblings, 0 replies; 70+ messages in thread
From: Felipe Contreras @ 2020-12-24  1:09 UTC (permalink / raw)
  To: Junio C Hamano, Felipe Contreras
  Cc: Pratyush Yadav, David Aguilar, Seth House, Git Mailing List,
	Christian Couder, git

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> >> You are not THE one making contribution.
> >
> > When I'm sending a patch, I have the role of "contributor".
> 
> Yes, you are a contributor, but there are other contributors to the
> change under discussion.

There are other people _contributing_, but typically they are not
assigned the role of _contributor_.

This has been my experience in all the open source projects I've worked
on.

My guess is that semantically the role of contributor is different
because they are the ones bearing the brunt of the work (thinking the
idea, coding, testing, cleaning, sending the patch, addressing comments,
re-coding, re-testing, re-cleaing, sending another patch... etc).

Of course everyone contributes, but in any given patch series, the
"contributor" contributes the most.

> > In your own release notes [1] you say:
> >
> >   New contributors whose contributions weren't in v2.29.0 are as
> >   follows.
> >
> > Presumably these are the people who contributed patches, not reviews.
> 
> If I said "These community members have their name as an author of a
> patch for the first time since v2.29", would that mean those who do
> not have any commit under their name are not community members?

No. It would just mean they had other roles (not contributor).

For example, when a company pays their employees to contribute to the
project, the company can be considered a contributor (e.g. Google).

> > I don't know why you feel the need to explain that to me. I have been
> > contributing to open source projects for more than 20 years.
> 
> Because you are acting as if you don't know and have to always be
> the right one no matter what.  You may not mean to do so, but that
> is how your behaviour appears to me (note that I did not know say
> "to others").

Yes, but you do not read minds, and you can't know what is happening
inside mine.

Attempting to do so is usually not a good idea [1].

> I won't have time to respond to your word games after I send this
> message.

Words are the only tool we have to communicate among minds.

There's a reason why linguistics is an entire field of study; some
people are trying to really understand what other people are really
trying to say.

I for one don't understand why you would change the subject of the
thread to "Nobody is THE one making contribution", and then not be
interested in understanding what others understand by the word
"contribution".

But I'm not going to read into it something you didn't attempt to say.

I don't read minds.

Cheers.

[1] https://cogbtherapy.com/cbt-blog/common-cognitive-distortions-mind-reading

-- 
Felipe Contreras

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

* Re: Nobody is THE one making contribution
  2020-12-23 15:51                                                 ` Felipe Contreras
  2020-12-23 20:56                                                   ` Junio C Hamano
@ 2020-12-24  2:01                                                   ` Ævar Arnfjörð Bjarmason
  2020-12-24  5:19                                                     ` Felipe Contreras
  1 sibling, 1 reply; 70+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2020-12-24  2:01 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Junio C Hamano, Pratyush Yadav, David Aguilar, Seth House,
	Git Mailing List, Christian Couder, git


On Wed, Dec 23 2020, Felipe Contreras wrote:

> When I express my dissenting opinion I'm not saying nobody should write
> a patch on top of mine. Of course they can. Anybody can take my code and
> do whatever they want with it (as long as they don't violate the license
> of the project).
>
> What they cannot do is add my Signed-off-by line to code I don't agree
> with.

I don't think that's what Signed-off-by means, per SubmittingPatches:

    To improve tracking of who did what, we ask you to certify that you
    wrote the patch or have the right to pass it on under the same
    license as ours, by "signing off" your patch[...under the DCO:
    https://developercertificate.org/]

So I find this rather unlikely, but let's say I author a patch for
git.git and send it to this ML with a Signed-off-by.

If someone else then takes that patch and changes it in a way that I
vehemently disagree with and gets Junio to accept it into git.git in its
altered form, that altered patch should still carry my Signed-off-by, as
well as that of whoever altered it.

Because it's a trailer for maintaining the licensing paper trail. It
doesn't mean "Ævar thinks this patch as it stands is a good idea"[1].

"No Discrimination Against Fields of Endeavor" is an integral part of
free software & open source. In our case it means that when you
contribute code under our COPYING terms someone else might use in a way
you don't approve of.

E.g. I'm sure that arms contractors, totalitarian regimes etc. or other
entities some might disapprove of are using git in some way.

That non-restriction on fields of endeavor also extends to individual
patches licensed under a free software license & the necessity to
maintain a paper trail about who their authors are and if they certified
them under the DCO.

1. As an aside, I haven't needed involvement from others to later
   realize that an integrated patch of mine with my SOB maybe wasn't
   such a good idea after all :)

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

* Re: Nobody is THE one making contribution
  2020-12-24  2:01                                                   ` Ævar Arnfjörð Bjarmason
@ 2020-12-24  5:19                                                     ` Felipe Contreras
  2020-12-24 12:30                                                       ` Ævar Arnfjörð Bjarmason
  2020-12-24 15:09                                                       ` Randall S. Becker
  0 siblings, 2 replies; 70+ messages in thread
From: Felipe Contreras @ 2020-12-24  5:19 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason, Felipe Contreras
  Cc: Junio C Hamano, Pratyush Yadav, David Aguilar, Seth House,
	Git Mailing List, Christian Couder, git

Ævar Arnfjörð Bjarmason wrote:
> On Wed, Dec 23 2020, Felipe Contreras wrote:
> 
> > When I express my dissenting opinion I'm not saying nobody should write
> > a patch on top of mine. Of course they can. Anybody can take my code and
> > do whatever they want with it (as long as they don't violate the license
> > of the project).
> >
> > What they cannot do is add my Signed-off-by line to code I don't agree
> > with.
> 
> I don't think that's what Signed-off-by means, per SubmittingPatches:
> 
>     To improve tracking of who did what, we ask you to certify that you
>     wrote the patch or have the right to pass it on under the same
>     license as ours, by "signing off" your patch[...under the DCO:
>     https://developercertificate.org/]

Yes, but the DCO requires (d):

  d. I understand and agree that this project and the contribution are
     public and that a record of the contribution (including all personal
     information I submit with it, including my sign-off) is maintained
     indefinitely and may be redistributed consistent with this project or
     the open source license(s) involved.

We can narrow down the part I'm talking about:

  d. I *agree* that a record of the contribution is maintained
     indefinitely.

I don't agree with that.

Moreover, the relevant definition of "sign off" in English in my opinion
is [1]:

  to approve or acknowledge something by or as if by a signature (sign
  off on a memo)

If I didn't put my "signature" in a commit, then it's not signed off by
me.

> So I find this rather unlikely, but let's say I author a patch for
> git.git and send it to this ML with a Signed-off-by.
> 
> If someone else then takes that patch and changes it in a way that I
> vehemently disagree with and gets Junio to accept it into git.git in its
> altered form, that altered patch should still carry my Signed-off-by, as
> well as that of whoever altered it.

I don't think so.

Even if you disregard clause (d) of the DCO, in English you didn't "sign
off" on that particular version of the patch.

> "No Discrimination Against Fields of Endeavor" is an integral part of
> free software & open source. In our case it means that when you
> contribute code under our COPYING terms someone else might use in a way
> you don't approve of.

Yes, you just have to make the record straight; do your changes in a
separate commit without my "sign off".

> E.g. I'm sure that arms contractors, totalitarian regimes etc. or other
> entities some might disapprove of are using git in some way.

Yes, and you can modify my patch and keep my s-o-b, I'm not going to sue
you.

I just don't think that's right.

> That non-restriction on fields of endeavor also extends to individual
> patches licensed under a free software license & the necessity to
> maintain a paper trail about who their authors are and if they certified
> them under the DCO.

Sure, so if you need to keep a paper trail about the copyright of the
code, why would you risk that simply because the author didn't agree on
the further changes.

Just do them on a separate commit. Problem solved.

> 1. As an aside, I haven't needed involvement from others to later
>    realize that an integrated patch of mine with my SOB maybe wasn't
>    such a good idea after all :)

Oh, me neither. But in this particular case I'm sure X is wrong, so I'm
stating unequivocally that I think X is wrong.

Plus, in my experience I usually think X is the case, and it's not only
after I roll up my sleeves and get my hands dirty that I realize what's
actually the case.

Changing your mind after you get your hands dirty is usually a good
thing. Which is another reason why I like Linus Torvalds' phrase: "talk
is cheap, show me the code".

Cheers.

[1] https://www.merriam-webster.com/dictionary/sign%20off

-- 
Felipe Contreras

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

* Re: Nobody is THE one making contribution
  2020-12-24  5:19                                                     ` Felipe Contreras
@ 2020-12-24 12:30                                                       ` Ævar Arnfjörð Bjarmason
  2020-12-24 15:26                                                         ` Felipe Contreras
  2020-12-24 15:09                                                       ` Randall S. Becker
  1 sibling, 1 reply; 70+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2020-12-24 12:30 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Junio C Hamano, Pratyush Yadav, David Aguilar, Seth House,
	Git Mailing List, Christian Couder, git


On Thu, Dec 24 2020, Felipe Contreras wrote:

> Ævar Arnfjörð Bjarmason wrote:
>> On Wed, Dec 23 2020, Felipe Contreras wrote:
>> 
>> > When I express my dissenting opinion I'm not saying nobody should write
>> > a patch on top of mine. Of course they can. Anybody can take my code and
>> > do whatever they want with it (as long as they don't violate the license
>> > of the project).
>> >
>> > What they cannot do is add my Signed-off-by line to code I don't agree
>> > with.
>> 
>> I don't think that's what Signed-off-by means, per SubmittingPatches:
>> 
>>     To improve tracking of who did what, we ask you to certify that you
>>     wrote the patch or have the right to pass it on under the same
>>     license as ours, by "signing off" your patch[...under the DCO:
>>     https://developercertificate.org/]
>
> Yes, but the DCO requires (d):
>
>   d. I understand and agree that this project and the contribution are
>      public and that a record of the contribution (including all personal
>      information I submit with it, including my sign-off) is maintained
>      indefinitely and may be redistributed consistent with this project or
>      the open source license(s) involved.
>
> We can narrow down the part I'm talking about:
>
>   d. I *agree* that a record of the contribution is maintained
>      indefinitely.
>
> I don't agree with that.

I don't understand you here. You don't agree that we retain
Signed-off-by lines indefinitely, or just in the case of amended
patches?

> Moreover, the relevant definition of "sign off" in English in my opinion
> is [1]:
>
>   to approve or acknowledge something by or as if by a signature (sign
>   off on a memo)
>
> If I didn't put my "signature" in a commit, then it's not signed off by
> me.

I think this use of 'signed off" makes perfect sense if you interpret
the sign-off to mean "I signed off on the copyright eligibility of this
work for inclusion" which is what I think it means.

Not "I signed off on my subjective approval of this patch & what it's
for etc.", which seems to be closer to your interpretation.

>> So I find this rather unlikely, but let's say I author a patch for
>> git.git and send it to this ML with a Signed-off-by.
>> 
>> If someone else then takes that patch and changes it in a way that I
>> vehemently disagree with and gets Junio to accept it into git.git in its
>> altered form, that altered patch should still carry my Signed-off-by, as
>> well as that of whoever altered it.
>
> I don't think so.
>
> Even if you disregard clause (d) of the DCO, in English you didn't "sign
> off" on that particular version of the patch.

[...addressed below...]

>> "No Discrimination Against Fields of Endeavor" is an integral part of
>> free software & open source. In our case it means that when you
>> contribute code under our COPYING terms someone else might use in a way
>> you don't approve of.
>
> Yes, you just have to make the record straight; do your changes in a
> separate commit without my "sign off".

We like to maintain "make test" passing for every commit, and sometimes
we have patches on the ML with a SOB that don't even compile yet, let
alone pass tests, because they were provided by their authors as "maybe
try this" or other near-pseudocode.

We also like to optimize patch order/size/splits/etc. for the benefit of
reviewers. Sometimes someone might send a patch with a SOB that's better
squashed into another one, or refactored into N commits spread across a
series etc.

>> E.g. I'm sure that arms contractors, totalitarian regimes etc. or other
>> entities some might disapprove of are using git in some way.
>
> Yes, and you can modify my patch and keep my s-o-b, I'm not going to sue
> you.
>
> I just don't think that's right.
>
>> That non-restriction on fields of endeavor also extends to individual
>> patches licensed under a free software license & the necessity to
>> maintain a paper trail about who their authors are and if they certified
>> them under the DCO.
>
> Sure, so if you need to keep a paper trail about the copyright of the
> code, why would you risk that simply because the author didn't agree on
> the further changes.
>
> Just do them on a separate commit. Problem solved.

I don't understand how the copyright paper trail is at risk just because
we combine N patches into one.

The important part is that we have a declaration that the sum of the
work (and whatever it's derived from) is properly licensed, that the
authors had the right to license it for inclusion etc.

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

* RE: Nobody is THE one making contribution
  2020-12-24  5:19                                                     ` Felipe Contreras
  2020-12-24 12:30                                                       ` Ævar Arnfjörð Bjarmason
@ 2020-12-24 15:09                                                       ` Randall S. Becker
  2020-12-24 15:37                                                         ` Felipe Contreras
  1 sibling, 1 reply; 70+ messages in thread
From: Randall S. Becker @ 2020-12-24 15:09 UTC (permalink / raw)
  To: 'Felipe Contreras',
	'Ævar Arnfjörð Bjarmason'
  Cc: 'Junio C Hamano', 'Pratyush Yadav',
	'David Aguilar', 'Seth House',
	'Git Mailing List', 'Christian Couder',
	git

On December 24, 2020 12:19 AM, Felipe Contreras wrote:
> To: Ævar Arnfjörð Bjarmason <avarab@gmail.com>; Felipe Contreras
> <felipe.contreras@gmail.com>
> Cc: Junio C Hamano <gitster@pobox.com>; Pratyush Yadav
> <me@yadavpratyush.com>; David Aguilar <davvid@gmail.com>; Seth House
> <seth@eseth.com>; Git Mailing List <git@vger.kernel.org>; Christian Couder
> <christian.couder@gmail.com>; git@sfconservancy.org
> Subject: Re: Nobody is THE one making contribution
> 
> Ævar Arnfjörð Bjarmason wrote:
> > On Wed, Dec 23 2020, Felipe Contreras wrote:
> >
> > > When I express my dissenting opinion I'm not saying nobody should
> > > write a patch on top of mine. Of course they can. Anybody can take
> > > my code and do whatever they want with it (as long as they don't
> > > violate the license of the project).
> > >
> > > What they cannot do is add my Signed-off-by line to code I don't
> > > agree with.
> >
> > I don't think that's what Signed-off-by means, per SubmittingPatches:
> >
> >     To improve tracking of who did what, we ask you to certify that you
> >     wrote the patch or have the right to pass it on under the same
> >     license as ours, by "signing off" your patch[...under the DCO:
> >     https://developercertificate.org/]
> 
> Yes, but the DCO requires (d):
> 
>   d. I understand and agree that this project and the contribution are
>      public and that a record of the contribution (including all personal
>      information I submit with it, including my sign-off) is maintained
>      indefinitely and may be redistributed consistent with this project or
>      the open source license(s) involved.
> 
> We can narrow down the part I'm talking about:
> 
>   d. I *agree* that a record of the contribution is maintained
>      indefinitely.
> 
> I don't agree with that.

Clause d is important to maintain compatibility with GRPD[1] rules about maintaining identifying information. This clause is more than about the contribution. It is about consent to maintain your name and email on record indefinitely, as part of the contribution, in the git repository, without the ability to rescind the permission at some point in the future.

Sincerely,
Randall

[1] https://eur-lex.europa.eu/eli/reg/2016/679/oj




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

* Re: Nobody is THE one making contribution
  2020-12-24 12:30                                                       ` Ævar Arnfjörð Bjarmason
@ 2020-12-24 15:26                                                         ` Felipe Contreras
  2020-12-24 22:57                                                           ` Junio C Hamano
  0 siblings, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-24 15:26 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason, Felipe Contreras
  Cc: Junio C Hamano, Pratyush Yadav, David Aguilar, Seth House,
	Git Mailing List, Christian Couder, git

Ævar Arnfjörð Bjarmason wrote:
> On Thu, Dec 24 2020, Felipe Contreras wrote:
> 
> > Ævar Arnfjörð Bjarmason wrote:
> >> On Wed, Dec 23 2020, Felipe Contreras wrote:
> >> 
> >> > When I express my dissenting opinion I'm not saying nobody should write
> >> > a patch on top of mine. Of course they can. Anybody can take my code and
> >> > do whatever they want with it (as long as they don't violate the license
> >> > of the project).
> >> >
> >> > What they cannot do is add my Signed-off-by line to code I don't agree
> >> > with.
> >> 
> >> I don't think that's what Signed-off-by means, per SubmittingPatches:
> >> 
> >>     To improve tracking of who did what, we ask you to certify that you
> >>     wrote the patch or have the right to pass it on under the same
> >>     license as ours, by "signing off" your patch[...under the DCO:
> >>     https://developercertificate.org/]
> >
> > Yes, but the DCO requires (d):
> >
> >   d. I understand and agree that this project and the contribution are
> >      public and that a record of the contribution (including all personal
> >      information I submit with it, including my sign-off) is maintained
> >      indefinitely and may be redistributed consistent with this project or
> >      the open source license(s) involved.
> >
> > We can narrow down the part I'm talking about:
> >
> >   d. I *agree* that a record of the contribution is maintained
> >      indefinitely.
> >
> > I don't agree with that.
> 
> I don't understand you here. You don't agree that we retain
> Signed-off-by lines indefinitely, or just in the case of amended
> patches?

The DCO requires that I agree that a record of my contribution is
maintained indefinitely.

If I don't agree that a record of a particular contribution is
maintained indefinitely, the DCO says you shouldn't use it.

> > Moreover, the relevant definition of "sign off" in English in my opinion
> > is [1]:
> >
> >   to approve or acknowledge something by or as if by a signature (sign
> >   off on a memo)
> >
> > If I didn't put my "signature" in a commit, then it's not signed off by
> > me.
> 
> I think this use of 'signed off" makes perfect sense if you interpret
> the sign-off to mean "I signed off on the copyright eligibility of this
> work for inclusion" which is what I think it means.
> 
> Not "I signed off on my subjective approval of this patch & what it's
> for etc.", which seems to be closer to your interpretation.

Why does it have to be only one meaning?

Junio doesn't sign off on a patch that he doesn't think is good.

Same happens with all the lieutenants of Linux.

> >> "No Discrimination Against Fields of Endeavor" is an integral part of
> >> free software & open source. In our case it means that when you
> >> contribute code under our COPYING terms someone else might use in a way
> >> you don't approve of.
> >
> > Yes, you just have to make the record straight; do your changes in a
> > separate commit without my "sign off".
> 
> We like to maintain "make test" passing for every commit, and sometimes
> we have patches on the ML with a SOB that don't even compile yet, let
> alone pass tests, because they were provided by their authors as "maybe
> try this" or other near-pseudocode.
> 
> We also like to optimize patch order/size/splits/etc. for the benefit of
> reviewers. Sometimes someone might send a patch with a SOB that's better
> squashed into another one, or refactored into N commits spread across a
> series etc.

Yes. And most of the time that's fine, because the original author is
not objecting to the clause (d).

> >> E.g. I'm sure that arms contractors, totalitarian regimes etc. or other
> >> entities some might disapprove of are using git in some way.
> >
> > Yes, and you can modify my patch and keep my s-o-b, I'm not going to sue
> > you.
> >
> > I just don't think that's right.
> >
> >> That non-restriction on fields of endeavor also extends to individual
> >> patches licensed under a free software license & the necessity to
> >> maintain a paper trail about who their authors are and if they certified
> >> them under the DCO.
> >
> > Sure, so if you need to keep a paper trail about the copyright of the
> > code, why would you risk that simply because the author didn't agree on
> > the further changes.
> >
> > Just do them on a separate commit. Problem solved.
> 
> I don't understand how the copyright paper trail is at risk just because
> we combine N patches into one.

It's not just a copyright paper trail, the DCO clearly states that the
author should:

  d. I *agree* that a record of the contribution is maintained
     indefinitely.

> The important part is that we have a declaration that the sum of the
> work (and whatever it's derived from) is properly licensed, that the
> authors had the right to license it for inclusion etc.

That's the important part, yes. It's not the only part.

Cheers.

-- 
Felipe Contreras

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

* RE: Nobody is THE one making contribution
  2020-12-24 15:09                                                       ` Randall S. Becker
@ 2020-12-24 15:37                                                         ` Felipe Contreras
  2020-12-24 22:40                                                           ` Junio C Hamano
  0 siblings, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-24 15:37 UTC (permalink / raw)
  To: Randall S. Becker, 'Felipe Contreras',
	'Ævar Arnfjörð Bjarmason'
  Cc: 'Junio C Hamano', 'Pratyush Yadav',
	'David Aguilar', 'Seth House',
	'Git Mailing List', 'Christian Couder',
	git

Randall S. Becker wrote:
> On December 24, 2020 12:19 AM, Felipe Contreras wrote:
> > We can narrow down the part I'm talking about:
> > 
> >   d. I *agree* that a record of the contribution is maintained
> >      indefinitely.
> > 
> > I don't agree with that.
> 
> Clause d is important to maintain compatibility with GRPD[1] rules
> about maintaining identifying information. This clause is more than
> about the contribution. It is about consent to maintain your name and
> email on record indefinitely, as part of the contribution, in the git
> repository, without the ability to rescind the permission at some
> point in the future.

I didn't mean I don't agree that clause (d) should be there.

I mean if in a particular contribution I don't agree that a record of
the contribution is maintained indefinitely with my name, then clause
(d) is not met. And it is not actually my true contribution, but a
bastardization of it made by somebody else.

You shouldn't assign to my name changes I don't agree with in
perpetuity.

-- 
Felipe Contreras

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

* Re: Code of conduct violation?
  2020-12-21 23:51                                     ` Code of conduct violation? Felipe Contreras
  2020-12-22  7:13                                       ` Junio C Hamano
  2020-12-22 15:01                                       ` Pratyush Yadav
@ 2020-12-24 21:00                                       ` David Aguilar
  2020-12-24 22:32                                         ` Felipe Contreras
  2 siblings, 1 reply; 70+ messages in thread
From: David Aguilar @ 2020-12-24 21:00 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Seth House, Junio C Hamano, Git Mailing List, Christian Couder, git

On Mon, Dec 21, 2020 at 3:51 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> David Aguilar wrote:
> > Just a heads-up that Felipe has a history on this list of creating
> > long, never-ending, pointless rant threads so sometimes the best
> > course of action is to ignore him.
>
> I think this can be considered a personal attack, which goes against the
> code of conduct.


Sorry Felipe -- there is no explanation that will satisfy you and this
is not the place to try and "make a case" for what was said. The
downsides of further engagement and messages that did not need to be
exchanged has already played itself out.

Please take this apology so that you can have the last word.  I am not
going to try and convince anyone that my response was appropriate.



> The code of conduct suggests behavior like:
>
>  * Being respectful of differing viewpoints and experiences
>  * Showing empathy towards other community members
>
> It seems to me the above comment is an example of the opposite.
>
> If you feel that my feedback is pointless, then don't engage with it.
> There's no need to put aggravating labels, especially on a public
> setting. Even if most people agree with you.
>
> I tolerate your opinion, but it's just that; an opinion.
>
> I think you should treat it as such (an opinion), and not blatantly
> disregard the viewpoints of people you disagree with, and worse;
> publicly suggest others do the same.


I could try and explain that being intolerant of my and others'
viewpoints and dismissing them as being inadequately defended or
non-existent is ironic, but arguing is a pointless endeavor.  I gave
you an honest, but inappropriate, response in public because being
honest and candid is a form of empathy, but that doesn't make it okay.

I appreciate your talents and tenacity. I can't ask you to be less
abrasive.  I can't ask you to not ignore the opinions of others.  All
I can offer you is my respect and an apology.

Sorry for saying that.

Much respect to you Felipe.  In respect for you, I will politely
refrain from responding to this thread.

Sometimes the best arguments are the ones that are not made.
-- 
David

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

* Re: Code of conduct violation?
  2020-12-24 21:00                                       ` Code of conduct violation? David Aguilar
@ 2020-12-24 22:32                                         ` Felipe Contreras
  0 siblings, 0 replies; 70+ messages in thread
From: Felipe Contreras @ 2020-12-24 22:32 UTC (permalink / raw)
  To: David Aguilar, Felipe Contreras
  Cc: Seth House, Junio C Hamano, Git Mailing List, Christian Couder, git

David Aguilar wrote:
> On Mon, Dec 21, 2020 at 3:51 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > David Aguilar wrote:
> > > Just a heads-up that Felipe has a history on this list of creating
> > > long, never-ending, pointless rant threads so sometimes the best
> > > course of action is to ignore him.
> >
> > I think this can be considered a personal attack, which goes against the
> > code of conduct.
> 
> Sorry Felipe -- there is no explanation that will satisfy you and this
> is not the place to try and "make a case" for what was said.

I'm not looking for an explanation from you. I'm just wondering if others
consider it a violation of the code of conduct.

> > The code of conduct suggests behavior like:
> >
> >  * Being respectful of differing viewpoints and experiences
> >  * Showing empathy towards other community members
> >
> > It seems to me the above comment is an example of the opposite.
> >
> > If you feel that my feedback is pointless, then don't engage with it.
> > There's no need to put aggravating labels, especially on a public
> > setting. Even if most people agree with you.
> >
> > I tolerate your opinion, but it's just that; an opinion.
> >
> > I think you should treat it as such (an opinion), and not blatantly
> > disregard the viewpoints of people you disagree with, and worse;
> > publicly suggest others do the same.
> 
> I could try and explain that being intolerant of my and others'
> viewpoints and dismissing them as being inadequately defended or
> non-existent is ironic, but arguing is a pointless endeavor.

I tolerate your viewpoint, at no point did I ever a attempt to silence
you, or urged others to ignore it.

In fact, I did the opposite; I engaged with you and replied to you more
than once [1] (you did not reply back).

I welcome your viewpoint. I just disagree with it.

> I gave you an honest, but inappropriate, response in public because
> being honest and candid is a form of empathy, but that doesn't make it
> okay.

Your response was not addressed to me.

I don't doubt your response was honest, I'm just think it was
innaproriate in a public forum. I'm glad you acknowledge that.

> I appreciate your talents and tenacity. I can't ask you to be less
> abrasive.

Thank you. Same goes from me.

> I can't ask you to not ignore the opinions of others.

Once again: I did not ignore your opinion, my response is sill waiting
your response [1].

You are free to keep defending your opinion. In fact, I would appreciate
so.

> All I can offer you is my respect and an apology.
> 
> Sorry for saying that.

Thank you. No hard feelings.

> Much respect to you Felipe.  In respect for you, I will politely
> refrain from responding to this thread.
> 
> Sometimes the best arguments are the ones that are not made.

I think arguments should be stated aloud.

I always welcome your arguments, and if you honestly think my point of
view doesn't make sense, you are completely free to state so candidly.
Just remember that I'm also entitled to have own opinion in disagreement
of yours.

Best wishes.

[1] https://lore.kernel.org/git/5fdffe7f7fbd_89f120860@natae.notmuch/

-- 
Felipe Contreras

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

* Re: Nobody is THE one making contribution
  2020-12-24 15:37                                                         ` Felipe Contreras
@ 2020-12-24 22:40                                                           ` Junio C Hamano
  0 siblings, 0 replies; 70+ messages in thread
From: Junio C Hamano @ 2020-12-24 22:40 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Randall S. Becker,
	'Ævar Arnfjörð Bjarmason',
	'Pratyush Yadav', 'David Aguilar',
	'Seth House', 'Git Mailing List',
	'Christian Couder',
	git

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Randall S. Becker wrote:
>> On December 24, 2020 12:19 AM, Felipe Contreras wrote:
>> > We can narrow down the part I'm talking about:
>> > 
>> >   d. I *agree* that a record of the contribution is maintained
>> >      indefinitely.
>> > 
>> > I don't agree with that.
>> 
>> Clause d is important to maintain compatibility with GRPD[1] rules
>> about maintaining identifying information. This clause is more than
>> about the contribution. It is about consent to maintain your name and
>> email on record indefinitely, as part of the contribution, in the git
>> repository, without the ability to rescind the permission at some
>> point in the future.
>
> I didn't mean I don't agree that clause (d) should be there.
>
> I mean if in a particular contribution I don't agree that a record of
> the contribution is maintained indefinitely with my name, then clause
> (d) is not met. And it is not actually my true contribution, but a
> bastardization of it made by somebody else.
>
> You shouldn't assign to my name changes I don't agree with in
> perpetuity.

The way I read our DCO, especially its part (b), is that it is very
much designed to allow editing, tweaking and improving on others'
patches with editor's sign-off while passing a patch around.



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

* Re: Nobody is THE one making contribution
  2020-12-24 15:26                                                         ` Felipe Contreras
@ 2020-12-24 22:57                                                           ` Junio C Hamano
  2020-12-27 17:29                                                             ` Felipe Contreras
  0 siblings, 1 reply; 70+ messages in thread
From: Junio C Hamano @ 2020-12-24 22:57 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Ævar Arnfjörð Bjarmason, Pratyush Yadav,
	David Aguilar, Seth House, Git Mailing List, Christian Couder,
	git

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> Not "I signed off on my subjective approval of this patch & what it's
>> for etc.", which seems to be closer to your interpretation.
>
> Why does it have to be only one meaning?
>
> Junio doesn't sign off on a patch that he doesn't think is good.
> Same happens with all the lieutenants of Linux.

I know some people try to indicate that a patch is not ready for
inclusion as-is by omitting sign-off.  That way, it would not be
enough for the maintainer to pick it up and add his or her own
sign-off---the maintainer has to go back to the author and ask it to
be signed off (which happens).

But I try not to use the lack of sign-off on my patches to mean
anything, because the technique does not work for me.  A patch I
sent without my sign-off may later turn out to be worth keeping, and
when I run "git am -s" on it, I'd have a full sign-off chain anyway.
There is no "going back to the author and ask" necessary.

In any case, I am not sure what you are trying to achieve by
mentioning the cases where patches are not signed-off.

The reason why some patches do not carry sign-off might be because
the sender does not wish to certify and that's OK.  But if you are
arguing that when you write "Signed-off-by:" your sign-off can mean
something other than what DCO says it means, what those people who
sometimes do not sign-off their work do would not be useful at all
to support your claim, I would have to say.  You need to find other
people's signed-off patch when they did not mean to certify them
under DCO, if "it does not have to be only one meaning" is what you
want to support.

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

* Re: Nobody is THE one making contribution
  2020-12-24 22:57                                                           ` Junio C Hamano
@ 2020-12-27 17:29                                                             ` Felipe Contreras
  2020-12-27 18:30                                                               ` Junio C Hamano
  0 siblings, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-27 17:29 UTC (permalink / raw)
  To: Junio C Hamano, Felipe Contreras
  Cc: Ævar Arnfjörð Bjarmason, Pratyush Yadav,
	David Aguilar, Seth House, Git Mailing List, Christian Couder,
	git

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> >> Not "I signed off on my subjective approval of this patch & what it's
> >> for etc.", which seems to be closer to your interpretation.
> >
> > Why does it have to be only one meaning?
> >
> > Junio doesn't sign off on a patch that he doesn't think is good.
> > Same happens with all the lieutenants of Linux.

> The reason why some patches do not carry sign-off might be because
> the sender does not wish to certify and that's OK.

That's one reason.

> But if you are arguing that when you write "Signed-off-by:" your
> sign-off can mean something other than what DCO says it means,

The DCO has clause (d), which clearly states the developer must agree
that a record of his/her contribution is maintained indefinitely (and
that includes his/her sign off).

So there's at least two meanings in the DCO itself.

Additionally, that's the meaning of the phrase "sign off" in English; I
approve of this.

-- 
Felipe Contreras

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

* Re: Nobody is THE one making contribution
  2020-12-27 17:29                                                             ` Felipe Contreras
@ 2020-12-27 18:30                                                               ` Junio C Hamano
  2020-12-27 18:47                                                                 ` Felipe Contreras
  0 siblings, 1 reply; 70+ messages in thread
From: Junio C Hamano @ 2020-12-27 18:30 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Ævar Arnfjörð Bjarmason, Pratyush Yadav,
	David Aguilar, Seth House, Git Mailing List, Christian Couder,
	git

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> But if you are arguing that when you write "Signed-off-by:" your
>> sign-off can mean something other than what DCO says it means,
>
> The DCO has clause (d), which clearly states the developer must agree
> that a record of his/her contribution is maintained indefinitely (and
> that includes his/her sign off).

Yes.  Are you saying that you are OK with (a)-(c) but not (d)?

> So there's at least two meanings in the DCO itself.

I am not sure what you mean.  DCO itself has a to d (four) clauses,
so that is not where your two comes from (unless you are hedging by
saying "at least two").  What are you counting to reach two?

Still puzzled.

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

* Re: Nobody is THE one making contribution
  2020-12-27 18:30                                                               ` Junio C Hamano
@ 2020-12-27 18:47                                                                 ` Felipe Contreras
  2020-12-28 10:39                                                                   ` Junio C Hamano
  0 siblings, 1 reply; 70+ messages in thread
From: Felipe Contreras @ 2020-12-27 18:47 UTC (permalink / raw)
  To: Junio C Hamano, Felipe Contreras
  Cc: Ævar Arnfjörð Bjarmason, Pratyush Yadav,
	David Aguilar, Seth House, Git Mailing List, Christian Couder,
	git

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> >> But if you are arguing that when you write "Signed-off-by:" your
> >> sign-off can mean something other than what DCO says it means,
> >
> > The DCO has clause (d), which clearly states the developer must agree
> > that a record of his/her contribution is maintained indefinitely (and
> > that includes his/her sign off).
> 
> Yes.  Are you saying that you are OK with (a)-(c) but not (d)?

I'm saying if the author of the patch states "I don't agree with a
record of my contribution being maintained indefinitely with my sign
off", then clause (d) isn't met.

-- 
Felipe Contreras

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

* Re: Nobody is THE one making contribution
  2020-12-27 18:47                                                                 ` Felipe Contreras
@ 2020-12-28 10:39                                                                   ` Junio C Hamano
  2020-12-28 14:27                                                                     ` Felipe Contreras
  0 siblings, 1 reply; 70+ messages in thread
From: Junio C Hamano @ 2020-12-28 10:39 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Ævar Arnfjörð Bjarmason, Pratyush Yadav,
	David Aguilar, Seth House, Git Mailing List, Christian Couder,
	git

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Junio C Hamano wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>> 
>> >> But if you are arguing that when you write "Signed-off-by:" your
>> >> sign-off can mean something other than what DCO says it means,
>> >
>> > The DCO has clause (d), which clearly states the developer must agree
>> > that a record of his/her contribution is maintained indefinitely (and
>> > that includes his/her sign off).
>> 
>> Yes.  Are you saying that you are OK with (a)-(c) but not (d)?
>
> I'm saying if the author of the patch states "I don't agree with a
> record of my contribution being maintained indefinitely with my sign
> off", then clause (d) isn't met.

Yeah, but then why does such an author add Signed-off-by: trailer to
begin with?  Here is what Documentation/SubmittingPatches tells the
authors:

    === Certify your work by adding your `Signed-off-by` trailer

    To improve tracking of who did what, we ask you to certify that you
    wrote the patch or have the right to pass it on under the same license
    as ours, by "signing off" your patch.  Without sign-off, we cannot
    accept your patches.

    If you can certify the below D-C-O:

    [[dco]]
    .Developer's Certificate of Origin 1.1
    ____
    By making a contribution to this project, I certify that:

    a. ...; or
    b. ...; or
    c. The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

    d. I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.
    ____

    you add a "Signed-off-by" trailer to your commit, that looks like
    this:

So, "by making a contribution", the author who added a Signed-off-by
trailer is certifying that (a|b|c)&d is true.

Perhaps we can tighten the language to say "If (and only if) you can
certify" and that may reduce confusion?

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

* Re: Nobody is THE one making contribution
  2020-12-28 10:39                                                                   ` Junio C Hamano
@ 2020-12-28 14:27                                                                     ` Felipe Contreras
  0 siblings, 0 replies; 70+ messages in thread
From: Felipe Contreras @ 2020-12-28 14:27 UTC (permalink / raw)
  To: Junio C Hamano, Felipe Contreras
  Cc: Ævar Arnfjörð Bjarmason, Pratyush Yadav,
	David Aguilar, Seth House, Git Mailing List, Christian Couder,
	git

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > Junio C Hamano wrote:
> >> Felipe Contreras <felipe.contreras@gmail.com> writes:
> >> 
> >> >> But if you are arguing that when you write "Signed-off-by:" your
> >> >> sign-off can mean something other than what DCO says it means,
> >> >
> >> > The DCO has clause (d), which clearly states the developer must agree
> >> > that a record of his/her contribution is maintained indefinitely (and
> >> > that includes his/her sign off).
> >> 
> >> Yes.  Are you saying that you are OK with (a)-(c) but not (d)?
> >
> > I'm saying if the author of the patch states "I don't agree with a
> > record of my contribution being maintained indefinitely with my sign
> > off", then clause (d) isn't met.
> 
> Yeah, but then why does such an author add Signed-off-by: trailer to
> begin with?

Why an author does anything is not something even the greatest
psychologist of all time would know with 100% certainty, unless he/she
reads minds, which nobody can do.

All we know is what the author does.

> So, "by making a contribution", the author who added a Signed-off-by
> trailer is certifying that (a|b|c)&d is true.

Yes, (d) is a requirement.

But agreeing (d) applies for patch A is not agreeing that it applies for
patch B.

Apples are not oranges.

> Perhaps we can tighten the language to say "If (and only if) you can
> certify" and that may reduce confusion?

Clause (d) still must apply.

-- 
Felipe Contreras

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

end of thread, other threads:[~2020-12-28 14:28 UTC | newest]

Thread overview: 70+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-16 17:43 [RFC/PATCH] mergetool: use resolved conflicts in all the views Felipe Contreras
2020-12-16 22:24 ` Junio C Hamano
2020-12-16 22:53   ` Seth House
2020-12-17  5:18     ` Junio C Hamano
2020-12-17  5:41     ` Felipe Contreras
2020-12-17  7:35       ` Johannes Sixt
2020-12-17  8:27         ` Felipe Contreras
2020-12-17 19:23           ` Johannes Sixt
2020-12-18  2:30             ` Felipe Contreras
2020-12-17  9:44         ` Seth House
2020-12-17 10:35           ` Felipe Contreras
2020-12-17 17:50             ` Seth House
2020-12-17 19:28               ` Junio C Hamano
2020-12-18  2:34                 ` Felipe Contreras
     [not found]                   ` <CANiSa6jMXTyfo43bUdC8601BvYKiF67HXo+QaiTh_-8KWyBsLg@mail.gmail.com>
2020-12-21  0:31                     ` Felipe Contreras
2020-12-18  2:05               ` Felipe Contreras
2020-12-18  2:35                 ` Seth House
2020-12-18  2:49                   ` Felipe Contreras
2020-12-18  5:49                     ` Seth House
2020-12-18  9:46                       ` Felipe Contreras
2020-12-19  0:13                         ` Seth House
2020-12-19  0:53                           ` Felipe Contreras
2020-12-19 11:14                           ` Junio C Hamano
2020-12-19 12:08                             ` Felipe Contreras
2020-12-19 18:26                               ` Junio C Hamano
2020-12-19 20:18                                 ` Felipe Contreras
2020-12-21  4:25                             ` Seth House
2020-12-21  5:34                               ` Felipe Contreras
2020-12-21  7:36                                 ` Seth House
2020-12-21 11:17                                   ` Felipe Contreras
2020-12-21 22:15                                   ` David Aguilar
2020-12-21 23:51                                     ` Code of conduct violation? Felipe Contreras
2020-12-22  7:13                                       ` Junio C Hamano
2020-12-22  9:58                                         ` Felipe Contreras
2020-12-22 15:01                                       ` Pratyush Yadav
2020-12-23  4:23                                         ` Felipe Contreras
2020-12-23  5:02                                           ` Junio C Hamano
2020-12-23  5:41                                             ` Felipe Contreras
2020-12-23 15:04                                               ` Nobody is THE one making contribution Junio C Hamano
2020-12-23 15:51                                                 ` Felipe Contreras
2020-12-23 20:56                                                   ` Junio C Hamano
2020-12-24  1:09                                                     ` Felipe Contreras
2020-12-24  2:01                                                   ` Ævar Arnfjörð Bjarmason
2020-12-24  5:19                                                     ` Felipe Contreras
2020-12-24 12:30                                                       ` Ævar Arnfjörð Bjarmason
2020-12-24 15:26                                                         ` Felipe Contreras
2020-12-24 22:57                                                           ` Junio C Hamano
2020-12-27 17:29                                                             ` Felipe Contreras
2020-12-27 18:30                                                               ` Junio C Hamano
2020-12-27 18:47                                                                 ` Felipe Contreras
2020-12-28 10:39                                                                   ` Junio C Hamano
2020-12-28 14:27                                                                     ` Felipe Contreras
2020-12-24 15:09                                                       ` Randall S. Becker
2020-12-24 15:37                                                         ` Felipe Contreras
2020-12-24 22:40                                                           ` Junio C Hamano
2020-12-24 21:00                                       ` Code of conduct violation? David Aguilar
2020-12-24 22:32                                         ` Felipe Contreras
2020-12-18 10:04                       ` [RFC/PATCH] mergetool: use resolved conflicts in all the views Junio C Hamano
2020-12-18 11:58                         ` Felipe Contreras
2020-12-19 18:52                           ` Junio C Hamano
2020-12-19 20:59                             ` Felipe Contreras
2020-12-20  6:44                               ` David Aguilar
2020-12-20  7:53                                 ` Felipe Contreras
2020-12-20 22:22                                   ` David Aguilar
2020-12-21  1:46                                     ` Felipe Contreras
2020-12-19  0:18                         ` Seth House
2020-12-16 23:41   ` Felipe Contreras
2020-12-17  5:19     ` Junio C Hamano
2020-12-17  5:43       ` Felipe Contreras
2020-12-17  2:35 ` [RFC/PATCH v2] " Felipe Contreras

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.