Jeff King <peff@xxxxxxxx> 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) {
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html