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

From: Steffen Prohaska <prohaska@zib.de>
Date: Oct 10, 2007, 15:44 UTC
Message-ID: <FBE79360-4B3A-4C92-B8AA-76989D933009@zib.de>
In-Reply-To: <Pine.LNX.4.64.0710101526060.4174@racer.site>

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