From 24933ba71305caa97fe1f756aef770d40f39fbda Mon Sep 17 00:00:00 2001 From: Philippe Blain Date: Mon, 11 Nov 2024 14:18:37 -0500 Subject: [PATCH 1/5] completion: complete '--tool-help' in 'git mergetool' Signed-off-by: Philippe Blain --- contrib/completion/git-completion.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 3d4dff3185ca95..b3b6aa3bae2919 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2331,7 +2331,7 @@ _git_mergetool () return ;; --*) - __gitcomp "--tool= --prompt --no-prompt --gui --no-gui" + __gitcomp "--tool= --tool-help --prompt --no-prompt --gui --no-gui" return ;; esac From 6f7f553b283078ba3c81190686b150a87d901240 Mon Sep 17 00:00:00 2001 From: Philippe Blain Date: Sun, 10 Nov 2024 16:42:38 -0500 Subject: [PATCH 2/5] git-mergetool--lib.sh: use TOOL_MODE when erroring about unknown tool In git-mergetool--lib.sh::get_merge_tool_path, we check if the chosen tool is valid via valid_tool and exit with an error message if not. This error message mentions "Unknown merge tool", even if the command the user tried was 'git difftool --tool=unknown'. Use the global 'TOOL_MODE' variable for a more correct error message. Signed-off-by: Philippe Blain --- git-mergetool--lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 1ff26170ffcff8..269a60ea44c4a6 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -474,7 +474,7 @@ get_merge_tool_path () { merge_tool="$1" if ! valid_tool "$merge_tool" then - echo >&2 "Unknown merge tool $merge_tool" + echo >&2 "Unknown $TOOL_MODE tool $merge_tool" exit 1 fi if diff_mode From 1d9e95c6cb14df3d3225210bde3c4877cb4cc540 Mon Sep 17 00:00:00 2001 From: Philippe Blain Date: Fri, 8 Nov 2024 16:47:28 -0500 Subject: [PATCH 3/5] 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 setup_user_tool, and we return with the exit code from setup_user_tool if it was called. setup_user_tool checks if {diff,merge}tool.$tool.cmd is set and quietly returns with an error if not. This leads to the following invocation quietly failing: git mergetool --tool=unknown which is not very user-friendly. Adjust setup_tool to output an error message before returning if setup_user_tool returned with an error. 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 initialization function, 2021-02-09), as before this commit an unknown mergetool would be diagnosed in get_merge_tool_path when called from run_merge_tool. Signed-off-by: Philippe Blain --- git-mergetool--lib.sh | 9 +++++++-- t/t7610-mergetool.sh | 8 ++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 269a60ea44c4a6..d7e410d9481449 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -159,7 +159,7 @@ check_unchanged () { } valid_tool () { - setup_tool "$1" && return 0 + setup_tool "$1" 2>/dev/null && return 0 cmd=$(get_merge_tool_cmd "$1") test -n "$cmd" } @@ -250,7 +250,12 @@ 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 diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index 22b3a85b3e960e..c077aba7cedde1 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -898,4 +898,12 @@ test_expect_success 'mergetool with guiDefault' ' git commit -m "branch1 resolved with mergetool" ' +test_expect_success 'mergetool with non-existent tool' ' + test_when_finished "git reset --hard" && + 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 "mergetool.absent.cmd not set for tool" out +' + test_done From f234e965543322d96fd6aa41d12f0f52c3599206 Mon Sep 17 00:00:00 2001 From: Philippe Blain Date: Mon, 11 Nov 2024 13:44:25 -0500 Subject: [PATCH 4/5] git-mergetool--lib.sh: add error message for unknown tool variant In setup_tool, we check if the given tool is a known variant of a tool, and quietly return with an error if not. This leads to the following invocation quietly failing: git mergetool --tool=vimdiff4 Add an error message before returning in this case. Signed-off-by: Philippe Blain --- git-mergetool--lib.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index d7e410d9481449..11ea181259f865 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -264,6 +264,7 @@ setup_tool () { if ! list_tool_variants | grep -q "^$tool$" then + echo "error: unknown tool variant '$tool'" >&2 return 1 fi From c16e9229ebb46916a59dc9c9fdc7b973480153d4 Mon Sep 17 00:00:00 2001 From: Philippe Blain Date: Mon, 11 Nov 2024 12:14:40 -0500 Subject: [PATCH 5/5] git-difftool--helper.sh: exit upon initialize_merge_tool errors Since the introduction of 'initialize_merge_tool' in de8dafbada (mergetool: break setup_tool out into separate initialization function, 2021-02-09), any errors from this function are ignored in git-difftool--helper.sh::launch_merge_tool, which is not the case for its call in git-mergetool.sh::merge_file. Despite the in-code comment, initialize_merge_tool (via its call to setup_tool) does different checks than run_merge_tool, so it makes sense to abort early if it encounters errors. Add exit calls if initialize_merge_tool fails. Signed-off-by: Philippe Blain --- git-difftool--helper.sh | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh index dd0c9a5b7f2b07..d32e47cc09eab0 100755 --- a/git-difftool--helper.sh +++ b/git-difftool--helper.sh @@ -61,9 +61,7 @@ launch_merge_tool () { export BASE eval $GIT_DIFFTOOL_EXTCMD '"$LOCAL"' '"$REMOTE"' else - initialize_merge_tool "$merge_tool" - # ignore the error from the above --- run_merge_tool - # will diagnose unusable tool by itself + initialize_merge_tool "$merge_tool" || exit 1 run_merge_tool "$merge_tool" fi } @@ -87,9 +85,7 @@ if test -n "$GIT_DIFFTOOL_DIRDIFF" then LOCAL="$1" REMOTE="$2" - initialize_merge_tool "$merge_tool" - # ignore the error from the above --- run_merge_tool - # will diagnose unusable tool by itself + initialize_merge_tool "$merge_tool" || exit 1 run_merge_tool "$merge_tool" false status=$?