Re: [PATCH 2/2] commit-template: add new line before status information

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Kaartic Sivaraam <kaarticsivaraam91196@xxxxxxxxx> 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.



> ---
>  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;



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux