git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* Git is removing . from the last part of user.name
@ 2021-08-22 14:19 Daniel P.
  2021-08-22 15:33 ` brian m. carlson
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel P. @ 2021-08-22 14:19 UTC (permalink / raw)
  To: git

If user.name's  value has a . as the last character of the last part
of the name, git is removing it from commit operations. But git-config
shows the .

example:

in .gitconfig:

[user]
    name = Daniel P.


`git config user.name`:

user.name=Daniel P.


from `git show`:

Author: Daniel P <danpltile@gmail.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git is removing . from the last part of user.name
  2021-08-22 14:19 Git is removing . from the last part of user.name Daniel P.
@ 2021-08-22 15:33 ` brian m. carlson
  2021-08-22 19:28   ` Jeff King
  0 siblings, 1 reply; 15+ messages in thread
From: brian m. carlson @ 2021-08-22 15:33 UTC (permalink / raw)
  To: Daniel P.; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 941 bytes --]

On 2021-08-22 at 14:19:33, Daniel P. wrote:
> If user.name's  value has a . as the last character of the last part
> of the name, git is removing it from commit operations. But git-config
> shows the .
> 
> example:
> 
> in .gitconfig:
> 
> [user]
>     name = Daniel P.
> 
> 
> `git config user.name`:
> 
> user.name=Daniel P.
> 
> 
> from `git show`:
> 
> Author: Daniel P <danpltile@gmail.com>

Yes, it does appear we do that.  We consider a period to be "crud" and
strip off trailing crud.  I think we should probably change that, since
in some places people write their family name first, and so a name like
“carlson brian m.” might be a thing people might want to write, in
addition to this particular case.

In any event, it's not very polite to "correct" people's names for them.
I myself have certainly run into that often enough.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git is removing . from the last part of user.name
  2021-08-22 15:33 ` brian m. carlson
@ 2021-08-22 19:28   ` Jeff King
  2021-08-22 19:35     ` Jeff King
                       ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Jeff King @ 2021-08-22 19:28 UTC (permalink / raw)
  To: brian m. carlson; +Cc: Daniel P., git

On Sun, Aug 22, 2021 at 03:33:31PM +0000, brian m. carlson wrote:

> On 2021-08-22 at 14:19:33, Daniel P. wrote:
> > If user.name's  value has a . as the last character of the last part
> > of the name, git is removing it from commit operations. But git-config
> > shows the .
> > 
> > example:
> > 
> > in .gitconfig:
> > 
> > [user]
> >     name = Daniel P.
> > 
> > 
> > `git config user.name`:
> > 
> > user.name=Daniel P.
> > 
> > 
> > from `git show`:
> > 
> > Author: Daniel P <danpltile@gmail.com>
> 
> Yes, it does appear we do that.  We consider a period to be "crud" and
> strip off trailing crud.  I think we should probably change that, since
> in some places people write their family name first, and so a name like
> “carlson brian m.” might be a thing people might want to write, in
> addition to this particular case.
> 
> In any event, it's not very polite to "correct" people's names for them.
> I myself have certainly run into that often enough.

A lot of this name-cleanup code came from an era where we were inferring
names from gecos fields or from hacky email parsing.

I agree that if somebody has given us a definite name via config, we
should mostly leave it intact (the exception being syntactic elements
like <>). But we may still want to keep some of the "crud" cleanup when
we are pulling from those other sources.

OTOH, this crud stuff goes all the way back to 5e5128ed1c (Remove
extraneous ',' ';' and '.' characters from the full name gecos field.,
2005-04-17). We warn in pretty big letters these days about pulling an
ident from gecos, and our rfc822 parsing is more robust than it once
was. So it may be time to just retire most of it. The unfortunate thing
is we won't know how many people complain until it's released.

On a somewhat lesser note, I'm tempted to say that "." probably was
never that useful (compared to say, comma, which is the gecos
separator), and we could probably just drop it from the crud list.

