| Message ID | 20230807170936.2336760-1-oswald.buddenhagen@gmx.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RESEND] format-patch: add --description-file option | expand |
Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes: > When formatting patches from a detached HEAD, there is no branch > description to derive the cover letter from. While with format-patch > one could post-process the generated file (which would be ugly enough), > scripting that with send-email would be *really* ugly. So add an option > to feed a description directly. I think it makes sense to give the same set of features to those who run format-patch from a detached HEAD as to those who run it on a branch. But personally I am not interested in a new feature that encourages use of send-email as a front-end to format-patch, which I consider is a misfeature, to make it easier for a set of patches without final proofreading to be sent out. Having said that, with my maintainer hat on, if we were to add a new feature to format-patch, it makes sense to allow it passed through send-email as well, since the (mis)feature already exists. Please elaborate a bit more on the use case, though. * "there is no branch description to derive from" makes a reader wonder what the workflow would become if you could do "git branch --add-description HEAD" to prepare a description, which would imply that what is more desirable might be a feature enhancement of the "branch" command, not "format-patch" or "send-email", to allow you to describe what you are doing on the HEAD. * Or does the end-user have a branch with description already prepared, but for some untold reason the tip of the branch is checked out on a detached HEAD? If so, an obviously better alternative design would be to add a feature that passes a branch name to format-patch and tell it to pretend that the user is working on the branch. That way, not just "description", any feature that makes the command use "which branch are we on?" information to enhance its behaviour we have right now or we will add to the command will all benefit. For example, builtin/log.c::cmd_format_match() uses branch_name only for calling read_branch_desc() via prepare_cover_text(), but it is perfectly reasonable for us to make the range-diff default derived based on the reflog of the "current branch" on, and "pretend we were on this branch" may help you in such a case. In other words, if a particular solution proposed (or not proposed) is sensible or not heavily depends on how the end-user ends up running format-patch (and sending the output out) on a detached HEAD, and where does the end-user want to take the description information from. No, the answer to the latter is not "the file specified with the --description-file option"; that is not a valid answer. The question is about how the contents of that file is populated and maintained. A feature to specify the template used when generating the cover letter may also work well for such a use case. Among placeholders to specify where to place auto-generated things like: - shortlog information - other ways to list commits in the series (e.g. listing of commit titles from "git log --oneline -r" may be more appropriate and readable than "shortlog" output especially when the series was written by multiple authors), - diffstat there would be a placeholder to stuff branch description output (for the normal case), and in your detached HEAD use case, you'd prepare such a template without using branch description placeholder, but instead prepare the description in place in the template before running format-patch. Which might actually be a better alternative. But all of that depends on what the expected use case to support is. Thanks.
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 373b46fc0d..8e515c7dbb 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -215,6 +215,10 @@ is greater than 100 bytes, then the mode will be `message`, otherwise If `<mode>` is `none`, both the cover letter subject and body will be populated with placeholder text. +--description-file=<file>:: + Use the contents of <file> instead of the branch's description + for generating the cover letter. + --subject-prefix=<subject prefix>:: Instead of the standard '[PATCH]' prefix in the subject line, instead use '[<subject prefix>]'. This diff --git a/builtin/log.c b/builtin/log.c index 1b119eaf0b..9c4738bbde 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1255,7 +1255,15 @@ static void show_diffstat(struct rev_info *rev, fprintf(rev->diffopt.file, "\n"); } +static void read_desc_file(struct strbuf *buf, const char *desc_file) +{ + if (strbuf_read_file(buf, desc_file, 2000) < 0) + die_errno(_("unable to read branch description file '%s'"), + desc_file); +} + static void prepare_cover_text(struct pretty_print_context *pp, + const char *description_file, const char *branch_name, struct strbuf *sb, const char *encoding, @@ -1269,7 +1277,9 @@ static void prepare_cover_text(struct pretty_print_context *pp, if (cover_from_description_mode == COVER_FROM_NONE) goto do_pp; - if (branch_name && *branch_name) + if (description_file && *description_file) + read_desc_file(&description_sb, description_file); + else if (branch_name && *branch_name) read_branch_desc(&description_sb, branch_name); if (!description_sb.len) goto do_pp; @@ -1315,6 +1325,7 @@ static void get_notes_args(struct strvec *arg, struct rev_info *rev) static void make_cover_letter(struct rev_info *rev, int use_separate_file, struct commit *origin, int nr, struct commit **list, + const char *description_file, const char *branch_name, int quiet) { @@ -1354,7 +1365,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file, pp.rev = rev; pp.print_email_subject = 1; pp_user_info(&pp, NULL, &sb, committer, encoding); - prepare_cover_text(&pp, branch_name, &sb, encoding, need_8bit_cte); + prepare_cover_text(&pp, description_file, branch_name, &sb, + encoding, need_8bit_cte); fprintf(rev->diffopt.file, "%s\n", sb.buf); strbuf_release(&sb); @@ -1895,6 +1907,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) int quiet = 0; const char *reroll_count = NULL; char *cover_from_description_arg = NULL; + char *description_file = NULL; char *branch_name = NULL; char *base_commit = NULL; struct base_tree_info bases; @@ -1938,6 +1951,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) OPT_STRING(0, "cover-from-description", &cover_from_description_arg, N_("cover-from-description-mode"), N_("generate parts of a cover letter based on a branch's description")), + OPT_FILENAME(0, "description-file", &description_file, + N_("use branch description from file")), OPT_CALLBACK_F(0, "subject-prefix", &rev, N_("prefix"), N_("use [<prefix>] instead of [PATCH]"), PARSE_OPT_NONEG, subject_prefix_callback), @@ -2323,7 +2338,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) if (thread) gen_message_id(&rev, "cover"); make_cover_letter(&rev, !!output_directory, - origin, nr, list, branch_name, quiet); + origin, nr, list, description_file, branch_name, quiet); print_bases(&bases, rev.diffopt.file); print_signature(rev.diffopt.file); total++; diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 3cf2b7a7fb..b31401876b 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1991,6 +1991,18 @@ test_expect_success 'cover letter using branch description (6)' ' grep hello actual ' +test_expect_success 'cover letter with --description-file' ' + test_when_finished "rm -f description.txt" && + echo "subject from file + +body from file" > description.txt && + git checkout rebuild-1 && + git format-patch --stdout --cover-letter --cover-from-description auto \ + --description-file description.txt main >actual && + grep "^Subject: \[PATCH 0/2\] subject from file$" actual && + grep "^body from file$" actual +' + test_expect_success 'cover letter with nothing' ' git format-patch --stdout --cover-letter >actual && test_line_count = 0 actual
When formatting patches from a detached HEAD, there is no branch description to derive the cover letter from. While with format-patch one could post-process the generated file (which would be ugly enough), scripting that with send-email would be *really* ugly. So add an option to feed a description directly. Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de> --- Cc: Jeff King <peff@peff.net> Cc: Taylor Blau <me@ttaylorr.com> Cc: Derrick Stolee <derrickstolee@github.com> Cc: Junio C Hamano <gitster@pobox.com> --- Documentation/git-format-patch.txt | 4 ++++ builtin/log.c | 21 ++++++++++++++++++--- t/t4014-format-patch.sh | 12 ++++++++++++ 3 files changed, 34 insertions(+), 3 deletions(-)