Re: [PATCH] Teach git-commit about commit message templates.

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jul 23, 2007, 11:58 UTC
Message-ID: <Pine.LNX.4.64.0707231239460.14781@racer.site>
In-Reply-To: <46A48949.1020501@midwinter.com>

Hi,

On Mon, 23 Jul 2007, Steven Grimm wrote:

Show 40 quoted lines

> Johannes Schindelin wrote:
> > Ah, I missed that.  But IIRC your patch does not wrap that logic behind test
> > ! -z "$templatefile", right?  So this is my modified suggestion:
> > 
> > test ! -z "$templatefile" && {
> > 	grep -vie '^Signed-off-by:' < "$GIT_DIR"/COMMIT_MSG" > "$GIT_DIR"/tmp1
> > 	grep -ve '^#' < "$templatefile" > "$GIT_DIR"/tmp1
> > 	trap 'rm "$GIT_DIR"/tmp[12]' 0
> > 	cmp "$GIT_DIR"/tmp[12] &&
> > 	die "Unchanged message; will not commit"
> > }
> >   
> 
> So you are suggesting I do this in addition to the existing git-commit
> stripping of Signed-off-by: lines? I can certainly do that, but I didn't want
> to make two passes over the commit message doing exactly the same stripping.
> 
> Hmm, maybe I should outline my understanding of the current (unpatched)
> behavior and what I want it to do. Currently:
> 
> * Strip off all comment lines (happens when COMMIT_MSG is created)
> * Strip off all Signed-off-by: lines
> * Trim whitespace
> * If the result has no content (`wc -l` == 0), abort.
> 
> With the patch, my intent was:
> 
> * Strip off all comment lines
> * Strip off all Signed-off-by: lines
> * Trim whitespace
> * If the result has no content (! -s file), abort.
> * If a template file was specified:
>   * Strip off all comment and Signed-off-by: lines from the template
>   * Trim whitespace from the template
>   * If the resulting trimmed template is the same as the trimmed commit
> message, abort.
> 
> So I guess before getting to the specifics of the code, I'll ask: does the
> above make sense as a design? I wanted to preserve the existing behavior in
> the absence of a template.

Yes, I think that makes sense.

Show 5 quoted lines

> Since the existing code is already stripping Signed-off-by: lines to test for
> a zero-length commit message, I figured I should reuse that work. However,
> it's no big deal to do it twice if people feel that results in more readable
> code -- certainly no human will ever notice the time it takes to re-grep the
> commit message.

Okay, but I was really confused by the big if thing.

I'd make sure that COMMIT_BAREMSG is removed with a trap, and really try to just enhance the

	if cnt=`grep -v -i '^Signed-off-by' "$GIT_DIR"/COMMIT_MSG |
	        git stripspace |
	        wc -l` &&
	   test 0 -lt $cnt

so that it reads something like

	if test -s "$GIT_DIR"/COMMIT_BAREMSG &&
		test -z "$templatefile" ||
		(grep -ve '^#' < "$templatefile" |
		 git stripspace |
		 git diff --quiet - "$GIT_DIR"/COMMIT_BAREMSG)
	then
		...

(Totally untested, of course.) Hmm?

Ciao, Dscho