{"thread":{"id":"22538","subject":"[PATCH] add new options to git format-patch: --cover-subject and --cover-blurb","startedAt":"2010-02-05T21:39:33Z","lastAt":"2010-02-06T19:13:15Z","messageCount":8,"participants":["Larry D'Anna","Wesley J. Landaker","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"133732","messageId":"1265405973-5670-1-git-send-email-larry@elder-gods.org","threadId":"22538","inReplyTo":null,"subject":"[PATCH] add new options to git format-patch: --cover-subject and --cover-blurb","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-05T21:39:33Z","receivedAt":"2010-02-05T21:39:33Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"This is useful because if you're preparing a patch series with a cover letter\nyou can easily put together one line to format and email the whole thing to\nyourself. You check to make sure everything is right, and then just change the\nrecipient address and run it again.\n\ngit send-email --to my@mydomain.org master..HEAD --cover-letter \\\n --cover-subject \"this is my patch series\" --cover-blurb \"$(cat blurb.txt)\"\n\ncheck the results in my inbox\n\ngit send-email --to git@vger.kernel.org master..HEAD --cover-letter \\\n --cover-subject \"this is my patch series\" --cover-blurb \"$(cat blurb.txt)\"\n\nSigned-off-by: Larry D'Anna \n---\n Documentation/git-format-patch.txt | 8 ++++++++\n builtin-log.c | 15 +++++++++++++--\n 2 files changed, 21 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 9674f9d..522c56f 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -176,6 +176,14 @@ will want to ensure that threading is disabled for `git send-email`.\n \tcontaining the shortlog and the overall diffstat. You can\n \tfill in a description in the file before sending it out.\n \n+--cover-subject=\n+\tInstead of using *** SUBJECT HERE ***, specify the subject line of the\n+\tcover letter.\n+\n+--cover-blurb=\n+\tInstead of using *** BLURB HERE ***, specify a blurb for the body of the\n+\tcover letter.\n+\n --suffix=.::\n \tInstead of using `.patch` as the suffix for generated\n \tfilenames, use specified suffix. A common alternative is\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 8d16832..e7ae37e 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -452,6 +452,8 @@ int cmd_log(int argc, const char **argv, const char *prefix)\n \n /* format-patch */\n \n+static const char *cover_subject = \"*** SUBJECT HERE ***\";\n+static const char *cover_blurb = \"*** BLURB HERE ***\";\n static const char *fmt_patch_suffix = \".patch\";\n static int numbered = 0;\n static int auto_number = 1;\n@@ -647,7 +649,6 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n {\n \tconst char *committer;\n \tconst char *subject_start = NULL;\n-\tconst char *body = \"*** SUBJECT HERE ***\\n\\n*** BLURB HERE ***\\n\";\n \tconst char *msg;\n \tconst char *extra_headers = rev->extra_headers;\n \tstruct shortlog log;\n@@ -695,12 +696,15 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t\tif (has_non_ascii(list[i]->buffer))\n \t\t\tneed_8bit_cte = 1;\n \n-\tmsg = body;\n \tpp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, DATE_RFC2822,\n \t\t encoding);\n+\n+\tmsg = cover_subject;\n \tpp_title_line(CMIT_FMT_EMAIL, &msg, &sb, subject_start, extra_headers,\n \t\t encoding, need_8bit_cte);\n+\tmsg = cover_blurb;\n \tpp_remainder(CMIT_FMT_EMAIL, &msg, &sb, 0);\n+\n \tprintf(\"%s\\n\", sb.buf);\n \n \tstrbuf_release(&sb);\n@@ -913,6 +917,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t \"print patches to standard out\"),\n \t\tOPT_BOOLEAN(0, \"cover-letter\", &cover_letter,\n \t\t\t \"generate a cover letter\"),\n+\t\tOPT_STRING(0, \"cover-subject\", &cover_subject, \"subject\",\n+\t\t\t\t \"use in the subject line of the cover letter\"),\n+\t\tOPT_STRING(0, \"cover-blurb\", &cover_blurb, \"blurb\",\n+\t\t\t\t \"use in the body of the cover letter\"),\n \t\tOPT_BOOLEAN(0, \"numbered-files\", &numbered_files,\n \t\t\t \"use simple number sequence for output file names\"),\n \t\tOPT_STRING(0, \"suffix\", &fmt_patch_suffix, \"sfx\",\n@@ -1048,6 +1056,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tif (rev.diffopt.output_format & DIFF_FORMAT_CHECKDIFF)\n \t\tdie(\"--check does not make sense\");\n \n+\tif (strchr(cover_subject, '\\n'))\n+\t\tdie(\"--cover-subject can not contain newlines\");\n+\n \tif (!use_patch_format &&\n \t\t(!rev.diffopt.output_format ||\n \t\t rev.diffopt.output_format == DIFF_FORMAT_PATCH))\n-- \n1.7.0.rc1.33.g07cf0f.dirty\n"},{"id":"133736","messageId":"201002051526.18205.wjl@icecavern.net","threadId":"22538","inReplyTo":"1265405973-5670-1-git-send-email-larry@elder-gods.org","subject":"Re: [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb","fromName":"Wesley J. Landaker","fromEmail":"wjl@icecavern.net","sentAt":"2010-02-05T22:26:17Z","receivedAt":"2010-02-05T22:26:17Z","isPatch":true,"sender":{"key":"wjl@icecavern.net","avatar":"https://avatars.githubusercontent.com/u/67229?v=4"},"body":"On Friday 05 February 2010 14:39:33 Larry D'Anna wrote:\n> This is useful because if you're preparing a patch series with a cover\n> letter you can easily put together one line to format and email the\n> whole thing to yourself. You check to make sure everything is right,\n> and then just change the recipient address and run it again.\n> \n> git send-email --to my@mydomain.org master..HEAD --cover-letter \\\n> --cover-subject \"this is my patch series\" --cover-blurb \"$(cat\n> blurb.txt)\"\n\nOne (minor?) issue is that the cover blub would be limited to the maximum \nallowed length of the command-line arguments set by the shell or OS. Since \nyou are just catting a file, maybe \"--cover-blub-file\" would be better?\n\nJust a thought.\n"},{"id":"133738","messageId":"7vfx5fwbws.fsf@alter.siamese.dyndns.org","threadId":"22538","inReplyTo":"201002051526.18205.wjl@icecavern.net","subject":"Re: [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-05T22:33:23Z","receivedAt":"2010-02-05T22:33:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Wesley J. Landaker\" writes:\n\n> On Friday 05 February 2010 14:39:33 Larry D'Anna wrote:\n>> This is useful because if you're preparing a patch series with a cover\n>> letter you can easily put together one line to format and email the\n>> whole thing to yourself. You check to make sure everything is right,\n>> and then just change the recipient address and run it again.\n>> \n>> git send-email --to my@mydomain.org master..HEAD --cover-letter \\\n>> --cover-subject \"this is my patch series\" --cover-blurb \"$(cat\n>> blurb.txt)\"\n>\n> One (minor?) issue is that the cover blub would be limited to the maximum \n> allowed length of the command-line arguments set by the shell or OS. Since \n> you are just catting a file, maybe \"--cover-blub-file\" would be better?\n>\n> Just a thought.\n\nThe placeholder in particular and the cover letter itself in general are\nmeant to be edited. I do not see much point in forcing people to edit yet\nanother file and have them specify with an cover-blurb option.\n\nNot very interested.\n"},{"id":"133739","messageId":"201002051553.27315.wjl@icecavern.net","threadId":"22538","inReplyTo":"7vfx5fwbws.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb","fromName":"Wesley J. Landaker","fromEmail":"wjl@icecavern.net","sentAt":"2010-02-05T22:53:27Z","receivedAt":"2010-02-05T22:53:27Z","isPatch":true,"sender":{"key":"wjl@icecavern.net","avatar":"https://avatars.githubusercontent.com/u/67229?v=4"},"body":"On Friday 05 February 2010 15:33:23 Junio C Hamano wrote:\n> The placeholder in particular and the cover letter itself in general are\n> meant to be edited. I do not see much point in forcing people to edit\n> yet another file and have them specify with an cover-blurb option.\n> \n> Not very interested.\n\nThe original use-case is also pretty close to just doing the following:\n\n$ git format-patch master..HEAD --cover-letter \n$ vi 0000-cover-letter.patch\n$ git send-email --to my@mydomain.org *.patch\n$ git send-email --to git@vger.kernel.org *.patch\n\nIsn't that just as easy as the proposed --cover-* options?\n"},{"id":"133740","messageId":"20100205225901.GA29821@cthulhu","threadId":"22538","inReplyTo":"7vfx5fwbws.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-05T22:59:01Z","receivedAt":"2010-02-05T22:59:01Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"* Junio C Hamano (gitster@pobox.com) [100205 17:33]:\n\n> The placeholder in particular and the cover letter itself in general are\n> meant to be edited. I do not see much point in forcing people to edit yet\n> another file and have them specify with an cover-blurb option.\n> \n> Not very interested.\n\nYes, they're meant to be edited, but if you look at the steps required to submit\na series with cover letter, it's clear it could be a bit streamlined:\n\n1) make your branch\n\n2) git format-patch --cover-letter\n\n3) edit the cover letter\n\n3) review the series, and realize you need to fix something, fix it.\n\n4) git format-patch --cover-letter again\n\n5) edit the cover letter, *again*. hopefully you didn't overwrite the old one.\n\n6) git send-email --to myself\n\n7) one last look over it in my inbox\n\n8) git send-email --to the list\n\nThe whole thing is a lot less annoying and error-prone if you can have\ngit-send-email call git-format-patch. \n\nBesides, you're not forcing anyone to edit an extra file. If you leave out\n--cover-subject or --cover-blurb it just behaves in exactly the same way it\nalways did.\n\n\n --larry\n"},{"id":"133741","messageId":"20100205230024.GB29821@cthulhu","threadId":"22538","inReplyTo":"201002051553.27315.wjl@icecavern.net","subject":"Re: [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-05T23:00:24Z","receivedAt":"2010-02-05T23:00:24Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"* Wesley J. Landaker (wjl@icecavern.net) [100205 17:53]:\n> On Friday 05 February 2010 15:33:23 Junio C Hamano wrote:\n> > The placeholder in particular and the cover letter itself in general are\n> > meant to be edited. I do not see much point in forcing people to edit\n> > yet another file and have them specify with an cover-blurb option.\n> > \n> > Not very interested.\n> \n> The original use-case is also pretty close to just doing the following:\n> \n> $ git format-patch master..HEAD --cover-letter \n> $ vi 0000-cover-letter.patch\n> $ git send-email --to my@mydomain.org *.patch\n> $ git send-email --to git@vger.kernel.org *.patch\n> \n> Isn't that just as easy as the proposed --cover-* options?\n\nExcept when you decide you need to modify it after sending it to yourself. \n\n --larry\n"},{"id":"133768","messageId":"7vtytvjhit.fsf@alter.siamese.dyndns.org","threadId":"22538","inReplyTo":"20100205225901.GA29821@cthulhu","subject":"Re: [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-06T01:10:34Z","receivedAt":"2010-02-06T01:10:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Larry D'Anna writes:\n\n> 1) make your branch\n>\n> 2) git format-patch --cover-letter\n>\n> 3) edit the cover letter\n>\n> 3) review the series, and realize you need to fix something, fix it.\n\nHmph, this begs a natural question: why didn't you review and realize that\nin step (1)?\n\n> 4) git format-patch --cover-letter again\n>\n> 5) edit the cover letter, *again*. hopefully you didn't overwrite the\n> old one.\n\nThis step I can understand and am very sympathetic to the cause, even\nthough I may not be convinced that the patch under discussion is the best\nsolution to the issue.\n\nWhat argument are you giving to the \"-o\" option? If your series changed\n(e.g. inserted or deleted a commit in the middle, retitled, etc.), and\nyour output is going to the same directory, you would end up with files\nwith duplicate serial numbers and you would need to purge the old one\nbefore your next invocation of send-email. For this reason, people\nquickly learn to either give a different -o location (so that they can\ncompare two versions), or to purge the old contents before running\nformat-patch. If the latter, it would be sufficient to save the old\n0000-cover before removing them, and if the former, the old cover is\nalready there. You can cut and paste from there while editing the new\none.\n\nThe thing I found suboptimal in your approach is that most often the cover\nletter is written to explain what the overall goal of the series is and\nhow each patch relates to each other to achieve that goal. In order to\neffectively do so, the overview format-patch leaves in 0000-cover template\nfile helps a lot (actually that is half the reason why it shows the\noverview---the other is for the recipients).\n\nYour approach forces the user to write the blurb part in a separate file\non blank sheet of paper _before_ running format-patch, iow, without the\nhelp of that series overview, if they want to take advantage of your \"I\ndon't want to lose what I wrote already\" feature. To put it another way,\npeople who use --cover-blurb would write suboptimal (or maybe useless)\nblurb text exactly because they don't look at the series overview while\nthey write it---the option encourages a bad cover letter to be sent to\nreviewers.\n\nI am hoping we can do better than that.\n\nIt might be sufficient for format-patch to notice a 0000-cover file that\nis already there, read the subject and blurb part and carry that forward,\ninstead of unconditionally writing \"*** SUBJECT HERE ***\" and stuff. That\nway, the user does not have to prepare a separate file before running\nformat-patch.\n\nBy scanning from the bottom of the existing 0000-cover file, skipping\ndiffstat part (easy to spot with regexp) and then skip backwards a block\nof text whose lines are one of:\n\n (1) two space indented---that's one-line-per-commit;\n\n (2) empty line---separator; or\n\n (3) unindented line that ends with '(' number ')' ':'---the author.\n\nThe remainder would be the BLURB. And you know it is much easier to find\nwhere the Subject: is ;-)\n"},{"id":"133819","messageId":"20100206191315.GA3732@progeny.tock","threadId":"22538","inReplyTo":"7vtytvjhit.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-06T19:13:15Z","receivedAt":"2010-02-06T19:13:15Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Larry D'Anna writes:\n \n>> 1) make your branch\n>>\n>> 2) git format-patch --cover-letter\n>>\n>> 3) edit the cover letter\n>>\n>> 3) review the series, and realize you need to fix something, fix it.\n>\n> Hmph, this begs a natural question: why didn't you review and realize that\n> in step (1)?\n\nOne answer: writing a cover letter forces one to reflect a little.\nPerhaps that is why the cover letter and review share step 3. ;-)\n\n> It might be sufficient for format-patch to notice a 0000-cover file that\n> is already there, read the subject and blurb part and carry that forward,\n> instead of unconditionally writing \"*** SUBJECT HERE ***\" and stuff. That\n> way, the user does not have to prepare a separate file before running\n> format-patch.\n\nFWIW I think this sounds sane and would be happy to see this feature.\n\nJonathan\n\n> By scanning from the bottom of the existing 0000-cover file, skipping\n> diffstat part (easy to spot with regexp) and then skip backwards a block\n> of text whose lines are one of:\n> \n> (1) two space indented---that's one-line-per-commit;\n> \n> (2) empty line---separator; or\n> \n> (3) unindented line that ends with '(' number ')' ':'---the author.\n> \n> The remainder would be the BLURB. And you know it is much easier to find\n> where the Subject: is ;-)\n"}]}