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

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> 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.

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