Re: [PATCH] Force new line at end of commit message

From: Shawn O. Pearce <spearce@spearce.org>
Date: Dec 25, 2007, 04:42 UTC
Message-ID: <20071225044202.GO14735@spearce.org>
In-Reply-To: <Pine.LNX.4.64.0712241835210.14355@wbgn129.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 9 quoted lines

> On Mon, 24 Dec 2007, Bernt Hansen wrote:
> 
> > git rebase --interactive formats the combined commit log message 
> > incorrectly when squashing 3 or more commits which have no newline on 
> > the last line of the commit message.
> 
> This is a patch for git-gui, so why not make that clear in the subject?  
> (And I have a hunch that Shawn would have liked the patch relative to 
> git-gui.git, not git.git...)

Indeed.

Most git-gui changes have a subject that starts with "git-gui:" so its clear in both the email and in the commit log that the change is a git-gui change. Remember, git-gui's logs show up in the core Git logs (as its merged with -s subtree) so having that git-gui: prefix does help people to localize the change within the overall suite.

git-am -3 does a reasonable job at correcting patches that are like
this one is (that aren't relative to git-gui.git) so that's less
of an issue for me.  And what git-am -3 cannot correct git-apply
-p2 usually does.  If that can't fix the patch then I'll usually
throw it back as its then most likely a true conflict.

> Further, there are other tools than rebase -i that like commit messages 
> better when terminated by a newline, and _that_ is what I would like to 
> read in the commit message for this patch.

Hmmph. For that reason alone I'm tempted to *not* apply Bernt's patch.

There is nothing that requires that a commit object end with an LF. So tools that make this assumption (that there is a trailing LF) while processing the body of a commit message are quite simply broken.

Its easy in fast-import to generate commits without a trailing LF. Or in many text editors its possible to save a file with no trailing LF on the last line. My favorite VI clone does that; if the file doesn't end with an LF when it opens its *damned* hard to get a trailing LF onto that last line. And yes, that's the editor I use for commit messages when I'm not using git-gui.

IMHO git-gui is producing valid commit messages, and always does so with no trailing LF, and any tool that is assuming a trailing LF is always present is broken.

Keeping git-gui behavior like this actually highlights the other tools that are broken (here Bernt found git-rebase--interactive).

I'd like to hear Junio's or Linus' two cents on the matter, but if we really want to say that all commits must end with an LF then maybe git-commit-tree, git-hash-object and git-fast-import should be performing that sort of validation before creating such an object in the ODB. Which is probably a change that shouldn't be made before 1.6.0 as its somewhat likely to break people's existing scripts.

-- 
Shawn.