[PATCH 0/2] format-patch --force-inbody-from

[PATCH 0/2] format-patch --force-inbody-from

[Junio C Hamano]

Users may be authoring and committing their commits under the same
e-mail address they use to send their patches from, in which case
they shouldn't need to use the in-body From: line in their outgoing
e-mails.  At the receiving end, "git am" will use the address on the
"From:" header of the incoming e-mail and all should be well.

Some mailing lists, however, mangle the From: address from what the
original sender had; in such an unfortunate situation, the user may
want to add the in-body "From:" header even for their own patch.

The two-patch series may help such users.

 * The first one introduces a small helper to separate the logic
   that decides when in-body From: is used.

 * The second one adds a minimum support with a new test.

Junio C Hamano (2):
  pretty: separate out the logic to decide the use of in-body from
  format-patch: allow forcing the use of in-body From: header

 builtin/log.c           |  2 ++
 pretty.c                | 13 ++++++++++++-
 revision.h              |  1 +
 t/t4014-format-patch.sh | 13 +++++++++++++
 4 files changed, 28 insertions(+), 1 deletion(-)

-- 
2.37.2-587-g47adba97a9

[PATCH 1/2] pretty: separate out the logic to decide the use of in-body from

[Junio C Hamano]

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.

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.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 pretty.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

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

[PATCH 2/2] format-patch: allow forcing the use of in-body From: header

[Junio C Hamano]

Users may be authoring and committing their commits under the same
e-mail address they use to send their patches from, in which case
they shouldn't need to use the in-body From: line in their outgoing
e-mails.  At the receiving end, "git am" will use the address on the
"From:" header of the incoming e-mail and all should be well.

Some mailing lists, however, mangle the From: address from what the
original sender had; in such an unfortunate situation, the user may
want to add the in-body "From:" header even for their own patch.

"git format-patch --[no-]force-inbody-from" was invented for such
users.

Note.  This is an uncooked early draft.  Things to think about
include (but not limited to, of course):

 * Should this rather be --use-inbody-from=yes,no,auto tristate,
   that defaults to "auto", which is the current behaviour i.e.
   "when --from is given, add it only when it does not match the
   payload".  "yes" would mean "always emit the --from address as
   in-body From:" and "no" would mean ... what?  "Ignore --from"?
   Then why is the user giving --from in the first place?

 * Should it be "inbody" or "in-body"?

 * Should it have a corresponding configuration variable?

 * Should this patch be scrapped and the feature should be done
   inside "git send-email" instead?

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/log.c           |  2 ++
 pretty.c                |  2 ++
 revision.h              |  1 +
 t/t4014-format-patch.sh | 13 +++++++++++++
 4 files changed, 18 insertions(+)

diff --git a/builtin/log.c b/builtin/log.c
index 9b937d59b8..83b2d01b49 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1897,6 +1897,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			   N_("show changes against <refspec> in cover letter or single patch")),
 		OPT_INTEGER(0, "creation-factor", &creation_factor,
 			    N_("percentage by which creation is weighted")),
+		OPT_BOOL(0, "force-inbody-from", &rev.force_inbody_from,
+			 N_("Use in-body From: even for your own commit")),
 		OPT_END()
 	};
 
diff --git a/pretty.c b/pretty.c
index 51e3fa5736..e266208c0b 100644
--- a/pretty.c
+++ b/pretty.c
@@ -483,6 +483,8 @@ static int use_inbody_from(const struct pretty_print_context *pp, const struct i
 		return 0;
 	if (ident_cmp(pp->from_ident, ident))
 		return 1;
+	if (pp->rev && pp->rev->force_inbody_from)
+		return 1;
 	return 0;
 }
 
