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

It also implies --allow-empty.

Sometimes the message just have to be the way user wants it.
For instance, a template can contain "#" characters, or the message
must be kept as close to its original source as possible for reimport
reasons. Or maybe the user just copied a shell script including its
comments into the commit message for future reference.

Signed-off-by: Alex Riesen <redacted>
---

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.

 Documentation/git-commit.txt |    7 ++++++-
 builtin-commit.c             |   10 ++++++++--
 t/t7502-commit.sh            |   17 +++++++++++++++++
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 4261384..862543f 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt

@@ -11,7 +11,7 @@ SYNOPSIS
 'git-commit' [-a | --interactive] [-s] [-v] [-u]
 	   [(-c | -C) <commit> | -F <file> | -m <msg> | --amend]
 	   [--allow-empty] [--no-verify] [-e] [--author <author>]
-	   [--] [[-i | -o ]<file>...]
+	   [--verbatim] [--] [[-i | -o ]<file>...]
 
 DESCRIPTION
 -----------

@@ -95,6 +95,11 @@ OPTIONS
 	from making such a commit.  This option bypasses the safety, and
 	is primarily for use by foreign scm interface scripts.
 
+--verbatim::
+	Inhibits stripping of leading and trailing spaces,
+	empty lines and #commentary from the commit message.
+	Implies --allow-empty.
+
 -e|--edit::
 	The message taken from file with `-F`, command line with
 	`-m`, and from file with `-C` are usually used as the

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

@@ -47,6 +47,7 @@ static char *logfile, *force_author, *template_file;
 static char *edit_message, *use_message;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, untracked_files, no_verify, allow_empty;
+static int verbatim_message;
 
 static int no_edit, initial_commit, in_merge;
 const char *only_include_assumed;

@@ -88,6 +89,7 @@ static struct option builtin_commit_options[] = {
 	OPT_BOOLEAN(0, "amend", &amend, "amend previous commit"),
 	OPT_BOOLEAN(0, "untracked-files", &untracked_files, "show all untracked files"),
 	OPT_BOOLEAN(0, "allow-empty", &allow_empty, "ok to record an empty change"),
+	OPT_BOOLEAN(0, "verbatim", &verbatim_message, "do not strip spaces and #comments from message"),
 
 	OPT_END()
 };

@@ -346,7 +348,8 @@ static int prepare_log_message(const char *index_file, const char *prefix)
 	if (fp == NULL)
 		die("could not open %s", git_path(commit_editmsg));
 
