{"thread":{"id":"60075","subject":"[PATCH RESEND] format-patch: add --description-file option","startedAt":"2023-08-07T17:09:51Z","lastAt":"2023-09-25T19:29:38Z","messageCount":14,"participants":["Oswald Buddenhagen","Junio C Hamano","Taylor Blau","Kristoffer Haugsbakk"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"480238","messageId":"20230807170936.2336760-1-oswald.buddenhagen@gmx.de","threadId":"60075","inReplyTo":null,"subject":"[PATCH RESEND] format-patch: add --description-file option","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-07T17:09:36Z","receivedAt":"2023-08-07T17:09:51Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"When formatting patches from a detached HEAD, there is no branch\ndescription to derive the cover letter from. While with format-patch\none could post-process the generated file (which would be ugly enough),\nscripting that with send-email would be *really* ugly. So add an option\nto feed a description directly.\n\nSigned-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n---\n\nCc: Jeff King <peff@peff.net>\nCc: Taylor Blau <me@ttaylorr.com>\nCc: Derrick Stolee <derrickstolee@github.com>\nCc: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/git-format-patch.txt |  4 ++++\n builtin/log.c                      | 21 ++++++++++++++++++---\n t/t4014-format-patch.sh            | 12 ++++++++++++\n 3 files changed, 34 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 373b46fc0d..8e515c7dbb 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -215,6 +215,10 @@ is greater than 100 bytes, then the mode will be `message`, otherwise\n If `<mode>` is `none`, both the cover letter subject and body will be\n populated with placeholder text.\n \n+--description-file=<file>::\n+\tUse the contents of <file> instead of the branch's description\n+\tfor generating the cover letter.\n+\n --subject-prefix=<subject prefix>::\n \tInstead of the standard '[PATCH]' prefix in the subject\n \tline, instead use '[<subject prefix>]'. This\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 1b119eaf0b..9c4738bbde 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1255,7 +1255,15 @@ static void show_diffstat(struct rev_info *rev,\n \tfprintf(rev->diffopt.file, \"\\n\");\n }\n \n+static void read_desc_file(struct strbuf *buf, const char *desc_file)\n+{\n+\tif (strbuf_read_file(buf, desc_file, 2000) < 0)\n+\t\tdie_errno(_(\"unable to read branch description file '%s'\"),\n+\t\t\t  desc_file);\n+}\n+\n static void prepare_cover_text(struct pretty_print_context *pp,\n+\t\t\t       const char *description_file,\n \t\t\t       const char *branch_name,\n \t\t\t       struct strbuf *sb,\n \t\t\t       const char *encoding,\n@@ -1269,7 +1277,9 @@ static void prepare_cover_text(struct pretty_print_context *pp,\n \tif (cover_from_description_mode == COVER_FROM_NONE)\n \t\tgoto do_pp;\n \n-\tif (branch_name && *branch_name)\n+\tif (description_file && *description_file)\n+\t\tread_desc_file(&description_sb, description_file);\n+\telse if (branch_name && *branch_name)\n \t\tread_branch_desc(&description_sb, branch_name);\n \tif (!description_sb.len)\n \t\tgoto do_pp;\n@@ -1315,6 +1325,7 @@ static void get_notes_args(struct strvec *arg, struct rev_info *rev)\n static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \t\t\t      struct commit *origin,\n \t\t\t      int nr, struct commit **list,\n+\t\t\t      const char *description_file,\n \t\t\t      const char *branch_name,\n \t\t\t      int quiet)\n {\n@@ -1354,7 +1365,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \tpp.rev = rev;\n \tpp.print_email_subject = 1;\n \tpp_user_info(&pp, NULL, &sb, committer, encoding);\n-\tprepare_cover_text(&pp, branch_name, &sb, encoding, need_8bit_cte);\n+\tprepare_cover_text(&pp, description_file, branch_name, &sb,\n+\t\t\t   encoding, need_8bit_cte);\n \tfprintf(rev->diffopt.file, \"%s\\n\", sb.buf);\n \n \tstrbuf_release(&sb);\n@@ -1895,6 +1907,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tint quiet = 0;\n \tconst char *reroll_count = NULL;\n \tchar *cover_from_description_arg = NULL;\n+\tchar *description_file = NULL;\n \tchar *branch_name = NULL;\n \tchar *base_commit = NULL;\n \tstruct base_tree_info bases;\n@@ -1938,6 +1951,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"cover-from-description\", &cover_from_description_arg,\n \t\t\t    N_(\"cover-from-description-mode\"),\n \t\t\t    N_(\"generate parts of a cover letter based on a branch's description\")),\n+\t\tOPT_FILENAME(0, \"description-file\", &description_file,\n+\t\t\t     N_(\"use branch description from file\")),\n \t\tOPT_CALLBACK_F(0, \"subject-prefix\", &rev, N_(\"prefix\"),\n \t\t\t    N_(\"use [<prefix>] instead of [PATCH]\"),\n \t\t\t    PARSE_OPT_NONEG, subject_prefix_callback),\n@@ -2323,7 +2338,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tif (thread)\n \t\t\tgen_message_id(&rev, \"cover\");\n \t\tmake_cover_letter(&rev, !!output_directory,\n-\t\t\t\t  origin, nr, list, branch_name, quiet);\n+\t\t\t\t  origin, nr, list, description_file, branch_name, quiet);\n \t\tprint_bases(&bases, rev.diffopt.file);\n \t\tprint_signature(rev.diffopt.file);\n \t\ttotal++;\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 3cf2b7a7fb..b31401876b 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1991,6 +1991,18 @@ test_expect_success 'cover letter using branch description (6)' '\n \tgrep hello actual\n '\n \n+test_expect_success 'cover letter with --description-file' '\n+\ttest_when_finished \"rm -f description.txt\" &&\n+\techo \"subject from file\n+\n+body from file\" > description.txt &&\n+\tgit checkout rebuild-1 &&\n+\tgit format-patch --stdout --cover-letter --cover-from-description auto \\\n+\t\t--description-file description.txt main >actual &&\n+\tgrep \"^Subject: \\[PATCH 0/2\\] subject from file$\" actual &&\n+\tgrep \"^body from file$\" actual\n+'\n+\n test_expect_success 'cover letter with nothing' '\n \tgit format-patch --stdout --cover-letter >actual &&\n \ttest_line_count = 0 actual\n-- \n2.40.0.152.g15d061e6df\n\n"},{"id":"480283","messageId":"xmqqttta9h6a.fsf@gitster.g","threadId":"60075","inReplyTo":"20230807170936.2336760-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH RESEND] format-patch: add --description-file option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-08T06:33:49Z","receivedAt":"2023-08-08T16:30:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> When formatting patches from a detached HEAD, there is no branch\n> description to derive the cover letter from. While with format-patch\n> one could post-process the generated file (which would be ugly enough),\n> scripting that with send-email would be *really* ugly. So add an option\n> to feed a description directly.\n\nI think it makes sense to give the same set of features to those who\nrun format-patch from a detached HEAD as to those who run it on a\nbranch.  But personally I am not interested in a new feature that\nencourages use of send-email as a front-end to format-patch, which I\nconsider is a misfeature, to make it easier for a set of patches\nwithout final proofreading to be sent out.\n\nHaving said that, with my maintainer hat on, if we were to add a new\nfeature to format-patch, it makes sense to allow it passed through\nsend-email as well, since the (mis)feature already exists.\n\nPlease elaborate a bit more on the use case, though.\n\n * \"there is no branch description to derive from\" makes a reader\n   wonder what the workflow would become if you could do \"git branch\n   --add-description HEAD\" to prepare a description, which would\n   imply that what is more desirable might be a feature enhancement\n   of the \"branch\" command, not \"format-patch\" or \"send-email\", to\n   allow you to describe what you are doing on the HEAD.\n\n * Or does the end-user have a branch with description already\n   prepared, but for some untold reason the tip of the branch is\n   checked out on a detached HEAD?\n\n   If so, an obviously better alternative design would be to add a\n   feature that passes a branch name to format-patch and tell it to\n   pretend that the user is working on the branch.  That way, not\n   just \"description\", any feature that makes the command use \"which\n   branch are we on?\" information to enhance its behaviour we have\n   right now or we will add to the command will all benefit.  For\n   example, builtin/log.c::cmd_format_match() uses branch_name only\n   for calling read_branch_desc() via prepare_cover_text(), but it\n   is perfectly reasonable for us to make the range-diff default\n   derived based on the reflog of the \"current branch\" on, and\n   \"pretend we were on this branch\" may help you in such a case.\n\nIn other words, if a particular solution proposed (or not proposed)\nis sensible or not heavily depends on how the end-user ends up\nrunning format-patch (and sending the output out) on a detached\nHEAD, and where does the end-user want to take the description\ninformation from. No, the answer to the latter is not \"the file\nspecified with the --description-file option\"; that is not a valid\nanswer.  The question is about how the contents of that file is\npopulated and maintained.\n\nA feature to specify the template used when generating the cover\nletter may also work well for such a use case.  Among placeholders\nto specify where to place auto-generated things like:\n\n - shortlog information\n - other ways to list commits in the series (e.g. listing of commit\n   titles from \"git log --oneline -r\" may be more appropriate and\n   readable than \"shortlog\" output especially when the series was\n   written by multiple authors),\n - diffstat\n\nthere would be a placeholder to stuff branch description output (for\nthe normal case), and in your detached HEAD use case, you'd prepare\nsuch a template without using branch description placeholder, but\ninstead prepare the description in place in the template before\nrunning format-patch.  Which might actually be a better alternative.\n\nBut all of that depends on what the expected use case to support is.\n\nThanks.\n"},{"id":"480377","messageId":"20230809171530.2564724-1-oswald.buddenhagen@gmx.de","threadId":"60075","inReplyTo":"xmqqttta9h6a.fsf@gitster.g","subject":"[PATCH v2] format-patch: add --description-file option","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-09T17:15:30Z","receivedAt":"2023-08-09T17:15:47Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"This patch makes it possible to directly feed a branch description to\nderive the cover letter from. The use case is formatting dynamically\ncreated temporary commits which are not referenced anywhere.\n\nThe most obvious alternative would be creating a temporary branch and\nsetting a description on it, but that doesn't seem particularly elegant.\n\nOne could also post-process the cover letter. This would be conceptually\nugly due to having to make assumptions about its format, and hooking\nthis into send-email would be *really* ugly.\n\nAs a variation of that, one could make it possible to feed a template\nfor the cover letter, into which the description could be embedded for\nthe given use case. This would still need to make assumptions about the\ntemplate, so while not as fragile, it would be still conceptually ugly,\nand simply not a good fit for the given use case.\n\nSigned-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n---\nv2:\n- improve commit message\n\nCc: Jeff King <peff@peff.net>\nCc: Taylor Blau <me@ttaylorr.com>\nCc: Derrick Stolee <derrickstolee@github.com>\nCc: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/git-format-patch.txt |  4 ++++\n builtin/log.c                      | 21 ++++++++++++++++++---\n t/t4014-format-patch.sh            | 12 ++++++++++++\n 3 files changed, 34 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 373b46fc0d..8e515c7dbb 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -215,6 +215,10 @@ is greater than 100 bytes, then the mode will be `message`, otherwise\n If `<mode>` is `none`, both the cover letter subject and body will be\n populated with placeholder text.\n \n+--description-file=<file>::\n+\tUse the contents of <file> instead of the branch's description\n+\tfor generating the cover letter.\n+\n --subject-prefix=<subject prefix>::\n \tInstead of the standard '[PATCH]' prefix in the subject\n \tline, instead use '[<subject prefix>]'. This\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 1b119eaf0b..9c4738bbde 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1255,7 +1255,15 @@ static void show_diffstat(struct rev_info *rev,\n \tfprintf(rev->diffopt.file, \"\\n\");\n }\n \n+static void read_desc_file(struct strbuf *buf, const char *desc_file)\n+{\n+\tif (strbuf_read_file(buf, desc_file, 2000) < 0)\n+\t\tdie_errno(_(\"unable to read branch description file '%s'\"),\n+\t\t\t  desc_file);\n+}\n+\n static void prepare_cover_text(struct pretty_print_context *pp,\n+\t\t\t       const char *description_file,\n \t\t\t       const char *branch_name,\n \t\t\t       struct strbuf *sb,\n \t\t\t       const char *encoding,\n@@ -1269,7 +1277,9 @@ static void prepare_cover_text(struct pretty_print_context *pp,\n \tif (cover_from_description_mode == COVER_FROM_NONE)\n \t\tgoto do_pp;\n \n-\tif (branch_name && *branch_name)\n+\tif (description_file && *description_file)\n+\t\tread_desc_file(&description_sb, description_file);\n+\telse if (branch_name && *branch_name)\n \t\tread_branch_desc(&description_sb, branch_name);\n \tif (!description_sb.len)\n \t\tgoto do_pp;\n@@ -1315,6 +1325,7 @@ static void get_notes_args(struct strvec *arg, struct rev_info *rev)\n static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \t\t\t      struct commit *origin,\n \t\t\t      int nr, struct commit **list,\n+\t\t\t      const char *description_file,\n \t\t\t      const char *branch_name,\n \t\t\t      int quiet)\n {\n@@ -1354,7 +1365,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \tpp.rev = rev;\n \tpp.print_email_subject = 1;\n \tpp_user_info(&pp, NULL, &sb, committer, encoding);\n-\tprepare_cover_text(&pp, branch_name, &sb, encoding, need_8bit_cte);\n+\tprepare_cover_text(&pp, description_file, branch_name, &sb,\n+\t\t\t   encoding, need_8bit_cte);\n \tfprintf(rev->diffopt.file, \"%s\\n\", sb.buf);\n \n \tstrbuf_release(&sb);\n@@ -1895,6 +1907,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tint quiet = 0;\n \tconst char *reroll_count = NULL;\n \tchar *cover_from_description_arg = NULL;\n+\tchar *description_file = NULL;\n \tchar *branch_name = NULL;\n \tchar *base_commit = NULL;\n \tstruct base_tree_info bases;\n@@ -1938,6 +1951,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"cover-from-description\", &cover_from_description_arg,\n \t\t\t    N_(\"cover-from-description-mode\"),\n \t\t\t    N_(\"generate parts of a cover letter based on a branch's description\")),\n+\t\tOPT_FILENAME(0, \"description-file\", &description_file,\n+\t\t\t     N_(\"use branch description from file\")),\n \t\tOPT_CALLBACK_F(0, \"subject-prefix\", &rev, N_(\"prefix\"),\n \t\t\t    N_(\"use [<prefix>] instead of [PATCH]\"),\n \t\t\t    PARSE_OPT_NONEG, subject_prefix_callback),\n@@ -2323,7 +2338,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tif (thread)\n \t\t\tgen_message_id(&rev, \"cover\");\n \t\tmake_cover_letter(&rev, !!output_directory,\n-\t\t\t\t  origin, nr, list, branch_name, quiet);\n+\t\t\t\t  origin, nr, list, description_file, branch_name, quiet);\n \t\tprint_bases(&bases, rev.diffopt.file);\n \t\tprint_signature(rev.diffopt.file);\n \t\ttotal++;\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 3cf2b7a7fb..b31401876b 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1991,6 +1991,18 @@ test_expect_success 'cover letter using branch description (6)' '\n \tgrep hello actual\n '\n \n+test_expect_success 'cover letter with --description-file' '\n+\ttest_when_finished \"rm -f description.txt\" &&\n+\techo \"subject from file\n+\n+body from file\" > description.txt &&\n+\tgit checkout rebuild-1 &&\n+\tgit format-patch --stdout --cover-letter --cover-from-description auto \\\n+\t\t--description-file description.txt main >actual &&\n+\tgrep \"^Subject: \\[PATCH 0/2\\] subject from file$\" actual &&\n+\tgrep \"^body from file$\" actual\n+'\n+\n test_expect_success 'cover letter with nothing' '\n \tgit format-patch --stdout --cover-letter >actual &&\n \ttest_line_count = 0 actual\n-- \n2.40.0.152.g15d061e6df\n\n"},{"id":"480591","messageId":"xmqq1qg9qmyq.fsf@gitster.g","threadId":"60075","inReplyTo":"20230809171530.2564724-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v2] format-patch: add --description-file option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-11T21:38:05Z","receivedAt":"2023-08-11T21:38:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> This patch makes it possible to directly feed a branch description to\n> derive the cover letter from. The use case is formatting dynamically\n> created temporary commits which are not referenced anywhere.\n>\n> The most obvious alternative would be creating a temporary branch and\n> setting a description on it, but that doesn't seem particularly elegant.\n\nElegance is quite subjective, but not having to create a temporary\nbranch is a good thing in itself.  Everything after \", but that\ndoesn't\" is better left out of the message here.\n\nIn the longer term, the templating system is the way to go to\nachieve more flexibility that is more general than the single\nproblem this patch is trying to solve, but we do not always have to\nwait for such a most general solution.\n\n> +--description-file=<file>::\n> +\tUse the contents of <file> instead of the branch's description\n> +\tfor generating the cover letter.\n\nOK.\n\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 1b119eaf0b..9c4738bbde 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -1255,7 +1255,15 @@ static void show_diffstat(struct rev_info *rev,\n>  \tfprintf(rev->diffopt.file, \"\\n\");\n>  }\n>  \n> +static void read_desc_file(struct strbuf *buf, const char *desc_file)\n> +{\n> +\tif (strbuf_read_file(buf, desc_file, 2000) < 0)\n> +\t\tdie_errno(_(\"unable to read branch description file '%s'\"),\n> +\t\t\t  desc_file);\n> +}\n\nYou would probably want to do \"2000\" -> \"0\" here.\n\n>  static void prepare_cover_text(struct pretty_print_context *pp,\n> +\t\t\t       const char *description_file,\n>  \t\t\t       const char *branch_name,\n\nThis is kind of suboptimal, but let's let it pass.\n\nA better design is to pass the description string itself to this\nfunction and the make_cover_letter() function, and have the higher\nlevel callers of the callchain prepare the either read_desc_file()\nor read_branch_desc() to prepare that string before calling into the\ncallchain.  Such a division of labor between the callers and this\nfunction will allow us to more easily add another option to the\ncommand, to feed the description string itself (instead of having to\ncreate a temporary file and storing the description in it).\n\nBut such a clean-up can be safely left for now and be done after the\ndust settles.\n\n> @@ -1269,7 +1277,9 @@ static void prepare_cover_text(struct pretty_print_context *pp,\n>  \tif (cover_from_description_mode == COVER_FROM_NONE)\n>  \t\tgoto do_pp;\n>  \n> -\tif (branch_name && *branch_name)\n> +\tif (description_file && *description_file)\n> +\t\tread_desc_file(&description_sb, description_file);\n> +\telse if (branch_name && *branch_name)\n>  \t\tread_branch_desc(&description_sb, branch_name);\n\nThis allows use of a custom description to override the branch\ndescription, which makes quite a lot of sense.\n\n> @@ -1315,6 +1325,7 @@ static void get_notes_args(struct strvec *arg, struct rev_info *rev)\n>  static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n>  \t\t\t      struct commit *origin,\n>  \t\t\t      int nr, struct commit **list,\n> +\t\t\t      const char *description_file,\n>  \t\t\t      const char *branch_name,\n\nI've already touched on this.\n\n>  \t\t\t      int quiet)\n>  {\n> @@ -1354,7 +1365,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n>  \tpp.rev = rev;\n>  \tpp.print_email_subject = 1;\n>  \tpp_user_info(&pp, NULL, &sb, committer, encoding);\n> -\tprepare_cover_text(&pp, branch_name, &sb, encoding, need_8bit_cte);\n> +\tprepare_cover_text(&pp, description_file, branch_name, &sb,\n> +\t\t\t   encoding, need_8bit_cte);\n>  \tfprintf(rev->diffopt.file, \"%s\\n\", sb.buf);\n>  \n>  \tstrbuf_release(&sb);\n> @@ -1895,6 +1907,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>  \tint quiet = 0;\n>  \tconst char *reroll_count = NULL;\n>  \tchar *cover_from_description_arg = NULL;\n> +\tchar *description_file = NULL;\n>  \tchar *branch_name = NULL;\n>  \tchar *base_commit = NULL;\n>  \tstruct base_tree_info bases;\n> @@ -1938,6 +1951,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>  \t\tOPT_STRING(0, \"cover-from-description\", &cover_from_description_arg,\n>  \t\t\t    N_(\"cover-from-description-mode\"),\n>  \t\t\t    N_(\"generate parts of a cover letter based on a branch's description\")),\n> +\t\tOPT_FILENAME(0, \"description-file\", &description_file,\n> +\t\t\t     N_(\"use branch description from file\")),\n>  \t\tOPT_CALLBACK_F(0, \"subject-prefix\", &rev, N_(\"prefix\"),\n>  \t\t\t    N_(\"use [<prefix>] instead of [PATCH]\"),\n>  \t\t\t    PARSE_OPT_NONEG, subject_prefix_callback),\n> @@ -2323,7 +2338,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>  \t\tif (thread)\n>  \t\t\tgen_message_id(&rev, \"cover\");\n>  \t\tmake_cover_letter(&rev, !!output_directory,\n> -\t\t\t\t  origin, nr, list, branch_name, quiet);\n> +\t\t\t\t  origin, nr, list, description_file, branch_name, quiet);\n>  \t\tprint_bases(&bases, rev.diffopt.file);\n>  \t\tprint_signature(rev.diffopt.file);\n>  \t\ttotal++;\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index 3cf2b7a7fb..b31401876b 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -1991,6 +1991,18 @@ test_expect_success 'cover letter using branch description (6)' '\n>  \tgrep hello actual\n>  '\n>  \n> +test_expect_success 'cover letter with --description-file' '\n> +\ttest_when_finished \"rm -f description.txt\" &&\n> +\techo \"subject from file\n> +\n> +body from file\" > description.txt &&\n\nIt is more conventional to write a multi-line file like so:\n\n\tcat >description.txt <<\\-EOF &&\n\tsubject from file\n\n\tbody from file\n\tEOF\n\nwhich would allow readers not to get distracted by the unindented\nsecond line.  Also redirection operator \">\" (or \"<\") sticks to the\ntarget file in our coding style.\n\n> +\tgit checkout rebuild-1 &&\n> +\tgit format-patch --stdout --cover-letter --cover-from-description auto \\\n> +\t\t--description-file description.txt main >actual &&\n> +\tgrep \"^Subject: \\[PATCH 0/2\\] subject from file$\" actual &&\n> +\tgrep \"^body from file$\" actual\n\nNice.\n\n> +'\n> +\n>  test_expect_success 'cover letter with nothing' '\n>  \tgit format-patch --stdout --cover-letter >actual &&\n>  \ttest_line_count = 0 actual\n\nThanks.\n"},{"id":"480604","messageId":"ZNa2VQQzZmaMzM4C@ugly","threadId":"60075","inReplyTo":"xmqq1qg9qmyq.fsf@gitster.g","subject":"Re: [PATCH v2] format-patch: add --description-file option","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-11T22:29:41Z","receivedAt":"2023-08-11T22:29:47Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Fri, Aug 11, 2023 at 02:38:05PM -0700, Junio C Hamano wrote:\n>> +\tif (strbuf_read_file(buf, desc_file, 2000) < 0)\n>\n>You would probably want to do \"2000\" -> \"0\" here.\n>\nhmm, yeah, i wonder where i got it from, given that there is no \nprecedent. i suppose i simply thought that 2k is a reasonably expectable \nmax size for a description. if you think the default 8k hint is a better \nidea, then let's go with it.\n\n>>  static void prepare_cover_text(struct pretty_print_context *pp,\n>> +\t\t\t       const char *description_file,\n>>  \t\t\t       const char *branch_name,\n>\n>This is kind of suboptimal, but let's let it pass.\n>\n>A better design is to pass the description string itself to this\n>function and the make_cover_letter() function, and have the higher\n>level callers of the callchain prepare the either read_desc_file()\n>or read_branch_desc() to prepare that string before calling into the\n>callchain. \n>\nthere is only one caller, and doing this change would essentially result \nin inlining prepare_cover_text(). probably not the best outcome.\n\n>Such a division of labor between the callers and this\n>function will allow us to more easily add another option to the\n>command, to feed the description string itself (instead of having to\n>create a temporary file and storing the description in it).\n>\nthat's a good point. in fact, passing in the description directly would \nprobably fit my use case better ... i just happened to already have the \ncode for creating that temp file anyway (for editing), so i didn't give \nit a second thought. i can add both options in the same go, given that \nit's almost no code.\n\nregards\n"},{"id":"480630","messageId":"xmqq7cq0oelh.fsf@gitster.g","threadId":"60075","inReplyTo":"ZNa2VQQzZmaMzM4C@ugly","subject":"Re: [PATCH v2] format-patch: add --description-file option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-12T08:21:46Z","receivedAt":"2023-08-12T08:22:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> On Fri, Aug 11, 2023 at 02:38:05PM -0700, Junio C Hamano wrote:\n>>> +\tif (strbuf_read_file(buf, desc_file, 2000) < 0)\n>>\n>>You would probably want to do \"2000\" -> \"0\" here.\n>>\n> hmm, yeah, i wonder where i got it from, given that there is no\n> precedent. i suppose i simply thought that 2k is a reasonably\n> expectable max size for a description. if you think the default 8k\n> hint is a better idea, then let's go with it.\n\nThe suggestion was not about 2000 vs 8kiB, though it seems we stick\nto power of 2 everywhere we are explicit.  Unless we know the exact\nsize from .st_size, that is.\n\nIt was primarily about this code not having any need to express its\nown preference and go with whatever is the default.\n\n> that's a good point. in fact, passing in the description directly\n> would probably fit my use case better ... i just happened to already\n> have the code for creating that temp file anyway (for editing), so i\n> didn't give it a second thought. i can add both options in the same\n> go, given that it's almost no code.\n\nOne thing that you may have to be careful about, if you also take\nstrings directly from the command line, is what to do with multiple\nof them.  \"git commit -m A -m B\" that makes A and B separate\nparagraphs with a break between them, I would think, would serve as\na good model that end-users already understand well.\n\nThanks.\n\n"},{"id":"480632","messageId":"ZNiS0TrN0mYD6N97@ugly","threadId":"60075","inReplyTo":"xmqq7cq0oelh.fsf@gitster.g","subject":"Re: [PATCH v2] format-patch: add --description-file option","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-13T08:22:41Z","receivedAt":"2023-08-13T08:22:50Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Sat, Aug 12, 2023 at 01:21:46AM -0700, Junio C Hamano wrote:\n>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>\n>> On Fri, Aug 11, 2023 at 02:38:05PM -0700, Junio C Hamano wrote:\n>>>> +\tif (strbuf_read_file(buf, desc_file, 2000) < 0)\n>>>\n>>>You would probably want to do \"2000\" -> \"0\" here.\n>>>\n>> hmm, yeah, i wonder where i got it from, given that there is no\n>> precedent. i suppose i simply thought that 2k is a reasonably\n>> expectable max size for a description. if you think the default 8k\n>> hint is a better idea, then let's go with it.\n>\n>The suggestion was not about 2000 vs 8kiB,\n>\ni know. i just mentioned the default for reference. it seems \"severely\" \noversized for the task - not that it would actually matter.\n\n>though it seems we stick\n>to power of 2 everywhere we are explicit.  Unless we know the exact\n>size from .st_size, that is.\n>\n>It was primarily about this code not having any need to express its\n>own preference and go with whatever is the default.\n>\narguably, just about every other instance which uses a fixed hint \ndoesn't need to, yet some of them do. it's somewhat obvious in the case \nof \"tiny\" hints, but there are also some cases of 1k. and the \nsequencer's do_commit() uses 2k for the message file, which is \"a funny \ncoincidence\".\nso i wonder whether there is some standard to go by.\n\n>> that's a good point. in fact, passing in the description directly\n>> would probably fit my use case better ... i just happened to already\n>> have the code for creating that temp file anyway (for editing), so i\n>> didn't give it a second thought. i can add both options in the same\n>> go, given that it's almost no code.\n>\n>One thing that you may have to be careful about, if you also take\n>strings directly from the command line, is what to do with multiple\n>of them.  \"git commit -m A -m B\" that makes A and B separate\n>paragraphs with a break between them, I would think, would serve as\n>a good model that end-users already understand well.\n>\nno worries about that, i'd just copy from commit anyway.\n\nhowever, this points out a potential problem, which makes me have second \nthoughts about my use case ... i would want to pass the entire contents \nin one argument, newlines and quotes included. i know that this is \ninherently ok on unix, but i wonder whether it would work reliably on \nwindows (the wrapper script is written in perl)?\n\nregards\n\n"},{"id":"480848","messageId":"20230821170720.577820-1-oswald.buddenhagen@gmx.de","threadId":"60075","inReplyTo":"xmqq1qg9qmyq.fsf@gitster.g","subject":"[PATCH v3] format-patch: add --description-file option","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-08-21T17:07:20Z","receivedAt":"2023-08-21T17:07:35Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"This patch makes it possible to directly feed a branch description to\nderive the cover letter from. The use case is formatting dynamically\ncreated temporary commits which are not referenced anywhere.\n\nThe most obvious alternative would be creating a temporary branch and\nsetting a description on it, but that doesn't seem particularly elegant.\n\nSigned-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n---\ni didn't implement --description for now, after all, as i'm not\nconfident that passing pre-formatted text on the command line works\nreliably on windows (the calling and the called program need to agree\non the quoting conventions, which is a pretty sizable can of worms),\nand passing it piece-wise is no particularly good fit for my use case.\n---\nv3:\n- beautify test\n- trim commit message\n- use default strbuf_read_file() size hint\nv2:\n- improve commit message\n\nCc: Jeff King <peff@peff.net>\nCc: Taylor Blau <me@ttaylorr.com>\nCc: Derrick Stolee <derrickstolee@github.com>\nCc: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/git-format-patch.txt |  4 ++++\n builtin/log.c                      | 21 ++++++++++++++++++---\n t/t4014-format-patch.sh            | 14 ++++++++++++++\n 3 files changed, 36 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 373b46fc0d..8e515c7dbb 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -215,6 +215,10 @@ is greater than 100 bytes, then the mode will be `message`, otherwise\n If `<mode>` is `none`, both the cover letter subject and body will be\n populated with placeholder text.\n \n+--description-file=<file>::\n+\tUse the contents of <file> instead of the branch's description\n+\tfor generating the cover letter.\n+\n --subject-prefix=<subject prefix>::\n \tInstead of the standard '[PATCH]' prefix in the subject\n \tline, instead use '[<subject prefix>]'. This\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 1b119eaf0b..967415e193 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1255,7 +1255,15 @@ static void show_diffstat(struct rev_info *rev,\n \tfprintf(rev->diffopt.file, \"\\n\");\n }\n \n+static void read_desc_file(struct strbuf *buf, const char *desc_file)\n+{\n+\tif (strbuf_read_file(buf, desc_file, 0) < 0)\n+\t\tdie_errno(_(\"unable to read branch description file '%s'\"),\n+\t\t\t  desc_file);\n+}\n+\n static void prepare_cover_text(struct pretty_print_context *pp,\n+\t\t\t       const char *description_file,\n \t\t\t       const char *branch_name,\n \t\t\t       struct strbuf *sb,\n \t\t\t       const char *encoding,\n@@ -1269,7 +1277,9 @@ static void prepare_cover_text(struct pretty_print_context *pp,\n \tif (cover_from_description_mode == COVER_FROM_NONE)\n \t\tgoto do_pp;\n \n-\tif (branch_name && *branch_name)\n+\tif (description_file && *description_file)\n+\t\tread_desc_file(&description_sb, description_file);\n+\telse if (branch_name && *branch_name)\n \t\tread_branch_desc(&description_sb, branch_name);\n \tif (!description_sb.len)\n \t\tgoto do_pp;\n@@ -1315,6 +1325,7 @@ static void get_notes_args(struct strvec *arg, struct rev_info *rev)\n static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \t\t\t      struct commit *origin,\n \t\t\t      int nr, struct commit **list,\n+\t\t\t      const char *description_file,\n \t\t\t      const char *branch_name,\n \t\t\t      int quiet)\n {\n@@ -1354,7 +1365,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \tpp.rev = rev;\n \tpp.print_email_subject = 1;\n \tpp_user_info(&pp, NULL, &sb, committer, encoding);\n-\tprepare_cover_text(&pp, branch_name, &sb, encoding, need_8bit_cte);\n+\tprepare_cover_text(&pp, description_file, branch_name, &sb,\n+\t\t\t   encoding, need_8bit_cte);\n \tfprintf(rev->diffopt.file, \"%s\\n\", sb.buf);\n \n \tstrbuf_release(&sb);\n@@ -1895,6 +1907,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tint quiet = 0;\n \tconst char *reroll_count = NULL;\n \tchar *cover_from_description_arg = NULL;\n+\tchar *description_file = NULL;\n \tchar *branch_name = NULL;\n \tchar *base_commit = NULL;\n \tstruct base_tree_info bases;\n@@ -1938,6 +1951,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"cover-from-description\", &cover_from_description_arg,\n \t\t\t    N_(\"cover-from-description-mode\"),\n \t\t\t    N_(\"generate parts of a cover letter based on a branch's description\")),\n+\t\tOPT_FILENAME(0, \"description-file\", &description_file,\n+\t\t\t     N_(\"use branch description from file\")),\n \t\tOPT_CALLBACK_F(0, \"subject-prefix\", &rev, N_(\"prefix\"),\n \t\t\t    N_(\"use [<prefix>] instead of [PATCH]\"),\n \t\t\t    PARSE_OPT_NONEG, subject_prefix_callback),\n@@ -2323,7 +2338,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tif (thread)\n \t\t\tgen_message_id(&rev, \"cover\");\n \t\tmake_cover_letter(&rev, !!output_directory,\n-\t\t\t\t  origin, nr, list, branch_name, quiet);\n+\t\t\t\t  origin, nr, list, description_file, branch_name, quiet);\n \t\tprint_bases(&bases, rev.diffopt.file);\n \t\tprint_signature(rev.diffopt.file);\n \t\ttotal++;\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 3cf2b7a7fb..e1d660130f 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1991,6 +1991,20 @@ test_expect_success 'cover letter using branch description (6)' '\n \tgrep hello actual\n '\n \n+test_expect_success 'cover letter with --description-file' '\n+\ttest_when_finished \"rm -f description.txt\" &&\n+\tcat >description.txt <<\\-EOF &&\n+\tsubject from file\n+\n+\tbody from file\n+\tEOF\n+\tgit checkout rebuild-1 &&\n+\tgit format-patch --stdout --cover-letter --cover-from-description auto \\\n+\t\t--description-file description.txt main >actual &&\n+\tgrep \"^Subject: \\[PATCH 0/2\\] subject from file$\" actual &&\n+\tgrep \"^body from file$\" actual\n+'\n+\n test_expect_success 'cover letter with nothing' '\n \tgit format-patch --stdout --cover-letter >actual &&\n \ttest_line_count = 0 actual\n-- \n2.40.0.152.g15d061e6df\n\n"},{"id":"480850","messageId":"xmqqwmxop98a.fsf@gitster.g","threadId":"60075","inReplyTo":"20230821170720.577820-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v3] format-patch: add --description-file option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-21T17:59:33Z","receivedAt":"2023-08-21T17:59:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> This patch makes it possible to directly feed a branch description to\n> derive the cover letter from. The use case is formatting dynamically\n> created temporary commits which are not referenced anywhere.\n>\n> The most obvious alternative would be creating a temporary branch and\n> setting a description on it, but that doesn't seem particularly elegant.\n>\n> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n> ---\n\nThe changed parts from the previous version all looked sensible.\n\nWill replace and requeue.\n\nThanks.\n"},{"id":"480904","messageId":"xmqq4jksoxu8.fsf@gitster.g","threadId":"60075","inReplyTo":"20230821170720.577820-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v3] format-patch: add --description-file option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-21T22:05:35Z","receivedAt":"2023-08-21T22:05:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n\n> +test_expect_success 'cover letter with --description-file' '\n> +\ttest_when_finished \"rm -f description.txt\" &&\n> +\tcat >description.txt <<\\-EOF &&\n\n\"<<\\-EOF\"  ->  \"<<-\\EOF\"\n\nNo need to resend as I've fixed it up locally already.\n\n\"make test\" would have caught this, by the way.\n\nThanks.\n"},{"id":"480970","messageId":"ZOZlnu1wC70BFp9p@nand.local","threadId":"60075","inReplyTo":"xmqq4jksoxu8.fsf@gitster.g","subject":"Re: [PATCH v3] format-patch: add --description-file option","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-08-23T20:01:34Z","receivedAt":"2023-08-23T20:02:17Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Aug 21, 2023 at 03:05:35PM -0700, Junio C Hamano wrote:\n> Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:\n>\n> > +test_expect_success 'cover letter with --description-file' '\n> > +\ttest_when_finished \"rm -f description.txt\" &&\n> > +\tcat >description.txt <<\\-EOF &&\n>\n> \"<<\\-EOF\"  ->  \"<<-\\EOF\"\n\nI'm late to the party, but this was the only thing that I noticed with\nthe patch, too. The rest LGTM.\n\nThanks,\nTaylor\n"},{"id":"482214","messageId":"a1920050-bedc-49d4-840d-350b8fd3c003@app.fastmail.com","threadId":"60075","inReplyTo":"20230821170720.577820-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v3] format-patch: add --description-file option","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-23T22:14:28Z","receivedAt":"2023-09-23T22:15:18Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Mon, Aug 21, 2023, at 19:07, Oswald Buddenhagen wrote:\n> This patch makes it possible to directly feed a branch description to\n> derive the cover letter from. The use case is formatting dynamically\n> created temporary commits which are not referenced anywhere.\n\nThanks for implementing this. I've just written cover letter text in\nseparate files and copied them into the generated files every time. (I\ndon't use branch descriptions.) I've wanted some convenient way to feed\nthese messages in, and if I end up writing a cover letter again I'll most\nprobably be using this new option.\n\nCheers\n\n-- \nKristoffer Haugsbakk\n"},{"id":"482299","messageId":"xmqq7coet6vm.fsf@gitster.g","threadId":"60075","inReplyTo":"a1920050-bedc-49d4-840d-350b8fd3c003@app.fastmail.com","subject":"Re: [PATCH v3] format-patch: add --description-file option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-25T19:01:01Z","receivedAt":"2023-09-25T19:01:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kristoffer Haugsbakk\" <code@khaugsbakk.name> writes:\n\n> On Mon, Aug 21, 2023, at 19:07, Oswald Buddenhagen wrote:\n>> This patch makes it possible to directly feed a branch description to\n>> derive the cover letter from. The use case is formatting dynamically\n>> created temporary commits which are not referenced anywhere.\n>\n> Thanks for implementing this. I've just written cover letter text in\n> separate files and copied them into the generated files every time. (I\n> don't use branch descriptions.) I've wanted some convenient way to feed\n> these messages in, and if I end up writing a cover letter again I'll most\n> probably be using this new option.\n\nThanks for a positive feedback.  The changes is already in 'master'\nsince the beginning of this month or so and its way to be part of\nthe next release, I believe.\n\n\n"},{"id":"482303","messageId":"b2089384-4e1c-4956-ad57-0969151a09a6@app.fastmail.com","threadId":"60075","inReplyTo":"xmqq7coet6vm.fsf@gitster.g","subject":"Re: [PATCH v3] format-patch: add --description-file option","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-25T19:29:13Z","receivedAt":"2023-09-25T19:29:38Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Mon, Sep 25, 2023, at 21:01, Junio C Hamano wrote:\n> Thanks for a positive feedback.  The changes is already in 'master'\n> since the beginning of this month or so and its way to be part of\n> the next release, I believe.\n\nYes, I tested it yesterday and it works (in conjunction with\n`--cover-from-description=subject`) exactly like I want it to. :D\n\n-- \nKristoffer Haugsbakk\n"}]}