Re: [PATCH 1/2] pretty: separate out the logic to decide the use of in-body from
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Aug 29, 2022, 11:32 UTC
- Message-ID
- <08p469q7-2os4-p391-9nr4-10q7o9s4414s@tzk.qr>
- In-Reply-To
- <20220826213203.3258022-2-gitster@pobox.com>
Hi Junio,
On Fri, 26 Aug 2022, Junio C Hamano wrote:
> When pretty-printing the log message for a given commit in e-mail > format (e.g. "git format-patch"), we add in-body "From:" header when > the author identity of the commit is different from the identity of > the person who is "sending" the mail.
The quotes around "sending" made me stumble over this a bit. Maybe replace it by saying "the person running the command"?
Show 6 quoted lines
> Split out the logic into a helper function. Because the "from_ident > must be set" condition is there not because it is used in the > ident_cmp() next, but because the codepath that is entered when this > logic says "Yes, you should use in-body from" requires values there > in from_ident member, so separate it out into an if() statement on > its own to clarify it.
Even after reading this three times, I had trouble understanding it. I then consulted the diff and started to grasp what you mean. I have no good idea how to improve the wording, but maybe you can give it another go? Or simply state that the condition was untangled a bit.
The diff looks good.
Ciao, Dscho
P.S.: I do not know how strongly you feel these days about lines longer than 80 columns, but personally I do not care about this rule, so I am more than just fine with adding such a line here.
Show 33 quoted lines
> diff --git a/pretty.c b/pretty.c
> index 6d819103fb..51e3fa5736 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -477,6 +477,15 @@ static void append_line_with_color(struct strbuf *sb, struct grep_opt *opt,
> }
> }
>
> +static int use_inbody_from(const struct pretty_print_context *pp, const struct ident_split *ident)
> +{
> + if (!pp->from_ident)
> + return 0;
> + if (ident_cmp(pp->from_ident, ident))
> + return 1;
> + return 0;
> +}
> +
> void pp_user_info(struct pretty_print_context *pp,
> const char *what, struct strbuf *sb,
> const char *line, const char *encoding)
> @@ -503,7 +512,7 @@ void pp_user_info(struct pretty_print_context *pp,
> map_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);
>
> if (cmit_fmt_is_mail(pp->fmt)) {
> - if (pp->from_ident && ident_cmp(pp->from_ident, &ident)) {
> + if (use_inbody_from(pp, &ident)) {
> struct strbuf buf = STRBUF_INIT;
>
> strbuf_addstr(&buf, "From: ");
> --
> 2.37.2-587-g47adba97a9
>
>