-	stripspace(&sb, 0);
+	if (!verbatim_message)
+		stripspace(&sb, 0);
 
 	if (signoff) {
 		struct strbuf sob;

@@ -512,6 +515,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		no_edit = 1;
 	if (edit_flag)
 		no_edit = 0;
+	if (verbatim_message)
+		allow_empty = 1;
 
 	if (get_sha1("HEAD", head_sha1))
 		initial_commit = 1;

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

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 21ac785..42f98bc 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh

@@ -89,4 +89,21 @@ test_expect_success 'verbose' '
 
 '
 
+test_expect_success 'verbatim commit messages' '
+
+	echo >>negative &&
+	git add negative &&
+	{ echo;echo "# text";echo; } >expect &&
+	git commit --verbatim -t expect -a &&
+	git cat-file -p HEAD |sed -e "1,/^\$/d" |head -n 3 >actual &&
+	diff -u expect actual &&
+	git commit --verbatim -F expect -a &&
+	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+	diff -u expect actual &&
+	git commit --verbatim -m "$(cat expect)" -a &&
+	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+	diff -u expect actual
+
+'
+
 test_done

-- 
1.5.4.rc1.33.gbd32b

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.");

Linus Torvalds, Thu, Dec 20, 2007 22:40:13 +0100:

On Thu, 20 Dec 2007, Alex Riesen wrote:

quoted

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.

Right

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.

Yes, I afraid I need both. I use "git commit -t" almost (submission in
perforce takes careful planning) every day. I also would like to keep
the empty leading and trailing lines (perforce default GUI P4Win does
not show them, but our scripts which check the descriptions will test
the description text according to template which does have trailing
empty lines).

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.

I did think about it. It wont be read, I believe (at least I ignore
the status listing git-commit generates today). But then, it can be
removed for verbatim message as no one (I think) will probably care.
Including the "Please..." so it does not look stupid in the commit
message later. Will do.

quoted

It also implies --allow-empty.

I disagree with this one too.

I agree. Will remove (I am not even sure myself, why I did that. It is
not even tested)

Sometimes the message just have to be the way user wants it.
For instance, a template can contain "#" characters, or the message
must be kept as close to its original source as possible for reimport
reasons. Or maybe the user just copied a shell script including its
comments into the commit message for future reference.

Signed-off-by: Alex Riesen <redacted>
---

Updated patch. It conflicts with yours a bit. Will update it

 Documentation/git-commit.txt |    7 ++++++-
 builtin-commit.c             |   20 ++++++++++++++------
 t/t7502-commit.sh            |   18 ++++++++++++++++++
 3 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 4261384..862543f 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt

@@ -11,7 +11,7 @@ SYNOPSIS
 'git-commit' [-a | --interactive] [-s] [-v] [-u]
 	   [(-c | -C) <commit> | -F <file> | -m <msg> | --amend]
 	   [--allow-empty] [--no-verify] [-e] [--author <author>]
-	   [--] [[-i | -o ]<file>...]
+	   [--verbatim] [--] [[-i | -o ]<file>...]
 
 DESCRIPTION
 -----------

@@ -95,6 +95,11 @@ OPTIONS
 	from making such a commit.  This option bypasses the safety, and
 	is primarily for use by foreign scm interface scripts.
 
+--verbatim::
+	Inhibits stripping of leading and trailing spaces,
+	empty lines and #commentary from the commit message.
+	Implies --allow-empty.
+
 -e|--edit::
 	The message taken from file with `-F`, command line with
 	`-m`, and from file with `-C` are usually used as the

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

@@ -47,6 +47,7 @@ static char *logfile, *force_author, *template_file;
 static char *edit_message, *use_message;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, untracked_files, no_verify, allow_empty;
+static int verbatim_message;
 
 static int no_edit, initial_commit, in_merge;
 const char *only_include_assumed;

@@ -88,6 +89,7 @@ static struct option builtin_commit_options[] = {
 	OPT_BOOLEAN(0, "amend", &amend, "amend previous commit"),
 	OPT_BOOLEAN(0, "untracked-files", &untracked_files, "show all untracked files"),
 	OPT_BOOLEAN(0, "allow-empty", &allow_empty, "ok to record an empty change"),
+	OPT_BOOLEAN(0, "verbatim", &verbatim_message, "do not strip spaces and #comments from message"),
 
 	OPT_END()
 };

@@ -346,7 +348,8 @@ static int prepare_log_message(const char *index_file, const char *prefix)
 	if (fp == NULL)
 		die("could not open %s", git_path(commit_editmsg));
 
-	stripspace(&sb, 0);
+	if (!verbatim_message)
+		stripspace(&sb, 0);
 
 	if (signoff) {
 		struct strbuf sob;

@@ -404,10 +407,11 @@ static int prepare_log_message(const char *index_file, const char *prefix)
 			"#\n",
 			git_path("MERGE_HEAD"));
 
-	fprintf(fp,
-		"\n"
-		"# Please enter the commit message for your changes.\n"
-		"# (Comment lines starting with '#' will not be included)\n");
+	if (!verbatim_message)
+		fprintf(fp,
+			"\n"
+			"# Please enter the commit message for your changes.\n"
+			"# (Comment lines starting with '#' will not be included)\n");
 	if (only_include_assumed)
 		fprintf(fp, "# %s\n", only_include_assumed);
 

@@ -431,6 +435,9 @@ static int message_is_empty(struct strbuf *sb, int start)
 	const char *nl;
 	int eol, i;
 
+	if (verbatim_message && sb->len)
+		return 1;
+
 	/* 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) {

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

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 21ac785..d549728 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh

@@ -89,4 +89,22 @@ test_expect_success 'verbose' '
 
 '
 
+test_expect_success 'verbatim commit messages' '
+
+	echo >>negative &&
+	{ echo;echo "# text";echo; } >expect &&
+	git commit --verbatim -t expect -a &&
+	git cat-file -p HEAD |sed -e "1,/^\$/d" |head -n 3 >actual &&
+	diff -u expect actual &&
+	echo >>negative &&
+	git commit --verbatim -F expect -a &&
+	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+	diff -u expect actual &&
+	echo >>negative &&
+	git commit --verbatim -m "$(cat expect)" -a &&
+	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+	diff -u expect actual
+
+'
+
 test_done

-- 
1.5.4.rc1.33.gbd32b

Originally-by: Linus Torvalds [off-list ref]
Signed-off-by: Alex Riesen <redacted>
---
 builtin-commit.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 3127247..1356d20 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c

@@ -441,7 +441,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;

@@ -821,7 +821,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		strbuf_setlen(&sb, p - sb.buf + 1);
 
 	if (!verbatim_message)
-		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.");

-- 
1.5.4.rc1.33.gbd32b

Signed-off-by: Alex Riesen <redacted>
---

Sorry. It must be late. It it must have been late yesterday
and still is today.

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

diff --git a/builtin-commit.c b/builtin-commit.c
index 1356d20..eae7661 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c

@@ -436,7 +436,7 @@ static int message_is_empty(struct strbuf *sb, int start)
 	int eol, i;
 
 	if (verbatim_message && sb->len)
-		return 1;
+		return 0;
 
 	/* See if the template is just a prefix of the message. */
 	strbuf_init(&tmpl, 0);

-- 
1.5.4.rc1.33.gbd32b

On Fri, 21 Dec 2007, Alex Riesen wrote:

Yes, I afraid I need both. I use "git commit -t" almost (submission in
perforce takes careful planning) every day. I also would like to keep
the empty leading and trailing lines (perforce default GUI P4Win does
not show them, but our scripts which check the descriptions will test
the description text according to template which does have trailing
empty lines).

Hmm. I think your updated patch was pretty good, although I still think it 
could be improved a bit. In particular, thinking more about it, I think we 
have more than an "on/off" switch - we really have three cases:

 a) strip whitespace _and_ comments
 b) strip unnecessary whitespace only
 c) leave things _totally_ alone

