Re: [PATCH] generate a valid rfc2047 mail header for multi-line subject.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2011/2/23 Junio C Hamano <gitster@xxxxxxxxx>:
> xzer <xiaozhu@xxxxxxxxx> writes:
>
>> Subject: Re: [PATCH] generate a valid rfc2047 mail header for multi-line subject.
>
> We prefer to have "[PATCH] subsystem: description without final full-stop" here.
>
>> There is still a problem that git-am will lost the line break.
>
> What does "still" refer to? ÂIt is unclear under what condition the
> command lose "the line break" (nor which line break you are refering to; I
> am guessing that you have a commit that begins with a multi-line paragraph
> and you are talking about line breaks between the lines in the first
> paragraph).
>

Yes, that is what I am refering, the line breaks in the first paragraph.

>> It's not easy to retain it, but as the first step, we can generate
>> a valid rfc2047 header now.
>
> Please describe what is broken (iow, "Given this sample input, we
> currently generate this output, which is not a valid rfc2047") and what
> the new output looks like ("Update pp_title_line() to generate this output
> instead.")
>

At present we can only concatenate the lines in the first paragraph so
that we can generate a valid rfc2047 mail, but we will lost the line breaks
after import the patch by git-am.

>> ---
>
> Missing sign-off with a real name.
>

I am sorry that I didn't find the document of submitting a patch until
yesterday, Thanks for your comment.

>> diff --git a/pretty.c b/pretty.c
>> index 8549934..f18a38d 100644
>> --- a/pretty.c
>> +++ b/pretty.c
>> @@ -249,6 +249,33 @@ needquote:
>> Â Â Â strbuf_addstr(sb, "?=");
>> Â}
>>
>> +static void add_rfc2047_multiline(struct strbuf *sb, const char *line, int len,
>> + Â Â Â Â Â Â Â Â Â Â Â const char *encoding)
>> +{
>> + Â Â int first = 1;
>> + Â Â char *mline = xmemdupz(line, len);
>> + Â Â const char *cline = mline;
>> + Â Â int offset = 0, linelen = 0;
>> + Â Â Â Âfor (;;) {
>
> You seem to have indent that uses SPs instead of HT around here...
>
>> + Â Â Â Â Â Â Â Âlinelen = get_one_line(cline);
>
> I can see you are trying to be careful not to let get_one_line() overstep
> past "len" the caller gave you by making a copy first, but is this
> overhead really necessary? ÂAfter all we know in this static function that
> the caller is feeding the contents from a strbuf, which always have a
> terminating NUL (and that is why it is Ok that get_one_line() is not a
> counted string interface).
>

I am not sure that who will call this function in future, I think since there is
a argument as len, so I'd better to obey the function declare.

>> +
>> + Â Â Â Â Â Â Â Âcline += linelen;
>> +
>> + Â Â Â Â Â Â Â Âif (!linelen)
>> + Â Â Â Â Â Â Â Â Â Â Â Âbreak;
>> +
>> + Â Â Â Â Â Â if (!first)
>> + Â Â Â Â Â Â Â Â Â Â strbuf_addf(sb, "\n ");
>> +
>> + Â Â Â Â Â Â offset = *(cline -1) == '\n';
>> +
>> + Â Â Â Â Â Â add_rfc2047(sb, cline-linelen, linelen-offset, encoding);
>> + Â Â Â Â Â Â first = 0;
>> +
>> + Â Â Â Â}
>> + Â Â free(mline);
>> +}
>
> So the general idea of this change (I am thinking aloud what should be in
> the updated commit log message as the problem description) is that:
>
> Â- We currently give an entire multi-line paragraph string to the
> Â add_rfc2047() function to be formatted as the title of the commit;
>
> Â- The add_rfc2047() functionjust passes "\n" through, without making it a
> Â folding whitespace followed by a newline, to help callers that want to
> Â use this function to produce a header line that is rfc 2822 conformant;
>
> Â- The patch introduces a new function add_rfc2047_multiline() that splits
> Â its input and performs line folding for such a caller (namely, the
> Â pp_title_line() function);
>
> Â- Another caller of add_rfc2047(), pp_user_info, is not changed, and it
> Â won't fold the name of the user that appear on the From: line.
>
> It is unclear if the last point is really the right thing to do, though.
> It is not a new problem that an author name that has a "\n" in it would
> break the output, but we probably would want to fix that case too here?
>

Your comment is just right for what I tried to do, I explained why I add a new
function for subject specially in the mail which replied to Jeff, I
want to remain
the line breaks after import the patch, so I think I need do something here
in future, it will be compatible with rfc2047 and also can be imported with
line breaks correctly. I don't know how yet, so I just want to left a
possibility.
So I introduce a new function for subject only.

xzer
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]