From: Brian Gernhardt <benji@silverinsanity.com>
Date: Sun, 05 Apr 2009 05:10:08 GMT
Subject: Re: [PATCH v2] mergetool--lib: add new merge tool TortoiseMerge
Message-ID: <DB42161D-8B55-4739-B418-D286B280EFF8@silverinsanity.com>
In-Reply-To: <1238904024-11238-1-git-send-email-davvid@gmail.com>


On Apr 5, 2009, at 12:00 AM, David Aguilar wrote:

> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index dcd4516..268485a 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -46,10 +46,13 @@ check_unchanged () {
>
> valid_tool () {
> 	case "$1" in
> -	kdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge |  
> vimdiff | gvimdiff | ecmerge | diffuse)
> +	kdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge |  
> vimdiff | gvimdiff | ecmerge | diffuse | tortoisemerge)
> 		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