Re: [PATCH] git-mergetool--lib.sh: fix mergetool.<tool>.* configurations ignored for known tools

From: Charles Bailey <charles@hashpling.org>
Date: Jun 9, 2010, 19:24 UTC
Message-ID: <4C0FEA7B.9030409@hashpling.org>
In-Reply-To: <1275705112-8088-2-git-send-email-sylvain@abstraction.fr>

On 05/06/2010 03:31, Sylvain Rabot wrote:

Show 15 quoted lines

> 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<sylvain@abstraction.fr>
> ---
>   git-mergetool--lib.sh |   60 +++++++++++++++++++++++++++++++++++++++++++++++++
>   1 files changed, 60 insertions(+), 0 deletions(-)
>

First, my apologies for only having had the time to skim this so far.

Can I just ask some basic questions about the purpose of this patch. Is it the intention that if mergetool.<tool>.cmd is set then you want to mergetool to behave 'as if' the merge tool wasn't a 'known' tool and just performed the "*)" case ?

If so, it seems like a lot of extra boiler-plate and error handling that doesn't exist in the normal "*)" case. Should we have have this in the "*)" case as well? If so, we should look to rework it so that we can re-use the code rather than duplicating it.

 From a user perspective, if they want to run a "known" tool but in a 
way that is different from the default behaviour can't they just give it 
a different name, e.g. merge.tool=my_kdiff3 , mergetool.my_kdiff3.cmd=... ?

Thanks,

Charles.

-- 
Almost dormant mergetool maintainer.