patch, 5 partsUse skip_blank_lines() for more commit messages

These patches are on top of js/find-commit-subject-ignore-leading-blanks (whether you want to put them into the same topic branch before merging or put them into their own add-on topic branch does not really matter to me).

It occurred to me that there might be more code locations where finding the commit object's body is hand-rolled, and could benefit from using the skip_blank_lines() function to handle not-quite-standard but still-allowed objects from being processed correctly.

I found a couple of instances, and made them consistent with the pretty-printing machinery.

I made fine-grained commits to allow for dropping contentious patches more easily.

There are four more instances which I did *not* change:

- fast-export: I designed this command to be as faithful in exporting
  the commits as possible. As such, even non-standard commit objects
  should be kept as-are.

- fmt-merge-msg: when showing signed commit messages, it is better to
  show the commit messages verbatim rather than trimming them.

- when signing a commit, it is better to use the original commit message
  than a munged one.

- notes' messages are left alone. I do not know whether there might be
  an intentional use of leading blank lines, but I wanted to err on the
  safe side.

I also considered treating tag messages the same way, but tags are much more likely to be signed, and I did not want to risk munging the tag messages just before signing them.

Johannes Schindelin (5):
  commit -C: skip blank lines at the beginning of the message
  sequencer: use skip_blank_lines() to find the commit subject
  reset --hard: skip blank lines when reporting the commit subject
  commit -S: avoid invalid pointer with empty message
  Skip blank lines when matching <commit>^{/foo}

 builtin/commit.c | 2 +-
 builtin/reset.c  | 2 +-
 commit.c         | 6 +++++-
 sequencer.c      | 6 ++----
 sha1_name.c      | 3 ++-
 5 files changed, 11 insertions(+), 8 deletions(-)

Published-As: https://github.com/dscho/git/releases/tag/more-blank-line-skipping-v1

Consistent with the pretty-printing machinery, we skip leading blank lines (if any) of existing commit messages.

While Git itself only produces commit objects with a single empty line between commit header and commit message, it is legal to have more than one blank line (i.e. lines containing only white space, or no characters) at the beginning of the commit message, and the pretty-printing code already handles that.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/commit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

use_message_buffer is the contents of the commit object read elsewhere, and strstr() skips the header part, so this follows exactly the same pattern as the one you fixed earlier. Good.

Just like we already taught the find_commit_subject() function (to make it consistent with the code in pretty.c), we now simply skip leading blank lines of the commit message.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

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

Here, msg.message was read in get_message() by calling logmsg_reencode() that reads from get_commit_buffer(). The code structure is exactly the same as the previous one.

Good.

When there are blank lines at the beginning of a commit message, the pretty printing machinery already skips them when showing a commit subject (or the complete commit message). We shall henceforth do the same when reporting the commit subject after the user called

	git reset --hard <commit>

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/reset.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

While it is not recommended, `git fsck` says that:

	Not having a body is not a crime [...]

... which means that we cannot assume that the commit buffer contains an empty line to separate header from body (essentially saying that there is only a header).

So let's tread carefully here.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 commit.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

When trying to match a pattern in the commit subject, simply skip leading blank lines in the commit message. This is consistent with the pretty-printing machinery: it silently ignores leading blank lines in the commit object's body.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sha1_name.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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

Hmm.

I just tried

    $ git commit -s -m '  pull: fast-forward "pull --rebase=true"'

expecting that ":/^  pull:" would find it ;-)

The point of this patch is to make it not work, of course, which may or may not be a good thing. I'd rather keep this separate from the other fixes.

Also 4/5 is another different class of true bugfix, I would suspect, so let's take it on a separate topic that can be merged down to older maintainance tracks.

Thanks.

Hi Junio,

On Wed, 29 Jun 2016, Junio C Hamano wrote:

This patch would try to address a different concern, though:

	git write-object -w --stdin <<-EOF
	tree $(git rev-parse HEAD:)
	parent $(git rev-parse HEAD)
	author B O Gus <bo@gus.com> 1234567890 +0000
	committer F A Ke <fa@ke.org> 1234567890 +0000

	There are two blank lines before the subject
	EOF

It is not about leading whitespace in the commit subject, but about blank lines before it.

Granted, our current tools do not generate such commit objects, but we accept them, and already handle them in some places.

Sure!

Thanks, Dscho

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

Thanks for following up on this one. We could have left them as low-hanging-fruit for new people, but doing these ourselves while we still remember is a good idea.