public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: Antoine Pelisse <apelisse@gmail•com>, git <git@vger•kernel.org>
Subject: Re: [PATCH] commit: search author pattern against mailmap
Date: Sun, 25 Aug 2013 22:27:52 -0700	[thread overview]
Message-ID: <xmqq1u5hkomf.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20130825165153.GC21092@sigill.intra.peff.net> (Jeff King's message of "Sun, 25 Aug 2013 12:51:53 -0400")

Jeff King <peff@peff•net> 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...

  parent reply	other threads:[~2013-08-26  5:28 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-23 13:48 [PATCH] git-commit: search author pattern against mailmap Antoine Pelisse
2013-08-23 17:44 ` Junio C Hamano
2013-08-23 18:35   ` Jeff King
2013-08-23 19:03     ` Junio C Hamano
2013-08-23 19:47       ` Antoine Pelisse
2013-08-23 20:44         ` Junio C Hamano
2013-08-24 14:07           ` [PATCH] commit: " Antoine Pelisse
2013-08-25  4:01             ` Jeff King
2013-08-25  5:16               ` Junio C Hamano
2013-08-25  9:47                 ` Antoine Pelisse
2013-08-25 10:01                 ` Antoine Pelisse
2013-08-25 10:30                   ` Jeff King
2013-08-25 13:37                     ` Antoine Pelisse
2013-08-25 16:51                       ` Jeff King
2013-08-25 20:42                         ` Antoine Pelisse
2013-08-26  5:27                         ` Junio C Hamano [this message]
2013-08-26 21:38                           ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq1u5hkomf.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=apelisse@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=peff@peff$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox