{"thread":{"id":"11918","subject":"[PATCH 4/4] Add a --cover-letter option to format-patch","startedAt":"2008-02-06T16:43:28Z","lastAt":"2008-02-07T00:04:27Z","messageCount":6,"participants":["Daniel Barkalow","Peter Oberndorfer","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"67673","messageId":"alpine.LNX.1.00.0802061141410.13593@iabervon.org","threadId":"11918","inReplyTo":null,"subject":"[PATCH 4/4] Add a --cover-letter option to format-patch","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-06T16:43:28Z","receivedAt":"2008-02-06T16:43:28Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"If --cover-letter is provided, generate a cover letter message before\nthe patches, numbered 0.\n\nOriginal patch thanks to Johannes Schindelin\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\n Documentation/git-format-patch.txt                 |    6 +\n builtin-log.c                                      |  215 ++++++++++++++------\n t/t4013-diff-various.sh                            |    1 +\n ...tch_--stdout_--cover-letter_-n_initial..master^ |  101 +++++++++\n 4 files changed, 265 insertions(+), 58 deletions(-)\n create mode 100644 t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 651efe6..7ec01a0 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -17,6 +17,7 @@ SYNOPSIS\n                    [--in-reply-to=Message-Id] [--suffix=.<sfx>]\n                    [--ignore-if-in-upstream]\n                    [--subject-prefix=Subject-Prefix]\n+                   [--cover-letter]\n \t\t   [ <since> | <revision range> ]\n \n DESCRIPTION\n@@ -139,6 +140,11 @@ include::diff-options.txt[]\n \tInstead of using `.patch` as the suffix for generated\n \tfilenames, use specified suffix.  A common alternative is\n \t`--suffix=.txt`.\n+\n+--cover-letter::\n+\tGenerate a cover letter template.  You still have to fill in\n+\ta description, but the shortlog and the diffstat will be\n+\tgenerated for you.\n +\n Note that you would need to include the leading dot `.` if you\n want a filename like `0001-description-of-my-change.patch`, and\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 1f74d66..c6e3f91 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -14,6 +14,7 @@\n #include \"reflog-walk.h\"\n #include \"patch-ids.h\"\n #include \"refs.h\"\n+#include \"run-command.h\"\n \n static int default_show_root = 1;\n static const char *fmt_patch_subject_prefix = \"PATCH\";\n@@ -453,74 +454,77 @@ static int git_format_config(const char *var, const char *value)\n }\n \n \n+static const char *get_oneline_for_filename(struct commit *commit,\n+\t\tint keep_subject)\n+{\n+\tstatic char filename[PATH_MAX];\n+\tchar *sol;\n+\tint len = 0;\n+\n+\tsol = strstr(commit->buffer, \"\\n\\n\");\n+\tif (!sol)\n+\t\tfilename[0] = '\\0';\n+\telse {\n+\t\tint j, space = 0;\n+\n+\t\tsol += 2;\n+\t\t/* strip [PATCH] or [PATCH blabla] */\n+\t\tif (!keep_subject && !prefixcmp(sol, \"[PATCH\")) {\n+\t\t\tchar *eos = strchr(sol + 6, ']');\n+\t\t\tif (eos) {\n+\t\t\t\twhile (isspace(*eos))\n+\t\t\t\t\teos++;\n+\t\t\t\tsol = eos;\n+\t\t\t}\n+\t\t}\n+\n+\t\tfor (j = 0; len < sizeof(filename) &&\n+\t\t\t\tsol[j] && sol[j] != '\\n'; j++) {\n+\t\t\tif (istitlechar(sol[j])) {\n+\t\t\t\tif (space) {\n+\t\t\t\t\tfilename[len++] = '-';\n+\t\t\t\t\tspace = 0;\n+\t\t\t\t}\n+\t\t\t\tfilename[len++] = sol[j];\n+\t\t\t\tif (sol[j] == '.')\n+\t\t\t\t\twhile (sol[j + 1] == '.')\n+\t\t\t\t\t\tj++;\n+\t\t\t} else\n+\t\t\t\tspace = 1;\n+\t\t}\n+\t\twhile (filename[len - 1] == '.'\n+\t\t       || filename[len - 1] == '-')\n+\t\t\tlen--;\n+\t\tfilename[len] = '\\0';\n+\t}\n+\treturn filename;\n+}\n+\n static FILE *realstdout = NULL;\n static const char *output_directory = NULL;\n \n-static int reopen_stdout(struct commit *commit, int nr, int keep_subject,\n-\t\t\t int numbered_files)\n+static int reopen_stdout(const char *oneline, int nr, int total)\n {\n \tchar filename[PATH_MAX];\n-\tchar *sol;\n \tint len = 0;\n \tint suffix_len = strlen(fmt_patch_suffix) + 1;\n \n \tif (output_directory) {\n-\t\tif (strlen(output_directory) >=\n+\t\tlen = snprintf(filename, sizeof(filename), \"%s\",\n+\t\t\t\toutput_directory);\n+\t\tif (len >=\n \t\t    sizeof(filename) - FORMAT_PATCH_NAME_MAX - suffix_len)\n \t\t\treturn error(\"name of output directory is too long\");\n-\t\tstrlcpy(filename, output_directory, sizeof(filename) - suffix_len);\n-\t\tlen = strlen(filename);\n \t\tif (filename[len - 1] != '/')\n \t\t\tfilename[len++] = '/';\n \t}\n \n-\tif (numbered_files) {\n-\t\tsprintf(filename + len, \"%d\", nr);\n-\t\tlen = strlen(filename);\n-\n-\t} else {\n-\t\tsprintf(filename + len, \"%04d\", nr);\n-\t\tlen = strlen(filename);\n-\n-\t\tsol = strstr(commit->buffer, \"\\n\\n\");\n-\t\tif (sol) {\n-\t\t\tint j, space = 1;\n-\n-\t\t\tsol += 2;\n-\t\t\t/* strip [PATCH] or [PATCH blabla] */\n-\t\t\tif (!keep_subject && !prefixcmp(sol, \"[PATCH\")) {\n-\t\t\t\tchar *eos = strchr(sol + 6, ']');\n-\t\t\t\tif (eos) {\n-\t\t\t\t\twhile (isspace(*eos))\n-\t\t\t\t\t\teos++;\n-\t\t\t\t\tsol = eos;\n-\t\t\t\t}\n-\t\t\t}\n-\n-\t\t\tfor (j = 0;\n-\t\t\t     j < FORMAT_PATCH_NAME_MAX - suffix_len - 5 &&\n-\t\t\t\t     len < sizeof(filename) - suffix_len &&\n-\t\t\t\t     sol[j] && sol[j] != '\\n';\n-\t\t\t     j++) {\n-\t\t\t\tif (istitlechar(sol[j])) {\n-\t\t\t\t\tif (space) {\n-\t\t\t\t\t\tfilename[len++] = '-';\n-\t\t\t\t\t\tspace = 0;\n-\t\t\t\t\t}\n-\t\t\t\t\tfilename[len++] = sol[j];\n-\t\t\t\t\tif (sol[j] == '.')\n-\t\t\t\t\t\twhile (sol[j + 1] == '.')\n-\t\t\t\t\t\t\tj++;\n-\t\t\t\t} else\n-\t\t\t\t\tspace = 1;\n-\t\t\t}\n-\t\t\twhile (filename[len - 1] == '.'\n-\t\t\t       || filename[len - 1] == '-')\n-\t\t\t\tlen--;\n-\t\t\tfilename[len] = 0;\n-\t\t}\n-\t\tif (len + suffix_len >= sizeof(filename))\n-\t\t\treturn error(\"Patch pathname too long\");\n+\tif (!filename)\n+\t\tlen += sprintf(filename + len, \"%d\", nr);\n+\telse {\n+\t\tlen += sprintf(filename + len, \"%04d-\", nr);\n+\t\tlen += snprintf(filename + len, sizeof(filename) - len - 1\n+\t\t\t\t- suffix_len, \"%s\", oneline);\n \t\tstrcpy(filename + len, fmt_patch_suffix);\n \t}\n \n@@ -591,6 +595,74 @@ static void gen_message_id(struct rev_info *info, char *base)\n \tinfo->message_id = buf.buf;\n }\n \n+static void make_cover_letter(struct rev_info *rev,\n+\t\tint use_stdout, int numbered, int numbered_files,\n+\t\t\t      struct commit *origin, struct commit *head)\n+{\n+\tconst char *committer;\n+\tconst char *origin_sha1, *head_sha1;\n+\tconst char *argv[7];\n+\tconst char *subject_start = NULL;\n+\tconst char *body = \"*** SUBJECT HERE ***\\n\\n\\n*** BLURB HERE ***\\n\";\n+\tconst char *msg;\n+\tconst char *extra_headers = rev->extra_headers;\n+\tstruct strbuf sb;\n+\tconst char *encoding = \"utf-8\";\n+\n+\tif (rev->commit_format != CMIT_FMT_EMAIL)\n+\t\tdie(\"Cover letter needs email format\");\n+\n+\tif (!use_stdout && reopen_stdout(numbered_files ?\n+\t\t\t\tNULL : \"cover-letter\", 0, rev->total))\n+\t\treturn;\n+\n+\torigin_sha1 = sha1_to_hex(origin ? origin->object.sha1 : null_sha1);\n+\thead_sha1 = sha1_to_hex(head->object.sha1);\n+\n+\twrite_email_headers(rev, head_sha1, &subject_start, &extra_headers);\n+\n+\tcommitter = git_committer_info(0);\n+\n+\tmsg = body;\n+\tstrbuf_init(&sb, 0);\n+\tadd_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, DATE_RFC2822,\n+\t\t      encoding);\n+\tpp_title_line(CMIT_FMT_EMAIL, &msg, &sb, subject_start, extra_headers,\n+\t\t      encoding, 0);\n+\tpp_remainder(CMIT_FMT_EMAIL, &msg, &sb, 0);\n+\tprintf(\"%s\\n\", sb.buf);\n+\n+\tstrbuf_release(&sb);\n+\n+\t/*\n+\t * We can only do diffstat with a unique reference point, and\n+\t * log is a bit tricky, so just skip it.\n+\t */\n+\tif (!origin)\n+\t\treturn;\n+\n+\targv[0] = \"shortlog\";\n+\targv[1] = head_sha1;\n+\targv[2] = \"--not\";\n+\targv[3] = origin_sha1;\n+\targv[4] = NULL;\n+\tfflush(stdout);\n+\trun_command_v_opt(argv, RUN_GIT_CMD);\n+\n+\targv[0] = \"diff\";\n+\targv[1] = \"--stat\";\n+\targv[2] = \"--summary\";\n+\targv[3] = head_sha1;\n+\targv[4] = \"--not\";\n+\targv[5] = origin_sha1;\n+\targv[6] = NULL;\n+\tfflush(stdout);\n+\trun_command_v_opt(argv, RUN_GIT_CMD);\n+\n+\tfflush(stdout);\n+\tprintf(\"\\n\");\n+}\n+\n static const char *clean_message_id(const char *msg_id)\n {\n \tchar ch;\n@@ -626,6 +698,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tint subject_prefix = 0;\n \tint ignore_if_in_upstream = 0;\n \tint thread = 0;\n+\tint cover_letter = 0;\n+\tstruct commit *origin = NULL, *head = NULL;\n \tconst char *in_reply_to = NULL;\n \tstruct patch_ids ids;\n \tchar *add_signoff = NULL;\n@@ -725,6 +799,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\trev.subject_prefix = argv[i] + 17;\n \t\t} else if (!prefixcmp(argv[i], \"--suffix=\"))\n \t\t\tfmt_patch_suffix = argv[i] + 9;\n+\t\telse if (!strcmp(argv[i], \"--cover-letter\"))\n+\t\t\tcover_letter = 1;\n \t\telse\n \t\t\targv[j++] = argv[i];\n \t}\n@@ -776,6 +852,20 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t * get_revision() to do the usual traversal.\n \t\t */\n \t}\n+\tif (cover_letter) {\n+\t\t/* remember the range */\n+\t\tint i;\n+\t\tfor (i = 0; i < rev.pending.nr; i++) {\n+\t\t\tstruct object *o = rev.pending.objects[i].item;\n+\t\t\tif (o->flags & UNINTERESTING)\n+\t\t\t\torigin = (struct commit *)o;\n+\t\t\telse\n+\t\t\t\thead = (struct commit *)o;\n+\t\t}\n+\t\t/* We can't generate a cover letter without any patches */\n+\t\tif (!head)\n+\t\t\treturn 0;\n+\t}\n \n \tif (ignore_if_in_upstream)\n \t\tget_patch_ids(&rev, &ids, prefix);\n@@ -802,9 +892,18 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tnumbered = 1;\n \tif (numbered)\n \t\trev.total = total + start_number - 1;\n-\trev.add_signoff = add_signoff;\n \tif (in_reply_to)\n \t\trev.ref_message_id = clean_message_id(in_reply_to);\n+\tif (cover_letter) {\n+\t\tif (thread) {\n+\t\t\tgen_message_id(&rev, \"cover\");\n+\t\t}\n+\t\tmake_cover_letter(&rev, use_stdout, numbered, numbered_files,\n+\t\t\t\t  origin, head);\n+\t\ttotal++;\n+\t\tstart_number--;\n+\t}\n+\trev.add_signoff = add_signoff;\n \twhile (0 <= --nr) {\n \t\tint shown;\n \t\tcommit = list[nr];\n@@ -819,10 +918,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t}\n \t\t\tgen_message_id(&rev, sha1_to_hex(commit->object.sha1));\n \t\t}\n-\t\tif (!use_stdout)\n-\t\t\tif (reopen_stdout(commit, rev.nr, keep_subject,\n-\t\t\t\t\t  numbered_files))\n-\t\t\t\tdie(\"Failed to create output files\");\n+\t\tif (!use_stdout && reopen_stdout(numbered_files ? NULL :\n+\t\t\t\tget_oneline_for_filename(commit, keep_subject),\n+\t\t\t\trev.nr, rev.total))\n+\t\t\tdie(\"Failed to create output files\");\n \t\tshown = log_tree_commit(&rev, commit);\n \t\tfree(commit->buffer);\n \t\tcommit->buffer = NULL;\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex 9eec754..6b4d1c5 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -245,6 +245,7 @@ format-patch --inline --stdout initial..master\n format-patch --inline --stdout --subject-prefix=TESTCASE initial..master\n config format.subjectprefix DIFFERENT_PREFIX\n format-patch --inline --stdout initial..master^^\n+format-patch --stdout --cover-letter -n initial..master^\n \n diff --abbrev initial..side\n diff -r initial..side\ndiff --git a/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^\nnew file mode 100644\nindex 0000000..311e207\n--- /dev/null\n+++ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^\n@@ -0,0 +1,101 @@\n+$ git format-patch --stdout --cover-letter -n initial..master^\n+From 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 Mon Sep 17 00:00:00 2001\n+From: C O Mitter <committer@example.com>\n+Date: Mon, 26 Jun 2006 00:05:00 +0000\n+Subject: [DIFFERENT_PREFIX 0/2] *** SUBJECT HERE ***\n+\n+\n+*** BLURB HERE ***\n+\n+A U Thor (2):\n+      Second\n+      Third\n+\n+ dir/sub |    4 ++++\n+ file0   |    3 +++\n+ file1   |    3 +++\n+ file2   |    3 ---\n+ 4 files changed, 10 insertions(+), 3 deletions(-)\n+ create mode 100644 file1\n+ delete mode 100644 file2\n+\n+From 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 Mon Sep 17 00:00:00 2001\n+From: A U Thor <author@example.com>\n+Date: Mon, 26 Jun 2006 00:01:00 +0000\n+Subject: [DIFFERENT_PREFIX 1/2] Second\n+\n+This is the second commit.\n+---\n+ dir/sub |    2 ++\n+ file0   |    3 +++\n+ file2   |    3 ---\n+ 3 files changed, 5 insertions(+), 3 deletions(-)\n+ delete mode 100644 file2\n+\n+diff --git a/dir/sub b/dir/sub\n+index 35d242b..8422d40 100644\n+--- a/dir/sub\n++++ b/dir/sub\n+@@ -1,2 +1,4 @@\n+ A\n+ B\n++C\n++D\n+diff --git a/file0 b/file0\n+index 01e79c3..b414108 100644\n+--- a/file0\n++++ b/file0\n+@@ -1,3 +1,6 @@\n+ 1\n+ 2\n+ 3\n++4\n++5\n++6\n+diff --git a/file2 b/file2\n+deleted file mode 100644\n+index 01e79c3..0000000\n+--- a/file2\n++++ /dev/null\n+@@ -1,3 +0,0 @@\n+-1\n+-2\n+-3\n+-- \n+g-i-t--v-e-r-s-i-o-n\n+\n+\n+From 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 Mon Sep 17 00:00:00 2001\n+From: A U Thor <author@example.com>\n+Date: Mon, 26 Jun 2006 00:02:00 +0000\n+Subject: [DIFFERENT_PREFIX 2/2] Third\n+\n+---\n+ dir/sub |    2 ++\n+ file1   |    3 +++\n+ 2 files changed, 5 insertions(+), 0 deletions(-)\n+ create mode 100644 file1\n+\n+diff --git a/dir/sub b/dir/sub\n+index 8422d40..cead32e 100644\n+--- a/dir/sub\n++++ b/dir/sub\n+@@ -2,3 +2,5 @@ A\n+ B\n+ C\n+ D\n++E\n++F\n+diff --git a/file1 b/file1\n+new file mode 100644\n+index 0000000..b1e6722\n+--- /dev/null\n++++ b/file1\n+@@ -0,0 +1,3 @@\n++A\n++B\n++C\n+-- \n+g-i-t--v-e-r-s-i-o-n\n+\n+$\n-- \n1.5.4.27.gf6864\n"},{"id":"67681","messageId":"200802061921.58282.kumbayo84@arcor.de","threadId":"11918","inReplyTo":"alpine.LNX.1.00.0802061141410.13593@iabervon.org","subject":"Re: [PATCH 4/4] Add a --cover-letter option to format-patch","fromName":"Peter Oberndorfer","fromEmail":"kumbayo84@arcor.de","sentAt":"2008-02-06T18:21:58Z","receivedAt":"2008-02-06T18:21:58Z","isPatch":true,"sender":{"key":"kumbayo84@arcor.de","avatar":"https://avatars.githubusercontent.com/u/1041267?v=4"},"body":"On Mittwoch 06 Februar 2008, Daniel Barkalow wrote:\n> If --cover-letter is provided, generate a cover letter message before\n> the patches, numbered 0.\n> \n> Original patch thanks to Johannes Schindelin\n> \n> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>\n> ---\n> +static int reopen_stdout(const char *oneline, int nr, int total)\n>  {\n>  \tchar filename[PATH_MAX];\n> -\tchar *sol;\n>  \tint len = 0;\n>  \tint suffix_len = strlen(fmt_patch_suffix) + 1;\n>  \n>  \tif (output_directory) {\n> -\t\tif (strlen(output_directory) >=\n> +\t\tlen = snprintf(filename, sizeof(filename), \"%s\",\n> +\t\t\t\toutput_directory);\n> +\t\tif (len >=\n>  \t\t    sizeof(filename) - FORMAT_PATCH_NAME_MAX - suffix_len)\n>  \t\t\treturn error(\"name of output directory is too long\");\n> -\t\tstrlcpy(filename, output_directory, sizeof(filename) - suffix_len);\n> -\t\tlen = strlen(filename);\n>  \t\tif (filename[len - 1] != '/')\n>  \t\t\tfilename[len++] = '/';\n>  \t}\n>  \n*snip*\n> +\tif (!filename)\n> +\t\tlen += sprintf(filename + len, \"%d\", nr);\nmaybe this should be !oneline instead?\n> +\telse {\n> +\t\tlen += sprintf(filename + len, \"%04d-\", nr);\n> +\t\tlen += snprintf(filename + len, sizeof(filename) - len - 1\n> +\t\t\t\t- suffix_len, \"%s\", oneline);\n>  \t\tstrcpy(filename + len, fmt_patch_suffix);\n>  \t}\n\n> +static void make_cover_letter(struct rev_info *rev,\n> +\t\tint use_stdout, int numbered, int numbered_files,\n> +\t\t\t      struct commit *origin, struct commit *head)\n> +{\n> +\tconst char *committer;\n> +\tconst char *origin_sha1, *head_sha1;\n> +\tconst char *argv[7];\n> +\tconst char *subject_start = NULL;\n> +\tconst char *body = \"*** SUBJECT HERE ***\\n\\n\\n*** BLURB HERE ***\\n\";\nI don't know git policy but maybe use\nconst char body[] = \"*** SUBJECT HERE ***\\n\\n\\n*** BLURB HERE ***\\n\";\nsince you don't change the pointer.\n(or remove the variable)\n> +\tconst char *msg;\n> +\tconst char *extra_headers = rev->extra_headers;\n> +\tstruct strbuf sb;\n> +\tconst char *encoding = \"utf-8\";\nsame here\n> +\n> +\tif (rev->commit_format != CMIT_FMT_EMAIL)\n> +\t\tdie(\"Cover letter needs email format\");\n\nIt might be useful to split the reopen_stdout/get_oneline_for_filename\ninto a separate patch.\n\nWhen i tried to do this --cover-letter function i went the way to create a \nempty fake commit and let log_tree_commit do all the formating stuff for me.\nBut i don't know if which way is preferred...\n\nDoes you patch set up a reply to chain,\nso patches are in reply to cover letter?\nI remember battling a bit to set it up reasonably.\n\nGreetings Peter\nWho tried to create this --cover-letter function\nbut gave up silently when his patch mails never reached the ML :-(\n\nI don't know if my patches are of any use (do not apply cleanly anymore, \nreading cover letter message from a file does not honor encoding in any way)\nBut i can send them to you if you want...\n"},{"id":"67683","messageId":"alpine.LNX.1.00.0802061340190.13593@iabervon.org","threadId":"11918","inReplyTo":"200802061921.58282.kumbayo84@arcor.de","subject":"Re: [PATCH 4/4] Add a --cover-letter option to format-patch","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-06T19:02:57Z","receivedAt":"2008-02-06T19:02:57Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 6 Feb 2008, Peter Oberndorfer wrote:\n\n> On Mittwoch 06 Februar 2008, Daniel Barkalow wrote:\n> > If --cover-letter is provided, generate a cover letter message before\n> > the patches, numbered 0.\n> > \n> > Original patch thanks to Johannes Schindelin\n> > \n> > Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>\n> > ---\n> > +static int reopen_stdout(const char *oneline, int nr, int total)\n> >  {\n> >  \tchar filename[PATH_MAX];\n> > -\tchar *sol;\n> >  \tint len = 0;\n> >  \tint suffix_len = strlen(fmt_patch_suffix) + 1;\n> >  \n> >  \tif (output_directory) {\n> > -\t\tif (strlen(output_directory) >=\n> > +\t\tlen = snprintf(filename, sizeof(filename), \"%s\",\n> > +\t\t\t\toutput_directory);\n> > +\t\tif (len >=\n> >  \t\t    sizeof(filename) - FORMAT_PATCH_NAME_MAX - suffix_len)\n> >  \t\t\treturn error(\"name of output directory is too long\");\n> > -\t\tstrlcpy(filename, output_directory, sizeof(filename) - suffix_len);\n> > -\t\tlen = strlen(filename);\n> >  \t\tif (filename[len - 1] != '/')\n> >  \t\t\tfilename[len++] = '/';\n> >  \t}\n> >  \n> *snip*\n> > +\tif (!filename)\n> > +\t\tlen += sprintf(filename + len, \"%d\", nr);\n> maybe this should be !oneline instead?\n\nYup.\n\n> > +\telse {\n> > +\t\tlen += sprintf(filename + len, \"%04d-\", nr);\n> > +\t\tlen += snprintf(filename + len, sizeof(filename) - len - 1\n> > +\t\t\t\t- suffix_len, \"%s\", oneline);\n> >  \t\tstrcpy(filename + len, fmt_patch_suffix);\n> >  \t}\n> \n> > +static void make_cover_letter(struct rev_info *rev,\n> > +\t\tint use_stdout, int numbered, int numbered_files,\n> > +\t\t\t      struct commit *origin, struct commit *head)\n> > +{\n> > +\tconst char *committer;\n> > +\tconst char *origin_sha1, *head_sha1;\n> > +\tconst char *argv[7];\n> > +\tconst char *subject_start = NULL;\n> > +\tconst char *body = \"*** SUBJECT HERE ***\\n\\n\\n*** BLURB HERE ***\\n\";\n> I don't know git policy but maybe use\n> const char body[] = \"*** SUBJECT HERE ***\\n\\n\\n*** BLURB HERE ***\\n\";\n> since you don't change the pointer.\n> (or remove the variable)\n\nIt's there to make it easy to get the message from somewhere else later, \nwith something of the form:\n\n\tif (have_cover_letter_text)\n\t\tbody = cover_letter_text;\n\nLikewise encoding.\n\n> > +\tconst char *msg;\n> > +\tconst char *extra_headers = rev->extra_headers;\n> > +\tstruct strbuf sb;\n> > +\tconst char *encoding = \"utf-8\";\n> same here\n> > +\n> > +\tif (rev->commit_format != CMIT_FMT_EMAIL)\n> > +\t\tdie(\"Cover letter needs email format\");\n> \n> It might be useful to split the reopen_stdout/get_oneline_for_filename\n> into a separate patch.\n\nPerhaps; I mostly just got lazy, since I had these changes together and \nhad already finished with my shuffling things around patch.\n\n> When i tried to do this --cover-letter function i went the way to create a \n> empty fake commit and let log_tree_commit do all the formating stuff for me.\n> But i don't know if which way is preferred...\n\nThis way looks cleaner and more obvious to me, anyway.\n\n> Does you patch set up a reply to chain,\n> so patches are in reply to cover letter?\n> I remember battling a bit to set it up reasonably.\n\nI believe that, if you use --thread, all of the later messages are replies \nto the first; if you use --cover-letter, the first is the cover letter.\n\n> Greetings Peter\n> Who tried to create this --cover-letter function\n> but gave up silently when his patch mails never reached the ML :-(\n> \n> I don't know if my patches are of any use (do not apply cleanly anymore, \n> reading cover letter message from a file does not honor encoding in any way)\n> But i can send them to you if you want...\n\nI think one other person's implementation was sufficient. :)\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"67704","messageId":"7vy79xvncu.fsf@gitster.siamese.dyndns.org","threadId":"11918","inReplyTo":"alpine.LNX.1.00.0802061141410.13593@iabervon.org","subject":"Re: [PATCH 4/4] Add a --cover-letter option to format-patch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-06T20:30:57Z","receivedAt":"2008-02-06T20:30:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n> index 651efe6..7ec01a0 100644\n> --- a/Documentation/git-format-patch.txt\n> +++ b/Documentation/git-format-patch.txt\n> @@ -17,6 +17,7 @@ SYNOPSIS\n>                     [--in-reply-to=Message-Id] [--suffix=.<sfx>]\n>                     [--ignore-if-in-upstream]\n>                     [--subject-prefix=Subject-Prefix]\n> +                   [--cover-letter]\n>  \t\t   [ <since> | <revision range> ]\n>  \n>  DESCRIPTION\n> @@ -139,6 +140,11 @@ include::diff-options.txt[]\n>  \tInstead of using `.patch` as the suffix for generated\n>  \tfilenames, use specified suffix.  A common alternative is\n>  \t`--suffix=.txt`.\n> +\n> +--cover-letter::\n> +\tGenerate a cover letter template.  You still have to fill in\n> +\ta description, but the shortlog and the diffstat will be\n> +\tgenerated for you.\n>  +\n>  Note that you would need to include the leading dot `.` if you\n>  want a filename like `0001-description-of-my-change.patch`, and\n\nHuh?  Leading dot?  The insertion is one paragraph too low or high.\n\n> diff --git a/builtin-log.c b/builtin-log.c\n> index 1f74d66..c6e3f91 100644\n> --- a/builtin-log.c\n> +++ b/builtin-log.c\n> @@ -453,74 +454,77 @@ static int git_format_config(...\n> ...\n>  static FILE *realstdout = NULL;\n>  static const char *output_directory = NULL;\n>  \n> +static int reopen_stdout(const char *oneline, int nr, int total)\n>  {\n>  \tchar filename[PATH_MAX];\n>  \tint len = 0;\n>  \tint suffix_len = strlen(fmt_patch_suffix) + 1;\n>  \n>  \tif (output_directory) {\n> +\t\tlen = snprintf(filename, sizeof(filename), \"%s\",\n> +\t\t\t\toutput_directory);\n> +\t\tif (len >=\n>  \t\t    sizeof(filename) - FORMAT_PATCH_NAME_MAX - suffix_len)\n>  \t\t\treturn error(\"name of output directory is too long\");\n>  \t\tif (filename[len - 1] != '/')\n>  \t\t\tfilename[len++] = '/';\n>  \t}\n>  \n> +\tif (!filename)\n> +\t\tlen += sprintf(filename + len, \"%d\", nr);\n\nHow can this trigger?  Do you mean \"oneline\"?\n\nI can understand that you do not want to pass numbered_files\nvariable separately to the function, but then there should be a\ncomment at the beginning of this function that says \"oneline is\nNULL under numbered_files mode, the caller is responsible for\nensuring that\".\n\n> +\telse {\n> +\t\tlen += sprintf(filename + len, \"%04d-\", nr);\n> +\t\tlen += snprintf(filename + len, sizeof(filename) - len - 1\n> +\t\t\t\t- suffix_len, \"%s\", oneline);\n>  \t\tstrcpy(filename + len, fmt_patch_suffix);\n>  \t}\n>  \n> @@ -591,6 +595,74 @@ static void gen_message_id(struct rev_info *info, char *base)\n> +\t/*\n> +\t * We can only do diffstat with a unique reference point, and\n> +\t * log is a bit tricky, so just skip it.\n> +\t */\n> +\tif (!origin)\n> +\t\treturn;\n\nMaybe we would want to leave a note in the output to tell your\nusers that we punted here.\n\n> +\targv[0] = \"shortlog\";\n> +\targv[1] = head_sha1;\n> +\targv[2] = \"--not\";\n> +\targv[3] = origin_sha1;\n> +\targv[4] = NULL;\n> +\tfflush(stdout);\n> +\trun_command_v_opt(argv, RUN_GIT_CMD);\n> +\n> +\targv[0] = \"diff\";\n> +\targv[1] = \"--stat\";\n> +\targv[2] = \"--summary\";\n> +\targv[3] = head_sha1;\n> +\targv[4] = \"--not\";\n> +\targv[5] = origin_sha1;\n> +\targv[6] = NULL;\n> +\tfflush(stdout);\n> +\trun_command_v_opt(argv, RUN_GIT_CMD);\n\nMakes me wonder if we already have enough infrastructure to do\nthis without spawning two new processes.  Not complaining, but\njust wondering.\n\n> @@ -776,6 +852,20 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>  \t\t * get_revision() to do the usual traversal.\n>  \t\t */\n>  \t}\n> +\tif (cover_letter) {\n> +\t\t/* remember the range */\n> +\t\tint i;\n> +\t\tfor (i = 0; i < rev.pending.nr; i++) {\n> +\t\t\tstruct object *o = rev.pending.objects[i].item;\n> +\t\t\tif (o->flags & UNINTERESTING)\n> +\t\t\t\torigin = (struct commit *)o;\n> +\t\t\telse\n> +\t\t\t\thead = (struct commit *)o;\n> +\t\t}\n\nWhat if you have more than one origin or head?  Perhaps the\npunting logic should be modified to make sure we only have one\nnegative and one positive and nothing else?\n\n> +\t\t/* We can't generate a cover letter without any patches */\n> +\t\tif (!head)\n> +\t\t\treturn 0;\n> +\t}\n\nThat's true, but with or without cover letter we won't have\nanything to format if we do not have any positive commit.\n\n> @@ -802,9 +892,18 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>  \t\tnumbered = 1;\n>  \tif (numbered)\n>  \t\trev.total = total + start_number - 1;\n> -\trev.add_signoff = add_signoff;\n>  \tif (in_reply_to)\n>  \t\trev.ref_message_id = clean_message_id(in_reply_to);\n> +\tif (cover_letter) {\n> +\t\tif (thread) {\n> +\t\t\tgen_message_id(&rev, \"cover\");\n> +\t\t}\n> +\t\tmake_cover_letter(&rev, use_stdout, numbered, numbered_files,\n> +\t\t\t\t  origin, head);\n> +\t\ttotal++;\n> +\t\tstart_number--;\n> +\t}\n> +\trev.add_signoff = add_signoff;\n\nNice attention to the detail, moving add_signoff after the cover\nletter.\n"},{"id":"67720","messageId":"alpine.LNX.1.00.0802061608200.13593@iabervon.org","threadId":"11918","inReplyTo":"7vy79xvncu.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 4/4] Add a --cover-letter option to format-patch","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-06T22:18:28Z","receivedAt":"2008-02-06T22:18:28Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 6 Feb 2008, Junio C Hamano wrote:\n\n> Daniel Barkalow <barkalow@iabervon.org> writes:\n> \n> > diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n> > index 651efe6..7ec01a0 100644\n> > --- a/Documentation/git-format-patch.txt\n> > +++ b/Documentation/git-format-patch.txt\n> > @@ -17,6 +17,7 @@ SYNOPSIS\n> >                     [--in-reply-to=Message-Id] [--suffix=.<sfx>]\n> >                     [--ignore-if-in-upstream]\n> >                     [--subject-prefix=Subject-Prefix]\n> > +                   [--cover-letter]\n> >  \t\t   [ <since> | <revision range> ]\n> >  \n> >  DESCRIPTION\n> > @@ -139,6 +140,11 @@ include::diff-options.txt[]\n> >  \tInstead of using `.patch` as the suffix for generated\n> >  \tfilenames, use specified suffix.  A common alternative is\n> >  \t`--suffix=.txt`.\n> > +\n> > +--cover-letter::\n> > +\tGenerate a cover letter template.  You still have to fill in\n> > +\ta description, but the shortlog and the diffstat will be\n> > +\tgenerated for you.\n> >  +\n> >  Note that you would need to include the leading dot `.` if you\n> >  want a filename like `0001-description-of-my-change.patch`, and\n> \n> Huh?  Leading dot?  The insertion is one paragraph too low or high.\n\nUh, right. :)\n\n> > diff --git a/builtin-log.c b/builtin-log.c\n> > index 1f74d66..c6e3f91 100644\n> > --- a/builtin-log.c\n> > +++ b/builtin-log.c\n> > @@ -453,74 +454,77 @@ static int git_format_config(...\n> > ...\n> >  static FILE *realstdout = NULL;\n> >  static const char *output_directory = NULL;\n> >  \n> > +static int reopen_stdout(const char *oneline, int nr, int total)\n> >  {\n> >  \tchar filename[PATH_MAX];\n> >  \tint len = 0;\n> >  \tint suffix_len = strlen(fmt_patch_suffix) + 1;\n> >  \n> >  \tif (output_directory) {\n> > +\t\tlen = snprintf(filename, sizeof(filename), \"%s\",\n> > +\t\t\t\toutput_directory);\n> > +\t\tif (len >=\n> >  \t\t    sizeof(filename) - FORMAT_PATCH_NAME_MAX - suffix_len)\n> >  \t\t\treturn error(\"name of output directory is too long\");\n> >  \t\tif (filename[len - 1] != '/')\n> >  \t\t\tfilename[len++] = '/';\n> >  \t}\n> >  \n> > +\tif (!filename)\n> > +\t\tlen += sprintf(filename + len, \"%d\", nr);\n> \n> How can this trigger?  Do you mean \"oneline\"?\n\nYes.\n\n> I can understand that you do not want to pass numbered_files\n> variable separately to the function, but then there should be a\n> comment at the beginning of this function that says \"oneline is\n> NULL under numbered_files mode, the caller is responsible for\n> ensuring that\".\n\nI think the code is clearer than the option. It doesn't append the first \nline of the commit if you don't call it with the first line of the commit. \nHow the user causes this to happen is a matter for cmd_format_patch to \ndetermine.\n\n> > +\telse {\n> > +\t\tlen += sprintf(filename + len, \"%04d-\", nr);\n> > +\t\tlen += snprintf(filename + len, sizeof(filename) - len - 1\n> > +\t\t\t\t- suffix_len, \"%s\", oneline);\n> >  \t\tstrcpy(filename + len, fmt_patch_suffix);\n> >  \t}\n> >  \n> > @@ -591,6 +595,74 @@ static void gen_message_id(struct rev_info *info, char *base)\n> > +\t/*\n> > +\t * We can only do diffstat with a unique reference point, and\n> > +\t * log is a bit tricky, so just skip it.\n> > +\t */\n> > +\tif (!origin)\n> > +\t\treturn;\n> \n> Maybe we would want to leave a note in the output to tell your\n> users that we punted here.\n\nI think that if you're in this situation, you pretty much aren't expecting \nanything here, and it would be a bit annoying to stick a note into the \nemail that the user then has to remove.\n\n> > +\targv[0] = \"shortlog\";\n> > +\targv[1] = head_sha1;\n> > +\targv[2] = \"--not\";\n> > +\targv[3] = origin_sha1;\n> > +\targv[4] = NULL;\n> > +\tfflush(stdout);\n> > +\trun_command_v_opt(argv, RUN_GIT_CMD);\n> > +\n> > +\targv[0] = \"diff\";\n> > +\targv[1] = \"--stat\";\n> > +\targv[2] = \"--summary\";\n> > +\targv[3] = head_sha1;\n> > +\targv[4] = \"--not\";\n> > +\targv[5] = origin_sha1;\n> > +\targv[6] = NULL;\n> > +\tfflush(stdout);\n> > +\trun_command_v_opt(argv, RUN_GIT_CMD);\n> \n> Makes me wonder if we already have enough infrastructure to do\n> this without spawning two new processes.  Not complaining, but\n> just wondering.\n\nI don't know. They're both things that have a tendancy to interfere with \nother things, I think. You'd probably know better than me. If you think \nit's likely to work, I can try...\n\n> > @@ -776,6 +852,20 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> >  \t\t * get_revision() to do the usual traversal.\n> >  \t\t */\n> >  \t}\n> > +\tif (cover_letter) {\n> > +\t\t/* remember the range */\n> > +\t\tint i;\n> > +\t\tfor (i = 0; i < rev.pending.nr; i++) {\n> > +\t\t\tstruct object *o = rev.pending.objects[i].item;\n> > +\t\t\tif (o->flags & UNINTERESTING)\n> > +\t\t\t\torigin = (struct commit *)o;\n> > +\t\t\telse\n> > +\t\t\t\thead = (struct commit *)o;\n> > +\t\t}\n> \n> What if you have more than one origin or head?  Perhaps the\n> punting logic should be modified to make sure we only have one\n> negative and one positive and nothing else?\n\nI think the command line parsing logic prevents us from getting more than \none head in cmd_format_patch. We can have any number of negatives, with 0 \nbeing the interesting case (\"git format-patch -3\"). I may have failed to \nconsider more than one negative (which would be okay, but should again \nskip the diffstat and, for now, the shortlog).\n\n> > +\t\t/* We can't generate a cover letter without any patches */\n> > +\t\tif (!head)\n> > +\t\t\treturn 0;\n> > +\t}\n> \n> That's true, but with or without cover letter we won't have\n> anything to format if we do not have any positive commit.\n\nIf we don't have a cover letter and don't have any positive commit, we can \nperfectly happily output what was asked of us (i.e., nothing). If we're \nasked for a cover letter, we fail, because we can't generate it.\n\n> > @@ -802,9 +892,18 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> >  \t\tnumbered = 1;\n> >  \tif (numbered)\n> >  \t\trev.total = total + start_number - 1;\n> > -\trev.add_signoff = add_signoff;\n> >  \tif (in_reply_to)\n> >  \t\trev.ref_message_id = clean_message_id(in_reply_to);\n> > +\tif (cover_letter) {\n> > +\t\tif (thread) {\n> > +\t\t\tgen_message_id(&rev, \"cover\");\n> > +\t\t}\n> > +\t\tmake_cover_letter(&rev, use_stdout, numbered, numbered_files,\n> > +\t\t\t\t  origin, head);\n> > +\t\ttotal++;\n> > +\t\tstart_number--;\n> > +\t}\n> > +\trev.add_signoff = add_signoff;\n> \n> Nice attention to the detail, moving add_signoff after the cover\n> letter.\n\nI think I actually added the cover letter before the sign-off, and then \nmoved in_reply_to above the cover letter, and diff just decided to show it \nthis way. But yes, the cover letter shouldn't be signed off.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"67737","messageId":"alpine.LSU.1.00.0802070002160.8543@racer.site","threadId":"11918","inReplyTo":"200802061921.58282.kumbayo84@arcor.de","subject":"Re: [PATCH 4/4] Add a --cover-letter option to format-patch","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-07T00:04:27Z","receivedAt":"2008-02-07T00:04:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nI don't know about you, but with lines...\n\nOn Wed, 6 Feb 2008, Peter Oberndorfer wrote:\n\n> On Mittwoch 06 Februar 2008, Daniel Barkalow wrote:\n>\n\nlike this:\n\n> > -\t\tstrlcpy(filename, output_directory, sizeof(filename) -\nsuffix_len);\n\n... (when the mailer wraps the original line), comments like this...\n\n> > +\t\tlen += sprintf(filename + len, \"%d\", nr);\n> maybe this should be !oneline instead?\n> > +\telse {\n\n... are extremely hard to spot.  You could help people who are too dumb to \ndo that easily (such as yours truly), by putting an empty line between \nevery text you quote and your comments.\n\nThanks,\nDscho\n"}]}