Re: [PATCH v2 1/4] Add log.mailmap as configurational option for mailmap location

From: Marius Storm-Olsen <marius@trolltech.com>
Date: Feb 2, 2009, 07:48 UTC
Message-ID: <4986A539.3080606@trolltech.com>
In-Reply-To: <7vwsc9o82r.fsf@gitster.siamese.dyndns.org>

Junio C Hamano said the following on 02.02.2009 04:01:

Show 8 quoted lines

> Marius Storm-Olsen <marius@trolltech.com> writes:
>> This allows us to override a repo mailmap file, or to use
>> mailmap files elsewhere than the repository root.
> 
> I think the new feature is not so well conceived.
> 
> Should it always be a wholesale override, or should it also support
> augmenting the repository version with a private copy?

Sure, I can rewrite it to be augmenting, rather than overriding. I assume that the normal .mailmap file should be parsed first, then the log.mailmap one?

Show 13 quoted lines

>> diff --git a/builtin-blame.c b/builtin-blame.c
>> index aae14ef..4b63775 100644
>> --- a/builtin-blame.c
>> +++ b/builtin-blame.c
>> @@ -2394,7 +2394,7 @@ parse_done:
>>  		die("reading graft file %s failed: %s",
>>  		    revs_file, strerror(errno));
>>  
>> -	read_mailmap(&mailmap, ".mailmap", NULL);
>> +	read_mailmap(&mailmap, NULL, NULL);
> 
> Your callers always seem to pass NULL for the second argument.  Doesn't it
> make a lot more sense to get rid of it?

Sure, but I left it in to allow "old-style" usage. Just in case the were use-cases for not using the log.mailmap one. I can nuke the arguments if you don't want them anymore.

-- 
.marius [@trolltech.com]
'if you know what you're doing, it's not research'