From: Junio C Hamano <gitster@pobox.com>
To: Prarit Bhargava <prarit@redhat.com>
Cc: git@vger.kernel.org, sandals@crustytoothpaste.net, peff@peff.net,
szeder.dev@gmail.com
Subject: Re: [PATCH v3 3/3] pretty: add "%aL"|"%al|%cL|%cl" option to output local-part of email addresses
Date: Fri, 25 Oct 2019 14:51:45 +0900 [thread overview]
Message-ID: <xmqqftjhcqz2.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20191024233617.18716-4-prarit@redhat.com> (Prarit Bhargava's message of "Thu, 24 Oct 2019 19:36:17 -0400")
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
> 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.
> 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)
> 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.
prev parent reply other threads:[~2019-10-25 5:51 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-24 23:36 [PATCH v3 0/3] Implement option to output local-part of email addresses Prarit Bhargava
2019-10-24 23:36 ` [PATCH v3 1/3] t6006: Use test-lib.sh definitions Prarit Bhargava
2019-10-25 6:05 ` Junio C Hamano
2019-10-24 23:36 ` [PATCH v3 2/3] t4203: " Prarit Bhargava
2019-10-25 6:18 ` Junio C Hamano
2019-10-24 23:36 ` [PATCH v3 3/3] pretty: add "%aL"|"%al|%cL|%cl" option to output local-part of email addresses Prarit Bhargava
2019-10-25 5:51 ` Junio C Hamano [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqftjhcqz2.fsf@gitster-ct.c.googlers.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
--cc=prarit@redhat.com \
--cc=sandals@crustytoothpaste.net \
--cc=szeder.dev@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).