{"thread":{"id":"58369","subject":"[PATCH 1/2] pretty: separate out the logic to decide the use of in-body from","startedAt":"2022-08-26T21:32:09Z","lastAt":"2022-08-30T20:14:17Z","messageCount":13,"participants":["Junio C Hamano","Johannes Schindelin","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"462013","messageId":"20220826213203.3258022-2-gitster@pobox.com","threadId":"58369","inReplyTo":"20220826213203.3258022-1-gitster@pobox.com","subject":"[PATCH 1/2] pretty: separate out the logic to decide the use of in-body from","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-26T21:32:02Z","receivedAt":"2022-08-26T21:32:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When pretty-printing the log message for a given commit in e-mail\nformat (e.g. \"git format-patch\"), we add in-body \"From:\" header when\nthe author identity of the commit is different from the identity of\nthe person who is \"sending\" the mail.\n\nSplit out the logic into a helper function. Because the \"from_ident\nmust be set\" condition is there not because it is used in the\nident_cmp() next, but because the codepath that is entered when this\nlogic says \"Yes, you should use in-body from\" requires values there\nin from_ident member, so separate it out into an if() statement on\nits own to clarify it.\n\nSigned-off-by: Junio C Hamano \n---\n pretty.c | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 6d819103fb..51e3fa5736 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -477,6 +477,15 @@ static void append_line_with_color(struct strbuf *sb, struct grep_opt *opt,\n \t}\n }\n \n+static int use_inbody_from(const struct pretty_print_context *pp, const struct ident_split *ident)\n+{\n+\tif (!pp->from_ident)\n+\t\treturn 0;\n+\tif (ident_cmp(pp->from_ident, ident))\n+\t\treturn 1;\n+\treturn 0;\n+}\n+\n void pp_user_info(struct pretty_print_context *pp,\n \t\t const char *what, struct strbuf *sb,\n \t\t const char *line, const char *encoding)\n@@ -503,7 +512,7 @@ void pp_user_info(struct pretty_print_context *pp,\n \t\tmap_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);\n \n \tif (cmit_fmt_is_mail(pp->fmt)) {\n-\t\tif (pp->from_ident && ident_cmp(pp->from_ident, &ident)) {\n+\t\tif (use_inbody_from(pp, &ident)) {\n \t\t\tstruct strbuf buf = STRBUF_INIT;\n \n \t\t\tstrbuf_addstr(&buf, \"From: \");\n-- \n2.37.2-587-g47adba97a9\n\n"},{"id":"462014","messageId":"20220826213203.3258022-1-gitster@pobox.com","threadId":"58369","inReplyTo":null,"subject":"[PATCH 0/2] format-patch --force-inbody-from","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-26T21:32:01Z","receivedAt":"2022-08-26T21:32:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Users may be authoring and committing their commits under the same\ne-mail address they use to send their patches from, in which case\nthey shouldn't need to use the in-body From: line in their outgoing\ne-mails. At the receiving end, \"git am\" will use the address on the\n\"From:\" header of the incoming e-mail and all should be well.\n\nSome mailing lists, however, mangle the From: address from what the\noriginal sender had; in such an unfortunate situation, the user may\nwant to add the in-body \"From:\" header even for their own patch.\n\nThe two-patch series may help such users.\n\n * The first one introduces a small helper to separate the logic\n that decides when in-body From: is used.\n\n * The second one adds a minimum support with a new test.\n\nJunio C Hamano (2):\n pretty: separate out the logic to decide the use of in-body from\n format-patch: allow forcing the use of in-body From: header\n\n builtin/log.c | 2 ++\n pretty.c | 13 ++++++++++++-\n revision.h | 1 +\n t/t4014-format-patch.sh | 13 +++++++++++++\n 4 files changed, 28 insertions(+), 1 deletion(-)\n\n-- \n2.37.2-587-g47adba97a9\n\n"},{"id":"462015","messageId":"20220826213203.3258022-3-gitster@pobox.com","threadId":"58369","inReplyTo":"20220826213203.3258022-1-gitster@pobox.com","subject":"[PATCH 2/2] format-patch: allow forcing the use of in-body From: header","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-26T21:32:03Z","receivedAt":"2022-08-26T21:32:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Users may be authoring and committing their commits under the same\ne-mail address they use to send their patches from, in which case\nthey shouldn't need to use the in-body From: line in their outgoing\ne-mails. At the receiving end, \"git am\" will use the address on the\n\"From:\" header of the incoming e-mail and all should be well.\n\nSome mailing lists, however, mangle the From: address from what the\noriginal sender had; in such an unfortunate situation, the user may\nwant to add the in-body \"From:\" header even for their own patch.\n\n\"git format-patch --[no-]force-inbody-from\" was invented for such\nusers.\n\nNote. This is an uncooked early draft. Things to think about\ninclude (but not limited to, of course):\n\n * Should this rather be --use-inbody-from=yes,no,auto tristate,\n that defaults to \"auto\", which is the current behaviour i.e.\n \"when --from is given, add it only when it does not match the\n payload\". \"yes\" would mean \"always emit the --from address as\n in-body From:\" and \"no\" would mean ... what? \"Ignore --from\"?\n Then why is the user giving --from in the first place?\n\n * Should it be \"inbody\" or \"in-body\"?\n\n * Should it have a corresponding configuration variable?\n\n * Should this patch be scrapped and the feature should be done\n inside \"git send-email\" instead?\n\nSigned-off-by: Junio C Hamano \n---\n builtin/log.c | 2 ++\n pretty.c | 2 ++\n revision.h | 1 +\n t/t4014-format-patch.sh | 13 +++++++++++++\n 4 files changed, 18 insertions(+)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 9b937d59b8..83b2d01b49 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1897,6 +1897,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"show changes against in cover letter or single patch\")),\n \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n \t\t\t N_(\"percentage by which creation is weighted\")),\n+\t\tOPT_BOOL(0, \"force-inbody-from\", &rev.force_inbody_from,\n+\t\t\t N_(\"Use in-body From: even for your own commit\")),\n \t\tOPT_END()\n \t};\n \ndiff --git a/pretty.c b/pretty.c\nindex 51e3fa5736..e266208c0b 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -483,6 +483,8 @@ static int use_inbody_from(const struct pretty_print_context *pp, const struct i\n \t\treturn 0;\n \tif (ident_cmp(pp->from_ident, ident))\n \t\treturn 1;\n+\tif (pp->rev && pp->rev->force_inbody_from)\n+\t\treturn 1;\n \treturn 0;\n }\n \ndiff --git a/revision.h b/revision.h\nindex bb91e7ed91..a2d3813a21 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -208,6 +208,7 @@ struct rev_info {\n \n \t/* Format info */\n \tint\t\tshow_notes;\n+\tunsigned int\tforce_inbody_from;\n \tunsigned int\tshown_one:1,\n \t\t\tshown_dashes:1,\n \t\t\tshow_merge:1,\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex fbec8ad2ef..a4ecd433e2 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1400,6 +1400,19 @@ test_expect_success '--from omits redundant in-body header' '\n \ttest_cmp expect patch.head\n '\n \n+test_expect_success 'with --force-inbody-from, --from keeps redundant in-body header' '\n+\tgit format-patch --force-inbody-from \\\n+\t\t-1 --stdout --from=\"A U Thor \" >patch &&\n+\tcat >expect <<-\\EOF &&\n+\tFrom: A U Thor \n+\n+\tFrom: A U Thor \n+\n+\tEOF\n+\tsed -ne \"/^From:/p; /^$/p; /^---$/q\" patch >patch.head &&\n+\ttest_cmp expect patch.head\n+'\n+\n test_expect_success 'in-body headers trigger content encoding' '\n \ttest_env GIT_AUTHOR_NAME=\"éxötìc\" test_commit exotic &&\n \ttest_when_finished \"git reset --hard HEAD^\" &&\n-- \n2.37.2-587-g47adba97a9\n\n"},{"id":"462080","messageId":"08p469q7-2os4-p391-9nr4-10q7o9s4414s@tzk.qr","threadId":"58369","inReplyTo":"20220826213203.3258022-2-gitster@pobox.com","subject":"Re: [PATCH 1/2] pretty: separate out the logic to decide the use of in-body from","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-08-29T11:32:27Z","receivedAt":"2022-08-29T12:14:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 26 Aug 2022, Junio C Hamano wrote:\n\n> When pretty-printing the log message for a given commit in e-mail\n> format (e.g. \"git format-patch\"), we add in-body \"From:\" header when\n> the author identity of the commit is different from the identity of\n> the person who is \"sending\" the mail.\n\nThe quotes around \"sending\" made me stumble over this a bit. Maybe replace\nit by saying \"the person running the command\"?\n\n> Split out the logic into a helper function. Because the \"from_ident\n> must be set\" condition is there not because it is used in the\n> ident_cmp() next, but because the codepath that is entered when this\n> logic says \"Yes, you should use in-body from\" requires values there\n> in from_ident member, so separate it out into an if() statement on\n> its own to clarify it.\n\nEven after reading this three times, I had trouble understanding it. I\nthen consulted the diff and started to grasp what you mean. I have no\ngood idea how to improve the wording, but maybe you can give it another\ngo? Or simply state that the condition was untangled a bit.\n\nThe diff looks good.\n\nCiao,\nDscho\n\nP.S.: I do not know how strongly you feel these days about lines longer\nthan 80 columns, but personally I do not care about this rule, so I am\nmore than just fine with adding such a line here.\n\n> diff --git a/pretty.c b/pretty.c\n> index 6d819103fb..51e3fa5736 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -477,6 +477,15 @@ static void append_line_with_color(struct strbuf *sb, struct grep_opt *opt,\n> \t}\n> }\n>\n> +static int use_inbody_from(const struct pretty_print_context *pp, const struct ident_split *ident)\n> +{\n> +\tif (!pp->from_ident)\n> +\t\treturn 0;\n> +\tif (ident_cmp(pp->from_ident, ident))\n> +\t\treturn 1;\n> +\treturn 0;\n> +}\n> +\n> void pp_user_info(struct pretty_print_context *pp,\n> \t\t const char *what, struct strbuf *sb,\n> \t\t const char *line, const char *encoding)\n> @@ -503,7 +512,7 @@ void pp_user_info(struct pretty_print_context *pp,\n> \t\tmap_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);\n>\n> \tif (cmit_fmt_is_mail(pp->fmt)) {\n> -\t\tif (pp->from_ident && ident_cmp(pp->from_ident, &ident)) {\n> +\t\tif (use_inbody_from(pp, &ident)) {\n> \t\t\tstruct strbuf buf = STRBUF_INIT;\n>\n> \t\t\tstrbuf_addstr(&buf, \"From: \");\n> --\n> 2.37.2-587-g47adba97a9\n>\n>\n"},{"id":"462083","messageId":"q84op991-3s0n-r0q5-32pn-096595o03rs8@tzk.qr","threadId":"58369","inReplyTo":"20220826213203.3258022-3-gitster@pobox.com","subject":"Re: [PATCH 2/2] format-patch: allow forcing the use of in-body From: header","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-08-29T11:48:05Z","receivedAt":"2022-08-29T12:25:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 26 Aug 2022, Junio C Hamano wrote:\n\n> Users may be authoring and committing their commits under the same\n> e-mail address they use to send their patches from, in which case\n> they shouldn't need to use the in-body From: line in their outgoing\n> e-mails. At the receiving end, \"git am\" will use the address on the\n> \"From:\" header of the incoming e-mail and all should be well.\n>\n> Some mailing lists, however, mangle the From: address from what the\n> original sender had; in such an unfortunate situation, the user may\n> want to add the in-body \"From:\" header even for their own patch.\n>\n> \"git format-patch --[no-]force-inbody-from\" was invented for such\n> users.\n>\n> Note. This is an uncooked early draft.\n\nDid you mean to mark the patch as [RFC], then?\n\n> Things to think about include (but not limited to, of course):\n>\n> * Should this rather be --use-inbody-from=yes,no,auto tristate,\n> that defaults to \"auto\", which is the current behaviour i.e.\n> \"when --from is given, add it only when it does not match the\n> payload\". \"yes\" would mean \"always emit the --from address as\n> in-body From:\" and \"no\" would mean ... what? \"Ignore --from\"?\n> Then why is the user giving --from in the first place?\n\nI would offer up the suggestion `--in-body-from={never,always,auto}` for\nconsideration.\n\n> * Should it be \"inbody\" or \"in-body\"?\n\nThe latter.\n\n> * Should it have a corresponding configuration variable?\n\nProbably. The commit message talks about mailing lists requiring different\nbehavior from the default, which is likely to affect all patches generated\nfrom a corresponding local checkout. Having a config variable would lower\nthe cognitive burden of having to remember this process detail.\n\n> * Should this patch be scrapped and the feature should be done\n> inside \"git send-email\" instead?\n\nSince it affects the `--pretty=email` mode, the current patch seems to aim\nfor the correct layer.\n\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 9b937d59b8..83b2d01b49 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -1897,6 +1897,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \t\t\t N_(\"show changes against in cover letter or single patch\")),\n> \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n> \t\t\t N_(\"percentage by which creation is weighted\")),\n> +\t\tOPT_BOOL(0, \"force-inbody-from\", &rev.force_inbody_from,\n> +\t\t\t N_(\"Use in-body From: even for your own commit\")),\n\nPlease start the usage text in lower-case, to keep it consistent with the\nrest of the usage texts.\n\nAlso, I would like to avoid the personal address \"you\" in that text, and\nalso the verb \"use\". Maybe something like this:\n\n\tshow in-body From: even if identical to the header\n\n> \t\tOPT_END()\n> \t};\n>\n> diff --git a/pretty.c b/pretty.c\n> index 51e3fa5736..e266208c0b 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -483,6 +483,8 @@ static int use_inbody_from(const struct pretty_print_context *pp, const struct i\n> \t\treturn 0;\n> \tif (ident_cmp(pp->from_ident, ident))\n> \t\treturn 1;\n> +\tif (pp->rev && pp->rev->force_inbody_from)\n> +\t\treturn 1;\n\nIt would probably make sense to move this before `ident_cmp()`, to avoid\nunneeded calls (\"is the ident the same? no? well, thank you for your\nanswer but I'll insert the header anyway!\").\n\n> \treturn 0;\n> }\n>\n> diff --git a/revision.h b/revision.h\n> index bb91e7ed91..a2d3813a21 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -208,6 +208,7 @@ struct rev_info {\n>\n> \t/* Format info */\n> \tint\t\tshow_notes;\n> +\tunsigned int\tforce_inbody_from;\n\nThe reason why this isn't added to the `:1` bits below is probably the\nanticipation of the tri-state, but if that tri-state never materializes,\nadding it as a bit is still the right thing to do.\n\n> \tunsigned int\tshown_one:1,\n> \t\t\tshown_dashes:1,\n> \t\t\tshow_merge:1,\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index fbec8ad2ef..a4ecd433e2 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -1400,6 +1400,19 @@ test_expect_success '--from omits redundant in-body header' '\n> \ttest_cmp expect patch.head\n> '\n>\n> +test_expect_success 'with --force-inbody-from, --from keeps redundant in-body header' '\n> +\tgit format-patch --force-inbody-from \\\n> +\t\t-1 --stdout --from=\"A U Thor \" >patch &&\n> +\tcat >expect <<-\\EOF &&\n> +\tFrom: A U Thor \n> +\n> +\tFrom: A U Thor \n> +\n> +\tEOF\n> +\tsed -ne \"/^From:/p; /^$/p; /^---$/q\" patch >patch.head &&\n> +\ttest_cmp expect patch.head\n> +'\n\nThe test script starts to look a bit non-DRY with all those repetitions of\n`A U Thor `, but that's hardly the responsibility of\nthis here patch to address.\n\nThank you,\nDscho\n\n> +\n> test_expect_success 'in-body headers trigger content encoding' '\n> \ttest_env GIT_AUTHOR_NAME=\"éxötìc\" test_commit exotic &&\n> \ttest_when_finished \"git reset --hard HEAD^\" &&\n> --\n> 2.37.2-587-g47adba97a9\n>\n>\n"},{"id":"462100","messageId":"xmqq7d2rot48.fsf@gitster.g","threadId":"58369","inReplyTo":"08p469q7-2os4-p391-9nr4-10q7o9s4414s@tzk.qr","subject":"Re: [PATCH 1/2] pretty: separate out the logic to decide the use of in-body from","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-29T17:29:59Z","receivedAt":"2022-08-29T17:30:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi Junio,\n>\n> On Fri, 26 Aug 2022, Junio C Hamano wrote:\n>\n>> When pretty-printing the log message for a given commit in e-mail\n>> format (e.g. \"git format-patch\"), we add in-body \"From:\" header when\n>> the author identity of the commit is different from the identity of\n>> the person who is \"sending\" the mail.\n>\n> The quotes around \"sending\" made me stumble over this a bit. Maybe replace\n> it by saying \"the person running the command\"?\n\nIt reads much better.\n\n>> Split out the logic into a helper function. Because the \"from_ident\n>> must be set\" condition is there not because it is used in the\n>> ident_cmp() next, but because the codepath that is entered when this\n>> logic says \"Yes, you should use in-body from\" requires values there\n>> in from_ident member, so separate it out into an if() statement on\n>> its own to clarify it.\n>\n> Even after reading this three times, I had trouble understanding it. I\n> then consulted the diff and started to grasp what you mean. I have no\n> good idea how to improve the wording, but maybe you can give it another\n> go? Or simply state that the condition was untangled a bit.\n\nYeah, (pp->from_ident != NULL) there serves two purposes. Whatever\nelse is in that if() statement, the body of the statement depends on\nthe pp->from_ident being non-NULL and the control must not enter\nthere otherwise. The other purpose is to guard the other half of\nthe if() statement, which happens to be ident_cmp() that looks at\nthe same pp->from_ident and depends on it being non-NULL.\n\nI think the condition gets much cleaner by untangling it to\n\n - use_inbody_from() function does *not* check pp->from_ident; it\n just assumes it is not NULL \n\n - the caller becomes\n\n\tif (pp->from_ident && use_inbody_from(...)) {\n\t\t... stuff that use pp->from_ident ...\n\t}\n\n> P.S.: I do not know how strongly you feel these days about lines longer\n> than 80 columns, but personally I do not care about this rule, so I am\n> more than just fine with adding such a line here.\n\nI allowed wider line for function decl, by inertia, for\ngreppability, but I should fix that. Thanks for noticing.\n\n"},{"id":"462101","messageId":"xmqqilmbne0l.fsf@gitster.g","threadId":"58369","inReplyTo":"q84op991-3s0n-r0q5-32pn-096595o03rs8@tzk.qr","subject":"Re: [PATCH 2/2] format-patch: allow forcing the use of in-body From: header","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-29T17:41:30Z","receivedAt":"2022-08-29T17:41:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> Note. This is an uncooked early draft.\n>\n> Did you mean to mark the patch as [RFC], then?\n\nIt is mixture between that and WIP.\n\n>> Things to think about include (but not limited to, of course):\n>>\n>> * Should this rather be --use-inbody-from=yes,no,auto tristate,\n>> that defaults to \"auto\", which is the current behaviour i.e.\n>> \"when --from is given, add it only when it does not match the\n>> payload\". \"yes\" would mean \"always emit the --from address as\n>> in-body From:\" and \"no\" would mean ... what? \"Ignore --from\"?\n>> Then why is the user giving --from in the first place?\n>\n> I would offer up the suggestion `--in-body-from={never,always,auto}` for\n> consideration.\n\nThat is essentially the same as the \"Boolean plus auto\" tristate, a\nvery common pattern in our UI. The problem is that false-never-no\ndoes not make much sense in this case, because you do not need it.\nYou can instead refrain from passing --from to achieve the same\neffect.\n\n>> * Should it be \"inbody\" or \"in-body\"?\n>\n> The latter.\n\nOK. This cascades up to 1/2 (there is a new helper function with\nthe phrase in its name).\n\n>> * Should it have a corresponding configuration variable?\n>\n> Probably. The commit message talks about mailing lists requiring different\n> behavior from the default, which is likely to affect all patches generated\n> from a corresponding local checkout. Having a config variable would lower\n> the cognitive burden of having to remember this process detail.\n\nOK.\n\n>> diff --git a/builtin/log.c b/builtin/log.c\n>> index 9b937d59b8..83b2d01b49 100644\n>> --- a/builtin/log.c\n>> +++ b/builtin/log.c\n>> @@ -1897,6 +1897,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>> \t\t\t N_(\"show changes against in cover letter or single patch\")),\n>> \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n>> \t\t\t N_(\"percentage by which creation is weighted\")),\n>> +\t\tOPT_BOOL(0, \"force-inbody-from\", &rev.force_inbody_from,\n>> +\t\t\t N_(\"Use in-body From: even for your own commit\")),\n>\n> Please start the usage text in lower-case, to keep it consistent with the\n> rest of the usage texts.\n\nRight.\n\n> Also, I would like to avoid the personal address \"you\" in that text, and\n> also the verb \"use\". Maybe something like this:\n>\n> \tshow in-body From: even if identical to the header\n\nMuch nicer. I'll take it.\n\n>> diff --git a/pretty.c b/pretty.c\n>> index 51e3fa5736..e266208c0b 100644\n>> --- a/pretty.c\n>> +++ b/pretty.c\n>> @@ -483,6 +483,8 @@ static int use_inbody_from(const struct pretty_print_context *pp, const struct i\n>> \t\treturn 0;\n>> \tif (ident_cmp(pp->from_ident, ident))\n>> \t\treturn 1;\n>> +\tif (pp->rev && pp->rev->force_inbody_from)\n>> +\t\treturn 1;\n>\n> It would probably make sense to move this before `ident_cmp()`, to avoid\n> unneeded calls (\"is the ident the same? no? well, thank you for your\n> answer but I'll insert the header anyway!\").\n\nI tend to prefer adding new things at the end when all things are\nequal, but in this case the new thing is an overriding condition, so\nit does make sense to have it before the call.\n\n>> diff --git a/revision.h b/revision.h\n>> index bb91e7ed91..a2d3813a21 100644\n>> --- a/revision.h\n>> +++ b/revision.h\n>> @@ -208,6 +208,7 @@ struct rev_info {\n>>\n>> \t/* Format info */\n>> \tint\t\tshow_notes;\n>> +\tunsigned int\tforce_inbody_from;\n>\n> The reason why this isn't added to the `:1` bits below is probably the\n> anticipation of the tri-state, but if that tri-state never materializes,\n> adding it as a bit is still the right thing to do.\n\nIt might make sense to turn this into the common \"Boolean plus auto\"\ntristate, but the utility of \"no\" in this case is dubious, so I was\nnot planning to go that route.\n\nThis member is a full fledged word because the address of it given\nto OPT_BOOL(), and we cannot take an address of a bitfield member in\na struct.\n\nBit-pinching in this struct is not very useful. Even if we traverse\na million commits in a single run, we will use a single \"struct\nrev_info\" instance.\n\nThanks for reading it over.\n"},{"id":"462114","messageId":"20220829213837.13849-1-gitster@pobox.com","threadId":"58369","inReplyTo":"20220826213203.3258022-1-gitster@pobox.com","subject":"[PATCH v2 0/3] format-patch --force-in-body-from","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-29T21:38:34Z","receivedAt":"2022-08-29T21:38:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Users may be authoring and committing their commits under the same\ne-mail address they use to send their patches from, in which case\nthey shouldn't need to use the in-body From: line in their outgoing\ne-mails. At the receiving end, \"git am\" will use the address on the\n\"From:\" header of the incoming e-mail and all should be well.\n\nSome mailing lists, however, mangle the From: address from what the\noriginal sender had; in such an unfortunate situation, the user may\nwant to add the in-body \"From:\" header even for their own patch.\n\nA new option `--force-in-body-from` from the command line of `git\nformat-patch`, and a new configuration variable `format.forceInBodyFrom`\nthat can be set per repository, are added to address this.\n\nChanges since the original submission are\n\n * The configuration variable support is new.\n\n * comes with documentation updates.\n\n * \"in-body\" is spelled as such, not \"inbody\".\n\n * the order of config and option parsing plus initializing the\n rev_info structure dictates a separate variable to be used for\n the former two and then copied to rev_info later.\n\nJunio C Hamano (3):\n pretty: separate out the logic to decide the use of in-body from\n format-patch: allow forcing the use of in-body From: header\n format-patch: learn format.forceInBodyFrom configuration variable\n\n Documentation/config/format.txt | 4 ++++\n Documentation/git-format-patch.txt | 11 +++++++++\n builtin/log.c | 9 ++++++++\n pretty.c | 12 +++++++++-\n revision.h | 1 +\n t/t4014-format-patch.sh | 37 ++++++++++++++++++++++++++++++\n 6 files changed, 73 insertions(+), 1 deletion(-)\n\n-- \n2.37.2-621-gd3a800faf0\n"},{"id":"462115","messageId":"20220829213837.13849-2-gitster@pobox.com","threadId":"58369","inReplyTo":"20220829213837.13849-1-gitster@pobox.com","subject":"[PATCH v2 1/3] pretty: separate out the logic to decide the use of in-body from","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-29T21:38:35Z","receivedAt":"2022-08-29T21:38:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When pretty-printing the log message for a given commit in the\ne-mail format (e.g. \"git format-patch\"), we add an in-body \"From:\"\nheader when the author identity of the commit is different from the\nidentity of the person whose identity appears in the header of the\ne-mail (the latter is passed with them \"--from\" option).\n\nSplit out the logic into a helper function, as we would want to\nextend the condition further.\n\nSigned-off-by: Junio C Hamano \n---\n pretty.c | 10 +++++++++-\n 1 file changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 6d819103fb..cf418a6b20 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -477,6 +477,14 @@ static void append_line_with_color(struct strbuf *sb, struct grep_opt *opt,\n \t}\n }\n \n+static int use_in_body_from(const struct pretty_print_context *pp,\n+\t\t\t const struct ident_split *ident)\n+{\n+\tif (ident_cmp(pp->from_ident, ident))\n+\t\treturn 1;\n+\treturn 0;\n+}\n+\n void pp_user_info(struct pretty_print_context *pp,\n \t\t const char *what, struct strbuf *sb,\n \t\t const char *line, const char *encoding)\n@@ -503,7 +511,7 @@ void pp_user_info(struct pretty_print_context *pp,\n \t\tmap_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);\n \n \tif (cmit_fmt_is_mail(pp->fmt)) {\n-\t\tif (pp->from_ident && ident_cmp(pp->from_ident, &ident)) {\n+\t\tif (pp->from_ident && use_in_body_from(pp, &ident)) {\n \t\t\tstruct strbuf buf = STRBUF_INIT;\n \n \t\t\tstrbuf_addstr(&buf, \"From: \");\n-- \n2.37.2-621-gd3a800faf0\n\n"},{"id":"462116","messageId":"20220829213837.13849-3-gitster@pobox.com","threadId":"58369","inReplyTo":"20220829213837.13849-1-gitster@pobox.com","subject":"[PATCH v2 2/3] format-patch: allow forcing the use of in-body From: header","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-29T21:38:36Z","receivedAt":"2022-08-29T21:38:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Users may be authoring and committing their commits under the same\ne-mail address they use to send their patches from, in which case\nthey shouldn't need to use the in-body From: line in their outgoing\ne-mails. At the receiving end, \"git am\" will use the address on the\n\"From:\" header of the incoming e-mail and all should be well.\n\nSome mailing lists, however, mangle the From: address from what the\noriginal sender had; in such a situation, the user may want to add\nthe in-body \"From:\" header even for their own patches.\n\n\"git format-patch --[no-]force-in-body-from\" was invented for such\nusers.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-format-patch.txt | 9 +++++++++\n builtin/log.c | 5 +++++\n pretty.c | 2 ++\n revision.h | 1 +\n t/t4014-format-patch.sh | 13 +++++++++++++\n 5 files changed, 30 insertions(+)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex be797d7a28..7c7f244e57 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -275,6 +275,15 @@ header). Note also that `git send-email` already handles this\n transformation for you, and this option should not be used if you are\n feeding the result to `git send-email`.\n \n+--[no-]force-in-body-from::\n+\tWith the e-mail sender specified via the `--from` option, by\n+\tdefault, an in-body \"From:\" to identify the real author of\n+\tthe commit is added at the top of the commit log message if\n+\tthe sender is different from the author. With this option,\n+\tthe in-body \"From:\" is added even when the sender and the\n+\tauthor have the same name and address, which may help if the\n+\tmailing list software mangles the sender's identity.\n+\n --add-header=
::\n \tAdd an arbitrary header to the email headers. This is in addition\n \tto any configured headers, and may be used multiple times.\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 9b937d59b8..78ccd37bd9 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -52,6 +52,7 @@ static int default_encode_email_headers = 1;\n static int decoration_style;\n static int decoration_given;\n static int use_mailmap_config = 1;\n+static unsigned int force_in_body_from;\n static const char *fmt_patch_subject_prefix = \"PATCH\";\n static int fmt_patch_name_max = FORMAT_PATCH_NAME_MAX_DEFAULT;\n static const char *fmt_pretty;\n@@ -1897,6 +1898,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"show changes against in cover letter or single patch\")),\n \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n \t\t\t N_(\"percentage by which creation is weighted\")),\n+\t\tOPT_BOOL(0, \"force-in-body-from\", &force_in_body_from,\n+\t\t\t N_(\"show in-body From: even if identical to the e-mail header\")),\n \t\tOPT_END()\n \t};\n \n@@ -1940,6 +1943,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |\n \t\t\t PARSE_OPT_KEEP_DASHDASH);\n \n+\trev.force_in_body_from = force_in_body_from;\n+\n \t/* Make sure \"0000-$sub.patch\" gives non-negative length for $sub */\n \tif (fmt_patch_name_max <= strlen(\"0000-\") + strlen(fmt_patch_suffix))\n \t\tfmt_patch_name_max = strlen(\"0000-\") + strlen(fmt_patch_suffix);\ndiff --git a/pretty.c b/pretty.c\nindex cf418a6b20..b7553e3fe0 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -480,6 +480,8 @@ static void append_line_with_color(struct strbuf *sb, struct grep_opt *opt,\n static int use_in_body_from(const struct pretty_print_context *pp,\n \t\t\t const struct ident_split *ident)\n {\n+\tif (pp->rev && pp->rev->force_in_body_from)\n+\t\treturn 1;\n \tif (ident_cmp(pp->from_ident, ident))\n \t\treturn 1;\n \treturn 0;\ndiff --git a/revision.h b/revision.h\nindex bb91e7ed91..6e346a60ab 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -221,6 +221,7 @@ struct rev_info {\n \t\t\tmissing_newline:1,\n \t\t\tdate_mode_explicit:1,\n \t\t\tpreserve_subject:1,\n+\t\t\tforce_in_body_from:1,\n \t\t\tencode_email_headers:1,\n \t\t\tinclude_header:1;\n \tunsigned int\tdisable_stdin:1;\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex fbec8ad2ef..347f7f7f35 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1400,6 +1400,19 @@ test_expect_success '--from omits redundant in-body header' '\n \ttest_cmp expect patch.head\n '\n \n+test_expect_success 'with --force-in-body-from, redundant in-body from is kept' '\n+\tgit format-patch --force-in-body-from \\\n+\t\t-1 --stdout --from=\"A U Thor \" >patch &&\n+\tcat >expect <<-\\EOF &&\n+\tFrom: A U Thor \n+\n+\tFrom: A U Thor \n+\n+\tEOF\n+\tsed -ne \"/^From:/p; /^$/p; /^---$/q\" patch >patch.head &&\n+\ttest_cmp expect patch.head\n+'\n+\n test_expect_success 'in-body headers trigger content encoding' '\n \ttest_env GIT_AUTHOR_NAME=\"éxötìc\" test_commit exotic &&\n \ttest_when_finished \"git reset --hard HEAD^\" &&\n-- \n2.37.2-621-gd3a800faf0\n\n"},{"id":"462117","messageId":"20220829213837.13849-4-gitster@pobox.com","threadId":"58369","inReplyTo":"20220829213837.13849-1-gitster@pobox.com","subject":"[PATCH v2 3/3] format-patch: learn format.forceInBodyFrom configuration variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-29T21:38:37Z","receivedAt":"2022-08-29T21:38:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"As the need to use the \"--force-in-body-from\" option primarily is\ntied to which mailing list the mails go to (and get their From:\naddress mangled), it is likely that a user who needs to use this\noption once to interact with their upstream project needs to use it\nfor all patches they send out.\n\nAdd a configuration variable, suitable for setting in the local\nconfiguration file per repository, for this.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/config/format.txt | 4 ++++\n Documentation/git-format-patch.txt | 2 ++\n builtin/log.c | 4 ++++\n t/t4014-format-patch.sh | 24 ++++++++++++++++++++++++\n 4 files changed, 34 insertions(+)\n\ndiff --git a/Documentation/config/format.txt b/Documentation/config/format.txt\nindex fdbc06a4d2..c7303d8d9f 100644\n--- a/Documentation/config/format.txt\n+++ b/Documentation/config/format.txt\n@@ -15,6 +15,10 @@ format.from::\n \tdifferent. If set to a non-boolean value, format-patch uses that\n \tvalue instead of your committer identity. Defaults to false.\n \n+format.forceInBodyFrom::\n+\tProvides the default value for the `--[no-]force-in-body-from`\n+\toption to format-patch. Defaults to false.\n+\n format.numbered::\n \tA boolean which can enable or disable sequence numbers in patch\n \tsubjects. It defaults to \"auto\" which enables it only if there\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 7c7f244e57..dfcc7da4c2 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -283,6 +283,8 @@ feeding the result to `git send-email`.\n \tthe in-body \"From:\" is added even when the sender and the\n \tauthor have the same name and address, which may help if the\n \tmailing list software mangles the sender's identity.\n+\tDefaults to the value of the `format.forceInBodyFrom`\n+\tconfiguration variable.\n \n --add-header=
::\n \tAdd an arbitrary header to the email headers. This is in addition\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 78ccd37bd9..776bc9afdb 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1007,6 +1007,10 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \t\t\tfrom = NULL;\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"format.forceinbodyfrom\")) {\n+\t\tforce_in_body_from = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(var, \"format.notes\")) {\n \t\tint b = git_parse_maybe_bool(value);\n \t\tif (b < 0)\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 347f7f7f35..ad5c029279 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1413,6 +1413,30 @@ test_expect_success 'with --force-in-body-from, redundant in-body from is kept'\n \ttest_cmp expect patch.head\n '\n \n+test_expect_success 'format.forceInBodyFrom, equivalent to --force-in-body-from' '\n+\tgit -c format.forceInBodyFrom=yes format-patch \\\n+\t\t-1 --stdout --from=\"A U Thor \" >patch &&\n+\tcat >expect <<-\\EOF &&\n+\tFrom: A U Thor \n+\n+\tFrom: A U Thor \n+\n+\tEOF\n+\tsed -ne \"/^From:/p; /^$/p; /^---$/q\" patch >patch.head &&\n+\ttest_cmp expect patch.head\n+'\n+\n+test_expect_success 'format.forceInBodyFrom, equivalent to --force-in-body-from' '\n+\tgit -c format.forceInBodyFrom=yes format-patch --no-force-in-body-from \\\n+\t\t-1 --stdout --from=\"A U Thor \" >patch &&\n+\tcat >expect <<-\\EOF &&\n+\tFrom: A U Thor \n+\n+\tEOF\n+\tsed -ne \"/^From:/p; /^$/p; /^---$/q\" patch >patch.head &&\n+\ttest_cmp expect patch.head\n+'\n+\n test_expect_success 'in-body headers trigger content encoding' '\n \ttest_env GIT_AUTHOR_NAME=\"éxötìc\" test_commit exotic &&\n \ttest_when_finished \"git reset --hard HEAD^\" &&\n-- \n2.37.2-621-gd3a800faf0\n\n"},{"id":"462236","messageId":"Yw5t9mfaIi647Qbl@coredump.intra.peff.net","threadId":"58369","inReplyTo":"20220829213837.13849-3-gitster@pobox.com","subject":"Re: [PATCH v2 2/3] format-patch: allow forcing the use of in-body From: header","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-08-30T20:07:18Z","receivedAt":"2022-08-30T20:07:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 29, 2022 at 02:38:36PM -0700, Junio C Hamano wrote:\n\n> +--[no-]force-in-body-from::\n> +\tWith the e-mail sender specified via the `--from` option, by\n> +\tdefault, an in-body \"From:\" to identify the real author of\n> +\tthe commit is added at the top of the commit log message if\n> +\tthe sender is different from the author. With this option,\n> +\tthe in-body \"From:\" is added even when the sender and the\n> +\tauthor have the same name and address, which may help if the\n> +\tmailing list software mangles the sender's identity.\n\nI find it a little curious that this option can only be used with\n\"--from\". That makes sense in a way, because this is a special case of\nthat feature, overriding the \"are they the same\" check.\n\nBut given that the use case is not to send somebody else's patch, but to\nduplicate your _own_ ident in both spots, it feels funny that you must\nalso say \"by the way, I am the sender of the email\". I.e., you have to\nsay:\n\n git format-patch --from='Me ' --force-in-body-from\n\nI guess it is not too bad because just \"--from\" will do the equivalent\nthing (picking \"me\" from your committer ident). It just feels kind of\nclunky that:\n\n git format-patch --force-in-body-from\n\nwill silently ignore the option.\n\nAll that said, I don't care _too_ strongly about it. I do suspect the\nfeature might be better placed in send-email (or possibly in addition).\nIf you are using send-email, then I think you're not supposed to use\n\"--from\" with format-patch at all, and you have no way of accessing this\nfeature.\n\n-Peff\n"},{"id":"462237","messageId":"Yw5vlTCJQCovVbAl@coredump.intra.peff.net","threadId":"58369","inReplyTo":"Yw5t9mfaIi647Qbl@coredump.intra.peff.net","subject":"Re: [PATCH v2 2/3] format-patch: allow forcing the use of in-body From: header","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-08-30T20:14:13Z","receivedAt":"2022-08-30T20:14:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 30, 2022 at 04:07:18PM -0400, Jeff King wrote:\n\n> All that said, I don't care _too_ strongly about it. I do suspect the\n> feature might be better placed in send-email (or possibly in addition).\n> If you are using send-email, then I think you're not supposed to use\n> \"--from\" with format-patch at all, and you have no way of accessing this\n> feature.\n\nThis \"not supposed to\" came from me looking at the documentation. But\nthis discussion implies that it's maybe not that big a deal:\n\n https://lore.kernel.org/git/xmqq8twlqwan.fsf@gitster.mtv.corp.google.com/\n\nApparently we were considering enabling \"--from\" by default, but I don't\nthink that ever happened.\n\n-Peff\n"}]}