diff --git a/revision.h b/revision.h
index bb91e7ed91..a2d3813a21 100644
--- a/revision.h
+++ b/revision.h
@@ -208,6 +208,7 @@ struct rev_info {
 
 	/* Format info */
 	int		show_notes;
+	unsigned int	force_inbody_from;
 	unsigned int	shown_one:1,
 			shown_dashes:1,
 			show_merge:1,
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index fbec8ad2ef..a4ecd433e2 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1400,6 +1400,19 @@ test_expect_success '--from omits redundant in-body header' '
 	test_cmp expect patch.head
 '
 
+test_expect_success 'with --force-inbody-from, --from keeps redundant in-body header' '
+	git format-patch --force-inbody-from \
+		-1 --stdout --from="A U Thor <author@example.com>" >patch &&
+	cat >expect <<-\EOF &&
+	From: A U Thor <author@example.com>
+
+	From: A U Thor <author@example.com>
+
+	EOF
+	sed -ne "/^From:/p; /^$/p; /^---$/q" patch >patch.head &&
+	test_cmp expect patch.head
+'
+
 test_expect_success 'in-body headers trigger content encoding' '
 	test_env GIT_AUTHOR_NAME="éxötìc" test_commit exotic &&
 	test_when_finished "git reset --hard HEAD^" &&
-- 
2.37.2-587-g47adba97a9

Re: [PATCH 1/2] pretty: separate out the logic to decide the use of in-body from

[Johannes Schindelin]

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

Re: [PATCH 2/2] format-patch: allow forcing the use of in-body From: header

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 5108 bytes --]

Hi Junio,

On Fri, 26 Aug 2022, Junio C Hamano wrote:

> Users may be authoring and committing their commits under the same
> e-mail address they use to send their patches from, in which case
> they shouldn't need to use the in-body From: line in their outgoing
> e-mails.  At the receiving end, "git am" will use the address on the
> "From:" header of the incoming e-mail and all should be well.
>
> Some mailing lists, however, mangle the From: address from what the
> original sender had; in such an unfortunate situation, the user may
> want to add the in-body "From:" header even for their own patch.
>
> "git format-patch --[no-]force-inbody-from" was invented for such
> users.
>
> Note.  This is an uncooked early draft.

Did you mean to mark the patch as [RFC], then?

> Things to think about include (but not limited to, of course):
>
>  * Should this rather be --use-inbody-from=yes,no,auto tristate,
>    that defaults to "auto", which is the current behaviour i.e.
>    "when --from is given, add it only when it does not match the
>    payload".  "yes" would mean "always emit the --from address as
>    in-body From:" and "no" would mean ... what?  "Ignore --from"?
>    Then why is the user giving --from in the first place?

I would offer up the suggestion `--in-body-from={never,always,auto}` for
consideration.

>  * Should it be "inbody" or "in-body"?

The latter.

>  * Should it have a corresponding configuration variable?

Probably. The commit message talks about mailing lists requiring different
behavior from the default, which is likely to affect all patches generated
from a corresponding local checkout. Having a config variable would lower
the cognitive burden of having to remember this process detail.

>  * Should this patch be scrapped and the feature should be done
>    inside "git send-email" instead?

Since it affects the `--pretty=email` mode, the current patch seems to aim
for the correct layer.

> diff --git a/builtin/log.c b/builtin/log.c
> index 9b937d59b8..83b2d01b49 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1897,6 +1897,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  			   N_("show changes against <refspec> in cover letter or single patch")),
>  		OPT_INTEGER(0, "creation-factor", &creation_factor,
>  			    N_("percentage by which creation is weighted")),
> +		OPT_BOOL(0, "force-inbody-from", &rev.force_inbody_from,
> +			 N_("Use in-body From: even for your own commit")),

Please start the usage text in lower-case, to keep it consistent with the
rest of the usage texts.

Also, I would like to avoid the personal address "you" in that text, and
also the verb "use". Maybe something like this:

	show in-body From: even if identical to the header

