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

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

[Prarit Bhargava]

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 "prarit@redhat.com" saves 11
columns.

Add a "%aL"|"%al|%cL|%cl" option that output the local-part of an email
address.

Also add tests for "%ae","%an", "%ce", and "%cn".

Changes in v2:
- Changed option to 'L' based on https://www.ietf.org/rfc/rfc2822.txt
  definition of 'local-part' of email addresses.
- added additional information to documentation for %cL and %cl
- added mailmap output test
- modified code to use mailmap output for "L" option
- modified code to check if email address field has '@' symbol
- modified tests based on input from Peff

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Junio C Hamano <gitster@pobox.com>
Cc: brian m. carlson <sandals@crustytoothpaste.net>
Cc: Jeff King <peff@peff.net>
---
 Documentation/pretty-formats.txt | 10 ++++++++++
 pretty.c                         |  9 ++++++++-
 t/t4202-log.sh                   | 30 ++++++++++++++++++++++++++++++
 t/test-lib.sh                    | 10 ++++++++--
 4 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index b87e2e83e6d0..9a1f900f114a 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -163,6 +163,11 @@ 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)
+'%aL':: author local-part (the portion of the email address preceding the '@'
+	symbol, respecting .mailmap, see linkgit:git-shortlog[1] or
+	linkgit:git-blame[1])
 '%ad':: author date (format respects --date= option)
 '%aD':: author date, RFC2822 style
 '%ar':: author date, relative
@@ -175,6 +180,11 @@ The placeholders are:
 '%ce':: committer email
 '%cE':: committer email (respecting .mailmap, see
 	linkgit:git-shortlog[1] or linkgit:git-blame[1])
+'%cl':: author local-part (the portion of the email address preceding the '@'
+	symbol)
+'%cL':: author local-part (the portion of the email address preceding the '@'
+	symbol, respecting .mailmap, see linkgit:git-shortlog[1] or
+	linkgit:git-blame[1])
 '%cd':: committer date (format respects --date= option)
 '%cD':: committer date, RFC2822 style
 '%cr':: committer date, relative
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;
+	}
 
 	if (!s.date_begin)
 		goto skip;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index e803ba402e9e..fa6ecf3588b7 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1729,4 +1729,34 @@ test_expect_success 'log --end-of-options' '
        test_cmp expect actual
 '
 
+test_expect_success 'log pretty %an %ae %al %aN %aE %aL' '
+	git checkout -b anaeal &&
+	test_commit anaeal_test anaeal_test_file &&
+	git log --pretty="%an%n%ae%n%al%n%aN%n%aE%n%aL" -1 > actual &&
+	{
+		echo "${GIT_AUTHOR_NAME}" &&
+		echo "${GIT_AUTHOR_EMAIL}" &&
+		echo "${TEST_AUTHOR_LOCALNAME}"
+		echo "${GIT_AUTHOR_NAME}" &&
+		echo "${GIT_AUTHOR_EMAIL}" &&
+		echo "${TEST_AUTHOR_LOCALNAME}"
+	} > expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log pretty %cn %ce %cl %cN %cE %cL' '
+	git checkout -b cncecl &&
+	test_commit cncecl_test cncecl_test_file &&
+	git log --pretty="%cn%n%ce%n%cl%n%cN%n%cE%n%cL" -1 > actual &&
+	{
+		echo "${GIT_COMMITTER_NAME}" &&
+		echo "${GIT_COMMITTER_EMAIL}" &&
+		echo "${TEST_COMMITTER_LOCALNAME}"
+		echo "${GIT_COMMITTER_NAME}" &&
+		echo "${GIT_COMMITTER_EMAIL}" &&
+		echo "${TEST_COMMITTER_LOCALNAME}"
+	} > expect &&
+	test_cmp expect actual
+'
+
 test_done
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e06fa02a0eec..5ef0ad8c1a2a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -404,14 +404,20 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
 unset XDG_CACHE_HOME
 unset XDG_CONFIG_HOME
 unset GITPERLLIB
