Johannes Schindelin [off-list ref] writes:
Hi Junio,
On Fri, 26 Aug 2022, Junio C Hamano wrote:
quoted
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"?
It reads much better.
quoted
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.
Yeah, (pp->from_ident != NULL) there serves two purposes. Whatever
else is in that if() statement, the body of the statement depends on
the pp->from_ident being non-NULL and the control must not enter
there otherwise. The other purpose is to guard the other half of
the if() statement, which happens to be ident_cmp() that looks at
the same pp->from_ident and depends on it being non-NULL.
I think the condition gets much cleaner by untangling it to
- use_inbody_from() function does *not* check pp->from_ident; it
just assumes it is not NULL
- the caller becomes
if (pp->from_ident && use_inbody_from(...)) {
... stuff that use pp->from_ident ...
}
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.
I allowed wider line for function decl, by inertia, for
greppability, but I should fix that. Thanks for noticing.