* [PATCH 0/2] format-patch: teach format.notes config option
@ 2019-04-27 19:25 Denton Liu
2019-04-27 19:25 ` [PATCH 1/2] git-format-patch.txt: document --no-notes option Denton Liu
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Denton Liu @ 2019-04-27 19:25 UTC (permalink / raw)
To: Git Mailing List
If a user sends patches with git-format-patch, they might want to take
advantage of notes to add additional commentary. However, if they are
forgetful (like me) they may forget to include the `--notes` option in
their invocation of git-format-patch.
Teach git-format-patch the `format.notes` config option where if its
value is true, notes will automatically be appended.
Denton Liu (2):
git-format-patch.txt: document --no-notes option
format-patch: teach format.notes config option
Documentation/config/format.txt | 4 ++++
Documentation/git-format-patch.txt | 7 ++++++-
builtin/log.c | 6 ++++++
t/t4014-format-patch.sh | 28 ++++++++++++++++++++++++++++
4 files changed, 44 insertions(+), 1 deletion(-)
--
2.21.0.1033.g0e8cc1100c
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/2] git-format-patch.txt: document --no-notes option 2019-04-27 19:25 [PATCH 0/2] format-patch: teach format.notes config option Denton Liu @ 2019-04-27 19:25 ` Denton Liu 2019-04-27 19:25 ` [PATCH 2/2] format-patch: teach format.notes config option Denton Liu 2019-05-08 15:02 ` [PATCH v2 0/2] " Denton Liu 2 siblings, 0 replies; 15+ messages in thread From: Denton Liu @ 2019-04-27 19:25 UTC (permalink / raw) To: Git Mailing List Internally, git-format-patch uses the `handle_revision_opt` parser. The parser handles the `--no-notes` option to negate an earlier `--notes` option, but it isn't documented. Document this option so that users are aware of it. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Documentation/git-format-patch.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 1af85d404f..2c3971390e 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -22,7 +22,8 @@ SYNOPSIS [--rfc] [--subject-prefix=Subject-Prefix] [(--reroll-count|-v) <n>] [--to=<email>] [--cc=<email>] - [--[no-]cover-letter] [--quiet] [--notes[=<ref>]] + [--[no-]cover-letter] [--quiet] + [--no-notes | --notes[=<ref>]] [--interdiff=<previous>] [--range-diff=<previous> [--creation-factor=<percent>]] [--progress] @@ -263,6 +264,7 @@ material (this may change in the future). for details. --notes[=<ref>]:: +--no-notes:: Append the notes (see linkgit:git-notes[1]) for the commit after the three-dash line. + -- 2.21.0.1033.g0e8cc1100c ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/2] format-patch: teach format.notes config option 2019-04-27 19:25 [PATCH 0/2] format-patch: teach format.notes config option Denton Liu 2019-04-27 19:25 ` [PATCH 1/2] git-format-patch.txt: document --no-notes option Denton Liu @ 2019-04-27 19:25 ` Denton Liu 2019-05-07 4:43 ` Junio C Hamano 2019-05-08 15:02 ` [PATCH v2 0/2] " Denton Liu 2 siblings, 1 reply; 15+ messages in thread From: Denton Liu @ 2019-04-27 19:25 UTC (permalink / raw) To: Git Mailing List In git-format-patch, notes can be appended with the `--notes` option. However, this must be specified by the user on an invocation-by-invocation basis. If a user is not careful, it's possible that they may forget to include it and generate a patch series without notes. Teach git-format-patch the `format.notes` config option where if its value is true, notes will automatically be appended. This option is overridable with the `--no-notes` option in case a user wishes not to append notes. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Documentation/config/format.txt | 4 ++++ Documentation/git-format-patch.txt | 3 +++ builtin/log.c | 6 ++++++ t/t4014-format-patch.sh | 28 ++++++++++++++++++++++++++++ 4 files changed, 41 insertions(+) diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt index dc77941c48..55327b6511 100644 --- a/Documentation/config/format.txt +++ b/Documentation/config/format.txt @@ -85,3 +85,7 @@ format.outputDirectory:: format.useAutoBase:: A boolean value which lets you enable the `--base=auto` option of format-patch by default. + +format.notes:: + A boolean value which lets you enable the `--notes` option of + format-patch by default. diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 2c3971390e..9ce5b8aaee 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -275,6 +275,9 @@ these explanations after `format-patch` has run but before sending, keeping them as Git notes allows them to be maintained between versions of the patch series (but see the discussion of the `notes.rewrite` configuration options in linkgit:git-notes[1] to use this workflow). ++ +The default is `--no-notes`, unless the `format.notes` configuration is +set. --[no-]signature=<signature>:: Add a signature to each message produced. Per RFC 3676 the signature diff --git a/builtin/log.c b/builtin/log.c index e43ee12fb1..3c6fd9b117 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -769,6 +769,7 @@ static const char *signature = git_version_string; static const char *signature_file; static int config_cover_letter; static const char *config_output_directory; +static int show_notes; enum { COVER_UNSET, @@ -864,6 +865,10 @@ static int git_format_config(const char *var, const char *value, void *cb) from = NULL; return 0; } + if (!strcmp(var, "format.notes")) { + show_notes = git_config_bool(var, value); + return 0; + } return git_log_config(var, value, cb); } @@ -1626,6 +1631,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) rev.max_parents = 1; rev.diffopt.flags.recursive = 1; rev.subject_prefix = fmt_patch_subject_prefix; + rev.show_notes = show_notes; memset(&s_r_opt, 0, sizeof(s_r_opt)); s_r_opt.def = "HEAD"; s_r_opt.revarg_opt = REVARG_COMMITTISH; diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index b6e2fdbc44..fe9522121a 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -738,6 +738,34 @@ test_expect_success 'format-patch --notes --signoff' ' sed "1,/^---$/d" out | grep "test message" ' +test_expect_success 'format-patch notes output control' ' + git notes add -m "notes config message" HEAD && + test_when_finished git notes remove HEAD && + + git format-patch -1 --stdout >out && + ! grep "notes config message" out && + git format-patch -1 --stdout --notes >out && + grep "notes config message" out && + git format-patch -1 --stdout --no-notes >out && + ! grep "notes config message" out && + git format-patch -1 --stdout --notes --no-notes >out && + ! grep "notes config message" out && + git format-patch -1 --stdout --no-notes --notes >out && + grep "notes config message" out && + + test_config format.notes true && + git format-patch -1 --stdout >out && + grep "notes config message" out && + git format-patch -1 --stdout --notes >out && + grep "notes config message" out && + git format-patch -1 --stdout --no-notes >out && + ! grep "notes config message" out && + git format-patch -1 --stdout --notes --no-notes >out && + ! grep "notes config message" out && + git format-patch -1 --stdout --no-notes --notes >out && + grep "notes config message" out +' + echo "fatal: --name-only does not make sense" > expect.name-only echo "fatal: --name-status does not make sense" > expect.name-status echo "fatal: --check does not make sense" > expect.check -- 2.21.0.1033.g0e8cc1100c ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] format-patch: teach format.notes config option 2019-04-27 19:25 ` [PATCH 2/2] format-patch: teach format.notes config option Denton Liu @ 2019-05-07 4:43 ` Junio C Hamano 0 siblings, 0 replies; 15+ messages in thread From: Junio C Hamano @ 2019-05-07 4:43 UTC (permalink / raw) To: Denton Liu; +Cc: Git Mailing List Denton Liu <liu.denton@gmail.com> writes: > In git-format-patch, notes can be appended with the `--notes` option. > However, this must be specified by the user on an > invocation-by-invocation basis. If a user is not careful, it's possible > that they may forget to include it and generate a patch series without > notes. > > Teach git-format-patch the `format.notes` config option where if its > value is true, notes will automatically be appended. This option is > overridable with the `--no-notes` option in case a user wishes not to > append notes. > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > Documentation/config/format.txt | 4 ++++ > Documentation/git-format-patch.txt | 3 +++ > builtin/log.c | 6 ++++++ > t/t4014-format-patch.sh | 28 ++++++++++++++++++++++++++++ > 4 files changed, 41 insertions(+) > > diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt > index dc77941c48..55327b6511 100644 > --- a/Documentation/config/format.txt > +++ b/Documentation/config/format.txt > @@ -85,3 +85,7 @@ format.outputDirectory:: > format.useAutoBase:: > A boolean value which lets you enable the `--base=auto` option of > format-patch by default. > + > +format.notes:: > + A boolean value which lets you enable the `--notes` option of > + format-patch by default. Whoa. Why should this be a boolean? I think I can do git format-patch --notes=amlog master.. and was hoping that this can be used to configure the option out of my command line typing. > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > index b6e2fdbc44..fe9522121a 100755 > --- a/t/t4014-format-patch.sh > +++ b/t/t4014-format-patch.sh > @@ -738,6 +738,34 @@ test_expect_success 'format-patch --notes --signoff' ' > sed "1,/^---$/d" out | grep "test message" > ' And if you look at the existing test above this, you'd notice that the command is tested is in "format-patch --notes=test" form. We should make sure that the same is done with the config in the new test, too. > +test_expect_success 'format-patch notes output control' ' > + git notes add -m "notes config message" HEAD && > + test_when_finished git notes remove HEAD && > + > + git format-patch -1 --stdout >out && > + ! grep "notes config message" out && > + git format-patch -1 --stdout --notes >out && > + grep "notes config message" out && > + git format-patch -1 --stdout --no-notes >out && > + ! grep "notes config message" out && > + git format-patch -1 --stdout --notes --no-notes >out && > + ! grep "notes config message" out && > + git format-patch -1 --stdout --no-notes --notes >out && > + grep "notes config message" out && > + > + test_config format.notes true && > + git format-patch -1 --stdout >out && > + grep "notes config message" out && > + git format-patch -1 --stdout --notes >out && > + grep "notes config message" out && > + git format-patch -1 --stdout --no-notes >out && > + ! grep "notes config message" out && > + git format-patch -1 --stdout --notes --no-notes >out && > + ! grep "notes config message" out && > + git format-patch -1 --stdout --no-notes --notes >out && > + grep "notes config message" out > +' > + > echo "fatal: --name-only does not make sense" > expect.name-only > echo "fatal: --name-status does not make sense" > expect.name-status > echo "fatal: --check does not make sense" > expect.check ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 0/2] format-patch: teach format.notes config option 2019-04-27 19:25 [PATCH 0/2] format-patch: teach format.notes config option Denton Liu 2019-04-27 19:25 ` [PATCH 1/2] git-format-patch.txt: document --no-notes option Denton Liu 2019-04-27 19:25 ` [PATCH 2/2] format-patch: teach format.notes config option Denton Liu @ 2019-05-08 15:02 ` Denton Liu 2019-05-08 15:02 ` [PATCH v2 1/2] git-format-patch.txt: document --no-notes option Denton Liu ` (2 more replies) 2 siblings, 3 replies; 15+ messages in thread From: Denton Liu @ 2019-05-08 15:02 UTC (permalink / raw) To: Git Mailing List; +Cc: Junio C Hamano Hi Junio, Thanks for the review. I made format.notes accept one (or more) refs now. Changes since v1: * Made format.notes accept a notes ref instead of a boolean Denton Liu (2): git-format-patch.txt: document --no-notes option format-patch: teach format.notes config option Documentation/config/format.txt | 13 ++++++ Documentation/git-format-patch.txt | 7 ++- builtin/log.c | 18 +++++++- t/t4014-format-patch.sh | 70 ++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 2 deletions(-) Range-diff against v1: 1: 48b6331d75 = 1: 4c3535f25b git-format-patch.txt: document --no-notes option 2: 1338045be4 ! 2: fe674bf63e format-patch: teach format.notes config option @@ -8,8 +8,9 @@ that they may forget to include it and generate a patch series without notes. - Teach git-format-patch the `format.notes` config option where if its - value is true, notes will automatically be appended. This option is + Teach git-format-patch the `format.notes` config option its value is a + notes ref that will be automatically be appended. The special value of + "standard" can be used to specify the standard notes. This option is overridable with the `--no-notes` option in case a user wishes not to append notes. @@ -24,8 +25,17 @@ format-patch by default. + +format.notes:: -+ A boolean value which lets you enable the `--notes` option of -+ format-patch by default. ++ A ref which specifies where to get the notes (see ++ linkgit:git-notes[1]) that are appended for the commit after the ++ three-dash line. +++ ++If the special value of "standard" is specified, then the standard notes ++ref is used (i.e. the notes ref used by `git notes` when no `--ref` ++argument is specified). If one wishes to use the ref ++`ref/notes/standard`, please use that literal instead. +++ ++This configuration can be specified multiple times in order to allow ++multiple notes refs to be included. diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt --- a/Documentation/git-format-patch.txt @@ -45,32 +55,45 @@ --- a/builtin/log.c +++ b/builtin/log.c @@ - static const char *signature_file; - static int config_cover_letter; - static const char *config_output_directory; -+static int show_notes; - enum { - COVER_UNSET, + static int git_format_config(const char *var, const char *value, void *cb) + { ++ struct rev_info *rev = cb; ++ + if (!strcmp(var, "format.headers")) { + if (!value) + die(_("format.headers without value")); @@ from = NULL; return 0; } + if (!strcmp(var, "format.notes")) { -+ show_notes = git_config_bool(var, value); ++ struct strbuf buf = STRBUF_INIT; ++ ++ rev->show_notes = 1; ++ if (!strcmp(value, "standard")) ++ rev->notes_opt.use_default_notes = 1; ++ else { ++ strbuf_addstr(&buf, value); ++ expand_notes_ref(&buf); ++ string_list_append(&rev->notes_opt.extra_notes_refs, ++ strbuf_detach(&buf, NULL)); ++ } + return 0; + } return git_log_config(var, value, cb); } @@ - rev.max_parents = 1; - rev.diffopt.flags.recursive = 1; - rev.subject_prefix = fmt_patch_subject_prefix; -+ rev.show_notes = show_notes; - memset(&s_r_opt, 0, sizeof(s_r_opt)); - s_r_opt.def = "HEAD"; - s_r_opt.revarg_opt = REVARG_COMMITTISH; + extra_to.strdup_strings = 1; + extra_cc.strdup_strings = 1; + init_log_defaults(); +- git_config(git_format_config, NULL); + repo_init_revisions(the_repository, &rev, prefix); ++ git_config(git_format_config, &rev); + rev.commit_format = CMIT_FMT_EMAIL; + rev.expand_tabs_in_log_default = 0; + rev.verbose_header = 1; diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh --- a/t/t4014-format-patch.sh @@ -94,7 +117,7 @@ + git format-patch -1 --stdout --no-notes --notes >out && + grep "notes config message" out && + -+ test_config format.notes true && ++ test_config format.notes standard && + git format-patch -1 --stdout >out && + grep "notes config message" out && + git format-patch -1 --stdout --notes >out && @@ -106,6 +129,48 @@ + git format-patch -1 --stdout --no-notes --notes >out && + grep "notes config message" out +' ++ ++test_expect_success 'format-patch with multiple notes refs' ' ++ git notes --ref note1 add -m "this is note 1" HEAD && ++ test_when_finished git notes --ref note1 remove HEAD && ++ git notes --ref note2 add -m "this is note 2" HEAD && ++ test_when_finished git notes --ref note2 remove HEAD && ++ ++ git format-patch -1 --stdout >out && ++ ! grep "this is note 1" out && ++ ! grep "this is note 2" out && ++ git format-patch -1 --stdout --notes=note1 >out && ++ grep "this is note 1" out && ++ ! grep "this is note 2" out && ++ git format-patch -1 --stdout --notes=note2 >out && ++ ! grep "this is note 1" out && ++ grep "this is note 2" out && ++ git format-patch -1 --stdout --notes=note1 --notes=note2 >out && ++ grep "this is note 1" out && ++ grep "this is note 2" out && ++ ++ test_config format.notes note1 && ++ git format-patch -1 --stdout >out && ++ grep "this is note 1" out && ++ ! grep "this is note 2" out && ++ git format-patch -1 --stdout --no-notes >out && ++ ! grep "this is note 1" out && ++ ! grep "this is note 2" out && ++ git format-patch -1 --stdout --notes=note2 >out && ++ grep "this is note 1" out && ++ grep "this is note 2" out && ++ git format-patch -1 --stdout --no-notes --notes=note2 >out && ++ ! grep "this is note 1" out && ++ grep "this is note 2" out && ++ ++ git config --add format.notes note2 && ++ git format-patch -1 --stdout >out && ++ grep "this is note 1" out && ++ grep "this is note 2" out && ++ git format-patch -1 --stdout --no-notes >out && ++ ! grep "this is note 1" out && ++ ! grep "this is note 2" out ++' + echo "fatal: --name-only does not make sense" > expect.name-only echo "fatal: --name-status does not make sense" > expect.name-status -- 2.21.0.1049.geb646f7864 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/2] git-format-patch.txt: document --no-notes option 2019-05-08 15:02 ` [PATCH v2 0/2] " Denton Liu @ 2019-05-08 15:02 ` Denton Liu 2019-05-08 15:02 ` [PATCH v2 2/2] format-patch: teach format.notes config option Denton Liu 2019-05-10 18:37 ` [PATCH v3 0/2] " Denton Liu 2 siblings, 0 replies; 15+ messages in thread From: Denton Liu @ 2019-05-08 15:02 UTC (permalink / raw) To: Git Mailing List; +Cc: Junio C Hamano Internally, git-format-patch uses the `handle_revision_opt` parser. The parser handles the `--no-notes` option to negate an earlier `--notes` option, but it isn't documented. Document this option so that users are aware of it. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Documentation/git-format-patch.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 1af85d404f..2c3971390e 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -22,7 +22,8 @@ SYNOPSIS [--rfc] [--subject-prefix=Subject-Prefix] [(--reroll-count|-v) <n>] [--to=<email>] [--cc=<email>] - [--[no-]cover-letter] [--quiet] [--notes[=<ref>]] + [--[no-]cover-letter] [--quiet] + [--no-notes | --notes[=<ref>]] [--interdiff=<previous>] [--range-diff=<previous> [--creation-factor=<percent>]] [--progress] @@ -263,6 +264,7 @@ material (this may change in the future). for details. --notes[=<ref>]:: +--no-notes:: Append the notes (see linkgit:git-notes[1]) for the commit after the three-dash line. + -- 2.21.0.1049.geb646f7864 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/2] format-patch: teach format.notes config option 2019-05-08 15:02 ` [PATCH v2 0/2] " Denton Liu 2019-05-08 15:02 ` [PATCH v2 1/2] git-format-patch.txt: document --no-notes option Denton Liu @ 2019-05-08 15:02 ` Denton Liu 2019-05-08 17:18 ` Beat Bolli 2019-05-10 18:37 ` [PATCH v3 0/2] " Denton Liu 2 siblings, 1 reply; 15+ messages in thread From: Denton Liu @ 2019-05-08 15:02 UTC (permalink / raw) To: Git Mailing List; +Cc: Junio C Hamano In git-format-patch, notes can be appended with the `--notes` option. However, this must be specified by the user on an invocation-by-invocation basis. If a user is not careful, it's possible that they may forget to include it and generate a patch series without notes. Teach git-format-patch the `format.notes` config option its value is a notes ref that will be automatically be appended. The special value of "standard" can be used to specify the standard notes. This option is overridable with the `--no-notes` option in case a user wishes not to append notes. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- One thing I'm worried about is that I'm not really sure using "standard" as the special value is a good idea. Would "auto" be a better special value? Documentation/config/format.txt | 13 ++++++ Documentation/git-format-patch.txt | 3 ++ builtin/log.c | 18 +++++++- t/t4014-format-patch.sh | 70 ++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 1 deletion(-) diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt index dc77941c48..e25f9cfc61 100644 --- a/Documentation/config/format.txt +++ b/Documentation/config/format.txt @@ -85,3 +85,16 @@ format.outputDirectory:: format.useAutoBase:: A boolean value which lets you enable the `--base=auto` option of format-patch by default. + +format.notes:: + A ref which specifies where to get the notes (see + linkgit:git-notes[1]) that are appended for the commit after the + three-dash line. ++ +If the special value of "standard" is specified, then the standard notes +ref is used (i.e. the notes ref used by `git notes` when no `--ref` +argument is specified). If one wishes to use the ref +`ref/notes/standard`, please use that literal instead. ++ +This configuration can be specified multiple times in order to allow +multiple notes refs to be included. diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 2c3971390e..9ce5b8aaee 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -275,6 +275,9 @@ these explanations after `format-patch` has run but before sending, keeping them as Git notes allows them to be maintained between versions of the patch series (but see the discussion of the `notes.rewrite` configuration options in linkgit:git-notes[1] to use this workflow). ++ +The default is `--no-notes`, unless the `format.notes` configuration is +set. --[no-]signature=<signature>:: Add a signature to each message produced. Per RFC 3676 the signature diff --git a/builtin/log.c b/builtin/log.c index e43ee12fb1..24954e42b0 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -779,6 +779,8 @@ enum { static int git_format_config(const char *var, const char *value, void *cb) { + struct rev_info *rev = cb; + if (!strcmp(var, "format.headers")) { if (!value) die(_("format.headers without value")); @@ -864,6 +866,20 @@ static int git_format_config(const char *var, const char *value, void *cb) from = NULL; return 0; } + if (!strcmp(var, "format.notes")) { + struct strbuf buf = STRBUF_INIT; + + rev->show_notes = 1; + if (!strcmp(value, "standard")) + rev->notes_opt.use_default_notes = 1; + else { + strbuf_addstr(&buf, value); + expand_notes_ref(&buf); + string_list_append(&rev->notes_opt.extra_notes_refs, + strbuf_detach(&buf, NULL)); + } + return 0; + } return git_log_config(var, value, cb); } @@ -1617,8 +1633,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) extra_to.strdup_strings = 1; extra_cc.strdup_strings = 1; init_log_defaults(); - git_config(git_format_config, NULL); repo_init_revisions(the_repository, &rev, prefix); + git_config(git_format_config, &rev); rev.commit_format = CMIT_FMT_EMAIL; rev.expand_tabs_in_log_default = 0; rev.verbose_header = 1; diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index b6e2fdbc44..e0127282ba 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -738,6 +738,76 @@ test_expect_success 'format-patch --notes --signoff' ' sed "1,/^---$/d" out | grep "test message" ' +test_expect_success 'format-patch notes output control' ' + git notes add -m "notes config message" HEAD && + test_when_finished git notes remove HEAD && + + git format-patch -1 --stdout >out && + ! grep "notes config message" out && + git format-patch -1 --stdout --notes >out && + grep "notes config message" out && + git format-patch -1 --stdout --no-notes >out && + ! grep "notes config message" out && + git format-patch -1 --stdout --notes --no-notes >out && + ! grep "notes config message" out && + git format-patch -1 --stdout --no-notes --notes >out && + grep "notes config message" out && + + test_config format.notes standard && + git format-patch -1 --stdout >out && + grep "notes config message" out && + git format-patch -1 --stdout --notes >out && + grep "notes config message" out && + git format-patch -1 --stdout --no-notes >out && + ! grep "notes config message" out && + git format-patch -1 --stdout --notes --no-notes >out && + ! grep "notes config message" out && + git format-patch -1 --stdout --no-notes --notes >out && + grep "notes config message" out +' + +test_expect_success 'format-patch with multiple notes refs' ' + git notes --ref note1 add -m "this is note 1" HEAD && + test_when_finished git notes --ref note1 remove HEAD && + git notes --ref note2 add -m "this is note 2" HEAD && + test_when_finished git notes --ref note2 remove HEAD && + + git format-patch -1 --stdout >out && + ! grep "this is note 1" out && + ! grep "this is note 2" out && + git format-patch -1 --stdout --notes=note1 >out && + grep "this is note 1" out && + ! grep "this is note 2" out && + git format-patch -1 --stdout --notes=note2 >out && + ! grep "this is note 1" out && + grep "this is note 2" out && + git format-patch -1 --stdout --notes=note1 --notes=note2 >out && + grep "this is note 1" out && + grep "this is note 2" out && + + test_config format.notes note1 && + git format-patch -1 --stdout >out && + grep "this is note 1" out && + ! grep "this is note 2" out && + git format-patch -1 --stdout --no-notes >out && + ! grep "this is note 1" out && + ! grep "this is note 2" out && + git format-patch -1 --stdout --notes=note2 >out && + grep "this is note 1" out && + grep "this is note 2" out && + git format-patch -1 --stdout --no-notes --notes=note2 >out && + ! grep "this is note 1" out && + grep "this is note 2" out && + + git config --add format.notes note2 && + git format-patch -1 --stdout >out && + grep "this is note 1" out && + grep "this is note 2" out && + git format-patch -1 --stdout --no-notes >out && + ! grep "this is note 1" out && + ! grep "this is note 2" out +' + echo "fatal: --name-only does not make sense" > expect.name-only echo "fatal: --name-status does not make sense" > expect.name-status echo "fatal: --check does not make sense" > expect.check -- 2.21.0.1049.geb646f7864 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] format-patch: teach format.notes config option 2019-05-08 15:02 ` [PATCH v2 2/2] format-patch: teach format.notes config option Denton Liu @ 2019-05-08 17:18 ` Beat Bolli 2019-05-08 17:31 ` Denton Liu 0 siblings, 1 reply; 15+ messages in thread From: Beat Bolli @ 2019-05-08 17:18 UTC (permalink / raw) To: git; +Cc: Git Mailing List, Junio C Hamano On 08.05.19 17:02, Denton Liu wrote: > In git-format-patch, notes can be appended with the `--notes` option. > However, this must be specified by the user on an > invocation-by-invocation basis. If a user is not careful, it's possible > that they may forget to include it and generate a patch series without > notes. > > Teach git-format-patch the `format.notes` config option its value is a s/its/. Its/ > notes ref that will be automatically be appended. The special value of Drop the second "be ". > "standard" can be used to specify the standard notes. This option is > overridable with the `--no-notes` option in case a user wishes not to > append notes. > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > One thing I'm worried about is that I'm not really sure using "standard" > as the special value is a good idea. Would "auto" be a better special > value? > > Documentation/config/format.txt | 13 ++++++ > Documentation/git-format-patch.txt | 3 ++ > builtin/log.c | 18 +++++++- > t/t4014-format-patch.sh | 70 ++++++++++++++++++++++++++++++ > 4 files changed, 103 insertions(+), 1 deletion(-) > > diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt > index dc77941c48..e25f9cfc61 100644 > --- a/Documentation/config/format.txt > +++ b/Documentation/config/format.txt > @@ -85,3 +85,16 @@ format.outputDirectory:: > format.useAutoBase:: > A boolean value which lets you enable the `--base=auto` option of > format-patch by default. > + > +format.notes:: > + A ref which specifies where to get the notes (see > + linkgit:git-notes[1]) that are appended for the commit after the > + three-dash line. > ++ > +If the special value of "standard" is specified, then the standard notes > +ref is used (i.e. the notes ref used by `git notes` when no `--ref` > +argument is specified). If one wishes to use the ref > +`ref/notes/standard`, please use that literal instead. > ++ > +This configuration can be specified multiple times in order to allow > +multiple notes refs to be included. > diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt > index 2c3971390e..9ce5b8aaee 100644 > --- a/Documentation/git-format-patch.txt > +++ b/Documentation/git-format-patch.txt > @@ -275,6 +275,9 @@ these explanations after `format-patch` has run but before sending, > keeping them as Git notes allows them to be maintained between versions > of the patch series (but see the discussion of the `notes.rewrite` > configuration options in linkgit:git-notes[1] to use this workflow). > ++ > +The default is `--no-notes`, unless the `format.notes` configuration is > +set. > > --[no-]signature=<signature>:: > Add a signature to each message produced. Per RFC 3676 the signature > diff --git a/builtin/log.c b/builtin/log.c > index e43ee12fb1..24954e42b0 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -779,6 +779,8 @@ enum { > > static int git_format_config(const char *var, const char *value, void *cb) > { > + struct rev_info *rev = cb; > + > if (!strcmp(var, "format.headers")) { > if (!value) > die(_("format.headers without value")); > @@ -864,6 +866,20 @@ static int git_format_config(const char *var, const char *value, void *cb) > from = NULL; > return 0; > } > + if (!strcmp(var, "format.notes")) { > + struct strbuf buf = STRBUF_INIT; > + > + rev->show_notes = 1; > + if (!strcmp(value, "standard")) > + rev->notes_opt.use_default_notes = 1; > + else { Use braces on both parts of the "if", if one part needs them. > + strbuf_addstr(&buf, value); > + expand_notes_ref(&buf); > + string_list_append(&rev->notes_opt.extra_notes_refs, > + strbuf_detach(&buf, NULL)); > + } > + return 0; > + } > > return git_log_config(var, value, cb); > } > @@ -1617,8 +1633,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > extra_to.strdup_strings = 1; > extra_cc.strdup_strings = 1; > init_log_defaults(); > - git_config(git_format_config, NULL); > repo_init_revisions(the_repository, &rev, prefix); > + git_config(git_format_config, &rev); > rev.commit_format = CMIT_FMT_EMAIL; > rev.expand_tabs_in_log_default = 0; > rev.verbose_header = 1; > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > index b6e2fdbc44..e0127282ba 100755 > --- a/t/t4014-format-patch.sh > +++ b/t/t4014-format-patch.sh > @@ -738,6 +738,76 @@ test_expect_success 'format-patch --notes --signoff' ' > sed "1,/^---$/d" out | grep "test message" > ' > > +test_expect_success 'format-patch notes output control' ' > + git notes add -m "notes config message" HEAD && > + test_when_finished git notes remove HEAD && > + > + git format-patch -1 --stdout >out && > + ! grep "notes config message" out && > + git format-patch -1 --stdout --notes >out && > + grep "notes config message" out && > + git format-patch -1 --stdout --no-notes >out && > + ! grep "notes config message" out && > + git format-patch -1 --stdout --notes --no-notes >out && > + ! grep "notes config message" out && > + git format-patch -1 --stdout --no-notes --notes >out && > + grep "notes config message" out && > + > + test_config format.notes standard && > + git format-patch -1 --stdout >out && > + grep "notes config message" out && > + git format-patch -1 --stdout --notes >out && > + grep "notes config message" out && > + git format-patch -1 --stdout --no-notes >out && > + ! grep "notes config message" out && > + git format-patch -1 --stdout --notes --no-notes >out && > + ! grep "notes config message" out && > + git format-patch -1 --stdout --no-notes --notes >out && > + grep "notes config message" out > +' > + > +test_expect_success 'format-patch with multiple notes refs' ' > + git notes --ref note1 add -m "this is note 1" HEAD && > + test_when_finished git notes --ref note1 remove HEAD && > + git notes --ref note2 add -m "this is note 2" HEAD && > + test_when_finished git notes --ref note2 remove HEAD && > + > + git format-patch -1 --stdout >out && > + ! grep "this is note 1" out && > + ! grep "this is note 2" out && > + git format-patch -1 --stdout --notes=note1 >out && > + grep "this is note 1" out && > + ! grep "this is note 2" out && > + git format-patch -1 --stdout --notes=note2 >out && > + ! grep "this is note 1" out && > + grep "this is note 2" out && > + git format-patch -1 --stdout --notes=note1 --notes=note2 >out && > + grep "this is note 1" out && > + grep "this is note 2" out && > + > + test_config format.notes note1 && > + git format-patch -1 --stdout >out && > + grep "this is note 1" out && > + ! grep "this is note 2" out && > + git format-patch -1 --stdout --no-notes >out && > + ! grep "this is note 1" out && > + ! grep "this is note 2" out && > + git format-patch -1 --stdout --notes=note2 >out && > + grep "this is note 1" out && > + grep "this is note 2" out && > + git format-patch -1 --stdout --no-notes --notes=note2 >out && > + ! grep "this is note 1" out && > + grep "this is note 2" out && > + > + git config --add format.notes note2 && > + git format-patch -1 --stdout >out && > + grep "this is note 1" out && > + grep "this is note 2" out && > + git format-patch -1 --stdout --no-notes >out && > + ! grep "this is note 1" out && > + ! grep "this is note 2" out > +' > + > echo "fatal: --name-only does not make sense" > expect.name-only > echo "fatal: --name-status does not make sense" > expect.name-status > echo "fatal: --check does not make sense" > expect.check > Cheers, Beat ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] format-patch: teach format.notes config option 2019-05-08 17:18 ` Beat Bolli @ 2019-05-08 17:31 ` Denton Liu 2019-05-08 17:52 ` Beat Bolli 0 siblings, 1 reply; 15+ messages in thread From: Denton Liu @ 2019-05-08 17:31 UTC (permalink / raw) To: Beat Bolli; +Cc: Git Mailing List, Junio C Hamano Hi Beat, On Wed, May 08, 2019 at 07:18:18PM +0200, Beat Bolli wrote: > On 08.05.19 17:02, Denton Liu wrote: > > In git-format-patch, notes can be appended with the `--notes` option. > > However, this must be specified by the user on an > > invocation-by-invocation basis. If a user is not careful, it's possible > > that they may forget to include it and generate a patch series without > > notes. > > > > Teach git-format-patch the `format.notes` config option its value is a > > s/its/. Its/ > > > notes ref that will be automatically be appended. The special value of > > Drop the second "be ". Thanks, I should've read through the log message more carefully before sending. > > > "standard" can be used to specify the standard notes. This option is > > overridable with the `--no-notes` option in case a user wishes not to > > append notes. > > > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > > --- > > One thing I'm worried about is that I'm not really sure using "standard" > > as the special value is a good idea. Would "auto" be a better special > > value? > > > > Documentation/config/format.txt | 13 ++++++ > > Documentation/git-format-patch.txt | 3 ++ > > builtin/log.c | 18 +++++++- > > t/t4014-format-patch.sh | 70 ++++++++++++++++++++++++++++++ > > 4 files changed, 103 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt > > index dc77941c48..e25f9cfc61 100644 > > --- a/Documentation/config/format.txt > > +++ b/Documentation/config/format.txt > > @@ -85,3 +85,16 @@ format.outputDirectory:: > > format.useAutoBase:: > > A boolean value which lets you enable the `--base=auto` option of > > format-patch by default. > > + > > +format.notes:: > > + A ref which specifies where to get the notes (see > > + linkgit:git-notes[1]) that are appended for the commit after the > > + three-dash line. > > ++ > > +If the special value of "standard" is specified, then the standard notes > > +ref is used (i.e. the notes ref used by `git notes` when no `--ref` > > +argument is specified). If one wishes to use the ref > > +`ref/notes/standard`, please use that literal instead. > > ++ > > +This configuration can be specified multiple times in order to allow > > +multiple notes refs to be included. > > diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt > > index 2c3971390e..9ce5b8aaee 100644 > > --- a/Documentation/git-format-patch.txt > > +++ b/Documentation/git-format-patch.txt > > @@ -275,6 +275,9 @@ these explanations after `format-patch` has run but before sending, > > keeping them as Git notes allows them to be maintained between versions > > of the patch series (but see the discussion of the `notes.rewrite` > > configuration options in linkgit:git-notes[1] to use this workflow). > > ++ > > +The default is `--no-notes`, unless the `format.notes` configuration is > > +set. > > > > --[no-]signature=<signature>:: > > Add a signature to each message produced. Per RFC 3676 the signature > > diff --git a/builtin/log.c b/builtin/log.c > > index e43ee12fb1..24954e42b0 100644 > > --- a/builtin/log.c > > +++ b/builtin/log.c > > @@ -779,6 +779,8 @@ enum { > > > > static int git_format_config(const char *var, const char *value, void *cb) > > { > > + struct rev_info *rev = cb; > > + > > if (!strcmp(var, "format.headers")) { > > if (!value) > > die(_("format.headers without value")); > > @@ -864,6 +866,20 @@ static int git_format_config(const char *var, const char *value, void *cb) > > from = NULL; > > return 0; > > } > > + if (!strcmp(var, "format.notes")) { > > + struct strbuf buf = STRBUF_INIT; > > + > > + rev->show_notes = 1; > > + if (!strcmp(value, "standard")) > > + rev->notes_opt.use_default_notes = 1; > > + else { > > Use braces on both parts of the "if", if one part needs them. I thought that the style preference in this project is to not use braces whenever it's unnecessary, even if it's on just one arm of an if-else. Thanks, Denton > > > + strbuf_addstr(&buf, value); > > + expand_notes_ref(&buf); > > + string_list_append(&rev->notes_opt.extra_notes_refs, > > + strbuf_detach(&buf, NULL)); > > + } > > + return 0; > > + } > > > > return git_log_config(var, value, cb); > > } > > @@ -1617,8 +1633,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > > extra_to.strdup_strings = 1; > > extra_cc.strdup_strings = 1; > > init_log_defaults(); > > - git_config(git_format_config, NULL); > > repo_init_revisions(the_repository, &rev, prefix); > > + git_config(git_format_config, &rev); > > rev.commit_format = CMIT_FMT_EMAIL; > > rev.expand_tabs_in_log_default = 0; > > rev.verbose_header = 1; > > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > > index b6e2fdbc44..e0127282ba 100755 > > --- a/t/t4014-format-patch.sh > > +++ b/t/t4014-format-patch.sh > > @@ -738,6 +738,76 @@ test_expect_success 'format-patch --notes --signoff' ' > > sed "1,/^---$/d" out | grep "test message" > > ' > > > > +test_expect_success 'format-patch notes output control' ' > > + git notes add -m "notes config message" HEAD && > > + test_when_finished git notes remove HEAD && > > + > > + git format-patch -1 --stdout >out && > > + ! grep "notes config message" out && > > + git format-patch -1 --stdout --notes >out && > > + grep "notes config message" out && > > + git format-patch -1 --stdout --no-notes >out && > > + ! grep "notes config message" out && > > + git format-patch -1 --stdout --notes --no-notes >out && > > + ! grep "notes config message" out && > > + git format-patch -1 --stdout --no-notes --notes >out && > > + grep "notes config message" out && > > + > > + test_config format.notes standard && > > + git format-patch -1 --stdout >out && > > + grep "notes config message" out && > > + git format-patch -1 --stdout --notes >out && > > + grep "notes config message" out && > > + git format-patch -1 --stdout --no-notes >out && > > + ! grep "notes config message" out && > > + git format-patch -1 --stdout --notes --no-notes >out && > > + ! grep "notes config message" out && > > + git format-patch -1 --stdout --no-notes --notes >out && > > + grep "notes config message" out > > +' > > + > > +test_expect_success 'format-patch with multiple notes refs' ' > > + git notes --ref note1 add -m "this is note 1" HEAD && > > + test_when_finished git notes --ref note1 remove HEAD && > > + git notes --ref note2 add -m "this is note 2" HEAD && > > + test_when_finished git notes --ref note2 remove HEAD && > > + > > + git format-patch -1 --stdout >out && > > + ! grep "this is note 1" out && > > + ! grep "this is note 2" out && > > + git format-patch -1 --stdout --notes=note1 >out && > > + grep "this is note 1" out && > > + ! grep "this is note 2" out && > > + git format-patch -1 --stdout --notes=note2 >out && > > + ! grep "this is note 1" out && > > + grep "this is note 2" out && > > + git format-patch -1 --stdout --notes=note1 --notes=note2 >out && > > + grep "this is note 1" out && > > + grep "this is note 2" out && > > + > > + test_config format.notes note1 && > > + git format-patch -1 --stdout >out && > > + grep "this is note 1" out && > > + ! grep "this is note 2" out && > > + git format-patch -1 --stdout --no-notes >out && > > + ! grep "this is note 1" out && > > + ! grep "this is note 2" out && > > + git format-patch -1 --stdout --notes=note2 >out && > > + grep "this is note 1" out && > > + grep "this is note 2" out && > > + git format-patch -1 --stdout --no-notes --notes=note2 >out && > > + ! grep "this is note 1" out && > > + grep "this is note 2" out && > > + > > + git config --add format.notes note2 && > > + git format-patch -1 --stdout >out && > > + grep "this is note 1" out && > > + grep "this is note 2" out && > > + git format-patch -1 --stdout --no-notes >out && > > + ! grep "this is note 1" out && > > + ! grep "this is note 2" out > > +' > > + > > echo "fatal: --name-only does not make sense" > expect.name-only > > echo "fatal: --name-status does not make sense" > expect.name-status > > echo "fatal: --check does not make sense" > expect.check > > > > Cheers, Beat ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] format-patch: teach format.notes config option 2019-05-08 17:31 ` Denton Liu @ 2019-05-08 17:52 ` Beat Bolli 0 siblings, 0 replies; 15+ messages in thread From: Beat Bolli @ 2019-05-08 17:52 UTC (permalink / raw) To: Denton Liu; +Cc: Git Mailing List, Junio C Hamano On 08.05.19 19:31, Denton Liu wrote: > Hi Beat, > > On Wed, May 08, 2019 at 07:18:18PM +0200, Beat Bolli wrote: >> On 08.05.19 17:02, Denton Liu wrote: >>> In git-format-patch, notes can be appended with the `--notes` option. >>> However, this must be specified by the user on an >>> invocation-by-invocation basis. If a user is not careful, it's possible >>> that they may forget to include it and generate a patch series without >>> notes. >>> >>> Teach git-format-patch the `format.notes` config option its value is a >> >> s/its/. Its/ >> >>> notes ref that will be automatically be appended. The special value of >> >> Drop the second "be ". > > Thanks, I should've read through the log message more carefully before > sending. > >> >>> "standard" can be used to specify the standard notes. This option is >>> overridable with the `--no-notes` option in case a user wishes not to >>> append notes. >>> >>> Signed-off-by: Denton Liu <liu.denton@gmail.com> >>> --- >>> One thing I'm worried about is that I'm not really sure using "standard" >>> as the special value is a good idea. Would "auto" be a better special >>> value? >>> >>> Documentation/config/format.txt | 13 ++++++ >>> Documentation/git-format-patch.txt | 3 ++ >>> builtin/log.c | 18 +++++++- >>> t/t4014-format-patch.sh | 70 ++++++++++++++++++++++++++++++ >>> 4 files changed, 103 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt >>> index dc77941c48..e25f9cfc61 100644 >>> --- a/Documentation/config/format.txt >>> +++ b/Documentation/config/format.txt >>> @@ -85,3 +85,16 @@ format.outputDirectory:: >>> format.useAutoBase:: >>> A boolean value which lets you enable the `--base=auto` option of >>> format-patch by default. >>> + >>> +format.notes:: >>> + A ref which specifies where to get the notes (see >>> + linkgit:git-notes[1]) that are appended for the commit after the >>> + three-dash line. >>> ++ >>> +If the special value of "standard" is specified, then the standard notes >>> +ref is used (i.e. the notes ref used by `git notes` when no `--ref` >>> +argument is specified). If one wishes to use the ref >>> +`ref/notes/standard`, please use that literal instead. >>> ++ >>> +This configuration can be specified multiple times in order to allow >>> +multiple notes refs to be included. >>> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt >>> index 2c3971390e..9ce5b8aaee 100644 >>> --- a/Documentation/git-format-patch.txt >>> +++ b/Documentation/git-format-patch.txt >>> @@ -275,6 +275,9 @@ these explanations after `format-patch` has run but before sending, >>> keeping them as Git notes allows them to be maintained between versions >>> of the patch series (but see the discussion of the `notes.rewrite` >>> configuration options in linkgit:git-notes[1] to use this workflow). >>> ++ >>> +The default is `--no-notes`, unless the `format.notes` configuration is >>> +set. >>> >>> --[no-]signature=<signature>:: >>> Add a signature to each message produced. Per RFC 3676 the signature >>> diff --git a/builtin/log.c b/builtin/log.c >>> index e43ee12fb1..24954e42b0 100644 >>> --- a/builtin/log.c >>> +++ b/builtin/log.c >>> @@ -779,6 +779,8 @@ enum { >>> >>> static int git_format_config(const char *var, const char *value, void *cb) >>> { >>> + struct rev_info *rev = cb; >>> + >>> if (!strcmp(var, "format.headers")) { >>> if (!value) >>> die(_("format.headers without value")); >>> @@ -864,6 +866,20 @@ static int git_format_config(const char *var, const char *value, void *cb) >>> from = NULL; >>> return 0; >>> } >>> + if (!strcmp(var, "format.notes")) { >>> + struct strbuf buf = STRBUF_INIT; >>> + >>> + rev->show_notes = 1; >>> + if (!strcmp(value, "standard")) >>> + rev->notes_opt.use_default_notes = 1; >>> + else { >> >> Use braces on both parts of the "if", if one part needs them. > > I thought that the style preference in this project is to not use braces > whenever it's unnecessary, even if it's on just one arm of an if-else. No, Documentation/CodingGuidelines mentions this case explicitly. Search for "multiple arms". ATB, Beat ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 0/2] format-patch: teach format.notes config option 2019-05-08 15:02 ` [PATCH v2 0/2] " Denton Liu 2019-05-08 15:02 ` [PATCH v2 1/2] git-format-patch.txt: document --no-notes option Denton Liu 2019-05-08 15:02 ` [PATCH v2 2/2] format-patch: teach format.notes config option Denton Liu @ 2019-05-10 18:37 ` Denton Liu 2019-05-10 18:37 ` [PATCH v3 1/2] git-format-patch.txt: document --no-notes option Denton Liu 2019-05-10 18:37 ` [PATCH v3 2/2] format-patch: teach format.notes config option Denton Liu 2 siblings, 2 replies; 15+ messages in thread From: Denton Liu @ 2019-05-10 18:37 UTC (permalink / raw) To: Git Mailing List; +Cc: Junio C Hamano, Beat Bolli Hi Beat, thanks for catching the style errors. This version fixes those. Changes since v2: * Fixed if-else code style * Fixed typoed errors in 2/2 log message Changes since v1: * Made format.notes accept a notes ref instead of a boolean Denton Liu (2): git-format-patch.txt: document --no-notes option format-patch: teach format.notes config option Documentation/config/format.txt | 13 ++++++ Documentation/git-format-patch.txt | 7 ++- builtin/log.c | 18 +++++++- t/t4014-format-patch.sh | 70 ++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 2 deletions(-) Range-diff against v2: 1: 4c3535f25b = 1: 4c3535f25b git-format-patch.txt: document --no-notes option 2: fe674bf63e ! 2: df864c4adf format-patch: teach format.notes config option @@ -8,8 +8,8 @@ that they may forget to include it and generate a patch series without notes. - Teach git-format-patch the `format.notes` config option its value is a - notes ref that will be automatically be appended. The special value of + Teach git-format-patch the `format.notes` config option. Its value is a + notes ref that will be automatically appended. The special value of "standard" can be used to specify the standard notes. This option is overridable with the `--no-notes` option in case a user wishes not to append notes. @@ -71,9 +71,9 @@ + struct strbuf buf = STRBUF_INIT; + + rev->show_notes = 1; -+ if (!strcmp(value, "standard")) ++ if (!strcmp(value, "standard")) { + rev->notes_opt.use_default_notes = 1; -+ else { ++ } else { + strbuf_addstr(&buf, value); + expand_notes_ref(&buf); + string_list_append(&rev->notes_opt.extra_notes_refs, -- 2.21.0.1049.geb646f7864 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 1/2] git-format-patch.txt: document --no-notes option 2019-05-10 18:37 ` [PATCH v3 0/2] " Denton Liu @ 2019-05-10 18:37 ` Denton Liu 2019-05-10 18:37 ` [PATCH v3 2/2] format-patch: teach format.notes config option Denton Liu 1 sibling, 0 replies; 15+ messages in thread From: Denton Liu @ 2019-05-10 18:37 UTC (permalink / raw) To: Git Mailing List; +Cc: Junio C Hamano, Beat Bolli Internally, git-format-patch uses the `handle_revision_opt` parser. The parser handles the `--no-notes` option to negate an earlier `--notes` option, but it isn't documented. Document this option so that users are aware of it. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Documentation/git-format-patch.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 1af85d404f..2c3971390e 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -22,7 +22,8 @@ SYNOPSIS [--rfc] [--subject-prefix=Subject-Prefix] [(--reroll-count|-v) <n>] [--to=<email>] [--cc=<email>] - [--[no-]cover-letter] [--quiet] [--notes[=<ref>]] + [--[no-]cover-letter] [--quiet] + [--no-notes | --notes[=<ref>]] [--interdiff=<previous>] [--range-diff=<previous> [--creation-factor=<percent>]] [--progress] @@ -263,6 +264,7 @@ material (this may change in the future). for details. --notes[=<ref>]:: +--no-notes:: Append the notes (see linkgit:git-notes[1]) for the commit after the three-dash line. + -- 2.21.0.1049.geb646f7864 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 2/2] format-patch: teach format.notes config option 2019-05-10 18:37 ` [PATCH v3 0/2] " Denton Liu 2019-05-10 18:37 ` [PATCH v3 1/2] git-format-patch.txt: document --no-notes option Denton Liu @ 2019-05-10 18:37 ` Denton Liu 2019-05-13 2:44 ` Junio C Hamano 1 sibling, 1 reply; 15+ messages in thread From: Denton Liu @ 2019-05-10 18:37 UTC (permalink / raw) To: Git Mailing List; +Cc: Junio C Hamano, Beat Bolli In git-format-patch, notes can be appended with the `--notes` option. However, this must be specified by the user on an invocation-by-invocation basis. If a user is not careful, it's possible that they may forget to include it and generate a patch series without notes. Teach git-format-patch the `format.notes` config option. Its value is a notes ref that will be automatically appended. The special value of "standard" can be used to specify the standard notes. This option is overridable with the `--no-notes` option in case a user wishes not to append notes. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Documentation/config/format.txt | 13 ++++++ Documentation/git-format-patch.txt | 3 ++ builtin/log.c | 18 +++++++- t/t4014-format-patch.sh | 70 ++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 1 deletion(-) diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt index dc77941c48..e25f9cfc61 100644 --- a/Documentation/config/format.txt +++ b/Documentation/config/format.txt @@ -85,3 +85,16 @@ format.outputDirectory:: format.useAutoBase:: A boolean value which lets you enable the `--base=auto` option of format-patch by default. + +format.notes:: + A ref which specifies where to get the notes (see + linkgit:git-notes[1]) that are appended for the commit after the + three-dash line. ++ +If the special value of "standard" is specified, then the standard notes +ref is used (i.e. the notes ref used by `git notes` when no `--ref` +argument is specified). If one wishes to use the ref +`ref/notes/standard`, please use that literal instead. ++ +This configuration can be specified multiple times in order to allow +multiple notes refs to be included. diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 2c3971390e..9ce5b8aaee 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -275,6 +275,9 @@ these explanations after `format-patch` has run but before sending, keeping them as Git notes allows them to be maintained between versions of the patch series (but see the discussion of the `notes.rewrite` configuration options in linkgit:git-notes[1] to use this workflow). ++ +The default is `--no-notes`, unless the `format.notes` configuration is +set. --[no-]signature=<signature>:: Add a signature to each message produced. Per RFC 3676 the signature diff --git a/builtin/log.c b/builtin/log.c index e43ee12fb1..9d26e0f248 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -779,6 +779,8 @@ enum { static int git_format_config(const char *var, const char *value, void *cb) { + struct rev_info *rev = cb; + if (!strcmp(var, "format.headers")) { if (!value) die(_("format.headers without value")); @@ -864,6 +866,20 @@ static int git_format_config(const char *var, const char *value, void *cb) from = NULL; return 0; } + if (!strcmp(var, "format.notes")) { + struct strbuf buf = STRBUF_INIT; + + rev->show_notes = 1; + if (!strcmp(value, "standard")) { + rev->notes_opt.use_default_notes = 1; + } else { + strbuf_addstr(&buf, value); + expand_notes_ref(&buf); + string_list_append(&rev->notes_opt.extra_notes_refs, + strbuf_detach(&buf, NULL)); + } + return 0; + } return git_log_config(var, value, cb); } @@ -1617,8 +1633,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) extra_to.strdup_strings = 1; extra_cc.strdup_strings = 1; init_log_defaults(); - git_config(git_format_config, NULL); repo_init_revisions(the_repository, &rev, prefix); + git_config(git_format_config, &rev); rev.commit_format = CMIT_FMT_EMAIL; rev.expand_tabs_in_log_default = 0; rev.verbose_header = 1; diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index b6e2fdbc44..e0127282ba 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -738,6 +738,76 @@ test_expect_success 'format-patch --notes --signoff' ' sed "1,/^---$/d" out | grep "test message" ' +test_expect_success 'format-patch notes output control' ' + git notes add -m "notes config message" HEAD && + test_when_finished git notes remove HEAD && + + git format-patch -1 --stdout >out && + ! grep "notes config message" out && + git format-patch -1 --stdout --notes >out && + grep "notes config message" out && + git format-patch -1 --stdout --no-notes >out && + ! grep "notes config message" out && + git format-patch -1 --stdout --notes --no-notes >out && + ! grep "notes config message" out && + git format-patch -1 --stdout --no-notes --notes >out && + grep "notes config message" out && + + test_config format.notes standard && + git format-patch -1 --stdout >out && + grep "notes config message" out && + git format-patch -1 --stdout --notes >out && + grep "notes config message" out && + git format-patch -1 --stdout --no-notes >out && + ! grep "notes config message" out && + git format-patch -1 --stdout --notes --no-notes >out && + ! grep "notes config message" out && + git format-patch -1 --stdout --no-notes --notes >out && + grep "notes config message" out +' + +test_expect_success 'format-patch with multiple notes refs' ' + git notes --ref note1 add -m "this is note 1" HEAD && + test_when_finished git notes --ref note1 remove HEAD && + git notes --ref note2 add -m "this is note 2" HEAD && + test_when_finished git notes --ref note2 remove HEAD && + + git format-patch -1 --stdout >out && + ! grep "this is note 1" out && + ! grep "this is note 2" out && + git format-patch -1 --stdout --notes=note1 >out && + grep "this is note 1" out && + ! grep "this is note 2" out && + git format-patch -1 --stdout --notes=note2 >out && + ! grep "this is note 1" out && + grep "this is note 2" out && + git format-patch -1 --stdout --notes=note1 --notes=note2 >out && + grep "this is note 1" out && + grep "this is note 2" out && + + test_config format.notes note1 && + git format-patch -1 --stdout >out && + grep "this is note 1" out && + ! grep "this is note 2" out && + git format-patch -1 --stdout --no-notes >out && + ! grep "this is note 1" out && + ! grep "this is note 2" out && + git format-patch -1 --stdout --notes=note2 >out && + grep "this is note 1" out && + grep "this is note 2" out && + git format-patch -1 --stdout --no-notes --notes=note2 >out && + ! grep "this is note 1" out && + grep "this is note 2" out && + + git config --add format.notes note2 && + git format-patch -1 --stdout >out && + grep "this is note 1" out && + grep "this is note 2" out && + git format-patch -1 --stdout --no-notes >out && + ! grep "this is note 1" out && + ! grep "this is note 2" out +' + echo "fatal: --name-only does not make sense" > expect.name-only echo "fatal: --name-status does not make sense" > expect.name-status echo "fatal: --check does not make sense" > expect.check -- 2.21.0.1049.geb646f7864 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/2] format-patch: teach format.notes config option 2019-05-10 18:37 ` [PATCH v3 2/2] format-patch: teach format.notes config option Denton Liu @ 2019-05-13 2:44 ` Junio C Hamano 2019-05-14 17:01 ` Denton Liu 0 siblings, 1 reply; 15+ messages in thread From: Junio C Hamano @ 2019-05-13 2:44 UTC (permalink / raw) To: Denton Liu; +Cc: Git Mailing List, Beat Bolli Denton Liu <liu.denton@gmail.com> writes: > @@ -864,6 +866,20 @@ static int git_format_config(const char *var, const char *value, void *cb) > from = NULL; > return 0; > } > + if (!strcmp(var, "format.notes")) { > + struct strbuf buf = STRBUF_INIT; > + > + rev->show_notes = 1; > + if (!strcmp(value, "standard")) { > + rev->notes_opt.use_default_notes = 1; > + } else { > + strbuf_addstr(&buf, value); > + expand_notes_ref(&buf); > + string_list_append(&rev->notes_opt.extra_notes_refs, > + strbuf_detach(&buf, NULL)); > + } > + return 0; > + } Unlike the command line option parser, this does not seem to touch rev->show_notes_given at all. Intended? I am wondering how well this implementation meshes with what 66b2ed09 ("Fix "log" family not to be too agressive about showing notes", 2010-01-20) wanted to do, which 894a9d33 ("Support showing notes from more than one notes tree", 2010-03-12) later extended. > +test_expect_success 'format-patch notes output control' ' > + git notes add -m "notes config message" HEAD && > + test_when_finished git notes remove HEAD && > + > + git format-patch -1 --stdout >out && > + ! grep "notes config message" out && > + git format-patch -1 --stdout --notes >out && > + grep "notes config message" out && > + git format-patch -1 --stdout --no-notes >out && > + ! grep "notes config message" out && > + git format-patch -1 --stdout --notes --no-notes >out && > + ! grep "notes config message" out && > + git format-patch -1 --stdout --no-notes --notes >out && > + grep "notes config message" out && > + > + test_config format.notes standard && I think we tend to spell these things "default". Alternatively, the format.notes configuration can be "bool or text", and make the variable set to 'true' mean "show notes, using the default ref". > + git format-patch -1 --stdout >out && > + grep "notes config message" out && > + git format-patch -1 --stdout --notes >out && > + grep "notes config message" out && > + git format-patch -1 --stdout --no-notes >out && > + ! grep "notes config message" out && > + git format-patch -1 --stdout --notes --no-notes >out && > + ! grep "notes config message" out && > + git format-patch -1 --stdout --no-notes --notes >out && > + grep "notes config message" out > +' OK. > +test_expect_success 'format-patch with multiple notes refs' ' > + git notes --ref note1 add -m "this is note 1" HEAD && > + test_when_finished git notes --ref note1 remove HEAD && > + git notes --ref note2 add -m "this is note 2" HEAD && > + test_when_finished git notes --ref note2 remove HEAD && > + ... > + git format-patch -1 --stdout --notes=note1 --notes=note2 >out && > + grep "this is note 1" out && > + grep "this is note 2" out && Do we promise the order in which these two lines appear in the output? > + test_config format.notes note1 && > + git format-patch -1 --stdout >out && > + grep "this is note 1" out && > + ! grep "this is note 2" out && > + git format-patch -1 --stdout --no-notes >out && > + ! grep "this is note 1" out && > + ! grep "this is note 2" out && > + git format-patch -1 --stdout --notes=note2 >out && > + grep "this is note 1" out && > + grep "this is note 2" out && So format.notes say note1 but the command line explicitly asks it wants note from note2, but the command still gives from note1 anyway. > + git format-patch -1 --stdout --no-notes --notes=note2 >out && > + ! grep "this is note 1" out && > + grep "this is note 2" out && And there is a way to work it around, i.e. clear everything configured with --no-notes and then name the one you want from the command line. I am not sure if the above is consistent with how our options and configurations interact in general. Shouldn't the --notes=note2 alone in the earlier example cancel format.notes=note1 configured? > + git config --add format.notes note2 && > + git format-patch -1 --stdout >out && > + grep "this is note 1" out && > + grep "this is note 2" out && > + git format-patch -1 --stdout --no-notes >out && > + ! grep "this is note 1" out && > + ! grep "this is note 2" out > +' > + > echo "fatal: --name-only does not make sense" > expect.name-only > echo "fatal: --name-status does not make sense" > expect.name-status > echo "fatal: --check does not make sense" > expect.check ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/2] format-patch: teach format.notes config option 2019-05-13 2:44 ` Junio C Hamano @ 2019-05-14 17:01 ` Denton Liu 0 siblings, 0 replies; 15+ messages in thread From: Denton Liu @ 2019-05-14 17:01 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List, Beat Bolli Hi Junio, On Mon, May 13, 2019 at 11:44:21AM +0900, Junio C Hamano wrote: > Denton Liu <liu.denton@gmail.com> writes: > > > @@ -864,6 +866,20 @@ static int git_format_config(const char *var, const char *value, void *cb) > > from = NULL; > > return 0; > > } > > + if (!strcmp(var, "format.notes")) { > > + struct strbuf buf = STRBUF_INIT; > > + > > + rev->show_notes = 1; > > + if (!strcmp(value, "standard")) { > > + rev->notes_opt.use_default_notes = 1; > > + } else { > > + strbuf_addstr(&buf, value); > > + expand_notes_ref(&buf); > > + string_list_append(&rev->notes_opt.extra_notes_refs, > > + strbuf_detach(&buf, NULL)); > > + } > > + return 0; > > + } > > Unlike the command line option parser, this does not seem to touch > rev->show_notes_given at all. Intended? I am wondering how well > this implementation meshes with what 66b2ed09 ("Fix "log" family not > to be too agressive about showing notes", 2010-01-20) wanted to do, > which 894a9d33 ("Support showing notes from more than one notes > tree", 2010-03-12) later extended. This was intended but I'm not 100% sure that it's correct. From what I could gleam from reading the code, `show_notes_given` is only used by the `cmd_log_init` function, which is not called by format-patch. As a result, I opted to not set that flag since it's not really "given" in the sense that a user didn't explicitly pass in a command-line option indicating they wanted notes. > > > +test_expect_success 'format-patch notes output control' ' > > + git notes add -m "notes config message" HEAD && > > + test_when_finished git notes remove HEAD && > > + > > + git format-patch -1 --stdout >out && > > + ! grep "notes config message" out && > > + git format-patch -1 --stdout --notes >out && > > + grep "notes config message" out && > > + git format-patch -1 --stdout --no-notes >out && > > + ! grep "notes config message" out && > > + git format-patch -1 --stdout --notes --no-notes >out && > > + ! grep "notes config message" out && > > + git format-patch -1 --stdout --no-notes --notes >out && > > + grep "notes config message" out && > > + > > + test_config format.notes standard && > > I think we tend to spell these things "default". > > Alternatively, the format.notes configuration can be "bool or text", > and make the variable set to 'true' mean "show notes, using the > default ref". I think I'l go with this approach. > > > + git format-patch -1 --stdout >out && > > + grep "notes config message" out && > > + git format-patch -1 --stdout --notes >out && > > + grep "notes config message" out && > > + git format-patch -1 --stdout --no-notes >out && > > + ! grep "notes config message" out && > > + git format-patch -1 --stdout --notes --no-notes >out && > > + ! grep "notes config message" out && > > + git format-patch -1 --stdout --no-notes --notes >out && > > + grep "notes config message" out > > +' > > OK. > > > +test_expect_success 'format-patch with multiple notes refs' ' > > + git notes --ref note1 add -m "this is note 1" HEAD && > > + test_when_finished git notes --ref note1 remove HEAD && > > + git notes --ref note2 add -m "this is note 2" HEAD && > > + test_when_finished git notes --ref note2 remove HEAD && > > + ... > > + git format-patch -1 --stdout --notes=note1 --notes=note2 >out && > > + grep "this is note 1" out && > > + grep "this is note 2" out && > > Do we promise the order in which these two lines appear in the output? According to the code, the order is stable. However, I just read through the documentation and I realised that the ablility to provide multiple notes refs is undocumented. In a future reroll, I'll document the fact that --notes can be provided multiple times. > > > + test_config format.notes note1 && > > + git format-patch -1 --stdout >out && > > + grep "this is note 1" out && > > + ! grep "this is note 2" out && > > + git format-patch -1 --stdout --no-notes >out && > > + ! grep "this is note 1" out && > > + ! grep "this is note 2" out && > > + git format-patch -1 --stdout --notes=note2 >out && > > + grep "this is note 1" out && > > + grep "this is note 2" out && > > So format.notes say note1 but the command line explicitly asks it > wants note from note2, but the command still gives from note1 > anyway. > > > + git format-patch -1 --stdout --no-notes --notes=note2 >out && > > + ! grep "this is note 1" out && > > + grep "this is note 2" out && > > And there is a way to work it around, i.e. clear everything > configured with --no-notes and then name the one you want from the > command line. > > I am not sure if the above is consistent with how our options and > configurations interact in general. Shouldn't the --notes=note2 > alone in the earlier example cancel format.notes=note1 configured? I borrowed this behaviour from how format.to behaves. In format-patch, `--to` gives a recipient that is used _in addition_ to any format.to variables. `--no-to` can override this. I made format.notes behave similarly. > > > + git config --add format.notes note2 && > > + git format-patch -1 --stdout >out && > > + grep "this is note 1" out && > > + grep "this is note 2" out && > > + git format-patch -1 --stdout --no-notes >out && > > + ! grep "this is note 1" out && > > + ! grep "this is note 2" out > > +' > > + > > echo "fatal: --name-only does not make sense" > expect.name-only > > echo "fatal: --name-status does not make sense" > expect.name-status > > echo "fatal: --check does not make sense" > expect.check ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-05-14 17:01 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-27 19:25 [PATCH 0/2] format-patch: teach format.notes config option Denton Liu 2019-04-27 19:25 ` [PATCH 1/2] git-format-patch.txt: document --no-notes option Denton Liu 2019-04-27 19:25 ` [PATCH 2/2] format-patch: teach format.notes config option Denton Liu 2019-05-07 4:43 ` Junio C Hamano 2019-05-08 15:02 ` [PATCH v2 0/2] " Denton Liu 2019-05-08 15:02 ` [PATCH v2 1/2] git-format-patch.txt: document --no-notes option Denton Liu 2019-05-08 15:02 ` [PATCH v2 2/2] format-patch: teach format.notes config option Denton Liu 2019-05-08 17:18 ` Beat Bolli 2019-05-08 17:31 ` Denton Liu 2019-05-08 17:52 ` Beat Bolli 2019-05-10 18:37 ` [PATCH v3 0/2] " Denton Liu 2019-05-10 18:37 ` [PATCH v3 1/2] git-format-patch.txt: document --no-notes option Denton Liu 2019-05-10 18:37 ` [PATCH v3 2/2] format-patch: teach format.notes config option Denton Liu 2019-05-13 2:44 ` Junio C Hamano 2019-05-14 17:01 ` Denton Liu
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).