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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/17/2016 06:42 PM, Junio C Hamano wrote:
Stefan Beller <sbeller@xxxxxxxxxx> writes:

On Fri, Oct 14, 2016 at 10:38 AM, Jonathan Tan <jonathantanmy@xxxxxxxxxx> 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 ;-)



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]