Re: [PATCH 2/2] format-patch: allow forcing the use of in-body From: header
From: Johannes Schindelin <hidden>
Date: 2022-08-29 12:25:45
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.
quoted hunk ↗ jump to hunk
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
quoted hunk ↗ jump to hunk
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!").
quoted hunk ↗ jump to hunk
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.
quoted hunk ↗ jump to hunk
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 [off-list ref]`, 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