{"thread":{"id":"59807","subject":"[RFC PATCH 0/3] range-diff: treat notes like `log`","startedAt":"2023-05-30T10:43:15Z","lastAt":"2023-09-21T19:39:06Z","messageCount":41,"participants":["Kristoffer Haugsbakk","Jeff King","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"477796","messageId":"cover.1685441207.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":null,"subject":"[RFC PATCH 0/3] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-05-30T10:41:28Z","receivedAt":"2023-05-30T10:43:15Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Currently, `range-diff` shows the default notes if no notes-related\narguments are given. This is also how `log` behaves. But unlike\n`range-diff`, `log` does *not* show the default notes if\n`--notes=<custom>` are given.\n\nThese changes are supposed to make `format-range` behave like `log` with\nregards to notes.\n\nThese changes also fixes an issue with notes being shared in the cover\nletter via `range-diff`, and that’s really the main motivation for\nmaking these changes.\n\n§ How `log` works\n\n`log` shows the default notes if no notes arguments are given. But if\nyou give it specific notes to show then it forgets about the default\nnotes. Further, there is the convenience `--notes` option which will\nshow the default notes again. These options are cumulative. For example:\n\n    git log --notes --notes=custom\n\nWill show the default notes as well as the `custom` notes.\n\nSee discussion in: https://lore.kernel.org/git/20110329143357.GA10771@sigill.intra.peff.net/\n\n§ How `range-format` works\n\n`range-format` passes `--notes` to `log`, which means that it does not\nhave the default behavior of `log` (forget the default logs if you say\ne.g. `--notes=custom`). However, the man page says that (under\n`--[no-]notes[=<ref>]`):\n\n> This flag is passed to the git log program (see git-log(1)) that generates the patches.\n\nThis makes me (at least) think that `range-format` is supposed to work\njust like `log` with regards to notes.\n\n§ `format-patch` and the difference between showing and sharing\n\n`format-patch` has a different default: it shows no notes. This makes\nsense in my opinion since `format-patch` is meant to be used to share\nchanges with others, and you might be surprised if your notes (which\nmight have only been notes to yourself) are sent out in your emails\n(keep in mind that notes refs are *not* pushed by default).\n\nBut the slightly faulty behavior of `range-diff` bleeds through to\n`format-patch` since the latter calls the former; if you have default\nnotes they can be shared in the range-diff on the cover letter, even\nthough `format-patch` isn’t supposed to show them.\n\n§ Code layout and approach to the problem\n\nAs I’ve said, my focus was on fixing `format-patch`, so I’ve considered\nhow `format-patch` calls `range-diff` which in turn calls `log`.\n\n`format-patch` is a command which is defined in `builtin/log.c`. For\nnotes in particular it in fact has some explicit logic for handling\nnotes based on the value of `rev`. (There seems to be no issues with\nthis part of the code; only the code in `range-diff.c` which passes the\ndefault argument to `log`.) It then calls\n`range-diff.c:show_range_diff`. That function on `master` loads some\ndefault arguments, among them `--notes`. It then eventually calls `log`\nas a subprocess.\n\nMy change consists of co-opting the deprecated `--standard-notes` and\nchanging its behavior so that it can be used in\n`range-diff.c:show_range_diff`.\n\nUsing a special switch/option was the only way I found in order to fix\nthis problem.\n\nI could have also created a new option but I thought that doing a\nsmaller change initially would be better.\n\n§ RFC\n\nThis is marked as RFC since I chose to co-opt a deprecated option in\norder to get `range-format` to work, in my opinion, as it should.\n\n§ Carbon copy\n\nBased on `contrib/contacts/git-contacts master..@`.\n\nKristoffer Haugsbakk (3):\n  range-diff: treat notes like `log`\n  doc: pretty-options: remove documentation for deprecated options\n  revision: comment `--no-standard-notes` as deprecated\n\n Documentation/pretty-options.txt |  1 -\n range-diff.c                     |  2 +-\n revision.c                       |  8 ++++++--\n t/t3206-range-diff.sh            | 28 ++++++++++++++++++++++++++++\n 4 files changed, 35 insertions(+), 4 deletions(-)\n\n-- \n2.41.0.rc2\n\n"},{"id":"477797","messageId":"8cc8eddfced1b64156008f7f923c430981d472af.1685441207.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1685441207.git.code@khaugsbakk.name","subject":"[RFC PATCH 1/3] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-05-30T10:41:29Z","receivedAt":"2023-05-30T10:43:26Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Currently, `range-diff` shows the default notes if no notes-related\narguments are given. This is also how `log` behaves. But unlike\n`range-diff`, `log` does *not* show the default notes if\n`--notes=<custom>` are given. In other words, this:\n\n    git log --notes=custom\n\nis equivalent to this:\n\n    git log --no-notes --notes=custom\n\nWhile:\n\n    git range-diff --notes=custom\n\nacts like this:\n\n    git log --notes --notes-custom\n\nThis can’t be how the user expects `range-diff` to behave given that the\nman page for `range diff` under `--[no-]notes[=<ref>]` says:\n\n> This flag is passed to the git log program (see git-log(1)) that\n> generates the patches.\n\nThis behavior also affects `format-patch` since it uses `range-diff` for\nthe cover letter. Unlike `log`, though, `format-patch` is not supposed\nto show the default notes if no notes-related arguments are given.[1]\nBut this promise is broken when the range-diff happens to have something\nto say about the changes to the default notes, since that will be shown\nin the cover letter.\n\nRemedy this by co-opting the `--standard-notes` option which has been\ndeprecated since ab18b2c0df[2] and which is currently only documented in\n`pretty-options`.\n\n† 1: See e.g. 66b2ed09c2 (Fix \"log\" family not to be too agressive about\n    showing notes, 2010-01-20).\n† 2: log/pretty-options: Document --[no-]notes and deprecate old notes\n    options, 2011-03-30\n\nSigned-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n range-diff.c          |  2 +-\n revision.c            |  7 +++++--\n t/t3206-range-diff.sh | 28 ++++++++++++++++++++++++++++\n 3 files changed, 34 insertions(+), 3 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 6a704e6f47..2c92af337f 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -59,7 +59,7 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t     \"--output-indicator-context=#\",\n \t\t     \"--no-abbrev-commit\",\n \t\t     \"--pretty=medium\",\n-\t\t     \"--notes\",\n+\t\t     \"--standard-notes\",\n \t\t     NULL);\n \tstrvec_push(&cp.args, range);\n \tif (other_arg)\ndiff --git a/revision.c b/revision.c\nindex b33cc1d106..a0ab7fb784 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2524,8 +2524,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\tdisable_display_notes(&revs->notes_opt, &revs->show_notes);\n \t\trevs->show_notes_given = 1;\n \t} else if (!strcmp(arg, \"--standard-notes\")) {\n-\t\trevs->show_notes_given = 1;\n-\t\trevs->notes_opt.use_default_notes = 1;\n+\t\tdisable_display_notes(&revs->notes_opt, &revs->show_notes);\n+\t\trevs->show_notes_given = 0;\n+\t\tenable_default_display_notes(&revs->notes_opt,\n+\t\t\t\t\t     &revs->show_notes);\n+\t\trevs->notes_opt.use_default_notes = -1;\n \t} else if (!strcmp(arg, \"--no-standard-notes\")) {\n \t\trevs->notes_opt.use_default_notes = 0;\n \t} else if (!strcmp(arg, \"--oneline\")) {\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex b5f4d6a653..b33afa1c6a 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -662,6 +662,20 @@ test_expect_success 'range-diff with multiple --notes' '\n \ttest_cmp expect actual\n '\n \n+# `range-diff` should act like `log` with regards to notes\n+test_expect_success 'range-diff with --notes=custom does not show default notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\tgit notes --ref=custom add -m \"topic note\" topic &&\n+\tgit notes --ref=custom add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\ttest_when_finished git notes --ref=custom remove topic unmodified &&\n+\tgit range-diff --notes=custom main..topic main..unmodified \\\n+\t\t>actual &&\n+\t! grep \"## Notes ##\" actual &&\n+\tgrep \"## Notes (custom) ##\" actual\n+'\n+\n test_expect_success 'format-patch --range-diff does not compare notes by default' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n@@ -679,6 +693,20 @@ test_expect_success 'format-patch --range-diff does not compare notes by default\n \t! grep \"note\" 0000-*\n '\n \n+test_expect_success 'format-patch --notes=custom --range-diff only compares custom notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes --ref=custom add -m \"topic note (custom)\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\tgit notes --ref=custom add -m \"unmodified note (custom)\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\ttest_when_finished git notes --ref=custom remove topic unmodified &&\n+\tgit format-patch --notes=custom --cover-letter --range-diff=$prev \\\n+\t\tmain..unmodified >actual &&\n+\ttest_when_finished \"rm 000?-*\" &&\n+\tgrep \"## Notes (custom) ##\" 0000-* &&\n+\t! grep \"## Notes ##\" 0000-*\n+'\n+\n test_expect_success 'format-patch --range-diff with --no-notes' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n-- \n2.41.0.rc2\n\n"},{"id":"477798","messageId":"8fa6600ae84096d8aaad8de387015a9352702d9c.1685441207.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1685441207.git.code@khaugsbakk.name","subject":"[RFC PATCH 2/3] doc: pretty-options: remove documentation for deprecated options","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-05-30T10:41:30Z","receivedAt":"2023-05-30T10:43:42Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Remove documentation for options which were deprecated in\nab18b2c0df (log/pretty-options: Document --[no-]notes and deprecate old\nnotes options, 2011-03-30).\n\nWe now use `--standard-notes` internally for `range-diff` (see previous\ncommit).\n\nLeave `--show-notes` as-is since we aren’t changing anything about it.\n\nSigned-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n Documentation/pretty-options.txt | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\nindex dc685be363..8c982609c9 100644\n--- a/Documentation/pretty-options.txt\n+++ b/Documentation/pretty-options.txt\n@@ -88,7 +88,6 @@ being displayed. Examples: \"--notes=foo\" will show only notes from\n \tfrom \"refs/notes/bar\".\n \n --show-notes[=<ref>]::\n---[no-]standard-notes::\n \tThese options are deprecated. Use the above --notes/--no-notes\n \toptions instead.\n endif::git-rev-list[]\n-- \n2.41.0.rc2\n\n"},{"id":"477799","messageId":"29ce1182134cd031b3e05a5e2774639d3d9a7cdf.1685441207.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1685441207.git.code@khaugsbakk.name","subject":"[RFC PATCH 3/3] revision: comment `--no-standard-notes` as deprecated","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-05-30T10:41:31Z","receivedAt":"2023-05-30T10:43:52Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"We still use `--standard-notes` but this option has no use and is no\nlonger documented anywhere.\n\nSigned-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n revision.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/revision.c b/revision.c\nindex a0ab7fb784..24219c741a 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2529,6 +2529,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\tenable_default_display_notes(&revs->notes_opt,\n \t\t\t\t\t     &revs->show_notes);\n \t\trevs->notes_opt.use_default_notes = -1;\n+\t\t/* Deprecated */\n \t} else if (!strcmp(arg, \"--no-standard-notes\")) {\n \t\trevs->notes_opt.use_default_notes = 0;\n \t} else if (!strcmp(arg, \"--oneline\")) {\n-- \n2.41.0.rc2\n\n"},{"id":"477897","messageId":"20230601182020.GC4165405@coredump.intra.peff.net","threadId":"59807","inReplyTo":"8cc8eddfced1b64156008f7f923c430981d472af.1685441207.git.code@khaugsbakk.name","subject":"Re: [RFC PATCH 1/3] range-diff: treat notes like `log`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-06-01T18:20:20Z","receivedAt":"2023-06-01T18:20:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 30, 2023 at 12:41:29PM +0200, Kristoffer Haugsbakk wrote:\n\n> Currently, `range-diff` shows the default notes if no notes-related\n> arguments are given. This is also how `log` behaves. But unlike\n> `range-diff`, `log` does *not* show the default notes if\n> `--notes=<custom>` are given. In other words, this:\n> \n>     git log --notes=custom\n> \n> is equivalent to this:\n> \n>     git log --no-notes --notes=custom\n> \n> While:\n> \n>     git range-diff --notes=custom\n> \n> acts like this:\n> \n>     git log --notes --notes-custom\n> \n> This can’t be how the user expects `range-diff` to behave given that the\n> man page for `range diff` under `--[no-]notes[=<ref>]` says:\n> \n> > This flag is passed to the git log program (see git-log(1)) that\n> > generates the patches.\n\nYeah, I certainly agree that the behavior of range-diff is surprising,\nand that this is a bug.\n\nI'd have expected the solution here would be for range-diff to stop\npassing \"--notes\", and let \"log\" decide whether to show notes (based on\nspecific --notes=foo it gets from other_arg).\n\nBut...\n\n> This behavior also affects `format-patch` since it uses `range-diff` for\n> the cover letter. Unlike `log`, though, `format-patch` is not supposed\n> to show the default notes if no notes-related arguments are given.[1]\n> But this promise is broken when the range-diff happens to have something\n> to say about the changes to the default notes, since that will be shown\n> in the cover letter.\n> \n> Remedy this by co-opting the `--standard-notes` option which has been\n> deprecated since ab18b2c0df[2] and which is currently only documented in\n> `pretty-options`.\n\nI'm not clear on whether you're actually fixing two separate bugs here,\nor if they need to be intertwined.\n\nIt seems like passing --standard-notes means that format-patch's\nrange-diff will still show the standard notes by default. Maybe I'm\nmisunderstanding the problem, though.\n\n-Peff\n"},{"id":"477927","messageId":"8f434f16-de65-40c2-80d3-28d123c7be7f@app.fastmail.com","threadId":"59807","inReplyTo":"20230601182020.GC4165405@coredump.intra.peff.net","subject":"Re: [RFC PATCH 1/3] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-06-02T10:06:16Z","receivedAt":"2023-06-02T10:07:11Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Hi Peff!\n\nOn Thu, Jun 1, 2023, at 20:20, Jeff King wrote:\n>> This can’t be how the user expects `range-diff` to behave given that the\n>> man page for `range diff` under `--[no-]notes[=<ref>]` says:\n>>\n>> > This flag is passed to the git log program (see git-log(1)) that\n>> > generates the patches.\n>\n> Yeah, I certainly agree that the behavior of range-diff is surprising,\n> and that this is a bug.\n>\n> I'd have expected the solution here would be for range-diff to stop\n> passing \"--notes\", and let \"log\" decide whether to show notes (based on\n> specific --notes=foo it gets from other_arg).\n\nI tried that (removing `--notes`) but it didn’t work out for me. I’d had\nhoped that `--notes` was just a holdover from a previous change that\nwasn’t needed any more, but it seems according to my testing that `range\ndiff` needs to pass *some* notes-related argument to `log` in order for\nthe test suite to pass.\n\nThinking about it again now it doesn’t really make sense in the isolated\ncase of `range-diff`. But maybe it’s needed in conjunction with\n`format-patch`.\n\nA relevant series is “range-diff: learn `--notes`” by Denton Liu\n(f3c7bfdde2 (Merge branch 'dl/range-diff-with-notes', 2019-12-05)).[1]\n\nOn the commit before that merge, `range-diff.c` does not pass `--notes`\nto `log`. Denton describes the behavior in the first cover letter:\n\n> When I was using range-diff at $DAYJOB earlier, I realised that it\n> includes commit notes as part of the commit message comparison.\n\nSo just like the default behavior of `log`.\n\nHowever, the next version (v2) is about teaching `range-diff` to pass\nnotes down to `log`:\n\n> This patchset teaches range-diff and format-patch to pass `--notes`\n> options down to the `git log` machinery. This should make the\n> behaviour more configurable for users who either don't want notes to\n> be displayed or want multiple notes refs to be displayed.\n\nSo apparently `range-diff` couldn’t pass on/down `--notes=<ref>` before\nthat.\n\nThat series also teaches `format-patch` to handle notes in 5b583e6a09\n(format-patch: pass notes configuration to range-diff, 2019-11-20).\n\n[1] https://lore.kernel.org/git/cover.1574125554.git.liu.denton@gmail.com/\n\n>\n> But...\n>\n>> This behavior also affects `format-patch` since it uses `range-diff` for\n>> the cover letter. Unlike `log`, though, `format-patch` is not supposed\n>> to show the default notes if no notes-related arguments are given.[1]\n>> But this promise is broken when the range-diff happens to have something\n>> to say about the changes to the default notes, since that will be shown\n>> in the cover letter.\n>>\n>> Remedy this by co-opting the `--standard-notes` option which has been\n>> deprecated since ab18b2c0df[2] and which is currently only documented in\n>> `pretty-options`.\n>\n> I'm not clear on whether you're actually fixing two separate bugs here,\n> or if they need to be intertwined.\n\nI guess it depends on your perspective. ;) I would say that they are so\nintertwined that fixing one also fixes the other.\n\n> It seems like passing --standard-notes means that format-patch's\n> range-diff will still show the standard notes by default. Maybe I'm\n> misunderstanding the problem, though.\n\nNo, that should still work. See the test `format-patch --range-diff does\nnot compare notes by default`.\n\nThat kind of thing seems to be handled by `log.c:get_notes_args`;\n`--no-notes` is sent to `log` if no notes should be shown.\n\nCheers\n\n-- \nKristoffer Haugsbakk\n"},{"id":"478225","messageId":"fbc1c47b92070de4f5d8fee1d031853fe0195ceb.1686505920.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1686505920.git.code@khaugsbakk.name","subject":"[PATCH v1 2/3] doc: pretty-options: remove documentation for deprecated options","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-06-11T18:15:55Z","receivedAt":"2023-06-11T18:19:05Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Remove documentation for options which were deprecated in\nab18b2c0df (log/pretty-options: Document --[no-]notes and deprecate old\nnotes options, 2011-03-30).\n\nWe now use `--standard-notes` internally for `range-diff` (see previous\ncommit).\n\nLeave `--show-notes` as-is since we aren’t changing anything about it.\n\nSigned-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n Documentation/pretty-options.txt | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\nindex dc685be363..8c982609c9 100644\n--- a/Documentation/pretty-options.txt\n+++ b/Documentation/pretty-options.txt\n@@ -88,7 +88,6 @@ being displayed. Examples: \"--notes=foo\" will show only notes from\n \tfrom \"refs/notes/bar\".\n \n --show-notes[=<ref>]::\n---[no-]standard-notes::\n \tThese options are deprecated. Use the above --notes/--no-notes\n \toptions instead.\n endif::git-rev-list[]\n-- \n2.41.0\n\n"},{"id":"478224","messageId":"9141f5a86e66276b672fc54783afe3b48b6227cf.1686505920.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1686505920.git.code@khaugsbakk.name","subject":"[PATCH v1 3/3] revision: comment `--no-standard-notes` as deprecated","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-06-11T18:15:56Z","receivedAt":"2023-06-11T18:19:07Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"We still use `--standard-notes` but this option has no use and is no\nlonger documented anywhere.\n\nSigned-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n revision.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/revision.c b/revision.c\nindex a0ab7fb784..24219c741a 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2529,6 +2529,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\tenable_default_display_notes(&revs->notes_opt,\n \t\t\t\t\t     &revs->show_notes);\n \t\trevs->notes_opt.use_default_notes = -1;\n+\t\t/* Deprecated */\n \t} else if (!strcmp(arg, \"--no-standard-notes\")) {\n \t\trevs->notes_opt.use_default_notes = 0;\n \t} else if (!strcmp(arg, \"--oneline\")) {\n-- \n2.41.0\n\n"},{"id":"478226","messageId":"6a4fe606cfae91ac9d0a69fef772bc56a507ef7d.1686505920.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1686505920.git.code@khaugsbakk.name","subject":"[PATCH v1 1/3] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-06-11T18:15:54Z","receivedAt":"2023-06-11T18:19:07Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Currently, `range-diff` shows the default notes if no notes-related\narguments are given. This is also how `log` behaves. But unlike\n`range-diff`, `log` does *not* show the default notes if\n`--notes=<custom>` are given. In other words, this:\n\n    git log --notes=custom\n\nis equivalent to this:\n\n    git log --no-notes --notes=custom\n\nWhile:\n\n    git range-diff --notes=custom\n\nacts like this:\n\n    git log --notes --notes-custom\n\nThis can’t be how the user expects `range-diff` to behave given that the\nman page for `range diff` under `--[no-]notes[=<ref>]` says:\n\n> This flag is passed to the git log program (see git-log(1)) that\n> generates the patches.\n\nThis behavior also affects `format-patch` since it uses `range-diff` for\nthe cover letter. Unlike `log`, though, `format-patch` is not supposed\nto show the default notes if no notes-related arguments are given.[1]\nBut this promise is broken when the range-diff happens to have something\nto say about the changes to the default notes, since that will be shown\nin the cover letter.\n\nRemedy this by co-opting the `--standard-notes` option which has been\ndeprecated since ab18b2c0df[2] and which is currently only documented in\n`pretty-options`.\n\n† 1: See e.g. 66b2ed09c2 (Fix \"log\" family not to be too agressive about\n    showing notes, 2010-01-20).\n† 2: log/pretty-options: Document --[no-]notes and deprecate old notes\n    options, 2011-03-30\n\nSigned-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n range-diff.c          |  2 +-\n revision.c            |  7 +++++--\n t/t3206-range-diff.sh | 28 ++++++++++++++++++++++++++++\n 3 files changed, 34 insertions(+), 3 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 6a704e6f47..2c92af337f 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -59,7 +59,7 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t     \"--output-indicator-context=#\",\n \t\t     \"--no-abbrev-commit\",\n \t\t     \"--pretty=medium\",\n-\t\t     \"--notes\",\n+\t\t     \"--standard-notes\",\n \t\t     NULL);\n \tstrvec_push(&cp.args, range);\n \tif (other_arg)\ndiff --git a/revision.c b/revision.c\nindex b33cc1d106..a0ab7fb784 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2524,8 +2524,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\tdisable_display_notes(&revs->notes_opt, &revs->show_notes);\n \t\trevs->show_notes_given = 1;\n \t} else if (!strcmp(arg, \"--standard-notes\")) {\n-\t\trevs->show_notes_given = 1;\n-\t\trevs->notes_opt.use_default_notes = 1;\n+\t\tdisable_display_notes(&revs->notes_opt, &revs->show_notes);\n+\t\trevs->show_notes_given = 0;\n+\t\tenable_default_display_notes(&revs->notes_opt,\n+\t\t\t\t\t     &revs->show_notes);\n+\t\trevs->notes_opt.use_default_notes = -1;\n \t} else if (!strcmp(arg, \"--no-standard-notes\")) {\n \t\trevs->notes_opt.use_default_notes = 0;\n \t} else if (!strcmp(arg, \"--oneline\")) {\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex b5f4d6a653..b33afa1c6a 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -662,6 +662,20 @@ test_expect_success 'range-diff with multiple --notes' '\n \ttest_cmp expect actual\n '\n \n+# `range-diff` should act like `log` with regards to notes\n+test_expect_success 'range-diff with --notes=custom does not show default notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\tgit notes --ref=custom add -m \"topic note\" topic &&\n+\tgit notes --ref=custom add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\ttest_when_finished git notes --ref=custom remove topic unmodified &&\n+\tgit range-diff --notes=custom main..topic main..unmodified \\\n+\t\t>actual &&\n+\t! grep \"## Notes ##\" actual &&\n+\tgrep \"## Notes (custom) ##\" actual\n+'\n+\n test_expect_success 'format-patch --range-diff does not compare notes by default' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n@@ -679,6 +693,20 @@ test_expect_success 'format-patch --range-diff does not compare notes by default\n \t! grep \"note\" 0000-*\n '\n \n+test_expect_success 'format-patch --notes=custom --range-diff only compares custom notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes --ref=custom add -m \"topic note (custom)\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\tgit notes --ref=custom add -m \"unmodified note (custom)\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\ttest_when_finished git notes --ref=custom remove topic unmodified &&\n+\tgit format-patch --notes=custom --cover-letter --range-diff=$prev \\\n+\t\tmain..unmodified >actual &&\n+\ttest_when_finished \"rm 000?-*\" &&\n+\tgrep \"## Notes (custom) ##\" 0000-* &&\n+\t! grep \"## Notes ##\" 0000-*\n+'\n+\n test_expect_success 'format-patch --range-diff with --no-notes' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n-- \n2.41.0\n\n"},{"id":"478227","messageId":"cover.1686505920.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1685441207.git.code@khaugsbakk.name","subject":"[PATCH v1 0/3] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-06-11T18:15:53Z","receivedAt":"2023-06-11T18:19:07Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"This is the first normal (not RFC) version of this series. The following\ncover letter is identical to the previous one up to but not including\nsection “RFC and version 1”.\n\nCheers\n\n🙛 🙙\n\nCurrently, `range-diff` shows the default notes if no notes-related\narguments are given. This is also how `log` behaves. But unlike\n`range-diff`, `log` does *not* show the default notes if\n`--notes=<custom>` are given.\n\nThese changes are supposed to make `format-range` behave like `log` with\nregards to notes.\n\nThese changes also fixes an issue with notes being shared in the cover\nletter via `range-diff`, and that’s really the main motivation for\nmaking these changes.\n\n§ How `log` works\n\n`log` shows the default notes if no notes arguments are given. But if\nyou give it specific notes to show then it forgets about the default\nnotes. Further, there is the convenience `--notes` option which will\nshow the default notes again. These options are cumulative. For example:\n\n    git log --notes --notes=custom\n\nWill show the default notes as well as the `custom` notes.\n\nSee discussion in: https://lore.kernel.org/git/20110329143357.GA10771@sigill.intra.peff.net/\n\n§ How `range-format` works\n\n`range-format` passes `--notes` to `log`, which means that it does not\nhave the default behavior of `log` (forget the default logs if you say\ne.g. `--notes=custom`). However, the man page says that (under\n`--[no-]notes[=<ref>]`):\n\n> This flag is passed to the git log program (see git-log(1)) that generates the patches.\n\nThis makes me (at least) think that `range-format` is supposed to work\njust like `log` with regards to notes.\n\n§ `format-patch` and the difference between showing and sharing\n\n`format-patch` has a different default: it shows no notes. This makes\nsense in my opinion since `format-patch` is meant to be used to share\nchanges with others, and you might be surprised if your notes (which\nmight have only been notes to yourself) are sent out in your emails\n(keep in mind that notes refs are *not* pushed by default).\n\nBut the slightly faulty behavior of `range-diff` bleeds through to\n`format-patch` since the latter calls the former; if you have default\nnotes they can be shared in the range-diff on the cover letter, even\nthough `format-patch` isn’t supposed to show them.\n\n§ Code layout and approach to the problem\n\nAs I’ve said, my focus was on fixing `format-patch`, so I’ve considered\nhow `format-patch` calls `range-diff` which in turn calls `log`.\n\n`format-patch` is a command which is defined in `builtin/log.c`. For\nnotes in particular it in fact has some explicit logic for handling\nnotes based on the value of `rev`. (There seems to be no issues with\nthis part of the code; only the code in `range-diff.c` which passes the\ndefault argument to `log`.) It then calls\n`range-diff.c:show_range_diff`. That function on `master` loads some\ndefault arguments, among them `--notes`. It then eventually calls `log`\nas a subprocess.\n\nMy change consists of co-opting the deprecated `--standard-notes` and\nchanging its behavior so that it can be used in\n`range-diff.c:show_range_diff`.\n\nUsing a special switch/option was the only way I found in order to fix\nthis problem.\n\nI could have also created a new option but I thought that doing a\nsmaller change initially would be better.\n\n§ RFC and version 1\n\nThe previous sendout was an RFC. There are no changes since the RFC.\n\nThe series was rebased on `v2.41.0`.\n\nKristoffer Haugsbakk (3):\n  range-diff: treat notes like `log`\n  doc: pretty-options: remove documentation for deprecated options\n  revision: comment `--no-standard-notes` as deprecated\n\n Documentation/pretty-options.txt |  1 -\n range-diff.c                     |  2 +-\n revision.c                       |  8 ++++++--\n t/t3206-range-diff.sh            | 28 ++++++++++++++++++++++++++++\n 4 files changed, 35 insertions(+), 4 deletions(-)\n\n--\n2.41.0\n"},{"id":"478314","messageId":"xmqqbkhk724x.fsf@gitster.g","threadId":"59807","inReplyTo":"9141f5a86e66276b672fc54783afe3b48b6227cf.1686505920.git.code@khaugsbakk.name","subject":"Re: [PATCH v1 3/3] revision: comment `--no-standard-notes` as deprecated","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-12T22:21:34Z","receivedAt":"2023-06-12T22:21:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristoffer Haugsbakk <code@khaugsbakk.name> writes:\n\n> We still use `--standard-notes` but this option has no use and is no\n> longer documented anywhere.\n>\n> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n> ---\n>  revision.c | 1 +\n>  1 file changed, 1 insertion(+)\n>\n> diff --git a/revision.c b/revision.c\n> index a0ab7fb784..24219c741a 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2529,6 +2529,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n>  \t\tenable_default_display_notes(&revs->notes_opt,\n>  \t\t\t\t\t     &revs->show_notes);\n>  \t\trevs->notes_opt.use_default_notes = -1;\n> +\t\t/* Deprecated */\n>  \t} else if (!strcmp(arg, \"--no-standard-notes\")) {\n>  \t\trevs->notes_opt.use_default_notes = 0;\n>  \t} else if (!strcmp(arg, \"--oneline\")) {\n\nWith the placement of this new comment, it is unclear which one\nbetween \"--standard-notes\" and \"--no-standard-notes\" is getting\ndeprecated (actually, the comment is placed inside the block for the\nformer, so it may be more natural to interpret that the comment\nmarks the former as deprecated).\n\n\t} else if (!strcmp(arg, \"--no-standard-notes\")) {\n+\t\t/* Deprecated */\n\t\trevs->notes_opt.use_default_notes = 0;\n\t}\n\nI am not commenting if it makes sense to declare that the option is\ndeprecated here---I'll leave it to others to argue for/against it.\nThe usual reasoning to add/maintain \"--no-foo\" is so that we can get\n\"cmd --foo --no-foo\" to naturally work, but it does not apply here?\n\n\n\n"},{"id":"478315","messageId":"xmqq7cs871ys.fsf@gitster.g","threadId":"59807","inReplyTo":"cover.1686505920.git.code@khaugsbakk.name","subject":"Re: [PATCH v1 0/3] range-diff: treat notes like `log`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-12T22:25:15Z","receivedAt":"2023-06-12T22:25:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristoffer Haugsbakk <code@khaugsbakk.name> writes:\n\n> This is the first normal (not RFC) version of this series. The following\n> cover letter is identical to the previous one up to but not including\n> section “RFC and version 1”.\n\nI have no strong opinion on the topic and did not comment on the RFC\nversion, either, so I am puzzled to be CC'ed, especially when the\nprimary author of the feature is not.\n\nDscho, how does this series look?\n\nThanks.\n"},{"id":"478321","messageId":"765971e5-fa3e-42bf-88df-9614c3b7e8dc@app.fastmail.com","threadId":"59807","inReplyTo":"xmqq7cs871ys.fsf@gitster.g","subject":"Re: [PATCH v1 0/3] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-06-13T05:43:29Z","receivedAt":"2023-06-13T05:43:58Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Tue, Jun 13, 2023, at 00:25, Junio C Hamano wrote:\n> Kristoffer Haugsbakk <code@khaugsbakk.name> writes:\n>\n>> This is the first normal (not RFC) version of this series. The following\n>> cover letter is identical to the previous one up to but not including\n>> section “RFC and version 1”.\n>\n> I have no strong opinion on the topic and did not comment on the RFC\n> version, either, so I am puzzled to be CC'ed, especially when the\n> primary author of the feature is not.\n\nThe CC list is the output of `contrib/contacts/git-contacts master..HEAD`.\n\nI’ll include Dscho if I make another series that touches this part of\nthe tree.\n\nCheers\n\n-- \nKristoffer Haugsbakk\n"},{"id":"478322","messageId":"70f3907f-5afd-40aa-bd02-9435a515e074@app.fastmail.com","threadId":"59807","inReplyTo":"xmqqbkhk724x.fsf@gitster.g","subject":"Re: [PATCH v1 3/3] revision: comment `--no-standard-notes` as deprecated","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-06-13T09:46:14Z","receivedAt":"2023-06-13T09:47:25Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Tue, Jun 13, 2023, at 00:21, Junio C Hamano wrote:\n>> [snip]\n>> diff --git a/revision.c b/revision.c\n>> index a0ab7fb784..24219c741a 100644\n>> --- a/revision.c\n>> +++ b/revision.c\n>> @@ -2529,6 +2529,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n>>  \t\tenable_default_display_notes(&revs->notes_opt,\n>>  \t\t\t\t\t     &revs->show_notes);\n>>  \t\trevs->notes_opt.use_default_notes = -1;\n>> +\t\t/* Deprecated */\n>>  \t} else if (!strcmp(arg, \"--no-standard-notes\")) {\n>>  \t\trevs->notes_opt.use_default_notes = 0;\n>>  \t} else if (!strcmp(arg, \"--oneline\")) {\n>\n> With the placement of this new comment, it is unclear which one\n> between \"--standard-notes\" and \"--no-standard-notes\" is getting\n> deprecated (actually, the comment is placed inside the block for the\n> former, so it may be more natural to interpret that the comment\n> marks the former as deprecated).\n>\n> \t} else if (!strcmp(arg, \"--no-standard-notes\")) {\n> +\t\t/* Deprecated */\n> \t\trevs->notes_opt.use_default_notes = 0;\n> \t}\n\nThanks, I’ll use that in the next version if this patch is still\nincluded.\n\n> I am not commenting if it makes sense to declare that the option is\n> deprecated here---I'll leave it to others to argue for/against it.\n> The usual reasoning to add/maintain \"--no-foo\" is so that we can get\n> \"cmd --foo --no-foo\" to naturally work, but it does not apply here?\n\nYeah, because I decided to piggyback on a deprecated command in order to\nmake the change. So this change moves `--standard-notes` from\n“deprecated” to “for internal use”, while `--no-standard-notes`’s status\nremains the same, which is “deprecated”.\n\nUnless the symmetry of `--[no-]standard-notes` turns out to be useful.\n\nLike I’ve said before, I would have preferred to find a way to solve\nthis issue without using an internal switch. But I wasn’t able to.\n\nCheers\n\n-- \nKristoffer Haugsbakk\n"},{"id":"481293","messageId":"cover.1693584310.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1686505920.git.code@khaugsbakk.name","subject":"[PATCH v2 0/3] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-01T16:18:59Z","receivedAt":"2023-09-01T16:20:31Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"The following cover letter is identical to the previous one up to but not\nincluding section “Changes since version 1”.\n\nCheers\n\n🙛 🙙\n\nCurrently, `range-diff` shows the default notes if no notes-related\narguments are given. This is also how `log` behaves. But unlike\n`range-diff`, `log` does *not* show the default notes if\n`--notes=<custom>` are given.\n\nThese changes are supposed to make `format-range` behave like `log` with\nregards to notes.\n\nThese changes also fixes an issue with notes being shared in the cover\nletter via `range-diff`, and that’s really the main motivation for\nmaking these changes.\n\n§ How `log` works\n\n`log` shows the default notes if no notes arguments are given. But if\nyou give it specific notes to show then it forgets about the default\nnotes. Further, there is the convenience `--notes` option which will\nshow the default notes again. These options are cumulative. For example:\n\n    git log --notes --notes=custom\n\nWill show the default notes as well as the `custom` notes.\n\nSee discussion in: https://lore.kernel.org/git/20110329143357.GA10771@sigill.intra.peff.net/\n\n§ How `range-format` works\n\n`range-format` passes `--notes` to `log`, which means that it does not\nhave the default behavior of `log` (forget the default logs if you say\ne.g. `--notes=custom`). However, the man page says that (under\n`--[no-]notes[=<ref>]`):\n\n> This flag is passed to the git log program (see git-log(1)) that generates the patches.\n\nThis makes me (at least) think that `range-format` is supposed to work\njust like `log` with regards to notes.\n\n§ `format-patch` and the difference between showing and sharing\n\n`format-patch` has a different default: it shows no notes. This makes\nsense in my opinion since `format-patch` is meant to be used to share\nchanges with others, and you might be surprised if your notes (which\nmight have only been notes to yourself) are sent out in your emails\n(keep in mind that notes refs are *not* pushed by default).\n\nBut the slightly faulty behavior of `range-diff` bleeds through to\n`format-patch` since the latter calls the former; if you have default\nnotes they can be shared in the range-diff on the cover letter, even\nthough `format-patch` isn’t supposed to show them.\n\n§ Code layout and approach to the problem\n\nAs I’ve said, my focus was on fixing `format-patch`, so I’ve considered\nhow `format-patch` calls `range-diff` which in turn calls `log`.\n\n`format-patch` is a command which is defined in `builtin/log.c`. For\nnotes in particular it in fact has some explicit logic for handling\nnotes based on the value of `rev`. (There seems to be no issues with\nthis part of the code; only the code in `range-diff.c` which passes the\ndefault argument to `log`.) It then calls\n`range-diff.c:show_range_diff`. That function on `master` loads some\ndefault arguments, among them `--notes`. It then eventually calls `log`\nas a subprocess.\n\nMy change consists of co-opting the deprecated `--standard-notes` and\nchanging its behavior so that it can be used in\n`range-diff.c:show_range_diff`.\n\nUsing a special switch/option was the only way I found in order to fix\nthis problem.\n\nI could have also created a new option but I thought that doing a\nsmaller change initially would be better.\n\n§ Changes since version 1\n\nPatch 3: change comment placement based on feedback.\n\n§ Rebase on upstream\n\nRebased on `master` (fc6bba66bc (Merge branch\n'js/allow-t4000-to-be-indented-with-spaces', 2023-08-14)).\n\n§ CI\n\nhttps://github.com/LemmingAvalanche/git/actions/runs/5868463450\n\nKristoffer Haugsbakk (3):\n  range-diff: treat notes like `log`\n  doc: pretty-options: remove documentation for deprecated options\n  revision: comment `--no-standard-notes` as deprecated\n\n Documentation/pretty-options.txt |  1 -\n range-diff.c                     |  2 +-\n revision.c                       |  8 ++++++--\n t/t3206-range-diff.sh            | 28 ++++++++++++++++++++++++++++\n 4 files changed, 35 insertions(+), 4 deletions(-)\n\nRange-diff against v1:\n1:  6a4fe606cf = 1:  e9a5910831 range-diff: treat notes like `log`\n2:  fbc1c47b92 = 2:  f7308b7abf doc: pretty-options: remove documentation for deprecated options\n3:  9141f5a86e ! 3:  80245bbb7e revision: comment `--no-standard-notes` as deprecated\n    @@ Metadata\n      ## Commit message ##\n         revision: comment `--no-standard-notes` as deprecated\n\n    -    We still use `--standard-notes` but this option has no use and is no\n    -    longer documented anywhere.\n    +    `--standard-notes` used to be deprecated but is now (since 6a4fe606cf[1])\n    +    used internally. Its negation `--no-standard-notes`, however, is still\n    +    deprecated even for internal use.\n    +\n    +    Mark this option as such.\n    +\n    +    † 1: range-diff: treat notes like `log`, 2023-05-19\n\n         Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n\n    +\n    + ## Notes (series) ##\n    +    • Move comment: https://lore.kernel.org/git/xmqqbkhk724x.fsf@gitster.g/\n    +    • Tweak commit message so that it's clearer why we are only commenting\n    +      “no” as deprecated and not the other option as well\n    +\n      ## revision.c ##\n     @@ revision.c: static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n    - \t\tenable_default_display_notes(&revs->notes_opt,\n      \t\t\t\t\t     &revs->show_notes);\n      \t\trevs->notes_opt.use_default_notes = -1;\n    -+\t\t/* Deprecated */\n      \t} else if (!strcmp(arg, \"--no-standard-notes\")) {\n    ++\t\t/* Deprecated */\n      \t\trevs->notes_opt.use_default_notes = 0;\n      \t} else if (!strcmp(arg, \"--oneline\")) {\n    + \t\trevs->verbose_header = 1;\n--\n2.41.0\n"},{"id":"481294","messageId":"e9a59108311369d8197b9870a8810d5283ec124f.1693584310.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1693584310.git.code@khaugsbakk.name","subject":"[PATCH v2 1/3] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-01T16:19:00Z","receivedAt":"2023-09-01T16:20:37Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Currently, `range-diff` shows the default notes if no notes-related\narguments are given. This is also how `log` behaves. But unlike\n`range-diff`, `log` does *not* show the default notes if\n`--notes=<custom>` are given. In other words, this:\n\n    git log --notes=custom\n\nis equivalent to this:\n\n    git log --no-notes --notes=custom\n\nWhile:\n\n    git range-diff --notes=custom\n\nacts like this:\n\n    git log --notes --notes-custom\n\nThis can’t be how the user expects `range-diff` to behave given that the\nman page for `range diff` under `--[no-]notes[=<ref>]` says:\n\n> This flag is passed to the git log program (see git-log(1)) that\n> generates the patches.\n\nThis behavior also affects `format-patch` since it uses `range-diff` for\nthe cover letter. Unlike `log`, though, `format-patch` is not supposed\nto show the default notes if no notes-related arguments are given.[1]\nBut this promise is broken when the range-diff happens to have something\nto say about the changes to the default notes, since that will be shown\nin the cover letter.\n\nRemedy this by co-opting the `--standard-notes` option which has been\ndeprecated since ab18b2c0df[2] and which is currently only documented in\n`pretty-options`.\n\n† 1: See e.g. 66b2ed09c2 (Fix \"log\" family not to be too agressive about\n    showing notes, 2010-01-20).\n† 2: log/pretty-options: Document --[no-]notes and deprecate old notes\n    options, 2011-03-30\n\nSigned-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n range-diff.c          |  2 +-\n revision.c            |  7 +++++--\n t/t3206-range-diff.sh | 28 ++++++++++++++++++++++++++++\n 3 files changed, 34 insertions(+), 3 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 2e86063491..f070e4a4ce 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -60,7 +60,7 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t     \"--output-indicator-context=#\",\n \t\t     \"--no-abbrev-commit\",\n \t\t     \"--pretty=medium\",\n-\t\t     \"--notes\",\n+\t\t     \"--standard-notes\",\n \t\t     NULL);\n \tstrvec_push(&cp.args, range);\n \tif (other_arg)\ndiff --git a/revision.c b/revision.c\nindex 2f4c53ea20..64aebc014b 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2495,8 +2495,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\tdisable_display_notes(&revs->notes_opt, &revs->show_notes);\n \t\trevs->show_notes_given = 1;\n \t} else if (!strcmp(arg, \"--standard-notes\")) {\n-\t\trevs->show_notes_given = 1;\n-\t\trevs->notes_opt.use_default_notes = 1;\n+\t\tdisable_display_notes(&revs->notes_opt, &revs->show_notes);\n+\t\trevs->show_notes_given = 0;\n+\t\tenable_default_display_notes(&revs->notes_opt,\n+\t\t\t\t\t     &revs->show_notes);\n+\t\trevs->notes_opt.use_default_notes = -1;\n \t} else if (!strcmp(arg, \"--no-standard-notes\")) {\n \t\trevs->notes_opt.use_default_notes = 0;\n \t} else if (!strcmp(arg, \"--oneline\")) {\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex b5f4d6a653..b33afa1c6a 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -662,6 +662,20 @@ test_expect_success 'range-diff with multiple --notes' '\n \ttest_cmp expect actual\n '\n \n+# `range-diff` should act like `log` with regards to notes\n+test_expect_success 'range-diff with --notes=custom does not show default notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\tgit notes --ref=custom add -m \"topic note\" topic &&\n+\tgit notes --ref=custom add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\ttest_when_finished git notes --ref=custom remove topic unmodified &&\n+\tgit range-diff --notes=custom main..topic main..unmodified \\\n+\t\t>actual &&\n+\t! grep \"## Notes ##\" actual &&\n+\tgrep \"## Notes (custom) ##\" actual\n+'\n+\n test_expect_success 'format-patch --range-diff does not compare notes by default' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n@@ -679,6 +693,20 @@ test_expect_success 'format-patch --range-diff does not compare notes by default\n \t! grep \"note\" 0000-*\n '\n \n+test_expect_success 'format-patch --notes=custom --range-diff only compares custom notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes --ref=custom add -m \"topic note (custom)\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\tgit notes --ref=custom add -m \"unmodified note (custom)\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\ttest_when_finished git notes --ref=custom remove topic unmodified &&\n+\tgit format-patch --notes=custom --cover-letter --range-diff=$prev \\\n+\t\tmain..unmodified >actual &&\n+\ttest_when_finished \"rm 000?-*\" &&\n+\tgrep \"## Notes (custom) ##\" 0000-* &&\n+\t! grep \"## Notes ##\" 0000-*\n+'\n+\n test_expect_success 'format-patch --range-diff with --no-notes' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n-- \n2.41.0\n\n"},{"id":"481295","messageId":"f7308b7abf767828c8c387704dcc5dd775f1c066.1693584310.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1693584310.git.code@khaugsbakk.name","subject":"[PATCH v2 2/3] doc: pretty-options: remove documentation for deprecated options","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-01T16:19:01Z","receivedAt":"2023-09-01T16:20:44Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Remove documentation for options which were deprecated in\nab18b2c0df (log/pretty-options: Document --[no-]notes and deprecate old\nnotes options, 2011-03-30).\n\nWe now use `--standard-notes` internally for `range-diff` (see previous\ncommit).\n\nLeave `--show-notes` as-is since we aren’t changing anything about it.\n\nSigned-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n Documentation/pretty-options.txt | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\nindex dc685be363..8c982609c9 100644\n--- a/Documentation/pretty-options.txt\n+++ b/Documentation/pretty-options.txt\n@@ -88,7 +88,6 @@ being displayed. Examples: \"--notes=foo\" will show only notes from\n \tfrom \"refs/notes/bar\".\n \n --show-notes[=<ref>]::\n---[no-]standard-notes::\n \tThese options are deprecated. Use the above --notes/--no-notes\n \toptions instead.\n endif::git-rev-list[]\n-- \n2.41.0\n\n"},{"id":"481296","messageId":"80245bbb7e5467c6d4dd056eff9f12b810ba7129.1693584310.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1693584310.git.code@khaugsbakk.name","subject":"[PATCH v2 3/3] revision: comment `--no-standard-notes` as deprecated","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-01T16:19:02Z","receivedAt":"2023-09-01T16:20:48Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"`--standard-notes` used to be deprecated but is now (since 6a4fe606cf[1])\nused internally. Its negation `--no-standard-notes`, however, is still\ndeprecated even for internal use.\n\nMark this option as such.\n\n† 1: range-diff: treat notes like `log`, 2023-05-19\n\nSigned-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n\nNotes (series):\n    • Move comment: https://lore.kernel.org/git/xmqqbkhk724x.fsf@gitster.g/\n    • Tweak commit message so that it's clearer why we are only commenting\n      “no” as deprecated and not the other option as well\n\n revision.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/revision.c b/revision.c\nindex 64aebc014b..44a04004a7 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2501,6 +2501,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\t\t\t\t     &revs->show_notes);\n \t\trevs->notes_opt.use_default_notes = -1;\n \t} else if (!strcmp(arg, \"--no-standard-notes\")) {\n+\t\t/* Deprecated */\n \t\trevs->notes_opt.use_default_notes = 0;\n \t} else if (!strcmp(arg, \"--oneline\")) {\n \t\trevs->verbose_header = 1;\n-- \n2.41.0\n\n"},{"id":"481349","messageId":"94b9535b-8c2a-eb8f-90fb-cd0f998ec57e@gmx.de","threadId":"59807","inReplyTo":"e9a59108311369d8197b9870a8810d5283ec124f.1693584310.git.code@khaugsbakk.name","subject":"Re: [PATCH v2 1/3] range-diff: treat notes like `log`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2023-09-03T12:17:59Z","receivedAt":"2023-09-03T12:18:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Kristoffer,\n\nOn Fri, 1 Sep 2023, Kristoffer Haugsbakk wrote:\n\n> Currently, `range-diff` shows the default notes if no notes-related\n> arguments are given. This is also how `log` behaves. But unlike\n> `range-diff`, `log` does *not* show the default notes if\n> `--notes=<custom>` are given. In other words, this:\n>\n>     git log --notes=custom\n>\n> is equivalent to this:\n>\n>     git log --no-notes --notes=custom\n>\n> While:\n>\n>     git range-diff --notes=custom\n>\n> acts like this:\n>\n>     git log --notes --notes-custom\n>\n> This can’t be how the user expects `range-diff` to behave given that the\n> man page for `range diff` under `--[no-]notes[=<ref>]` says:\n>\n> > This flag is passed to the git log program (see git-log(1)) that\n> > generates the patches.\n>\n> This behavior also affects `format-patch` since it uses `range-diff` for\n> the cover letter. Unlike `log`, though, `format-patch` is not supposed\n> to show the default notes if no notes-related arguments are given.[1]\n> But this promise is broken when the range-diff happens to have something\n> to say about the changes to the default notes, since that will be shown\n> in the cover letter.\n\nVery well explained.\n\nThe root cause for this is 8cf51561d1e (range-diff: fix a crash in parsing\ngit-log output, 2020-04-15) which added the `--notes` argument in\n`read_patches()`' call. The commit message explains why this is needed:\nthe (necessary) `--pretty=medium` would turn off the notes, therefore\n`--notes` had to be added to reinstate the original behavior (except, as\nyou pointed out, in the case `--notes=<ref>` was specified).\n\n> Remedy this by co-opting the `--standard-notes` option which has been\n> deprecated since ab18b2c0df[2] and which is currently only documented in\n> `pretty-options`.\n\nThis sounds a bit less desirable, though, than passing the `--notes`\nargument only as needed. This patch (on top of `notes-range-diff` in your\nfork) lets the new test still pass while leaving `--standard-notes`\ndeprecated:\n\n-- snipsnap --\ndiff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\nindex 8c982609c99..dc685be363a 100644\n--- a/Documentation/pretty-options.txt\n+++ b/Documentation/pretty-options.txt\n@@ -88,6 +88,7 @@ being displayed. Examples: \"--notes=foo\" will show only notes from\n \tfrom \"refs/notes/bar\".\n\n --show-notes[=<ref>]::\n+--[no-]standard-notes::\n \tThese options are deprecated. Use the above --notes/--no-notes\n \toptions instead.\n endif::git-rev-list[]\ndiff --git a/range-diff.c b/range-diff.c\nindex f070e4a4ceb..fbb81a92cc0 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -41,12 +41,20 @@ static int read_patches(const char *range, struct string_list *list,\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf buf = STRBUF_INIT, contents = STRBUF_INIT;\n \tstruct patch_util *util = NULL;\n-\tint in_header = 1;\n+\tint i, implicit_notes_arg = 1, in_header = 1;\n \tchar *line, *current_filename = NULL;\n \tssize_t len;\n \tsize_t size;\n \tint ret = -1;\n\n+\tfor (i = 0; other_arg && i < other_arg->nr; i++)\n+\t\tif (!strcmp(other_arg->v[i], \"--notes\") ||\n+\t\t    starts_with(other_arg->v[i], \"--notes=\") ||\n+\t\t    !strcmp(other_arg->v[i], \"--no-notes\")) {\n+\t\t\timplicit_notes_arg = 0;\n+\t\t\tbreak;\n+\t\t}\n+\n \tstrvec_pushl(&cp.args, \"log\", \"--no-color\", \"-p\", \"--no-merges\",\n \t\t     \"--reverse\", \"--date-order\", \"--decorate=no\",\n \t\t     \"--no-prefix\", \"--submodule=short\",\n@@ -60,8 +68,9 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t     \"--output-indicator-context=#\",\n \t\t     \"--no-abbrev-commit\",\n \t\t     \"--pretty=medium\",\n-\t\t     \"--standard-notes\",\n \t\t     NULL);\n+\tif (implicit_notes_arg)\n+\t\t     strvec_push(&cp.args, \"--notes\");\n \tstrvec_push(&cp.args, range);\n \tif (other_arg)\n \t\tstrvec_pushv(&cp.args, other_arg->v);\ndiff --git a/revision.c b/revision.c\nindex 44a04004a70..2f4c53ea207 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2495,13 +2495,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\tdisable_display_notes(&revs->notes_opt, &revs->show_notes);\n \t\trevs->show_notes_given = 1;\n \t} else if (!strcmp(arg, \"--standard-notes\")) {\n-\t\tdisable_display_notes(&revs->notes_opt, &revs->show_notes);\n-\t\trevs->show_notes_given = 0;\n-\t\tenable_default_display_notes(&revs->notes_opt,\n-\t\t\t\t\t     &revs->show_notes);\n-\t\trevs->notes_opt.use_default_notes = -1;\n+\t\trevs->show_notes_given = 1;\n+\t\trevs->notes_opt.use_default_notes = 1;\n \t} else if (!strcmp(arg, \"--no-standard-notes\")) {\n-\t\t/* Deprecated */\n \t\trevs->notes_opt.use_default_notes = 0;\n \t} else if (!strcmp(arg, \"--oneline\")) {\n \t\trevs->verbose_header = 1;\n"},{"id":"481374","messageId":"8642763e-d3e7-49c4-b2ea-d5e4bebfbca5@app.fastmail.com","threadId":"59807","inReplyTo":"94b9535b-8c2a-eb8f-90fb-cd0f998ec57e@gmx.de","subject":"Re: [PATCH v2 1/3] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-04T17:10:49Z","receivedAt":"2023-09-04T17:14:02Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Hi Dscho\n\nOn Sun, Sep 3, 2023, at 14:17, Johannes Schindelin wrote:\n> Hi Kristoffer,\n>\n> On Fri, 1 Sep 2023, Kristoffer Haugsbakk wrote:\n>\n> > [snip]\n>\n> Very well explained.\n>\n> The root cause for this is 8cf51561d1e (range-diff: fix a crash in parsing\n> git-log output, 2020-04-15) which added the `--notes` argument in\n> `read_patches()`' call. The commit message explains why this is needed:\n> the (necessary) `--pretty=medium` would turn off the notes, therefore\n> `--notes` had to be added to reinstate the original behavior (except, as\n> you pointed out, in the case `--notes=<ref>` was specified).\n\nThat's interesting. I didn't find that commit in my history spelunking.\n\n>> Remedy this by co-opting the `--standard-notes` option which has been\n>> deprecated since ab18b2c0df[2] and which is currently only documented in\n>> `pretty-options`.\n>\n> This sounds a bit less desirable, though, than passing the `--notes`\n> argument only as needed. This patch (on top of `notes-range-diff` in your\n> fork) lets the new test still pass while leaving `--standard-notes`\n> deprecated:\n>\n> [snip]\n\nI like it. I didn't like my solution but it was the only thing that I\ncould get to work. I would like to use your solution instead. Thank you.\n\nMaybe you could supply a commit message for v3? v3 would then consist of\ntwo commits:\n\n1. Your fix\n2. Those two tests\n\nOr should it be handled in some other way?\n\nCheers\n\n-- \nKristoffer Haugsbakk\n"},{"id":"481383","messageId":"843bc10e-03ab-b37a-68f7-d6055dc9308c@gmx.de","threadId":"59807","inReplyTo":"8642763e-d3e7-49c4-b2ea-d5e4bebfbca5@app.fastmail.com","subject":"Re: [PATCH v2 1/3] range-diff: treat notes like `log`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2023-09-05T10:56:51Z","receivedAt":"2023-09-05T15:59:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Kristoffer,\n\nOn Mon, 4 Sep 2023, Kristoffer Haugsbakk wrote:\n\n> I would like to use your solution instead. Thank you.\n\nPlease do! You're welcome.\n\n> Maybe you could supply a commit message for v3? v3 would then consist of\n> two commits:\n>\n> 1. Your fix\n> 2. Those two tests\n>\n> Or should it be handled in some other way?\n\nPersonally, I'd prefer it if you just squashed the changes into your\npatch. If you want, I'd be delighted about a Helped-by (or even\nCo-authored-by) footer, but taking ownership would be fine with me, too.\n\nCiao,\nJohannes\n"},{"id":"481417","messageId":"xmqqledkqn3o.fsf@gitster.g","threadId":"59807","inReplyTo":"843bc10e-03ab-b37a-68f7-d6055dc9308c@gmx.de","subject":"Re: [PATCH v2 1/3] range-diff: treat notes like `log`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-05T22:19:55Z","receivedAt":"2023-09-05T22:19:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi Kristoffer,\n>\n> On Mon, 4 Sep 2023, Kristoffer Haugsbakk wrote:\n>\n>> I would like to use your solution instead. Thank you.\n>\n> Please do! You're welcome.\n>\n>> Maybe you could supply a commit message for v3? v3 would then consist of\n>> two commits:\n>>\n>> 1. Your fix\n>> 2. Those two tests\n>>\n>> Or should it be handled in some other way?\n>\n> Personally, I'd prefer it if you just squashed the changes into your\n> patch. If you want, I'd be delighted about a Helped-by (or even\n> Co-authored-by) footer, but taking ownership would be fine with me, too.\n\nSounds very sensible.\n\nThanks, both.  Will look forward to seeing an updated series.\n"},{"id":"481660","messageId":"cover.1694383247.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1693584310.git.code@khaugsbakk.name","subject":"[PATCH v3 0/1] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-10T22:06:22Z","receivedAt":"2023-09-10T22:07:10Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Hi\n\nThe cover letter up until the “Changes” section is mostly the same except\nI deleted the justification for my approach to the problem since we don't\nuse that approach any more.\n\nCheers\n\n🙛 🙙\n\nCurrently, `range-diff` shows the default notes if no notes-related\narguments are given. This is also how `log` behaves. But unlike\n`range-diff`, `log` does *not* show the default notes if\n`--notes=<custom>` are given.\n\nThese changes are supposed to make `format-range` behave like `log` with\nregards to notes.\n\nThese changes also fixes an issue with notes being shared in the cover\nletter via `range-diff`, and that’s really the main motivation for\nmaking these changes.\n\n§ How `log` works\n\n`log` shows the default notes if no notes arguments are given. But if\nyou give it specific notes to show then it forgets about the default\nnotes. Further, there is the convenience `--notes` option which will\nshow the default notes again. These options are cumulative. For example:\n\n    git log --notes --notes=custom\n\nWill show the default notes as well as the `custom` notes.\n\nSee discussion in: https://lore.kernel.org/git/20110329143357.GA10771@sigill.intra.peff.net/\n\n§ How `range-format` works\n\n`range-format` passes `--notes` to `log`, which means that it does not\nhave the default behavior of `log` (forget the default logs if you say\ne.g. `--notes=custom`). However, the man page says that (under\n`--[no-]notes[=<ref>]`):\n\n> This flag is passed to the git log program (see git-log(1)) that generates the patches.\n\nThis makes me (at least) think that `range-format` is supposed to work\njust like `log` with regards to notes.\n\n§ `format-patch` and the difference between showing and sharing\n\n`format-patch` has a different default: it shows no notes. This makes\nsense in my opinion since `format-patch` is meant to be used to share\nchanges with others, and you might be surprised if your notes (which\nmight have only been notes to yourself) are sent out in your emails\n(keep in mind that notes refs are *not* pushed by default).\n\nBut the slightly faulty behavior of `range-diff` bleeds through to\n`format-patch` since the latter calls the former; if you have default\nnotes they can be shared in the range-diff on the cover letter, even\nthough `format-patch` isn’t supposed to show them.\n\n§ Changes since version 2\n\nDscho provided an [alternative] solution. My three patches and his patch\nhave been squashed into one.\n\n🔗 alternative: https://lore.kernel.org/git/94b9535b-8c2a-eb8f-90fb-cd0f998ec57e@gmx.de/\n\n§ CI\n\nhttps://github.com/LemmingAvalanche/git/actions/runs/6139150031\n\n\nKristoffer Haugsbakk (1):\n  range-diff: treat notes like `log`\n\n range-diff.c          | 13 +++++++++++--\n t/t3206-range-diff.sh | 28 ++++++++++++++++++++++++++++\n 2 files changed, 39 insertions(+), 2 deletions(-)\n\nRange-diff against v2:\n1:  e9a5910831 ! 1:  a37dfb3748 range-diff: treat notes like `log`\n    @@ Commit message\n             git log --notes --notes-custom\n\n         This can’t be how the user expects `range-diff` to behave given that the\n    -    man page for `range diff` under `--[no-]notes[=<ref>]` says:\n    +    man page for `range-diff` under `--[no-]notes[=<ref>]` says:\n\n    -    > This flag is passed to the git log program (see git-log(1)) that\n    +    > This flag is passed to the `git log` program (see git-log(1)) that\n         > generates the patches.\n\n         This behavior also affects `format-patch` since it uses `range-diff` for\n    @@ Commit message\n         to say about the changes to the default notes, since that will be shown\n         in the cover letter.\n\n    -    Remedy this by co-opting the `--standard-notes` option which has been\n    -    deprecated since ab18b2c0df[2] and which is currently only documented in\n    -    `pretty-options`.\n    +    Remedy this by only conditionally passing in `--notes` to `range-diff`.\n    +\n    +    § Root cause\n    +\n    +    8cf51561d1e (range-diff: fix a crash in parsing git-log output,\n    +    2020-04-15) added `--notes` in order to deal with a side-effect of\n    +    `--pretty=medium`:\n    +\n    +    > To fix this explicitly set the output format of the internally executed\n    +    > `git log` with `--pretty=medium`. Because that cancels `--notes`, add\n    +    > explicitly `--notes` at the end.\n    +\n    +    § Authors\n    +\n    +    • Fix by Johannes\n    +    • Tests by Kristoffer\n\n         † 1: See e.g. 66b2ed09c2 (Fix \"log\" family not to be too agressive about\n             showing notes, 2010-01-20).\n    -    † 2: log/pretty-options: Document --[no-]notes and deprecate old notes\n    -        options, 2011-03-30\n\n    +    Co-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n         Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n\n      ## range-diff.c ##\n    +@@ range-diff.c: static int read_patches(const char *range, struct string_list *list,\n    + \tstruct child_process cp = CHILD_PROCESS_INIT;\n    + \tstruct strbuf buf = STRBUF_INIT, contents = STRBUF_INIT;\n    + \tstruct patch_util *util = NULL;\n    +-\tint in_header = 1;\n    ++\tint i, implicit_notes_arg = 1, in_header = 1;\n    + \tchar *line, *current_filename = NULL;\n    + \tssize_t len;\n    + \tsize_t size;\n    + \tint ret = -1;\n    +\n    ++\tfor (i = 0; other_arg && i < other_arg->nr; i++)\n    ++\t\tif (!strcmp(other_arg->v[i], \"--notes\") ||\n    ++\t\t    starts_with(other_arg->v[i], \"--notes=\") ||\n    ++\t\t    !strcmp(other_arg->v[i], \"--no-notes\")) {\n    ++\t\t\timplicit_notes_arg = 0;\n    ++\t\t\tbreak;\n    ++\t\t}\n    ++\n    + \tstrvec_pushl(&cp.args, \"log\", \"--no-color\", \"-p\", \"--no-merges\",\n    + \t\t     \"--reverse\", \"--date-order\", \"--decorate=no\",\n    + \t\t     \"--no-prefix\", \"--submodule=short\",\n     @@ range-diff.c: static int read_patches(const char *range, struct string_list *list,\n      \t\t     \"--output-indicator-context=#\",\n      \t\t     \"--no-abbrev-commit\",\n      \t\t     \"--pretty=medium\",\n     -\t\t     \"--notes\",\n    -+\t\t     \"--standard-notes\",\n      \t\t     NULL);\n    ++\tif (implicit_notes_arg)\n    ++\t\t     strvec_push(&cp.args, \"--notes\");\n      \tstrvec_push(&cp.args, range);\n      \tif (other_arg)\n    -\n    - ## revision.c ##\n    -@@ revision.c: static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n    - \t\tdisable_display_notes(&revs->notes_opt, &revs->show_notes);\n    - \t\trevs->show_notes_given = 1;\n    - \t} else if (!strcmp(arg, \"--standard-notes\")) {\n    --\t\trevs->show_notes_given = 1;\n    --\t\trevs->notes_opt.use_default_notes = 1;\n    -+\t\tdisable_display_notes(&revs->notes_opt, &revs->show_notes);\n    -+\t\trevs->show_notes_given = 0;\n    -+\t\tenable_default_display_notes(&revs->notes_opt,\n    -+\t\t\t\t\t     &revs->show_notes);\n    -+\t\trevs->notes_opt.use_default_notes = -1;\n    - \t} else if (!strcmp(arg, \"--no-standard-notes\")) {\n    - \t\trevs->notes_opt.use_default_notes = 0;\n    - \t} else if (!strcmp(arg, \"--oneline\")) {\n    + \t\tstrvec_pushv(&cp.args, other_arg->v);\n\n      ## t/t3206-range-diff.sh ##\n     @@ t/t3206-range-diff.sh: test_expect_success 'range-diff with multiple --notes' '\n2:  f7308b7abf < -:  ---------- doc: pretty-options: remove documentation for deprecated options\n3:  80245bbb7e < -:  ---------- revision: comment `--no-standard-notes` as deprecated\n--\n2.42.0\n"},{"id":"481661","messageId":"a37dfb3748e23b4f5081bc9a3c80a5c546101f1d.1694383248.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1694383247.git.code@khaugsbakk.name","subject":"[PATCH v3 1/1] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-10T22:06:23Z","receivedAt":"2023-09-10T22:07:25Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Currently, `range-diff` shows the default notes if no notes-related\narguments are given. This is also how `log` behaves. But unlike\n`range-diff`, `log` does *not* show the default notes if\n`--notes=<custom>` are given. In other words, this:\n\n    git log --notes=custom\n\nis equivalent to this:\n\n    git log --no-notes --notes=custom\n\nWhile:\n\n    git range-diff --notes=custom\n\nacts like this:\n\n    git log --notes --notes-custom\n\nThis can’t be how the user expects `range-diff` to behave given that the\nman page for `range-diff` under `--[no-]notes[=<ref>]` says:\n\n> This flag is passed to the `git log` program (see git-log(1)) that\n> generates the patches.\n\nThis behavior also affects `format-patch` since it uses `range-diff` for\nthe cover letter. Unlike `log`, though, `format-patch` is not supposed\nto show the default notes if no notes-related arguments are given.[1]\nBut this promise is broken when the range-diff happens to have something\nto say about the changes to the default notes, since that will be shown\nin the cover letter.\n\nRemedy this by only conditionally passing in `--notes` to `range-diff`.\n\n§ Root cause\n\n8cf51561d1e (range-diff: fix a crash in parsing git-log output,\n2020-04-15) added `--notes` in order to deal with a side-effect of\n`--pretty=medium`:\n\n> To fix this explicitly set the output format of the internally executed\n> `git log` with `--pretty=medium`. Because that cancels `--notes`, add\n> explicitly `--notes` at the end.\n\n§ Authors\n\n• Fix by Johannes\n• Tests by Kristoffer\n\n† 1: See e.g. 66b2ed09c2 (Fix \"log\" family not to be too agressive about\n    showing notes, 2010-01-20).\n\nCo-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nSigned-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n range-diff.c          | 13 +++++++++++--\n t/t3206-range-diff.sh | 28 ++++++++++++++++++++++++++++\n 2 files changed, 39 insertions(+), 2 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 2e86063491..fbb81a92cc 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -41,12 +41,20 @@ static int read_patches(const char *range, struct string_list *list,\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf buf = STRBUF_INIT, contents = STRBUF_INIT;\n \tstruct patch_util *util = NULL;\n-\tint in_header = 1;\n+\tint i, implicit_notes_arg = 1, in_header = 1;\n \tchar *line, *current_filename = NULL;\n \tssize_t len;\n \tsize_t size;\n \tint ret = -1;\n \n+\tfor (i = 0; other_arg && i < other_arg->nr; i++)\n+\t\tif (!strcmp(other_arg->v[i], \"--notes\") ||\n+\t\t    starts_with(other_arg->v[i], \"--notes=\") ||\n+\t\t    !strcmp(other_arg->v[i], \"--no-notes\")) {\n+\t\t\timplicit_notes_arg = 0;\n+\t\t\tbreak;\n+\t\t}\n+\n \tstrvec_pushl(&cp.args, \"log\", \"--no-color\", \"-p\", \"--no-merges\",\n \t\t     \"--reverse\", \"--date-order\", \"--decorate=no\",\n \t\t     \"--no-prefix\", \"--submodule=short\",\n@@ -60,8 +68,9 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t     \"--output-indicator-context=#\",\n \t\t     \"--no-abbrev-commit\",\n \t\t     \"--pretty=medium\",\n-\t\t     \"--notes\",\n \t\t     NULL);\n+\tif (implicit_notes_arg)\n+\t\t     strvec_push(&cp.args, \"--notes\");\n \tstrvec_push(&cp.args, range);\n \tif (other_arg)\n \t\tstrvec_pushv(&cp.args, other_arg->v);\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex b5f4d6a653..b33afa1c6a 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -662,6 +662,20 @@ test_expect_success 'range-diff with multiple --notes' '\n \ttest_cmp expect actual\n '\n \n+# `range-diff` should act like `log` with regards to notes\n+test_expect_success 'range-diff with --notes=custom does not show default notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\tgit notes --ref=custom add -m \"topic note\" topic &&\n+\tgit notes --ref=custom add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\ttest_when_finished git notes --ref=custom remove topic unmodified &&\n+\tgit range-diff --notes=custom main..topic main..unmodified \\\n+\t\t>actual &&\n+\t! grep \"## Notes ##\" actual &&\n+\tgrep \"## Notes (custom) ##\" actual\n+'\n+\n test_expect_success 'format-patch --range-diff does not compare notes by default' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n@@ -679,6 +693,20 @@ test_expect_success 'format-patch --range-diff does not compare notes by default\n \t! grep \"note\" 0000-*\n '\n \n+test_expect_success 'format-patch --notes=custom --range-diff only compares custom notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes --ref=custom add -m \"topic note (custom)\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\tgit notes --ref=custom add -m \"unmodified note (custom)\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\ttest_when_finished git notes --ref=custom remove topic unmodified &&\n+\tgit format-patch --notes=custom --cover-letter --range-diff=$prev \\\n+\t\tmain..unmodified >actual &&\n+\ttest_when_finished \"rm 000?-*\" &&\n+\tgrep \"## Notes (custom) ##\" 0000-* &&\n+\t! grep \"## Notes ##\" 0000-*\n+'\n+\n test_expect_success 'format-patch --range-diff with --no-notes' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n-- \n2.42.0\n\n"},{"id":"481676","messageId":"6e368e95-da48-1afc-01e8-58e23a6117dd@gmx.de","threadId":"59807","inReplyTo":"cover.1694383247.git.code@khaugsbakk.name","subject":"Re: [PATCH v3 0/1] range-diff: treat notes like `log`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2023-09-11T13:23:31Z","receivedAt":"2023-09-11T21:38:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Kristoffer,\n\nOn Mon, 11 Sep 2023, Kristoffer Haugsbakk wrote:\n\n> § Changes since version 2\n>\n> Dscho provided an [alternative] solution. My three patches and his patch\n> have been squashed into one.\n>\n> 🔗 alternative: https://lore.kernel.org/git/94b9535b-8c2a-eb8f-90fb-cd0f998ec57e@gmx.de/\n\nThank you, this iteration looks good to me!\n\nCiao,\nJohannes\n"},{"id":"481679","messageId":"xmqqzg1strgx.fsf@gitster.g","threadId":"59807","inReplyTo":"a37dfb3748e23b4f5081bc9a3c80a5c546101f1d.1694383248.git.code@khaugsbakk.name","subject":"Re: [PATCH v3 1/1] range-diff: treat notes like `log`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-11T19:55:26Z","receivedAt":"2023-09-11T21:38:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristoffer Haugsbakk <code@khaugsbakk.name> writes:\n\n>> To fix this explicitly set the output format of the internally executed\n>> `git log` with `--pretty=medium`. Because that cancels `--notes`, add\n>> explicitly `--notes` at the end.\n>\n> § Authors\n>\n> • Fix by Johannes\n> • Tests by Kristoffer\n>\n> † 1: See e.g. 66b2ed09c2 (Fix \"log\" family not to be too agressive about\n>     showing notes, 2010-01-20).\n>\n> Co-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n> ---\n\nOK, Dscho, does this round look acceptable to you?\n\nIt feels UGLY to iterate over args _without_ actually parsing them,\nat least to me.  Such a non-parsing look breaks at least in two ways\nover time. (1) a mechanism may be introduced laster, similar to\n\"--\", that allows other_arg->v[] array to mark \"here is where the\ndashed options end\".  Now the existing loop keeps reading to the end\nand finds \"--notes\" that is not a dashed option but is part of the\nnormal command line arguments in \"other arg\".  (2) Among the dashed\noptions passed in the other_arg->v[], there may be an option that\ntakes a string value, and a value that happens to be \"--notes\" is\nmistaken as asking for \"notes\" (iow \"git log -G --notes\" is looking\nfor commits with changes that contain \"double dash followed by en oh\ntee ee es\").\n\nI think \"git range-diff -G --notes\" (or \"-S --notes\") shows that\nthis new non-parsing loop is already broken.  It looks for a change\nthat has \"--notes\" correctly, but at the same time, triggers that\n\"ah, we have an explicit --notes so drop the implicit_notes_arg\nflag\" logic.\n\n>  range-diff.c          | 13 +++++++++++--\n>  t/t3206-range-diff.sh | 28 ++++++++++++++++++++++++++++\n>  2 files changed, 39 insertions(+), 2 deletions(-)\n>\n> diff --git a/range-diff.c b/range-diff.c\n> index 2e86063491..fbb81a92cc 100644\n> --- a/range-diff.c\n> +++ b/range-diff.c\n> @@ -41,12 +41,20 @@ static int read_patches(const char *range, struct string_list *list,\n>  \tstruct child_process cp = CHILD_PROCESS_INIT;\n>  \tstruct strbuf buf = STRBUF_INIT, contents = STRBUF_INIT;\n>  \tstruct patch_util *util = NULL;\n> -\tint in_header = 1;\n> +\tint i, implicit_notes_arg = 1, in_header = 1;\n>  \tchar *line, *current_filename = NULL;\n>  \tssize_t len;\n>  \tsize_t size;\n>  \tint ret = -1;\n>  \n> +\tfor (i = 0; other_arg && i < other_arg->nr; i++)\n> +\t\tif (!strcmp(other_arg->v[i], \"--notes\") ||\n> +\t\t    starts_with(other_arg->v[i], \"--notes=\") ||\n> +\t\t    !strcmp(other_arg->v[i], \"--no-notes\")) {\n> +\t\t\timplicit_notes_arg = 0;\n> +\t\t\tbreak;\n> +\t\t}\n> +\n>  \tstrvec_pushl(&cp.args, \"log\", \"--no-color\", \"-p\", \"--no-merges\",\n>  \t\t     \"--reverse\", \"--date-order\", \"--decorate=no\",\n>  \t\t     \"--no-prefix\", \"--submodule=short\",\n> @@ -60,8 +68,9 @@ static int read_patches(const char *range, struct string_list *list,\n>  \t\t     \"--output-indicator-context=#\",\n>  \t\t     \"--no-abbrev-commit\",\n>  \t\t     \"--pretty=medium\",\n> -\t\t     \"--notes\",\n>  \t\t     NULL);\n> +\tif (implicit_notes_arg)\n> +\t\t     strvec_push(&cp.args, \"--notes\");\n>  \tstrvec_push(&cp.args, range);\n>  \tif (other_arg)\n>  \t\tstrvec_pushv(&cp.args, other_arg->v);\n> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\n> index b5f4d6a653..b33afa1c6a 100755\n> --- a/t/t3206-range-diff.sh\n> +++ b/t/t3206-range-diff.sh\n> @@ -662,6 +662,20 @@ test_expect_success 'range-diff with multiple --notes' '\n>  \ttest_cmp expect actual\n>  '\n>  \n> +# `range-diff` should act like `log` with regards to notes\n> +test_expect_success 'range-diff with --notes=custom does not show default notes' '\n> +\tgit notes add -m \"topic note\" topic &&\n> +\tgit notes add -m \"unmodified note\" unmodified &&\n> +\tgit notes --ref=custom add -m \"topic note\" topic &&\n> +\tgit notes --ref=custom add -m \"unmodified note\" unmodified &&\n> +\ttest_when_finished git notes remove topic unmodified &&\n> +\ttest_when_finished git notes --ref=custom remove topic unmodified &&\n> +\tgit range-diff --notes=custom main..topic main..unmodified \\\n> +\t\t>actual &&\n> +\t! grep \"## Notes ##\" actual &&\n> +\tgrep \"## Notes (custom) ##\" actual\n> +'\n> +\n>  test_expect_success 'format-patch --range-diff does not compare notes by default' '\n>  \tgit notes add -m \"topic note\" topic &&\n>  \tgit notes add -m \"unmodified note\" unmodified &&\n> @@ -679,6 +693,20 @@ test_expect_success 'format-patch --range-diff does not compare notes by default\n>  \t! grep \"note\" 0000-*\n>  '\n>  \n> +test_expect_success 'format-patch --notes=custom --range-diff only compares custom notes' '\n> +\tgit notes add -m \"topic note\" topic &&\n> +\tgit notes --ref=custom add -m \"topic note (custom)\" topic &&\n> +\tgit notes add -m \"unmodified note\" unmodified &&\n> +\tgit notes --ref=custom add -m \"unmodified note (custom)\" unmodified &&\n> +\ttest_when_finished git notes remove topic unmodified &&\n> +\ttest_when_finished git notes --ref=custom remove topic unmodified &&\n> +\tgit format-patch --notes=custom --cover-letter --range-diff=$prev \\\n> +\t\tmain..unmodified >actual &&\n> +\ttest_when_finished \"rm 000?-*\" &&\n> +\tgrep \"## Notes (custom) ##\" 0000-* &&\n> +\t! grep \"## Notes ##\" 0000-*\n> +'\n> +\n>  test_expect_success 'format-patch --range-diff with --no-notes' '\n>  \tgit notes add -m \"topic note\" topic &&\n>  \tgit notes add -m \"unmodified note\" unmodified &&\n"},{"id":"481832","messageId":"dd2958c5-58bf-86dd-b666-9033259a8e1a@gmx.de","threadId":"59807","inReplyTo":"xmqqzg1strgx.fsf@gitster.g","subject":"Re: [PATCH v3 1/1] range-diff: treat notes like `log`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2023-09-14T08:29:31Z","receivedAt":"2023-09-14T08:29:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 11 Sep 2023, Junio C Hamano wrote:\n\n> Kristoffer Haugsbakk <code@khaugsbakk.name> writes:\n>\n> >> To fix this explicitly set the output format of the internally executed\n> >> `git log` with `--pretty=medium`. Because that cancels `--notes`, add\n> >> explicitly `--notes` at the end.\n> >\n> > § Authors\n> >\n> > • Fix by Johannes\n> > • Tests by Kristoffer\n> >\n> > † 1: See e.g. 66b2ed09c2 (Fix \"log\" family not to be too agressive about\n> >     showing notes, 2010-01-20).\n> >\n> > Co-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n> > Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n> > ---\n>\n> OK, Dscho, does this round look acceptable to you?\n>\n> It feels UGLY to iterate over args _without_ actually parsing them,\n> at least to me.  Such a non-parsing look breaks at least in two ways\n> over time. (1) a mechanism may be introduced laster, similar to\n> \"--\", that allows other_arg->v[] array to mark \"here is where the\n> dashed options end\".  Now the existing loop keeps reading to the end\n> and finds \"--notes\" that is not a dashed option but is part of the\n> normal command line arguments in \"other arg\".  (2) Among the dashed\n> options passed in the other_arg->v[], there may be an option that\n> takes a string value, and a value that happens to be \"--notes\" is\n> mistaken as asking for \"notes\" (iow \"git log -G --notes\" is looking\n> for commits with changes that contain \"double dash followed by en oh\n> tee ee es\").\n>\n> I think \"git range-diff -G --notes\" (or \"-S --notes\") shows that\n> this new non-parsing loop is already broken.  It looks for a change\n> that has \"--notes\" correctly, but at the same time, triggers that\n> \"ah, we have an explicit --notes so drop the implicit_notes_arg\n> flag\" logic.\n\nRight, `-G --notes` is a good argument to rethink this.\n\nA much more surgical way to address the issue at hand might be this\n(Kristoffer, what do you think? It's missing documentation for the\nnewly-introduced `--show-notes-by-default` option, but you get the idea):\n\n-- snipsnap --\ndiff --git a/range-diff.c b/range-diff.c\nindex fbb81a92cc0..56f6870ff91 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -41,20 +41,12 @@ static int read_patches(const char *range, struct string_list *list,\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf buf = STRBUF_INIT, contents = STRBUF_INIT;\n \tstruct patch_util *util = NULL;\n-\tint i, implicit_notes_arg = 1, in_header = 1;\n+\tint in_header = 1;\n \tchar *line, *current_filename = NULL;\n \tssize_t len;\n \tsize_t size;\n \tint ret = -1;\n\n-\tfor (i = 0; other_arg && i < other_arg->nr; i++)\n-\t\tif (!strcmp(other_arg->v[i], \"--notes\") ||\n-\t\t    starts_with(other_arg->v[i], \"--notes=\") ||\n-\t\t    !strcmp(other_arg->v[i], \"--no-notes\")) {\n-\t\t\timplicit_notes_arg = 0;\n-\t\t\tbreak;\n-\t\t}\n-\n \tstrvec_pushl(&cp.args, \"log\", \"--no-color\", \"-p\", \"--no-merges\",\n \t\t     \"--reverse\", \"--date-order\", \"--decorate=no\",\n \t\t     \"--no-prefix\", \"--submodule=short\",\n@@ -68,9 +60,8 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t     \"--output-indicator-context=#\",\n \t\t     \"--no-abbrev-commit\",\n \t\t     \"--pretty=medium\",\n+\t\t     \"--show-notes-by-default\",\n \t\t     NULL);\n-\tif (implicit_notes_arg)\n-\t\t     strvec_push(&cp.args, \"--notes\");\n \tstrvec_push(&cp.args, range);\n \tif (other_arg)\n \t\tstrvec_pushv(&cp.args, other_arg->v);\ndiff --git a/revision.c b/revision.c\nindex 2f4c53ea207..49d385257ac 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2484,6 +2484,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->break_bar = xstrdup(optarg);\n \t\trevs->track_linear = 1;\n \t\trevs->track_first_time = 1;\n+\t} else if (!strcmp(arg, \"--show-notes-by-default\")) {\n+\t\trevs->show_notes_by_default = 1;\n \t} else if (skip_prefix(arg, \"--show-notes=\", &optarg) ||\n \t\t   skip_prefix(arg, \"--notes=\", &optarg)) {\n \t\tif (starts_with(arg, \"--show-notes=\") &&\n@@ -3054,6 +3056,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \tif (revs->expand_tabs_in_log < 0)\n \t\trevs->expand_tabs_in_log = revs->expand_tabs_in_log_default;\n\n+\tif (!revs->show_notes_given && revs->show_notes_by_default) {\n+\t\tenable_default_display_notes(&revs->notes_opt, &revs->show_notes);\n+\t\trevs->show_notes_given = 1;\n+\t}\n+\n \treturn left;\n }\n\ndiff --git a/revision.h b/revision.h\nindex 82ab400139d..50091bbd13f 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -253,6 +253,7 @@ struct rev_info {\n \t\t\tshown_dashes:1,\n \t\t\tshow_merge:1,\n \t\t\tshow_notes_given:1,\n+\t\t\tshow_notes_by_default:1,\n \t\t\tshow_signature:1,\n \t\t\tpretty_given:1,\n \t\t\tabbrev_commit:1,\n"},{"id":"481844","messageId":"xmqqpm2kk9sm.fsf@gitster.g","threadId":"59807","inReplyTo":"dd2958c5-58bf-86dd-b666-9033259a8e1a@gmx.de","subject":"Re: [PATCH v3 1/1] range-diff: treat notes like `log`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-14T16:18:49Z","receivedAt":"2023-09-14T16:18:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> A much more surgical way to address the issue at hand might be this\n> (Kristoffer, what do you think? It's missing documentation for the\n> newly-introduced `--show-notes-by-default` option, but you get the idea):\n\nClever.  I was wondering if the option parsing done by the revision\nmachinery was readily available for us to use from range-diff code,\nbut because we are forking out to \"log\" anyway, giving it the new\n\"--show-notes-by-default\" option does sound like a much easier way\nout.  It would help something like\n\n\t[alias] ln = !\"git log --show-notes-by-default\"\n\nas well.  Nicely designed.\n\n"},{"id":"481851","messageId":"c7d1e196-9521-45a7-b41c-80499f19f546@app.fastmail.com","threadId":"59807","inReplyTo":"dd2958c5-58bf-86dd-b666-9033259a8e1a@gmx.de","subject":"Re: [PATCH v3 1/1] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-14T20:25:37Z","receivedAt":"2023-09-14T20:26:03Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Thu, Sep 14, 2023, at 10:29, Johannes Schindelin wrote:\n>> [...]\n>\n> Right, `-G --notes` is a good argument to rethink this.\n>\n> A much more surgical way to address the issue at hand might be this\n> (Kristoffer, what do you think? It's missing documentation for the\n> newly-introduced `--show-notes-by-default` option, but you get the idea):\n\nLooks good to me. It seems like an explicit argument is the only way to\nmake this work.\n\n-- \nKristoffer Haugsbakk\n"},{"id":"481999","messageId":"xmqqled3hsip.fsf@gitster.g","threadId":"59807","inReplyTo":"c7d1e196-9521-45a7-b41c-80499f19f546@app.fastmail.com","subject":"Re: [PATCH v3 1/1] range-diff: treat notes like `log`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-19T01:16:14Z","receivedAt":"2023-09-19T01:16:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kristoffer Haugsbakk\" <code@khaugsbakk.name> writes:\n\n> On Thu, Sep 14, 2023, at 10:29, Johannes Schindelin wrote:\n>>> [...]\n>>\n>> Right, `-G --notes` is a good argument to rethink this.\n>>\n>> A much more surgical way to address the issue at hand might be this\n>> (Kristoffer, what do you think? It's missing documentation for the\n>> newly-introduced `--show-notes-by-default` option, but you get the idea):\n>\n> Looks good to me. It seems like an explicit argument is the only way to\n> make this work.\n\nWill one of you tie the loose ends?\n\nThanks.\n"},{"id":"482009","messageId":"69e8d116-05eb-4b16-aa5d-62da1ff818cc@app.fastmail.com","threadId":"59807","inReplyTo":"xmqqled3hsip.fsf@gitster.g","subject":"Re: [PATCH v3 1/1] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-19T09:12:16Z","receivedAt":"2023-09-19T09:12:43Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Tue, Sep 19, 2023, at 03:16, Junio C Hamano wrote:\n> Will one of you tie the loose ends?\n>\n> Thanks.\n\nI will this afternoon (CEST).\n\nCheers\n\n-- \nKristoffer Haugsbakk\n"},{"id":"482021","messageId":"cover.1695144790.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1694383247.git.code@khaugsbakk.name","subject":"[PATCH v4 0/1] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-19T18:05:31Z","receivedAt":"2023-09-19T18:06:15Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Hi\n\nSame cover letter up until but not including “Changes ...”.\n\nCheers\n\n🙛 🙙\n\nCurrently, `range-diff` shows the default notes if no notes-related\narguments are given. This is also how `log` behaves. But unlike\n`range-diff`, `log` does *not* show the default notes if\n`--notes=<custom>` are given.\n\nThese changes are supposed to make `format-range` behave like `log` with\nregards to notes.\n\nThese changes also fixes an issue with notes being shared in the cover\nletter via `range-diff`, and that’s really the main motivation for\nmaking these changes.\n\n§ How `log` works\n\n`log` shows the default notes if no notes arguments are given. But if\nyou give it specific notes to show then it forgets about the default\nnotes. Further, there is the convenience `--notes` option which will\nshow the default notes again. These options are cumulative. For example:\n\n    git log --notes --notes=custom\n\nWill show the default notes as well as the `custom` notes.\n\nSee discussion in: https://lore.kernel.org/git/20110329143357.GA10771@sigill.intra.peff.net/\n\n§ How `range-format` works\n\n`range-format` passes `--notes` to `log`, which means that it does not\nhave the default behavior of `log` (forget the default logs if you say\ne.g. `--notes=custom`). However, the man page says that (under\n`--[no-]notes[=<ref>]`):\n\n> This flag is passed to the git log program (see git-log(1)) that generates the patches.\n\nThis makes me (at least) think that `range-format` is supposed to work\njust like `log` with regards to notes.\n\n§ `format-patch` and the difference between showing and sharing\n\n`format-patch` has a different default: it shows no notes. This makes\nsense in my opinion since `format-patch` is meant to be used to share\nchanges with others, and you might be surprised if your notes (which\nmight have only been notes to yourself) are sent out in your emails\n(keep in mind that notes refs are *not* pushed by default).\n\nBut the slightly faulty behavior of `range-diff` bleeds through to\n`format-patch` since the latter calls the former; if you have default\nnotes they can be shared in the range-diff on the cover letter, even\nthough `format-patch` isn’t supposed to show them.\n\n§ Changes since version 3\n\nDscho [rewrote] the fix: introduce a new option to pass to `git log`.\n\n🔗 rewrote: https://lore.kernel.org/git/dd2958c5-58bf-86dd-b666-9033259a8e1a@gmx.de/\n\n§ CI (WIP)\n\nhttps://github.com/LemmingAvalanche/git/actions/runs/6238806624\n\nKristoffer Haugsbakk (1):\n  range-diff: treat notes like `log`\n\n Documentation/pretty-options.txt |  4 ++++\n range-diff.c                     |  2 +-\n revision.c                       |  7 +++++++\n revision.h                       |  1 +\n t/t3206-range-diff.sh            | 28 ++++++++++++++++++++++++++++\n 5 files changed, 41 insertions(+), 1 deletion(-)\n\nRange-diff against v3:\n1:  a37dfb3748 ! 1:  244e102cc4 range-diff: treat notes like `log`\n    @@ Commit message\n         to say about the changes to the default notes, since that will be shown\n         in the cover letter.\n\n    -    Remedy this by only conditionally passing in `--notes` to `range-diff`.\n    +    Remedy this by introducing `--show-notes-by-default` that `range-diff` can\n    +    use to tell the `log` subprocess what to do.\n\n         § Root cause\n\n    @@ Commit message\n         Co-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n         Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n\n    - ## range-diff.c ##\n    -@@ range-diff.c: static int read_patches(const char *range, struct string_list *list,\n    - \tstruct child_process cp = CHILD_PROCESS_INIT;\n    - \tstruct strbuf buf = STRBUF_INIT, contents = STRBUF_INIT;\n    - \tstruct patch_util *util = NULL;\n    --\tint in_header = 1;\n    -+\tint i, implicit_notes_arg = 1, in_header = 1;\n    - \tchar *line, *current_filename = NULL;\n    - \tssize_t len;\n    - \tsize_t size;\n    - \tint ret = -1;\n    + ## Documentation/pretty-options.txt ##\n    +@@ Documentation/pretty-options.txt: message by 4 spaces (i.e.  'medium', which is the default, 'full',\n    + and 'fuller').\n\n    -+\tfor (i = 0; other_arg && i < other_arg->nr; i++)\n    -+\t\tif (!strcmp(other_arg->v[i], \"--notes\") ||\n    -+\t\t    starts_with(other_arg->v[i], \"--notes=\") ||\n    -+\t\t    !strcmp(other_arg->v[i], \"--no-notes\")) {\n    -+\t\t\timplicit_notes_arg = 0;\n    -+\t\t\tbreak;\n    -+\t\t}\n    + ifndef::git-rev-list[]\n    ++--show-notes-by-default::\n    ++\tShow the default notes (see `--notes`) unless subsequent arguments\n    ++\tare used to display specific notes.\n     +\n    - \tstrvec_pushl(&cp.args, \"log\", \"--no-color\", \"-p\", \"--no-merges\",\n    - \t\t     \"--reverse\", \"--date-order\", \"--decorate=no\",\n    - \t\t     \"--no-prefix\", \"--submodule=short\",\n    + --notes[=<ref>]::\n    + \tShow the notes (see linkgit:git-notes[1]) that annotate the\n    + \tcommit, when showing the commit log message.  This is the default\n    +\n    + ## range-diff.c ##\n     @@ range-diff.c: static int read_patches(const char *range, struct string_list *list,\n      \t\t     \"--output-indicator-context=#\",\n      \t\t     \"--no-abbrev-commit\",\n      \t\t     \"--pretty=medium\",\n     -\t\t     \"--notes\",\n    ++\t\t     \"--show-notes-by-default\",\n      \t\t     NULL);\n    -+\tif (implicit_notes_arg)\n    -+\t\t     strvec_push(&cp.args, \"--notes\");\n      \tstrvec_push(&cp.args, range);\n      \tif (other_arg)\n    - \t\tstrvec_pushv(&cp.args, other_arg->v);\n    +\n    + ## revision.c ##\n    +@@ revision.c: static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n    + \t\trevs->break_bar = xstrdup(optarg);\n    + \t\trevs->track_linear = 1;\n    + \t\trevs->track_first_time = 1;\n    ++\t} else if (!strcmp(arg, \"--show-notes-by-default\")) {\n    ++\t\trevs->show_notes_by_default = 1;\n    + \t} else if (skip_prefix(arg, \"--show-notes=\", &optarg) ||\n    + \t\t   skip_prefix(arg, \"--notes=\", &optarg)) {\n    + \t\tif (starts_with(arg, \"--show-notes=\") &&\n    +@@ revision.c: int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n    + \tif (revs->expand_tabs_in_log < 0)\n    + \t\trevs->expand_tabs_in_log = revs->expand_tabs_in_log_default;\n    +\n    ++\tif (!revs->show_notes_given && revs->show_notes_by_default) {\n    ++\t\tenable_default_display_notes(&revs->notes_opt, &revs->show_notes);\n    ++\t\trevs->show_notes_given = 1;\n    ++\t}\n    ++\n    + \treturn left;\n    + }\n    +\n    +\n    + ## revision.h ##\n    +@@ revision.h: struct rev_info {\n    + \t\t\tshown_dashes:1,\n    + \t\t\tshow_merge:1,\n    + \t\t\tshow_notes_given:1,\n    ++\t\t\tshow_notes_by_default:1,\n    + \t\t\tshow_signature:1,\n    + \t\t\tpretty_given:1,\n    + \t\t\tabbrev_commit:1,\n\n      ## t/t3206-range-diff.sh ##\n     @@ t/t3206-range-diff.sh: test_expect_success 'range-diff with multiple --notes' '\n--\n2.42.0\n"},{"id":"482022","messageId":"244e102cc4693bb6291e03cffea6df05cdb29df3.1695144790.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1695144790.git.code@khaugsbakk.name","subject":"[PATCH v4 1/1] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-19T18:05:32Z","receivedAt":"2023-09-19T18:06:17Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Currently, `range-diff` shows the default notes if no notes-related\narguments are given. This is also how `log` behaves. But unlike\n`range-diff`, `log` does *not* show the default notes if\n`--notes=<custom>` are given. In other words, this:\n\n    git log --notes=custom\n\nis equivalent to this:\n\n    git log --no-notes --notes=custom\n\nWhile:\n\n    git range-diff --notes=custom\n\nacts like this:\n\n    git log --notes --notes-custom\n\nThis can’t be how the user expects `range-diff` to behave given that the\nman page for `range-diff` under `--[no-]notes[=<ref>]` says:\n\n> This flag is passed to the `git log` program (see git-log(1)) that\n> generates the patches.\n\nThis behavior also affects `format-patch` since it uses `range-diff` for\nthe cover letter. Unlike `log`, though, `format-patch` is not supposed\nto show the default notes if no notes-related arguments are given.[1]\nBut this promise is broken when the range-diff happens to have something\nto say about the changes to the default notes, since that will be shown\nin the cover letter.\n\nRemedy this by introducing `--show-notes-by-default` that `range-diff` can\nuse to tell the `log` subprocess what to do.\n\n§ Root cause\n\n8cf51561d1e (range-diff: fix a crash in parsing git-log output,\n2020-04-15) added `--notes` in order to deal with a side-effect of\n`--pretty=medium`:\n\n> To fix this explicitly set the output format of the internally executed\n> `git log` with `--pretty=medium`. Because that cancels `--notes`, add\n> explicitly `--notes` at the end.\n\n§ Authors\n\n• Fix by Johannes\n• Tests by Kristoffer\n\n† 1: See e.g. 66b2ed09c2 (Fix \"log\" family not to be too agressive about\n    showing notes, 2010-01-20).\n\nCo-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nSigned-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n Documentation/pretty-options.txt |  4 ++++\n range-diff.c                     |  2 +-\n revision.c                       |  7 +++++++\n revision.h                       |  1 +\n t/t3206-range-diff.sh            | 28 ++++++++++++++++++++++++++++\n 5 files changed, 41 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\nindex dc685be363..dcd501ee50 100644\n--- a/Documentation/pretty-options.txt\n+++ b/Documentation/pretty-options.txt\n@@ -59,6 +59,10 @@ message by 4 spaces (i.e.  'medium', which is the default, 'full',\n and 'fuller').\n \n ifndef::git-rev-list[]\n+--show-notes-by-default::\n+\tShow the default notes (see `--notes`) unless subsequent arguments\n+\tare used to display specific notes.\n+\n --notes[=<ref>]::\n \tShow the notes (see linkgit:git-notes[1]) that annotate the\n \tcommit, when showing the commit log message.  This is the default\ndiff --git a/range-diff.c b/range-diff.c\nindex 2e86063491..56f6870ff9 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -60,7 +60,7 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t     \"--output-indicator-context=#\",\n \t\t     \"--no-abbrev-commit\",\n \t\t     \"--pretty=medium\",\n-\t\t     \"--notes\",\n+\t\t     \"--show-notes-by-default\",\n \t\t     NULL);\n \tstrvec_push(&cp.args, range);\n \tif (other_arg)\ndiff --git a/revision.c b/revision.c\nindex 2f4c53ea20..49d385257a 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2484,6 +2484,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->break_bar = xstrdup(optarg);\n \t\trevs->track_linear = 1;\n \t\trevs->track_first_time = 1;\n+\t} else if (!strcmp(arg, \"--show-notes-by-default\")) {\n+\t\trevs->show_notes_by_default = 1;\n \t} else if (skip_prefix(arg, \"--show-notes=\", &optarg) ||\n \t\t   skip_prefix(arg, \"--notes=\", &optarg)) {\n \t\tif (starts_with(arg, \"--show-notes=\") &&\n@@ -3054,6 +3056,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \tif (revs->expand_tabs_in_log < 0)\n \t\trevs->expand_tabs_in_log = revs->expand_tabs_in_log_default;\n \n+\tif (!revs->show_notes_given && revs->show_notes_by_default) {\n+\t\tenable_default_display_notes(&revs->notes_opt, &revs->show_notes);\n+\t\trevs->show_notes_given = 1;\n+\t}\n+\n \treturn left;\n }\n \ndiff --git a/revision.h b/revision.h\nindex 82ab400139..50091bbd13 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -253,6 +253,7 @@ struct rev_info {\n \t\t\tshown_dashes:1,\n \t\t\tshow_merge:1,\n \t\t\tshow_notes_given:1,\n+\t\t\tshow_notes_by_default:1,\n \t\t\tshow_signature:1,\n \t\t\tpretty_given:1,\n \t\t\tabbrev_commit:1,\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex b5f4d6a653..b33afa1c6a 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -662,6 +662,20 @@ test_expect_success 'range-diff with multiple --notes' '\n \ttest_cmp expect actual\n '\n \n+# `range-diff` should act like `log` with regards to notes\n+test_expect_success 'range-diff with --notes=custom does not show default notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\tgit notes --ref=custom add -m \"topic note\" topic &&\n+\tgit notes --ref=custom add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\ttest_when_finished git notes --ref=custom remove topic unmodified &&\n+\tgit range-diff --notes=custom main..topic main..unmodified \\\n+\t\t>actual &&\n+\t! grep \"## Notes ##\" actual &&\n+\tgrep \"## Notes (custom) ##\" actual\n+'\n+\n test_expect_success 'format-patch --range-diff does not compare notes by default' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n@@ -679,6 +693,20 @@ test_expect_success 'format-patch --range-diff does not compare notes by default\n \t! grep \"note\" 0000-*\n '\n \n+test_expect_success 'format-patch --notes=custom --range-diff only compares custom notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes --ref=custom add -m \"topic note (custom)\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\tgit notes --ref=custom add -m \"unmodified note (custom)\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\ttest_when_finished git notes --ref=custom remove topic unmodified &&\n+\tgit format-patch --notes=custom --cover-letter --range-diff=$prev \\\n+\t\tmain..unmodified >actual &&\n+\ttest_when_finished \"rm 000?-*\" &&\n+\tgrep \"## Notes (custom) ##\" 0000-* &&\n+\t! grep \"## Notes ##\" 0000-*\n+'\n+\n test_expect_success 'format-patch --range-diff with --no-notes' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n-- \n2.42.0\n\n"},{"id":"482027","messageId":"xmqqled2ezfh.fsf@gitster.g","threadId":"59807","inReplyTo":"244e102cc4693bb6291e03cffea6df05cdb29df3.1695144790.git.code@khaugsbakk.name","subject":"Re: [PATCH v4 1/1] range-diff: treat notes like `log`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-19T19:27:30Z","receivedAt":"2023-09-19T19:27:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristoffer Haugsbakk <code@khaugsbakk.name> writes:\n\n> Currently, `range-diff` shows the default notes if no notes-related\n> arguments are given. This is also how `log` behaves. But unlike\n> `range-diff`, `log` does *not* show the default notes if\n> `--notes=<custom>` are given. In other words, this:\n>\n>     git log --notes=custom\n>\n> is equivalent to this:\n>\n>     git log --no-notes --notes=custom\n>\n> While:\n>\n>     git range-diff --notes=custom\n>\n> acts like this:\n>\n>     git log --notes --notes-custom\n>\n> This can’t be how the user expects `range-diff` to behave given that the\n> man page for `range-diff` under `--[no-]notes[=<ref>]` says:\n>\n>> This flag is passed to the `git log` program (see git-log(1)) that\n>> generates the patches.\n>\n> This behavior also affects `format-patch` since it uses `range-diff` for\n> the cover letter. Unlike `log`, though, `format-patch` is not supposed\n> to show the default notes if no notes-related arguments are given.[1]\n> But this promise is broken when the range-diff happens to have something\n> to say about the changes to the default notes, since that will be shown\n> in the cover letter.\n>\n> Remedy this by introducing `--show-notes-by-default` that `range-diff` can\n> use to tell the `log` subprocess what to do.\n\nVery well described.  I think the rest of the proposed log message\nis redundant now we have quite a good write-up above.\n\n>  ifndef::git-rev-list[]\n> +--show-notes-by-default::\n> +\tShow the default notes (see `--notes`) unless subsequent arguments\n> +\tare used to display specific notes.\n> +\n>  --notes[=<ref>]::\n>  \tShow the notes (see linkgit:git-notes[1]) that annotate the\n>  \tcommit, when showing the commit log message.  This is the default\n\nI think the new entry should come after the description of `--notes`,\nwhich is the primary option around the \"notes\" feature.\n\nIn the description, I think \"subsequent\" is misphrased.  It makes it\nsound as if\n\n    $ git log --show-notes-by-default --notes=amlog\n\nwould stop showing the notes from the default notes tree (because\nthe notes from the .git/refs/notes/amlog is explicitly asked for),\nwhile\n\n    $ git log --notes=amlog --show-notes-by-default\n\nwould show both the default and the custom notes, which is not what\nthe code does, I think, in this hunk:\n\n> @@ -3054,6 +3056,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n>  \tif (revs->expand_tabs_in_log < 0)\n>  \t\trevs->expand_tabs_in_log = revs->expand_tabs_in_log_default;\n>  \n> +\tif (!revs->show_notes_given && revs->show_notes_by_default) {\n> +\t\tenable_default_display_notes(&revs->notes_opt, &revs->show_notes);\n> +\t\trevs->show_notes_given = 1;\n> +\t}\n> +\n>  \treturn left;\n>  }\n\nOther than the above minor nits, looks very good.\n\nThanks.\n\n"},{"id":"482028","messageId":"9f9610fa-f1cc-404a-9496-d8b77ca05d5b@app.fastmail.com","threadId":"59807","inReplyTo":"244e102cc4693bb6291e03cffea6df05cdb29df3.1695144790.git.code@khaugsbakk.name","subject":"Re: [PATCH v4 1/1] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-19T19:27:57Z","receivedAt":"2023-09-19T19:29:43Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"The original CI failed.[1] Then I rebased on `master` (d4a83d07b8c (The\ntenth batch, 2023-09-18)) which also failed.[2] Problem in t5559.\n\nSo: fair warning. :)\n\n[1] https://github.com/LemmingAvalanche/git/actions/runs/6238806624\n[2] https://github.com/LemmingAvalanche/git/actions/runs/6239585493\n\n-- \nKristoffer Haugsbakk\n"},{"id":"482029","messageId":"xmqqfs3aeyo4.fsf@gitster.g","threadId":"59807","inReplyTo":"9f9610fa-f1cc-404a-9496-d8b77ca05d5b@app.fastmail.com","subject":"Re: [PATCH v4 1/1] range-diff: treat notes like `log`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-19T19:43:55Z","receivedAt":"2023-09-19T19:44:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kristoffer Haugsbakk\" <code@khaugsbakk.name> writes:\n\n> The original CI failed.[1] Then I rebased on `master` (d4a83d07b8c (The\n> tenth batch, 2023-09-18)) which also failed.[2] Problem in t5559.\n>\n> So: fair warning. :)\n>\n> [1] https://github.com/LemmingAvalanche/git/actions/runs/6238806624\n> [2] https://github.com/LemmingAvalanche/git/actions/runs/6239585493\n\nI think you can ignore these macOS ones that spew\n\n    == Info: [HTTP/2] [1] ...\n\nin the log.  The adjustments necessary for the redaction code to\ndeal with these new messages from cURL are known and are cooking.\n"},{"id":"482030","messageId":"6ad470bd-207f-4735-9ab5-2da5010d9ef4@app.fastmail.com","threadId":"59807","inReplyTo":"xmqqled2ezfh.fsf@gitster.g","subject":"Re: [PATCH v4 1/1] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-19T19:44:20Z","receivedAt":"2023-09-19T19:45:51Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Tue, Sep 19, 2023, at 21:27, Junio C Hamano wrote:\n> Kristoffer Haugsbakk <code@khaugsbakk.name> writes:\n>\n>> [snip]\n>\n> Very well described.  I think the rest of the proposed log message\n> is redundant now we have quite a good write-up above.\n\nThanks!\n\n>\n>>  ifndef::git-rev-list[]\n>> +--show-notes-by-default::\n>> +\tShow the default notes (see `--notes`) unless subsequent arguments\n>> +\tare used to display specific notes.\n>> +\n>>  --notes[=<ref>]::\n>>  \tShow the notes (see linkgit:git-notes[1]) that annotate the\n>>  \tcommit, when showing the commit log message.  This is the default\n>\n> I think the new entry should come after the description of `--notes`,\n> which is the primary option around the \"notes\" feature.\n>\n> In the description, I think \"subsequent\" is misphrased.  It makes it\n> sound as if\n>\n>     $ git log --show-notes-by-default --notes=amlog\n>\n> would stop showing the notes from the default notes tree (because\n> the notes from the .git/refs/notes/amlog is explicitly asked for),\n> while\n>\n>     $ git log --notes=amlog --show-notes-by-default\n>\n> would show both the default and the custom notes, which is not what\n> the code does, I think, in this hunk:\n>\n>> @@ -3054,6 +3056,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n>>  \tif (revs->expand_tabs_in_log < 0)\n>>  \t\trevs->expand_tabs_in_log = revs->expand_tabs_in_log_default;\n>>\n>> +\tif (!revs->show_notes_given && revs->show_notes_by_default) {\n>> +\t\tenable_default_display_notes(&revs->notes_opt, &revs->show_notes);\n>> +\t\trevs->show_notes_given = 1;\n>> +\t}\n>> +\n>>  \treturn left;\n>>  }\n>\n> Other than the above minor nits, looks very good.\n>\n> Thanks.\n\nOkay I think I understand. With that in mind I would change it to the\npatch below.\n\nI can make a new version if that looks okay.\n\n-- >8 --\ndiff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\nindex dcd501ee505..d2df1aad647 100644\n--- a/Documentation/pretty-options.txt\n+++ b/Documentation/pretty-options.txt\n@@ -59,10 +59,6 @@ message by 4 spaces (i.e.  'medium', which is the default, 'full',\n and 'fuller').\n \n ifndef::git-rev-list[]\n---show-notes-by-default::\n-\tShow the default notes (see `--notes`) unless subsequent arguments\n-\tare used to display specific notes.\n-\n --notes[=<ref>]::\n \tShow the notes (see linkgit:git-notes[1]) that annotate the\n \tcommit, when showing the commit log message.  This is the default\n@@ -91,6 +87,10 @@ being displayed. Examples: \"--notes=foo\" will show only notes from\n \t\"--notes --notes=foo --no-notes --notes=bar\" will only show notes\n \tfrom \"refs/notes/bar\".\n \n+--show-notes-by-default::\n+\tShow the default notes unless arguments are given for displaying\n+\tspecific notes.\n+\n --show-notes[=<ref>]::\n --[no-]standard-notes::\n \tThese options are deprecated. Use the above --notes/--no-notes\n"},{"id":"482031","messageId":"xmqqbkdyeybf.fsf@gitster.g","threadId":"59807","inReplyTo":"6ad470bd-207f-4735-9ab5-2da5010d9ef4@app.fastmail.com","subject":"Re: [PATCH v4 1/1] range-diff: treat notes like `log`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-19T19:51:32Z","receivedAt":"2023-09-19T19:51:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kristoffer Haugsbakk\" <code@khaugsbakk.name> writes:\n\n> Okay I think I understand. With that in mind I would change it to the\n> patch below.\n>\n> I can make a new version if that looks okay.\n\nLooking good, although I would say \"unless other options are given\"\n(\"other\" is optional) instead of \"arguments\" if I were writing this.\n\nThanks.\n"},{"id":"482035","messageId":"cover.1695154855.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1695144790.git.code@khaugsbakk.name","subject":"[PATCH v5 0/1] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-19T20:26:49Z","receivedAt":"2023-09-19T20:27:28Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Hi\n\nSee the previous cover letter for context.\n\n§ Changes since version 4\n\n• Remove “Root cause” section from commit message\n• Doc change according to feedback\n\nKristoffer Haugsbakk (1):\n  range-diff: treat notes like `log`\n\n Documentation/pretty-options.txt |  4 ++++\n range-diff.c                     |  2 +-\n revision.c                       |  7 +++++++\n revision.h                       |  1 +\n t/t3206-range-diff.sh            | 28 ++++++++++++++++++++++++++++\n 5 files changed, 41 insertions(+), 1 deletion(-)\n\nRange-diff against v4:\n1:  244e102cc46 ! 1:  6e114271a2e range-diff: treat notes like `log`\n    @@ Commit message\n         Remedy this by introducing `--show-notes-by-default` that `range-diff` can\n         use to tell the `log` subprocess what to do.\n\n    -    § Root cause\n    -\n    -    8cf51561d1e (range-diff: fix a crash in parsing git-log output,\n    -    2020-04-15) added `--notes` in order to deal with a side-effect of\n    -    `--pretty=medium`:\n    -\n    -    > To fix this explicitly set the output format of the internally executed\n    -    > `git log` with `--pretty=medium`. Because that cancels `--notes`, add\n    -    > explicitly `--notes` at the end.\n    -\n         § Authors\n\n         • Fix by Johannes\n    @@ Commit message\n         Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n\n      ## Documentation/pretty-options.txt ##\n    -@@ Documentation/pretty-options.txt: message by 4 spaces (i.e.  'medium', which is the default, 'full',\n    - and 'fuller').\n    +@@ Documentation/pretty-options.txt: being displayed. Examples: \"--notes=foo\" will show only notes from\n    + \t\"--notes --notes=foo --no-notes --notes=bar\" will only show notes\n    + \tfrom \"refs/notes/bar\".\n\n    - ifndef::git-rev-list[]\n     +--show-notes-by-default::\n    -+\tShow the default notes (see `--notes`) unless subsequent arguments\n    -+\tare used to display specific notes.\n    ++\tShow the default notes unless options for displaying specific\n    ++\tnotes are given.\n     +\n    - --notes[=<ref>]::\n    - \tShow the notes (see linkgit:git-notes[1]) that annotate the\n    - \tcommit, when showing the commit log message.  This is the default\n    + --show-notes[=<ref>]::\n    + --[no-]standard-notes::\n    + \tThese options are deprecated. Use the above --notes/--no-notes\n\n      ## range-diff.c ##\n     @@ range-diff.c: static int read_patches(const char *range, struct string_list *list,\n--\n2.42.0\n"},{"id":"482036","messageId":"6e114271a2e7d2323193bd58bb307f60101942ce.1695154855.git.code@khaugsbakk.name","threadId":"59807","inReplyTo":"cover.1695154855.git.code@khaugsbakk.name","subject":"[PATCH v5 1/1] range-diff: treat notes like `log`","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-09-19T20:26:50Z","receivedAt":"2023-09-19T20:27:30Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Currently, `range-diff` shows the default notes if no notes-related\narguments are given. This is also how `log` behaves. But unlike\n`range-diff`, `log` does *not* show the default notes if\n`--notes=<custom>` are given. In other words, this:\n\n    git log --notes=custom\n\nis equivalent to this:\n\n    git log --no-notes --notes=custom\n\nWhile:\n\n    git range-diff --notes=custom\n\nacts like this:\n\n    git log --notes --notes-custom\n\nThis can’t be how the user expects `range-diff` to behave given that the\nman page for `range-diff` under `--[no-]notes[=<ref>]` says:\n\n> This flag is passed to the `git log` program (see git-log(1)) that\n> generates the patches.\n\nThis behavior also affects `format-patch` since it uses `range-diff` for\nthe cover letter. Unlike `log`, though, `format-patch` is not supposed\nto show the default notes if no notes-related arguments are given.[1]\nBut this promise is broken when the range-diff happens to have something\nto say about the changes to the default notes, since that will be shown\nin the cover letter.\n\nRemedy this by introducing `--show-notes-by-default` that `range-diff` can\nuse to tell the `log` subprocess what to do.\n\n§ Authors\n\n• Fix by Johannes\n• Tests by Kristoffer\n\n† 1: See e.g. 66b2ed09c2 (Fix \"log\" family not to be too agressive about\n    showing notes, 2010-01-20).\n\nCo-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nSigned-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n Documentation/pretty-options.txt |  4 ++++\n range-diff.c                     |  2 +-\n revision.c                       |  7 +++++++\n revision.h                       |  1 +\n t/t3206-range-diff.sh            | 28 ++++++++++++++++++++++++++++\n 5 files changed, 41 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\nindex dc685be363a..335395b727f 100644\n--- a/Documentation/pretty-options.txt\n+++ b/Documentation/pretty-options.txt\n@@ -87,6 +87,10 @@ being displayed. Examples: \"--notes=foo\" will show only notes from\n \t\"--notes --notes=foo --no-notes --notes=bar\" will only show notes\n \tfrom \"refs/notes/bar\".\n \n+--show-notes-by-default::\n+\tShow the default notes unless options for displaying specific\n+\tnotes are given.\n+\n --show-notes[=<ref>]::\n --[no-]standard-notes::\n \tThese options are deprecated. Use the above --notes/--no-notes\ndiff --git a/range-diff.c b/range-diff.c\nindex ca5493984a5..c45b6d849cb 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -60,7 +60,7 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t     \"--output-indicator-context=#\",\n \t\t     \"--no-abbrev-commit\",\n \t\t     \"--pretty=medium\",\n-\t\t     \"--notes\",\n+\t\t     \"--show-notes-by-default\",\n \t\t     NULL);\n \tstrvec_push(&cp.args, range);\n \tif (other_arg)\ndiff --git a/revision.c b/revision.c\nindex 2f4c53ea207..49d385257ac 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2484,6 +2484,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->break_bar = xstrdup(optarg);\n \t\trevs->track_linear = 1;\n \t\trevs->track_first_time = 1;\n+\t} else if (!strcmp(arg, \"--show-notes-by-default\")) {\n+\t\trevs->show_notes_by_default = 1;\n \t} else if (skip_prefix(arg, \"--show-notes=\", &optarg) ||\n \t\t   skip_prefix(arg, \"--notes=\", &optarg)) {\n \t\tif (starts_with(arg, \"--show-notes=\") &&\n@@ -3054,6 +3056,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \tif (revs->expand_tabs_in_log < 0)\n \t\trevs->expand_tabs_in_log = revs->expand_tabs_in_log_default;\n \n+\tif (!revs->show_notes_given && revs->show_notes_by_default) {\n+\t\tenable_default_display_notes(&revs->notes_opt, &revs->show_notes);\n+\t\trevs->show_notes_given = 1;\n+\t}\n+\n \treturn left;\n }\n \ndiff --git a/revision.h b/revision.h\nindex 82ab400139d..50091bbd13f 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -253,6 +253,7 @@ struct rev_info {\n \t\t\tshown_dashes:1,\n \t\t\tshow_merge:1,\n \t\t\tshow_notes_given:1,\n+\t\t\tshow_notes_by_default:1,\n \t\t\tshow_signature:1,\n \t\t\tpretty_given:1,\n \t\t\tabbrev_commit:1,\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex b5f4d6a6530..b33afa1c6aa 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -662,6 +662,20 @@ test_expect_success 'range-diff with multiple --notes' '\n \ttest_cmp expect actual\n '\n \n+# `range-diff` should act like `log` with regards to notes\n+test_expect_success 'range-diff with --notes=custom does not show default notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\tgit notes --ref=custom add -m \"topic note\" topic &&\n+\tgit notes --ref=custom add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\ttest_when_finished git notes --ref=custom remove topic unmodified &&\n+\tgit range-diff --notes=custom main..topic main..unmodified \\\n+\t\t>actual &&\n+\t! grep \"## Notes ##\" actual &&\n+\tgrep \"## Notes (custom) ##\" actual\n+'\n+\n test_expect_success 'format-patch --range-diff does not compare notes by default' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n@@ -679,6 +693,20 @@ test_expect_success 'format-patch --range-diff does not compare notes by default\n \t! grep \"note\" 0000-*\n '\n \n+test_expect_success 'format-patch --notes=custom --range-diff only compares custom notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes --ref=custom add -m \"topic note (custom)\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\tgit notes --ref=custom add -m \"unmodified note (custom)\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\ttest_when_finished git notes --ref=custom remove topic unmodified &&\n+\tgit format-patch --notes=custom --cover-letter --range-diff=$prev \\\n+\t\tmain..unmodified >actual &&\n+\ttest_when_finished \"rm 000?-*\" &&\n+\tgrep \"## Notes (custom) ##\" 0000-* &&\n+\t! grep \"## Notes ##\" 0000-*\n+'\n+\n test_expect_success 'format-patch --range-diff with --no-notes' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n-- \n2.42.0\n\n"},{"id":"482099","messageId":"d9c73723-7489-0f4f-c899-eaa14dc4280c@gmx.de","threadId":"59807","inReplyTo":"6e114271a2e7d2323193bd58bb307f60101942ce.1695154855.git.code@khaugsbakk.name","subject":"Re: [PATCH v5 1/1] range-diff: treat notes like `log`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2023-09-21T12:30:35Z","receivedAt":"2023-09-21T19:39:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Kristoffer,\n\nthis iteration looks good to me!\n\nThank you so much,\nJohannes\n\nOn Tue, 19 Sep 2023, Kristoffer Haugsbakk wrote:\n\n> Currently, `range-diff` shows the default notes if no notes-related\n> arguments are given. This is also how `log` behaves. But unlike\n> `range-diff`, `log` does *not* show the default notes if\n> `--notes=<custom>` are given. In other words, this:\n>\n>     git log --notes=custom\n>\n> is equivalent to this:\n>\n>     git log --no-notes --notes=custom\n>\n> While:\n>\n>     git range-diff --notes=custom\n>\n> acts like this:\n>\n>     git log --notes --notes-custom\n>\n> This can’t be how the user expects `range-diff` to behave given that the\n> man page for `range-diff` under `--[no-]notes[=<ref>]` says:\n>\n> > This flag is passed to the `git log` program (see git-log(1)) that\n> > generates the patches.\n>\n> This behavior also affects `format-patch` since it uses `range-diff` for\n> the cover letter. Unlike `log`, though, `format-patch` is not supposed\n> to show the default notes if no notes-related arguments are given.[1]\n> But this promise is broken when the range-diff happens to have something\n> to say about the changes to the default notes, since that will be shown\n> in the cover letter.\n>\n> Remedy this by introducing `--show-notes-by-default` that `range-diff` can\n> use to tell the `log` subprocess what to do.\n>\n> § Authors\n>\n> • Fix by Johannes\n> • Tests by Kristoffer\n>\n> † 1: See e.g. 66b2ed09c2 (Fix \"log\" family not to be too agressive about\n>     showing notes, 2010-01-20).\n>\n> Co-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n> ---\n>  Documentation/pretty-options.txt |  4 ++++\n>  range-diff.c                     |  2 +-\n>  revision.c                       |  7 +++++++\n>  revision.h                       |  1 +\n>  t/t3206-range-diff.sh            | 28 ++++++++++++++++++++++++++++\n>  5 files changed, 41 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\n> index dc685be363a..335395b727f 100644\n> --- a/Documentation/pretty-options.txt\n> +++ b/Documentation/pretty-options.txt\n> @@ -87,6 +87,10 @@ being displayed. Examples: \"--notes=foo\" will show only notes from\n>  \t\"--notes --notes=foo --no-notes --notes=bar\" will only show notes\n>  \tfrom \"refs/notes/bar\".\n>\n> +--show-notes-by-default::\n> +\tShow the default notes unless options for displaying specific\n> +\tnotes are given.\n> +\n>  --show-notes[=<ref>]::\n>  --[no-]standard-notes::\n>  \tThese options are deprecated. Use the above --notes/--no-notes\n> diff --git a/range-diff.c b/range-diff.c\n> index ca5493984a5..c45b6d849cb 100644\n> --- a/range-diff.c\n> +++ b/range-diff.c\n> @@ -60,7 +60,7 @@ static int read_patches(const char *range, struct string_list *list,\n>  \t\t     \"--output-indicator-context=#\",\n>  \t\t     \"--no-abbrev-commit\",\n>  \t\t     \"--pretty=medium\",\n> -\t\t     \"--notes\",\n> +\t\t     \"--show-notes-by-default\",\n>  \t\t     NULL);\n>  \tstrvec_push(&cp.args, range);\n>  \tif (other_arg)\n> diff --git a/revision.c b/revision.c\n> index 2f4c53ea207..49d385257ac 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2484,6 +2484,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n>  \t\trevs->break_bar = xstrdup(optarg);\n>  \t\trevs->track_linear = 1;\n>  \t\trevs->track_first_time = 1;\n> +\t} else if (!strcmp(arg, \"--show-notes-by-default\")) {\n> +\t\trevs->show_notes_by_default = 1;\n>  \t} else if (skip_prefix(arg, \"--show-notes=\", &optarg) ||\n>  \t\t   skip_prefix(arg, \"--notes=\", &optarg)) {\n>  \t\tif (starts_with(arg, \"--show-notes=\") &&\n> @@ -3054,6 +3056,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n>  \tif (revs->expand_tabs_in_log < 0)\n>  \t\trevs->expand_tabs_in_log = revs->expand_tabs_in_log_default;\n>\n> +\tif (!revs->show_notes_given && revs->show_notes_by_default) {\n> +\t\tenable_default_display_notes(&revs->notes_opt, &revs->show_notes);\n> +\t\trevs->show_notes_given = 1;\n> +\t}\n> +\n>  \treturn left;\n>  }\n>\n> diff --git a/revision.h b/revision.h\n> index 82ab400139d..50091bbd13f 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -253,6 +253,7 @@ struct rev_info {\n>  \t\t\tshown_dashes:1,\n>  \t\t\tshow_merge:1,\n>  \t\t\tshow_notes_given:1,\n> +\t\t\tshow_notes_by_default:1,\n>  \t\t\tshow_signature:1,\n>  \t\t\tpretty_given:1,\n>  \t\t\tabbrev_commit:1,\n> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\n> index b5f4d6a6530..b33afa1c6aa 100755\n> --- a/t/t3206-range-diff.sh\n> +++ b/t/t3206-range-diff.sh\n> @@ -662,6 +662,20 @@ test_expect_success 'range-diff with multiple --notes' '\n>  \ttest_cmp expect actual\n>  '\n>\n> +# `range-diff` should act like `log` with regards to notes\n> +test_expect_success 'range-diff with --notes=custom does not show default notes' '\n> +\tgit notes add -m \"topic note\" topic &&\n> +\tgit notes add -m \"unmodified note\" unmodified &&\n> +\tgit notes --ref=custom add -m \"topic note\" topic &&\n> +\tgit notes --ref=custom add -m \"unmodified note\" unmodified &&\n> +\ttest_when_finished git notes remove topic unmodified &&\n> +\ttest_when_finished git notes --ref=custom remove topic unmodified &&\n> +\tgit range-diff --notes=custom main..topic main..unmodified \\\n> +\t\t>actual &&\n> +\t! grep \"## Notes ##\" actual &&\n> +\tgrep \"## Notes (custom) ##\" actual\n> +'\n> +\n>  test_expect_success 'format-patch --range-diff does not compare notes by default' '\n>  \tgit notes add -m \"topic note\" topic &&\n>  \tgit notes add -m \"unmodified note\" unmodified &&\n> @@ -679,6 +693,20 @@ test_expect_success 'format-patch --range-diff does not compare notes by default\n>  \t! grep \"note\" 0000-*\n>  '\n>\n> +test_expect_success 'format-patch --notes=custom --range-diff only compares custom notes' '\n> +\tgit notes add -m \"topic note\" topic &&\n> +\tgit notes --ref=custom add -m \"topic note (custom)\" topic &&\n> +\tgit notes add -m \"unmodified note\" unmodified &&\n> +\tgit notes --ref=custom add -m \"unmodified note (custom)\" unmodified &&\n> +\ttest_when_finished git notes remove topic unmodified &&\n> +\ttest_when_finished git notes --ref=custom remove topic unmodified &&\n> +\tgit format-patch --notes=custom --cover-letter --range-diff=$prev \\\n> +\t\tmain..unmodified >actual &&\n> +\ttest_when_finished \"rm 000?-*\" &&\n> +\tgrep \"## Notes (custom) ##\" 0000-* &&\n> +\t! grep \"## Notes ##\" 0000-*\n> +'\n> +\n>  test_expect_success 'format-patch --range-diff with --no-notes' '\n>  \tgit notes add -m \"topic note\" topic &&\n>  \tgit notes add -m \"unmodified note\" unmodified &&\n> --\n> 2.42.0\n>\n>\n"}]}