On Wed, Aug 26, 2015 at 9:48 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Christian Couder <christian.couder@xxxxxxxxx> writes:
>
>> We currently ignore the first line passed to `git interpret-trailers`,
>> when looking for the beginning of the trailers.
>>
>> Unfortunately this does not work well when a commit is created with a
>> line break in the title, using for example the following command:
>>
>> git commit -m 'place of
>> code: change we made'
>>
>> In this special case, it is best to look at the first line and if it
>> does not contain only spaces, consider that the second line is not a
>> trailer.
>> ---
>
> Missing sign-off,
Ok, will add it.
[...]
> I think the analysis behind the first patch is correct. It stops
> the backward scan of the main loop to reach there by realizing that
> the first line, which must be the first line of the patch title
> paragraph, can never be a trailer.
>
> To extend that correct realization to cover the case where the title
> paragraph has more than one line, the right thing to do is to scan
> forward from the beginning to find the first paragraph break, which
> must be the end of the title paragraph, and exclude the whole thing,
> wouldn't it?
>
> That is, I am wondering why the patch is not more like this (there
> may be off-by-one, but just to illustrate the approach; I didn't
> even compile test this one so...)?
>
> Puzzled...
>
> static int find_trailer_start(struct strbuf **lines, int count)
> {
> - int start, only_spaces = 1;
> + int start, end_of_title, only_spaces = 1;
> +
> + /* The first paragraph is the title and cannot be trailer */
> + for (start = 0; start < count; start++)
> + if (!lines[start]->len)
> + break; /* paragraph break */
> + end_of_title = start;
>
> /*
> * Get the start of the trailers by looking starting from the end
> * for a line with only spaces before lines with one separator.
> - * The first line must not be analyzed as the others as it
> - * should be either the message title or a blank line.
> */
> - for (start = count - 1; start >= 1; start--) {
> + for (start = count - 1; start >= end_of_title; start--) {
> if (lines[start]->buf[0] == comment_line_char)
> continue;
> if (contains_only_spaces(lines[start]->buf)) {
Yeah, we can do that. It will be clearer.
Thanks,
Christian.
--
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