[BUG] You can't have single quote in your username

Note how `git log` discards the ending quote character:
```
root@NBR1710R:~# git init repo
Initialized empty Git repository in /root/repo/.git/
root@NBR1710R:~# cd repo/
root@NBR1710R:~/repo# git config user.name Les Actualite\'
root@NBR1710R:~/repo# cat .git/config
[core]
        repositoryformatversion = 0
        filemode = true
        bare = false
        logallrefupdates = true
[user]
        name = Les
root@NBR1710R:~/repo# git config user.name "Les Actualite\'"
root@NBR1710R:~/repo# cat .git/config
[core]
        repositoryformatversion = 0
        filemode = true
        bare = false
        logallrefupdates = true
[user]
        name = Les Actualite\\'
root@NBR1710R:~/repo# touch foo
root@NBR1710R:~/repo# git add foo
root@NBR1710R:~/repo# git commit -m 'first'
[master (root-commit) a78e11f] first
 Committer: Les Actualite <root@NBR1710R>
Your name and email address were configured automatically based
on your username and hostname. Please check that they are accurate.
You can suppress this message by setting them explicitly. Run the
following command and follow the instructions in your editor to edit
your configuration file:

    git config --global --edit

After doing this, you may fix the identity used for this commit with:

    git commit --amend --reset-author

 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 foo
root@NBR1710R:~/repo# git log
commit a78e11ff0707bd4f1bea195735a7fc8b7ee2b9f8 (HEAD -> master)
Author: Les Actualite <root@NBR1710R>
Date:   Thu Aug 22 14:25:11 2019 +0200

    first
```

I can't test with the development tree right now, ``` root@NBR1710R:~/repo# git --version git version 2.17.1 ```

On 22/08/19 02:32PM, Giuseppe Crinò wrote:

Hi,

I took a quick look into this problem. The issue is not with git log, but instead with git commit. When you commit, builtin/commit.c::author_info() calls ident.c::fmt_ident(), which in turn calls ident.c::strbuf_addstr_without_crud(), which in turn calls ident.c::crud().

This strbuf_addstr_without_crud() function removes various characters from the start and end of the author info, one of which is the single quotation. I'm not sure why this is done, the more experienced folk where will have the answer.

Anyway, the fix is simple enough. Remove the lines that mark single quotes as crud in the crud() function. This will fix the issue for the future commits. If you need to fix it for past commits, you need to re-write your history with the fix applied.

Below is the quick-and-dirty patch that fixes this. If there is no reason for this patch to be dropped, I'll send a proper one once some other people have commented.

-- >8 --

Pratyush Yadav <me@yadavpratyush.com> writes:

The logic there exists in order to remove cruft around the name on a typical e-mail header (remember, most of the very core-ish part of the Git was written and got solidified back when the Linux kernel was the primary client of the system, and many commits were created via "am"), e.g. sender's mail client may send something like this:

    From: 'Foo bar Baz' <my@name.xz>

and we do not want to take surrounding sq pair as part of the name.

On Thu, Aug 22, 2019 at 6:24 PM Junio C Hamano <gitster@pobox.com> wrote:

I see but _standard_ rules to escape characters should apply as I understand this. 'Foo bar Baz\'s' should be allowed as it should "Foo bar Baz's". They should resolve to the same name too

On Fri, 23 Aug 2019 09:20:47 +0200 Giuseppe Crinò <giuscri@gmail.com> wrote:

It would not be removed in this case regardless of the qouting. It is neither at the start nor at the end.

Thanks

Michal

Am 22.08.19 um 14:32 schrieb Giuseppe Crinò:

You can name yourself anything you like, of course, and I don't actually speak French, but wouldn't it rather be "L'actualité"?

I just reply as a fellow accent-bearer, feel free to ignore me..

René

On Thu, Aug 22, 2019 at 5:32 AM Giuseppe Crinò <giuscri@gmail.com> wrote:

<snip>

