Re: [PATCH 1/2] mergetool: honor mergetool.$tool.trustExitCode for built-in tools

David Aguilar [off-list ref] writes:

Built-in merge tools contain a hard-coded assumption about
whether or not a tool's exit code can be trusted to determine
the success or failure of a merge.  Tools whose exit codes are
not trusted contain calls to check_unchanged() in their
merge_cmd() functions.

A problem with this is that the trustExitCode configuration is
not honored for built-in tools.

Teach built-in tools to honor the trustExitCode configuration.
Extend run_merge_cmd() so that it is responsible for calling
check_unchanged() when a tool's exit code cannot be trusted.
Remove check_unchanged() calls from scriptlets since they are no
longer responsible for calling it.

When no configuration is present, exit_code_trustable() is
checked to see whether the exit code should be trusted.
The default implementation returns false.

Tools whose exit codes can be trusted override
exit_code_trustable() to true.

Reported-by: Dun Peal <redacted>
Signed-off-by: David Aguilar <redacted>
---
 git-mergetool--lib.sh    | 56 ++++++++++++++++++++++++++++++++++++++----------
 mergetools/araxis        |  2 --
 mergetools/bc            |  2 --
 mergetools/codecompare   |  2 --
 mergetools/deltawalker   |  6 +++++-
 mergetools/diffmerge     |  4 ++++
 mergetools/diffuse       |  2 --
 mergetools/ecmerge       |  2 --
 mergetools/emerge        |  4 ++++
 mergetools/examdiff      |  2 --
 mergetools/kdiff3        |  4 ++++
 mergetools/kompare       |  4 ++++
 mergetools/meld          |  3 +--
 mergetools/opendiff      |  2 --
 mergetools/p4merge       |  2 --
 mergetools/tkdiff        |  4 ++++
 mergetools/tortoisemerge |  2 --
 mergetools/vimdiff       |  2 --
 mergetools/winmerge      |  2 --
 mergetools/xxdiff        |  2 --
 20 files changed, 71 insertions(+), 38 deletions(-)

OK, so the idea is that the backends that used to touch $BACKUP and
used to call check_unchanged stop doing these two things, while
other backends override exit_code_trustable().  That makes the
change to the backends very straightforward to understand.

 # Run a either a configured or built-in merge tool
 run_merge_cmd () {
-	merge_cmd "$1"
+	mergetool_trust_exit_code=$(trust_exit_code "$1")
+	if test "$mergetool_trust_exit_code" = "true"
+	then
+		merge_cmd "$1"
+	else
+		touch "$BACKUP"
+		merge_cmd "$1"
+		check_unchanged
+	fi
 }

Looks sensible.  Thanks.  Will queue.