Re: [PATCH v4] [GSOC] commit: add --trailer option

[Junio C Hamano <gitster@pobox.com>]

"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:

> @@ -958,6 +970,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  
>  	fclose(s->fp);
>  
> +	if (run_trailer.args.nr != 4) {
> +		run_trailer.git_cmd = 1;
> +		run_command(&run_trailer);

This hardcoded magic 4 is very brittle.  It probably makes sense to
have another string vector, that is used only to accumulate
--trailer arguments and nothing else, and check if that string
vector is empty here.

IOW this part would become ...

	if (trailer_args.nr) {
		strvec_pushl(&run_trailer.args,	"interpret-trailers", 
			     "--in-place", ...);
		strvec_pushv(&run_trailer.args, trailer_args.v);
		run_trailer.git_cmd = 1;
		run_command(&run_trailer);
	}

> +	} else
> +		strvec_clear(&run_trailer.args);

... and there is no need to have "else" that won't need to do
anything.

>  	/*
>  	 * Reject an attempt to record a non-merge empty commit without
>  	 * explicit --allow-empty. In the cherry-pick case, it may be
> @@ -1507,6 +1525,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  		OPT_STRING(0, "fixup", &fixup_message, N_("commit"), N_("use autosquash formatted message to fixup specified commit")),
>  		OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
>  		OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
> +		OPT_CALLBACK(0, "trailer", &trailer, N_("trailer"), N_("trailer(s) to add"), opt_pass_trailer),
>  		OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
>  		OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
>  		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),
> @@ -1577,6 +1596,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  			die(_("could not parse HEAD commit"));
>  	}
>  	verbose = -1; /* unspecified */
> +	strvec_pushl(&run_trailer.args, "interpret-trailers",
> +		    "--in-place", "--where=end", git_path_commit_editmsg(), NULL);

And this line will be gone.

>  	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
>  					  builtin_commit_usage,
>  					  prefix, current_head, &s);