Subject: Re: [PATCH] commit: search author pattern against mailmap

From: Junio C Hamano <[email-masked]>
Date: Sun, 25 Aug 2013 22:27:52 -0700

Jeff King <[email-masked]> writes:

> Exactly. Sample (largely untested) patch is below if you want to use it
> as a starting point. There are probably a few additional cleanups on top
> (e.g., "git log" understands "--mailmap", which should probably be
> centralized to handle_revision_opt).
>
> I'm on the fence. It doesn't actually save that many lines of code, and
> I guess it's possible that somebody would want a custom mailmap in the
> future. Even though you can't do it right now, all it would take is
> exposing read_mailmap_file and read_mailmap_blob outside of mailmap.c.
> Of course, it would be easy to expose map_user_from at the same time.

I am of two minds on this, but if I were forced to pick one _today_,
I would have to say that I am moderately negative to the approach.

Having to always specify that you want to use mailmap and make sure
you read it is a bit cumbersome from callers' point of view, and
using a singleton global may be one attractive way to do so.

It however regresses the "you can choose which mailmap to apply"
structure we already have, it would make things less libifiable, and
will make it harder to allow a single Git process work on two or
more independent repositories (yes, we would need to restructure the
object API to allow us to manage multiple object stores, the ref
API, etc. in a way similar to how we weaned ourselves away from the
single "active_cache" abstraction in the index API). I am personally
OK to declare that we should _never_ touch more than one repository
in a single process, but submodule support already does this to some
extent, so...

I think it is a reasonable tentative solution to hook a singleton
instance to something that is commonly used, e.g. the rev_info
structure, for large subset of commands that do use the structure
chosen to host that singleton instance, but those that do not work
based on revision traversal (e.g. "grep") need to also honor mailmap
consistently, so we must keep the lower level API that takes an
explicit mailmap instance for them anyway.

So...