On 10/23/19 10:29 PM, Junio C Hamano wrote:
> "brian m. carlson" <sandals@xxxxxxxxxxxxxxxxxxxx> writes:
>
>>> + if (part == 'u' || part == 'U') { /* username */
>>> + maillen = strstr(s.mail_begin, "@") - s.mail_begin;
>>> + strbuf_add(sb, mail, maillen);
>>> + return placeholder_len;
>>> + }
>>
>> This branch doesn't appear to do anything different for the mailmap and
>> non-mailmap cases. Perhaps adding an additional test that demonstrates
>> the difference would be a good idea.
>
> Yes, and the bug that would be exposed is the lack of call to
> mailmap.
>
> Perhaps along this line (I may have off-by-one or two tho)?
>
> pretty.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/pretty.c b/pretty.c
> index e4ed14effe..4b76d022c6 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -696,15 +696,27 @@ static size_t format_person_part(struct strbuf *sb, char part,
> mail = s.mail_begin;
> maillen = s.mail_end - s.mail_begin;
>
> - if (part == 'N' || part == 'E') /* mailmap lookup */
> + if (part == 'N' || part == 'E' || part == 'L') /* mailmap lookup */
> mailmap_name(&mail, &maillen, &name, &namelen);
> - if (part == 'n' || part == 'N') { /* name */
> +
> + switch (part) {
> + case 'n': case 'N':
> strbuf_add(sb, name, namelen);
> return placeholder_len;
> - }
> - if (part == 'e' || part == 'E') { /* email */
> + case 'l': case 'L':
> + {
> + const char *at = memchr(mail, '@', maillen);
> + if (at) {
> + maillen -= at - mail + 1;
> + mail = at + 1;
^^^ If the mail is 'prarit@xxxxxxxxxx' the output of these lines is
maillen = maillen - at - mail - 1
which is (16 - [6] - 1) = 9.
and mail = at + 1 points to "redhat.com"
This is the reverse of what we want. IMO I suggest (sorry for the
cut-and-paste) which keeps the code easy to read.
diff --git a/pretty.c b/pretty.c
index b32f0369531c..93eb6e837071 100644
--- a/pretty.c
+++ b/pretty.c
@@ -696,7 +696,7 @@ static size_t format_person_part(struct strbuf *sb, char par
t,
mail = s.mail_begin;
maillen = s.mail_end - s.mail_begin;
- if (part == 'N' || part == 'E') /* mailmap lookup */
+ if (part == 'N' || part == 'E' || part == 'L') /* mailmap lookup */
mailmap_name(&mail, &maillen, &name, &namelen);
if (part == 'n' || part == 'N') { /* name */
strbuf_add(sb, name, namelen);
@@ -706,6 +706,13 @@ static size_t format_person_part(struct strbuf *sb, char pa
rt,
strbuf_add(sb, mail, maillen);
return placeholder_len;
}
+ if (part == 'l' || part == 'L') { /* local-part */
+ const char *at = memchr(mail, '@', maillen);
+ if (at)
+ maillen = at - mail;
+ strbuf_add(sb, mail, maillen);
+ return placeholder_len;
+ }
if (!s.date_begin)
Let me submit a v2 with all the suggested changes so that you can review my work
so far. We can continue the discussion there.
P.
> + }
> + }
> + /* fall through */
> + case 'e': case 'E':
> strbuf_add(sb, mail, maillen);
> return placeholder_len;
> + default:
> + break;
> }
>
> if (!s.date_begin)
>
