{"thread":{"id":"19752","subject":"[PATCH] send-email: Refuse to send cover-letter template subject","startedAt":"2009-06-08T21:34:12Z","lastAt":"2009-06-12T16:06:51Z","messageCount":8,"participants":["Thomas Rast","Junio C Hamano","Markus Heidelberg"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"115845","messageId":"7fedc4b76fed03e4db6a2bb7453609c9cd02928a.1244496564.git.trast@student.ethz.ch","threadId":"19752","inReplyTo":null,"subject":"[PATCH] send-email: Refuse to send cover-letter template subject","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-06-08T21:34:12Z","receivedAt":"2009-06-08T21:34:12Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Every so often, someone sends out an unedited cover-letter template.\nAdd a simple check to send-email that refuses to send if the subject\ncontains \"*** SUBJECT HERE ***\", with an option --force to override.\n\nSigned-off-by: Thomas Rast \n---\n git-send-email.perl | 15 +++++++++++++++\n t/t9001-send-email.sh | 36 ++++++++++++++++++++++++++++++++++++\n 2 files changed, 51 insertions(+), 0 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 3d6a982..293ee46 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -82,6 +82,7 @@\n --[no-]validate * Perform patch sanity checks. Default on.\n --[no-]format-patch * understand any non optional arguments as\n `git format-patch` ones.\n+ --force * Send even if safety checks would prevent it.\n \n EOT\n \texit(1);\n@@ -159,6 +160,7 @@\n my ($quiet, $dry_run) = (0, 0);\n my $format_patch;\n my $compose_filename;\n+my $force = 0;\n \n # Handle interactive edition of files.\n my $multiedit;\n@@ -268,6 +270,7 @@\n \t\t \"thread!\" => \\$thread,\n \t\t \"validate!\" => \\$validate,\n \t\t \"format-patch!\" => \\$format_patch,\n+\t\t \"force\" => \\$force,\n \t );\n \n unless ($rc) {\n@@ -638,6 +641,18 @@ sub get_patch_subject($) {\n \treturn undef;\n }\n \n+\n+if (!$force) {\n+\tfor my $f (@files) {\n+\t\tif (get_patch_subject($f) =~ /\\*\\*\\* SUBJECT HERE \\*\\*\\*/) {\n+\t\t\tdie \"Refusing to send because the patch\\n\\t$f\\n\"\n+\t\t\t\t. \"has the template subject '*** SUBJECT HERE ***'. \"\n+\t\t\t\t. \"Pass --force if you really want to send.\\n\";\n+\t\t}\n+\t}\n+\n+}\n+\n my $prompting = 0;\n if (!defined $sender) {\n \t$sender = $repoauthor || $repocommitter || '';\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex ce26ea4..af5e73e 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -621,4 +621,40 @@ test_expect_success 'in-reply-to but no threading' '\n \tgrep \"In-Reply-To: \"\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+test_expect_success 'refusing to send cover letter template' '\n+\tclean_fake_sendmail &&\n+\trm -fr outdir &&\n+\tgit-format-patch --cover-letter -2 -o outdir &&\n+\ttest_must_fail git send-email \\\n+\t --from=\"Example \" \\\n+\t --to=nobody@example.com \\\n+\t --smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t outdir/0002-*.patch \\\n+\t outdir/0000-*.patch \\\n+\t outdir/0001-*.patch \\\n+ 2>errors >out &&\n+\tgrep \"SUBJECT HERE\" errors &&\n+\ttest -z \"$(ls msgtxt*)\"\n+'\n+\n+test_expect_success '--force sends cover letter template anyway' '\n+\tclean_fake_sendmail &&\n+\trm -fr outdir &&\n+\tgit-format-patch --cover-letter -2 -o outdir &&\n+\tgit send-email \\\n+\t --force \\\n+\t --from=\"Example \" \\\n+\t --to=nobody@example.com \\\n+\t --smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t outdir/0002-*.patch \\\n+\t outdir/0000-*.patch \\\n+\t outdir/0001-*.patch \\\n+ 2>errors >out &&\n+\t! grep \"SUBJECT HERE\" errors &&\n+\ttest -n \"$(ls msgtxt*)\"\n+'\n+\n test_done\n-- \n1.6.3.2.335.gc04d\n"},{"id":"115866","messageId":"7v4ouqwa42.fsf@alter.siamese.dyndns.org","threadId":"19752","inReplyTo":"7fedc4b76fed03e4db6a2bb7453609c9cd02928a.1244496564.git.trast@student.ethz.ch","subject":"Re: [PATCH] send-email: Refuse to send cover-letter template subject","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-08T23:59:09Z","receivedAt":"2009-06-08T23:59:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> Every so often, someone sends out an unedited cover-letter template.\n> Add a simple check to send-email that refuses to send if the subject\n> contains \"*** SUBJECT HERE ***\", with an option --force to override.\n\nGood ;-). More valuable to detect would be an empty \"blurb\" section\n(i.e. not \"unedited *** BLURB HERE ***\" string, but literally, there is\nnothing said in the message other than the shortstat).\n\nOne twist is that I am reasonably sure that somebody will soon fix the\nshortstat part of the cover letter with something more reasonable\n(i.e. not categorized by author, but instead retaining the order of\npatches, and for an added bonus, append names of authors other than\nyourself at the end of the line in parentheses, or something).\n"},{"id":"115885","messageId":"200906091019.11000.trast@student.ethz.ch","threadId":"19752","inReplyTo":"7v4ouqwa42.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] send-email: Refuse to send cover-letter template subject","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-06-09T08:19:05Z","receivedAt":"2009-06-09T08:19:05Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Junio C Hamano wrote:\n> Thomas Rast writes:\n> \n> > Every so often, someone sends out an unedited cover-letter template.\n> > Add a simple check to send-email that refuses to send if the subject\n> > contains \"*** SUBJECT HERE ***\", with an option --force to override.\n> \n> Good ;-). More valuable to detect would be an empty \"blurb\" section\n> (i.e. not \"unedited *** BLURB HERE ***\" string, but literally, there is\n> nothing said in the message other than the shortstat).\n\nWouldn't that be AI complete? As in, whatever regex (or similar) you\nmight cook up could probably be matched by *someone's* style of patch\ndescriptions, and then it suddenly becomes rather hard to tell them\napart.\n\nI was really only trying to prevent the case where the user formatted\nseveral times in a row because he noticed mistakes in the patches, and\nforgot to re-edit the cover letter. At least that's how I once sent\nout one with a real description but the template subject.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"115930","messageId":"200906092025.18643.markus.heidelberg@web.de","threadId":"19752","inReplyTo":"7v4ouqwa42.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] send-email: Refuse to send cover-letter template subject","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-06-09T18:25:18Z","receivedAt":"2009-06-09T18:25:18Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Junio C Hamano, 09.06.2009:\n> Thomas Rast writes:\n> \n> > Every so often, someone sends out an unedited cover-letter template.\n> > Add a simple check to send-email that refuses to send if the subject\n> > contains \"*** SUBJECT HERE ***\", with an option --force to override.\n> \n> Good ;-). More valuable to detect would be an empty \"blurb\" section\n> (i.e. not \"unedited *** BLURB HERE ***\" string, but literally, there is\n> nothing said in the message other than the shortstat).\n\nMy mail \"[PATCH 0/6] fixes for git-send-email\" had an empty blurb\nsection because it didn't need any additional description.\nI wouldn't want git to refuse sending it.\n"},{"id":"115939","messageId":"7v4oupmdv5.fsf@alter.siamese.dyndns.org","threadId":"19752","inReplyTo":"200906092025.18643.markus.heidelberg@web.de","subject":"Re: [PATCH] send-email: Refuse to send cover-letter template subject","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-09T19:00:30Z","receivedAt":"2009-06-09T19:00:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Markus Heidelberg writes:\n\n>> Good ;-). More valuable to detect would be an empty \"blurb\" section\n>> (i.e. not \"unedited *** BLURB HERE ***\" string, but literally, there is\n>> nothing said in the message other than the shortstat).\n>\n> My mail \"[PATCH 0/6] fixes for git-send-email\" had an empty blurb\n> section because it didn't need any additional description.\n> I wouldn't want git to refuse sending it.\n\nI actually wouldn't have wanted to see that [0/6] with empty description.\n\nIf the list of 6 patch titles give clear enough description of the theme\nof the whole series, you would not need [0/6] to set the stage, and that\nis the best kind of series. For your send-email patches, you certainly\nchose very good titles for them.\n"},{"id":"115942","messageId":"200906092132.10256.markus.heidelberg@web.de","threadId":"19752","inReplyTo":"7v4oupmdv5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] send-email: Refuse to send cover-letter template subject","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-06-09T19:32:09Z","receivedAt":"2009-06-09T19:32:09Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Junio C Hamano, 09.06.2009:\n> Markus Heidelberg writes:\n> >\n> > My mail \"[PATCH 0/6] fixes for git-send-email\" had an empty blurb\n> > section because it didn't need any additional description.\n> > I wouldn't want git to refuse sending it.\n> \n> I actually wouldn't have wanted to see that [0/6] with empty description.\n\nI thought the diffstat is a nice overview as well. So there is at least\nsome value in a blurb-less cover letter. But I guess they are debatable.\n\n> If the list of 6 patch titles give clear enough description of the theme\n> of the whole series, you would not need [0/6] to set the stage,\n\nI prefer --thread --no-chain-reply-to for more than 2 patches, so it\nseems I sometimes only abuse the 0/X as a common anchor for 1..X/X. I'd\nfind it strange to use 1/X for that.\n\n> and that is the best kind of series.\n\nHmm, so then you prefer no 0/X at all.\n"},{"id":"116141","messageId":"200906121348.19103.trast@student.ethz.ch","threadId":"19752","inReplyTo":"200906091019.11000.trast@student.ethz.ch","subject":"Re: [PATCH] send-email: Refuse to send cover-letter template subject","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-06-12T11:48:16Z","receivedAt":"2009-06-12T11:48:16Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Thomas Rast wrote:\n> Junio C Hamano wrote:\n> > Thomas Rast writes:\n> > \n> > > Every so often, someone sends out an unedited cover-letter template.\n> > > Add a simple check to send-email that refuses to send if the subject\n> > > contains \"*** SUBJECT HERE ***\", with an option --force to override.\n> > \n> > Good ;-). More valuable to detect would be an empty \"blurb\" section\n> > (i.e. not \"unedited *** BLURB HERE ***\" string, but literally, there is\n> > nothing said in the message other than the shortstat).\n> \n> Wouldn't that be AI complete? As in, whatever regex (or similar) you\n> might cook up could probably be matched by *someone's* style of patch\n> descriptions, and then it suddenly becomes rather hard to tell them\n> apart.\n\nFor a contrived example, compare\n\n-- 8< --\nThomas Rast (1):\n Documentation: teach stash/pop workflow instead of stash/apply\n-- >8 --\n\nand\n\n-- 8< --\nPatch (1):\n Some explanation of what patch number 1 does\n-- >8 --\n\nExcept for the \" (1):\", the whole contents of the shortstat are\ngiven by the patches. So I don't think it's possible to tell them\napart without figuring out what the shortlog would have said for the\nactual patches, and comparing that to the contents. I'd rather not\nparse the patches for author info and subject.\n\nWhat do you want me to do about the patch? Should I extend it so that\nit traps both \"*** BLURB HERE ***\" and \"*** SUBJECT HERE ***\"?\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"116166","messageId":"7vr5xp8mhw.fsf@alter.siamese.dyndns.org","threadId":"19752","inReplyTo":"200906121348.19103.trast@student.ethz.ch","subject":"Re: [PATCH] send-email: Refuse to send cover-letter template subject","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-12T16:06:51Z","receivedAt":"2009-06-12T16:06:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> Thomas Rast wrote:\n>> Junio C Hamano wrote:\n>> > Thomas Rast writes:\n>> > \n>> > > Every so often, someone sends out an unedited cover-letter template.\n>> > > Add a simple check to send-email that refuses to send if the subject\n>> > > contains \"*** SUBJECT HERE ***\", with an option --force to override.\n>> > \n>> > Good ;-). More valuable to detect would be an empty \"blurb\" section\n>> > (i.e. not \"unedited *** BLURB HERE ***\" string, but literally, there is\n>> > nothing said in the message other than the shortstat).\n> ...\n> What do you want me to do about the patch? Should I extend it so that\n> it traps both \"*** BLURB HERE ***\" and \"*** SUBJECT HERE ***\"?\n\nOh, nothing.\n\nI did not mean to say \"if you do not add this unrelated thing, your patch\nis not acceptable.\" Please take my \"oh I wish there is another feature to\ndo this similar but different thing\" as a hint for possible follow-up\npatches, and they do not necessarily have to be done by you but can be\ndone by anybody, or for that matter they do not have to be done by anybody\nlike this particular case where it turns out to be \"why bother? it cannot\nbe done sensibly and usefully\".\n\nThanks, and sorry. I'll try to be more explicit next time.\n"}]}