If you look closely here, in the "git commit" output, you can see that, as Pratyush indicated, it was actually "git commit" that dropped the trailing apostrophe, and "git log" is simply presenting the information as it exists in the repository.

If your goal is an accented "e", wouldn't it be better to set your name using é, rather than a trailing apostrophe? "git commit" would likely preserve that without issue.

Best regards, Bryan Turner

On Thu, Aug 22, 2019 at 09:58:48AM -0700, Bryan Turner wrote:

Hmm, I don't think it's a good idea to get into the business of telling contributors how to write their names. There tends to be an axiom that "all assumptions developers make about human names are false."

Does it make more sense to replace this strbuf_addstr_without_crud() setup with something more intelligent (i.e. checking for matching crud on either end, like ^[$crudchars].*\1$? We already check for matched <>.

- Emily

On 22/08/19 10:08AM, Emily Shaffer wrote:

Sounds like something easy enough to implement. There are two types of characters that crud() removes: there are the ones which _should_ appear on both the start and end (', ", <, >), and the ones which don't necessarily have to (., ,, :, ;, \).

So we'd need to handle two cases. For the former type, remove a character both at the start and at the end. For the latter, remove only where they appear.

If this sounds like something reasonable to do, I'll send a patch fixing this.

On Fri, Aug 23, 2019 at 12:13:12AM +0530, Pratyush Yadav wrote:

If we go down this route, then someone might want to write ő as o" or ű as u", which still supposed to be used in pairs, but what if someone wants to write ä as a:, ö as o:, ü as u:, ç as "c,", ş as "s,", etc.

What I wonder is whether we really have to remove crud from the user name if it comes from the configuration.

On Fri, Aug 23, 2019 at 10:29 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:

I don't know any language that uses ő or ű so I can't tell but it's definitely possible. In my country, sometimes the last name ends with an accented letter (mine ends with an "o grave") and people end up use a single quote to avoid pissing off computers. That's standard thing.

In my current team I don't know enough of the setup of my colleagues (they may use CP1252) and that ò may be badly decoded.

On Fri, Aug 23, 2019 at 11:35:48AM +0200, Giuseppe Crinò wrote:

The Hungarian alphabet has both, and I've seen them written like this on old typewritten documents. But that was decades ago, nowadays they are either written properly as ő and ű, or without any accent at all (banks...).

On Fri, Aug 23, 2019 at 10:29:00AM +0200, SZEDER Gábor wrote:

Yes. If the primary use of removing crud is to remove quotes from a quoted name (as in `From: 'Foo baz Bar'`) why not directly removing pairs of quotes around the name and leave crud _inside_ the name?

On Sat, Aug 24, 2019 at 05:49:52PM +0000, Giuseppe Crinò wrote:

I keep forgetting crud is removed *around* the name.

By the way, it's probably more conservative to remove crud (without removing quotes) and finally remove surrounding quote pairs.

On Fri, Aug 23, 2019 at 10:29:00AM +0200, SZEDER Gábor wrote:

Probably not. I think most of this crud-handling is really about pulling names out of email. Digging around in the archive, one of the few entries that actually talks about this crud-removal being helpful is:

  https://public-inbox.org/git/alpine.LFD.2.00.0812010836280.3256@nehalem.linux-foundation.org/

So it might make sense to push these rules into "git mailinfo" instead of applying them everywhere. But we'd still need something at least for GECOS, where "Your Name,,,," is common.

Unfortunately since these rules have been in place for ages, we don't really know how often they're helping. Loosening it just for user.name, etc is probably the most conservative choice.

We'd still want to keep the low-level removal of "<>\n", since those are syntactically significant to Git (i.e., if they sneak in you end up with a broken commit object). There's some discussion in this thread (which is also about trailing single-quotes!):

  https://public-inbox.org/git/20120629124122.GG1258@camk.edu.pl/

Another real-world case that would be helped is: "Your Name, Jr.":

  https://public-inbox.org/git/CAMP44s2OoxUVFE57e8C2Km7rcGn88KkLXxdaA3s5RE6nZ7TU=A@mail.gmail.com/

-Peff

On Mon, Aug 26, 2019 at 03:14:55PM -0400, Jeff King wrote:

What's the GECOS you mean?

On Tue, 27 Aug 2019 13:51:49 +0000 "Giuseppe Crino'" <giuscri@gmail.com> wrote:

https://en.wikipedia.org/wiki/Gecos_field

On Mon, Aug 26, 2019 at 03:14:55PM -0400, Jeff King wrote:

As I understand this, those commas are *not* removed by strbuf_addstr_without_crud(). Instead they're skipped from /etc/pass -- see `ident.c/copy_gecos()`.

On Wed, Aug 28, 2019 at 02:33:15PM +0000, Giuseppe Crino' wrote:

Yeah, you're right. This is a good example of how we are completely unclear on how often the crud-removal is helping. :)

But I think that does put us one step closer to possibly dropping the crud-removal, if gecos is already handled (and hopefully "mailinfo" is already cleaning up the names it parsers).

-Peff

On Mon, Aug 26, 2019 at 03:14:55PM -0400, Jeff King wrote:

Would it work to change `strbuf_addstr_without_crud()` such that instead of removing crud from the beginning and *then* crud from the end, it removes crud as long as it's palindromic? Then the function proceeds to remove "<>\n" from the string as before.

That would implement the following _mappings_:

'Foo bar' => Foo bar Micheal Jr. => Micheal Jr. Mr. McDonalds => Mr. McDonalds "John Baz" => John Baz "JJ\x0a" => JJ

What do you think?

On Sat, Aug 31, 2019 at 01:17:48PM +0000, Giuseppe Crinò wrote:

I think we don't have enough data to really know how much it will help or hurt. :)

On the plus side, it is less risky than dropping the end-point crud removal entirely.

But it still risks losing a case where some code path relies on the crud cleanup for odd cases (mismatched delimiters, or interleaved delimiters, or non-delimiter crud mixed in with delimiters).

On the one hand, I am actually OK with dropping the crud removal entirely and seeing what happens, and this is a lesser form of that. On the other hand, it puts us in a funny in-between situation where tools can _usually_ get away without cleaning up the names, but occasionally get bit. It might be easier to just say that we don't do cleanup.

So I dunno. There is no patch to be discussed, and I am not volunteering to write one. So I think whoever chooses to do so has a lot of control over what is proposed. :)

-Peff

Jeff King <peff@peff.net> writes:

Rather, they can propose what they want, but they have a lot of tough defending to do on their choice. Lack of potential harm is much harder to prove than coming up with a single example that harms.

I'd rather leave the sleeping dog lie, if we need to encourage people to live in 21st century and step outside US-ASCII to do so, then do that instead.

On Mon, 02 Sep 2019 12:25:37 -0700 Junio C Hamano <gitster@pobox.com> wrote:

As much as I would love to agree Windows is dragging us down here. I don't think there is any other major (or even just not completely obscure) platform that does not support UTF-8 in the terminal.

Thanks

Michal

On Mon, Sep 02, 2019 at 12:25:37PM -0700, Junio C Hamano wrote:

+1 to let the sleeping dog lie. When you say we should encourage people to step outside US-ASCII what do you mean by that?

On Mon, Sep 02, 2019 at 09:50:48PM +0200, Michal Suchánek wrote:

Well, I don't know how much Windows is related to that. On my Linux machine my user.name ends with a single quote: I have a US keyboard but my name ends with an "o grave" and I can't type it.

On Tue, 3 Sep 2019 07:51:54 +0000 Giuseppe Crinò <giuscri@gmail.com> wrote:

Have you ever heard about XCompose?

Thanks

Michal

On Thu, Aug 22, 2019 at 10:08:17AM -0700, Emily Shaffer wrote:

Well, maybe the contributor didn't know that Git can cope with accented characters in user names.