Re: [PATCH 04/10] mailmap: simplify map_user() interface
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 6, 2013, 22:56 UTC
- Message-ID
- <7vehhxnco3.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1357421206-5014-5-git-send-email-apelisse@gmail.com>
Antoine Pelisse <apelisse@gmail.com> writes:
Show 37 quoted lines
> diff --git a/builtin/blame.c b/builtin/blame.c
> index dd4aff9..86450e3 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> ...
> @@ -1356,51 +1356,61 @@ static void get_ac_line(const char *inbuf, const char *what,
> len = strlen(tmp);
> else
> len = endp - tmp;
>
> if (split_ident_line(&ident, tmp, len)) {
> error_out:
> /* Ugh */
> + tmp = "(unknown)";
> + strbuf_addstr(name, tmp);
> + strbuf_addstr(mail, tmp);
> + strbuf_addstr(tz, tmp);
> *time = 0;
> return;
> }
>
> namelen = ident.name_end - ident.name_begin;
> + tmpname = ident.name_begin;
>
> + maillen = ident.mail_end - ident.mail_begin;
> + tmpmail = ident.mail_begin;
>
> *time = strtoul(ident.date_begin, NULL, 10);
>
> + len = ident.tz_end - ident.tz_begin;
> + strbuf_add(tz, ident.tz_begin, len);
>
> /*
> * Now, convert both name and e-mail using mailmap
> */
> + map_user(&mailmap, &tmpmail, &maillen,
> + &tmpname, &namelen);I like the general simplification this change gives us, but do we still want to name these variables "tmp"-something? At least to me, it makes it look like the variable holds a pointer to a piece of memory that was temporarily allocated. Calling it "mail_begin" or something might be less confusing.
The changes to pretty.c (pp_user_info) and shortlog.c (insert_one_record) calls these variables mailbuf and namebuf, so perhaps these are better names?