Merge branch 'mf/format-patch-commit-list-format' into jch

Improve the recently introduced `git format-patch
--commit-list-format` (formerly `--cover-letter-format`) option,
including a new "modern" preset and better CLI ergonomics.

* mf/format-patch-commit-list-format:
  format-patch: --commit-list-format without prefix
  format-patch: add preset for --commit-list-format
  format-patch: wrap generate_commit_list_cover()
  format.commitListFormat: strip meaning from empty
  format-patch: rename --cover-letter-format option
  format-patch: refactor generate_commit_list_cover
  pretty.c: better die message %(count) and %(total)

Author: gitster

Documentation/config/format.adoc

@@ -104,7 +104,7 @@ format.coverLetter::
 format.commitListFormat::
 	When the `--cover-letter-format` option is not given, `format-patch`
 	uses the value of this variable to decide how to format the title of
-	each commit. Default to `shortlog`.
+	each commit. Defaults to `shortlog`.
 
 format.outputDirectory::
 	Set a custom directory to store the resulting files instead of the

Documentation/git-format-patch.adoc

@@ -24,7 +24,7 @@ SYNOPSIS
 		   [(--reroll-count|-v) <n>]
 		   [--to=<email>] [--cc=<email>]
 		   [--[no-]cover-letter] [--quiet]
-		   [--cover-letter-format=<format-spec>]
+		   [--commit-list-format=<format-spec>]
 		   [--[no-]encode-email-headers]
 		   [--no-notes | --notes[=<ref>]]
 		   [--interdiff=<previous>]
@@ -323,16 +323,17 @@ feeding the result to `git send-email`.
 	containing the branch description, shortlog and the overall diffstat.  You can
 	fill in a description in the file before sending it out.
 
---cover-letter-format=<format-spec>::
-	Specify the format in which to generate the commit list of the
-	patch series. This option is available if the user wants to use
-	an alternative to the default `shortlog` format. The accepted
-	values for format-spec are "shortlog" or a format string
-	prefixed with `log:`.
+--commit-list-format=<format-spec>::
+	Specify the format in which to generate the commit list of the patch
+	series. The accepted values for format-spec are `shortlog`, `modern` or a
+	format-string prefixed with `log:`.
 	e.g. `log: %s (%an)`
-	If defined, defaults to the `format.commitListFormat` configuration
+	The user is allowed to drop the prefix if the format-string contains a
+	`%<placeholder>`.
+	If not given, defaults to the `format.commitListFormat` configuration
 	variable.
-	This option is relevant only if a cover letter is generated.
+	This option implies the use of `--cover-letter` unless
+	`--no-cover-letter` is given.
 
 --encode-email-headers::
 --no-encode-email-headers::

builtin/log.c

@@ -40,6 +40,7 @@
 #include "progress.h"
 #include "commit-slab.h"
 #include "advice.h"
+#include "utf8.h"
 
 #include "commit-reach.h"
 #include "range-diff.h"
