Re: [PATCH 4/5] pretty: Use mailmap to display username and email
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 11, 2012, 22:46 UTC
- Message-ID
- <7vehiw6wc1.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1355264493-8298-5-git-send-email-apelisse@gmail.com>
Antoine Pelisse <apelisse@gmail.com> writes:
Show 35 quoted lines
> Use the mailmap information to display the correct
> username and email address in all log commands.
>
> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
> ---
> pretty.c | 46 +++++++++++++++++++++++++++++++---------------
> 1 file changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/pretty.c b/pretty.c
> index 6730add..e232aaa 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -387,6 +387,8 @@ void pp_user_info(const struct pretty_print_context *pp,
> const char *what, struct strbuf *sb,
> const char *line, const char *encoding)
> {
> + char person_name[1024];
> + char person_mail[1024];
> struct ident_split ident;
> int linelen, namelen;
> char *line_end, *date;
> @@ -405,41 +407,55 @@ void pp_user_info(const struct pretty_print_context *pp,
> if (split_ident_line(&ident, line, linelen))
> return;
>
> - namelen = ident.mail_end - ident.name_begin + 1;
> + memcpy(person_mail, ident.mail_begin, ident.mail_end - ident.mail_begin);
> + person_mail[ident.mail_end - ident.mail_begin] = 0;
> +
> + memcpy(person_name, ident.name_begin, ident.name_end - ident.name_begin);
> + person_name[ident.name_end - ident.name_begin] = 0;
> +
> + if (pp->mailmap)
> + map_user(pp->mailmap, person_mail, sizeof(person_mail),
> + person_name, sizeof(person_name));This is why I said that we may want to rethink the API signature of the map_user() function. Now this caller has the hardcoded limit for name and mail parts, and it needs to make copies of strings into two arrays only because map_user() expects to get two fixed-size buffers that are to be rewritten in-place. If we changed map_user() to take two strbufs (one for name, the other for mail) to be updated in place, we would still need to make copies, but later code in this function may be able to lose a few strlen() calls.
Or it might be better to make those two strbufs output-only parameter, e.g.
map_user(struct string_list *mailmap,
const char *name, size_t namelen,
const char *mail, size_t maillen,
struct strbuf *name_out, struct strbuf *mail_out);then after split_ident_line(), this caller could feed two pointers into the original "line" as name and mail parameter, without making any copies (the callee has to make a copy but it has to be done anyway when the name/mail is mapped). I suspect it would make this caller simpler, but I do not know how invasive such changes are for other callers of map_user().
Such an update can be left outside of this series, of course.
Show 5 quoted lines
> + strbuf_addch(sb, ' '); > + strbuf_addch(sb, '<'); > + strbuf_add(sb, person_mail, strlen(person_mail)); > + strbuf_addch(sb, '>'); > strbuf_addch(sb, '\n');
Is that strbuf_addf(sb, " <%s>\n", person_mail)?