On 10/17/2016 06:42 PM, Junio C Hamano wrote:
Stefan Beller <sbel...@google.com> writes:

On Fri, Oct 14, 2016 at 10:38 AM, Jonathan Tan <jonathanta...@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.

The only "offending" character is the space (according to RFC 822), but that sounds like a good rule to have.

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.

I think that "Signed-off-by:" is not guaranteed to be present. Defining a trailer line as "a line starting with a token, then optional whitespace, then separator", maybe the following rule: - at least one trailer line generated by Git ("(cherry picked by" or "Signed-off-by") or configured in the "trailer" section in gitconfig
OR
- at least 3/4 logical trailer lines (I'm wondering if this should be 100% trailer lines)

?

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

Reply via email to