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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 8, 2010, 21:05 UTC
Message-ID: <7vzkz5s0a3.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100608083445.GC14366@gmail.com>

David Aguilar <davvid@gmail.com> writes:

> Hi, sorry for the delay in responding to this email.

Thanks for a review.

Show 7 quoted lines

> 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.

I concur; it is not just about simplicity, but setting the value to the parent directory of the tool feels downright confusing.

Show 7 quoted lines

>> +		# mergetool.<tool>.path is the same as mergetool.<tool>.cmd
>> ...
>> +	fi
>
> This section is getting pretty nested.
> Should we break the handling for configs-that-override-builtins
> into a separate function?

Sounds like a sane thing to do.