-Peff

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git is removing . from the last part of user.name
  2021-08-22 19:28   ` Jeff King
@ 2021-08-22 19:35     ` Jeff King
  2021-08-23 16:25     ` Junio C Hamano
  2023-07-30 21:38     ` Thomas J. Faughnan Jr.
  2 siblings, 0 replies; 15+ messages in thread
From: Jeff King @ 2021-08-22 19:35 UTC (permalink / raw)
  To: brian m. carlson; +Cc: Daniel P., git

On Sun, Aug 22, 2021 at 03:28:57PM -0400, Jeff King wrote:

> On a somewhat lesser note, I'm tempted to say that "." probably was
> never that useful (compared to say, comma, which is the gecos
> separator), and we could probably just drop it from the crud list.

This does break a few tests, but none that I think explicitly were
arguing for keeping the dot. One was just a general crud test, and the
other was just documenting the current behavior while testing something
else (and perhaps even argues _for_ the change, as somebody bothered to
write --author='Jane D.' in the first place).

---
diff --git a/ident.c b/ident.c
index 85d9ba7120..2d136d27c8 100644
--- a/ident.c
+++ b/ident.c
@@ -198,7 +198,6 @@ void reset_ident_date(void)
 static int crud(unsigned char c)
 {
 	return  c <= 32  ||
-		c == '.' ||
 		c == ',' ||
 		c == ':' ||
 		c == ';' ||
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 0b2d21ec55..a1cdc2905f 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -466,7 +466,7 @@ test_expect_success 'gitmailmap(5) example output: example #1' '
 	Author Jane Doe <jane@laptop.(none)> maps to Jane Doe <jane@laptop.(none)>
 	Committer C O Mitter <committer@example.com> maps to C O Mitter <committer@example.com>
 
-	Author Jane D <jane@desktop.(none)> maps to Jane Doe <jane@desktop.(none)>
+	Author Jane D. <jane@desktop.(none)> maps to Jane Doe <jane@desktop.(none)>
 	Committer C O Mitter <committer@example.com> maps to C O Mitter <committer@example.com>
 	EOF
 	git -C doc log --reverse --pretty=format:"Author %an <%ae> maps to %aN <%aE>%nCommitter %cn <%ce> maps to %cN <%cE>%n" >actual &&
@@ -494,7 +494,7 @@ test_expect_success 'gitmailmap(5) example output: example #2' '
 	Author Jane Doe <jane@laptop.(none)> maps to Jane Doe <jane@example.com>
 	Committer C O Mitter <committer@example.com> maps to C O Mitter <committer@example.com>
 
-	Author Jane D <jane@desktop.(none)> maps to Jane Doe <jane@example.com>
+	Author Jane D. <jane@desktop.(none)> maps to Jane Doe <jane@example.com>
 	Committer C O Mitter <committer@example.com> maps to C O Mitter <committer@example.com>
 	EOF
 	git -C doc log --reverse --pretty=format:"Author %an <%ae> maps to %aN <%aE>%nCommitter %cn <%ce> maps to %cN <%cE>%n" >actual &&
diff --git a/t/t7518-ident-corner-cases.sh b/t/t7518-ident-corner-cases.sh
index 905957bd0a..738c723861 100755
--- a/t/t7518-ident-corner-cases.sh
+++ b/t/t7518-ident-corner-cases.sh
@@ -18,7 +18,7 @@ test_expect_success 'empty name and missing email' '
 '
 
 test_expect_success 'commit rejects all-crud name' '
-	test_must_fail env GIT_AUTHOR_NAME=" .;<>" \
+	test_must_fail env GIT_AUTHOR_NAME=" ,;<>" \
 		git commit --allow-empty -m foo
 '
 

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: Git is removing . from the last part of user.name
  2021-08-22 19:28   ` Jeff King
  2021-08-22 19:35     ` Jeff King
@ 2021-08-23 16:25     ` Junio C Hamano
  2023-07-30 21:38     ` Thomas J. Faughnan Jr.
  2 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2021-08-23 16:25 UTC (permalink / raw)
  To: Jeff King; +Cc: brian m. carlson, Daniel P., git

Jeff King <peff@peff.net> writes:

> On a somewhat lesser note, I'm tempted to say that "." probably was
> never that useful (compared to say, comma, which is the gecos
> separator), and we could probably just drop it from the crud list.

Yeah, the only excuse I can think of using to justify treating "."
any specially can be seen in the contrast between the To: and Cc:
header of this message, where the human-readable part of brian and
daniel must be inside a pair of double-quotes while your name needs
no such quoting.

But we do not remove "crud()" crud from the middle, so that's not
even it (the only crud we remvoe from the middle are newlines and
'<' and '>').




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git is removing . from the last part of user.name
  2021-08-22 19:28   ` Jeff King
  2021-08-22 19:35     ` Jeff King
  2021-08-23 16:25     ` Junio C Hamano
@ 2023-07-30 21:38     ` Thomas J. Faughnan Jr.
  2023-07-31 15:56       ` Junio C Hamano
  2 siblings, 1 reply; 15+ messages in thread
From: Thomas J. Faughnan Jr. @ 2023-07-30 21:38 UTC (permalink / raw)
  To: Jeff King, brian m. carlson; +Cc: Daniel P., git

> On a somewhat lesser note, I'm tempted to say that "." probably was
> never that useful (compared to say, comma, which is the gecos
> separator), and we could probably just drop it from the crud list.

Would this change still be considered? Or alternatively a git config
option to ignore "." when checking if a character is crud?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git is removing . from the last part of user.name
  2023-07-30 21:38     ` Thomas J. Faughnan Jr.
@ 2023-07-31 15:56       ` Junio C Hamano
  2023-07-31 21:02         ` brian m. carlson
  0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2023-07-31 15:56 UTC (permalink / raw)
  To: Thomas J. Faughnan Jr.; +Cc: Jeff King, brian m. carlson, Daniel P., git

"Thomas J. Faughnan Jr." <thomas@faughnan.net> writes:

>> On a somewhat lesser note, I'm tempted to say that "." probably was
>> never that useful (compared to say, comma, which is the gecos
>> separator), and we could probably just drop it from the crud list.
>
> Would this change still be considered? Or alternatively a git config
> option to ignore "." when checking if a character is crud?

That is certainly a blast from the past ;-)

