[PATCH] mergetool: do not enable hideResolved by default

[Jonathan Nieder <jrnieder@gmail.com>]

A typical mergetool uses four panes, showing the content of the file
being resolved from MERGE_BASE ('BASE'), HEAD ('LOCAL'), MERGE_HEAD
('REMOTE'), and the working copy.  This allows understanding the
conflicts in context: by seeing the entire content of the file from
MERGE_HEAD, say, we can see the full intent of the code we are pulling
in and understand what they were trying to do that conflicted with our
own changes.

Sometimes, though, the exact content of these three competing versions
of a file is not so important.  Especially if the mergetool supports
folding unchanged lines, the new 'mergetool.hideResolved' feature can
be helpful for allowing a person resolving a merge to focus on the
portion with conflicts.  For sections of the file where BASE matched
LOCAL or REMOTE, this feature makes all three versions match the
resolved version, so that the user resolving can focus exclusively on
the portions with conflicts.  In other words, hideResolved makes a
multi-pane merge tool show a similar amount of information to the file
with conflict markers with conflictstyle=diff3, saving the operator
from having to pay attention to parts that resolved cleanly.

98ea309b3f (mergetool: add hideResolved configuration, 2021-02-09)
which introduced this setting enabled it by default, explaining:

    No adverse effects were noted in a small survey of popular mergetools[1]
    so this behavior defaults to `true`. However it can be globally disabled
    by setting `mergetool.hideResolved` to `false`.

In practice, however, this has proved confusing for users.  No
indication is shown in the UI that the base, local, and remote
versions shown have been modified by additional resolution.
Especially in cases where conflicts involve elements beyond textual
conflict, it has resulted in incorrect resolutions and wasted work to
figure out what happened.  Flip the default back to the traditional
behavior of `false`: although the old behavior involves slightly
slower merges in the only-textual-conflicts case, it prevents this
kind of painful moment of betrayal by one's tools, which is more
important.

Should we want to migrate to hideResolved=true in the future, we still
can.  It just requires a more careful migration, including a period
where "git mergetool" shows a warning or errors out in affected cases.

Reported-by: Dana Dahlstrom <dahlstrom@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Hi,

Seth House wrote:

> No adverse effects were noted in a small survey of popular mergetools[1]
> so this behavior defaults to `true`. However it can be globally disabled
> by setting `mergetool.hideResolved` to `false`.

Thanks much for protecting this by a flag.  We tried this out
internally at Google when it hit "next" and not too long later
realized that the new default of "true" is not workable for us.  I
don't believe it's the right default for Git, either, hence this
patch.

Thanks for working on the merge resolution workflow; it's much
appreciated.

Sincerely,
Jonathan

 Documentation/config/mergetool.txt | 2 +-
 git-mergetool.sh                   | 9 ++-------
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt
index 90f76f5b9b..cafbbef46a 100644
--- a/Documentation/config/mergetool.txt
+++ b/Documentation/config/mergetool.txt
@@ -53,7 +53,7 @@ mergetool.hideResolved::
 	resolution. This flag causes 'LOCAL' and 'REMOTE' to be overwriten so
 	that only the unresolved conflicts are presented to the merge tool. Can
 	be configured per-tool via the `mergetool.<tool>.hideResolved`
-	configuration variable. Defaults to `true`.
+	configuration variable. Defaults to `false`.
 
 mergetool.keepBackup::
 	After performing a merge, the original file with conflict markers
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 911470a5b2..f751d9cfe2 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -358,13 +358,8 @@ merge_file () {
 		    enabled=false
 		fi
 	else
-		# The user does not have a preference. Ask the tool.
-		if hide_resolved_enabled
-		then
-		    enabled=true
-		else
-		    enabled=false
-		fi
+		# The user does not have a preference. Default to disabled.
+		enabled=false
 	fi
 
 	if test "$enabled" = true
-- 
2.31.0.rc1.246.gcd05c9c855