Re: [PATCH v3 3/3] pretty: add "%aL"|"%al|%cL|%cl" option to output local-part of email addresses

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 25, 2019, 05:51 UTC
Message-ID: <xmqqftjhcqz2.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20191024233617.18716-4-prarit@redhat.com>

Prarit Bhargava <prarit@redhat.com> writes:

> Subject: Re: [PATCH v3 3/3] pretty: add "%aL"|"%al|%cL|%cl" option to output local-part of

That's somewhat strange paring of quotation marks (the same appears later in the proposed log message).

I'd retitle to

    pretty: add "%aL" etc. to show local-part of email addresses

and rewrite this

Show 6 quoted lines

> Displaying only the author's username saves a lot of columns on the screen.
> For example displaying "prarit" instead of "prarit@redhat.com" saves 11
> columns.
>
> Add a "%aL"|"%al|%cL|%cl" option that output the local-part of an email
> address.

like so:

    Existing 'e/E' (as in "%ae" and "%aE") placeholders would show
    the author's address as "prarit@redhat.com", which would waste
    columns to show the same domain-part for all contributors when
    used in redhat-only project.  Introduce 'l/L' placeholders that
    strip '@' and domain part from the e-mail address.

if I were explaining this patch.

Show 10 quoted lines

> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
> index b87e2e83e6d0..13bac67c446f 100644
> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -163,6 +163,10 @@ The placeholders are:
>  '%ae':: author email
>  '%aE':: author email (respecting .mailmap, see linkgit:git-shortlog[1]
>  	or linkgit:git-blame[1])
> +'%al':: author local-part (the portion of the email address preceding the '@'
> +	symbol)

In a document like RFC2822 that is clearly about e-mail, the phrase "local-part" alone would be sufficient to convey what we are talking about, but not here. Let's say "email local-part" to qualify. That would also allow us to shorten the explanation in the parentheses, perhaps like so?

    author email local-part (the part before the '@' sign)

Show 24 quoted lines

> 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 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 */
>  		strbuf_add(sb, name, namelen);
> @@ -706,6 +706,13 @@ static size_t format_person_part(struct strbuf *sb, char part,
>  		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;
> +	}

Nicely done.

Overall, looking quite better.

Thanks.