I actually was wondering the opposite should be done, i.e. be
consistent and remove '.'  just like '<' and '>' even from the
middle of names.  '.' in the human-readable name part anywhere on
To: and Cc: lines, not just at the end, causes mailers to barf,
unless such name is quoted.

And from the consistency point of view, I think that a configuration
variable that tells Git to include/exclude "." from the "crud()"
letter should make us drop/keep "." from anywhere in the name, not
just at the tail end.  If we decide that adding such a configuration
variable is a good idea, that is.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git is removing . from the last part of user.name
  2023-07-31 15:56       ` Junio C Hamano
@ 2023-07-31 21:02         ` brian m. carlson
  2023-07-31 21:44           ` [PATCH] ident: don't consider trailing dot crud brian m. carlson
  0 siblings, 1 reply; 15+ messages in thread
From: brian m. carlson @ 2023-07-31 21:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas J. Faughnan Jr., Jeff King, Daniel P., git

[-- Attachment #1: Type: text/plain, Size: 1085 bytes --]

On 2023-07-31 at 15:56:09, Junio C Hamano wrote:
> "Thomas J. Faughnan Jr." <thomas@faughnan.net> writes:
> 
> >> On a somewhat lesser note, I'm tempted to say that "." probably was
> >> never that useful (compared to say, comma, which is the gecos
> >> separator), and we could probably just drop it from the crud list.
> >
> > Would this change still be considered? Or alternatively a git config
> > option to ignore "." when checking if a character is crud?
> 
> That is certainly a blast from the past ;-)
> 
> I actually was wondering the opposite should be done, i.e. be
> consistent and remove '.'  just like '<' and '>' even from the
> middle of names.  '.' in the human-readable name part anywhere on
> To: and Cc: lines, not just at the end, causes mailers to barf,
> unless such name is quoted.

For obvious reasons, I think that's a terrible idea.  I don't think we
should have such a config option, and should instead just drop "." from
the crud list.  I would welcome such a change.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH] ident: don't consider trailing dot crud
  2023-07-31 21:02         ` brian m. carlson
