From 4ddd9f289debef7878f737b05de9a56989152b33 Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Sun, 4 Oct 2026 19:58:34 +0200 Subject: [PATCH 1/2] format-patch: simplify get_notes_arg parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 85bd88a7 (revision: add rdiff_log_arg to rev_info, 2025-09-25) added `rdiff_log_arg` to `struct rev_info`. `get_notes_arg` was changed to take a second parameter, namely that member: get_notes_args(&(rev.rdiff_log_arg), &rev); But this is obviously unnecessary; we can just use `&rev`. Now is also a good time to format this `for_each...` line since it’s gotten quite long. Signed-off-by: Kristoffer Haugsbakk Signed-off-by: Junio C Hamano --- builtin/log.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 350b35c556..560af00e2f 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1333,16 +1333,18 @@ static int get_notes_refs(struct string_list_item *item, void *arg) return 0; } -static void get_notes_args(struct strvec *arg, struct rev_info *rev) +static void get_notes_args(struct rev_info *rev) { if (!rev->show_notes) { - strvec_push(arg, "--no-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 && !rev->notes_opt.extra_notes_refs.nr)) { - strvec_push(arg, "--notes"); + strvec_push(&rev->rdiff_log_arg, "--notes"); } else { - for_each_string_list(&rev->notes_opt.extra_notes_refs, get_notes_refs, arg); + for_each_string_list(&rev->notes_opt.extra_notes_refs, + get_notes_refs, + &rev->rdiff_log_arg); } } @@ -2404,7 +2406,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.rdiff_log_arg), &rev); + get_notes_args(&rev); } /* From 5008bde9900db114f106e3262108ce726b879a11 Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Sun, 4 Oct 2026 19:58:35 +0200 Subject: [PATCH 2/2] format-patch: learn --[no-]range-diff-notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Unlike `--creation-factor`, `--[no-]range-diff-notes` does not error out when used without `--range-diff`. This flexibility accommodates workflows where users might configure default options in aliases or wrapper scripts, allowing `--range-diff` to be toggled independently. 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 Signed-off-by: Kristoffer Haugsbakk Signed-off-by: Junio C Hamano --- 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 191f64b77d..2399ba2445 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=`:: +`--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=` to use +__ 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[=]`:: `--no-notes`:: Append the notes (see linkgit:git-notes[1]) for the commit diff --git a/builtin/log.c b/builtin/log.c index 560af00e2f..445400ba78 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1327,15 +1327,44 @@ do_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 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 @@ done: 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 ef92704de3..679a707c87 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 &&