Le 14/11/2017 à 07:14, Junio C Hamano a écrit :
> Nicolas Morey-Chaisemartin <NMoreyChaisemartin@xxxxxxx> 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.
The \n from fprintf(rev->diffopt.file, "%s\n", sb.buf); added an extra line after the cover which is fine for the default one but changed the commit value (at least at some point while I was playing with scissors)
so I moved it up there to avoid adding a if() around the fprintf
>
>> @@ -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.
The added code is to avoid the get_cover_from_commit generating a single strbuf that needs to be reparse/resplit by pp_user_info/pp_title_line/pp_remainder.
There was a helper that did the right job in my case so I took it ;)
The triple dash is so that the diffstat/shortlog as not seen as part of the cover letter.
As said in the cover letter for this series, it kinda breaks legacy behaviour right now.
It should either be printed only for cover-at-tip, or a new separator should be added.
>
> 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 ***".
I guess so.
And the branch description should probably not be added when using cover-at-tip either.
