Re: [PATCH 1/2] log: grep author/committer using mailmap

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 26, 2012, 19:27 UTC
Message-ID: <7vr4mcobpu.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1356195512-4846-2-git-send-email-apelisse@gmail.com>

Antoine Pelisse <apelisse@gmail.com> writes:

Show 46 quoted lines

> Currently mailmap can be used to display log authors and committers
> but there no way to use mailmap to find commits with mapped values.
>
> This commit allows those commands to work:
>
>     git log --use-mailmap --author mapped_name_or_email
>     git log --use-mailmap --committer mapped_name_or_email
>
> Of course it only works if the --use-mailmap option is used.
>
> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
> ---
> I probably missed something but I didn't find the connection with
> commit 2d10c55. Let me know if I went the wrong direction.
>
>  revision.c         |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  t/t4203-mailmap.sh |   18 ++++++++++++++++++
>  2 files changed, 71 insertions(+)
>
> diff --git a/revision.c b/revision.c
> index 95d21e6..fb9fd41 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -13,6 +13,7 @@
>  #include "decorate.h"
>  #include "log-tree.h"
>  #include "string-list.h"
> +#include "mailmap.h"
>
>  volatile show_early_output_fn_t show_early_output;
>
> @@ -2219,6 +2220,50 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
>  	return 0;
>  }
>
> +static int commit_rewrite_authors(struct strbuf *buf, const char *what, struct string_list *mailmap)
> +{
> +	char *author, *endp;
> +	size_t len;
> +	struct strbuf name = STRBUF_INIT;
> +	struct strbuf mail = STRBUF_INIT;
> +	struct ident_split ident;
> +
> +	author = strstr(buf->buf, what);
> +	if (!author)
> +		goto error;

This does not stop at the end of the header part and would match a random line in the log message that happens to begin with "author "; is this something we would worry about, or would we leave it to "fsck"?

> +	author += strlen(what);
> +	endp = strstr(author, "\n");

Using strchr(author, '\n') would feel more natural. Also rename "author" to "person" or something, as you would be using this function for the committer information as well?

Show 11 quoted lines

> +	if (!endp)
> +		goto error;
> +
> +	len = endp - author;
> +
> +	if (split_ident_line(&ident, author, len)) {
> +	error:
> +		strbuf_release(&name);
> +		strbuf_release(&mail);
> +
> +		return 1;

We usually signal error by returning a negative integer. It does not matter too much in this case as no callers seem to check the return value from this function, though.

Show 12 quoted lines

> +	}
> +
> +	strbuf_add(&name, ident.name_begin, ident.name_end - ident.name_begin);
> +	strbuf_add(&mail, ident.mail_begin, ident.mail_end - ident.mail_begin);
> +
> +	map_user(mailmap, &mail, &name);
> +
> +	strbuf_addf(&name, " <%s>", mail.buf);
> +
> +	strbuf_splice(buf, ident.name_begin - buf->buf,
> +		      ident.mail_end - ident.name_begin + 1,
> +		      name.buf, name.len);

Would it give us better performance if we splice only when map_user() tells us that we actually rewrote the ident?

Show 60 quoted lines

> +	strbuf_release(&name);
> +	strbuf_release(&mail);
> +
> +	return 0;
> +}
> +
>  static int commit_match(struct commit *commit, struct rev_info *opt)
>  {
>  	int retval;
> @@ -2237,6 +2282,14 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
>  	if (buf.len)
>  		strbuf_addstr(&buf, commit->buffer);
>
> +	if (opt->mailmap) {
> +		if (!buf.len)
> +			strbuf_addstr(&buf, commit->buffer);
> +
> +		commit_rewrite_authors(&buf, "\nauthor ", opt->mailmap);
> +		commit_rewrite_authors(&buf, "\ncommitter ", opt->mailmap);
> +	}
> +
>  	/* Append "fake" message parts as needed */
>  	if (opt->show_notes) {
>  		if (!buf.len)
> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
> index db043dc..e16187f 100755
> --- a/t/t4203-mailmap.sh
> +++ b/t/t4203-mailmap.sh
> @@ -248,11 +248,29 @@ Author: Other Author <other@author.xx>
>  Author: Some Dude <some@dude.xx>
>  Author: A U Thor <author@example.com>
>  EOF
> +
>  test_expect_success 'Log output with --use-mailmap' '
>  	git log --use-mailmap | grep Author >actual &&
>  	test_cmp expect actual
>  '
>
> +cat >expect <<\EOF
> +Author: Santa Claus <santa.claus@northpole.xx>
> +Author: Santa Claus <santa.claus@northpole.xx>
> +EOF
> +
> +test_expect_success 'Grep author with --use-mailmap' '
> +	git log --use-mailmap --author Santa | grep Author >actual &&
> +	test_cmp expect actual
> +'
> +
> +>expect
> +
> +test_expect_success 'Only grep replaced author with --use-mailmap' '
> +	git log --use-mailmap --author "<cto@coompany.xx>" >actual &&
> +	test_cmp expect actual
> +'
> +
>  # git blame
>  cat >expect <<\EOF
>  ^OBJI (A U Thor     DATE 1) one
> --
> 1.7.9.5