Re: [PATCH] Do not strip empty lines / trailing spaces from a commit message template

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Thu, Mar 11, 2010 at 03:12:13AM -0500, Jeff King wrote:
>
>> >  	if (fp == NULL)
>> >  		die_errno("could not open '%s'", git_path(commit_editmsg));
>> >  
>> > -	if (cleanup_mode != CLEANUP_NONE)
>> > +	if (cleanup_mode != CLEANUP_NONE && strcmp(hook_arg1, "template"))
>> >  		stripspace(&sb, 0);
>> 
>> And the code looks OK, though admittedly I am not too familiar with this
>> chunk of code (at first I was confused that you would have to look at
>> hook_arg1, but apparently there is no other variable that contains the
>> result of that big if-else chain).


I suspect that the attached would be much easier to read and understand.

> BTW, a subtle point for anyone else reviewing this patch: we also call
> stripspace in message_is_empty to skip over an untouched template. But
> that code path is stil OK, because we stripspace the whole message that
> comes back from the user before calling message_is_empty(), so the
> result should be the same for an untouched template.
>
> -Peff

Thanks for a patch and a review.

> How about a test to check the new behavior?

Speaking of tests, t2203 will segfault with your patch.  I don't think the
following does, though.

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

diff --git a/builtin-commit.c b/builtin-commit.c
index 8a68dd3..14488d5 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -534,6 +534,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	const char *hook_arg1 = NULL;
 	const char *hook_arg2 = NULL;
 	int ident_shown = 0;
+	int clean_message_contents = (cleanup_mode != CLEANUP_NONE);
 
 	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
 		return 0;
@@ -571,6 +572,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		if (strbuf_read_file(&sb, template_file, 0) < 0)
 			die_errno("could not read '%s'", template_file);
 		hook_arg1 = "template";
+		clean_message_contents = 0;
 	}
 
 	/*
@@ -584,7 +586,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	if (fp == NULL)
 		die_errno("could not open '%s'", git_path(commit_editmsg));
 
-	if (cleanup_mode != CLEANUP_NONE && strcmp(hook_arg1, "template"))
+	if (clean_message_contents)
 		stripspace(&sb, 0);
 
 	if (signoff) {