Re: [PATCH] commit-template: improve readability of commit template

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 28, 2017, 16:48 UTC
Message-ID: <xmqqshikvz1s.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170628132910.7940-1-kaarticsivaraam91196@gmail.com>

Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:

Show 14 quoted lines

> The commit template adds the optional parts without
> a new line to distinguish them. This results in
> difficulty in interpreting it's content. Add new
> lines to separate the distinct parts of the template.
>
> The warning about usage of 'explicit paths without
> any corresponding options' has outlived it's purpose of
> warning users about the usage '--only' as default rather
> than '--include'. Remove it.
>
> Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
> ---
>  builtin/commit.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)

I think I can agree with both of the changes, but this commit does two things that may be better done in two separate commits. If I were doing this patch, I probably would make [PATCH 1/2] about removing the only_include_assumed (which is all except for the hunk at -877,8) and then [PATCH 2/2] about giving a separating blank line before the "git status" output.

> The warning about usage of 'explicit paths without
> any corresponding options' has outlived it's purpose of
> warning users about the usage '--only' as default rather
> than '--include'. Remove it.

With a bit more digging of the history:

    The notice that "git commit <paths>" default to "git commit
    --only <paths>" was there since 756e3ee0 ("Merge branch
    'jc/commit'", 2006-02-14).  Back then, existing users of Git
    expected the command doing "git commit --include <paths>", and
    after we changed the behaviour of the command to align with
    other people's "$scm commit <paths>", we added the text to help
    them transition their expectations.  Remove the message that now
    has outlived its usefulness.

perhaps.

Thanks.

Show 41 quoted lines

> diff --git a/builtin/commit.c b/builtin/commit.c
> index 8d1cac062..22d17e6f2 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -139,7 +139,6 @@ static enum commit_whence whence;
>  static int sequencer_in_use;
>  static int use_editor = 1, include_status = 1;
>  static int show_ignored_in_status, have_option_m;
> -static const char *only_include_assumed;
>  static struct strbuf message = STRBUF_INIT;
>  
>  static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
> @@ -841,9 +840,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  				  "with '%c' will be kept; you may remove them"
>  				  " yourself if you want to.\n"
>  				  "An empty message aborts the commit.\n"), comment_line_char);
> -		if (only_include_assumed)
> -			status_printf_ln(s, GIT_COLOR_NORMAL,
> -					"%s", only_include_assumed);
>  
>  		/*
>  		 * These should never fail because they come from our own
> @@ -877,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;
> @@ -1208,8 +1203,6 @@ static int parse_and_validate_options(int argc, const char *argv[],
>  		die(_("Only one of --include/--only/--all/--interactive/--patch can be used."));
>  	if (argc == 0 && (also || (only && !amend && !allow_empty)))
>  		die(_("No paths with --include/--only does not make sense."));
> -	if (argc > 0 && !also && !only)
> -		only_include_assumed = _("Explicit paths specified without -i or -o; assuming --only paths...");
>  	if (!cleanup_arg || !strcmp(cleanup_arg, "default"))
>  		cleanup_mode = use_editor ? CLEANUP_ALL : CLEANUP_SPACE;
>  	else if (!strcmp(cleanup_arg, "verbatim"))