Re: [RFC 3/3] log: add an option to generate cover letter from a branch tip

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 14, 2017, 06:14 UTC
Message-ID: <xmqq7eut4cae.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <936c2b33-3432-f113-d84b-0623246ec673@suse.de>

Nicolas Morey-Chaisemartin <NMoreyChaisemartin@suse.de> writes:

> -	const char *body = "*** SUBJECT HERE ***\n\n*** BLURB HERE ***\n";
> -	const char *msg;
> +	const char *body = "*** SUBJECT HERE ***\n\n*** BLURB HERE ***\n\n";

Hmmmm.

Show 26 quoted lines

> @@ -1021,17 +1021,21 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
>  	if (!branch_name)
>  		branch_name = find_branch_name(rev);
>  
> -	msg = body;
>  	pp.fmt = CMIT_FMT_EMAIL;
>  	pp.date_mode.type = DATE_RFC2822;
>  	pp.rev = rev;
>  	pp.print_email_subject = 1;
> -	pp_user_info(&pp, NULL, &sb, committer, encoding);
> -	pp_title_line(&pp, &msg, &sb, encoding, need_8bit_cte);
> -	pp_remainder(&pp, &msg, &sb, 0);
> -	add_branch_description(&sb, branch_name);
> -	fprintf(rev->diffopt.file, "%s\n", sb.buf);
>  
> +	if (!cover_at_tip_commit) {
> +		pp_user_info(&pp, NULL, &sb, committer, encoding);
> +		pp_title_line(&pp, &body, &sb, encoding, need_8bit_cte);
> +		pp_remainder(&pp, &body, &sb, 0);
> +	} else {
> +		pretty_print_commit(&pp, cover_at_tip_commit, &sb);
> +	}
> +	add_branch_description(&sb, branch_name);
> +	fprintf(rev->diffopt.file, "%s", sb.buf);
> +	fprintf(rev->diffopt.file, "---\n", sb.buf);
>  	strbuf_release(&sb);

I would have expected that this feature would not change anything other than replacing the constant string *body we unconditionally print with the log message of the empty commit at the tip, so from that expectation, I was hoping that a patch looked nothing more than this:

 builtin/log.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/builtin/log.c b/builtin/log.c
index 6c1fa896ad..0af19d5b36 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -986,6 +986,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 			      struct commit *origin,
 			      int nr, struct commit **list,
 			      const char *branch_name,
+			      struct commit *cover,
 			      int quiet)
 {
 	const char *committer;
@@ -1021,7 +1022,10 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	if (!branch_name)
 		branch_name = find_branch_name(rev);
 
-	msg = body;
+	if (cover)
+		msg = get_cover_from_commit(cover);
+	else
+		msg = body;
 	pp.fmt = CMIT_FMT_EMAIL;
 	pp.date_mode.type = DATE_RFC2822;
 	pp.rev = rev;


plus a newly written function get_cover_from_commit().  Why does
this patch need to change a lot more than that, I have to wonder.

This is totally unrelated, but I wonder if it makes sense to do
something similar for branch.description, too.  If the user has a
meaningful description prepared with "git branch --edit-desc", it is
somewhat insulting to the user to still add "*** BLURB HERE ***".