@ 2023-07-31 21:44           ` brian m. carlson
  2023-07-31 21:49             ` Junio C Hamano
  2023-07-31 21:56             ` Thomas J. Faughnan Jr.
  0 siblings, 2 replies; 15+ messages in thread
From: brian m. carlson @ 2023-07-31 21:44 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, Thomas J. Faughnan Jr., Daniel P.

When we process a user's name (as in user.name), we strip all trailing
crud from it.  Right now, we consider a dot trailing crud, and strip it
off.

However, this is unsuitable for many personal names because humans
frequently have abbreviated suffixes, such as "Jr." or "Sr." at the end
of their names, and this corrupts them.  Some other users may wish to
use an abbreviated name or initial, which will pose a problem especially
in cultures that write the family name first, followed by the personal
name.

Since the current approach causes lots of practical problems, let's
avoid it by no longer considering a dot to be crud.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 ident.c                       |  1 -
 t/t4203-mailmap.sh            |  4 ++--
 t/t7518-ident-corner-cases.sh | 11 ++++++++++-
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/ident.c b/ident.c
index 08be4d0747..cc7afdbf81 100644
--- a/ident.c
+++ b/ident.c
@@ -203,7 +203,6 @@ void reset_ident_date(void)
 static int crud(unsigned char c)
 {
 	return  c <= 32  ||
-		c == '.' ||
 		c == ',' ||
 		c == ':' ||
 		c == ';' ||
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index fa7f987284..2016132f51 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -466,7 +466,7 @@ test_expect_success 'gitmailmap(5) example output: example #1' '
 	Author Jane Doe <jane@laptop.(none)> maps to Jane Doe <jane@laptop.(none)>
 	Committer C O Mitter <committer@example.com> maps to C O Mitter <committer@example.com>
 
-	Author Jane D <jane@desktop.(none)> maps to Jane Doe <jane@desktop.(none)>
+	Author Jane D. <jane@desktop.(none)> maps to Jane Doe <jane@desktop.(none)>
 	Committer C O Mitter <committer@example.com> maps to C O Mitter <committer@example.com>
 	EOF
 	git -C doc log --reverse --pretty=format:"Author %an <%ae> maps to %aN <%aE>%nCommitter %cn <%ce> maps to %cN <%cE>%n" >actual &&
@@ -494,7 +494,7 @@ test_expect_success 'gitmailmap(5) example output: example #2' '
 	Author Jane Doe <jane@laptop.(none)> maps to Jane Doe <jane@example.com>
 	Committer C O Mitter <committer@example.com> maps to C O Mitter <committer@example.com>
 
-	Author Jane D <jane@desktop.(none)> maps to Jane Doe <jane@example.com>
+	Author Jane D. <jane@desktop.(none)> maps to Jane Doe <jane@example.com>
 	Committer C O Mitter <committer@example.com> maps to C O Mitter <committer@example.com>
 	EOF
 	git -C doc log --reverse --pretty=format:"Author %an <%ae> maps to %aN <%aE>%nCommitter %cn <%ce> maps to %cN <%cE>%n" >actual &&
diff --git a/t/t7518-ident-corner-cases.sh b/t/t7518-ident-corner-cases.sh
index fffdb6ff2e..9ab2ae2f3b 100755
--- a/t/t7518-ident-corner-cases.sh
+++ b/t/t7518-ident-corner-cases.sh
@@ -20,10 +20,19 @@ test_expect_success 'empty name and missing email' '
 '
 
 test_expect_success 'commit rejects all-crud name' '
-	test_must_fail env GIT_AUTHOR_NAME=" .;<>" \
+	test_must_fail env GIT_AUTHOR_NAME=" ,;<>" \
 		git commit --allow-empty -m foo
 '
 
+test_expect_success 'commit does not strip trailing dot' '
+	author_name="Pat Doe Jr." &&
+	env GIT_AUTHOR_NAME="$author_name" \
+		git commit --allow-empty -m foo &&
+	git log -1 --format=%an >actual &&
+	echo "$author_name" >expected &&
+	test_cmp actual expected
+'
+
 # We must test the actual error message here, as an unwanted
 # auto-detection could fail for other reasons.
 test_expect_success 'empty configured name does not auto-detect' '

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] ident: don't consider trailing dot crud
  2023-07-31 21:44           ` [PATCH] ident: don't consider trailing dot crud brian m. carlson
