threads / patch / 10221

v2mergetool: support setting path to tool as config var mergetool.<tool>.path

Subject: [PATCH v2] mergetool: support setting path to tool as config var mergetool.<tool>.path

## tl;dr

7 messages between Oct 9, 2007 and Oct 14, 2007. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Steffen Prohaska· Oct 9, 2007, 20:54 UTC · lore

This commit adds a mechanism to provide absolute paths to the external programs called by 'git mergetool'. A path can be specified in the configuation variable mergetool.<tool>.path. The configuration variable is similar to how we name branches and remotes. It is extensible if we need to specify more details about a tool.

The mechanism added by this patch is orthogonal to the existing
merge.tool configuration variable:
1) merge.tool selects a tool from a list of known programs for which
command line arguments and the mechanism for returning merge results is know.
2) mergetool.<tool>.path provides details on how to locate the selected
tool in the filesystem. Paths for several tools can be configured simultaneously.

The mechanism is especially useful on Windows, where external programs are unlikely to be in PATH.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Documentation/git-mergetool.txt |    6 ++
 git-mergetool.sh                |   97 +++++++++++++++++++++-----------------
 2 files changed, 60 insertions(+), 43 deletions(-)

This updated patch is based on the the comments and suggestions by Theodor, Frank, and Johannes.

    Steffen
Show changes to 2 files +60 −43

Documentation/git-mergetool.txt, git-mergetool.sh

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 6c32c6d..17a33e7 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -31,6 +31,12 @@ If a merge resolution program is not specified, 'git mergetool'
 will use the configuration variable merge.tool.  If the
 configuration variable merge.tool is not set, 'git mergetool'
 will pick a suitable default.
++
+You can explicitly provide a full path to the tool by setting the
+configuration variable mergetool.<tool>.path. For example, you
+can configure the absolute path to kdiff3 by setting
+mergetool.kdiff3.path. Otherise, 'git mergetool' assumes the tool
+is available in PATH.
 
 Author
 ------
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 9f4f313..1e4583f 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -192,10 +192,10 @@ merge_file () {
     case "$merge_tool" in
 	kdiff3)
 	    if base_present ; then
-		(kdiff3 --auto --L1 "$path (Base)" --L2 "$path (Local)" --L3 "$path (Remote)" \
+		("$merge_tool_path" --auto --L1 "$path (Base)" --L2 "$path (Local)" --L3 "$path (Remote)" \
 		    -o "$path" -- "$BASE" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
 	    else
-		(kdiff3 --auto --L1 "$path (Local)" --L2 "$path (Remote)" \
+		("$merge_tool_path" --auto --L1 "$path (Local)" --L2 "$path (Remote)" \
 		    -o "$path" -- "$LOCAL" "$REMOTE" > /dev/null 2>&1)
 	    fi
 	    status=$?
@@ -203,35 +203,35 @@ merge_file () {
 	    ;;
 	tkdiff)
 	    if base_present ; then
-		tkdiff -a "$BASE" -o "$path" -- "$LOCAL" "$REMOTE"
+		"$merge_tool_path" -a "$BASE" -o "$path" -- "$LOCAL" "$REMOTE"
 	    else
-		tkdiff -o "$path" -- "$LOCAL" "$REMOTE"
+		"$merge_tool_path" -o "$path" -- "$LOCAL" "$REMOTE"
 	    fi
 	    status=$?
 	    save_backup
 	    ;;
 	meld|vimdiff)
 	    touch "$BACKUP"
-	    $merge_tool -- "$LOCAL" "$path" "$REMOTE"
+	    "$merge_tool_path" -- "$LOCAL" "$path" "$REMOTE"
 	    check_unchanged
 	    save_backup
 	    ;;
 	gvimdiff)
 		touch "$BACKUP"
-		gvimdiff -f -- "$LOCAL" "$path" "$REMOTE"
+		"$merge_tool_path" -f -- "$LOCAL" "$path" "$REMOTE"
 		check_unchanged
 		save_backup
 		;;
 	xxdiff)
 	    touch "$BACKUP"
 	    if base_present ; then