-GIT_AUTHOR_EMAIL=author@example.com
+TEST_AUTHOR_LOCALNAME=author
+TEST_AUTHOR_DOMAIN=example.com
+GIT_AUTHOR_EMAIL=${TEST_AUTHOR_LOCALNAME}@${TEST_AUTHOR_DOMAIN}
 GIT_AUTHOR_NAME='A U Thor'
-GIT_COMMITTER_EMAIL=committer@example.com
+TEST_COMMITTER_LOCALNAME=committer
+TEST_COMMITTER_DOMAIN=example.com
+GIT_COMMITTER_EMAIL=${TEST_COMMITTER_LOCALNAME}@${TEST_COMMITTER_DOMAIN}
 GIT_COMMITTER_NAME='C O Mitter'
 GIT_MERGE_VERBOSITY=5
 GIT_MERGE_AUTOEDIT=no
 export GIT_MERGE_VERBOSITY GIT_MERGE_AUTOEDIT
+export TEST_AUTHOR_LOCALNAME TEST_AUTHOR_DOMAIN
 export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
+export TEST_COMMITTER_LOCALNAME TEST_COMMITTER_DOMAIN
 export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
 export EDITOR
 
-- 
2.21.0

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

[Jeff King]

On Thu, Oct 24, 2019 at 08:53:32AM -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 "prarit@redhat.com" saves 11
> columns.
> 
> Add a "%aL"|"%al|%cL|%cl" option that output the local-part of an email
> address.
> 
> Also add tests for "%ae","%an", "%ce", and "%cn".

Thanks, this is looking better, but I think there are still a few minor
bits to address.

> Changes in v2:
> - Changed option to 'L' based on https://www.ietf.org/rfc/rfc2822.txt
>   definition of 'local-part' of email addresses.
> - added additional information to documentation for %cL and %cl
> - added mailmap output test
> - modified code to use mailmap output for "L" option
> - modified code to check if email address field has '@' symbol
> - modified tests based on input from Peff

This change list is very welcome, but it should generally go after the
"---", so that it's not part of the commit message (i.e., it is for
people reading this email and reviewing now, but people reading "git
log" later would not know or care about v1).

> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: Junio C Hamano <gitster@pobox.com>
> Cc: brian m. carlson <sandals@crustytoothpaste.net>
> Cc: Jeff King <peff@peff.net>

Likewise we do not generally use "cc:" trailers in this project.

> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
> index b87e2e83e6d0..9a1f900f114a 100644
> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -163,6 +163,11 @@ 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)

I'm not sure if it is worth saying something like "preceding the @
symbol, or the whole address if no @ symbol". It's a pretty rare case,
I'd think, and it does clutter up the wording. So just a thought.

> +'%aL':: author local-part (the portion of the email address preceding the '@'
> +	symbol, respecting .mailmap, see linkgit:git-shortlog[1] or
> +	linkgit:git-blame[1])

This description gets pretty long. I wonder if we can simplify by
referring to earlier formats, which would also make clear to the user
the relationship between the formats. Perhaps:

  '%aL':: author local-part (see '%al'), respecting .mailmap (see '%aE')

And ditto for %cL.

> 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 */

I think this is sufficient to fix the mailmap issue. Good.

> @@ -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;
> +	}

