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

[Junio C Hamano <gitster@pobox.com>]

Antoine Pelisse <apelisse@gmail.com> writes:

> 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?

> +	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.

> +	}
> +
> +	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?

> +	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