Re: [PATCH] interpret-trailers: obey scissors lines

From: Junio C Hamano <gitster@pobox.com>
Date: May 15, 2017, 04:23 UTC
Message-ID: <xmqq7f1iwxxd.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqk25iwz7l.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 15 quoted lines

>         This can be done by the same logic as the existing helper
>         function, wt_status_truncate_message_at_cut_line(), uses,
>         but it wants the caller to pass a strbuf to it.  Because the
>         helper function ignore_non_trailer() used by the command
>         takes a <pointer, length> pair, not a strbuf, steal the
>         logic from wt_status_truncate_message_at_cut_line() to
>         create a new wt_status_strip_scissors() helper function that
>         takes <poiter, length> pair, and make ignore_non_trailer()
>         call it to help "interpret-trailers".  Since there is only
>         one caller of wt_status_truncate_message_at_cut_line() in
>         cmd_commit(), rewrite it to call wt_status_strip_scissors()
>         helper instead and remove the old helper that no longer has
>         any caller.
>
> The last paragraph would have saved me from getting puzzled.

And re-reading the above, wt_status_strip_scissors() does not sound right to me.

Yes, I am bikeshedding at this point, but names matter. I prefer to keep the distinction between the two clear by not calling the cut_line[] array "scissors", but in any case, the new function is not about "finding" the cut-line or scissors, in other words, wt_status_locate_scissors() is not a good name either (we do not say "not found" when there is none).

We are locating the logical end of the commit log message. It ends at "---\n", if exists, in the output of "git format-patch", and it ends at the cut-line, if exists, in "commit -v" editor buffer, and if there aren't these funny End-Of-Message marks, then we return the location of the last byte. IOW, the helper function you added will be the place to add more logic in the future if we ever found the need to notice other kinds of "logical end" markers (which may be ones we will invent in the future) while accepting a proposed commit log message.

So how about calling it wt_status_locate_end() or something like that, without limiting ourselves only to the "scissors"?