And I think this does the counting correctly (mail[maillen] is the "@",
so we wouldn't include it; good).

> +test_expect_success 'log pretty %an %ae %al %aN %aE %aL' '
> +	git checkout -b anaeal &&
> +	test_commit anaeal_test anaeal_test_file &&

Not a big deal, but we could simplify this a bit, I think. There's no
need to make a new branch. And there's no need to specify the filename
to test_commit. IMHO it's worth being as simple as possible in the
tests, because then a reader doesn't have to think about whether those
details are important or not.

> +	git log --pretty="%an%n%ae%n%al%n%aN%n%aE%n%aL" -1 > actual &&

I didn't think about this before, but...surely we're testing %an, etc
already?

Indeed, it looks like t6006 already covers that. Maybe you should be
adding to that test? I note that it just hardcodes "author@example.com"
in the expectation. I'd be OK with either following the lead there, or
doing a separate preparatory patch to use $GIT_AUTHOR_EMAIL, etc.

> +	{
> +		echo "${GIT_AUTHOR_NAME}" &&
> +		echo "${GIT_AUTHOR_EMAIL}" &&
> +		echo "${TEST_AUTHOR_LOCALNAME}"
> +		echo "${GIT_AUTHOR_NAME}" &&
> +		echo "${GIT_AUTHOR_EMAIL}" &&
> +		echo "${TEST_AUTHOR_LOCALNAME}"
> +	} > expect &&

The expectation for %aE is the same as for %ae. So we're not really
testing that we actually applied the mailmap. It looks like t4203 has
some tests for %aE; you'd probably want to check %aL there.

> +test_expect_success 'log pretty %cn %ce %cl %cN %cE %cL' '

Likewise, both of the spots I mentioned cover the committer formats,
too.

> +TEST_AUTHOR_LOCALNAME=author
> +TEST_AUTHOR_DOMAIN=example.com
> +GIT_AUTHOR_EMAIL=${TEST_AUTHOR_LOCALNAME}@${TEST_AUTHOR_DOMAIN}

If you follow the lead of t6006 and just hard-code these, then this hunk
can go away. But if you do keep it, note that...

>  export GIT_MERGE_VERBOSITY GIT_MERGE_AUTOEDIT
> +export TEST_AUTHOR_LOCALNAME TEST_AUTHOR_DOMAIN
>  export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
> +export TEST_COMMITTER_LOCALNAME TEST_COMMITTER_DOMAIN

These lines are unnecessary. We have to export GIT_AUTHOR_EMAIL, etc, so
that the child git-commit process can read it. But there's no need to do
so for TEST_*, which are meant to be used by the test scripts
themselves.

-Peff

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

[Prarit Bhargava]

On 10/24/19 12:16 PM, Jeff King wrote:
> On Thu, Oct 24, 2019 at 08:53:32AM -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 "prarit@redhat.com" saves 11
>> columns.
>>
>> Add a "%aL"|"%al|%cL|%cl" option that output the local-part of an email
>> address.
>>
>> Also add tests for "%ae","%an", "%ce", and "%cn".
>
> Thanks, this is looking better, but I think there are still a few minor
> bits to address.
>

Thanks :)

>> Changes in v2:
>> - Changed option to 'L' based on https://www.ietf.org/rfc/rfc2822.txt
>>   definition of 'local-part' of email addresses.
>> - added additional information to documentation for %cL and %cl
>> - added mailmap output test
>> - modified code to use mailmap output for "L" option
>> - modified code to check if email address field has '@' symbol
>> - modified tests based on input from Peff
>
> This change list is very welcome, but it should generally go after the
> "---", so that it's not part of the commit message (i.e., it is for
> people reading this email and reviewing now, but people reading "git
> log" later would not know or care about v1).
>
>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>> Cc: Junio C Hamano <gitster@pobox.com>
>> Cc: brian m. carlson <sandals@crustytoothpaste.net>
>> Cc: Jeff King <peff@peff.net>
>
> Likewise we do not generally use "cc:" trailers in this project.
>
>> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
>> index b87e2e83e6d0..9a1f900f114a 100644
>> --- a/Documentation/pretty-formats.txt
>> +++ b/Documentation/pretty-formats.txt
>> @@ -163,6 +163,11 @@ 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)
>
> I'm not sure if it is worth saying something like "preceding the @
> symbol, or the whole address if no @ symbol". It's a pretty rare case,
> I'd think, and it does clutter up the wording. So just a thought.
>

After adding the text I've decided to keep it the same.  As you pointed out it
ended up being too long.

>> +'%aL':: author local-part (the portion of the email address preceding the '@'
>> +	symbol, respecting .mailmap, see linkgit:git-shortlog[1] or
>> +	linkgit:git-blame[1])
>
> This description gets pretty long. I wonder if we can simplify by
> referring to earlier formats, which would also make clear to the user
> the relationship between the formats. Perhaps:
>
>   '%aL':: author local-part (see '%al'), respecting .mailmap (see '%aE')
>
> And ditto for %cL.

