Re: Bug Report: Multi-line trailers containing empty lines break parsing

From: Taylor Blau <me@ttaylorr.com>
Date: Feb 16, 2021, 19:47 UTC
Message-ID: <YCwhPG6RaAhU9ljg@nand.local>
In-Reply-To: <xmqqmtw3pzu3.fsf@gitster.c.googlers.com>

On Tue, Feb 16, 2021 at 11:39:00AM -0800, Junio C Hamano wrote:

Show 6 quoted lines

> A few comments (not pointing out bugs, but just sharing
> observations).
>
>  - if the line before "trailer: single" were not an empty line but a
>    line with a single SP on it (which is is_blank_line()), would the
>    new logic get confused?

Oof. That breaks the new test, but it makes me worried about whether this can be parsed without ambiguity. I think not, but here I'd defer to Christian or Jonathan Tan.

Show 5 quoted lines

>  - if the second "multi:" trailer did not have the funny blank line
>    before "_two", the expected output would still be "multi:"
>    followed by "one two three", iow, the line after the second
>    "multi: one" is a total no-op?  If we added many more " \n" lines
>    there, they are all absorbed and ignored?  It somehow feels wrong

That's definitely the outcome of this patch, but I agree it feels wrong. I'm not sure that we define the behavior that strictly in git-interpret-trailers(1), so we have some wiggle room, I guess.

Thanks, Taylor