Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages

On Thu, 20 Dec 2007, Alex Riesen wrote:

I just happen to have a corporate template (for perforce messages, I
reuse it my git mirror repo) which contains "#" and at least one time
lost my bash comments in a commit.

I think that this is a real bug, but I don't think this is something that 
we should add a flag for.

Basically, I don't think we should really strip lines starting with '#' 
unless *we* added them. In particular, I don't think we should strip them 
at all unless we're running the editor.

So I think that instead of your thing, we should do somethign like the 
appended, which allows you to do things like

	git commit -m "# Message starting with a hash-mark"

which the current code makes impossible ("empty commit message").

That may be enough for your case, although it still does leave the "use 
editor on a template thing", so if that is your usage scenario, I guess we 
still do need a flag for it.

But even if we *do* add a flag (like "--verbatim") you should at the 
*least* also then remove the

	"# (Comment lines starting with '#' will not be included)\n"

printout! Which you didn't.

So I say NAK on this patch.

It also implies --allow-empty.

I disagree with this one too.

We have had *way* too many problems with various tools generating bogus 
empty commits. I get them from stgit users (and I think this is a serious 
BUG in stgit, dammit!), but I have this memory of some other usage 
scenario that did it too. 

In other words, empty commits are almost always just bogus. And dammit, if 
they aren't bogus, you should *say* so. No "implied" permissions, please. 
If you really want your commits to be empty, what's the downside of just 
adding an explicit "--allow-empty"?

		Linus

---
 builtin-commit.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 0a91013..4685938 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c

@@ -434,7 +434,7 @@ static int message_is_empty(struct strbuf *sb, int start)
 	/* See if the template is just a prefix of the message. */
 	strbuf_init(&tmpl, 0);
 	if (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) {
-		stripspace(&tmpl, 1);
+		stripspace(&tmpl, !no_edit);
 		if (start + tmpl.len <= sb->len &&
 		    memcmp(tmpl.buf, sb->buf + start, tmpl.len) == 0)
 			start += tmpl.len;

@@ -813,7 +813,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	if (p != NULL)
 		strbuf_setlen(&sb, p - sb.buf + 1);
 
-	stripspace(&sb, 1);
+	stripspace(&sb, !no_edit);
 	if (sb.len < header_len || message_is_empty(&sb, header_len)) {
 		rollback_index_files();
 		die("no commit message?  aborting commit.");