Re: [PATCH] pretty: Add "%aU"|"%au" option to output author's username

From

Prarit Bhargava <prarit@redhat.com>

Date

Oct 24, 2019, 12:43 UTC

Message-ID

<4bc612f9-0b9a-1bab-9746-15ff25bd6f5f@redhat.com>

In-Reply-To

<xmqqa79qho4v.fsf@gitster-ct.c.googlers.com>

On 10/23/19 10:29 PM, Junio C Hamano wrote:

> "brian m. carlson" <sandals@crustytoothpaste.net> 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@redhat.com' 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"

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)
>