@ 2023-07-31 21:49             ` Junio C Hamano
  2023-08-02 16:49               ` Junio C Hamano
  2023-07-31 21:56             ` Thomas J. Faughnan Jr.
  1 sibling, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2023-07-31 21:49 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Jeff King, Thomas J. Faughnan Jr., Daniel P.

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> When we process a user's name (as in user.name), we strip all trailing
> crud from it.  Right now, we consider a dot trailing crud, and strip it
> off.

We consider a leading or trailing dot crud, I think (applies also to
the title of the patch).  Otherwise the change, together with the
test updates, all look good.

I wonder if this needs some credit to those involved in the original
thread?

Thanks.

> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  ident.c                       |  1 -
>  t/t4203-mailmap.sh            |  4 ++--
>  t/t7518-ident-corner-cases.sh | 11 ++++++++++-
>  3 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/ident.c b/ident.c
> index 08be4d0747..cc7afdbf81 100644
> --- a/ident.c
> +++ b/ident.c
> @@ -203,7 +203,6 @@ void reset_ident_date(void)
>  static int crud(unsigned char c)
>  {
>  	return  c <= 32  ||
> -		c == '.' ||
>  		c == ',' ||
>  		c == ':' ||
>  		c == ';' ||
> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
> index fa7f987284..2016132f51 100755
> --- a/t/t4203-mailmap.sh
> +++ b/t/t4203-mailmap.sh
> @@ -466,7 +466,7 @@ test_expect_success 'gitmailmap(5) example output: example #1' '
>  	Author Jane Doe <jane@laptop.(none)> maps to Jane Doe <jane@laptop.(none)>
>  	Committer C O Mitter <committer@example.com> maps to C O Mitter <committer@example.com>
>  
> -	Author Jane D <jane@desktop.(none)> maps to Jane Doe <jane@desktop.(none)>
> +	Author Jane D. <jane@desktop.(none)> maps to Jane Doe <jane@desktop.(none)>
>  	Committer C O Mitter <committer@example.com> maps to C O Mitter <committer@example.com>
>  	EOF
>  	git -C doc log --reverse --pretty=format:"Author %an <%ae> maps to %aN <%aE>%nCommitter %cn <%ce> maps to %cN <%cE>%n" >actual &&
> @@ -494,7 +494,7 @@ test_expect_success 'gitmailmap(5) example output: example #2' '
>  	Author Jane Doe <jane@laptop.(none)> maps to Jane Doe <jane@example.com>
>  	Committer C O Mitter <committer@example.com> maps to C O Mitter <committer@example.com>
>  
> -	Author Jane D <jane@desktop.(none)> maps to Jane Doe <jane@example.com>
> +	Author Jane D. <jane@desktop.(none)> maps to Jane Doe <jane@example.com>
>  	Committer C O Mitter <committer@example.com> maps to C O Mitter <committer@example.com>
>  	EOF
>  	git -C doc log --reverse --pretty=format:"Author %an <%ae> maps to %aN <%aE>%nCommitter %cn <%ce> maps to %cN <%cE>%n" >actual &&
> diff --git a/t/t7518-ident-corner-cases.sh b/t/t7518-ident-corner-cases.sh
> index fffdb6ff2e..9ab2ae2f3b 100755
> --- a/t/t7518-ident-corner-cases.sh
> +++ b/t/t7518-ident-corner-cases.sh
> @@ -20,10 +20,19 @@ test_expect_success 'empty name and missing email' '
>  '
>  
>  test_expect_success 'commit rejects all-crud name' '
> -	test_must_fail env GIT_AUTHOR_NAME=" .;<>" \
> +	test_must_fail env GIT_AUTHOR_NAME=" ,;<>" \
>  		git commit --allow-empty -m foo
>  '
>  
> +test_expect_success 'commit does not strip trailing dot' '
> +	author_name="Pat Doe Jr." &&
> +	env GIT_AUTHOR_NAME="$author_name" \
> +		git commit --allow-empty -m foo &&
> +	git log -1 --format=%an >actual &&
> +	echo "$author_name" >expected &&
> +	test_cmp actual expected
> +'
> +
>  # We must test the actual error message here, as an unwanted
>  # auto-detection could fail for other reasons.
>  test_expect_success 'empty configured name does not auto-detect' '

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] ident: don't consider trailing dot crud
  2023-07-31 21:44           ` [PATCH] ident: don't consider trailing dot crud brian m. carlson
  2023-07-31 21:49             ` Junio C Hamano
@ 2023-07-31 21:56             ` Thomas J. Faughnan Jr.
  2023-07-31 22:05               ` Junio C Hamano
  1 sibling, 1 reply; 15+ messages in thread
From: Thomas J. Faughnan Jr. @ 2023-07-31 21:56 UTC (permalink / raw)
  To: brian m. carlson, git; +Cc: Jeff King, Junio C Hamano, Daniel P.

I agree that simply dropping the "." from the crud list is the best
solution. Hope this gets merged.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] ident: don't consider trailing dot crud
  2023-07-31 21:56             ` Thomas J. Faughnan Jr.
