* [PATCH] format-patch: add --mboxrd alias for --pretty=mboxrd @ 2022-11-14 9:41 Eric Wong 2022-11-14 16:53 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 6+ messages in thread From: Eric Wong @ 2022-11-14 9:41 UTC (permalink / raw) To: git mboxrd is a superior output format when used with --stdout and needs more exposure. Including pretty-formats.txt would be excessive, since documenting --pretty= for `git format-patch' would likely be confusing to users. Instead of documenting --pretty, add an --mboxrd alias to save keystrokes and improve documentation. Signed-off-by: Eric Wong <e@80x24.org> --- Also, --mboxrd without --stdout makes no sense to me, so I'm considering warning on it... Side note: some of the OPT_* switches might be misplaced under the "Messaging" OPT_GROUP... Documentation/git-format-patch.txt | 6 +++++- builtin/log.c | 7 +++++++ contrib/completion/git-completion.bash | 2 +- t/t4014-format-patch.sh | 6 ++++-- t/t4150-am.sh | 2 +- 5 files changed, 18 insertions(+), 5 deletions(-) diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index dfcc7da4c211..b080d3c61e31 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -9,7 +9,7 @@ git-format-patch - Prepare patches for e-mail submission SYNOPSIS -------- [verse] -'git format-patch' [-k] [(-o|--output-directory) <dir> | --stdout] +'git format-patch' [-k] [(-o|--output-directory) <dir> | --stdout] [--mboxrd] [--no-thread | --thread[=<style>]] [(--attach|--inline)[=<boundary>] | --no-attach] [-s | --signoff] @@ -145,6 +145,10 @@ include::diff-options.txt[] Print all commits to the standard output in mbox format, instead of creating a file for each one. +--mboxrd:: + Use the robust "mboxrd" format with `--stdout` to escape + "^>+From " lines. + --attach[=<boundary>]:: Create multipart/mixed attachment, the first part of which is the commit message and the patch itself in the diff --git a/builtin/log.c b/builtin/log.c index 5eafcf26b49b..13f5deb7a5c0 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1872,6 +1872,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) struct strbuf rdiff2 = STRBUF_INIT; struct strbuf rdiff_title = STRBUF_INIT; int creation_factor = -1; + int mboxrd = 0; const struct option builtin_format_patch_options[] = { OPT_CALLBACK_F('n', "numbered", &numbered, NULL, @@ -1883,6 +1884,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) OPT_BOOL('s', "signoff", &do_signoff, N_("add a Signed-off-by trailer")), OPT_BOOL(0, "stdout", &use_stdout, N_("print patches to standard out")), + OPT_BOOL(0, "mboxrd", &mboxrd, + N_("use the robust mboxrd format with --stdout")), OPT_BOOL(0, "cover-letter", &cover_letter, N_("generate a cover letter")), OPT_BOOL(0, "numbered-files", &just_numbers, @@ -2106,6 +2109,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) rev.diffopt.close_file, "--output", !!output_directory, "--output-directory"); + /* should we warn on --mboxrd w/o --stdout? */ + if (mboxrd) + rev.commit_format = CMIT_FMT_MBOXRD; + if (use_stdout) { setup_pager(); } else if (!rev.diffopt.close_file) { diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index ba5c395d2d80..f6e2fbdcfa68 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1815,7 +1815,7 @@ _git_fetch () __git_format_patch_extra_options=" --full-index --not --all --no-prefix --src-prefix= - --dst-prefix= --notes + --dst-prefix= --notes --mboxrd " _git_format_patch () diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index de1da4673da9..69ed8b1ffaa1 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -2281,7 +2281,7 @@ test_expect_success 'format-patch --attach cover-letter only is non-multipart' ' test_line_count = 1 output ' -test_expect_success 'format-patch --pretty=mboxrd' ' +test_expect_success 'format-patch --mboxrd' ' sp=" " && cat >msg <<-INPUT_END && mboxrd should escape the body @@ -2316,7 +2316,9 @@ test_expect_success 'format-patch --pretty=mboxrd' ' INPUT_END C=$(git commit-tree HEAD^^{tree} -p HEAD <msg) && - git format-patch --pretty=mboxrd --stdout -1 $C~1..$C >patch && + git format-patch --mboxrd --stdout -1 $C~1..$C >patch && + git format-patch --pretty=mboxrd --stdout -1 $C~1..$C >compat && + test_cmp patch compat && git grep -h --no-index -A11 \ "^>From could trip up a loose mbox parser" patch >actual && test_cmp expect actual diff --git a/t/t4150-am.sh b/t/t4150-am.sh index cdad4b688078..9a128c16a6ee 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -1033,7 +1033,7 @@ test_expect_success 'am --patch-format=mboxrd handles mboxrd' ' >From extra escape for reversibility INPUT_END git commit -F msg && - git format-patch --pretty=mboxrd --stdout -1 >mboxrd1 && + git format-patch --mboxrd --stdout -1 >mboxrd1 && grep "^>From could trip up a loose mbox parser" mboxrd1 && git checkout -f first && git am --patch-format=mboxrd mboxrd1 && ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] format-patch: add --mboxrd alias for --pretty=mboxrd 2022-11-14 9:41 [PATCH] format-patch: add --mboxrd alias for --pretty=mboxrd Eric Wong @ 2022-11-14 16:53 ` Ævar Arnfjörð Bjarmason 2022-11-14 20:59 ` Eric Wong 0 siblings, 1 reply; 6+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2022-11-14 16:53 UTC (permalink / raw) To: Eric Wong; +Cc: git On Mon, Nov 14 2022, Eric Wong wrote: > mboxrd is a superior output format when used with --stdout and > needs more exposure. Including pretty-formats.txt would be > excessive, since documenting --pretty= for `git format-patch' > would likely be confusing to users. > > Instead of documenting --pretty, add an --mboxrd alias to save > keystrokes and improve documentation. > > Signed-off-by: Eric Wong <e@80x24.org> > --- > Also, --mboxrd without --stdout makes no sense to me, > so I'm considering warning on it... > > Side note: some of the OPT_* switches might be misplaced > under the "Messaging" OPT_GROUP... Makes sense, but.... > +--mboxrd:: > + Use the robust "mboxrd" format with `--stdout` to escape > + "^>+From " lines. > + ...Rather than repeat ourselves, shouldn't we (or in addition to this) link to a manpage that discusses the --pretty=* formats. I was going to say that you can also use the "ifdef" asciidoc syntax, but for one paragraph that's probably overkill... > --attach[=<boundary>]:: > Create multipart/mixed attachment, the first part of > which is the commit message and the patch itself in the > diff --git a/builtin/log.c b/builtin/log.c > index 5eafcf26b49b..13f5deb7a5c0 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -1872,6 +1872,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > struct strbuf rdiff2 = STRBUF_INIT; > struct strbuf rdiff_title = STRBUF_INIT; > int creation_factor = -1; > + int mboxrd = 0; > > const struct option builtin_format_patch_options[] = { > OPT_CALLBACK_F('n', "numbered", &numbered, NULL, > @@ -1883,6 +1884,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > OPT_BOOL('s', "signoff", &do_signoff, N_("add a Signed-off-by trailer")), > OPT_BOOL(0, "stdout", &use_stdout, > N_("print patches to standard out")), > + OPT_BOOL(0, "mboxrd", &mboxrd, > + N_("use the robust mboxrd format with --stdout")), > OPT_BOOL(0, "cover-letter", &cover_letter, > N_("generate a cover letter")), > OPT_BOOL(0, "numbered-files", &just_numbers, > @@ -2106,6 +2109,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > rev.diffopt.close_file, "--output", > !!output_directory, "--output-directory"); > > + /* should we warn on --mboxrd w/o --stdout? */ Does that go for --pretty=mboxrd too? > + if (mboxrd) > + rev.commit_format = CMIT_FMT_MBOXRD; > + > if (use_stdout) { > setup_pager(); > } else if (!rev.diffopt.close_file) { > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index ba5c395d2d80..f6e2fbdcfa68 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -1815,7 +1815,7 @@ _git_fetch () > > __git_format_patch_extra_options=" > --full-index --not --all --no-prefix --src-prefix= > - --dst-prefix= --notes > + --dst-prefix= --notes --mboxrd > " > > _git_format_patch () > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > index de1da4673da9..69ed8b1ffaa1 100755 > --- a/t/t4014-format-patch.sh > +++ b/t/t4014-format-patch.sh > @@ -2281,7 +2281,7 @@ test_expect_success 'format-patch --attach cover-letter only is non-multipart' ' > test_line_count = 1 output > ' > > -test_expect_success 'format-patch --pretty=mboxrd' ' > +test_expect_success 'format-patch --mboxrd' ' > sp=" " && > cat >msg <<-INPUT_END && > mboxrd should escape the body > @@ -2316,7 +2316,9 @@ test_expect_success 'format-patch --pretty=mboxrd' ' > INPUT_END > > C=$(git commit-tree HEAD^^{tree} -p HEAD <msg) && > - git format-patch --pretty=mboxrd --stdout -1 $C~1..$C >patch && > + git format-patch --mboxrd --stdout -1 $C~1..$C >patch && > + git format-patch --pretty=mboxrd --stdout -1 $C~1..$C >compat && > + test_cmp patch compat && > git grep -h --no-index -A11 \ > "^>From could trip up a loose mbox parser" patch >actual && > test_cmp expect actual > diff --git a/t/t4150-am.sh b/t/t4150-am.sh > index cdad4b688078..9a128c16a6ee 100755 > --- a/t/t4150-am.sh > +++ b/t/t4150-am.sh > @@ -1033,7 +1033,7 @@ test_expect_success 'am --patch-format=mboxrd handles mboxrd' ' > >From extra escape for reversibility > INPUT_END > git commit -F msg && > - git format-patch --pretty=mboxrd --stdout -1 >mboxrd1 && > + git format-patch --mboxrd --stdout -1 >mboxrd1 && > grep "^>From could trip up a loose mbox parser" mboxrd1 && > git checkout -f first && > git am --patch-format=mboxrd mboxrd1 && Doesn't this leave us without coverage for the --pretty=mboxrd variant? I must admit I'm not a big outright fan of this, the "log-like" is confusing enough, with those accepting some options, ignoring others etc, now we're adding command-specific aliases too... Why not just document that we accept --pretty=<some subset>? E.g. see "range-diff"'s docs for an existing case where we discuss accepting a limited subset. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] format-patch: add --mboxrd alias for --pretty=mboxrd 2022-11-14 16:53 ` Ævar Arnfjörð Bjarmason @ 2022-11-14 20:59 ` Eric Wong 2022-12-18 4:24 ` Junio C Hamano 0 siblings, 1 reply; 6+ messages in thread From: Eric Wong @ 2022-11-14 20:59 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > On Mon, Nov 14 2022, Eric Wong wrote: > > > mboxrd is a superior output format when used with --stdout and > > needs more exposure. Including pretty-formats.txt would be > > excessive, since documenting --pretty= for `git format-patch' > > would likely be confusing to users. > > > > Instead of documenting --pretty, add an --mboxrd alias to save > > keystrokes and improve documentation. > > > > Signed-off-by: Eric Wong <e@80x24.org> > > --- > > Also, --mboxrd without --stdout makes no sense to me, > > so I'm considering warning on it... > > > > Side note: some of the OPT_* switches might be misplaced > > under the "Messaging" OPT_GROUP... > > Makes sense, but.... > > > +--mboxrd:: > > + Use the robust "mboxrd" format with `--stdout` to escape > > + "^>+From " lines. > > + > > ...Rather than repeat ourselves, shouldn't we (or in addition to this) > link to a manpage that discusses the --pretty=* formats. I was going to > say that you can also use the "ifdef" asciidoc syntax, but for one > paragraph that's probably overkill... As I noted in the commit message, I think discussing --pretty=* in the context of format-patch would be confusing for users. > > --attach[=<boundary>]:: > > Create multipart/mixed attachment, the first part of > > which is the commit message and the patch itself in the > > diff --git a/builtin/log.c b/builtin/log.c > > index 5eafcf26b49b..13f5deb7a5c0 100644 > > --- a/builtin/log.c > > +++ b/builtin/log.c > > @@ -1872,6 +1872,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > > struct strbuf rdiff2 = STRBUF_INIT; > > struct strbuf rdiff_title = STRBUF_INIT; > > int creation_factor = -1; > > + int mboxrd = 0; > > > > const struct option builtin_format_patch_options[] = { > > OPT_CALLBACK_F('n', "numbered", &numbered, NULL, > > @@ -1883,6 +1884,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > > OPT_BOOL('s', "signoff", &do_signoff, N_("add a Signed-off-by trailer")), > > OPT_BOOL(0, "stdout", &use_stdout, > > N_("print patches to standard out")), > > + OPT_BOOL(0, "mboxrd", &mboxrd, > > + N_("use the robust mboxrd format with --stdout")), > > OPT_BOOL(0, "cover-letter", &cover_letter, > > N_("generate a cover letter")), > > OPT_BOOL(0, "numbered-files", &just_numbers, > > @@ -2106,6 +2109,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > > rev.diffopt.close_file, "--output", > > !!output_directory, "--output-directory"); > > > > + /* should we warn on --mboxrd w/o --stdout? */ > > Does that go for --pretty=mboxrd too? Again, I prefer to mention --pretty= as little as possible when it comes to format-patch > > + if (mboxrd) > > + rev.commit_format = CMIT_FMT_MBOXRD; > > + It could be something like this: if (rev.commit_format == CMIT_FMT_MBOXRD && !use_stdout) warning("mboxrd without --stdout makes no sense\n"); But I'm on the fence about the warning. > > if (use_stdout) { > > setup_pager(); > > } else if (!rev.diffopt.close_file) { > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > > index ba5c395d2d80..f6e2fbdcfa68 100644 > > --- a/contrib/completion/git-completion.bash > > +++ b/contrib/completion/git-completion.bash > > @@ -1815,7 +1815,7 @@ _git_fetch () > > > > __git_format_patch_extra_options=" > > --full-index --not --all --no-prefix --src-prefix= > > - --dst-prefix= --notes > > + --dst-prefix= --notes --mboxrd > > " > > > > _git_format_patch () > > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > > index de1da4673da9..69ed8b1ffaa1 100755 > > --- a/t/t4014-format-patch.sh > > +++ b/t/t4014-format-patch.sh > > @@ -2281,7 +2281,7 @@ test_expect_success 'format-patch --attach cover-letter only is non-multipart' ' > > test_line_count = 1 output > > ' > > > > -test_expect_success 'format-patch --pretty=mboxrd' ' > > +test_expect_success 'format-patch --mboxrd' ' > > sp=" " && > > cat >msg <<-INPUT_END && > > mboxrd should escape the body > > @@ -2316,7 +2316,9 @@ test_expect_success 'format-patch --pretty=mboxrd' ' > > INPUT_END > > > > C=$(git commit-tree HEAD^^{tree} -p HEAD <msg) && > > - git format-patch --pretty=mboxrd --stdout -1 $C~1..$C >patch && > > + git format-patch --mboxrd --stdout -1 $C~1..$C >patch && > > + git format-patch --pretty=mboxrd --stdout -1 $C~1..$C >compat && > > + test_cmp patch compat && > > git grep -h --no-index -A11 \ > > "^>From could trip up a loose mbox parser" patch >actual && > > test_cmp expect actual > > diff --git a/t/t4150-am.sh b/t/t4150-am.sh > > index cdad4b688078..9a128c16a6ee 100755 > > --- a/t/t4150-am.sh > > +++ b/t/t4150-am.sh > > @@ -1033,7 +1033,7 @@ test_expect_success 'am --patch-format=mboxrd handles mboxrd' ' > > >From extra escape for reversibility > > INPUT_END > > git commit -F msg && > > - git format-patch --pretty=mboxrd --stdout -1 >mboxrd1 && > > + git format-patch --mboxrd --stdout -1 >mboxrd1 && > > grep "^>From could trip up a loose mbox parser" mboxrd1 && > > git checkout -f first && > > git am --patch-format=mboxrd mboxrd1 && > > Doesn't this leave us without coverage for the --pretty=mboxrd variant? These two lines were added to t/t4014-format-patch.sh above to test --pretty=mboxrd: + git format-patch --pretty=mboxrd --stdout -1 $C~1..$C >compat && + test_cmp patch compat && > I must admit I'm not a big outright fan of this, the "log-like" is > confusing enough, with those accepting some options, ignoring others > etc, now we're adding command-specific aliases too... > > Why not just document that we accept --pretty=<some subset>? E.g. see > "range-diff"'s docs for an existing case where we discuss accepting a > limited subset. Again, I think discussing --pretty= is confusing to users if they might end up using raw/full/fuller/etc (especially if they're relying on tab completion). I that was the reason --pretty= was never documented in the format-patch manpage (which I had nothing to do with). Which section of the range-diff man page are you referring to? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] format-patch: add --mboxrd alias for --pretty=mboxrd 2022-11-14 20:59 ` Eric Wong @ 2022-12-18 4:24 ` Junio C Hamano 2022-12-22 20:16 ` [PATCH v2] format-patch: support format.mboxrd with --stdout Eric Wong 0 siblings, 1 reply; 6+ messages in thread From: Junio C Hamano @ 2022-12-18 4:24 UTC (permalink / raw) To: Eric Wong; +Cc: Ævar Arnfjörð Bjarmason, git Eric Wong <e@80x24.org> writes: > As I noted in the commit message, I think discussing --pretty=* > in the context of format-patch would be confusing for users. Sensible. >> > + if (mboxrd) >> > + rev.commit_format = CMIT_FMT_MBOXRD; >> > + > > It could be something like this: > > if (rev.commit_format == CMIT_FMT_MBOXRD && !use_stdout) > warning("mboxrd without --stdout makes no sense\n"); > > But I'm on the fence about the warning. Does it really hurt when generating individual files, or does it naturally degenerate to the same as the plain old mbox, or something? If it does not hurt, then let's not clutter the output with a message that may make the user worried unnecessarily. Having said all that, if --pretty=mboxrd is usable, do we really need the --mboxrd short-hand? If we plan to eventually switch the default output format to mboxrd (which I am assuming your longer term vision), wouldn't it be the traditional format that may need a short-hand when something goes wrong? This change does not seem to be something we cannot live without, and as a step in the direction to move all of us to mboxrd, this feels somewhat a roundabout step. I wonder if it would be more direct to - declare that we will eventually switch to use mboxrd by default; - give a configuration knob to retain the current email as default; - do the usual deprecation dance. After all, between email and mboxrd, the e-mailed patch format is not something we choose per-invocation basis, is it? Picking a suitable format per project and setting it in .git/config, or picking a suitble format for yourself and setting it in ~/.gitconfig, and not having to think about it afterwards may be a better use of our users' time. Thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] format-patch: support format.mboxrd with --stdout 2022-12-18 4:24 ` Junio C Hamano @ 2022-12-22 20:16 ` Eric Wong 2022-12-23 1:34 ` Junio C Hamano 0 siblings, 1 reply; 6+ messages in thread From: Eric Wong @ 2022-12-22 20:16 UTC (permalink / raw) To: Junio C Hamano; +Cc: Ævar Arnfjörð Bjarmason, git Junio C Hamano <gitster@pobox.com> wrote: > Eric Wong <e@80x24.org> writes: > >> > + if (mboxrd) > >> > + rev.commit_format = CMIT_FMT_MBOXRD; > >> > + > > > > It could be something like this: > > > > if (rev.commit_format == CMIT_FMT_MBOXRD && !use_stdout) > > warning("mboxrd without --stdout makes no sense\n"); > > > > But I'm on the fence about the warning. OK, so the warning won't be necessary if using format.mboxrd via config. > Does it really hurt when generating individual files, or does it > naturally degenerate to the same as the plain old mbox, or > something? If it does not hurt, then let's not clutter the output > with a message that may make the user worried unnecessarily. mboxrd is bad for individual files since it can leave '>From' escaping if applied w/o `--patch-format mboxrd". My revised patch only enables mboxrd if --stdout is in use. > Having said all that, if --pretty=mboxrd is usable, do we really > need the --mboxrd short-hand? If we plan to eventually switch the > default output format to mboxrd (which I am assuming your longer > term vision), wouldn't it be the traditional format that may need a > short-hand when something goes wrong? Actually, I never considered making mboxrd a config file knob. But yes, it seems like a good idea. I hadn't considered making it the default, actually. The current `--pretty=email --stdout' is probably "good enough" for many users since the "From "-splitting in mailsplit is strict and legitimate commit messages are unlikely to have splittable "From " lines. > This change does not seem to be something we cannot live without, That double negative confused me :x "This change seems to be something we can live without" Yes, I agree. > and as a step in the direction to move all of us to mboxrd, this > feels somewhat a roundabout step. I wonder if it would be more > direct to > > - declare that we will eventually switch to use mboxrd by default; > > - give a configuration knob to retain the current email as default; > > - do the usual deprecation dance. I don't know if changing the default+deprecation is necessary, but thanks for suggesting a config knob. > After all, between email and mboxrd, the e-mailed patch format is > not something we choose per-invocation basis, is it? Right. > Picking a suitable format per project and setting it in .git/config, > or picking a suitble format for yourself and setting it in > ~/.gitconfig, and not having to think about it afterwards may be a > better use of our users' time. Agreed. Thanks for the suggestions. ------------8<------------- Subject: [PATCH v2] format-patch: support format.mboxrd with --stdout mboxrd is a more robust output format when used with --stdout and needs more exposure. Introducing this config knob lets users choose the more robust format for all their --stdout uses. Relying on --pretty=mboxrd and including all of pretty-formats.txt in the `git format-patch' documentation would likely be confusing to users. Furthermore, this setting is useful across multiple invocations. So introduce `format.mboxrd' as a boolean configuration knob that changes the default --pretty=email format to --pretty=mboxrd when (and only when) --stdout is in use. Signed-off-by: Eric Wong <e@80x24.org> --- Interdiff: diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt index c7303d8d9f..3bd78269e2 100644 --- a/Documentation/config/format.txt +++ b/Documentation/config/format.txt @@ -139,3 +139,7 @@ For example, ------------ + will only show notes from `refs/notes/bar`. + +format.mboxrd:: + A boolean value which enables the robust "mboxrd" format when + `--stdout` is in use to escape "^>+From " lines. diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index b080d3c61e..dfcc7da4c2 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -9,7 +9,7 @@ git-format-patch - Prepare patches for e-mail submission SYNOPSIS -------- [verse] -'git format-patch' [-k] [(-o|--output-directory) <dir> | --stdout] [--mboxrd] +'git format-patch' [-k] [(-o|--output-directory) <dir> | --stdout] [--no-thread | --thread[=<style>]] [(--attach|--inline)[=<boundary>] | --no-attach] [-s | --signoff] @@ -145,10 +145,6 @@ include::diff-options.txt[] Print all commits to the standard output in mbox format, instead of creating a file for each one. ---mboxrd:: - Use the robust "mboxrd" format with `--stdout` to escape - "^>+From " lines. - --attach[=<boundary>]:: Create multipart/mixed attachment, the first part of which is the commit message and the patch itself in the diff --git a/builtin/log.c b/builtin/log.c index 387560cc89..057e299c24 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -52,6 +52,7 @@ static int decoration_style; static int decoration_given; static int use_mailmap_config = 1; static unsigned int force_in_body_from; +static int stdout_mboxrd; static const char *fmt_patch_subject_prefix = "PATCH"; static int fmt_patch_name_max = FORMAT_PATCH_NAME_MAX_DEFAULT; static const char *fmt_pretty; @@ -1077,6 +1078,10 @@ static int git_format_config(const char *var, const char *value, void *cb) cover_from_description_mode = parse_cover_from_description(value); return 0; } + if (!strcmp(var, "format.mboxrd")) { + stdout_mboxrd = git_config_bool(var, value); + return 0; + } return git_log_config(var, value, cb); } @@ -1871,7 +1876,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) struct strbuf rdiff2 = STRBUF_INIT; struct strbuf rdiff_title = STRBUF_INIT; int creation_factor = -1; - int mboxrd = 0; const struct option builtin_format_patch_options[] = { OPT_CALLBACK_F('n', "numbered", &numbered, NULL, @@ -1883,8 +1887,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) OPT_BOOL('s', "signoff", &do_signoff, N_("add a Signed-off-by trailer")), OPT_BOOL(0, "stdout", &use_stdout, N_("print patches to standard out")), - OPT_BOOL(0, "mboxrd", &mboxrd, - N_("use the robust mboxrd format with --stdout")), OPT_BOOL(0, "cover-letter", &cover_letter, N_("generate a cover letter")), OPT_BOOL(0, "numbered-files", &just_numbers, @@ -2108,8 +2110,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) rev.diffopt.close_file, "--output", !!output_directory, "--output-directory"); - /* should we warn on --mboxrd w/o --stdout? */ - if (mboxrd) + if (use_stdout && stdout_mboxrd) rev.commit_format = CMIT_FMT_MBOXRD; if (use_stdout) { diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 73cbbd0ff8..dc95c34cc8 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1835,7 +1835,7 @@ _git_fetch () __git_format_patch_extra_options=" --full-index --not --all --no-prefix --src-prefix= - --dst-prefix= --notes --mboxrd + --dst-prefix= --notes " _git_format_patch () diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 69ed8b1ffa..012f155e10 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -2281,7 +2281,7 @@ test_expect_success 'format-patch --attach cover-letter only is non-multipart' ' test_line_count = 1 output ' -test_expect_success 'format-patch --mboxrd' ' +test_expect_success '-c format.mboxrd format-patch' ' sp=" " && cat >msg <<-INPUT_END && mboxrd should escape the body @@ -2316,7 +2316,7 @@ test_expect_success 'format-patch --mboxrd' ' INPUT_END C=$(git commit-tree HEAD^^{tree} -p HEAD <msg) && - git format-patch --mboxrd --stdout -1 $C~1..$C >patch && + git -c format.mboxrd format-patch --stdout -1 $C~1..$C >patch && git format-patch --pretty=mboxrd --stdout -1 $C~1..$C >compat && test_cmp patch compat && git grep -h --no-index -A11 \ diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 9a128c16a6..7646e856d5 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -1033,7 +1033,7 @@ test_expect_success 'am --patch-format=mboxrd handles mboxrd' ' >From extra escape for reversibility INPUT_END git commit -F msg && - git format-patch --mboxrd --stdout -1 >mboxrd1 && + git -c format.mboxrd format-patch --stdout -1 >mboxrd1 && grep "^>From could trip up a loose mbox parser" mboxrd1 && git checkout -f first && git am --patch-format=mboxrd mboxrd1 && Documentation/config/format.txt | 4 ++++ builtin/log.c | 8 ++++++++ t/t4014-format-patch.sh | 6 ++++-- t/t4150-am.sh | 2 +- 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt index c7303d8d9f..3bd78269e2 100644 --- a/Documentation/config/format.txt +++ b/Documentation/config/format.txt @@ -139,3 +139,7 @@ For example, ------------ + will only show notes from `refs/notes/bar`. + +format.mboxrd:: + A boolean value which enables the robust "mboxrd" format when + `--stdout` is in use to escape "^>+From " lines. diff --git a/builtin/log.c b/builtin/log.c index 89447a5083..057e299c24 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -52,6 +52,7 @@ static int decoration_style; static int decoration_given; static int use_mailmap_config = 1; static unsigned int force_in_body_from; +static int stdout_mboxrd; static const char *fmt_patch_subject_prefix = "PATCH"; static int fmt_patch_name_max = FORMAT_PATCH_NAME_MAX_DEFAULT; static const char *fmt_pretty; @@ -1077,6 +1078,10 @@ static int git_format_config(const char *var, const char *value, void *cb) cover_from_description_mode = parse_cover_from_description(value); return 0; } + if (!strcmp(var, "format.mboxrd")) { + stdout_mboxrd = git_config_bool(var, value); + return 0; + } return git_log_config(var, value, cb); } @@ -2105,6 +2110,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) rev.diffopt.close_file, "--output", !!output_directory, "--output-directory"); + if (use_stdout && stdout_mboxrd) + rev.commit_format = CMIT_FMT_MBOXRD; + if (use_stdout) { setup_pager(); } else if (!rev.diffopt.close_file) { diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index de1da4673d..012f155e10 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -2281,7 +2281,7 @@ test_expect_success 'format-patch --attach cover-letter only is non-multipart' ' test_line_count = 1 output ' -test_expect_success 'format-patch --pretty=mboxrd' ' +test_expect_success '-c format.mboxrd format-patch' ' sp=" " && cat >msg <<-INPUT_END && mboxrd should escape the body @@ -2316,7 +2316,9 @@ test_expect_success 'format-patch --pretty=mboxrd' ' INPUT_END C=$(git commit-tree HEAD^^{tree} -p HEAD <msg) && - git format-patch --pretty=mboxrd --stdout -1 $C~1..$C >patch && + git -c format.mboxrd format-patch --stdout -1 $C~1..$C >patch && + git format-patch --pretty=mboxrd --stdout -1 $C~1..$C >compat && + test_cmp patch compat && git grep -h --no-index -A11 \ "^>From could trip up a loose mbox parser" patch >actual && test_cmp expect actual diff --git a/t/t4150-am.sh b/t/t4150-am.sh index cdad4b6880..7646e856d5 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -1033,7 +1033,7 @@ test_expect_success 'am --patch-format=mboxrd handles mboxrd' ' >From extra escape for reversibility INPUT_END git commit -F msg && - git format-patch --pretty=mboxrd --stdout -1 >mboxrd1 && + git -c format.mboxrd format-patch --stdout -1 >mboxrd1 && grep "^>From could trip up a loose mbox parser" mboxrd1 && git checkout -f first && git am --patch-format=mboxrd mboxrd1 && ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] format-patch: support format.mboxrd with --stdout 2022-12-22 20:16 ` [PATCH v2] format-patch: support format.mboxrd with --stdout Eric Wong @ 2022-12-23 1:34 ` Junio C Hamano 0 siblings, 0 replies; 6+ messages in thread From: Junio C Hamano @ 2022-12-23 1:34 UTC (permalink / raw) To: Eric Wong; +Cc: Ævar Arnfjörð Bjarmason, git Eric Wong <e@80x24.org> writes: > Subject: [PATCH v2] format-patch: support format.mboxrd with --stdout > > mboxrd is a more robust output format when used with --stdout > and needs more exposure. Introducing this config knob lets > users choose the more robust format for all their --stdout > uses. > > Relying on --pretty=mboxrd and including all of pretty-formats.txt > in the `git format-patch' documentation would likely be > confusing to users. Furthermore, this setting is useful across > multiple invocations. So introduce `format.mboxrd' as a boolean > configuration knob that changes the default --pretty=email format > to --pretty=mboxrd when (and only when) --stdout is in use. Sounds sensible. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-23 1:35 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-14 9:41 [PATCH] format-patch: add --mboxrd alias for --pretty=mboxrd Eric Wong 2022-11-14 16:53 ` Ævar Arnfjörð Bjarmason 2022-11-14 20:59 ` Eric Wong 2022-12-18 4:24 ` Junio C Hamano 2022-12-22 20:16 ` [PATCH v2] format-patch: support format.mboxrd with --stdout Eric Wong 2022-12-23 1:34 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).