Re: [PATCH 04/10] mailmap: simplify map_user() interface
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:55:40
Antoine Pelisse [off-list ref] writes:
quoted hunk
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?