From: David Aguilar <davvid@gmail.com>
Date: Mon, 06 Apr 2009 09:27:36 GMT
Subject: Re: [PATCH v2] mergetool--lib: add new merge tool TortoiseMerge
Message-ID: <20090406092736.GA16408@gmail.com>
In-Reply-To: <DB42161D-8B55-4739-B418-D286B280EFF8@silverinsanity.com>

On  0, Brian Gernhardt <benji@silverinsanity.com> wrote:
>
> On Apr 5, 2009, at 12:00 AM, David Aguilar wrote:
>
>> valid_tool () {
>> 	case "$1" in
>> 	<lots-of-stuff>)
>> 		if test "$1" = "kompare" && ! diff_mode; then
>> 			return 1
>> 		fi
>> +		if test "$1" = "tortoisemerge" && ! merge_mode; then
>> +			return 1
>> +		fi
>> 		;; # happy
>> 	*)
>> 		if test -z "$(get_merge_tool_cmd "$1")"; then
>
> Why is `case "$1"` being followed by two `if test "$1" =`s?
> Wouldn't it be simpler to have separate case arms for them?
> Especially with how long that list is getting...
>
> ~~ Brian

It would.  It wasn't until after the rewrite that I remembered
this email... oh well [PATCH v2 14/14] it is

-- 

	David