Am 09.10.2012 21:30, schrieb Junio C Hamano:
> Jan H. Schönherr <schnhrr@xxxxxxxxxxxxxxx> writes:
...
>> static int is_rfc2047_special(char ch)
>> {
>> + /*
>> + * We encode ' ' using '=20' even though rfc2047
>> + * allows using '_' for readability. Unfortunately,
>> + * many programs do not understand this and just
>> + * leave the underscore in place.
>> + */
>
> The sentence break made me read the above three times to understand
> what it is trying to say. "Unfortunately" refers to what happens if
> we were to use '_', but it initially appeared to be describing some
> bug due to our encoding ' ' as '=20'. Perhaps like this?
>
> /*
> * rfc2047 allows '_' to encode ' ' for readability, but
> * many programs do not understand ...; encode ' ' using
> * '=20' instead to avoid the problem.
> */
I was just moving that comment (and the following check) around,
but I'll update the comment in the next version.
>> + if (ch == ' ' || ch == '\n')
>> + return 1;
>
> The comment justifies why this "if (ch == ' ')", which could be part
> of the "return" below, separately is done, but nothing explains why
> you add '\n' (and not other controls, e.g. '\t') to the mix.
The check for '\n' was introduced in commit c22e7de3
("format-patch: rfc2047-encode newlines in headers").
The commit log was:
These should generally never happen, as we already
concatenate multiples in subjects into a single line. But
let's be defensive, since not encoding them means we will
output malformed headers.
Having again a look at RFC 2047, I see that we should be
even more strict and not allow any non-printable character to
be passed through unencoded. I guess that adds another patch to
the series. Hmm... Maybe I can split patch 4 into two patches,
one that mostly fixes is_rfc2047_special() and one that
avoids 822 quoting when doing 2047 encoding.
>
>> return (non_ascii(ch) || (ch == '=') || (ch == '?') || (ch == '_'));
>> }
>>
>> static void add_rfc2047(struct strbuf *sb, const char *line, int len,
>> const char *encoding)
>> {
>> - static const int max_length = 78; /* per rfc2822 */
>> + static const int max_length = 76; /* per rfc2047 */
>> int i;
>> int line_len;
>>
>> @@ -286,7 +295,7 @@ static void add_rfc2047(struct strbuf *sb, const char *line, int len,
>> if ((i + 1 < len) && (ch == '=' && line[i+1] == '?'))
>> goto needquote;
>> }
>> - strbuf_add_wrapped_bytes(sb, line, len, -line_len, 1, max_length+1);
>> + strbuf_add_wrapped_bytes(sb, line, len, -line_len, 1, 78+1);
>> return;
>
> Yuck. If you do want to retain 78 for non-quoted output for
> backward compatibility, that is OK, but if that is the case, please
> introduce a new constant "max_quoted_length" or something to stand
> for 76 and use it in the "needquote:" part below.
Will do.
Regards
Jan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html