and on top of that we also have the issue of an editor.

So my patch basically said that in the absense of an editor, we'll still 
clean up whitespace, but not comments (ie "no_edit" implies doing (b) 
rather than (b)), while your patch basically results in (c) regardless of 
whether we run an editor or not.

But that still leaves one case: do we ever want to do (b) even *if* we use 
an editor? There's another possible choice: our old behaviour of (a) in 
the presense of an editor is now gone.

Now, that last choice (ie "case (a) without an editor") is not only 
unlikely to be anything people want to do anyway, it's also easy enough to 
do by just using "git stripspace -s" on whatever non-editor thing you feed 
to "git commit", so I don't think we need to worry about that one.

But the "maybe you want to run an editor, and you _do_ want case (b)" 
sounds like a case that is not at all unlikely. I could easily see the 
case where you want to have a template that uses '#', and *despite* that 
you want to (a) allow the user to edit things _and_ (b) clean up 
whitespace too.

So I'd almost suggest you make the "--verbatim" flag a three-way switch, 
to allow "totally verbatim" (leave everything in place) and a "don't touch 
comments" (just fix up whitespace) mode.

Hmm? Does that make sense to you?

			Linus

On 2007.12.20 15:55:18 -0800, Linus Torvalds wrote:

On Fri, 21 Dec 2007, Alex Riesen wrote:

quoted

Yes, I afraid I need both. I use "git commit -t" almost (submission in
perforce takes careful planning) every day. I also would like to keep
the empty leading and trailing lines (perforce default GUI P4Win does
not show them, but our scripts which check the descriptions will test
the description text according to template which does have trailing
empty lines).

Hmm. I think your updated patch was pretty good, although I still think it 
could be improved a bit. In particular, thinking more about it, I think we 
have more than an "on/off" switch - we really have three cases:

 a) strip whitespace _and_ comments
 b) strip unnecessary whitespace only
 c) leave things _totally_ alone

and on top of that we also have the issue of an editor.

So my patch basically said that in the absense of an editor, we'll still 
clean up whitespace, but not comments (ie "no_edit" implies doing (b) 
rather than (b)), while your patch basically results in (c) regardless of 
whether we run an editor or not.

But that still leaves one case: do we ever want to do (b) even *if* we use 
an editor? There's another possible choice: our old behaviour of (a) in 
the presense of an editor is now gone.

Now, that last choice (ie "case (a) without an editor") is not only 
unlikely to be anything people want to do anyway, it's also easy enough to 
do by just using "git stripspace -s" on whatever non-editor thing you feed 
to "git commit", so I don't think we need to worry about that one.

But the "maybe you want to run an editor, and you _do_ want case (b)" 
sounds like a case that is not at all unlikely. I could easily see the 
case where you want to have a template that uses '#', and *despite* that 
you want to (a) allow the user to edit things _and_ (b) clean up 
whitespace too.

So I'd almost suggest you make the "--verbatim" flag a three-way switch, 
to allow "totally verbatim" (leave everything in place) and a "don't touch 
comments" (just fix up whitespace) mode.

Hmm? Does that make sense to you?

Hm, this is a bit more intrusive, but should catch most cases.

At the top of the comments in the commit message template add:
#GIT CUT HERE
(And adjust the descriptive text)

That line hopefully being uncommon enough to not affect any existing
stuff.

If that line is present, comment lines above it are kept, otherwise
they're removed. Whitespace is always fixed(?).

Results:
 - git commit -m "# Foo and bar"
   * keeps the comment, looks like the expected thing

 - git commit with editor
   * Comments that are manually added are kept
   * For the (probably seldom) case, that you want manually added
     comments to be stripped, you can still remove the "#GIT CUR HERE"
     line

 - git commit with template
   * The existing templates probably won't have a "#GIT CUT HERE" line,
     so we're backwards compatible
   * Templates that want to keep the comments can simple get a "#GIT CUT
     HERE" line at the end and Just Work, regardless of whether or not
     you forget to pass --verbatim.


Hmm?

thanks,
Björn

On Fri, 21 Dec 2007, Bj?rn Steinbrink wrote:

Hm, this is a bit more intrusive, but should catch most cases.

At the top of the comments in the commit message template add:
#GIT CUT HERE
(And adjust the descriptive text)

Ouch. I'd personally hate to see something like that at the top and then 
have to save it. I always add my text to the top of the message, and all 
the pre-made messages for me are at the top (ie the kinds you get with 
"git commit --amend", where the top of the thing is the old message).

That said, I might well agree with this approach if we made the marker 
line be the *last* line of the message, ie make it be something that ends 
with (ignoring empty whitespace at the end, of course, since those are 
invisible in most editors):

	# Remove this line to keep all comment lines

or something like that.

That still keeps the question about whitespace cleanups. But if it's just 
about whitespace or no whitespace, then a simple "--verbatim" flag would 
work.

			Linus