I have added this for v3.

<snip>

> I didn't think about this before, but...surely we're testing %an, etc
> already?
>
> Indeed, it looks like t6006 already covers that. Maybe you should be
> adding to that test? I note that it just hardcodes "author@example.com"
> in the expectation. I'd be OK with either following the lead there, or
> doing a separate preparatory patch to use $GIT_AUTHOR_EMAIL, etc.
>

I've done separate preparatory patches for both t6006 and t4203 for v3.

>> +	{
>> +		echo "${GIT_AUTHOR_NAME}" &&
>> +		echo "${GIT_AUTHOR_EMAIL}" &&
>> +		echo "${TEST_AUTHOR_LOCALNAME}"
>> +		echo "${GIT_AUTHOR_NAME}" &&
>> +		echo "${GIT_AUTHOR_EMAIL}" &&
>> +		echo "${TEST_AUTHOR_LOCALNAME}"
>> +	} > expect &&
>
> The expectation for %aE is the same as for %ae. So we're not really
> testing that we actually applied the mailmap. It looks like t4203 has
> some tests for %aE; you'd probably want to check %aL there.
>
>> +test_expect_success 'log pretty %cn %ce %cl %cN %cE %cL' '
>
> Likewise, both of the spots I mentioned cover the committer formats,
> too.
>
>> +TEST_AUTHOR_LOCALNAME=author
>> +TEST_AUTHOR_DOMAIN=example.com
>> +GIT_AUTHOR_EMAIL=${TEST_AUTHOR_LOCALNAME}@${TEST_AUTHOR_DOMAIN}
>
> If you follow the lead of t6006 and just hard-code these, then this hunk
> can go away. But if you do keep it, note that...
>

I implemented the suggested tests in both test files in v3.

>>  export GIT_MERGE_VERBOSITY GIT_MERGE_AUTOEDIT
>> +export TEST_AUTHOR_LOCALNAME TEST_AUTHOR_DOMAIN
>>  export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
>> +export TEST_COMMITTER_LOCALNAME TEST_COMMITTER_DOMAIN
>
> These lines are unnecessary. We have to export GIT_AUTHOR_EMAIL, etc, so
> that the child git-commit process can read it. But there's no need to do
> so for TEST_*, which are meant to be used by the test scripts
> themselves.
>

Removed in v3.  I will post after running the test suite.

Thanks,

P.

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

[SZEDER Gábor]

Just a couple of test nits:

On Thu, Oct 24, 2019 at 08:53:32AM -0400, Prarit Bhargava wrote:
> +test_expect_success 'log pretty %an %ae %al %aN %aE %aL' '
> +	git checkout -b anaeal &&
> +	test_commit anaeal_test anaeal_test_file &&
> +	git log --pretty="%an%n%ae%n%al%n%aN%n%aE%n%aL" -1 > actual &&

Style: no space between redirection and filename, i.e. >actual

> +	{
> +		echo "${GIT_AUTHOR_NAME}" &&
> +		echo "${GIT_AUTHOR_EMAIL}" &&
> +		echo "${TEST_AUTHOR_LOCALNAME}"

Broken &&-chain (though with just a bunch of echos it won't really
make much of a difference)

> +		echo "${GIT_AUTHOR_NAME}" &&
> +		echo "${GIT_AUTHOR_EMAIL}" &&
> +		echo "${TEST_AUTHOR_LOCALNAME}"

All these variables stand on their own, so the curly braces around
them are unnecessary.

> +	} > expect &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'log pretty %cn %ce %cl %cN %cE %cL' '
> +	git checkout -b cncecl &&
> +	test_commit cncecl_test cncecl_test_file &&
> +	git log --pretty="%cn%n%ce%n%cl%n%cN%n%cE%n%cL" -1 > actual &&
> +	{
> +		echo "${GIT_COMMITTER_NAME}" &&
> +		echo "${GIT_COMMITTER_EMAIL}" &&
> +		echo "${TEST_COMMITTER_LOCALNAME}"
> +		echo "${GIT_COMMITTER_NAME}" &&
> +		echo "${GIT_COMMITTER_EMAIL}" &&
> +		echo "${TEST_COMMITTER_LOCALNAME}"
> +	} > expect &&
> +	test_cmp expect actual
> +'
> +

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

