Re: Oddidies in the .mailmap parser & future syntax extensions

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sep 10, 2021, 19:50 UTC
Message-ID: <87h7esb3ig.fsf@evledraar.gmail.com>
In-Reply-To: <xmqq1r5wti5a.fsf@gitster.g>

On Fri, Sep 10 2021, Junio C Hamano wrote:

Show 37 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> [Changed subject]
>
> [jc: culled CC addresses]
>
>> I'd expect:
>>
>>     Foo <foo@example.com> Bar
>>
>> To be an alias/shorthand for:
>>
>>     Foo <foo@example.com> Bar <foo@example.com>
>
> OK.
>
>> More annoying is that this:
>>
>>     New <foo@example.com> <bar@example.com>
>>     <foo@example.com> <zar@example.com>
>>
>> Doesn't mean the same as:
>> ...
>> I.e. I'd expect the name to map to the empty string, *unless* we saw an
>> earlier address, i.e. just as we do for the first bar -> foo line (we
>> map it to a name of "New", we don't map it to an empty name).
>
> You expect the first one to map (anyname, <bar@example.com>) to
> ("New", <foo@example.com>) and you describe the second one does not
> map the human-readable part to "New", but it is unclear what the
> code does, or why you expect it to map to "" (or what your
> expectation is, for that matter, exactly---do you want an empty
> string, or do you want "New", or something else???).
>
> FWIW, if we were designing it from scratch, I'd expect the second
> one to map (anyname, <zar@example.com>) to ($1, <foo@example.com>),
> keeping the human-readable part as-is and only map the e-mail part.

Yes, that's what it does now. I.e. we'll create two mappings from those lines, namely (in line-by-line regex-like pseudosyntax):

    [[(.*), bar@example.com>] => [New, foo@example.com]]
    [[(.*), zar@example.com>] => [$1, foo@example.com]]

I.e. for the second one we'll retain whatever name we found there.

> Or do you expect that when these two entries appear together, the
> first entry with "New" is carried over to the second entry?

Yes, I'd expect it to be stateful and implicitly do:

    [[(.*), bar@example.com>] => [New, foo@example.com]]
    [[(.*), zar@example.com>] => [find_last_name_for_address_in_mailmap(find_last_explicit_) || $1, foo@example.com]]

I.e. we'd map the entries for you in git.git like:
    
    diff --git a/.mailmap b/.mailmap
    index 9c6a446bdfb..2a884981e9d 100644
    --- a/.mailmap
    +++ b/.mailmap
    @@ -127,8 +127,8 @@ Julian Phillips <julian@quantumfyre.co.uk> <jp3@quantumfyre.co.uk>
     Junio C Hamano <gitster@pobox.com> <gitster@pobox.com>
    -Junio C Hamano <gitster@pobox.com> <junio@hera.kernel.org>
    -Junio C Hamano <gitster@pobox.com> <junio@kernel.org>
    -Junio C Hamano <gitster@pobox.com> <junio@pobox.com>
    -Junio C Hamano <gitster@pobox.com> <junio@twinsun.com>
    -Junio C Hamano <gitster@pobox.com> <junkio@cox.net>
    -Junio C Hamano <gitster@pobox.com> <junkio@twinsun.com>
    +<gitster@pobox.com> <junio@hera.kernel.org>
    +<gitster@pobox.com> <junio@kernel.org>
    +<gitster@pobox.com> <junio@pobox.com>
    +<gitster@pobox.com> <junio@twinsun.com>
    +<gitster@pobox.com> <junkio@cox.net>
    +<gitster@pobox.com> <junkio@twinsun.com>

Which are currently mostly redundant, except for one old commit under junio@twinsun.com where the " C " wasn't present in your name field.

One might say that's a feature, and you'd like to be explicit about when
to map addresses and when to map names, i.e. if we were trying to
minimize the size of the .mailmap then this would be the most minimal
thing we can get away with currently:
    
    diff --git a/.mailmap b/.mailmap
    index 9c6a446bdfb..4668f9b32f2 100644
    --- a/.mailmap
    +++ b/.mailmap
    @@ -127,8 +127,8 @@ Julian Phillips <julian@quantumfyre.co.uk> <jp3@quantumfyre.co.uk>
     Junio C Hamano <gitster@pobox.com> <gitster@pobox.com>
    -Junio C Hamano <gitster@pobox.com> <junio@hera.kernel.org>
    -Junio C Hamano <gitster@pobox.com> <junio@kernel.org>
    -Junio C Hamano <gitster@pobox.com> <junio@pobox.com>
    +<gitster@pobox.com> <junio@hera.kernel.org>
    +<gitster@pobox.com> <junio@kernel.org>
    +<gitster@pobox.com> <junio@pobox.com>
     Junio C Hamano <gitster@pobox.com> <junio@twinsun.com>
    -Junio C Hamano <gitster@pobox.com> <junkio@cox.net>
    -Junio C Hamano <gitster@pobox.com> <junkio@twinsun.com>
    +<gitster@pobox.com> <junkio@cox.net>
    +<gitster@pobox.com> <junkio@twinsun.com>

But I just can't imagine cases where the proposed shorthand isn't useful for everyone.

Who wants to use mailmap, *and* map one e-mail address to another, *and* has an entry explicitly mapping the name, but *would* mind having git be auto-smart and follow the chain of that explicit name mapping if there's an entry after that with an an e-mail -> that-earlier-email mapping?

I think the answer is "nobody" and it would be unambiguously helpful,
but maybe I'm wrong.

Show 13 quoted lines

>> Doing that would be strictly backwards compatible, i.e. now we'll
>> entirely ignore the 3rd E-Mail address. It does mean we also
>> accidentally support things like:
>>
>>     New <foo@example.com> <bar@example.com> # A comment, because we ignore everything after the 2nd address
>>
>> But don't tell anyone I told you that :) But that is something that
>> might technically have inadvertently closed the door to future syntax
>> extensions, but we could probably do them anyway, or at worst have some
>> heuristic.
>
> I vaguely recall that it was not an accident but a deliberate
> feature to allow comments, but don't tell anyone I told you that.

Which works, right until someone has an old name entry that looks like a comment :)