[PATCH 3/3] 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.

So it would be useful to turn off range diff notes handling with
something like `--no-range-diff-notes`. This could then be turned on
again with `--range-diff-notes`.

An off/on switch is enough for this behavior. However, a bare (no arg)
option (together with the negation) is not consistent with `--[no-]notes
[=<ref>]` and could cause confusion. And we are both conceptually and
literally constructing an argument list to pass on to git-range-diff(1),
which does have the same option format as git-format-patch(1). Moreover,
it is useful to be able to specify exactly what notes you want
git-format-patch(1) and git-range-diff(1) to use.[1] So let’s generalize
it so that you can pass in whatever notes refs you want.

But now we are faced with a problem that `--notes` does not have; how do
we distinguish an empty `struct string_list` meaning these two things?:

• No such options given
• `--no-range-diff-notes`

Well, we can’t. Therefore we need `rdiff_override_notes` to set whenever
any of these options are given.

However, we may also want to turn *off* this override. Just like how we
can countermand any notes ref we pass in:

    --notes=custom --no-notes

To that end, let’s make `--range-diff-notes` when the list of options is
empty special. Then it means: go back to using whatever git-format-
patch(1) wants to use.

Now, `--notes` is a bit special in that it has an optional
argument. Implementing this with a parse-options callback is not
user-friendly; the following does *not* mean what it looks like:

    --parse-option --another-option

Namely, it is not a bare `--parse-option` followed by another
option. Rather, it’s one option:

    --parse-option=--another-option

And we need the bare `--range-diff-notes` form in order to turn off
notes overriding. For that reason, let’s implement these new options in
`revision.c:handle_revision_opt`, just like the `--notes` options are.

† 1: For example, let say we have two notes ref that are used for a
     patch series:

     1. testing. What the user has done to test this iteration.
     2. changelog. The same example from the introduction.

     You could include both notes on the patches but only show `testing` in
     the range diff.

***

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`. Yes, we could introduce struct member `rdiff_notes_arg_used` or
something in order to detect the same condition. Or turn `rdiff_notes_
override` into a tri-state `int`. But the extra code is not worth that
in my opinion.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---

Notes (testing):
    CI: https://github.com/LemmingAvalanche/git/actions/runs/32762207178

 Documentation/git-format-patch.adoc |  17 +++++
 builtin/log.c                       |   5 +-
 revision.c                          |  13 ++++
 revision.h                          |   5 ++
 t/t3206-range-diff.sh               | 105 ++++++++++++++++++++++++++++
 5 files changed, 144 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-format-patch.adoc b/Documentation/git-format-patch.adoc
index 191f64b77d1..e0ba435dfcf 100644
--- a/Documentation/git-format-patch.adoc
+++ b/Documentation/git-format-patch.adoc
@@ -378,6 +378,23 @@ 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. For example, you can use `--no-range-diff-notes` to
+	turn off all notes in the range diff. The default behavior is
+	to display the same notes in the range diff as on the patches
+	(see `--notes`).
++
+You may want to turn off this notes override after it has been
+activated. Use this sequence to do that:
++
+----
+--no-range-diff-notes --range-diff-notes
+----
++
+Now the range diff is back to displaying the same notes as the patches.
+
 `--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 28a93c45463..de997bc9ab0 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1335,7 +1335,10 @@ static int get_notes_refs(struct string_list_item *item, void *arg)
 
 static void get_notes_args(struct rev_info *rev)
 {
-	if (!rev->show_notes) {
+	if (rev->rdiff_override_notes) {
+		if (!rev->rdiff_notes_arg.nr)
+			strvec_push(&rev->rdiff_notes_arg, "--no-notes");
+	} else if (!rev->show_notes) {
 		strvec_push(&rev->rdiff_notes_arg, "--no-notes");
 	} else if (rev->notes_opt.use_default_notes > 0 ||
 		   (rev->notes_opt.use_default_notes == -1 &&
diff --git a/revision.c b/revision.c
index 50dc8b19913..1e21f2861cc 100644
--- a/revision.c
+++ b/revision.c
@@ -2625,6 +2625,19 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->notes_opt.use_default_notes = 1;
 	} else if (!strcmp(arg, "--no-standard-notes")) {
 		revs->notes_opt.use_default_notes = 0;
+	} else if (!strcmp(arg, "--no-range-diff-notes")) {
+		strvec_clear(&revs->rdiff_notes_arg);
+		revs->rdiff_override_notes = 1;
+	} else if (!strcmp(arg, "--range-diff-notes")) {
+		/*
+		 * Allow the user to use '--no-range-diff-notes
+		 * --range-diff-notes' in order to go back to
+		 * using the 'format-patch' notes behavior
+		 */
+		revs->rdiff_override_notes = revs->rdiff_notes_arg.nr;
+	} else if (skip_prefix(arg, "--range-diff-notes=", &optarg)) {
+		strvec_pushf(&revs->rdiff_notes_arg, "--notes=%s", optarg);
+		revs->rdiff_override_notes = 1;
 	} else if (!strcmp(arg, "--oneline")) {
 		revs->verbose_header = 1;
 		get_commit_format("oneline", revs);
diff --git a/revision.h b/revision.h
index 39cca04d9e5..e8dbf774b00 100644
--- a/revision.h
+++ b/revision.h
@@ -351,6 +351,11 @@ struct rev_info {
 	/* range-diff */
 	const char *rdiff1;
 	const char *rdiff2;
+	/*
+	 * whether to use 'rdiff_notes_arg' or inherited
+	 * notes behavior
+	 */
+	bool rdiff_override_notes;
 	struct strvec rdiff_notes_arg;
 	int creation_factor;
 	const char *rdiff_title;
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index ef92704de39..db238d0a5a1 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -845,6 +845,111 @@ test_expect_success 'format-patch --range-diff with multiple notes' '
 	test_cmp expect actual
 '
 
+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=$prev --notes=custom \
+		--no-range-diff-notes --cover-letter \
+		main..unmodified >actual &&
+	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 --range-diff-notes uses --notes behavior' '
+	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=$prev --notes=custom \
+		--range-diff-notes --cover-letter \
+		main..unmodified >actual &&
+	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=$prev --notes=patch \
+		--range-diff-notes=rdiff --cover-letter \
+		main..unmodified >actual &&
+	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 --range-diff-notes uses --notes behavior' '
+	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=$prev --notes=custom \
+		--no-range-diff-notes --range-diff-notes --cover-letter \
+		main..unmodified >actual &&
+	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 --range-diff-notes uses --notes behavior' '
+	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=$prev --notes=custom \
+		--range-diff-notes --cover-letter \
+		main..unmodified >actual &&
+	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-range-diff-notes does not use default notes' '
+	test_when_finished "git notes remove topic unmodified || :" &&
+	git notes add -m "topic note1" topic &&
+	git notes add -m "unmodified note1" unmodified &&
+	test_when_finished "rm -f 000?-*" &&
+	git format-patch --range-diff=$prev \
+		--no-range-diff-notes --cover-letter \
+		main..unmodified >actual &&
+	test_grep ! "^Notes:" 0004-* &&
+	test_grep "^Range-diff:" 0000-cover-letter* &&
+	test_grep ! "## Notes ##" 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=$prev \
+		--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 --no-notes --range-diff=$prev \
+		--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.13.g85d2d65e389