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