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