Re: [PATCH] commit & merge: modularize the empty message validator

From: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
Date: Jul 13, 2017, 18:15 UTC
Message-ID: <1499969722.5973.2.camel@gmail.com>
In-Reply-To: <xmqq8tju3eqp.fsf@gitster.mtv.corp.google.com>

On Tue, 2017-07-11 at 13:22 -0700, Junio C Hamano wrote:

Show 12 quoted lines

> I think the "validation" done with the rest_is_empty() is somewhat
> bogus.  Why should we reject a commit without a message and a
> trailer block with only signed-off-by lines, while accepting a
> commit without a message and a trailer block as long as the trailer
> block has something equally meaningless by itself, like
> "Helped-by:"?  I think we should inspect the proposed commit log
> message taken from the editor, find its tail ignoring the trailing
> comment using ignore_non_trailer, and further separate the result
> into (<message>, <trailers>, <junk at the tail>) using the same
> logic used by the interpret-trailers tool, and then complain when
> <message> turns out to be empty, to be truly useful and consistent.
>

I have a few doubts for which I need clarification to move on with this.

    1. If we abort when the <message> part is empty wouldn't it be too
    restrictive ?

    IOW, Wouldn't it affect users of "git commit -‍-cleanup=verbatim"
    who wish to commit only the comments or parts of it ?
    (I'm not sure if someone would find that useful)

    2. Is it ok to use the "find_trailer_start" function of "trailer.c"
    to locate the trailer?

    Note: It has a little issue that it wouldn't detect the trailer if
    the message comprises of one trailer alone and no other text. This
    case occurs while aborting a commit started using "git commit -s".
    Any possibilities to overcome the issue?

    3. Ignoring point 1 for now, What other helper methods except the
    ones listed below could be helpful in the separating the cleaned up
    commit message into the <message>, <trailer>, <junk-at-tail> ?

        * ignore_non_trailer
        * find_trailer_start

-- 
Kaartic