[PATCH v5 2/2] format-patch: learn --[no-]range-diff-notes

[kristofferhaugsbakk@fastmail.com]

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)

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 <gitster@pobox.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---

Notes (series):
    v5:
    • Msg: Shorten paragraph about “why not error out like
      --creation-factor...” while keeping the exact same
      information.[1] Now the commit message fits on one screen for
      me! (1080p)
      🔗 1: https://lore.kernel.org/git/xmqqqzi5touh.fsf@gitster.g/
    • Msg: ... Also drop the thematic breaks (***). I think the
      paragraphs flow well enough now to the point that they are not
      needed.
    
    ---
    
    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