From: Charles Bailey Date: Wed, 09 Jun 2010 19:24:43 GMT Subject: Re: [PATCH] git-mergetool--lib.sh: fix mergetool..* configurations ignored for known tools 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: > At this time when you define merge.tool with a known tool, > such as meld, p4merge, diffuse ... etc, mergetool..* > configurations are ignored and git mergetool will use its > own templates. > > This patch adds a detection for mergetool..cmd configuration > in the run_merge_tool function. If the configuration is set, it will > try to run the tool with mergetool..path if its set. It also > consider the mergetool..trustExitCode configuration. > > Signed-off-by: Sylvain Rabot > --- > 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..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.