Re: [PATCH v3 0/2] Make find_commit_subject() consistent with --format=%s

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 22, 2016, 17:21 UTC
Message-ID: <xmqq1t3pnnpq.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <cover.1466587806.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

Show 5 quoted lines

> In an intermediate iteration of my rebase--helper patches, I
> accidentally generated commits with more than one empty line
> between the header and the commit message. When using
> find_commit_subject() to show the oneline, it turned up empty, even
> if the output of `git show --format=%s` looked fine.

Much easier to read with s/this developer/I/g ;-)

Show 7 quoted lines

> Turned out that the pretty-printing machinery helpfully skipped any
> blank lines before the commit message.
>
> In the first iteration of this patch, I failed to notice that
> the skip_empty_lines() function used by the pretty printing (which is
> declared static, and therefore I originally did not use it in order to
> keep the patch as minimal as possible) skips also blank lines.

By the way, I think skip_empty_lines() is misnamed, and I think your use of "blank lines" in the previous paragraph indicates that you agree ;-) It probably was OK back when it was a file-local static helper in pretty.c, but it becomes a part of the global API with 1/2, renaming it to skip_blank_lines() may be a good thing to do there at the same time.

I could do the tweaking while queuing if you too think it should happen; that way we'd save one roundtrip ;-).

> To make things truly consistent, I now just make the skip_empty_lines()
> function public, and then use it.

Thanks.  Interdiff looks sensible.