From: Dan Zwell <dzwell@gmail.com>
Date: Tue, 13 Nov 2007 01:39:28 GMT
Subject: Re: Subject: [PATCH 2/3] Let git-add--interactive read colors from .gitconfig
Message-ID: <47390050.1020907@zwell.net>
In-Reply-To: <7vve89f6qy.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
>> +			$fg_done = "true";
>> +		}
>> +		elsif ($word =~ /black|red|green|yellow/ ||
>> +			   $word =~ /blue|magenta|cyan|white/) {
> 
> 	exists $color_name{$word}
> 
> with
> 
> 	my %color_name = map { $_ => 1 } qw(black red ... white);
> 
> at the beginning?
> 
I don't see the advantage of doing it that way. After all, we're pattern 
matching. Does using a hash, an array, and a call to map() gain us 
something? I think a regular expression is clearer. Of course, as Jeff 
pointed out, I should have used a whitespace-agnostic regular expression.
+		elsif ($word =~ /black|red|green|yellow|
+			blue|magenta|cyan|white/x ) {

I agreed with the rest of your suggestions, and will implement them in 
the next round of changes, later this week.

Dan