diff mbox series

[v3] format-patch: add --description-file option

Message ID 20230821170720.577820-1-oswald.buddenhagen@gmx.de (mailing list archive)
State New, archived
Headers show
Series [v3] format-patch: add --description-file option | expand

Commit Message

Oswald Buddenhagen Aug. 21, 2023, 5:07 p.m. UTC
This patch makes it possible to directly feed a branch description to
derive the cover letter from. The use case is formatting dynamically
created temporary commits which are not referenced anywhere.

The most obvious alternative would be creating a temporary branch and
setting a description on it, but that doesn't seem particularly elegant.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
i didn't implement --description for now, after all, as i'm not
confident that passing pre-formatted text on the command line works
reliably on windows (the calling and the called program need to agree
on the quoting conventions, which is a pretty sizable can of worms),
and passing it piece-wise is no particularly good fit for my use case.
---
v3:
- beautify test
- trim commit message
- use default strbuf_read_file() size hint
v2:
- improve commit message

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            | 14 ++++++++++++++
 3 files changed, 36 insertions(+), 3 deletions(-)

Comments

Junio C Hamano Aug. 21, 2023, 5:59 p.m. UTC | #1
Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:

> This patch makes it possible to directly feed a branch description to
> derive the cover letter from. The use case is formatting dynamically
> created temporary commits which are not referenced anywhere.
>
> The most obvious alternative would be creating a temporary branch and
> setting a description on it, but that doesn't seem particularly elegant.
>
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> ---

The changed parts from the previous version all looked sensible.

Will replace and requeue.

Thanks.
Junio C Hamano Aug. 21, 2023, 10:05 p.m. UTC | #2
Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:

> +test_expect_success 'cover letter with --description-file' '
> +	test_when_finished "rm -f description.txt" &&
> +	cat >description.txt <<\-EOF &&

"<<\-EOF"  ->  "<<-\EOF"

No need to resend as I've fixed it up locally already.

"make test" would have caught this, by the way.

Thanks.
Taylor Blau Aug. 23, 2023, 8:01 p.m. UTC | #3
On Mon, Aug 21, 2023 at 03:05:35PM -0700, Junio C Hamano wrote:
> Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
>
> > +test_expect_success 'cover letter with --description-file' '
> > +	test_when_finished "rm -f description.txt" &&
> > +	cat >description.txt <<\-EOF &&
>
> "<<\-EOF"  ->  "<<-\EOF"

I'm late to the party, but this was the only thing that I noticed with
the patch, too. The rest LGTM.

Thanks,
Taylor
Kristoffer Haugsbakk Sept. 23, 2023, 10:14 p.m. UTC | #4
On Mon, Aug 21, 2023, at 19:07, Oswald Buddenhagen wrote:
> This patch makes it possible to directly feed a branch description to
> derive the cover letter from. The use case is formatting dynamically
> created temporary commits which are not referenced anywhere.

Thanks for implementing this. I've just written cover letter text in
separate files and copied them into the generated files every time. (I
don't use branch descriptions.) I've wanted some convenient way to feed
these messages in, and if I end up writing a cover letter again I'll most
probably be using this new option.

Cheers
Junio C Hamano Sept. 25, 2023, 7:01 p.m. UTC | #5
"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:

> On Mon, Aug 21, 2023, at 19:07, Oswald Buddenhagen wrote:
>> This patch makes it possible to directly feed a branch description to
>> derive the cover letter from. The use case is formatting dynamically
>> created temporary commits which are not referenced anywhere.
>
> Thanks for implementing this. I've just written cover letter text in
> separate files and copied them into the generated files every time. (I
> don't use branch descriptions.) I've wanted some convenient way to feed
> these messages in, and if I end up writing a cover letter again I'll most
> probably be using this new option.

Thanks for a positive feedback.  The changes is already in 'master'
since the beginning of this month or so and its way to be part of
the next release, I believe.
Kristoffer Haugsbakk Sept. 25, 2023, 7:29 p.m. UTC | #6
On Mon, Sep 25, 2023, at 21:01, Junio C Hamano wrote:
> Thanks for a positive feedback.  The changes is already in 'master'
> since the beginning of this month or so and its way to be part of
> the next release, I believe.

Yes, I tested it yesterday and it works (in conjunction with
`--cover-from-description=subject`) exactly like I want it to. :D
diff mbox series

Patch

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..967415e193 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, 0) < 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..e1d660130f 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1991,6 +1991,20 @@  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" &&
+	cat >description.txt <<\-EOF &&
+	subject from file
+
+	body from file
+	EOF
+	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