@ 2023-07-31 22:05               ` Junio C Hamano
  0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2023-07-31 22:05 UTC (permalink / raw)
  To: Thomas J. Faughnan Jr.; +Cc: brian m. carlson, git, Jeff King, Daniel P.

"Thomas J. Faughnan Jr." <thomas@faughnan.net> writes:

> I agree that simply dropping the "." from the crud list is the best
> solution. Hope this gets merged.

Sure, I do too hope this gets into a shape that can be merged.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] ident: don't consider trailing dot crud
  2023-07-31 21:49             ` Junio C Hamano
@ 2023-08-02 16:49               ` Junio C Hamano
  2023-08-02 21:27                 ` brian m. carlson
  0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2023-08-02 16:49 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Jeff King, Thomas J. Faughnan Jr., Daniel P.

Junio C Hamano <gitster@pobox.com> writes:

> I wonder if this needs some credit to those involved in the original
> thread?

I've had this on 'seen' as-is, hoping to see a quick update so that
we can merge it down to 'next' before -rc0; here is a minimally
touched-up version I'll replace it with.

Thanks.

------- >8 ------------- >8 ------------- >8 ------------- >8 -------
From: "brian m. carlson" <sandals@crustytoothpaste.net>
Subject: [PATCH] ident: don't consider '.' a crud

When we process a user's name (as in user.name), we strip all
leading and trailing crud from it.  Right now, we consider a dot
a crud character, and strip it off.

However, this is unsuitable for many personal names because humans
frequently have abbreviated suffixes, such as "Jr." or "Sr." at the end
of their names, and this corrupts them.  Some other users may wish to
use an abbreviated name or initial, which will pose a problem especially
in cultures that write the family name first, followed by the personal
name.

Since the current approach causes lots of practical problems, let's
avoid it by no longer considering a dot to be crud.

Note that "." in the name forces the entire name to be quoted to
please mailers, but stripping "." only at the beginning and the end
does not help a name with "." in the middle (like "brian m. carlson")
so this change will not make it much worse.  A name like "Given
Family, Jr." that did not have to be quoted now would need to be, in
order to be placed on the e-mail headers, though.

