improve "git format-patch --commit-list-format"
cover improve "git format-patch --commit-list-format"
This series aims to improve the --commit-list-format (former --cover-letter-format) option for format-patch by improving a bit the user interaction with the option (and its configuration variable counterpart), removing the ability to use the configuration variable in an ambiguous way (which also causes problems when interacting with it through the cli interface) and introducing a new format preset.
This series is based on top of 67006b9db8 (The 15th batch, 2026-03-12) with the following series merged into it:
- mf/format-patch-cover-letter-format at 51ed9f7e72 (docs: add usage
for the cover-letter fmt feature, 2026-03-07)
[1/7] pretty.c: better die message %(count) and %(total) (Mirko Faina) [2/7] format-patch: refactor generate_commit_list_cover (Mirko Faina) [3/7] format-patch: rename --cover-letter-format option (Mirko Faina) [4/7] format.commitListFormat: strip meaning from empty (Mirko Faina) [5/7] format-patch: wrap generate_commit_list_cover() (Mirko Faina) [6/7] format-patch: add preset for --commit-list-format (Mirko Faina) [7/7] format-patch: --commit-list-format without prefix (Mirko Faina)
Documentation/config/format.adoc | 2 +- Documentation/git-format-patch.adoc | 19 ++++---- builtin/log.c | 35 +++++++------- pretty.c | 4 +- t/t4014-format-patch.sh | 72 +++++++++++++++++++---------- t/t9902-completion.sh | 1 - 6 files changed, 80 insertions(+), 53 deletions(-)
Signature · 1 line
2.53.0.959.g497ff81fa9
PATCH 1/7 pretty.c: better die message %(count) and %(total)
Improve die messages for commands that do not support %(count) and %(total)
--- pretty.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pretty.c +2 -2
@@ -1551,7 +1551,7 @@ 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 ;
@@ -1559,7 +1559,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */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 ; }
Signature · 1 line
2.53.0.959.g497ff81fa9
PATCH 2/7 format-patch: refactor generate_commit_list_cover
In reply to Mirko FainaRefactor for readability and remove unnecessary initialization.
--- builtin/log.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
builtin/log.c +3 -4
@@ -1376,12 +1376,11 @@ static void generate_commit_list_cover(FILE *cover_file, const char *format,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_reset (&commit_line);
Signature · 1 line
2.53.0.959.g497ff81fa9
PATCH 3/7 format-patch: rename --cover-letter-format option
In reply to Mirko FainaTo align the name of the configuration variable and the name of the command line option, either one should change name. By changing the name of the option we get the added benefit of having --cover-<TAB> expand to --cover-letter without ambiguity.
If the user gives the --cover-letter-format option it would be reasonable to expect that the user wants to generate the cover letter despite not giving --cover-letter.
Rename --cover-letter-format to --commit-list-format and make it imply --cover-letter unless --no-cover-letter is given.
--- Documentation/git-format-patch.adoc | 17 ++++++------ builtin/log.c | 4 ++- t/t4014-format-patch.sh | 41 +++++++++++++++-------------- t/t9902-completion.sh | 1 - 4 files changed, 32 insertions(+), 31 deletions(-)
Documentation/git-format-patch.adoc +8 -9
@@ -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,15 @@ 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" or a format + string prefixed with `log:`. e.g. `log: %s (%an)` - If defined, defaults to the `format.commitListFormat` configuration + 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 +3 -1
@@ -2014,7 +2014,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")),
@@ -2358,6 +2358,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 ) {
t/t4014-format-patch.sh +21 -20
70 lines not on this page · raw message
t/t9902-completion.sh +0 -1
@@ -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
Signature · 1 line
2.53.0.959.g497ff81fa9
PATCH 4/7 format.commitListFormat: strip meaning from empty
In reply to Mirko FainaThe configuration variable format.commitListFormat allows for an empty value. This is unusual and can create issues when interacting with this configuration variable through the cli interface.
Strip meaning to format.commitListFormat with an empty value.
--- builtin/log.c | 11 +---------- t/t4014-format-patch.sh | 11 ----------- 2 files changed, 1 insertion(+), 21 deletions(-)
builtin/log.c +1 -10
@@ -1055,17 +1055,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);
t/t4014-format-patch.sh +0 -11
@@ -451,17 +451,6 @@ 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_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 format-patch -o patches HEAD~2 && - grep -E "^[[[:digit:]]+/[[:digit:]]+] .*" patches/0000-cover-letter.patch >result && - test_line_count = 2 result -' - test_expect_success 'cover letter config commitlistformat set to shortlog' ' test_when_finished "rm -rf patches result" && test_when_finished "git config unset format.coverletter" &&
Signature · 1 line
2.53.0.959.g497ff81fa9
PATCH 5/7 format-patch: wrap generate_commit_list_cover()
In reply to Mirko FainaWhile most conventions should not allow for the text lines in commit messages to get too long, when they do it could make emails harder to read.
Teach generate_commit_list_cover() to wrap its commit lines if they happend to be too long.
--- builtin/log.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
builtin/log.c +7 -1
@@ -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"
@@ -1364,6 +1365,7 @@ 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 ;
@@ -1373,12 +1375,16 @@ static void generate_commit_list_cover(FILE *cover_file, const char *format, 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,
Signature · 1 line
2.53.0.959.g497ff81fa9
Re: [PATCH 5/7] format-patch: wrap generate_commit_list_cover()
Quoted 6 lines · Mirko Faina
While most conventions should not allow for the text lines in commit messages to get too long, when they do it could make emails harder to read.
Teach
generate_commit_list_cover()to wrap its commit lines if they happend to be too long.
s/happend/happen/
But “happen to be” is a bit redundant. You can just say: wrap if they are too long.
Quoted 3 lines
Signed-off-by Mirko Faina <[email protected]>[snip]
Re: [PATCH 5/7] format-patch: wrap generate_commit_list_cover()
Quoted 5 lines · Kristoffer Haugsbakk
Quoted 4 lines · Mirko Faina
While most conventions should not allow for the text lines in commit messages to get too long, when they do it could make emails harder to read.
Quoted 7 lines
Quoted 2 lines
Teach
generate_commit_list_cover()to wrap its commit lines if they happend to be too long.s/happend/happen/
But “happen to be” is a bit redundant. You can just say: wrap if they are too long.
Will do, thanks
PATCH 6/7 format-patch: add preset for --commit-list-format
In reply to Mirko Faina"git format-patch --commit-list-format" enables the user to make their own format for the commit list in the cover letter. It would be nice to have a ready to use format to replace shortlog.
Teach make_cover_letter() the "modern" format preset. This new format is the same as: "log:[%(count)/%(total)] %s".
--- Documentation/config/format.adoc | 2 +- Documentation/git-format-patch.adoc | 4 ++-- builtin/log.c | 3 +++ t/t4014-format-patch.sh | 20 +++++++++++++++----- 4 files changed, 21 insertions(+), 8 deletions(-)
Documentation/config/format.adoc +1 -1
@@ -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 +2 -2
@@ -325,8 +325,8 @@ feeding the result to `git send-email`. --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" or a format - string prefixed with `log:`. + series. The accepted values for format-spec are `shortlog`, `modern` or a + format string prefixed with `log:`. e.g. `log: %s (%an)` If not given, defaults to the `format.commitListFormat` configuration variable.
builtin/log.c +3 -0
@@ -1445,6 +1445,9 @@ 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 die(_("'%s' is not a valid format string"), format);
t/t4014-format-patch.sh +15 -5
@@ -392,18 +392,17 @@ test_expect_success 'cover letter with subject, author and count' ' test_grep "^\[1/1\] This is a subject (A U Thor)$" patches/0000-cover-letter.patch ' -test_expect_success 'cover letter with author and count' ' +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="log:[%(count)/%(total)] %an" \ - -o patches HEAD~1 && - test_grep "^\[1/1\] A U Thor$" patches/0000-cover-letter.patch + 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' ' +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" &&
@@ -451,6 +450,17 @@ test_expect_success 'cover letter config with count and author' ' test_line_count = 2 result ' +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 && + git config set format.commitlistformat modern && + git format-patch -o patches HEAD~2 && + grep -E "^[[[:digit:]]+/[[:digit:]]+] .*$" patches/0000-cover-letter.patch >result && + test_line_count = 2 result +' + test_expect_success 'cover letter config commitlistformat set to shortlog' ' test_when_finished "rm -rf patches result" && test_when_finished "git config unset format.coverletter" &&
Signature · 1 line
2.53.0.959.g497ff81fa9
PATCH 7/7 format-patch: --commit-list-format without prefix
In reply to Mirko FainaHaving to prefix a custom format-string with "log:" when passed from the cli interface can be annoying for many users. It would be great if it could be dropped an it were still accepted.
Teach make_cover_letter() to accept custom format-strings if a placeholder is detected.
Note that both here and in "git log --format" the check is done naively by just checking for the presence of a '%'.
--- Documentation/git-format-patch.adoc | 4 +++- builtin/log.c | 2 ++ t/t4014-format-patch.sh | 24 ++++++++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-)
Documentation/git-format-patch.adoc +3 -1
@@ -326,8 +326,10 @@ feeding the result to `git send-email`. --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:`. + format-string prefixed with `log:`. e.g. `log: %s (%an)` + 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 implies the use of `--cover-letter` unless
builtin/log.c +2 -0
@@ -1448,6 +1448,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,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);
t/t4014-format-patch.sh +24 -0
@@ -392,6 +392,30 @@ test_expect_success 'cover letter with subject, author and count' ' test_grep "^\[1/1\] This is a subject (A U Thor)$" patches/0000-cover-letter.patch ' +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 test_file" && + touch test_file && + git add test_file && + git commit -m "This is a subject" && + 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 fail when no prefix and no placeholder' ' + rm -rf patches && + test_when_finished "git reset --hard HEAD~1" && + test_when_finished "rm -rf patches test_file err" && + touch test_file && + git add test_file && + git commit -m "This is a subject" && + 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 modern format' ' test_when_finished "git reset --hard HEAD~1" && test_when_finished "rm -rf patches test_file" &&
Signature · 1 line
2.53.0.959.g497ff81fa9
Re: [PATCH 7/7] format-patch: --commit-list-format without prefix
Quoted 2 lines · Mirko Faina
Having to prefix a custom format-string with "log:" when passed from the cli interface can be annoying for many users. It would be great if it
s/cli interface/CLI/
s/for many users// ? It’s a general assertion.
Quoted 1 line
could be dropped an it were still accepted.
s/an/and/
But maybe instead:
I would be great if this prefix wasn't required.
Quoted 10 lines
Teach
make_cover_letter()to accept custom format-strings if a placeholder is detected.Note that both here and in "git log --format" the check is done naively by just checking for the presence of a '%'.
Signed-off-by Mirko Faina <[email protected]>---
[snip]
Re: [PATCH 7/7] format-patch: --commit-list-format without prefix
Quoted 8 lines · Kristoffer Haugsbakk
Quoted 2 lines · Mirko Faina
Having to prefix a custom format-string with "log:" when passed from the cli interface can be annoying for many users. It would be great if it
s/cli interface/CLI/
s/for many users// ? It’s a general assertion.
Quoted 7 lines
Quoted 1 line
could be dropped an it were still accepted.
s/an/and/
But maybe instead:
I would be great if this prefix wasn't required.
s/I/It/ :P
Will do, thank you
v2 cover improve "git format-patch --commit-list-format"
In reply to Mirko FainaI've applied the suggestions that Kristoffer made and added an additional patch that documents %(count) and %(total), but I'm not sure if I've placed them in the right location. It's in the "Placeholders that expand to information extracted from the commit" section despite not being the most accurate, but making a new section just for "Placeholders that expand to information from patch series" didn't seem right.
This series is based on top of 67006b9db8 (The 15th batch, 2026-03-12) with the following series merged into it:
- mf/format-patch-cover-letter-format at 51ed9f7e72 (docs: add usage
for the cover-letter fmt feature, 2026-03-07)
[1/8] pretty.c: better die message %(count) and %(total) (Mirko Faina) [2/8] format-patch: refactor generate_commit_list_cover (Mirko Faina) [3/8] format-patch: rename --cover-letter-format option (Mirko Faina) [4/8] docs/pretty-formats: add %(count) and %(total) (Mirko Faina) [5/8] format.commitListFormat: strip meaning from empty (Mirko Faina) [6/8] format-patch: wrap generate_commit_list_cover() (Mirko Faina) [7/8] format-patch: add preset for --commit-list-format (Mirko Faina) [8/8] format-patch: --commit-list-format without prefix (Mirko Faina)
Documentation/config/format.adoc | 2 +- Documentation/git-format-patch.adoc | 19 ++++---- Documentation/pretty-formats.adoc | 4 ++ builtin/log.c | 35 +++++++------- pretty.c | 4 +- t/t4014-format-patch.sh | 72 +++++++++++++++++++---------- t/t9902-completion.sh | 1 - 7 files changed, 84 insertions(+), 53 deletions(-)
Signature · 1 line
2.53.0.1018.g2bb0e51243
PATCH v2 1/8 pretty.c: better die message %(count) and %(total)
Improve die messages for commands that do not support %(count) and %(total)
--- pretty.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pretty.c +2 -2
@@ -1551,7 +1551,7 @@ 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 ;
@@ -1559,7 +1559,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */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 ; }
Signature · 1 line
2.53.0.1018.g2bb0e51243
PATCH v2 2/8 format-patch: refactor generate_commit_list_cover
In reply to Mirko FainaRefactor for readability and remove unnecessary initialization.
--- builtin/log.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
builtin/log.c +3 -4
@@ -1376,12 +1376,11 @@ static void generate_commit_list_cover(FILE *cover_file, const char *format,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_reset (&commit_line);
Signature · 1 line
2.53.0.1018.g2bb0e51243
PATCH v2 3/8 format-patch: rename --cover-letter-format option
In reply to Mirko FainaTo align the name of the configuration variable and the name of the command line option, either one should change name. By changing the name of the option we get the added benefit of having --cover-<TAB> expand to --cover-letter without ambiguity.
If the user gives the --cover-letter-format option it would be reasonable to expect that the user wants to generate the cover letter despite not giving --cover-letter.
Rename --cover-letter-format to --commit-list-format and make it imply --cover-letter unless --no-cover-letter is given.
--- Documentation/git-format-patch.adoc | 17 ++++++------ builtin/log.c | 4 ++- t/t4014-format-patch.sh | 41 +++++++++++++++-------------- t/t9902-completion.sh | 1 - 4 files changed, 32 insertions(+), 31 deletions(-)
Documentation/git-format-patch.adoc +8 -9
@@ -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,15 @@ 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" or a format + string prefixed with `log:`. e.g. `log: %s (%an)` - If defined, defaults to the `format.commitListFormat` configuration + 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 +3 -1
@@ -2014,7 +2014,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")),
@@ -2358,6 +2358,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 ) {
t/t4014-format-patch.sh +21 -20
70 lines not on this page · raw message
t/t9902-completion.sh +0 -1
@@ -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
Signature · 1 line
2.53.0.1018.g2bb0e51243
PATCH v2 4/8 docs/pretty-formats: add %(count) and %(total)
In reply to Mirko FainaWhen --commit-list-format has been introduced to format-patch, two new placeholders have been added to the PRETTY FORMATS code without being documented. Do so now.
--- Documentation/pretty-formats.adoc | 4 ++++ 1 file changed, 4 insertions(+)
Documentation/pretty-formats.adoc +4 -0
@@ -253,6 +253,10 @@ The placeholders are: linkgit:git-rev-list[1]) +%d+:: ref names, like the --decorate option of linkgit:git-log[1] +%D+:: ref names without the " (", ")" wrapping. ++%(count)+:: the number of a patch within a patch series. Used only in + `--commit-list-format` in `format-patch` ++%(total)+:: the number of tatal patches in a patch series. Used only in + `--commit-list-format` in `format-patch` ++%(decorate++`[:<option>,...]`++)++:: ref names with custom decorations. The `decorate` string may be followed by a colon and zero or more comma-separated options. Option values may contain
Signature · 1 line
2.53.0.1018.g2bb0e51243
Re: [PATCH v2 4/8] docs/pretty-formats: add %(count) and %(total)
Quoted 1 line · Mirko Faina
When --commit-list-format has been introduced to format-patch, two new
The past tense here is wrong.
s/has been/was/
Quoted 1 line
placeholders have been added to the PRETTY FORMATS code without being
s/have been/were/
Quoted 13 lines
documented. Do so now.
Signed-off-by Mirko Faina <[email protected]>--- Documentation/pretty-formats.adoc | 4 ++++ 1 file changed, 4 insertions(+)Documentation/pretty-formats.adoc +0
-0
Documentation/pretty-formats.adoc The placeholders are: · line 258
Quoted 6 lines
Documentation/pretty-formats.adoc +3
-0@@ -253,6 +253,10 @@ The placeholders are: linkgit:git-rev-list[1]) +%d+:: ref names, like the --decorate option of linkgit:git-log[1] +%D+:: ref names without the " (", ")" wrapping. ++%(count)+:: the number of a patch within a patch series. Used only in + `--commit-list-format` in `format-patch` ++%(total)+:: the number of tatal patches in a patch series. Used only
s/the number of tatal patches/the total number of patches/ ?
Quoted 9 lines
in
Documentation/pretty-formats.adoc +1
-0+ `--commit-list-format` in `format-patch` ++%(decorate++`[:<option>,...]`++)++:: ref names with custom decorations. The `decorate` string may befollowed by a
colon and zero or more comma-separated options. Option values may
contain
--
2.53.0.1018.g2bb0e51243
Re: [PATCH v2 4/8] docs/pretty-formats: add %(count) and %(total)
Quoted 10 lines · Kristoffer Haugsbakk
Quoted 1 line · Mirko Faina
When --commit-list-format has been introduced to format-patch, two new
The past tense here is wrong.
s/has been/was/
Quoted 1 line
placeholders have been added to the PRETTY FORMATS code without being
s/have been/were/
Will fix.
Quoted 7 lines
Quoted 7 lines
documented. Do so now.
Signed-off-by Mirko Faina <[email protected]>--- Documentation/pretty-formats.adoc | 4 ++++ 1 file changed, 4 insertions(+)
Quoted 14 lines
Documentation/pretty-formats.adoc The placeholders are: · line 258
Quoted 12 lines
Documentation/pretty-formats.adoc +3
-0@@ -253,6 +253,10 @@ The placeholders are: linkgit:git-rev-list[1]) +%d+:: ref names, like the --decorate option of linkgit:git-log[1] +%D+:: ref names without the " (", ")" wrapping. ++%(count)+:: the number of a patch within a patch series. Used only in + `--commit-list-format` in `format-patch` ++%(total)+:: the number of tatal patches in a patch series. Used onlys/the number of tatal patches/the total number of patches/ ?
Will fix.
Thank you for the review
PATCH v2 5/8 format.commitListFormat: strip meaning from empty
In reply to Mirko FainaThe configuration variable format.commitListFormat allows for an empty value. This is unusual and can create issues when interacting with this configuration variable through the cli interface.
Strip meaning to format.commitListFormat with an empty value.
--- builtin/log.c | 11 +---------- t/t4014-format-patch.sh | 11 ----------- 2 files changed, 1 insertion(+), 21 deletions(-)
builtin/log.c +1 -10
@@ -1055,17 +1055,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);
t/t4014-format-patch.sh +0 -11
@@ -451,17 +451,6 @@ 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_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 format-patch -o patches HEAD~2 && - grep -E "^[[[:digit:]]+/[[:digit:]]+] .*" patches/0000-cover-letter.patch >result && - test_line_count = 2 result -' - test_expect_success 'cover letter config commitlistformat set to shortlog' ' test_when_finished "rm -rf patches result" && test_when_finished "git config unset format.coverletter" &&
Signature · 1 line
2.53.0.1018.g2bb0e51243
PATCH v2 6/8 format-patch: wrap generate_commit_list_cover()
In reply to Mirko FainaWhile most conventions should not allow for the text lines in commit messages to get too long, when they do it could make emails harder to read.
Teach generate_commit_list_cover() to wrap its commit lines if they are too long.
--- builtin/log.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
builtin/log.c +7 -1
@@ -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"
@@ -1364,6 +1365,7 @@ 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 ;
@@ -1373,12 +1375,16 @@ static void generate_commit_list_cover(FILE *cover_file, const char *format, 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,
Signature · 1 line
2.53.0.1018.g2bb0e51243
PATCH v2 7/8 format-patch: add preset for --commit-list-format
In reply to Mirko Faina"git format-patch --commit-list-format" enables the user to make their own format for the commit list in the cover letter. It would be nice to have a ready to use format to replace shortlog.
Teach make_cover_letter() the "modern" format preset. This new format is the same as: "log:[%(count)/%(total)] %s".
--- Documentation/config/format.adoc | 2 +- Documentation/git-format-patch.adoc | 4 ++-- builtin/log.c | 3 +++ t/t4014-format-patch.sh | 20 +++++++++++++++----- 4 files changed, 21 insertions(+), 8 deletions(-)
Documentation/config/format.adoc +1 -1
@@ -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 +2 -2
@@ -325,8 +325,8 @@ feeding the result to `git send-email`. --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" or a format - string prefixed with `log:`. + series. The accepted values for format-spec are `shortlog`, `modern` or a + format string prefixed with `log:`. e.g. `log: %s (%an)` If not given, defaults to the `format.commitListFormat` configuration variable.
builtin/log.c +3 -0
@@ -1445,6 +1445,9 @@ 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 die(_("'%s' is not a valid format string"), format);
t/t4014-format-patch.sh +15 -5
@@ -392,18 +392,17 @@ test_expect_success 'cover letter with subject, author and count' ' test_grep "^\[1/1\] This is a subject (A U Thor)$" patches/0000-cover-letter.patch ' -test_expect_success 'cover letter with author and count' ' +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="log:[%(count)/%(total)] %an" \ - -o patches HEAD~1 && - test_grep "^\[1/1\] A U Thor$" patches/0000-cover-letter.patch + 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' ' +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" &&
@@ -451,6 +450,17 @@ test_expect_success 'cover letter config with count and author' ' test_line_count = 2 result ' +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 && + git config set format.commitlistformat modern && + git format-patch -o patches HEAD~2 && + grep -E "^[[[:digit:]]+/[[:digit:]]+] .*$" patches/0000-cover-letter.patch >result && + test_line_count = 2 result +' + test_expect_success 'cover letter config commitlistformat set to shortlog' ' test_when_finished "rm -rf patches result" && test_when_finished "git config unset format.coverletter" &&
Signature · 1 line
2.53.0.1018.g2bb0e51243
PATCH v2 8/8 format-patch: --commit-list-format without prefix
In reply to Mirko FainaHaving to prefix a custom format-string with "log:" when passed from the CLI can be annoying. It would be great if this prefix wasn't required.
Teach make_cover_letter() to accept custom format-strings without the "log:" prefix if a placeholder is detected.
Note that both here and in "git log --format" the check is done naively by just checking for the presence of a '%'.
--- Documentation/git-format-patch.adoc | 4 +++- builtin/log.c | 2 ++ t/t4014-format-patch.sh | 24 ++++++++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-)
Documentation/git-format-patch.adoc +3 -1
@@ -326,8 +326,10 @@ feeding the result to `git send-email`. --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:`. + format-string prefixed with `log:`. e.g. `log: %s (%an)` + 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 implies the use of `--cover-letter` unless
builtin/log.c +2 -0
@@ -1448,6 +1448,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,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);
t/t4014-format-patch.sh +24 -0
@@ -392,6 +392,30 @@ test_expect_success 'cover letter with subject, author and count' ' test_grep "^\[1/1\] This is a subject (A U Thor)$" patches/0000-cover-letter.patch ' +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 test_file" && + touch test_file && + git add test_file && + git commit -m "This is a subject" && + 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 fail when no prefix and no placeholder' ' + rm -rf patches && + test_when_finished "git reset --hard HEAD~1" && + test_when_finished "rm -rf patches test_file err" && + touch test_file && + git add test_file && + git commit -m "This is a subject" && + 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 modern format' ' test_when_finished "git reset --hard HEAD~1" && test_when_finished "rm -rf patches test_file" &&
Signature · 1 line
2.53.0.1018.g2bb0e51243
v3 cover improve "git format-patch --commit-list-format"
In reply to Mirko FainaNot much has changed, just applied the suggestions Kristoffer made. Thank you again for the review
[1/8] pretty.c: better die message %(count) and %(total) (Mirko Faina) [2/8] format-patch: refactor generate_commit_list_cover (Mirko Faina) [3/8] format-patch: rename --cover-letter-format option (Mirko Faina) [4/8] docs/pretty-formats: add %(count) and %(total) (Mirko Faina) [5/8] format.commitListFormat: strip meaning from empty (Mirko Faina) [6/8] format-patch: wrap generate_commit_list_cover() (Mirko Faina) [7/8] format-patch: add preset for --commit-list-format (Mirko Faina) [8/8] format-patch: --commit-list-format without prefix (Mirko Faina)
Documentation/config/format.adoc | 2 +- Documentation/git-format-patch.adoc | 19 ++++---- Documentation/pretty-formats.adoc | 4 ++ builtin/log.c | 35 +++++++------- pretty.c | 4 +- t/t4014-format-patch.sh | 72 +++++++++++++++++++---------- t/t9902-completion.sh | 1 - 7 files changed, 84 insertions(+), 53 deletions(-)
Range-diff against v2:
1: a0d26c5999 = 1: a0d26c5999 pretty.c: better die message %(count) and %(total) 2: 883dd358b6 = 2: 883dd358b6 format-patch: refactor generate_commit_list_cover 3: 5d061d6398 = 3: 5d061d6398 format-patch: rename --cover-letter-format option 4: 7b1e5cbb24 ! 4: ce7d1bd1fe docs/pretty-formats: add %(count) and %(total)
@@ Metadata
## Commit message ##
docs/pretty-formats: add %(count) and %(total)
- When --commit-list-format has been introduced to format-patch, two new
- placeholders have been added to the PRETTY FORMATS code without being
+ When --commit-list-format was introduced to format-patch, two new
+ placeholders were added to the PRETTY FORMATS code without being
documented. Do so now.
@@ Documentation/pretty-formats.adoc: The placeholders are:
+%D+:: ref names without the " (", ")" wrapping.
++%(count)+:: the number of a patch within a patch series. Used only in
+ --commit-list-format in format-patch
-++%(total)+:: the number of tatal patches in a patch series. Used only in +++%(total)+:: the total number of patches in a patch series. Used only in
+ --commit-list-format in format-patch
++%(decorate++[:<option>,...]++)++::
ref names with custom decorations. The decorate string may be followed by a
5: 3cb0a0a088 ! 5: 66d30249bb format.commitListFormat: strip meaning from empty
@@ Commit message
The configuration variable format.commitListFormat allows for an empty value. This is unusual and can create issues when interacting with this
- configuration variable through the cli interface.
+ configuration variable through the CLI.
Strip meaning to format.commitListFormat with an empty value.
6: 3f547451a2 = 6: c722fc9d0f format-patch: wrap generate_commit_list_cover() 7: fdcb68e650 = 7: bdd8f1fb57 format-patch: add preset for --commit-list-format 8: ef0d3ed876 = 8: d125458e33 format-patch: --commit-list-format without prefix
Signature · 1 line
2.53.0.1118.gaef5881109
PATCH v3 1/8 pretty.c: better die message %(count) and %(total)
Improve die messages for commands that do not support %(count) and %(total)
--- pretty.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pretty.c +2 -2
@@ -1551,7 +1551,7 @@ 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 ;
@@ -1559,7 +1559,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */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 ; }
Signature · 1 line
2.53.0.1118.gaef5881109
PATCH v3 2/8 format-patch: refactor generate_commit_list_cover
In reply to Mirko FainaRefactor for readability and remove unnecessary initialization.
--- builtin/log.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
builtin/log.c +3 -4
@@ -1376,12 +1376,11 @@ static void generate_commit_list_cover(FILE *cover_file, const char *format,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_reset (&commit_line);
Signature · 1 line
2.53.0.1118.gaef5881109
PATCH v3 3/8 format-patch: rename --cover-letter-format option
In reply to Mirko FainaTo align the name of the configuration variable and the name of the command line option, either one should change name. By changing the name of the option we get the added benefit of having --cover-<TAB> expand to --cover-letter without ambiguity.
If the user gives the --cover-letter-format option it would be reasonable to expect that the user wants to generate the cover letter despite not giving --cover-letter.
Rename --cover-letter-format to --commit-list-format and make it imply --cover-letter unless --no-cover-letter is given.
--- Documentation/git-format-patch.adoc | 17 ++++++------ builtin/log.c | 4 ++- t/t4014-format-patch.sh | 41 +++++++++++++++-------------- t/t9902-completion.sh | 1 - 4 files changed, 32 insertions(+), 31 deletions(-)
Documentation/git-format-patch.adoc +8 -9
@@ -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,15 @@ 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" or a format + string prefixed with `log:`. e.g. `log: %s (%an)` - If defined, defaults to the `format.commitListFormat` configuration + 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 +3 -1
@@ -2014,7 +2014,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")),
@@ -2358,6 +2358,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 ) {
t/t4014-format-patch.sh +21 -20
70 lines not on this page · raw message
t/t9902-completion.sh +0 -1
@@ -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
Signature · 1 line
2.53.0.1118.gaef5881109
PATCH v3 4/8 docs/pretty-formats: add %(count) and %(total)
In reply to Mirko FainaWhen --commit-list-format was introduced to format-patch, two new placeholders were added to the PRETTY FORMATS code without being documented. Do so now.
--- Documentation/pretty-formats.adoc | 4 ++++ 1 file changed, 4 insertions(+)
Documentation/pretty-formats.adoc +4 -0
@@ -253,6 +253,10 @@ The placeholders are: linkgit:git-rev-list[1]) +%d+:: ref names, like the --decorate option of linkgit:git-log[1] +%D+:: ref names without the " (", ")" wrapping. ++%(count)+:: the number of a patch within a patch series. Used only in + `--commit-list-format` in `format-patch` ++%(total)+:: the total number of patches in a patch series. Used only in + `--commit-list-format` in `format-patch` ++%(decorate++`[:<option>,...]`++)++:: ref names with custom decorations. The `decorate` string may be followed by a colon and zero or more comma-separated options. Option values may contain
Signature · 1 line
2.53.0.1118.gaef5881109
PATCH v3 5/8 format.commitListFormat: strip meaning from empty
In reply to Mirko FainaThe configuration variable format.commitListFormat allows for an empty value. This is unusual and can create issues when interacting with this configuration variable through the CLI.
Strip meaning to format.commitListFormat with an empty value.
--- builtin/log.c | 11 +---------- t/t4014-format-patch.sh | 11 ----------- 2 files changed, 1 insertion(+), 21 deletions(-)
builtin/log.c +1 -10
@@ -1055,17 +1055,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);
t/t4014-format-patch.sh +0 -11
@@ -451,17 +451,6 @@ 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_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 format-patch -o patches HEAD~2 && - grep -E "^[[[:digit:]]+/[[:digit:]]+] .*" patches/0000-cover-letter.patch >result && - test_line_count = 2 result -' - test_expect_success 'cover letter config commitlistformat set to shortlog' ' test_when_finished "rm -rf patches result" && test_when_finished "git config unset format.coverletter" &&
Signature · 1 line
2.53.0.1118.gaef5881109
PATCH v3 6/8 format-patch: wrap generate_commit_list_cover()
In reply to Mirko FainaWhile most conventions should not allow for the text lines in commit messages to get too long, when they do it could make emails harder to read.
Teach generate_commit_list_cover() to wrap its commit lines if they are too long.
--- builtin/log.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
builtin/log.c +7 -1
@@ -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"
@@ -1364,6 +1365,7 @@ 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 ;
@@ -1373,12 +1375,16 @@ static void generate_commit_list_cover(FILE *cover_file, const char *format, 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,
Signature · 1 line
2.53.0.1118.gaef5881109
PATCH v3 7/8 format-patch: add preset for --commit-list-format
In reply to Mirko Faina"git format-patch --commit-list-format" enables the user to make their own format for the commit list in the cover letter. It would be nice to have a ready to use format to replace shortlog.
Teach make_cover_letter() the "modern" format preset. This new format is the same as: "log:[%(count)/%(total)] %s".
--- Documentation/config/format.adoc | 2 +- Documentation/git-format-patch.adoc | 4 ++-- builtin/log.c | 3 +++ t/t4014-format-patch.sh | 20 +++++++++++++++----- 4 files changed, 21 insertions(+), 8 deletions(-)
Documentation/config/format.adoc +1 -1
@@ -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 +2 -2
@@ -325,8 +325,8 @@ feeding the result to `git send-email`. --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" or a format - string prefixed with `log:`. + series. The accepted values for format-spec are `shortlog`, `modern` or a + format string prefixed with `log:`. e.g. `log: %s (%an)` If not given, defaults to the `format.commitListFormat` configuration variable.
builtin/log.c +3 -0
@@ -1445,6 +1445,9 @@ 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 die(_("'%s' is not a valid format string"), format);
t/t4014-format-patch.sh +15 -5
@@ -392,18 +392,17 @@ test_expect_success 'cover letter with subject, author and count' ' test_grep "^\[1/1\] This is a subject (A U Thor)$" patches/0000-cover-letter.patch ' -test_expect_success 'cover letter with author and count' ' +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="log:[%(count)/%(total)] %an" \ - -o patches HEAD~1 && - test_grep "^\[1/1\] A U Thor$" patches/0000-cover-letter.patch + 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' ' +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" &&
@@ -451,6 +450,17 @@ test_expect_success 'cover letter config with count and author' ' test_line_count = 2 result ' +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 && + git config set format.commitlistformat modern && + git format-patch -o patches HEAD~2 && + grep -E "^[[[:digit:]]+/[[:digit:]]+] .*$" patches/0000-cover-letter.patch >result && + test_line_count = 2 result +' + test_expect_success 'cover letter config commitlistformat set to shortlog' ' test_when_finished "rm -rf patches result" && test_when_finished "git config unset format.coverletter" &&
Signature · 1 line
2.53.0.1118.gaef5881109
PATCH v3 8/8 format-patch: --commit-list-format without prefix
In reply to Mirko FainaHaving to prefix a custom format-string with "log:" when passed from the CLI can be annoying. It would be great if this prefix wasn't required.
Teach make_cover_letter() to accept custom format-strings without the "log:" prefix if a placeholder is detected.
Note that both here and in "git log --format" the check is done naively by just checking for the presence of a '%'.
--- Documentation/git-format-patch.adoc | 4 +++- builtin/log.c | 2 ++ t/t4014-format-patch.sh | 24 ++++++++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-)
Documentation/git-format-patch.adoc +3 -1
@@ -326,8 +326,10 @@ feeding the result to `git send-email`. --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:`. + format-string prefixed with `log:`. e.g. `log: %s (%an)` + 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 implies the use of `--cover-letter` unless
builtin/log.c +2 -0
@@ -1448,6 +1448,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,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);
t/t4014-format-patch.sh +24 -0
@@ -392,6 +392,30 @@ test_expect_success 'cover letter with subject, author and count' ' test_grep "^\[1/1\] This is a subject (A U Thor)$" patches/0000-cover-letter.patch ' +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 test_file" && + touch test_file && + git add test_file && + git commit -m "This is a subject" && + 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 fail when no prefix and no placeholder' ' + rm -rf patches && + test_when_finished "git reset --hard HEAD~1" && + test_when_finished "rm -rf patches test_file err" && + touch test_file && + git add test_file && + git commit -m "This is a subject" && + 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 modern format' ' test_when_finished "git reset --hard HEAD~1" && test_when_finished "rm -rf patches test_file" &&
Signature · 1 line
2.53.0.1118.gaef5881109
Quoted 12 lines · Mirko Faina
Not much has changed, just applied the suggestions Kristoffer made. Thank you again for the review
[1/8]
pretty.c: better die message %(count) and %(total) (Mirko Faina) [2/8] format-patch: refactorgenerate_commit_list_cover(Mirko Faina) [3/8] format-patch: rename --cover-letter-format option (Mirko Faina) [4/8] docs/pretty-formats: add %(count) and %(total) (Mirko Faina) [5/8] format.commitListFormat: strip meaning from empty (Mirko Faina) [6/8] format-patch: wrapgenerate_commit_list_cover()(Mirko Faina) [7/8] format-patch: add preset for --commit-list-format (Mirko Faina) [8/8] format-patch: --commit-list-format without prefix (Mirko Faina)
Quoted 8 lines
Documentation/config/format.adoc | 2 +- Documentation/git-format-patch.adoc | 19 ++++---- Documentation/pretty-formats.adoc | 4 ++ builtin/log.c | 35 +++++++------- pretty.c | 4 +- t/t4014-format-patch.sh | 72 +++++++++++++++++++---------- t/t9902-completion.sh | 1 - 7 files changed, 84 insertions(+), 53 deletions(-)
All incremental changes look reasonable to me, and it seems we have already reached the point of diminishing returns?
It is possible that people are only commenting on low-hanging obvious typoes and mistakes without seeing a bigger picture, but I think I've read through an earlier iteration of the series, and found it more-or-less solid, and I do not think there was a drastic change of course since then, so I am happy to mark the topic for 'next' now.
Unless other people find bigger issues remaining in the series, that is, of course ;-)
Thanks.
Quoted 8 lines · Junio C Hamano
Quoted 2 lines
[snip]
7 files changed, 84 insertions(+), 53 deletions(-)All incremental changes look reasonable to me, and it seems we have already reached the point of diminishing returns?
It is possible that people are only commenting on low-hanging obvious typoes and mistakes without seeing a bigger picture,
All my comments here are from spotting things accidentally and are not in any way holistic assessments. ;)
Quoted 7 lines
but I think I've read through an earlier iteration of the series, and found it more-or-less solid, and I do not think there was a drastic change of course since then, so I am happy to mark the topic for 'next' now.
Unless other people find bigger issues remaining in the series, that is, of course ;-)
Quoted 33 lines · Junio C Hamano
Quoted 12 lines · Mirko Faina
Not much has changed, just applied the suggestions Kristoffer made. Thank you again for the review
[1/8]
pretty.c: better die message %(count) and %(total) (Mirko Faina) [2/8] format-patch: refactorgenerate_commit_list_cover(Mirko Faina) [3/8] format-patch: rename --cover-letter-format option (Mirko Faina) [4/8] docs/pretty-formats: add %(count) and %(total) (Mirko Faina) [5/8] format.commitListFormat: strip meaning from empty (Mirko Faina) [6/8] format-patch: wrapgenerate_commit_list_cover()(Mirko Faina) [7/8] format-patch: add preset for --commit-list-format (Mirko Faina) [8/8] format-patch: --commit-list-format without prefix (Mirko Faina)Quoted 8 lines
Documentation/config/format.adoc | 2 +- Documentation/git-format-patch.adoc | 19 ++++---- Documentation/pretty-formats.adoc | 4 ++ builtin/log.c | 35 +++++++------- pretty.c | 4 +- t/t4014-format-patch.sh | 72 +++++++++++++++++++---------- t/t9902-completion.sh | 1 - 7 files changed, 84 insertions(+), 53 deletions(-)All incremental changes look reasonable to me, and it seems we have already reached the point of diminishing returns?
It is possible that people are only commenting on low-hanging obvious typoes and mistakes without seeing a bigger picture, but I think I've read through an earlier iteration of the series, and found it more-or-less solid, and I do not think there was a drastic change of course since then, so I am happy to mark the topic for 'next' now.
Quoted 2 lines
Unless other people find bigger issues remaining in the series, that is, of course ;-)
I've just had a look at what you merged to next and it looks sound to me. The tweaks to the new feature in this series are very welcome improvements.
Thanks
Phillip
Quoted 2 lines
Thanks.
Quoted 11 lines · Phillip Wood
Quoted 10 lines · Junio C Hamano
Quoted 8 lines · Mirko Faina
Not much has changed, just applied the suggestions Kristoffer made. Thank you again for the review
...
[7/8] format-patch: add preset for --commit-list-format (Mirko Faina) [8/8] format-patch: --commit-list-format without prefix (Mirko Faina)
...Documentation/config/format.adoc | 2 +- Documentation/git-format-patch.adoc | 19 ++++----
Quoted 3 lines
I've just had a look at what you merged to next and it looks sound to me. The tweaks to the new feature in this series are very welcome improvements.
Yup, I tried "modern" and generally liked it very much.
The appearance of the list looked a bit odd that each element in a list of things with heading was shown without any indentation before the heading, though. I'd probably use
--commit-list-format=" %(count): %<(72,trunc)%s"
or something like that myself.
I seem to be getting spurious blank lines between the lines with the above, when I lengthen and shorten 72 in the format string above, though. I haven't figured out what is broken, though.
Quoted 6 lines · Junio C Hamano
Yup, I tried "modern" and generally liked it very much.
The appearance of the list looked a bit odd that each element in a list of things with heading was shown without any indentation before the heading, though. I'd probably use
Quoted 7 lines
--commit-list-format=" %(count): %<(72,trunc)%s"or something like that myself.
I seem to be getting spurious blank lines between the lines with the above, when I lengthen and shorten 72 in the format string above, though. I haven't figured out what is broken, though.
Could this be because of strbuf_add_wrapped_text()? In a previous review someone pointed out that there was no wrapping, so now each entry goes through a pass of strbuf_add_wrapped_text() which should wrap when the line gets longer than MAIL_DEFAULT_WRAP (72 characters). Since you guarantee at most 72 chars for %s, if it ever gets to that the line would be at least 76 chars long due to " n: ".
Quoted 9 lines · Mirko Faina
Quoted 8 lines · Junio C Hamano
Yup, I tried "modern" and generally liked it very much.
The appearance of the list looked a bit odd that each element in a list of things with heading was shown without any indentation before the heading, though. I'd probably use
--commit-list-format=" %(count): %<(72,trunc)%s"
Quoted 7 lines
Quoted 5 lines
or something like that myself.
I seem to be getting spurious blank lines between the lines with the above, when I lengthen and shorten 72 in the format string above, though. I haven't figured out what is broken, though.
Could this be because of
strbuf_add_wrapped_text()?
Ahh, I think that would explain it. In general, I think it is a mistake to use strbuf_addwrapped_text() for any end-user configurable output. After all, wouldn't %w(w,i1,i2) work in the format string?
Quoted 2 lines
In a previous review
someone pointed out that there was no wrapping,
When the payload _can_ be specified to wrap (i.e., end-user configurable output format), the wrapping should not be forced by the mechanism. A project that is not ours may want to keep a single long line for their commit list entries.
I do not mind if the default "modern" were defined to include %w() to force wrapping to those who follow the default, of course. But do not unconditionally wrap what the end-user formatted to their liking.
Quoted 9 lines · Junio C Hamano
When the payload _can_ be specified to wrap (i.e., end-user configurable output format), the wrapping should not be forced by the mechanism. A project that is not ours may want to keep a single long line for their commit list entries.
I do not mind if the default "modern" were defined to include %w() to force wrapping to those who follow the default, of course. But do not unconditionally wrap what the end-user formatted to their liking.
I see, since there's not much to edit, I'll send the changes regarding the docs and the wrapping in a single series (I saw that you already made mf/format-patch-commit-list-format-doc but I hope it is not a problem), instead of submitting a new one.
Quoted 11 lines · Mirko Faina
Quoted 9 lines · Junio C Hamano
When the payload _can_ be specified to wrap (i.e., end-user configurable output format), the wrapping should not be forced by the mechanism. A project that is not ours may want to keep a single long line for their commit list entries.
I do not mind if the default "modern" were defined to include %w() to force wrapping to those who follow the default, of course. But do not unconditionally wrap what the end-user formatted to their liking.
Quoted 4 lines
I see, since there's not much to edit, I'll send the changes regarding the docs and the wrapping in a single series (I saw that you already made mf/format-patch-commit-list-format-doc but I hope it is not a problem), instead of submitting a new one.
I do not quite know what you are planning to do with the above four lines, but you do not have to answer me here, as we'll know soon enough once you post them. I'll just have to remember *not* to merge the new -doc topic to 'next' until that happens ;-).
Quoted 4 lines · Mirko Faina
Not much has changed, just applied the suggestions Kristoffer made. Thank you again for the review
Quoted 8 lines
[1/8]
pretty.c: better die message %(count) and %(total) (Mirko Faina) [2/8] format-patch: refactorgenerate_commit_list_cover(Mirko Faina) [3/8] format-patch: rename --cover-letter-format option (Mirko Faina) [4/8] docs/pretty-formats: add %(count) and %(total) (Mirko Faina) [5/8] format.commitListFormat: strip meaning from empty (Mirko Faina) [6/8] format-patch: wrapgenerate_commit_list_cover()(Mirko Faina) [7/8] format-patch: add preset for --commit-list-format (Mirko Faina) [8/8] format-patch: --commit-list-format without prefix (Mirko Faina)
I've just noticed:
- we don't specify the default value in git-format-patch(1), so I have to jump from there to git-config(1) to look it up
- we don't specify what the "modern" format means
Maybe that was all hashed out already, in which case links appreciated :)
Signature · 1 line
D. Ben Knoble
Quoted 14 lines · D. Ben Knoble
Quoted 4 lines · Mirko Faina
Not much has changed, just applied the suggestions Kristoffer made. Thank you again for the review
Quoted 8 lines
[1/8]
pretty.c: better die message %(count) and %(total) (Mirko Faina) [2/8] format-patch: refactorgenerate_commit_list_cover(Mirko Faina) [3/8] format-patch: rename --cover-letter-format option (Mirko Faina) [4/8] docs/pretty-formats: add %(count) and %(total) (Mirko Faina) [5/8] format.commitListFormat: strip meaning from empty (Mirko Faina) [6/8] format-patch: wrapgenerate_commit_list_cover()(Mirko Faina) [7/8] format-patch: add preset for --commit-list-format (Mirko Faina) [8/8] format-patch: --commit-list-format without prefix (Mirko Faina)
Quoted 6 lines
I've just noticed:
- we don't specify the default value in git-format-patch(1), so I have to jump from there to git-config(1) to look it up
- we don't specify what the "modern" format meansMaybe that was all hashed out already, in which case links appreciated :)
No, you're right. It doesn't say what the default is in --commit-list-format, it just says that it defaults to format.commitListFormat if not given.
I will send a patch to fix the docs. I guess I should also change the text for --cover-letter as the commit list can now be something other than a shortlog.
Thank you for pointing out.