[Prarit Bhargava]

On 10/24/19 4:29 PM, SZEDER Gábor wrote:
> 
> Just a couple of test nits:
> 
> On Thu, Oct 24, 2019 at 08:53:32AM -0400, Prarit Bhargava wrote:
>> +test_expect_success 'log pretty %an %ae %al %aN %aE %aL' '
>> +	git checkout -b anaeal &&
>> +	test_commit anaeal_test anaeal_test_file &&
>> +	git log --pretty="%an%n%ae%n%al%n%aN%n%aE%n%aL" -1 > actual &&
> 
> Style: no space between redirection and filename, i.e. >actual
> 
>> +	{
>> +		echo "${GIT_AUTHOR_NAME}" &&
>> +		echo "${GIT_AUTHOR_EMAIL}" &&
>> +		echo "${TEST_AUTHOR_LOCALNAME}"
> 
> Broken &&-chain (though with just a bunch of echos it won't really
> make much of a difference)
> 
>> +		echo "${GIT_AUTHOR_NAME}" &&
>> +		echo "${GIT_AUTHOR_EMAIL}" &&
>> +		echo "${TEST_AUTHOR_LOCALNAME}"
> 
> All these variables stand on their own, so the curly braces around
> them are unnecessary.

I've changed this code (based on Peff's suggestions) to other tests, however,
AFAIK using braces around is considered "good practice".

It can't harm anything to have braces but if the preferred git coding style is
to only use them when necessary I will remove them.

P.

> 
>> +	} > expect &&
>> +	test_cmp expect actual
>> +'
>> +
>> +test_expect_success 'log pretty %cn %ce %cl %cN %cE %cL' '
>> +	git checkout -b cncecl &&
>> +	test_commit cncecl_test cncecl_test_file &&
>> +	git log --pretty="%cn%n%ce%n%cl%n%cN%n%cE%n%cL" -1 > actual &&
>> +	{
>> +		echo "${GIT_COMMITTER_NAME}" &&
>> +		echo "${GIT_COMMITTER_EMAIL}" &&
>> +		echo "${TEST_COMMITTER_LOCALNAME}"
>> +		echo "${GIT_COMMITTER_NAME}" &&
>> +		echo "${GIT_COMMITTER_EMAIL}" &&
>> +		echo "${TEST_COMMITTER_LOCALNAME}"
>> +	} > expect &&
>> +	test_cmp expect actual
>> +'
>> +

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

[Junio C Hamano]

Prarit Bhargava <prarit@redhat.com> writes:

> It can't harm anything to have braces but if the preferred git coding style is
> to only use them when necessary I will remove them.

It harms readability.  Don't try to be different just for the sake
of being different, especially when you think "it can't harm
anything" aka "there is no strong reason to do so (or not to)".
Instead, help others by making sure that their eyes do not have to
get distracted for such meaningless differences from existing parts
of the system.

Thanks.

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

[Prarit Bhargava]

On 10/25/19 1:08 AM, Junio C Hamano wrote:
> Prarit Bhargava <prarit@redhat.com> writes:
> 
>> It can't harm anything to have braces but if the preferred git coding style is
>> to only use them when necessary I will remove them.
> 
> It harms readability.  Don't try to be different just for the sake
> of being different, especially when you think "it can't harm
> anything" aka "there is no strong reason to do so (or not to)".
> Instead, help others by making sure that their eyes do not have to
> get distracted for such meaningless differences from existing parts
> of the system.

Ha :)  I did pull them for v3 after doing a search.  I didn't reply properly and
should have said "I'll do a search".  Admittedly I'm concerned in a few spots in
the code where it *seems* like something could go wrong if the code is changed.
But nothing worth patching over.

P.

> 
> Thanks.
>