@@ -1055,17 +1056,8 @@ static int git_format_config(const char *var, const char *value,
 		return 0;
 	}
 	if (!strcmp(var, "format.commitlistformat")) {
-		struct strbuf tmp = STRBUF_INIT;
-		strbuf_init(&tmp, 0);
-		if (value)
-			strbuf_addstr(&tmp, value);
-		else
-			strbuf_addstr(&tmp, "log:[%(count)/%(total)] %s");
-
 		FREE_AND_NULL(cfg->fmt_cover_letter_commit_list);
-		git_config_string(&cfg->fmt_cover_letter_commit_list, var, tmp.buf);
-		strbuf_release(&tmp);
-		return 0;
+		return git_config_string(&cfg->fmt_cover_letter_commit_list, var, value);
 	}
 	if (!strcmp(var, "format.outputdirectory")) {
 		FREE_AND_NULL(cfg->config_output_directory);
@@ -1373,22 +1365,26 @@ static void generate_commit_list_cover(FILE *cover_file, const char *format,
 				       struct commit **list, int n)
 {
 	struct strbuf commit_line = STRBUF_INIT;
+	struct strbuf wrapped_line = STRBUF_INIT;
 	struct pretty_print_context ctx = {0};
 	struct rev_info rev = REV_INFO_INIT;
 
-	strbuf_init(&commit_line, 0);
 	rev.total = n;
 	ctx.rev = &rev;
-	for (int i = n - 1; i >= 0; i--) {
-		rev.nr = n - i;
-		repo_format_commit_message(the_repository, list[i], format,
+	for (int i = 1; i <= n; i++) {
+		rev.nr = i;
+		repo_format_commit_message(the_repository, list[n - i], format,
 				&commit_line, &ctx);
-		fprintf(cover_file, "%s\n", commit_line.buf);
+		strbuf_add_wrapped_text(&wrapped_line, commit_line.buf, 0, 0,
+					MAIL_DEFAULT_WRAP);
+		fprintf(cover_file, "%s\n", wrapped_line.buf);
 		strbuf_reset(&commit_line);
+		strbuf_reset(&wrapped_line);
 	}
 	fprintf(cover_file, "\n");
 
 	strbuf_release(&commit_line);
+	strbuf_release(&wrapped_line);
 }
 
 static void make_cover_letter(struct rev_info *rev, int use_separate_file,
@@ -1449,6 +1445,11 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 		generate_commit_list_cover(rev->diffopt.file, format, list, nr);
 	else if (!strcmp(format, "shortlog"))
 		generate_shortlog_cover_letter(&log, rev, list, nr);
+	else if (!strcmp(format, "modern"))
+		generate_commit_list_cover(rev->diffopt.file, "[%(count)/%(total)] %s",
+					   list, nr);
+	else if (strchr(format, '%'))
+		generate_commit_list_cover(rev->diffopt.file, format, list, nr);
 	else
 		die(_("'%s' is not a valid format string"), format);
 
@@ -2015,7 +2016,7 @@ int cmd_format_patch(int argc,
 			    N_("print patches to standard out")),
 		OPT_BOOL(0, "cover-letter", &cover_letter,
 			    N_("generate a cover letter")),
-		OPT_STRING(0, "cover-letter-format", &cover_letter_fmt, N_("format-spec"),
+		OPT_STRING(0, "commit-list-format", &cover_letter_fmt, N_("format-spec"),
 			    N_("format spec used for the commit list in the cover letter")),
 		OPT_BOOL(0, "numbered-files", &just_numbers,
 			    N_("use simple number sequence for output file names")),
@@ -2359,6 +2360,8 @@ int cmd_format_patch(int argc,
 		cover_letter_fmt = cfg.fmt_cover_letter_commit_list;
 		if (!cover_letter_fmt)
 			cover_letter_fmt = "shortlog";
+	} else if (cover_letter == -1) {
+		cover_letter = 1;
 	}
 
 	if (cover_letter == -1) {

pretty.c

@@ -1551,15 +1551,15 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 
 	if (starts_with(placeholder, "(count)")) {
 		if (!c->pretty_ctx->rev)
-			die(_("this format specifier can't be used with this command"));
+			die(_("%s is not supported by this command"), "%(count)");
 		strbuf_addf(sb, "%0*d", decimal_width(c->pretty_ctx->rev->total),
 			    c->pretty_ctx->rev->nr);
 		return 7;
 	}
 
 	if (starts_with(placeholder, "(total)")) {
 		if (!c->pretty_ctx->rev)
-			die(_("this format specifier can't be used with this command"));
+			die(_("%s is not supported by this command"), "%(total)");
 		strbuf_addf(sb, "%d", c->pretty_ctx->rev->total);
 		return 7;
 	}

t/t4014-format-patch.sh

@@ -383,49 +383,73 @@ test_expect_success 'filename limit applies only to basename' '
 test_expect_success 'cover letter with subject, author and count' '
 	rm -rf patches &&
 	test_when_finished "git reset --hard HEAD~1" &&
-	test_when_finished "rm -rf patches result test_file" &&
+	test_when_finished "rm -rf patches test_file" &&
 	touch test_file &&
 	git add test_file &&
 	git commit -m "This is a subject" &&
-	git format-patch --cover-letter \
-	--cover-letter-format="log:[%(count)/%(total)] %s (%an)" -o patches HEAD~1 &&
-	grep "^\[1/1\] This is a subject (A U Thor)$" patches/0000-cover-letter.patch >result &&
-	test_line_count = 1 result
+	git format-patch --commit-list-format="log:[%(count)/%(total)] %s (%an)" \
+	-o patches HEAD~1 &&
+	test_grep "^\[1/1\] This is a subject (A U Thor)$" patches/0000-cover-letter.patch
 '
 
-test_expected_success 'cover letter with author and count' '
+test_expect_success 'cover letter with custom format no prefix' '
+	rm -rf patches &&
 	test_when_finished "git reset --hard HEAD~1" &&
-	test_when_finished "rm -rf patches result test_file" &&
+	test_when_finished "rm -rf patches test_file" &&
 	touch test_file &&
 	git add test_file &&
 	git commit -m "This is a subject" &&
-	git format-patch --cover-letter \
-	--cover-letter-format="log:[%(count)/%(total)] %an" -o patches HEAD~1 &&
-	grep "^\[1/1\] A U Thor$" patches/0000-cover-letter.patch >result &&
-	test_line_count = 1 result
+	git format-patch --commit-list-format="[%(count)/%(total)] %s (%an)" \
+	-o patches HEAD~1 &&
+	test_grep "^\[1/1\] This is a subject (A U Thor)$" patches/0000-cover-letter.patch
 '
 
-test_expect_success 'cover letter shortlog' '
+test_expect_success 'cover letter fail when no prefix and no placeholder' '
+	rm -rf patches &&
 	test_when_finished "git reset --hard HEAD~1" &&
-	test_when_finished "rm -rf patches result test_file" &&
+	test_when_finished "rm -rf patches test_file err" &&
 	touch test_file &&
 	git add test_file &&
 	git commit -m "This is a subject" &&
-	git format-patch --cover-letter --cover-letter-format=shortlog \
-	-o patches HEAD~1 &&
-	sed -n -e "/^A U Thor/p;" patches/0000-cover-letter.patch >result &&
-	test_line_count = 1 result
+	test_must_fail git format-patch --commit-list-format="this should fail" \
+	-o patches HEAD~1 2>err &&
+	test_grep "is not a valid format string" err
 '
 
-test_expect_success 'cover letter no format' '
+test_expect_success 'cover letter modern format' '
+	test_when_finished "git reset --hard HEAD~1" &&
+	test_when_finished "rm -rf patches test_file" &&
+	touch test_file &&
+	git add test_file &&
+	git commit -m "This is a subject" &&
+	git format-patch --commit-list-format="modern" -o patches HEAD~1 &&
+	test_grep "^\[1/1\] This is a subject$" patches/0000-cover-letter.patch
+'
+
+test_expect_success 'cover letter shortlog format' '
+	test_when_finished "git reset --hard HEAD~1" &&
+	test_when_finished "rm -rf expect patches result test_file" &&
+	cat >expect <<-"EOF" &&
+	A U Thor (1):
+	  This is a subject
+	EOF
+	touch test_file &&
+	git add test_file &&
+	git commit -m "This is a subject" &&
+	git format-patch --commit-list-format=shortlog -o patches HEAD~1 &&
+	grep -E -A 1 "^A U Thor \([[:digit:]]+\):$" patches/0000-cover-letter.patch >result &&
+	cat result &&
+	test_cmp expect result
+'
+
+test_expect_success 'no cover letter but with format specified' '
 	test_when_finished "git reset --hard HEAD~1" &&
 	test_when_finished "rm -rf patches result test_file" &&
 	touch test_file &&
 	git add test_file &&
 	git commit -m "This is a subject" &&
-	git format-patch --cover-letter -o patches HEAD~1 &&
-	sed -n -e "/^A U Thor/p;" patches/0000-cover-letter.patch >result &&
-	test_line_count = 1 result
+	git format-patch --no-cover-letter --commit-list-format="[%(count)] %s" -o patches HEAD~1 &&
+	test_path_is_missing patches/0000-cover-letter.patch
 '
 
 test_expect_success 'cover letter config with count, subject and author' '
@@ -450,14 +474,14 @@ test_expect_success 'cover letter config with count and author' '
 	test_line_count = 2 result
 '
 
-test_expect_success 'cover letter config commitlistformat set but no format' '
+test_expect_success 'cover letter config commitlistformat set to modern' '
 	test_when_finished "rm -rf patches result" &&
 	test_when_finished "git config unset format.coverletter" &&
 	test_when_finished "git config unset format.commitlistformat" &&
 	git config set format.coverletter true &&
-	printf "\tcommitlistformat" >> .git/config &&
+	git config set format.commitlistformat modern &&
 	git format-patch -o patches HEAD~2 &&
-	grep -E "^[[[:digit:]]+/[[:digit:]]+] .*" patches/0000-cover-letter.patch >result &&
+	grep -E "^[[[:digit:]]+/[[:digit:]]+] .*$" patches/0000-cover-letter.patch >result &&
 	test_line_count = 2 result
 '
 

t/t9902-completion.sh

@@ -2775,7 +2775,6 @@ test_expect_success PERL 'send-email' '
 	test_completion "git send-email --cov" <<-\EOF &&
 	--cover-from-description=Z
 	--cover-letter Z
-	--cover-letter-format=Z
 	EOF
 	test_completion "git send-email --val" <<-\EOF &&
 	--validate Z