-		xxdiff -X --show-merged-pane \
+		"$merge_tool_path" -X --show-merged-pane \
 		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
 		    -R 'Accel.Search: "Ctrl+F"' \
 		    -R 'Accel.SearchForward: "Ctrl-G"' \
 		    --merged-file "$path" -- "$LOCAL" "$BASE" "$REMOTE"
 	    else
-		xxdiff -X --show-merged-pane \
+		"$merge_tool_path" -X --show-merged-pane \
 		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
 		    -R 'Accel.Search: "Ctrl+F"' \
 		    -R 'Accel.SearchForward: "Ctrl-G"' \
@@ -243,18 +243,18 @@ merge_file () {
 	opendiff)
 	    touch "$BACKUP"
 	    if base_present; then
-		opendiff "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$path" | cat
+		"$merge_tool_path" "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$path" | cat
 	    else
-		opendiff "$LOCAL" "$REMOTE" -merge "$path" | cat
+		"$merge_tool_path" "$LOCAL" "$REMOTE" -merge "$path" | cat
 	    fi
 	    check_unchanged
 	    save_backup
 	    ;;
 	emerge)
 	    if base_present ; then
-		emacs -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$(basename "$path")"
+		"$merge_tool_path" -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$(basename "$path")"
 	    else
-		emacs -f emerge-files-command "$LOCAL" "$REMOTE" "$(basename "$path")"
+		"$merge_tool_path" -f emerge-files-command "$LOCAL" "$REMOTE" "$(basename "$path")"
 	    fi
 	    status=$?
 	    save_backup
@@ -297,17 +297,38 @@ do
     shift
 done
 
+valid_tool() {
+	case "$1" in
+		kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff)
+			;; # happy
+		*)
+			return 1
+			;;
+	esac
+}
+
+init_merge_tool_path() {
+	merge_tool_path=`git config mergetool.$1.path`
+	if test -z "$merge_tool_path" ; then
+		case "$merge_tool" in
+			emerge)
+				merge_tool_path=emacs
+				;;
+			*)
+				merge_tool_path=$merge_tool
+				;;
+		esac
+	fi
+}
+
+
 if test -z "$merge_tool"; then
     merge_tool=`git config merge.tool`
-    case "$merge_tool" in
-	kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | "")
-	    ;; # happy
-	*)
+    test -n "$merge_tool" || valid_tool "$merge_tool" || {
 	    echo >&2 "git config option merge.tool set to unknown tool: $merge_tool"
 	    echo >&2 "Resetting to default..."
 	    unset merge_tool
-	    ;;
-    esac
+    }
 fi
 
 if test -z "$merge_tool" ; then
@@ -329,40 +350,30 @@ if test -z "$merge_tool" ; then
     merge_tool_candidates="$merge_tool_candidates opendiff emerge vimdiff"
     echo "merge tool candidates: $merge_tool_candidates"
     for i in $merge_tool_candidates; do
-        if test $i = emerge ; then
-            cmd=emacs
-        else
-            cmd=$i
-        fi
-        if type $cmd > /dev/null 2>&1; then
+        init_merge_tool_path $i
+        if type "$merge_tool_path" > /dev/null 2>&1; then
             merge_tool=$i
             break
         fi
     done
     if test -z "$merge_tool" ; then
-	echo "No available merge resolution programs available."
+	echo "No known merge resolution program available."
 	exit 1
     fi
+else
+    valid_tool "$merge_tool" || {
+        echo >&2 "Unknown merge_tool $merge_tool"
+        exit 1
+    }
+
+    init_merge_tool_path "$merge_tool"
+
+    if ! type "$merge_tool_path" > /dev/null 2>&1; then
+        echo "The merge tool $merge_tool is not available as '$merge_tool_path'"
+        exit 1
+    fi
 fi
 
-case "$merge_tool" in
-    kdiff3|tkdiff|meld|xxdiff|vimdiff|gvimdiff|opendiff)
-	if ! type "$merge_tool" > /dev/null 2>&1; then
-	    echo "The merge tool $merge_tool is not available"
-	    exit 1
-	fi
-	;;
-    emerge)
-	if ! type "emacs" > /dev/null 2>&1; then
-	    echo "Emacs is not available"
-	    exit 1
-	fi
-	;;
-    *)
-	echo "Unknown merge tool: $merge_tool"
-	exit 1
-	;;
-esac
 
 if test $# -eq 0 ; then
 	files=`git ls-files -u | sed -e 's/^[^	]*	//' | sort -u`
