Re: [PATCH] send-email: handle adjacent RFC 2047-encoded words properly

From: Роман Донченко <dpb@corrigendum.ru>
Date: Nov 24, 2014, 18:09 UTC
Message-ID: <op.xpucqml8nngjn5@freezie>
In-Reply-To: <CAPc5daVjNDg5CcWsMwfn=DZhwpCBdU2LYXOpFWZwhx2p8hLRww@mail.gmail.com>

Junio C Hamano <gitster@pobox.com> писал в своём письме Mon, 24 Nov 2014 10:27:51 +0300:

Show 24 quoted lines

> On Sun, Nov 23, 2014 at 3:50 PM, Роман Донченко <dpb@corrigendum.ru>  
> wrote:
>> The RFC says that they are to be concatenated after decoding (i.e. the
>> intervening whitespace is ignored).
>>
>> I change the sender's name to an all-Cyrillic string in the tests so  
>> that
>> its encoded form goes over the 76 characters in a line limit, forcing
>> format-patch to split it into multiple encoded words.
>>
>> Since I have to modify the regular expression for an encoded word  
>> anyway,
>> I take the opportunity to bring it closer to the spec, most notably
>> disallowing embedded spaces and making it case-insensitive (thus  
>> allowing
>> the encoding to be specified as both "q" and "Q").
>>
>> Signed-off-by: Роман Донченко <dpb@corrigendum.ru>
>
> This sounds like a worthy thing to do in general.
>
> I wonder if the C implementation we have for mailinfo needs similar
> update, though. I vaguely recall that we have case-insensitive start for
> q/b segments, but do not remember the details offhand.

That's what git am uses, right? I think that already works correctly (or at least doesn't have the bug this patch fixes). I didn't do extensive testing or look at the code, though.

Show 12 quoted lines

>
> Was the change to the test to use Cyrillic really necessary, or did it
> suffice if you simply extended the existsing "Funny Name" spelled with
> strange accents, but you substituted the whole string anyway?
>
> Until I found out what the new string says by running web-based
> translation on it, I felt somewhat uneasy. As I do not read
> Cyrillic/Russian, we may have been adding some profanity without
> knowing. It turns out that the string just says "Cyrillic Name", so I am
> not against using the new string, but it simply looked odd to replace the
> string whole-sale when you merely need a longer string. It made it look
> as if a bug was specific to Cyrillic when it wasn't.

Ah, if only I had thought of including profanity beforehand. ;-)

Seriously though, I just needed to hit the 76 character limit, and switching the keyboard layout is a lot easier than copypasting Latin letters with diacritics (plus I had trouble coming up with a long enough extension of "Funny Name"...). I can see how that's problematic, though; I'll change it.

> As you may notice by reading "git log --no-merges" from recent history,
> we tend not to say "I did X, I did Y". If the tone of the above message
> were more similar to them, it may have been easier to read.

Technically, I said "I do", not "I did"... but sure, point taken.

Roman.