Teach git-commit about commit message templates.

These are useful in organizations that enforce particular formats for commit messages, e.g., to specify bug IDs or test plans. Use of the template is not enforced; it is simply used as the initial content when the editor is invoked.

---
This came up at my company when someone was comparing git to svn; we use svn's commit template feature.

I know we can abuse the pre-commit hook and stick a template in the SQUASH_MSG file (thanks to Dscho for that idea on IRC) but that has the annoying side effect of using the template as the commit message if you change your mind and quit the editor without entering anything, rather than aborting the commit.

I also know that git-commit is being builtin-ified, but as far as I know that's still a WIP and this will work today.

This isn't perfect -- what you'd ideally want is a way to avoid using the template for purely local "checkpoint of work in progress" commits while still using it for commits you intend to publish. But obviously there's no way for git to know which is which. For local commits you can easily just empty out the editor and type a message from scratch.

I was surprised there wasn't already a test script for commit, but I guess it gets tested implicitly in every other script anyway.

 Documentation/git-commit.txt |    8 ++++
 git-commit.sh                |   54 +++++++++++++++++++++--
 t/t7500-commit.sh            |   96 ++++++++++++++++++++++++++++++++++++++++++
 t/t7500/add-comments         |    4 ++
 t/t7500/add-content          |    3 +
 t/t7500/add-signed-off       |    3 +
 6 files changed, 163 insertions(+), 5 deletions(-)
 create mode 100755 t/t7500-commit.sh
 create mode 100755 t/t7500/add-comments
 create mode 100755 t/t7500/add-content
 create mode 100755 t/t7500/add-signed-off

index 8e0e7e2..3f36c67 100644
@@ -74,6 +74,14 @@ OPTIONS
 -m <msg>|--message=<msg>::
 	Use the given <msg> as the commit message.
 
+-t <file>|--template=<file>::
+	Use the contents of the given file as the initial version
+	of the commit message. The editor is invoked and you can
+	make subsequent changes. If a message is specified using
+	the `-m` or `-F` options, this option has no effect. The
+	template file may also be specified using the `commit.template`
+	configuration variable.
+
 -s|--signoff::
 	Add Signed-off-by line at the end of the commit message.
 

new file mode 100755
index 0000000..a72e65c
@@ -0,0 +1,4 @@
+#!/bin/sh
+echo "# this is a new comment" >> "$1"
+echo "# and so is this" >> "$1"
+exit 0

new file mode 100755
index 0000000..2fa3d86
@@ -0,0 +1,3 @@
+#!/bin/sh
+echo "commit message" >> "$1"
+exit 0

new file mode 100755
index 0000000..e1d856a
@@ -0,0 +1,3 @@
+#!/bin/sh
+echo "Signed-off-by: foo <bar@frotz>" >> "$1"
+exit 0

1.5.3.rc2.4.g726f9

Hi,

The template can itself have comments -- instructions or explanations of fields to fill in, for example -- and since comments have been stripped from COMMIT_MSG at this point, a comparison against such a template would always fail. And, consistent with the current behavior, simply adding a Signed-off-by: line shouldn't count as supplying a commit message.

I could do this test before stripping comments from COMMIT_MSG, but then I'd still fail the comparison if the user just deleted some comment lines manually, which also seems wrong to me -- the comments should be totally ignored when doing this comparison, IMO. Plus that wouldn't ignore Signed-off-by: lines.

If I'm coming at the design the wrong way, I'm of course happy to adjust it, but insensitivity to both comments and Signed-off-by: lines seemed like the right behavior from the user's POV to me, and I didn't see a cleaner way to do it.

Thanks for looking at the patch!

-Steve

Hi,

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"
}

Ciao,
Dscho

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.

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.

-Steve

Offhand, an "interesting" side effect of the above I can see is that you will cry "wolf" if the only thing the user did is to add his own Signed-off-by: line ;-) But that is sane in the context of coming up with totally new commit log message.

I am more worried about how this should interact with cases where you usually do not start the log message from scratch. For example, are there cases / policies where being able to use templates to leave comments on merge commits are needed? Squash-commits? Perhaps "apply this template but only when you have hand resolved a conflicting merges"?

Or even the case of amending a commit made by somebody else, without changing the tree contents, in order to make the commit log message to conform to the company standard?

Yes, that is intentional -- the existing git-commit code wasn't treating Signed-off-by: as content when it checked for a commit message, so I wanted to preserve that behavior.

Those are all valid cases. They also happen to be ones that are not personally useful to me; none of the stuff in my company's commit template would really apply to merges. I'm happy to do it if you think those cases need to be handled before this can go in, but given there's at least one user out there who doesn't need those things, I wonder if this makes more sense to hold off on until someone actually asks for it.

It's not clear to me that I can do anything useful in that case, given that there's no way to tell in what way the commit message needs to be fixed up. If it's a case of, "This engineer committed without using the template at all," then I can see an argument for loading the template above / below the existing comment, but IMO a more common case will be something like, "The format is right, but the bug ID isn't filled in or is incorrect," in which case any template I supply would be useless and annoying.

Is your hunch about the common case different than mine? Again, I can go address this if it'll actually be useful. Maybe by always including the template if the command-line option is used, but skipping it in these cases if the config option is used? Or something like that? (On the other hand, if you have to specify the template on the command line, maybe it's easier to just load it into the editor as needed.)

-Steve

No, I am way more cunning and lazy than that. I did not have hunch about the common case, so I just had you (and anybody else who would join the thread) do necessary thinking and guessing for me ;-)

Ah, good. So it sounds like what I've got is at least an okay first cut from a functionality point of view. Dscho (or anyone else), any objections to the code in v2 of my patch? I believe I addressed all the feedback from v1.

-Steve

I have already queued it for 1.5.3-rc3. Thanks.

Hi,

Yes, I think that makes sense.

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