-- 
1.5.3.3.127.g40d17
Johannes Schindelin· Oct 10, 2007, 14:33 UTC · re: Steffen Prohaska · lore

Re: [PATCH v2] mergetool: support setting path to tool as config var mergetool.<tool>.path

Hi,
On Tue, 9 Oct 2007, Steffen Prohaska wrote:
Show 5 quoted lines
> This commit adds a mechanism to provide absolute paths to the external 
> programs called by 'git mergetool'. A path can be specified in the 
> configuation variable mergetool.<tool>.path. The configuration variable 
> is similar to how we name branches and remotes. It is extensible if we 
> need to specify more details about a tool.
Okay, let's step back a bit.

What does mergetool do? It calls different merge helpers, each with its own convention how to call it. For example, tkdiff is called either as

		tkdiff -a "$BASE" -o "$path" -- "$LOCAL" "$REMOTE"
or as
		tkdiff -o "$path" -- "$LOCAL" "$REMOTE"
depending if there is a base or not.  Another example is gvimdiff:
		gvimdiff -f -- "$LOCAL" "$path" "$REMOTE"
which seems not to care if there is a base.

Now, would it not be much better if we had a way to specify the tool and the convention indepentently? Like

merge.tkdiff.path = C:\bla\blub\wish.exe C:\blub\bleh\tkdiff.tcl merge.tkdiff.options = -o %p -- %l %r merge.tkdiff.optionsWithBase = -a %b -o %p -- %l %r

and have defaults for the tools we have in git-mergetool.sh _already_?

Ciao, Dscho

Steffen Prohaska· Oct 10, 2007, 15:44 UTC · re: Johannes Schindelin · lore

Re: [PATCH v2] mergetool: support setting path to tool as config var mergetool.<tool>.path

On Oct 10, 2007, at 4:33 PM, Johannes Schindelin wrote:
Show 41 quoted lines
> Hi,
>
> On Tue, 9 Oct 2007, Steffen Prohaska wrote:
>
>> This commit adds a mechanism to provide absolute paths to the  
>> external
>> programs called by 'git mergetool'. A path can be specified in the
>> configuation variable mergetool.<tool>.path. The configuration  
>> variable
>> is similar to how we name branches and remotes. It is extensible  
>> if we
>> need to specify more details about a tool.
>
> Okay, let's step back a bit.
>
> What does mergetool do?  It calls different merge helpers, each  
> with its
> own convention how to call it.  For example, tkdiff is called  
> either as
>
> 		tkdiff -a "$BASE" -o "$path" -- "$LOCAL" "$REMOTE"
>
> or as
>
> 		tkdiff -o "$path" -- "$LOCAL" "$REMOTE"
>
> depending if there is a base or not.  Another example is gvimdiff:
>
> 		gvimdiff -f -- "$LOCAL" "$path" "$REMOTE"
>
> which seems not to care if there is a base.
>
> Now, would it not be much better if we had a way to specify the  
> tool and
> the convention indepentently?  Like
>
> merge.tkdiff.path = C:\bla\blub\wish.exe C:\blub\bleh\tkdiff.tcl
> merge.tkdiff.options = -o %p -- %l %r
> merge.tkdiff.optionsWithBase = -a %b -o %p -- %l %r
>
> and have defaults for the tools we have in git-mergetool.sh _already_?

If you provide a generic mechanism to call an external tool, there's no need to name the tool. A single mechanism (let's call it external) would be sufficient, like

mergetool.external.path = c:\any\path\merge.exe mergetool.external.arg2way = %l %r --merge2 --to=%p mergetool.external.arg3way = %b %l %r --merge3 --to=%p

But this places the burdon on the user to figure out the command line syntax and specify it correctly using git-config. Things like proper escaping may be a hassel.

The solution I'm proposing is more user-friendly. Only the information that is hard to figure out automatically and is easy to provide by the user is asked for. The user only needs to tell the path to the executable.

