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

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

>
>> @@ -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 ***".