git-mergetool: improve error code paths and messages

Changes in v2: As suggested by Junio:

* 3/5: moved the error message to setup_tool itself, and adjusted the commit message
* 3/5: made the test more robust
* 4/5: adjusted the error message

v1: These are a few improvements to improve existing error messages in 'git mergetool', and make sure that errors are not quiet, along with a small completion update in 1/1.

Philippe Blain (5):
completion: complete '--tool-help' in 'git mergetool' git-mergetool--lib.sh: use TOOL_MODE when erroring about unknown tool git-mergetool--lib.sh: add error message if 'setup_user_tool' fails git-mergetool--lib.sh: add error message for unknown tool variant git-difftool--helper.sh: exit upon initialize_merge_tool errors

 contrib/completion/git-completion.bash |  2 +-
 git-difftool--helper.sh                |  8 ++------
 git-mergetool--lib.sh                  | 12 +++++++++---
 t/t7610-mergetool.sh                   |  8 ++++++++
 4 files changed, 20 insertions(+), 10 deletions(-)

base-commit: b31fb630c0fc6869a33ed717163e8a1210460d94
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1827%2Fphil-blain%2Fabsent-mergetool-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1827/phil-blain/absent-mergetool-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1827

Range-diff vs v1:

 1:  24933ba7130 = 1:  24933ba7130 completion: complete '--tool-help' in 'git mergetool'
 2:  6f7f553b283 = 2:  6f7f553b283 git-mergetool--lib.sh: use TOOL_MODE when erroring about unknown tool
 3:  79c3a6ffe8f ! 3:  1d9e95c6cb1 git-mergetool--lib.sh: add error message in 'setup_user_tool'

@@ Metadata
Author: Philippe Blain <[email protected]>

## Commit message ##
- git-mergetool--lib.sh: add error message in 'setup_user_tool'
+ git-mergetool--lib.sh: add error message if 'setup_user_tool' fails

In git-mergetool--lib.sh::setup_tool, we check if the given tool is a known builtin tool, a known variant, or a user-defined tool by calling
@@ Commit message

git mergetool --tool=unknown

- which is not very user-friendly. Adjust setup_user_tool to output an
- error message before returning if {diff,merge}tool.$tool.cmd is not set.
+ which is not very user-friendly. Adjust setup_tool to output an error
+ message before returning if setup_user_tool returned with an error.

- Adjust the second call to setup_user_tool in setup_tool to silence this
- new error, as this call is only meant to allow users to redefine 'cmd'
- for a builtin tool; it is not an error if they have not done so (which
- is why we do not check the return status of this call).
+ Note that we do not check the result of the second call to
+ setup_user_tool in setup_tool, as this call is only meant to allow users
+ to redefine 'cmd' for a builtin tool; it is not an error if they have
+ not done so.

Note that this behaviour of quietly failing is a regression dating back to de8dafbada (mergetool: break setup_tool out into separate
@@ git-mergetool--lib.sh: check_unchanged () {
cmd=$(get_merge_tool_cmd "$1")
test -n "$cmd"
}
-
- setup_user_tool () {
- merge_tool_cmd=$(get_merge_tool_cmd "$tool")
-- test -n "$merge_tool_cmd" || return 1
-+ if test -z "$merge_tool_cmd"
-+ then
-+ echo >&2 "error: ${TOOL_MODE}tool.$tool.cmd not set for tool '$tool'"
-+ return 1
-+ fi
-
- diff_cmd () {
- ( eval $merge_tool_cmd )
@@ git-mergetool--lib.sh: setup_tool () {
+ . "$MERGE_TOOLS_DIR/${tool%[0-9]}"
+ else
+ setup_user_tool
+- return $?
++ rc=$?
++ if test $rc -ne 0
++ then
++ echo >&2 "error: ${TOOL_MODE}tool.$tool.cmd not set for tool '$tool'"
++ fi
++ return $rc
+ fi

# Now let the user override the default command for the tool. If
- # they have not done so then this will return 1 which we ignore.
-- setup_user_tool
-+ setup_user_tool 2>/dev/null
-
- if ! list_tool_variants | grep -q "^$tool$"
- then

## t/t7610-mergetool.sh ##
@@ t/t7610-mergetool.sh: test_expect_success 'mergetool with guiDefault' '
@@ t/t7610-mergetool.sh: test_expect_success 'mergetool with guiDefault' '
+ git checkout -b test$test_count branch1 &&
+ test_must_fail git merge main &&
+ yes "" | test_must_fail git mergetool --tool=absent >out 2>&1 &&
-+ test_grep -i "not set for tool" out
++ test_grep "mergetool.absent.cmd not set for tool" out
+'
+
test_done
4: 74b83caa1e5 ! 4: f234e965543 git-mergetool--lib.sh: add error message for unknown tool variant

     @@ git-mergetool--lib.sh: setup_tool () {
       
       	if ! list_tool_variants | grep -q "^$tool$"
       	then
     -+		echo "error: unknown ${tool%[0-9]} variant '$tool'" >&2
     ++		echo "error: unknown tool variant '$tool'" >&2
       		return 1
       	fi

5: be0b86f0890 = 5: c16e9229ebb git-difftool--helper.sh: exit upon initialize_merge_tool errors

gitgitgadget