Re: [PATCH 4/5] pretty: Use mailmap to display username and email

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 4/5] pretty: Use mailmap to display username and email

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:55:30

Antoine Pelisse [off-list ref] writes:
quoted hunk
Use the mailmap information to display the correct
username and email address in all log commands.

Signed-off-by: Antoine Pelisse <redacted>
---
 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.
+		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)?

Re: [PATCH 4/5] pretty: Use mailmap to display username and email

From: Antoine Pelisse <hidden>
Date: 2016-06-15 22:55:30

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().
It makes a lot of sense.
blame.c::get_commit_info() hard code the length
shortlog.c::insert_one_record() hard code the length
pretty.c::format_person_part() hard code the length

I don't think it will be invasive.
Such an update can be left outside of this series, of course.
I will try to make it at the beginning of the series. It will avoid unnecessary
conflicts.
quoted
+             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)?
Of couse ;) Fixed.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help