[GSoC PATCH] format-patch: write numbered list in cover letter

[GSoC PATCH] format-patch: write numbered list in cover letter

[Pablo Sabater]

Cover letter generated with 'git format-patch --cover-letter' uses
shortlog grouping commits by author. For a single author patch grouping
by author add useless information and makes it hard to follow patch
references: "second patch does x and y patch does z"

Replace the shortlog with a numbered list of patches:

  [1/2]: first commit
  [2/2]: second commit

shortlog grouping by author is lost, both for single author patches
and multiple author patches

suggested as #leftoverbits by Junio C Hamano at
https://lore.kernel.org/git/xmqqbjhjxp2d.fsf@gitster.g/

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
I haven't tried to group by author on multiple authors patches to keep it simple 
but I believe it's doable.

 Documentation/git-format-patch.adoc           |  5 ++--
 builtin/log.c                                 | 26 +++++++++----------
 ..._--stdout_--cover-letter_-n_initial..main^ |  5 ++--
 t/t4014-format-patch.sh                       | 16 +++---------
 4 files changed, 21 insertions(+), 31 deletions(-)

diff --git a/Documentation/git-format-patch.adoc b/Documentation/git-format-patch.adoc
index 36146006fa..420f05099c 100644
--- a/Documentation/git-format-patch.adoc
+++ b/Documentation/git-format-patch.adoc
@@ -319,8 +319,9 @@ feeding the result to `git send-email`.
 --cover-letter::
 --no-cover-letter::
 	In addition to the patches, generate a cover letter file
-	containing the branch description, shortlog and the overall diffstat.  You can
-	fill in a description in the file before sending it out.
+	containing the branch description, numbered [n/m] list of patches and
+	the overall diffstat.  You can fill in a description in the file before
+	sending it out.
 
 --encode-email-headers::
 --no-encode-email-headers::
diff --git a/builtin/log.c b/builtin/log.c
index 7cb919bca9..002af4fa59 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1332,7 +1332,6 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 			      const struct format_config *cfg)
 {
 	const char *from;
-	struct shortlog log;
 	struct strbuf sb = STRBUF_INIT;
 	int i;
 	const char *encoding = "UTF-8";
@@ -1340,6 +1339,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 	struct pretty_print_context pp = {0};
 	struct commit *head = list[0];
 	char *to_free = NULL;
+	struct strbuf oneline = STRBUF_INIT;
+	struct pretty_print_context ctx = {0};
 
 	if (!cmit_fmt_is_mail(rev->commit_format))
 		die(_("cover letter needs email format"));
@@ -1376,18 +1377,17 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 	free(pp.after_subject);
 	strbuf_release(&sb);
 
-	shortlog_init(&log);
-	log.wrap_lines = 1;
-	log.wrap = MAIL_DEFAULT_WRAP;
-	log.in1 = 2;
-	log.in2 = 4;
-	log.file = rev->diffopt.file;
-	log.groups = SHORTLOG_GROUP_AUTHOR;
-	shortlog_finish_setup(&log);
-	for (i = 0; i < nr; i++)
-		shortlog_add_commit(&log, list[i]);
-
-	shortlog_output(&log);
+	ctx.fmt = CMIT_FMT_USERFORMAT;
+	ctx.output_encoding = get_log_output_encoding();
+
+	for (i = nr - 1; i >= 0; i--) {
+		strbuf_reset(&oneline);
+		repo_format_commit_message(the_repository, list[i], "%s", &oneline, &ctx);
+		fprintf(rev->diffopt.file, "  [%d/%d]: %s\n", nr - i, nr, oneline.buf);
+	}
+	fprintf(rev->diffopt.file, "\n");
+
+	strbuf_release(&oneline);
 
 	/* We can only do diffstat with a unique reference point */
 	if (origin)
diff --git a/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..main^ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..main^
index 567f222198..ad78528e1e 100644
--- a/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..main^
+++ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..main^
@@ -6,9 +6,8 @@ Subject: [DIFFERENT_PREFIX 0/2] *** SUBJECT HERE ***
 
 *** BLURB HERE ***
 
-A U Thor (2):
-  Second
-  Third
+  [1/2]: Second
+  [2/2]: Third
 
  dir/sub | 4 ++++
  file0   | 3 +++
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 2135b65cee..3ffccb8ee8 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -693,20 +693,10 @@ test_expect_success 'cover-letter inherits diff options' '
 	grep "file => foo .* 0 *\$" 0000-cover-letter.patch
 '
 
-cat >expect <<EOF
-  This is an excessively long subject line for a message due to the
-    habit some projects have of not having a short, one-line subject at
-    the start of the commit message, but rather sticking a whole
-    paragraph right at the start as the only thing in the commit
-    message. It had better not become the filename for the patch.
-  foo
-
-EOF
-
-test_expect_success 'shortlog of cover-letter wraps overly-long onelines' '
+test_expect_success 'cover-letter lists patches in numbered format' '
 	git format-patch --cover-letter -2 &&
-	sed -e "1,/A U Thor/d" -e "/^\$/q" 0000-cover-letter.patch >output &&
-	test_cmp expect output
+	grep "^  \[1/2\]:" 0000-cover-letter.patch &&
+	grep "^  \[2/2\]:" 0000-cover-letter.patch
 '
 
 cat >expect <<EOF
-- 
2.43.0

Re: [GSoC PATCH] format-patch: write numbered list in cover letter

[Junio C Hamano]

Pablo Sabater <pabloosabaterr@gmail.com> writes:

> Cover letter generated with 'git format-patch --cover-letter' uses
> shortlog grouping commits by author. For a single author patch grouping
> by author add useless information and makes it hard to follow patch
> references: "second patch does x and y patch does z"
>
> Replace the shortlog with a numbered list of patches:
>
>   [1/2]: first commit
>   [2/2]: second commit
>
> shortlog grouping by author is lost, both for single author patches
> and multiple author patches
>
> suggested as #leftoverbits by Junio C Hamano at
> https://lore.kernel.org/git/xmqqbjhjxp2d.fsf@gitster.g/
>
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
> I haven't tried to group by author on multiple authors patches to keep it simple 
> but I believe it's doable.

How does this compare with

  https://lore.kernel.org/git/cover.1772839973.git.mroik@delayed.space/

I wonder.

Re: [GSoC PATCH] format-patch: write numbered list in cover letter

[Pablo]

> How does this compare with
>
>   https://lore.kernel.org/git/cover.1772839973.git.mroik@delayed.space/
>
> I wonder.

I'm sorry, seems I didn't double check that it wasn't already started
by someone.
And my version is way below Mirko, I'll search for another place to help

Pablo