This is based on a weather-balloon patch by Jeff King sent in Aug 2021
https://lore.kernel.org/git/YSKm8Q8nyTavQaox@coredump.intra.peff.net/

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

1:  4ce0751970 ! 1:  9478b6dadc ident: don't consider trailing dot crud
    @@ Metadata
     Author: brian m. carlson <sandals@crustytoothpaste.net>
     
      ## Commit message ##
    -    ident: don't consider trailing dot crud
    +    ident: don't consider '.' a crud
     
    -    When we process a user's name (as in user.name), we strip all trailing
    -    crud from it.  Right now, we consider a dot trailing crud, and strip it
    -    off.
    +    When we process a user's name (as in user.name), we strip all
    +    leading and trailing crud from it.  Right now, we consider a dot
    +    a crud character, and strip it off.
     
         However, this is unsuitable for many personal names because humans
         frequently have abbreviated suffixes, such as "Jr." or "Sr." at the end
    @@ Commit message
         Since the current approach causes lots of practical problems, let's
         avoid it by no longer considering a dot to be crud.
     
    +    Note that "." in the name forces the entire name to be quoted to
    +    please mailers, but stripping "." only at the beginning and the end
    +    does not help a name with "." in the middle (like "brian m. carlson")
    +    so this change will not make it much worse.  A name like "Given
    +    Family, Jr." that did not have to be quoted now would need to be, in
    +    order to be placed on the e-mail headers, though.
    +
    +    This is based on a weather-balloon patch by Jeff King sent in Aug 2021
    +    https://lore.kernel.org/git/YSKm8Q8nyTavQaox@coredump.intra.peff.net/
    +
         Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
         Signed-off-by: Junio C Hamano <gitster@pobox.com>
     



 ident.c                       |  1 -
 t/t4203-mailmap.sh            |  4 ++--
 t/t7518-ident-corner-cases.sh | 11 ++++++++++-
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/ident.c b/ident.c
index 8fad92d700..8d490df7d5 100644
--- a/ident.c
+++ b/ident.c
@@ -203,7 +203,6 @@ void reset_ident_date(void)
 static int crud(unsigned char c)
 {
 	return  c <= 32  ||
-		c == '.' ||
 		c == ',' ||
 		c == ':' ||
 		c == ';' ||
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index fa7f987284..2016132f51 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -466,7 +466,7 @@ test_expect_success 'gitmailmap(5) example output: example #1' '
 	Author Jane Doe <jane@laptop.(none)> maps to Jane Doe <jane@laptop.(none)>
 	Committer C O Mitter <committer@example.com> maps to C O Mitter <committer@example.com>
 
-	Author Jane D <jane@desktop.(none)> maps to Jane Doe <jane@desktop.(none)>
+	Author Jane D. <jane@desktop.(none)> maps to Jane Doe <jane@desktop.(none)>
 	Committer C O Mitter <committer@example.com> maps to C O Mitter <committer@example.com>
 	EOF
 	git -C doc log --reverse --pretty=format:"Author %an <%ae> maps to %aN <%aE>%nCommitter %cn <%ce> maps to %cN <%cE>%n" >actual &&
@@ -494,7 +494,7 @@ test_expect_success 'gitmailmap(5) example output: example #2' '
 	Author Jane Doe <jane@laptop.(none)> maps to Jane Doe <jane@example.com>
 	Committer C O Mitter <committer@example.com> maps to C O Mitter <committer@example.com>
 
-	Author Jane D <jane@desktop.(none)> maps to Jane Doe <jane@example.com>
+	Author Jane D. <jane@desktop.(none)> maps to Jane Doe <jane@example.com>
 	Committer C O Mitter <committer@example.com> maps to C O Mitter <committer@example.com>
 	EOF
 	git -C doc log --reverse --pretty=format:"Author %an <%ae> maps to %aN <%aE>%nCommitter %cn <%ce> maps to %cN <%cE>%n" >actual &&
diff --git a/t/t7518-ident-corner-cases.sh b/t/t7518-ident-corner-cases.sh
index fffdb6ff2e..9ab2ae2f3b 100755
--- a/t/t7518-ident-corner-cases.sh
+++ b/t/t7518-ident-corner-cases.sh
@@ -20,10 +20,19 @@ test_expect_success 'empty name and missing email' '
 '
 
 test_expect_success 'commit rejects all-crud name' '
-	test_must_fail env GIT_AUTHOR_NAME=" .;<>" \
+	test_must_fail env GIT_AUTHOR_NAME=" ,;<>" \
 		git commit --allow-empty -m foo
 '
 
+test_expect_success 'commit does not strip trailing dot' '
+	author_name="Pat Doe Jr." &&
+	env GIT_AUTHOR_NAME="$author_name" \
+		git commit --allow-empty -m foo &&
+	git log -1 --format=%an >actual &&
+	echo "$author_name" >expected &&
+	test_cmp actual expected
+'
+
 # We must test the actual error message here, as an unwanted
 # auto-detection could fail for other reasons.
 test_expect_success 'empty configured name does not auto-detect' '
-- 
2.41.0-478-gee48e70a82


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] ident: don't consider trailing dot crud
  2023-08-02 16:49               ` Junio C Hamano
@ 2023-08-02 21:27                 ` brian m. carlson
  2023-08-02 21:38                   ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: brian m. carlson @ 2023-08-02 21:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King, Thomas J. Faughnan Jr., Daniel P.

