{"thread":{"id":"51110","subject":"[PATCH v4 0/2] format-patch: teach format.notes config option","startedAt":"2019-05-16T23:13:49Z","lastAt":"2019-12-09T08:24:57Z","messageCount":5,"participants":["Denton Liu","Elijah Newren"],"isPatch":true,"patchVersion":4,"patchTotal":2},"messages":[{"id":"375760","messageId":"cover.1558048212.git.liu.denton@gmail.com","threadId":"51110","inReplyTo":null,"subject":"[PATCH v4 0/2] format-patch: teach format.notes config option","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-05-16T23:13:44Z","receivedAt":"2019-05-16T23:13:49Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Junio,\n\nI've changed format.notes so that it can accept a boolean as well.\nHopefully, my last email has addressed the remainder of your comments.\n\nChanges since v3:\n\n* Made format.notes accept a boolean instead of \"standard\" to get\n  default notes\n\nChanges since v2:\n\n* Fixed if-else code style\n* Fixed typoed errors in 2/2 log message\n\nChanges since v1:\n\n* Made format.notes accept a notes ref instead of a boolean\n\n\nDenton Liu (2):\n  git-format-patch.txt: document --no-notes option\n  format-patch: teach format.notes config option\n\n Documentation/config/format.txt    | 15 +++++++\n Documentation/git-format-patch.txt |  7 ++-\n builtin/log.c                      | 20 ++++++++-\n t/t4014-format-patch.sh            | 70 ++++++++++++++++++++++++++++++\n 4 files changed, 110 insertions(+), 2 deletions(-)\n\nRange-diff against v3:\n1:  4c3535f25b = 1:  4c3535f25b git-format-patch.txt: document --no-notes option\n2:  df864c4adf ! 2:  7cb770e190 format-patch: teach format.notes config option\n    @@ -25,14 +25,16 @@\n      \tformat-patch by default.\n     +\n     +format.notes::\n    -+\tA ref which specifies where to get the notes (see\n    -+\tlinkgit:git-notes[1]) that are appended for the commit after the\n    -+\tthree-dash line.\n    ++\tProvides the default value for the `--notes` option to\n    ++\tformat-patch. Accepts a boolean value, or a ref which specifies\n    ++\twhere to get notes. If false, format-patch defaults to\n    ++\t`--no-notes`. If true, format-patch defaults to `--notes`. If\n    ++\tset to a non-boolean value, format-patch defaults to\n    ++\t`--notes=<ref>`, where `ref` is the non-boolean value. Defaults\n    ++\tto false.\n     ++\n    -+If the special value of \"standard\" is specified, then the standard notes\n    -+ref is used (i.e. the notes ref used by `git notes` when no `--ref`\n    -+argument is specified). If one wishes to use the ref\n    -+`ref/notes/standard`, please use that literal instead.\n    ++If one wishes to use the ref `ref/notes/true`, please use that literal\n    ++instead.\n     ++\n     +This configuration can be specified multiple times in order to allow\n     +multiple notes refs to be included.\n    @@ -69,15 +71,17 @@\n      \t}\n     +\tif (!strcmp(var, \"format.notes\")) {\n     +\t\tstruct strbuf buf = STRBUF_INIT;\n    -+\n    ++\t\tint b = git_parse_maybe_bool(value);\n    ++\t\tif (!b)\n    ++\t\t\treturn 0;\n     +\t\trev->show_notes = 1;\n    -+\t\tif (!strcmp(value, \"standard\")) {\n    -+\t\t\trev->notes_opt.use_default_notes = 1;\n    -+\t\t} else {\n    ++\t\tif (b < 0) {\n     +\t\t\tstrbuf_addstr(&buf, value);\n     +\t\t\texpand_notes_ref(&buf);\n     +\t\t\tstring_list_append(&rev->notes_opt.extra_notes_refs,\n     +\t\t\t\t\tstrbuf_detach(&buf, NULL));\n    ++\t\t} else {\n    ++\t\t\trev->notes_opt.use_default_notes = 1;\n     +\t\t}\n     +\t\treturn 0;\n     +\t}\n    @@ -117,7 +121,7 @@\n     +\tgit format-patch -1 --stdout --no-notes --notes >out &&\n     +\tgrep \"notes config message\" out &&\n     +\n    -+\ttest_config format.notes standard &&\n    ++\ttest_config format.notes true &&\n     +\tgit format-patch -1 --stdout >out &&\n     +\tgrep \"notes config message\" out &&\n     +\tgit format-patch -1 --stdout --notes >out &&\n-- \n2.21.0.1049.geb646f7864\n\n"},{"id":"375761","messageId":"4c3535f25bdffc8d94617c74f7c078cce59a044f.1558048212.git.liu.denton@gmail.com","threadId":"51110","inReplyTo":"cover.1558048212.git.liu.denton@gmail.com","subject":"[PATCH v4 1/2] git-format-patch.txt: document --no-notes option","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-05-16T23:13:47Z","receivedAt":"2019-05-16T23:13:51Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Internally, git-format-patch uses the `handle_revision_opt` parser. The\nparser handles the `--no-notes` option to negate an earlier `--notes`\noption, but it isn't documented. Document this option so that users are\naware of it.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n Documentation/git-format-patch.txt | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 1af85d404f..2c3971390e 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -22,7 +22,8 @@ SYNOPSIS\n \t\t   [--rfc] [--subject-prefix=Subject-Prefix]\n \t\t   [(--reroll-count|-v) <n>]\n \t\t   [--to=<email>] [--cc=<email>]\n-\t\t   [--[no-]cover-letter] [--quiet] [--notes[=<ref>]]\n+\t\t   [--[no-]cover-letter] [--quiet]\n+\t\t   [--no-notes | --notes[=<ref>]]\n \t\t   [--interdiff=<previous>]\n \t\t   [--range-diff=<previous> [--creation-factor=<percent>]]\n \t\t   [--progress]\n@@ -263,6 +264,7 @@ material (this may change in the future).\n \tfor details.\n \n --notes[=<ref>]::\n+--no-notes::\n \tAppend the notes (see linkgit:git-notes[1]) for the commit\n \tafter the three-dash line.\n +\n-- \n2.21.0.1049.geb646f7864\n\n"},{"id":"375762","messageId":"7cb770e190f235accf50e4af3ec9a964f26358c5.1558048212.git.liu.denton@gmail.com","threadId":"51110","inReplyTo":"cover.1558048212.git.liu.denton@gmail.com","subject":"[PATCH v4 2/2] format-patch: teach format.notes config option","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-05-16T23:14:14Z","receivedAt":"2019-05-16T23:14:19Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In git-format-patch, notes can be appended with the `--notes` option.\nHowever, this must be specified by the user on an\ninvocation-by-invocation basis. If a user is not careful, it's possible\nthat they may forget to include it and generate a patch series without\nnotes.\n\nTeach git-format-patch the `format.notes` config option. Its value is a\nnotes ref that will be automatically appended. The special value of\n\"standard\" can be used to specify the standard notes. This option is\noverridable with the `--no-notes` option in case a user wishes not to\nappend notes.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n Documentation/config/format.txt    | 15 +++++++\n Documentation/git-format-patch.txt |  3 ++\n builtin/log.c                      | 20 ++++++++-\n t/t4014-format-patch.sh            | 70 ++++++++++++++++++++++++++++++\n 4 files changed, 107 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/format.txt b/Documentation/config/format.txt\nindex dc77941c48..414a5a8a9d 100644\n--- a/Documentation/config/format.txt\n+++ b/Documentation/config/format.txt\n@@ -85,3 +85,18 @@ format.outputDirectory::\n format.useAutoBase::\n \tA boolean value which lets you enable the `--base=auto` option of\n \tformat-patch by default.\n+\n+format.notes::\n+\tProvides the default value for the `--notes` option to\n+\tformat-patch. Accepts a boolean value, or a ref which specifies\n+\twhere to get notes. If false, format-patch defaults to\n+\t`--no-notes`. If true, format-patch defaults to `--notes`. If\n+\tset to a non-boolean value, format-patch defaults to\n+\t`--notes=<ref>`, where `ref` is the non-boolean value. Defaults\n+\tto false.\n++\n+If one wishes to use the ref `ref/notes/true`, please use that literal\n+instead.\n++\n+This configuration can be specified multiple times in order to allow\n+multiple notes refs to be included.\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 2c3971390e..9ce5b8aaee 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -275,6 +275,9 @@ these explanations after `format-patch` has run but before sending,\n keeping them as Git notes allows them to be maintained between versions\n of the patch series (but see the discussion of the `notes.rewrite`\n configuration options in linkgit:git-notes[1] to use this workflow).\n++\n+The default is `--no-notes`, unless the `format.notes` configuration is\n+set.\n \n --[no-]signature=<signature>::\n \tAdd a signature to each message produced. Per RFC 3676 the signature\ndiff --git a/builtin/log.c b/builtin/log.c\nindex e43ee12fb1..dad63cffc6 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -779,6 +779,8 @@ enum {\n \n static int git_format_config(const char *var, const char *value, void *cb)\n {\n+\tstruct rev_info *rev = cb;\n+\n \tif (!strcmp(var, \"format.headers\")) {\n \t\tif (!value)\n \t\t\tdie(_(\"format.headers without value\"));\n@@ -864,6 +866,22 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \t\t\tfrom = NULL;\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"format.notes\")) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tint b = git_parse_maybe_bool(value);\n+\t\tif (!b)\n+\t\t\treturn 0;\n+\t\trev->show_notes = 1;\n+\t\tif (b < 0) {\n+\t\t\tstrbuf_addstr(&buf, value);\n+\t\t\texpand_notes_ref(&buf);\n+\t\t\tstring_list_append(&rev->notes_opt.extra_notes_refs,\n+\t\t\t\t\tstrbuf_detach(&buf, NULL));\n+\t\t} else {\n+\t\t\trev->notes_opt.use_default_notes = 1;\n+\t\t}\n+\t\treturn 0;\n+\t}\n \n \treturn git_log_config(var, value, cb);\n }\n@@ -1617,8 +1635,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \textra_to.strdup_strings = 1;\n \textra_cc.strdup_strings = 1;\n \tinit_log_defaults();\n-\tgit_config(git_format_config, NULL);\n \trepo_init_revisions(the_repository, &rev, prefix);\n+\tgit_config(git_format_config, &rev);\n \trev.commit_format = CMIT_FMT_EMAIL;\n \trev.expand_tabs_in_log_default = 0;\n \trev.verbose_header = 1;\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex b6e2fdbc44..4d5719fe2c 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -738,6 +738,76 @@ test_expect_success 'format-patch --notes --signoff' '\n \tsed \"1,/^---$/d\" out | grep \"test message\"\n '\n \n+test_expect_success 'format-patch notes output control' '\n+\tgit notes add -m \"notes config message\" HEAD &&\n+\ttest_when_finished git notes remove HEAD &&\n+\n+\tgit format-patch -1 --stdout >out &&\n+\t! grep \"notes config message\" out &&\n+\tgit format-patch -1 --stdout --notes >out &&\n+\tgrep \"notes config message\" out &&\n+\tgit format-patch -1 --stdout --no-notes >out &&\n+\t! grep \"notes config message\" out &&\n+\tgit format-patch -1 --stdout --notes --no-notes >out &&\n+\t! grep \"notes config message\" out &&\n+\tgit format-patch -1 --stdout --no-notes --notes >out &&\n+\tgrep \"notes config message\" out &&\n+\n+\ttest_config format.notes true &&\n+\tgit format-patch -1 --stdout >out &&\n+\tgrep \"notes config message\" out &&\n+\tgit format-patch -1 --stdout --notes >out &&\n+\tgrep \"notes config message\" out &&\n+\tgit format-patch -1 --stdout --no-notes >out &&\n+\t! grep \"notes config message\" out &&\n+\tgit format-patch -1 --stdout --notes --no-notes >out &&\n+\t! grep \"notes config message\" out &&\n+\tgit format-patch -1 --stdout --no-notes --notes >out &&\n+\tgrep \"notes config message\" out\n+'\n+\n+test_expect_success 'format-patch with multiple notes refs' '\n+\tgit notes --ref note1 add -m \"this is note 1\" HEAD &&\n+\ttest_when_finished git notes --ref note1 remove HEAD &&\n+\tgit notes --ref note2 add -m \"this is note 2\" HEAD &&\n+\ttest_when_finished git notes --ref note2 remove HEAD &&\n+\n+\tgit format-patch -1 --stdout >out &&\n+\t! grep \"this is note 1\" out &&\n+\t! grep \"this is note 2\" out &&\n+\tgit format-patch -1 --stdout --notes=note1 >out &&\n+\tgrep \"this is note 1\" out &&\n+\t! grep \"this is note 2\" out &&\n+\tgit format-patch -1 --stdout --notes=note2 >out &&\n+\t! grep \"this is note 1\" out &&\n+\tgrep \"this is note 2\" out &&\n+\tgit format-patch -1 --stdout --notes=note1 --notes=note2 >out &&\n+\tgrep \"this is note 1\" out &&\n+\tgrep \"this is note 2\" out &&\n+\n+\ttest_config format.notes note1 &&\n+\tgit format-patch -1 --stdout >out &&\n+\tgrep \"this is note 1\" out &&\n+\t! grep \"this is note 2\" out &&\n+\tgit format-patch -1 --stdout --no-notes >out &&\n+\t! grep \"this is note 1\" out &&\n+\t! grep \"this is note 2\" out &&\n+\tgit format-patch -1 --stdout --notes=note2 >out &&\n+\tgrep \"this is note 1\" out &&\n+\tgrep \"this is note 2\" out &&\n+\tgit format-patch -1 --stdout --no-notes --notes=note2 >out &&\n+\t! grep \"this is note 1\" out &&\n+\tgrep \"this is note 2\" out &&\n+\n+\tgit config --add format.notes note2 &&\n+\tgit format-patch -1 --stdout >out &&\n+\tgrep \"this is note 1\" out &&\n+\tgrep \"this is note 2\" out &&\n+\tgit format-patch -1 --stdout --no-notes >out &&\n+\t! grep \"this is note 1\" out &&\n+\t! grep \"this is note 2\" out\n+'\n+\n echo \"fatal: --name-only does not make sense\" > expect.name-only\n echo \"fatal: --name-status does not make sense\" > expect.name-status\n echo \"fatal: --check does not make sense\" > expect.check\n-- \n2.21.0.1049.geb646f7864\n\n"},{"id":"387707","messageId":"CABPp-BF44+6gvZVNimKf-k7AWbOjw3OK-cJeFunNR96wvZGkcw@mail.gmail.com","threadId":"51110","inReplyTo":"7cb770e190f235accf50e4af3ec9a964f26358c5.1558048212.git.liu.denton@gmail.com","subject":"Re: [PATCH v4 2/2] format-patch: teach format.notes config option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-12-08T07:48:59Z","receivedAt":"2019-12-08T07:49:17Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Denton,\n\nI was recently digging in this area (due to the recent\nrebase-applies-patches-incorrectly bug report) and found a couple bugs\nfrom this old commit:\n\nOn Thu, May 16, 2019 at 5:27 PM Denton Liu <liu.denton@gmail.com> wrote:\n>\n> In git-format-patch, notes can be appended with the `--notes` option.\n> However, this must be specified by the user on an\n> invocation-by-invocation basis. If a user is not careful, it's possible\n> that they may forget to include it and generate a patch series without\n> notes.\n>\n> Teach git-format-patch the `format.notes` config option. Its value is a\n> notes ref that will be automatically appended. The special value of\n> \"standard\" can be used to specify the standard notes. This option is\n> overridable with the `--no-notes` option in case a user wishes not to\n> append notes.\n>\n> Signed-off-by: Denton Liu <liu.denton@gmail.com>\n> ---\n>  Documentation/config/format.txt    | 15 +++++++\n>  Documentation/git-format-patch.txt |  3 ++\n>  builtin/log.c                      | 20 ++++++++-\n>  t/t4014-format-patch.sh            | 70 ++++++++++++++++++++++++++++++\n>  4 files changed, 107 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt\n> index dc77941c48..414a5a8a9d 100644\n> --- a/Documentation/config/format.txt\n> +++ b/Documentation/config/format.txt\n> @@ -85,3 +85,18 @@ format.outputDirectory::\n>  format.useAutoBase::\n>         A boolean value which lets you enable the `--base=auto` option of\n>         format-patch by default.\n> +\n> +format.notes::\n> +       Provides the default value for the `--notes` option to\n> +       format-patch. Accepts a boolean value, or a ref which specifies\n> +       where to get notes. If false, format-patch defaults to\n> +       `--no-notes`. If true, format-patch defaults to `--notes`. If\n> +       set to a non-boolean value, format-patch defaults to\n> +       `--notes=<ref>`, where `ref` is the non-boolean value. Defaults\n> +       to false.\n> ++\n> +If one wishes to use the ref `ref/notes/true`, please use that literal\n> +instead.\n> ++\n> +This configuration can be specified multiple times in order to allow\n> +multiple notes refs to be included.\n> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n> index 2c3971390e..9ce5b8aaee 100644\n> --- a/Documentation/git-format-patch.txt\n> +++ b/Documentation/git-format-patch.txt\n> @@ -275,6 +275,9 @@ these explanations after `format-patch` has run but before sending,\n>  keeping them as Git notes allows them to be maintained between versions\n>  of the patch series (but see the discussion of the `notes.rewrite`\n>  configuration options in linkgit:git-notes[1] to use this workflow).\n> ++\n> +The default is `--no-notes`, unless the `format.notes` configuration is\n> +set.\n>\n>  --[no-]signature=<signature>::\n>         Add a signature to each message produced. Per RFC 3676 the signature\n> diff --git a/builtin/log.c b/builtin/log.c\n> index e43ee12fb1..dad63cffc6 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -779,6 +779,8 @@ enum {\n>\n>  static int git_format_config(const char *var, const char *value, void *cb)\n>  {\n> +       struct rev_info *rev = cb;\n> +\n>         if (!strcmp(var, \"format.headers\")) {\n>                 if (!value)\n>                         die(_(\"format.headers without value\"));\n> @@ -864,6 +866,22 @@ static int git_format_config(const char *var, const char *value, void *cb)\n>                         from = NULL;\n>                 return 0;\n>         }\n> +       if (!strcmp(var, \"format.notes\")) {\n> +               struct strbuf buf = STRBUF_INIT;\n> +               int b = git_parse_maybe_bool(value);\n> +               if (!b)\n> +                       return 0;\n> +               rev->show_notes = 1;\n> +               if (b < 0) {\n> +                       strbuf_addstr(&buf, value);\n> +                       expand_notes_ref(&buf);\n> +                       string_list_append(&rev->notes_opt.extra_notes_refs,\n> +                                       strbuf_detach(&buf, NULL));\n> +               } else {\n> +                       rev->notes_opt.use_default_notes = 1;\n> +               }\n> +               return 0;\n> +       }\n\nWhat if someone has multiple format.notes entries in their config\nfile, but the last entry is \"false\" -- shouldn't that disable notes?\nAlso, what if they specify both \"true\" and e.g.\n\"refs/notes/my-cool-notes\"?  In that case, should it show\nrefs/notes/my-cool-notes because that's obviously showing some notes\nso it satisfies true as well as the specific request about which note,\nor should it treat \"true\" the same as the-default-notes-ref and then\nadd the two refs together and show them both?\n\n>\n>         return git_log_config(var, value, cb);\n>  }\n> @@ -1617,8 +1635,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>         extra_to.strdup_strings = 1;\n>         extra_cc.strdup_strings = 1;\n>         init_log_defaults();\n> -       git_config(git_format_config, NULL);\n>         repo_init_revisions(the_repository, &rev, prefix);\n> +       git_config(git_format_config, &rev);\n\nCalling git_config() after repo_init_revisions() breaks things;\ngenerally git_config() should always be called first.  Here,\ngit_format_config() can set up parameters used by\nrepo_init_revisions(), and by reversing the order of the two you end\nup ignoring settings specified by the user (e.g. diff.context having a\nvalue of 5).  This came up due to the bug report at\nhttps://lore.kernel.org/git/xmqqa78d2qmk.fsf@gitster-ct.c.googlers.com/T/#mb6a09958ff10acde295b37a9136bc3791fd4a2c2\n(though fixing the issue there _also_ requires fixing git_am_config()\nto call git_diff_ui_config()).  To break the circular dependency here,\nwe'd need to store the information that git_format_config() discovers\nin some data structure besides rev, and then after the\nrepo_init_revisions() call has finished then update rev.\n\nI was just going to do that, but then ran into the questions above\nabout multiple format.notes entries in the config file, and am not as\nsure about what should be done about that stuff (and I don't want to\ntry to translate the current behavior as-is while tweaking where the\nstuff is stored, both because I'm not sure of the right behavior and\nbecause I don't want future folks to blame the code to me when they\nhit bugs in this area), so I'm firing off this email instead.\n\nSo, um...help?\n\nThanks,\nElijah\n"},{"id":"387737","messageId":"20191209082451.GA57882@generichostname","threadId":"51110","inReplyTo":"CABPp-BF44+6gvZVNimKf-k7AWbOjw3OK-cJeFunNR96wvZGkcw@mail.gmail.com","subject":"Re: [PATCH v4 2/2] format-patch: teach format.notes config option","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-12-09T08:24:51Z","receivedAt":"2019-12-09T08:24:57Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Elijah,\n\nOn Sat, Dec 07, 2019 at 11:48:59PM -0800, Elijah Newren wrote:\n> > @@ -864,6 +866,22 @@ static int git_format_config(const char *var, const char *value, void *cb)\n> >                         from = NULL;\n> >                 return 0;\n> >         }\n> > +       if (!strcmp(var, \"format.notes\")) {\n> > +               struct strbuf buf = STRBUF_INIT;\n> > +               int b = git_parse_maybe_bool(value);\n> > +               if (!b)\n> > +                       return 0;\n> > +               rev->show_notes = 1;\n> > +               if (b < 0) {\n> > +                       strbuf_addstr(&buf, value);\n> > +                       expand_notes_ref(&buf);\n> > +                       string_list_append(&rev->notes_opt.extra_notes_refs,\n> > +                                       strbuf_detach(&buf, NULL));\n> > +               } else {\n> > +                       rev->notes_opt.use_default_notes = 1;\n> > +               }\n> > +               return 0;\n> > +       }\n> \n> What if someone has multiple format.notes entries in their config\n> file, but the last entry is \"false\" -- shouldn't that disable notes?\n> Also, what if they specify both \"true\" and e.g.\n> \"refs/notes/my-cool-notes\"?  In that case, should it show\n> refs/notes/my-cool-notes because that's obviously showing some notes\n> so it satisfies true as well as the specific request about which note,\n> or should it treat \"true\" the same as the-default-notes-ref and then\n> add the two refs together and show them both?\n\nI think I'll just copy the existing logic of --notes, --notes=<ref> and\n--no-notes from revision.c to `format.notes = true`, \n`format.notes = <ref>` and `format.notes = false` respectively. IOW,\nwith `format.notes = true`, we'll unconditionally use the default notes,\nwith `format.notes = <ref>`, we'll append <ref> to the reflist and with\n`format.notes = false`, we'll clear and unset the notes refs.\n\nIt seems like that logic has been around for almost a decade and I don't\nthink anyone's complained about it so I think it should be safe to\nduplicate.\n\n> \n> >\n> >         return git_log_config(var, value, cb);\n> >  }\n> > @@ -1617,8 +1635,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> >         extra_to.strdup_strings = 1;\n> >         extra_cc.strdup_strings = 1;\n> >         init_log_defaults();\n> > -       git_config(git_format_config, NULL);\n> >         repo_init_revisions(the_repository, &rev, prefix);\n> > +       git_config(git_format_config, &rev);\n> \n> Calling git_config() after repo_init_revisions() breaks things;\n> generally git_config() should always be called first.  Here,\n> git_format_config() can set up parameters used by\n> repo_init_revisions(), and by reversing the order of the two you end\n> up ignoring settings specified by the user (e.g. diff.context having a\n> value of 5).  This came up due to the bug report at\n> https://lore.kernel.org/git/xmqqa78d2qmk.fsf@gitster-ct.c.googlers.com/T/#mb6a09958ff10acde295b37a9136bc3791fd4a2c2\n> (though fixing the issue there _also_ requires fixing git_am_config()\n> to call git_diff_ui_config()).  To break the circular dependency here,\n> we'd need to store the information that git_format_config() discovers\n> in some data structure besides rev, and then after the\n> repo_init_revisions() call has finished then update rev.\n\nI see, I'll move the git_config() back up while I'm at it.\n\n> \n> I was just going to do that, but then ran into the questions above\n> about multiple format.notes entries in the config file, and am not as\n> sure about what should be done about that stuff (and I don't want to\n> try to translate the current behavior as-is while tweaking where the\n> stuff is stored, both because I'm not sure of the right behavior and\n> because I don't want future folks to blame the code to me when they\n> hit bugs in this area), so I'm firing off this email instead.\n\nSounds good, I don't want my bugs blamed on anyone else either ;)\n\nI'll try to get a patchset out soon and hopefully you'll be able to base\nyour work off of that.\n\nThanks,\n\nDenton\n\n> \n> So, um...help?\n> \n> Thanks,\n> Elijah\n"}]}