git-mergetool 'knows' about the correct command line syntax. There's no need to ask the user. The command line syntax is fixed and know. No option here. "git mergetool" could, for example, know that a certain tool just doesn't support 3-way.

And the code of git-mergetool is also quite easy. The only input validation that is needed is to check if mergetool.<tool>.path points to a valid executable. If we provide a complex syntax for specifying command line options we may have to do a lot more of input validation and processing.

I strongly favor my solution of including the command line syntax in git-mergetool, and only ask the user for the path. I'm not against a generic mechanism to configure any tool. However, I do not plan to implement it.

If someone plans to develop a generic mechanism I see two options:
1) What's explained above.
2) _Define_ the parameters passed to the external command and ask the  
user
    to write a wrapper script to call his tool of choice. Similar to  
what
    GIT_EXTERNAL_DIFF does. Wrapper scripts would go to contrib/.
Both solutions would require more work from the user than needed.
	Steffen
Johannes Schindelin· Oct 10, 2007, 17:03 UTC · re: Steffen Prohaska · lore

Re: [PATCH v2] mergetool: support setting path to tool as config var mergetool.<tool>.path

Hi,
On Wed, 10 Oct 2007, Steffen Prohaska wrote:
Show 47 quoted lines
> On Oct 10, 2007, at 4:33 PM, Johannes Schindelin wrote:
> 
> > On Tue, 9 Oct 2007, Steffen Prohaska wrote:
> > 
> > > This commit adds a mechanism to provide absolute paths to the 
> > > external programs called by 'git mergetool'. A path can be specified 
> > > in the configuation variable mergetool.<tool>.path. The 
> > > configuration variable is similar to how we name branches and 
> > > remotes. It is extensible if we need to specify more details about a 
> > > tool.
> > 
> > Okay, let's step back a bit.
> > 
> > What does mergetool do?  It calls different merge helpers, each with its
> > own convention how to call it.  For example, tkdiff is called either as
> > 
> > 		tkdiff -a "$BASE" -o "$path" -- "$LOCAL" "$REMOTE"
> > 
> > or as
> > 
> > 		tkdiff -o "$path" -- "$LOCAL" "$REMOTE"
> > 
> > depending if there is a base or not.  Another example is gvimdiff:
> > 
> > 		gvimdiff -f -- "$LOCAL" "$path" "$REMOTE"
> > 
> > which seems not to care if there is a base.
> > 
> > Now, would it not be much better if we had a way to specify the tool 
> > and the convention indepentently?  Like
> > 
> > merge.tkdiff.path = C:\bla\blub\wish.exe C:\blub\bleh\tkdiff.tcl
> > merge.tkdiff.options = -o %p -- %l %r
> > merge.tkdiff.optionsWithBase = -a %b -o %p -- %l %r
> > 
> > and have defaults for the tools we have in git-mergetool.sh _already_?
> 
> If you provide a generic mechanism to call an external tool, there's no 
> need to name the tool. A single mechanism (let's call it external) would 
> be sufficient, like
> 
> mergetool.external.path = c:\any\path\merge.exe
> mergetool.external.arg2way = %l %r --merge2 --to=%p
> mergetool.external.arg3way = %b %l %r --merge3 --to=%p
> 
> But this places the burdon on the user to figure out the command line syntax
> and specify it correctly using git-config.

Guess why I did not want to have a single mechanism. I did _not_ propose to place the burden on the user to figure out the command line.

Besides, I do not want to do away with the automatic detection of the tool, which _implies_ having a list of defaults for the command line syntax, which in turn _commands_ having the name instead of "external".

While your solution is an obvious short term fix, I maintain that my proposal is the Right Thing in the long run.

Ciao, Dscho

Steffen Prohaska· Oct 10, 2007, 17:40 UTC · re: Johannes Schindelin · lore

Re: [PATCH v2] mergetool: support setting path to tool as config var mergetool.<tool>.path

On Oct 10, 2007, at 7:03 PM, Johannes Schindelin wrote:
Show 71 quoted lines
> Hi,
>
> On Wed, 10 Oct 2007, Steffen Prohaska wrote:
>
>> On Oct 10, 2007, at 4:33 PM, Johannes Schindelin wrote:
>>
>>> On Tue, 9 Oct 2007, Steffen Prohaska wrote:
>>>
>>>> This commit adds a mechanism to provide absolute paths to the
>>>> external programs called by 'git mergetool'. A path can be  
>>>> specified
>>>> in the configuation variable mergetool.<tool>.path. The
>>>> configuration variable is similar to how we name branches and
>>>> remotes. It is extensible if we need to specify more details  
>>>> about a
>>>> tool.
>>>
>>> Okay, let's step back a bit.
>>>
>>> What does mergetool do?  It calls different merge helpers, each  
>>> with its
>>> own convention how to call it.  For example, tkdiff is called  
>>> either as
>>>
>>> 		tkdiff -a "$BASE" -o "$path" -- "$LOCAL" "$REMOTE"
>>>
>>> or as
>>>
>>> 		tkdiff -o "$path" -- "$LOCAL" "$REMOTE"
>>>
>>> depending if there is a base or not.  Another example is gvimdiff:
>>>
>>> 		gvimdiff -f -- "$LOCAL" "$path" "$REMOTE"
>>>
>>> which seems not to care if there is a base.
>>>
>>> Now, would it not be much better if we had a way to specify the tool
>>> and the convention indepentently?  Like
>>>
>>> merge.tkdiff.path = C:\bla\blub\wish.exe C:\blub\bleh\tkdiff.tcl
>>> merge.tkdiff.options = -o %p -- %l %r
>>> merge.tkdiff.optionsWithBase = -a %b -o %p -- %l %r
>>>
>>> and have defaults for the tools we have in git-mergetool.sh  
>>> _already_?
>>
>> If you provide a generic mechanism to call an external tool,  
>> there's no
>> need to name the tool. A single mechanism (let's call it external)  
>> would
>> be sufficient, like
>>
>> mergetool.external.path = c:\any\path\merge.exe
>> mergetool.external.arg2way = %l %r --merge2 --to=%p
>> mergetool.external.arg3way = %b %l %r --merge3 --to=%p
>>
>> But this places the burdon on the user to figure out the command  
>> line syntax
>> and specify it correctly using git-config.
>
> Guess why I did not want to  have a single mechanism.  I did _not_  
> propose
> to place the burden on the user to figure out the command line.
>
> Besides, I do not want to do away with the automatic detection of the
> tool, which _implies_ having a list of defaults for the command line
> syntax, which in turn _commands_ having the name instead of  
> "external".
>
> While your solution is an obvious short term fix, I maintain that my
> proposal is the Right Thing in the long run.

I did not intend to replace everything with an external mechanism. What I wanted to say is the following: If you add a generic mechanism in addition to the known tools it is sufficient to add a single generic mechanism called 'external'. You could then choose from the list of known tools or you could choose 'external' and provide everything needed as described above.

	Steffen
Steffen Prohaska· Oct 14, 2007, 12:52 UTC · re: Steffen Prohaska · lore

Re: [PATCH v2] mergetool: support setting path to tool as config var mergetool.<tool>.path

On Oct 9, 2007, at 10:54 PM, Steffen Prohaska wrote:
> This commit adds a mechanism to provide absolute paths to the
> external programs called by 'git mergetool'. A path can be
> specified in the configuation variable mergetool.<tool>.path.

Any news on this patch? Will you apply it (or recommend that Junio does)?

	Steffen
Theodore Tso· Oct 14, 2007, 13:02 UTC · re: Steffen Prohaska · lore

Re: [PATCH v2] mergetool: support setting path to tool as config var mergetool.<tool>.path

On Sun, Oct 14, 2007 at 02:52:25PM +0200, Steffen Prohaska wrote:
Show 7 quoted lines
>
>> This commit adds a mechanism to provide absolute paths to the
>> external programs called by 'git mergetool'. A path can be
>> specified in the configuation variable mergetool.<tool>.path.
>
> Any news on this patch?
> Will you apply it (or recommend that Junio does)?

Hmm, for some reason I never received the v2 version of the patch. I see Johannes' comments to it, but not the original e-mail for some reason. It doesn't seem to be in the spam filter, so I'm not sure what happened.

Can you resend, please?   Thanks!!
							- Ted

← back to recent threads