Re: [PATCH v3 5/6] trailer: allow non-trailers in trailer block

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 18, 2016, 01:42 UTC
Message-ID: <xmqq7f96sa9i.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79kYLq1qA4_Qg2x5Fiu5AmGBZdozm4zk6K7LkU+uJ1LNUTw@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 10 quoted lines

> On Fri, Oct 14, 2016 at 10:38 AM, Jonathan Tan <jonathantanmy@google.com> wrote:
>>
>>  Existing trailers are extracted from the input message by looking for
>> -a group of one or more lines that contain a colon (by default), where
>> +a group of one or more lines in which at least one line contains a
>> +colon (by default), where
>
> Please see commit
> 578e6021c0819d7be1179e05e7ce0e6fdb2a01b7
> for an example where I think this is overly broad.

Hmph.  That's a merge.

    Merge branch 'rs/c-auto-resets-attributes'

    When "%C(auto)" appears at the very beginning of the pretty format
    string, it did not need to issue the reset sequence, but it did.

    * rs/c-auto-resets-attributes:
      pretty: avoid adding reset for %C(auto) if output is empty

And neither of the two colon containing line remotely resembles how a typical RFC-822 header is formatted. So that may serve as a hint to how we can tighten it without introducing false negative.

Show 17 quoted lines

> Another made up example, that I'd want to feed
> in commit -s eventually:
>
> --8<--
> demonstrate colons in Java
>
> First paragraph is not interesting.
>
> Also if using another Language such as Java, where I point out
> Class::function() to be problematic
> --8<--
>
> This would lack the white space between the last paragraph and
> the Sign off ?
>
> So for this patch I am mostly concerned about false positives hidden
> in actual text.

Yes.

These are exactly why I mentioned "if certian number or percentage" in my earlier suggestion.

I think in practice, "A paragraph with at least one Signed-off-by: line, and has no more than 3/4 of the (logical) lines that do not resemble how a typical RFC-822 header is formatted" or something along that line would give us a reasonable safety.

Your Java example will fail the criteria in two ways, so we'd be safe ;-)