>  		OPT_END()
>  	};
>
> diff --git a/pretty.c b/pretty.c
> index 51e3fa5736..e266208c0b 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -483,6 +483,8 @@ static int use_inbody_from(const struct pretty_print_context *pp, const struct i
>  		return 0;
>  	if (ident_cmp(pp->from_ident, ident))
>  		return 1;
> +	if (pp->rev && pp->rev->force_inbody_from)
> +		return 1;

It would probably make sense to move this before `ident_cmp()`, to avoid
unneeded calls ("is the ident the same? no? well, thank you for your
answer but I'll insert the header anyway!").

>  	return 0;
>  }
>
> diff --git a/revision.h b/revision.h
> index bb91e7ed91..a2d3813a21 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -208,6 +208,7 @@ struct rev_info {
>
>  	/* Format info */
>  	int		show_notes;
> +	unsigned int	force_inbody_from;

The reason why this isn't added to the `:1` bits below is probably the
anticipation of the tri-state, but if that tri-state never materializes,
adding it as a bit is still the right thing to do.

>  	unsigned int	shown_one:1,
>  			shown_dashes:1,
>  			show_merge:1,
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index fbec8ad2ef..a4ecd433e2 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1400,6 +1400,19 @@ test_expect_success '--from omits redundant in-body header' '
>  	test_cmp expect patch.head
>  '
>
> +test_expect_success 'with --force-inbody-from, --from keeps redundant in-body header' '
> +	git format-patch --force-inbody-from \
> +		-1 --stdout --from="A U Thor <author@example.com>" >patch &&
> +	cat >expect <<-\EOF &&
> +	From: A U Thor <author@example.com>
> +
> +	From: A U Thor <author@example.com>
> +
> +	EOF
> +	sed -ne "/^From:/p; /^$/p; /^---$/q" patch >patch.head &&
> +	test_cmp expect patch.head
> +'

The test script starts to look a bit non-DRY with all those repetitions of
`A U Thor <author@example.com>`, but that's hardly the responsibility of
this here patch to address.

Thank you,
Dscho

> +
>  test_expect_success 'in-body headers trigger content encoding' '
>  	test_env GIT_AUTHOR_NAME="éxötìc" test_commit exotic &&
>  	test_when_finished "git reset --hard HEAD^" &&
> --
> 2.37.2-587-g47adba97a9
>
>

Re: [PATCH 1/2] pretty: separate out the logic to decide the use of in-body from

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

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

It reads much better.

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

Re: [PATCH 2/2] format-patch: allow forcing the use of in-body From: header

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Note.  This is an uncooked early draft.
>
> Did you mean to mark the patch as [RFC], then?

It is mixture between that and WIP.

>> Things to think about include (but not limited to, of course):
>>
>>  * Should this rather be --use-inbody-from=yes,no,auto tristate,
>>    that defaults to "auto", which is the current behaviour i.e.
>>    "when --from is given, add it only when it does not match the
>>    payload".  "yes" would mean "always emit the --from address as
>>    in-body From:" and "no" would mean ... what?  "Ignore --from"?
>>    Then why is the user giving --from in the first place?
>
> I would offer up the suggestion `--in-body-from={never,always,auto}` for
> consideration.

That is essentially the same as the "Boolean plus auto" tristate, a
very common pattern in our UI.  The problem is that false-never-no
does not make much sense in this case, because you do not need it.
You can instead refrain from passing --from to achieve the same
effect.

>>  * Should it be "inbody" or "in-body"?
>
> The latter.

OK.  This cascades up to 1/2 (there is a new helper function with
the phrase in its name).

>>  * Should it have a corresponding configuration variable?
>
> Probably. The commit message talks about mailing lists requiring different
> behavior from the default, which is likely to affect all patches generated
> from a corresponding local checkout. Having a config variable would lower
> the cognitive burden of having to remember this process detail.

OK.

>> diff --git a/builtin/log.c b/builtin/log.c
>> index 9b937d59b8..83b2d01b49 100644
>> --- a/builtin/log.c
>> +++ b/builtin/log.c
>> @@ -1897,6 +1897,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>>  			   N_("show changes against <refspec> in cover letter or single patch")),
>>  		OPT_INTEGER(0, "creation-factor", &creation_factor,
>>  			    N_("percentage by which creation is weighted")),
>> +		OPT_BOOL(0, "force-inbody-from", &rev.force_inbody_from,
>> +			 N_("Use in-body From: even for your own commit")),
>
> Please start the usage text in lower-case, to keep it consistent with the
> rest of the usage texts.

Right.

> Also, I would like to avoid the personal address "you" in that text, and
> also the verb "use". Maybe something like this:
>
> 	show in-body From: even if identical to the header

Much nicer.  I'll take it.

>> diff --git a/pretty.c b/pretty.c
>> index 51e3fa5736..e266208c0b 100644
>> --- a/pretty.c
>> +++ b/pretty.c
>> @@ -483,6 +483,8 @@ static int use_inbody_from(const struct pretty_print_context *pp, const struct i
>>  		return 0;
>>  	if (ident_cmp(pp->from_ident, ident))
>>  		return 1;
>> +	if (pp->rev && pp->rev->force_inbody_from)
>> +		return 1;
>
> It would probably make sense to move this before `ident_cmp()`, to avoid
> unneeded calls ("is the ident the same? no? well, thank you for your
> answer but I'll insert the header anyway!").

I tend to prefer adding new things at the end when all things are
equal, but in this case the new thing is an overriding condition, so
it does make sense to have it before the call.

>> diff --git a/revision.h b/revision.h
>> index bb91e7ed91..a2d3813a21 100644
>> --- a/revision.h
>> +++ b/revision.h
>> @@ -208,6 +208,7 @@ struct rev_info {
>>
>>  	/* Format info */
>>  	int		show_notes;
>> +	unsigned int	force_inbody_from;
>
> The reason why this isn't added to the `:1` bits below is probably the
> anticipation of the tri-state, but if that tri-state never materializes,
> adding it as a bit is still the right thing to do.

It might make sense to turn this into the common "Boolean plus auto"
tristate, but the utility of "no" in this case is dubious, so I was
not planning to go that route.

This member is a full fledged word because the address of it given
to OPT_BOOL(), and we cannot take an address of a bitfield member in
a struct.

Bit-pinching in this struct is not very useful.  Even if we traverse
a million commits in a single run, we will use a single "struct
rev_info" instance.

Thanks for reading it over.

[PATCH v2 0/3] format-patch --force-in-body-from

[Junio C Hamano]

Users may be authoring and committing their commits under the same
e-mail address they use to send their patches from, in which case
they shouldn't need to use the in-body From: line in their outgoing
e-mails.  At the receiving end, "git am" will use the address on the
"From:" header of the incoming e-mail and all should be well.

Some mailing lists, however, mangle the From: address from what the
original sender had; in such an unfortunate situation, the user may
want to add the in-body "From:" header even for their own patch.

A new option `--force-in-body-from` from the command line of `git
format-patch`, and a new configuration variable `format.forceInBodyFrom`
that can be set per repository, are added to address this.

Changes since the original submission are

 * The configuration variable support is new.

 * comes with documentation updates.

 * "in-body" is spelled as such, not "inbody".

 * the order of config and option parsing plus initializing the
   rev_info structure dictates a separate variable to be used for
   the former two and then copied to rev_info later.

Junio C Hamano (3):
  pretty: separate out the logic to decide the use of in-body from
  format-patch: allow forcing the use of in-body From: header
  format-patch: learn format.forceInBodyFrom configuration variable

 Documentation/config/format.txt    |  4 ++++
 Documentation/git-format-patch.txt | 11 +++++++++
 builtin/log.c                      |  9 ++++++++
 pretty.c                           | 12 +++++++++-
 revision.h                         |  1 +
 t/t4014-format-patch.sh            | 37 ++++++++++++++++++++++++++++++
 6 files changed, 73 insertions(+), 1 deletion(-)

-- 
2.37.2-621-gd3a800faf0

[PATCH v2 1/3] pretty: separate out the logic to decide the use of in-body from

[Junio C Hamano]

When pretty-printing the log message for a given commit in the
e-mail format (e.g. "git format-patch"), we add an in-body "From:"
header when the author identity of the commit is different from the
identity of the person whose identity appears in the header of the
e-mail (the latter is passed with them "--from" option).

Split out the logic into a helper function, as we would want to
extend the condition further.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 pretty.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/pretty.c b/pretty.c
index 6d819103fb..cf418a6b20 100644
--- a/pretty.c
+++ b/pretty.c
@@ -477,6 +477,14 @@ static void append_line_with_color(struct strbuf *sb, struct grep_opt *opt,
 	}
 }
 
+static int use_in_body_from(const struct pretty_print_context *pp,
+			    const struct ident_split *ident)
+{
+	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 +511,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 (pp->from_ident && use_in_body_from(pp, &ident)) {
 			struct strbuf buf = STRBUF_INIT;
 
 			strbuf_addstr(&buf, "From: ");
-- 
2.37.2-621-gd3a800faf0

[PATCH v2 2/3] format-patch: allow forcing the use of in-body From: header

[Junio C Hamano]

Users may be authoring and committing their commits under the same
e-mail address they use to send their patches from, in which case
they shouldn't need to use the in-body From: line in their outgoing
e-mails.  At the receiving end, "git am" will use the address on the
"From:" header of the incoming e-mail and all should be well.

Some mailing lists, however, mangle the From: address from what the
original sender had; in such a situation, the user may want to add
the in-body "From:" header even for their own patches.

"git format-patch --[no-]force-in-body-from" was invented for such
users.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-format-patch.txt |  9 +++++++++
 builtin/log.c                      |  5 +++++
 pretty.c                           |  2 ++
 revision.h                         |  1 +
 t/t4014-format-patch.sh            | 13 +++++++++++++
 5 files changed, 30 insertions(+)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index be797d7a28..7c7f244e57 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -275,6 +275,15 @@ header). Note also that `git send-email` already handles this
 transformation for you, and this option should not be used if you are
 feeding the result to `git send-email`.
 
+--[no-]force-in-body-from::
+	With the e-mail sender specified via the `--from` option, by
+	default, an in-body "From:" to identify the real author of
+	the commit is added at the top of the commit log message if
+	the sender is different from the author.  With this option,
+	the in-body "From:" is added even when the sender and the
+	author have the same name and address, which may help if the
+	mailing list software mangles the sender's identity.
+
 --add-header=<header>::
 	Add an arbitrary header to the email headers.  This is in addition
 	to any configured headers, and may be used multiple times.
diff --git a/builtin/log.c b/builtin/log.c
index 9b937d59b8..78ccd37bd9 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -52,6 +52,7 @@ static int default_encode_email_headers = 1;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config = 1;
+static unsigned int force_in_body_from;
 static const char *fmt_patch_subject_prefix = "PATCH";
 static int fmt_patch_name_max = FORMAT_PATCH_NAME_MAX_DEFAULT;
 static const char *fmt_pretty;
@@ -1897,6 +1898,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			   N_("show changes against <refspec> in cover letter or single patch")),
 		OPT_INTEGER(0, "creation-factor", &creation_factor,
 			    N_("percentage by which creation is weighted")),
+		OPT_BOOL(0, "force-in-body-from", &force_in_body_from,
+			 N_("show in-body From: even if identical to the e-mail header")),
 		OPT_END()
 	};
 
@@ -1940,6 +1943,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
 			     PARSE_OPT_KEEP_DASHDASH);
 
+	rev.force_in_body_from = force_in_body_from;
+
 	/* Make sure "0000-$sub.patch" gives non-negative length for $sub */
 	if (fmt_patch_name_max <= strlen("0000-") + strlen(fmt_patch_suffix))
 		fmt_patch_name_max = strlen("0000-") + strlen(fmt_patch_suffix);
diff --git a/pretty.c b/pretty.c
index cf418a6b20..b7553e3fe0 100644
--- a/pretty.c
+++ b/pretty.c
@@ -480,6 +480,8 @@ static void append_line_with_color(struct strbuf *sb, struct grep_opt *opt,
 static int use_in_body_from(const struct pretty_print_context *pp,
 			    const struct ident_split *ident)
 {
+	if (pp->rev && pp->rev->force_in_body_from)
+		return 1;
 	if (ident_cmp(pp->from_ident, ident))
 		return 1;
 	return 0;
diff --git a/revision.h b/revision.h
index bb91e7ed91..6e346a60ab 100644
--- a/revision.h
+++ b/revision.h
@@ -221,6 +221,7 @@ struct rev_info {
 			missing_newline:1,
 			date_mode_explicit:1,
 			preserve_subject:1,
+			force_in_body_from:1,
 			encode_email_headers:1,
 			include_header:1;
 	unsigned int	disable_stdin:1;
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index fbec8ad2ef..347f7f7f35 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1400,6 +1400,19 @@ test_expect_success '--from omits redundant in-body header' '
 	test_cmp expect patch.head
 '
 
+test_expect_success 'with --force-in-body-from, redundant in-body from is kept' '
+	git format-patch --force-in-body-from \
+		-1 --stdout --from="A U Thor <author@example.com>" >patch &&
+	cat >expect <<-\EOF &&
+	From: A U Thor <author@example.com>
+
+	From: A U Thor <author@example.com>
+
+	EOF
+	sed -ne "/^From:/p; /^$/p; /^---$/q" patch >patch.head &&
+	test_cmp expect patch.head
+'
+
 test_expect_success 'in-body headers trigger content encoding' '
 	test_env GIT_AUTHOR_NAME="éxötìc" test_commit exotic &&
 	test_when_finished "git reset --hard HEAD^" &&
-- 
2.37.2-621-gd3a800faf0

[PATCH v2 3/3] format-patch: learn format.forceInBodyFrom configuration variable

[Junio C Hamano]

As the need to use the "--force-in-body-from" option primarily is
tied to which mailing list the mails go to (and get their From:
address mangled), it is likely that a user who needs to use this
option once to interact with their upstream project needs to use it
for all patches they send out.

Add a configuration variable, suitable for setting in the local
configuration file per repository, for this.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config/format.txt    |  4 ++++
 Documentation/git-format-patch.txt |  2 ++
 builtin/log.c                      |  4 ++++
 t/t4014-format-patch.sh            | 24 ++++++++++++++++++++++++
 4 files changed, 34 insertions(+)

diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index fdbc06a4d2..c7303d8d9f 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -15,6 +15,10 @@ format.from::
 	different.  If set to a non-boolean value, format-patch uses that
 	value instead of your committer identity.  Defaults to false.
 
+format.forceInBodyFrom::
+	Provides the default value for the `--[no-]force-in-body-from`
+	option to format-patch.  Defaults to false.
+
 format.numbered::
 	A boolean which can enable or disable sequence numbers in patch
 	subjects.  It defaults to "auto" which enables it only if there
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 7c7f244e57..dfcc7da4c2 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -283,6 +283,8 @@ feeding the result to `git send-email`.
 	the in-body "From:" is added even when the sender and the
 	author have the same name and address, which may help if the
 	mailing list software mangles the sender's identity.
+	Defaults to the value of the `format.forceInBodyFrom`
+	configuration variable.
 
 --add-header=<header>::
 	Add an arbitrary header to the email headers.  This is in addition
diff --git a/builtin/log.c b/builtin/log.c
index 78ccd37bd9..776bc9afdb 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1007,6 +1007,10 @@ static int git_format_config(const char *var, const char *value, void *cb)
 			from = NULL;
 		return 0;
 	}
+	if (!strcmp(var, "format.forceinbodyfrom")) {
+		force_in_body_from = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "format.notes")) {
 		int b = git_parse_maybe_bool(value);
 		if (b < 0)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 347f7f7f35..ad5c029279 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1413,6 +1413,30 @@ test_expect_success 'with --force-in-body-from, redundant in-body from is kept'
 	test_cmp expect patch.head
 '
 
+test_expect_success 'format.forceInBodyFrom, equivalent to --force-in-body-from' '
+	git -c format.forceInBodyFrom=yes format-patch \
+		-1 --stdout --from="A U Thor <author@example.com>" >patch &&
+	cat >expect <<-\EOF &&
+	From: A U Thor <author@example.com>
+
+	From: A U Thor <author@example.com>
+
+	EOF
+	sed -ne "/^From:/p; /^$/p; /^---$/q" patch >patch.head &&
+	test_cmp expect patch.head
+'
+
+test_expect_success 'format.forceInBodyFrom, equivalent to --force-in-body-from' '
+	git -c format.forceInBodyFrom=yes format-patch --no-force-in-body-from \
+		-1 --stdout --from="A U Thor <author@example.com>" >patch &&
+	cat >expect <<-\EOF &&
+	From: A U Thor <author@example.com>
+
+	EOF
+	sed -ne "/^From:/p; /^$/p; /^---$/q" patch >patch.head &&
+	test_cmp expect patch.head
+'
+
 test_expect_success 'in-body headers trigger content encoding' '
 	test_env GIT_AUTHOR_NAME="éxötìc" test_commit exotic &&
 	test_when_finished "git reset --hard HEAD^" &&
-- 
2.37.2-621-gd3a800faf0

Re: [PATCH v2 2/3] format-patch: allow forcing the use of in-body From: header

[Jeff King]

On Mon, Aug 29, 2022 at 02:38:36PM -0700, Junio C Hamano wrote:

> +--[no-]force-in-body-from::
> +	With the e-mail sender specified via the `--from` option, by
> +	default, an in-body "From:" to identify the real author of
> +	the commit is added at the top of the commit log message if
> +	the sender is different from the author.  With this option,
> +	the in-body "From:" is added even when the sender and the
> +	author have the same name and address, which may help if the
> +	mailing list software mangles the sender's identity.

I find it a little curious that this option can only be used with
"--from". That makes sense in a way, because this is a special case of
that feature, overriding the "are they the same" check.

But given that the use case is not to send somebody else's patch, but to
duplicate your _own_ ident in both spots, it feels funny that you must
also say "by the way, I am the sender of the email". I.e., you have to
say:

  git format-patch --from='Me <me@example.com>' --force-in-body-from

I guess it is not too bad because just "--from" will do the equivalent
thing (picking "me" from your committer ident). It just feels kind of
clunky that:

  git format-patch --force-in-body-from

will silently ignore the option.

All that said, I don't care _too_ strongly about it. I do suspect the
feature might be better placed in send-email (or possibly in addition).
If you are using send-email, then I think you're not supposed to use
"--from" with format-patch at all, and you have no way of accessing this
feature.

-Peff

Re: [PATCH v2 2/3] format-patch: allow forcing the use of in-body From: header

[Jeff King]

On Tue, Aug 30, 2022 at 04:07:18PM -0400, Jeff King wrote:

> All that said, I don't care _too_ strongly about it. I do suspect the
> feature might be better placed in send-email (or possibly in addition).
> If you are using send-email, then I think you're not supposed to use
> "--from" with format-patch at all, and you have no way of accessing this
> feature.

This "not supposed to" came from me looking at the documentation. But
this discussion implies that it's maybe not that big a deal:

  https://lore.kernel.org/git/xmqq8twlqwan.fsf@gitster.mtv.corp.google.com/

Apparently we were considering enabling "--from" by default, but I don't
think that ever happened.

-Peff