Re: [PATCH v2 2/2] trailer: support multiline title

> Christian Couder <christian.couder@gmail.com> 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,

> 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)) {