Re: [PATCH] git-mergetool--lib.sh: fix mergetool.<tool>.* configurations ignored for known tools
From: David Aguilar <hidden>
Date: 2016-06-15 22:48:56
Hi, sorry for the delay in responding to this email. On Sat, Jun 05, 2010 at 04:31:52AM +0200, Sylvain Rabot wrote:
quoted hunk ↗ jump to hunk
At this time when you define merge.tool with a known tool, such as meld, p4merge, diffuse ... etc, mergetool.<tool>.* configurations are ignored and git mergetool will use its own templates. This patch adds a detection for mergetool.<tool>.cmd configuration in the run_merge_tool function. If the configuration is set, it will try to run the tool with mergetool.<tool>.path if its set. It also consider the mergetool.<tool>.trustExitCode configuration. Signed-off-by: Sylvain Rabot <redacted> --- git-mergetool--lib.sh | 60 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 60 insertions(+), 0 deletions(-)diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 51dd0d6..2a58d88 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh@@ -84,9 +84,69 @@ get_merge_tool_cmd () { run_merge_tool () { merge_tool_path="$(get_merge_tool_path "$1")" || exit + merge_tool_cmd="$(get_merge_tool_cmd "$1")" + merge_tool_cmd_base="$(echo $merge_tool_cmd | cut -f1 -d " ")" base_present="$2" status=0 + # if mergetool.<tool>.cmd is set we execute it, not a template + if test -n "$merge_tool_cmd"; then + # mergetool.<tool>.path is empty + if test -z "$merge_tool_path"; then + # mergetool.<tool>.cmd not found + if ! $(which "$merge_tool_cmd_base" > /dev/null 2>&1); then + echo >&2 "Configuration mergetool.$1.cmd \"$merge_tool_cmd_base\" not found" + exit 1 + else + merge_cmd="$merge_tool_path/$merge_tool_cmd" + fi + # mergetool.<tool>.path is a path
Files and Directories are both paths...
+ elif test -d "$merge_tool_path"; then
But...
+ # mergetool.<tool>.cmd not found + if !test -f "$merge_tool_path/$merge_tool_cmd_base"; then + echo >&2 "Configuration mergetool.$1.cmd \"$(echo "$merge_tool_path/$merge_tool_cmd_base" | sed 's#//\+#/#')\" not found" + exit 1 + # mergetool.<tool>.cmd not executable + elif !test -x "$merge_tool_path/$merge_tool_cmd_base"; then + echo >&2 "Configuration mergetool.$1.cmd \"$(echo "$merge_tool_path/$merge_tool_cmd_base" | sed 's#//\+#/#')\" is not executable" + exit 1 + # tool ok + else + merge_cmd="$merge_tool_path/$merge_tool_cmd" + fi
I don't think we ever signed up to support this configuration. mergetool.<tool>.path has always (from my naive reading of the documentation) been the absolute path to <tool>. I don't think it should have a dual-role where it can be either the tool's parent directory or the path to the tool itself. I would prefer to keep it as simple as possible, if we can.
+ # mergetool.<tool>.path is the same as mergetool.<tool>.cmd + elif test "$merge_tool_path" = "$merge_tool_cmd_base"; then + # mergetool.<tool>.cmd not found + if ! $(which "$merge_tool_cmd_base" > /dev/null 2>&1); then + echo >&2 "Configuration mergetool.$1.cmd \"$merge_tool_cmd_base\" not found" + exit 1 + else + merge_cmd="$merge_tool_cmd" + fi + # mergetool.<tool>.path is the tool itself + elif $(which "$merge_tool_path" > /dev/null 2>&1); then + merge_cmd="$merge_tool_path $merge_tool_cmd" + # mergetool.<tool>.path invalid + else + echo >&2 "Configuration mergetool.$1.path \"$merge_tool_path\" is not valid path" + exit 1 + fi + + # trust exit code + trust_exit_code="$(git config --bool mergetool."$1".trustExitCode || echo false)" + + if test "$trust_exit_code" = "false"; then + touch "$BACKUP" + (eval "$merge_cmd") + check_unchanged + return $status + else + (eval "$merge_cmd") + status=$? + return $status + fi + fi
This section is getting pretty nested. Should we break the handling for configs-that-override-builtins into a separate function?
+ case "$1" in kdiff3) if merge_mode; then -- 1.7.1
One last thing -- I tried to fetch from the repo you mentioned elsewhere in this thread but it was offline. Cheers, -- David