[PATCH v4 2/2] format-patch: learn --[no-]range-diff-notes
- From
- kristofferhaugsbakk@fastmail.com <kristofferhaugsbakk@fastmail.com>
- Date
- Oct 4, 2026, 10:17 UTC
- Message-ID
- <V4_format-patch_learn_--range-diff-notes.d5e@m5gid.xyz>
- In-Reply-To
- <V4_CV_format-patch_learn_--range-diff-notes.d5c@m5gid.xyz>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
git-format-patch(1) passes on the notes behavior that it is using for the patches to git-range-diff(1). In turn you get the same Git notes displayed in the range diff as the ones you used to generate the patches. And that makes sense in most cases.
However, I often make notes between series versions that mostly prepend to the original. They end up looking like this:
v3:
[desc.]
v2:
[descr.]
v1:
[descr.]These notes are meant for the git-format-patch(1) output since they document the iterations. But including them also includes them in the range diff. And they have nothing useful to say there.
Let’s teach git-format-patch(1) `--[no-]range-diff-notes` so that we can pass in different notes refs to the range diff, or just turn them off entirely.
In addition to storing the list of notes, we also need a boolean `override` to distinguish these two cases:
1. No such options were given and empty list (use `--notes`) 2. Options were given and empty list (`--no-...` given; don’t use notes)
***
Note that using `--creation-factor` without `--range-diff` will cause the command to die. But this is not the case for `--[no-]range-diff- notes`; we would have to check `rdiff_notes.override`, which is a sticky value (cannot be turned off). The reason is that it is potentially inconvenient to error out since it would not let you turn off `--range-diff` in, say, some alias that uses `--no-range-diff- notes`. Granted, it is difficult for me to come up with a concrete use case since `--range-diff` requires a value, specifically a value which is probably not that reusable (revision range), and yet you have something like an alias set up with it. But why spend code closing that door? There is no usability upside to erroring out.
***
Add two tests here for the single-patch case, i.e. the case where the range diff is on the patch and not in the cover letter. These are meant as regression tests based on my encounter with single-patch range diff notes handling bug.[1]
† 1: 155986b4 (format-patch: handle range-diff on notes correctly for
single patches, 2025-09-25)Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> ---
Notes (series):
v4:
• Msg: Trim all the expository fat, which only loses the footnote
about “what if you had a changelog and testing notes” (in terms
of “real substance”) as a trade for getting to the point quite
quickly (relatively speaking)[1]
🔗 1: https://lore.kernel.org/git/30249b7b-b6f7-4065-9a83-db93d69ad0f1@app.fastmail.com/#t
• An obvious refactor: call `parse_opt_string_list` instead of
manually inlining it along with a comment saying “we inlined
it”[1]
• Trim the fat from the doc. Straightforward explanation: use this to get
`<ref>` instead. Use multiple times for more refs. `--no-...` to
turn off. Lifted from the proposal by Junio with some
modifications (use `<ref>` to more tersely discuss “a different
notes ref”)[1]
• Msg: credit help
• `clang-format` on `rdiff_notes_cb`
🔗 1: https://lore.kernel.org/git/xmqqy0cgvwpi.fsf@gitster.g/
---
v3:
• Remove repeated and redundant `test_when_finished` on
patch files[1]
🔗 1: https://lore.kernel.org/git/CV_format-patch_learn_--range-diff-notes.c57@msgid.xyz/T/#m06803e233a2e385e694432d45ecf402f7a67e482
---
v2:
This version drops the whole functionality around being able to *go
back* (and forth) to using `--notes` for the range diff.[1] The
behavior was too complex to explain and motivate compared to the
utility (little).
🔗 1: https://lore.kernel.org/git/8f0a076b-4822-44e2-a842-cc1e39ae1c1d@app.fastmail.com/#t
Also:
• Use a parse-options callback for the option instead of
`revision.c:handle_revision_opt`
• Msg: Rewrite the (former last) paragraph about why we are not
erroring when `--range-diff-notes` is given without
`--range-diff`. Partly because the facts have changed; now we
cannot turn off the `override` bit/flag. But it’s just many words
to say that: why spend code disallowing something that you might
as well allow?
• Add a couple more tests, so simple that they also have an
accompanying comment each explaining why they exist
• Msg: Add a paragraph explaining why there are two tests specifically
for the single-patch case. It’s not just to cover every permutation.
• Remove useless `>actual` in tests that don’t test `actual` (they
test the patch files instead)
• Fix (kind of) the tests that use `$prev` as in:
git format-patch --range-diff=$prev
This is a very questionable and indirect use from this part of the
suite:
for prev in topic main..topic
do
[body]
done
I.e. it is just `main..topic`. This is monkey-see-monkey-do code
from my previous visit of this file. Which then turns out in turn
is a monkey-_ from *another* author. I think the existing `$prev`
should get a cleanup (separately).Notes (testing):
v4:
• Compiled and ran `t3206-range-diff`.
• Ran `make html` and looked at git-format-patch(1).Documentation/git-format-patch.adoc | 11 ++++ builtin/log.c | 42 +++++++++++++- t/t3206-range-diff.sh | 86 +++++++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-format-patch.adoc b/Documentation/git-format-patch.adoc index 191f64b77d1..2399ba24454 100644 --- a/Documentation/git-format-patch.adoc +++ b/Documentation/git-format-patch.adoc @@ -378,6 +378,17 @@ case is to show comparison with an older iteration of the same topic and the tool should find more correspondence between the two sets of patches. +`--range-diff-notes=<ref>`:: +`--no-range-diff-notes`:: + Used with `--range-diff`, tweak what notes to display in the + range diff. ++ +The default behavior is to display the same notes in the range diff as +on the patches; see `--notes`. Use `--range-diff-notes=<ref>` to use +_<ref>_ for the range diff instead. This option can be given multiple +times to show notes from multiple refs. Use `--no-range-diff-notes` to +disable notes in the range diff. + `--notes[=<ref>]`:: `--no-notes`:: Append the notes (see linkgit:git-notes[1]) for the commit diff --git a/builtin/log.c b/builtin/log.c index 560af00e2fd..445400ba782 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1327,15 +1327,44 @@ static void prepare_cover_text(struct pretty_print_context *pp, strbuf_release(&subject_sb); } +struct rdiff_notes { + /* + * True if we want to override the notes behavior + * of 'format-patch' + */ + bool override; + struct string_list notes; +}; + +static int rdiff_notes_cb(const struct option *option, + const char *arg, + int unset) +{ + struct option opt = *option; + struct rdiff_notes *rdiff_notes = option->value; + + rdiff_notes->override = 1; + opt.value = &rdiff_notes->notes; + return parse_opt_string_list(&opt, arg, unset); +} + static int get_notes_refs(struct string_list_item *item, void *arg) { strvec_pushf(arg, "--notes=%s", item->string); return 0; } -static void get_notes_args(struct rev_info *rev) +static void get_notes_args(struct rdiff_notes *rdiff_notes, + struct rev_info *rev) { - if (!rev->show_notes) { + if (rdiff_notes->override) { + if (rdiff_notes->notes.nr) + for_each_string_list(&rdiff_notes->notes, + get_notes_refs, + &rev->rdiff_log_arg); + else + strvec_push(&rev->rdiff_log_arg, "--no-notes"); + } else if (!rev->show_notes) { strvec_push(&rev->rdiff_log_arg, "--no-notes"); } else if (rev->notes_opt.use_default_notes > 0 || (rev->notes_opt.use_default_notes == -1 && @@ -1995,6 +2024,9 @@ int cmd_format_patch(int argc, struct strbuf rdiff1 = STRBUF_INIT; struct strbuf rdiff2 = STRBUF_INIT; struct strbuf rdiff_title = STRBUF_INIT; + struct rdiff_notes rdiff_notes = { + .notes = STRING_LIST_INIT_NODUP, + }; const char *rfc = NULL; int creation_factor = -1; const char *signature = git_version_string; @@ -2091,6 +2123,9 @@ int cmd_format_patch(int argc, parse_opt_object_name), OPT_STRING(0, "range-diff", &rdiff_prev, N_("refspec"), N_("show changes against <refspec> in cover letter or single patch")), + OPT_CALLBACK_F(0, "range-diff-notes", &rdiff_notes, N_("note"), + N_("override notes behavior for the range diff"), + 0, rdiff_notes_cb), OPT_INTEGER(0, "creation-factor", &creation_factor, N_("percentage by which creation is weighted")), OPT_BOOL(0, "force-in-body-from", &force_in_body_from, @@ -2406,7 +2441,7 @@ int cmd_format_patch(int argc, rev.rdiff_title = diff_title(&rdiff_title, reroll_count, _("Range-diff:"), _("Range-diff against v%d:")); - get_notes_args(&rev); + get_notes_args(&rdiff_notes, &rev); } /* @@ -2570,6 +2605,7 @@ int cmd_format_patch(int argc, release_revisions(&rev); format_config_release(&cfg); strvec_clear(&rev.rdiff_log_arg); + string_list_clear(&rdiff_notes.notes, 0); return 0; } diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index ef92704de39..679a707c873 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -845,6 +845,92 @@ test_expect_success 'format-patch --range-diff with multiple notes' ' test_cmp expect actual ' +# Unlike '--notes', '--range-diff-notes' requires a value +test_expect_success 'format-patch --range-diff-notes requires a value' ' + cat >expect <<-EOF && + error: option \`range-diff-notes${SQ} requires a value + EOF + test_must_fail git format-patch --range-diff=main..topic \ + --cover-letter --range-diff-notes 2>actual && + test_cmp expect actual +' + +# The '--range-diff-notes' has no effect but is allowed +test_expect_success 'format-patch --range-diff-notes=not-a-note (no --range-diff)' ' + test_when_finished "rm -f 000?-*" && + git format-patch --range-diff-notes=not-a-note --cover-letter \ + main..unmodified && + test_file_not_empty 0000-cover-letter* && + test_grep ! "^Range-diff:" 0000-cover-letter* && + test_grep ! "## Notes " 0000-cover-letter* +' + +test_expect_success 'format-patch --range-diff --notes=custom --no-range-diff-notes' ' + test_when_finished "git notes --ref=custom remove topic unmodified || :" && + git notes --ref=custom add -m "topic note1" topic && + git notes --ref=custom add -m "unmodified note1" unmodified && + test_when_finished "rm -f 000?-*" && + git format-patch --range-diff=main..topic --notes=custom \ + --no-range-diff-notes --cover-letter \ + main..unmodified && + test_grep "^Notes (custom):" 0004-* && + test_grep "^Range-diff:" 0000-cover-letter* && + test_grep ! "## Notes (custom) ##" 0000-cover-letter* +' + +test_expect_success 'format-patch --range-diff --no-notes --range-diff-notes=custom' ' + test_when_finished "git notes --ref=custom remove topic unmodified || :" && + git notes --ref=custom add -m "topic note1" topic && + git notes --ref=custom add -m "unmodified note1" unmodified && + test_when_finished "rm -f 000?-*" && + git format-patch --range-diff=main..topic --no-notes \ + --range-diff-notes=custom --cover-letter \ + main..unmodified && + test_grep ! "^Notes (custom):" 0004-* && + test_grep "^Range-diff:" 0000-cover-letter* && + test_grep "## Notes (custom) ##" 0000-cover-letter* +' + +test_expect_success 'format-patch --range-diff --notes=patch --range-diff-notes=rdiff' ' + test_when_finished "git notes --ref=patch remove topic unmodified || :" && + git notes --ref=patch add -m "only for patch 1" topic && + git notes --ref=patch add -m "only for patch 2" unmodified && + test_when_finished "git notes --ref=rdiff remove topic unmodified || :" && + git notes --ref=rdiff add -m "only for range diff 1" topic && + git notes --ref=rdiff add -m "only for range diff 2" unmodified && + test_when_finished "rm -f 000?-*" && + git format-patch --range-diff=main..topic --notes=patch \ + --range-diff-notes=rdiff --cover-letter \ + main..unmodified && + test_grep "^Notes (patch):" 0004-* && + test_grep ! "^Notes (rdiff):" 0004-* && + test_grep "^Range-diff:" 0000-cover-letter* && + test_grep "## Notes (rdiff) ##" 0000-cover-letter* && + test_grep ! "## Notes (patch) ##" 0000-cover-letter* +' + +test_expect_success 'format-patch --range-diff --no-range-diff-notes on single patch' ' + test_when_finished "git notes --ref=custom remove HEAD unmodified || :" && + git notes --ref=custom add -m "topic note (custom)" HEAD && + git notes --ref=custom add -m "unmodified note (custom)" unmodified && + git format-patch --notes=custom --range-diff=main..topic \ + --no-range-diff-notes -1 --stdout >actual && + test_grep "Notes (custom):" actual && + test_grep "^Range-diff:" actual && + test_grep ! "## Notes (custom) ##" actual +' + +test_expect_success 'format-patch --range-diff --range-diff-notes=custom on single patch' ' + test_when_finished "git notes --ref=custom remove HEAD unmodified || :" && + git notes --ref=custom add -m "topic note (custom)" HEAD && + git notes --ref=custom add -m "unmodified note (custom)" unmodified && + git format-patch --range-diff=main..topic \ + --range-diff-notes=custom -1 --stdout >actual && + test_grep ! "Notes (custom):" actual && + test_grep "^Range-diff:" actual && + test_grep "## Notes (custom) ##" actual +' + test_expect_success '--left-only/--right-only' ' git switch --orphan left-right && test_commit first &&
-- 2.55.0.793.gc667de3f2c5