On Tue, Oct 22, 2019 at 07:28:47PM -0400, Prarit Bhargava wrote:
> In many projects the number of contributors is low enough that users know
> each other and the full email address doesn't need to be displayed.
> Displaying only the author's username saves a lot of columns on the screen.
> For example displaying "prarit" instead of "pra...@redhat.com" saves 11
> columns.
>
> Add a "%aU"|"%au" option that outputs the author's email username.
Like others, this seems potentially useful even if I probably wouldn't
use it myself. Another more complicated way to think of it would be to
give a list of domains to omit (so if 90% of the committers are
@redhat.com, we can skip that, but the one-off contributor from another
domain gets their fully qualified name.
But that's a lot more complicated. I don't mind doing the easy thing
now, and even if we later grew the more complicated thing, I wouldn't be
sad to still have this easy one as an option.
> --- a/pretty.c
> +++ b/pretty.c
> @@ -706,6 +706,11 @@ static size_t format_person_part(struct strbuf *sb, char
> part,
> strbuf_add(sb, mail, maillen);
> return placeholder_len;
> }
> + if (part == 'u' || part == 'U') { /* username */
> + maillen = strstr(s.mail_begin, "@") - s.mail_begin;
> + strbuf_add(sb, mail, maillen);
> + return placeholder_len;
> + }
What happens if the email doesn't have an "@"? I think you'd either end
up printing a bunch of extra cruft (because you're not limiting the
search for "@" to the boundaries from split_ident_line) or you'd
crash (if there's no "@" at all, and you'd get a huge maillen).
There's also no need to use the slower strstr() when looking for a
single character. So perhaps:
const char *at = memchr(mail, '@', maillen);
if (at)
maillen = at - mail;
strbuf_add(sb, mail, maillen);
> +test_expect_success 'log pretty %an %ae %au' '
As others noted, this could cover %aU, too (which is broken; you need to
handle 'U' alongside 'E' and 'N' earlier in format_person_part()).
> + git checkout -b anaeau &&
> + test_commit anaeau_test anaeau_test_file &&
> + git log --pretty="%an" > actual &&
> + git log --pretty="%ae" >> actual &&
> + git log --pretty="%au" >> actual &&
Maybe:
git log --pretty="%an %ae %au"
or
git log --pretty="%an%n%ae%n%au"
which is shorter and runs more efficiently?
> + git log > full &&
> + name=$(cat full | grep "^Author: " | awk -F "Author: " " { print \$2 }
> " | awk -F " <" " { print \$1 } ") &&
> + email=$(cat full | grep "^Author: " | awk -F "<" " { print \$2 } " |
> awk -F ">" " { print \$1 } ") &&
> + username=$(cat full | grep "^Author: " | awk -F "<" " { print \$2 } " |
> awk -F ">" " { print \$1 } " | awk -F "@" " { print \$1 } " ) &&
> + echo "${name}" > expect &&
> + echo "${email}" >> expect &&
> + echo "${username}" >> expect &&
These values come from our hard-coded test setup, so it would be more
readable to just expect those:
{
echo "$GIT_AUTHOR_NAME" &&
echo "$GIT_AUTHOR_EMAIL" &&
echo "$GIT_AUTHOR_EMAIL" | sed "s/@.*//"
} >expect
For the last one, also I wouldn't be upset to see test-lib.sh do
something like:
TEST_AUTHOR_USERNAME=author
TEST_AUTHOR_DOMAIN=example.com
GIT_AUTHOR_NAME=$TEST_AUTHOR_USERNAME@$TEST_AUTHOR_DOMAIN
to let tests like this pick out the individual values if they want.
-Peff
