Re: [PATCH 2/2] commit-template: add new line before status information
From: Junio C Hamano <hidden>
Date: 2017-06-29 18:17:57
Kaartic Sivaraam [off-list ref] writes:
The commit template adds the optional parts without a new line to distinguish them. This results in difficulty in interpreting it's content, specifically for inexperienced users. Add new lines to separate the distinct parts of the template.
The rationale of this has changed in this final version, hasn't it, especially with the removal of the "include/only warning" bit? We used to add a blank line to separate the "we are committing for somebody else", which is an optional part, from the status output, only when we have the optional part. Now with your change, we unconditionally have a blank before the part that would have been shown by "git status" to separate the two parts out.
quoted hunk
--- I tried writing tests to ensure that the new line is added but as it seems to require checking multi-line, special options of grep were required to check. I tried the following, test_expect_success 'new line found before status message' ' ! (GIT_EDITOR="cat >editor-input" git commit) && grep -Pz "#\n# On branch" editor-input ' It worked well locally but seems to make the build with GETTEXT_POISON=YesPlease to fail. So, I removed it. Not sure how to write a good test for this change, sorry :( builtin/commit.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)diff --git a/builtin/commit.c b/builtin/commit.c index 64701c8f4..22d17e6f2 100644 --- a/builtin/commit.c +++ b/builtin/commit.c@@ -873,8 +873,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, (int)(ci.name_end - ci.name_begin), ci.name_begin, (int)(ci.mail_end - ci.mail_begin), ci.mail_begin); - if (ident_shown) - status_printf_ln(s, GIT_COLOR_NORMAL, "%s", ""); + status_printf_ln(s, GIT_COLOR_NORMAL, "%s", ""); /* Add new line for clarity */ saved_color_setting = s->use_color; s->use_color = 0;