[-- Attachment #1: Type: text/plain, Size: 685 bytes --]

On 2023-08-02 at 16:49:32, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > I wonder if this needs some credit to those involved in the original
> > thread?
> 
> I've had this on 'seen' as-is, hoping to see a quick update so that
> we can merge it down to 'next' before -rc0; here is a minimally
> touched-up version I'll replace it with.

That's fine.  I was planning to do a re-roll today (since my Tuesdays
tend to be occupied with French classes), but I'm happy with what you
have.  Let me know if you're expecting a re-roll nevertheless, and I'll
try to get one out to you.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] ident: don't consider trailing dot crud
  2023-08-02 21:27                 ` brian m. carlson
@ 2023-08-02 21:38                   ` Junio C Hamano
  0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2023-08-02 21:38 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Jeff King, Thomas J. Faughnan Jr., Daniel P.

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> On 2023-08-02 at 16:49:32, Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>> 
>> > I wonder if this needs some credit to those involved in the original
>> > thread?
>> 
>> I've had this on 'seen' as-is, hoping to see a quick update so that
>> we can merge it down to 'next' before -rc0; here is a minimally
>> touched-up version I'll replace it with.
>
> That's fine.  I was planning to do a re-roll today (since my Tuesdays
> tend to be occupied with French classes), but I'm happy with what you
> have.  Let me know if you're expecting a re-roll nevertheless, and I'll
> try to get one out to you.

Nah, your message I am responding to is just as good as a reroll.

After all we are not propagating cryptographic signatures in your
e-mail to resulting commits, so it would not make any difference ;-)

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-08-02 21:38 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-22 14:19 Git is removing . from the last part of user.name Daniel P.
2021-08-22 15:33 ` brian m. carlson
2021-08-22 19:28   ` Jeff King
2021-08-22 19:35     ` Jeff King
2021-08-23 16:25     ` Junio C Hamano
2023-07-30 21:38     ` Thomas J. Faughnan Jr.
2023-07-31 15:56       ` Junio C Hamano
2023-07-31 21:02         ` brian m. carlson
2023-07-31 21:44           ` [PATCH] ident: don't consider trailing dot crud brian m. carlson
2023-07-31 21:49             ` Junio C Hamano
2023-08-02 16:49               ` Junio C Hamano
2023-08-02 21:27                 ` brian m. carlson
2023-08-02 21:38                   ` Junio C Hamano
2023-07-31 21:56             ` Thomas J. Faughnan Jr.
2023-07-31 22:05               ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).