{"thread":{"id":"56361","subject":"[RFC PATCH] send-email: allow fixing the cover letter subject","startedAt":"2021-08-24T11:42:00Z","lastAt":"2021-08-26T19:31:59Z","messageCount":6,"participants":["Carlo Marcelo Arenas Belón","Marvin Häuser","Carlo Arenas"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"433541","messageId":"20210824114135.54810-1-carenas@gmail.com","threadId":"56361","inReplyTo":null,"subject":"[RFC PATCH] send-email: allow fixing the cover letter subject","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-08-24T11:41:35Z","receivedAt":"2021-08-24T11:42:00Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"a03bc5b6ad (send-email: Refuse to send cover-letter template\nsubject, 2009-06-08) fixes a common problem with the direct use\nof `git send-email` to send topics, where the cover letter will\nbe a template by accident.\n\nAllow using --subject to fix the --cover-letter subject and skip\nthe safety check without having to use --force.\n\nNote that the use of --compose is also allowed, and will also\nget its Subject set through the commandline, but it might make\nmore sense to instead make them incompatible and force editing\nthe cover letter instead, hence why sending this only as a RFC.\n\npatch is based on another one[1] still in fly, and the documentation\nchange is long overdue, but probably should need a better\nexplanation; lastly using a non ASCII subject also needs fixing,\nbut it also affects compose, so was left out for now.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n\n[1] https://lore.kernel.org/git/4db7759c-2123-533b-9f89-954c07f5832a@posteo.de/\n---\n Documentation/git-send-email.txt |  3 +--\n git-send-email.perl              |  8 ++++++--\n t/t9001-send-email.sh            | 16 +++++++++++++++-\n 3 files changed, 22 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 3db4eab4ba..9d2bfa6095 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -113,8 +113,7 @@ is not set, this will be prompted for.\n \n --subject=<string>::\n \tSpecify the initial subject of the email thread.\n-\tOnly necessary if --compose is also set.  If --compose\n-\tis not set, this will be prompted for.\n+\tOnly necessary if --compose or --cover-letter are also set.\n \n --to=<address>,...::\n \tSpecify the primary recipient of the emails generated. Generally, this\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex fd79849530..a33328a5d9 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -997,7 +997,7 @@ sub file_declares_8bit_cte {\n \t\t\t\t  default => \"UTF-8\");\n }\n \n-if (!$force) {\n+if (!$force and not defined $initial_subject) {\n \tfor my $f (@files) {\n \t\tif (get_patch_subject($f) =~ /\\Q*** SUBJECT HERE ***\\E/) {\n \t\t\tdie sprintf(__(\"Refusing to send because the patch\\n\\t%s\\n\"\n@@ -1719,7 +1719,6 @@ sub process_file {\n \t@xh = ();\n \tmy $input_format = undef;\n \tmy @header = ();\n-\t$subject = $initial_subject;\n \t$message = \"\";\n \t$message_num++;\n \t# First unfold multiline header fields\n@@ -1747,6 +1746,11 @@ sub process_file {\n \t\tif (defined $input_format && $input_format eq 'mbox') {\n \t\t\tif (/^Subject:\\s+(.*)$/i) {\n \t\t\t\t$subject = $1;\n+\n+\t\t\t\tif (defined $initial_subject &&\n+\t\t\t\t$subject =~ /\\Q*** SUBJECT HERE ***\\E/) {\n+\t\t\t\t\t$subject = $initial_subject;\n+\t\t\t\t}\n \t\t\t}\n \t\t\telsif (/^From:\\s+(.*)$/i) {\n \t\t\t\t($author, $author_encoding) = unquote_rfc2047($1);\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 747872d5be..043532fe85 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1728,7 +1728,6 @@ test_expect_success $PREREQ 'CRLF and sendemail.transferencoding=base64' '\n \ttest_cmp expected actual\n '\n \n-\n # Note that the patches in this test are deliberately out of order; we\n # want to make sure it works even if the cover-letter is not in the\n # first mail.\n@@ -1765,6 +1764,21 @@ test_expect_success $PREREQ '--force sends cover letter template anyway' '\n \ttest -n \"$(ls msgtxt*)\"\n '\n \n+test_expect_success $PREREQ '--subject can be used to fix the cover letter' '\n+\tclean_fake_sendmail &&\n+\tgit send-email \\\n+\t\t--cover-letter -2 \\\n+\t\t--subject=\"foo\" \\\n+\t\t--from=\"Example <nobody@example.com>\" \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t2>errors >out &&\n+\t! grep \"SUBJECT HERE\" errors &&\n+\tls msgtxt* >list &&\n+\ttest_line_count = 3 list &&\n+\tgrep \"Subject: foo\" msgtxt1\n+'\n+\n test_cover_addresses () {\n \theader=\"$1\"\n \tshift\n-- \n2.33.0.481.g26d3bed244\n\n"},{"id":"433560","messageId":"5a38e420-eea3-a5f2-38c5-64682bd1ba2e@posteo.de","threadId":"56361","inReplyTo":"20210824114135.54810-1-carenas@gmail.com","subject":"Re: [RFC PATCH] send-email: allow fixing the cover letter subject","fromName":"Marvin Häuser","fromEmail":"mhaeuser@posteo.de","sentAt":"2021-08-24T15:19:23Z","receivedAt":"2021-08-24T15:19:27Z","isPatch":true,"sender":{"key":"mhaeuser@posteo.de","avatar":"https://avatars.githubusercontent.com/u/8659494?v=4"},"body":"On 24/08/2021 13:41, Carlo Marcelo Arenas Belón wrote:\n> @@ -1719,7 +1719,6 @@ sub process_file {\n>   \t@xh = ();\n>   \tmy $input_format = undef;\n>   \tmy @header = ();\n> -\t$subject = $initial_subject;\n\nThis change from my patch is still controversial, and I'd rather not \nsubmit the addition if it's immediately removed again.\nWhy are you dropping it here anyway, does it break any related \nfunctionality?\n\nBest regards,\nMarvin\n"},{"id":"433566","messageId":"CAPUEspgmKPhbPGs+ujy7KVJCyNxS95ph2Dwvd7A7cH2J0em20g@mail.gmail.com","threadId":"56361","inReplyTo":"5a38e420-eea3-a5f2-38c5-64682bd1ba2e@posteo.de","subject":"Re: [RFC PATCH] send-email: allow fixing the cover letter subject","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2021-08-24T16:11:16Z","receivedAt":"2021-08-24T16:11:49Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Tue, Aug 24, 2021 at 8:19 AM Marvin Häuser <mhaeuser@posteo.de> wrote:\n> On 24/08/2021 13:41, Carlo Marcelo Arenas Belón wrote:\n> > @@ -1719,7 +1719,6 @@ sub process_file {\n> >       @xh = ();\n> >       my $input_format = undef;\n> >       my @header = ();\n> > -     $subject = $initial_subject;\n>\n> This change from my patch is still controversial, and I'd rather not\n> submit the addition if it's immediately removed again.\n> Why are you dropping it here anyway, does it break any related\n> functionality?\n\nI was expecting it to break --compose, but it didn't because in there\nwe were using $initial_subject directly instead.\n\nAFAIK; I couldn't see a difference eitherway, but as Peff pointed out\nit doesn't make sense leaving it only as a global so just found\ninstead a more useful place for it to be reset in the loop, which also\n\"fix\" another issue of mine.\n\nAs pointed out in my original email; it doesn't fix the whole problem\nthough, because you still want to edit the rest of the cover letter\n(could use --annotate for that though), but at least it is less\nawkward than the current situation, where you will get the series\nsubmission aborted because of an incorrect subject, even if a correct\none was provided and ended up being ignored.\n\nCarlo\n\nCarlo\n"},{"id":"433713","messageId":"e781b15c-8c59-9827-b052-ebe971b01cad@posteo.de","threadId":"56361","inReplyTo":"CAPUEspgmKPhbPGs+ujy7KVJCyNxS95ph2Dwvd7A7cH2J0em20g@mail.gmail.com","subject":"Re: [RFC PATCH] send-email: allow fixing the cover letter subject","fromName":"Marvin Häuser","fromEmail":"mhaeuser@posteo.de","sentAt":"2021-08-25T06:31:18Z","receivedAt":"2021-08-25T06:31:23Z","isPatch":true,"sender":{"key":"mhaeuser@posteo.de","avatar":"https://avatars.githubusercontent.com/u/8659494?v=4"},"body":"On 24/08/2021 18:11, Carlo Arenas wrote:\n> On Tue, Aug 24, 2021 at 8:19 AM Marvin Häuser <mhaeuser@posteo.de> wrote:\n>> On 24/08/2021 13:41, Carlo Marcelo Arenas Belón wrote:\n>>> @@ -1719,7 +1719,6 @@ sub process_file {\n>>>        @xh = ();\n>>>        my $input_format = undef;\n>>>        my @header = ();\n>>> -     $subject = $initial_subject;\n>> This change from my patch is still controversial, and I'd rather not\n>> submit the addition if it's immediately removed again.\n>> Why are you dropping it here anyway, does it break any related\n>> functionality?\n> I was expecting it to break --compose, but it didn't because in there\n> we were using $initial_subject directly instead.\n\nRight, ok.\n\n> AFAIK; I couldn't see a difference eitherway, but as Peff pointed out\n> it doesn't make sense leaving it only as a global so just found\n> instead a more useful place for it to be reset in the loop, which also\n> \"fix\" another issue of mine.\n\nWell, this only \"sometimes\" resets it (from a control flow perspective), \nwhich is exactly what I tried to avoid globally.\nCan we somehow agree on something? Because I'd like to submit my patch asap.\n\nBest regards,\nMarvin\n"},{"id":"433820","messageId":"CAPUEspga0yGn-u-7poRQZomJPAH43AHyegd=chR_z+3gxRxVdw@mail.gmail.com","threadId":"56361","inReplyTo":"e781b15c-8c59-9827-b052-ebe971b01cad@posteo.de","subject":"Re: [RFC PATCH] send-email: allow fixing the cover letter subject","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2021-08-26T10:58:14Z","receivedAt":"2021-08-26T10:58:28Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Tue, Aug 24, 2021 at 11:31 PM Marvin Häuser <mhaeuser@posteo.de> wrote:\n> On 24/08/2021 18:11, Carlo Arenas wrote:\n> > On Tue, Aug 24, 2021 at 8:19 AM Marvin Häuser <mhaeuser@posteo.de> wrote:\n> >> On 24/08/2021 13:41, Carlo Marcelo Arenas Belón wrote:\n> >>> @@ -1719,7 +1719,6 @@ sub process_file {\n> >>>        @xh = ();\n> >>>        my $input_format = undef;\n> >>>        my @header = ();\n> >>> -     $subject = $initial_subject;\n> Can we somehow agree on something? Because I'd like to submit my patch asap.\n\nI think you are correct, and from a flow point of view it should be\nreset there with the rest.\nAdding something to the commit message to explain it might help, and\nthe documentation path\nI suggested (or something better) might be worth squashing to better\nexplain why the last test case that resets in-reply-to headers is\ncorrect.\n\nCarlo\n"},{"id":"433864","messageId":"a048cc56-2bf4-ee7b-5478-d4bd80dd5380@posteo.de","threadId":"56361","inReplyTo":"CAPUEspga0yGn-u-7poRQZomJPAH43AHyegd=chR_z+3gxRxVdw@mail.gmail.com","subject":"Re: [RFC PATCH] send-email: allow fixing the cover letter subject","fromName":"Marvin Häuser","fromEmail":"mhaeuser@posteo.de","sentAt":"2021-08-26T19:31:54Z","receivedAt":"2021-08-26T19:31:59Z","isPatch":true,"sender":{"key":"mhaeuser@posteo.de","avatar":"https://avatars.githubusercontent.com/u/8659494?v=4"},"body":"On 26/08/2021 12:58, Carlo Arenas wrote:\n> I think you are correct, and from a flow point of view it should be\n> reset there with the rest.\n> Adding something to the commit message to explain it might help, and\n> the documentation path\n> I suggested (or something better) might be worth squashing to better\n> explain why the last test case that resets in-reply-to headers is\n> correct.\n\nSquashing, you mean from your patch into mine? If that is what you mean, \ncan you maybe do that and add SOB Peff for the other tests? It could \njust be one patch set really. If that is not what you mean, I will \nsubmit our (Peff's and my changes) patch as-is tomorrow.\n\nBest regards,\nMarvin\n\n>\n> Carlo\n\n"}]}