Templates should be just that: Forms that the user fills out, and forms
have blanks. If people are attached to not having extra whitespace in the
editor, they can simply clean up their templates.
Signed-off-by: Sebastian Schuberth <redacted>
---
builtin-commit.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index f4c7344..8a68dd3 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -584,7 +584,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)
+ if (cleanup_mode != CLEANUP_NONE && strcmp(hook_arg1, "template"))
stripspace(&sb, 0);
if (signoff) {--
1.7.0.2.msysgit.0.8.g888e.dirty
On Wed, Mar 10, 2010 at 04:57:11PM +0100, Sebastian Schuberth wrote:
Templates should be just that: Forms that the user fills out, and forms
have blanks. If people are attached to not having extra whitespace in the
editor, they can simply clean up their templates.
Rationale makes sense to me...
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).
How about a test to check the new behavior?
-Peff
On Thu, Mar 11, 2010 at 03:12:13AM -0500, Jeff King wrote:
quoted
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).
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