format-patch: learn --[no-]range-diff-notes

From: Kristoffer Haugsbakk <[email protected]>

Topic name: kh/format-patch-range-diff-notes

Topic summary: Teach 'format-patch' options to tweak notes output in the range diff independent of what notes are output in the patches.

See patch 3/3 for details.

This is motivated by wanting to turn off range diff notes, but the goal here is to implement it in full generality.

(How many of us git format-patch --notes users are there out there? More than a dozen?)

I have implemented this behavior for myself and used it for many months. But that was hacky and only suitable for one person’s use. So this is a completely new implementation. In other words: this is new code, *not* tested for months.

§ CI

https://github.com/LemmingAvalanche/git/actions/runs/32762207178

I seem to have finally learned now that I ought to push to my public Git tree for CI, not my private one. The latter seems to consistently give me “insufficient funds” errors. But I don’t know.

[1/3] format-patch: simplify get_notes_arg parameters [2/3] revision.h: rename struct member to reflect notes role [3/3] format-patch: learn --[no-]range-diff-notes

 Documentation/git-format-patch.adoc |  17 +++++
 builtin/log.c                       |  21 +++---
 log-tree.c                          |   2 +-
 revision.c                          |  13 ++++
 revision.h                          |   9 ++-
 t/t3206-range-diff.sh               | 105 ++++++++++++++++++++++++++++
 6 files changed, 156 insertions(+), 11 deletions(-)

base-commit: 1a3e64c6c4a623626ff0687008732a8e007e2a1c

2.55.0.13.g85d2d65e389

From: Kristoffer Haugsbakk <[email protected]>

85bd88a7 (revision: add rdiff_log_arg to rev_info, 2025-09-25) added rdiff_log_arg to struct rev_info. I changed get_notes_arg by simply replacing the first argument with an access on this struct member. But the second argument was already struct rev_info. So I should have just simplified to *only* passing that parameter. Let’s do that now.

Now is also a good time to format this for_each... line since it’s gotten quite long.

---

Notes (testing):
just compile tested

 builtin/log.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

index 350b35c5563..560af00e2fd 100644
@@ -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);
 	}
 
 	/*

2.55.0.13.g85d2d65e389

From: Kristoffer Haugsbakk <[email protected]>

The struct rev_info member rdiff_log_arg is only used to pass --[no-]notes options to git-range-diff(1), which in turn passes it on to git-log(1). The “log” in the name is fine since other code paths could choose to use it to pass something else on to git-range-diff(1) (as long as it makes sense to git-log(1)). However, we will in the next commit change revision.c:handle_revision_opt to push and clear this strvec based on notes options that the user passes. That means that only one type of git-log(1) option will be suitable for it. So let’s rename it to rdiff_notes_arg.

This structure member got its “log” name in 85bd88a7 (revision: add rdiff_log_arg to rev_info, 2025-09-25), which was based on the renaming of the range-diff.c variable other_arg to log_arg.[1] Now, in range-diff.c this log_arg really is used for multiple different git-log(1) options, namely --[no-]notes and --remerge-diff. But we can keep this rev_info member notes-only.

† 1: in 71fd6c69 (range-diff: rename other_arg to log_arg, 2025-09-25)

---

Notes (testing):
just compile tested

 builtin/log.c | 10 +++++-----
 log-tree.c    |  2 +-
 revision.h    |  4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

index 560af00e2fd..28a93c45463 100644
@@ -1336,15 +1336,15 @@ 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) {
-		strvec_push(&rev->rdiff_log_arg, "--no-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 &&
 		    !rev->notes_opt.extra_notes_refs.nr)) {
-		strvec_push(&rev->rdiff_log_arg, "--notes");
+		strvec_push(&rev->rdiff_notes_arg, "--notes");
 	} else {
 		for_each_string_list(&rev->notes_opt.extra_notes_refs,
 				     get_notes_refs,
-				     &rev->rdiff_log_arg);
+				     &rev->rdiff_notes_arg);
 	}
 }
 

@@ -1475,7 +1475,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 			.dual_color = 1,
 			.max_memory = RANGE_DIFF_MAX_MEMORY_DEFAULT,
 			.diffopt = &opts,
-			.log_arg = &rev->rdiff_log_arg
+			.log_arg = &rev->rdiff_notes_arg
 		};
 
 		repo_diff_setup(the_repository, &opts);

@@ -2569,7 +2569,7 @@ int cmd_format_patch(int argc,
 	rev.diffopt.no_free = 0;
 	release_revisions(&rev);
 	format_config_release(&cfg);
-	strvec_clear(&rev.rdiff_log_arg);
+	strvec_clear(&rev.rdiff_notes_arg);
 	return 0;
 }
 

index 83a3c4bf9b1..fd6ddf32af4 100644
@@ -718,7 +718,7 @@ static void show_diff_of_diff(struct rev_info *opt)
 			.dual_color = 1,
 			.max_memory = RANGE_DIFF_MAX_MEMORY_DEFAULT,
 			.diffopt = &opts,
-			.log_arg = &opt->rdiff_log_arg
+			.log_arg = &opt->rdiff_notes_arg
 		};
 
 		memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));

index acf6d06b241..39cca04d9e5 100644
@@ -351,7 +351,7 @@ struct rev_info {
 	/* range-diff */
 	const char *rdiff1;
 	const char *rdiff2;
-	struct strvec rdiff_log_arg;
+	struct strvec rdiff_notes_arg;
 	int creation_factor;
 	const char *rdiff_title;
 

@@ -432,7 +432,7 @@ struct rev_info {
 	.expand_tabs_in_log = -1, \
 	.commit_format = CMIT_FMT_DEFAULT, \
 	.expand_tabs_in_log_default = 8, \
-	.rdiff_log_arg = STRVEC_INIT, \
+	.rdiff_notes_arg = STRVEC_INIT, \
 }
 
 /**

2.55.0.13.g85d2d65e389

From: Kristoffer Haugsbakk <[email protected]>

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.

---

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(-)

index 191f64b77d1..e0ba435dfcf 100644
@@ -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

index 28a93c45463..de997bc9ab0 100644
@@ -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 &&

index 50dc8b19913..1e21f2861cc 100644
@@ -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);

index 39cca04d9e5..e8dbf774b00 100644
@@ -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;

2.55.0.13.g85d2d65e389

You will not get patch notes and not get range diff notes. That --range-diff-notes told it to use the patch notes which you just turned off/emptied the list.

Code-wise, the list of notes is cleared so you you would have to change the --notes implementation if you want to keep a sort of shadow list of not-patch-notes-but-RD-notes.
And another problem, or fact, is that format-patch does not show notes by default. So what should --RD-notes show? The default notes?

Thanks

sent from mobile

IOW, the design of how these options interact does not support the usecase I gave?

I do not know. My preference actually is not to introuce a new option whose interaction with the existing --notes option cannot be defined in simple terms.

Correct as far as I understand the use case.

Let's drop this topic then.

sent from mobile

That is fine by me. I was hoping that you'd come up with a way to add this new option with simpler-to-explain interactions. E.g., when only --notes exists on the command line, it is used as the material compared by the range-diff and as the material inserted into the final output, but when both options exist, they work independently, i.e., --notes gets used only as the final output, while --range-diff-notes gets used only for comparison material, or something like that.

This is how it works. The --range-diff-notes behavior that the doc discusses is just the special case when the list of notes for the range diff is empty.

That this wasn’t clear is the fault of the doc here.

Seeing as how the doc was unclear and did not spell out how you can build two separate list of notes, here’s a draft of a rewrite:

--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. But you can use these options to use a different list of notes. For example, say you have given three notes refs to --notes. At this point those same three notes will be displayed in the range diff. But then you pass --range-diff-notes=<ref>. Now the range diff will only display _<ref>_. You can of course pass more refs to this option, just like --notes. And you can also turn off all notes with --no-range-diff-notes.
+
You may want to turn off this notes override behavior 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. Going back to the three --notes example: now the range diff will show all three notes again.

[snip]

[nit: should we call this "no notes" override behavior? Otherwise I think we are referring to --range-diff-notes=<ref> overriding --notes=…]

A bit long, but easy to follow and understand the interactions, I think. The examples are helpful.

D. Ben Knoble

(I will shorten range-diff to RD for semi-brevity)

What I mean here by “notes override behavior” is turning off all --[no-]RD-notes options. It means turning off --RD-notes as well as --no-RD-notes. And without the override you are back to the default behavior where --notes dictates the notes for the range diff.

So that the utility is a bit more clear than these unmotivated examples, here’s an example alias (with forced linebreaks):

    my-fp = format-patch --notes=review --notes=testing
        --notes=attribution --notes=changelog
        --range-diff-notes=changelog

The patches will have four notes while the range diff will have one.

But you may want to disregard that last --RD-notes and in turn get all of the notes in the range diff. But without repeating yourself. Then you can do this:

    my-fp --no-range-diff-notes --range-diff-notes

The option (the negation) is not sufficient since it would turn off all range diff notes. But this special meaning of --RD-notes allows you to go back to just regular --notes behavior. That --RD-notes has a special meaning when the list of range diff notes is empty does not lose anything since --range-diff-notes would just be a noöp otherwise.[1]

But I should point out in this doc that bare --RD-notes does not use the default notes.

Of course, there could be a dedicated option to turn these options off.

Or to just not support it. ;)

(my standard verbosity level might not be doing me any favors on this point.)

***

That might seem like a lot of “power” for something as niche as overriding-then-reverting patch contra range diff notes. But code wise I don’t think the price is high... :)

† 1: I just tested the behavior of --notes (no arg) on
format-patch. Yes, it does respect the default notes ref just like git-log(1) does. So an alternative would be to have --RD-notes do the same.

But I do not think some convenient default notes ref is good for a command which is supposed to generate patches for email sendout. For log you can make convenient notes to yourself and conveniently display them. But format-patch should demand more intentionality. (I also wrote about this on a bugfix for format-patch behavior some years ago.)[2]
† 2: I suspect there is a bug-looking like behavior in that
format-patch seems to use notes.displayRef for the default notes (not just /refs/notes/commits). It should just respect format.notes, I think. But I can look at that later.

Thanks. I noticed the lines kept creeping up, but it is more involved than most options; an option for passing on to another command which also overrides the behavior of another option.

Thanks for taking a look at this niche topic. Though I see that you are one of the dozen of us[3] who use Git notes on his submissions. ;)

🔗 3: https://lore.kernel.org/git/[email protected]/T/#m6a7cbbe0fc456e7e62125d903b706ae5a547315b

Aha! I _did_ misunderstand, then :) I thought this example in the proposal was for the case where "my-fp" has "--no-RD-notes" and we wanted to re-add them with "--RD-notes".

Heh, definitely a bit confusing, but spelled out it makes sense.

I thought about that, as well, after re-absorbing the examples. I'm not sure what to call it, though. "disable-RD-notes" is a mouthful and doesn't seem to have precedence from my (spotty!) memory of various subcommands.

Reading from the sidelines, it seems we have often gotten ourselves in trouble because the code was easy and too easily reflected in the user interface. OTOH, I'm not sure what else to do here ;)

Huh, interesting. "git help format-patch" says format.notes turns on "--notes", so I would guess without looking further that it is a boolean.

Yet "git help config" says it can provide a ref.

So, yeah, I would expect format-patch should use format.notes over notes.displayRef.

The price of flexibility ;)

<3

D. Ben Knoble

Up to this point it is quite clear how the two interact. Even though it does not appear in the above paragraph, the rules essentially are "Without --range-diff-notes, the refs that are specified by --notes are used for both purposes" and "When you use --range-diff-notes, --notes and --range-diff-notes give independent sets of notes, the former is shown only in the output, the latter is used only for comparison".

But the following paragraph, while it may be correctly describing what the code does, does not tell me why you would even want to do so.

For example, if you have --notes=foo --notes=bar always given in an alias, i.e.

    [alias] fmt = format-patch --notes=foo --notes=bar

but in one invocation you would want to use different set of notes only for comparison, you would

    git fmt --range-diff-notes=

if you do not want any notes participate in the comparison, or

    git fmt --range-diff-notes=bar

you want only 'bar' to be used in the comparison.

If you had --range-diff-notes=foo in a similar way in an alias,

    [alias] fmtr = format-patch --range-diff-notes=foo --notes=bar

you may need a way to tell that 'foo' no longer participates in the comparison with

    git fmtr --no-range-diff-notes

If the rule is that once you say --no-range-diff-notes the internal state is reset and the command behaves as if no --range-diff-notes option is ever given [*], then that would still leave --notes=bar so the command would beave as if

    git format-patch --notes=bar

were given, which means bar will now affect both, so if you want 'bar' not to be used for comparison, you would need some way to pretend as if you said

    git format-patch --range-diff-notes= --notes=bar

and ...

... may be a way to do so, perhaps?

BUT I think that is a strange interpretation and notation. Normal people would rather assume, once you said --no-range-diff-notes, you do not want any notes to be used for range-diff comparison. IOW, I find the earlier rule [*] that makes --no-range-diff-notes only tell the command to pretend that no --range-diff-notes is ever given, which leads to the above conclusion, a source of confusion.

If the rule were "if you say --no-range-diff-notes, you are saying that you do not want any notes used for range-diff" (and similarly "if you say --no-notes you are saying that you do not want any notes used"), would it make the workaround in the last part unnecessary? Under such a world order,

    git fmtr --no-range-diff-notes

would mean that --no-range-diff-notes tells that you do not want any notes participate in the comparison, so any --notes in the alias definition of fmtr would be used only for the final display. And

    git fmtr --no-range-diff-notes --range-diff-notes

would tell the command that on top of the previous state, you are adding 0 notes to the set of notes used for comparisons, so it would be a no op. If it were

    git fmtr --no-range-diff-notes --range-diff-notes=bar

then you'd let --notes in the fmtr alias definition to be used for final display, --range-diff-notes in the fmtr alias definition to be totally ignored, and bar is used for comparison.

Would that logically make sense and make it easier to understand?

Thanks.

Side note: using --range-diff-notes= (empty arg) to signal no-notes would be inconsistent with --notes. Those options just take that value. Then they inevitably output:

    $ git log --notes=
    warning: notes ref refs/notes/ is invalid
    [output]

Thanks for the detailed walkthrough.

I don’t understand why you contrast these two approaches:

(I’m using RD as a shorthand for range-diff again)

1. --no-RD-notes means “revert to whatever --notes is up to”, as if no --[no-]RD-notes of any kind were ever given
2. --no-RD-notes means “no range diff/comparison notes at all”

Since (2) was the only design I presented. Is the point that you can use these two approaches to eventually find a way to implement the “revert to --notes behavior”? Well, if so I understand.

You seem to be saying that (1), which is not in my implementation, is used which in turn necessitates the workaround presented in the part of the doc that you presented. But that’s not the case.

Here we lose the power to revert to what --notes is using. (Which you demonstrated the utility of with the alias.) But I think that is fine. It is a niche behavior of a niche option. Does not warrant the end-user to think this hard at all.

So here is my redesign:

• There are only --no-RD-notes and --RD-notes=<ref>, i.e. the last one has to have an argument. Since we have no use for arg-less --RD-notes any more.
• That means that we can use a regular pars-opts callback instead of adding it to revision.c:handle_revision_opt.
• The same rule about interaction with patch notes: no such RD notes means that the patches notes determine what notes the range diff gets. *With* any such options, however, they are determined only by those options. That includes turning off all range diff notes with --no-RD-notes.
• No feature for the niche behavior of turning *back on* “use the patch notes” behavior for the range diff notes

Thoughts? I’ll try to work on the reroll in the meantime.

Ah, I didn't know that one. It sounds like a UI bug we can safely fix without worrying about being backward incompatible.

No. I thought #1 was what you were doing, which was how I thought was the only way for the command line you suggested in an earlier message would make sense.

You may want to turn off this notes override behavior 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. Going back to the three --notes example: now the range diff will show all three notes again.

Under the interpretation #2, the first --no-RD-notes tells us that we won't use notes for comparison, and then the next --RD-notes tells us that we use notes listed as parameter to it (which is "no notes") for comparison, so the "notes override behaviour" is not turned off. We use no notes for comparison, and use the ones that are given with --notes=<note> only for display.

Under the interpretation #1, the first --no-RD-notes would make the command behave as if no --RD-notes were even given, and --notes=<note> would be used both for comparison and display. Then --RD-notes that says there is no particular notes you want for comparison would make the <note> given earlier with --notes=<note> not to be used for comparison. After spelling it out like this, it seems that even #1 does not turn off this notes override behaviour, either. I admit that I wasn't thinking about interpretation #1 too deeply as I wasn't interested in seeing it happen.

So it is good that we agree we want to use the interpretation #2. Which means the "You may want to turn off ..." part of the documentation inaccurate (I think I've already suggested striking it off in an earlier message).

From: Kristoffer Haugsbakk <[email protected]>

Topic name (applied): kh/format-patch-range-diff-notes

Topic summary: Teach 'format-patch' options to tweak notes output in the range diff independent of what notes are output in the patches.

Hey, sorry if someone got duplicate emails right now! I tried to send out about ten minutes ago but it didn’t hit the list. It turned out that there was no To header.

Well I don’t know if emails without To are sent to the Cc addresses.

***

See patch 2/2 for details.

This is motivated by wanting to turn off range diff notes, but the goal here is to implement it in full generality.

(How many of us git format-patch --notes users are there out there? More than a dozen? Maybe just D. Ben Knoble and me?)

I have implemented this behavior for myself and used it for many months. But that was hacky and only suitable for one person’s use. So this is a completely new implementation. In other words: this is new code, *not* tested for months.

§ Changes in 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/[email protected]/#t

This also means that the implementation is quite different. Now it just uses a parse-options callback instead of adding if/else to revision.c:handle_revision_opt.

See patch 2/2 for details.

Version 1 patch 2/3 is dropped. It was a rename motivated by the changes to struct rev_info in version 1 patch 3/3, which is now gone. The v1 3/3 change needed the struct member to stay notes-only, but that is no longer required.

[1/2] format-patch: simplify get_notes_arg parameters [2/2] format-patch: learn --[no-]range-diff-notes

 Documentation/git-format-patch.adoc | 15 +++++
 builtin/log.c                       | 62 ++++++++++++++++++--
 t/t3206-range-diff.sh               | 87 +++++++++++++++++++++++++++++
 3 files changed, 158 insertions(+), 6 deletions(-)

Interdiff against v1:

index e0ba435dfcf..5907f299a8d 100644
@@ -378,22 +378,20 @@ 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>]`::
+`--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`).
+	range diff.
 +
-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.
+The default behavior is to display the same notes in the range diff as
+on the patches; see `--notes`. But you can use these options to use a
+different list of notes. For example, say you have given three notes
+refs to `--notes`. At this point those same three notes will be
+displayed in the range diff. But then you pass
+`--range-diff-notes=<ref>`. Now the range diff will only display
+_<ref>_. You can of course pass more refs to this option, just like
+`--notes`. And you can also turn off all range diff notes with
+`--no-range-diff-notes`.
 
 `--notes[=<ref>]`::
 `--no-notes`::

index fd6ddf32af4..83a3c4bf9b1 100644
@@ -718,7 +718,7 @@ static void show_diff_of_diff(struct rev_info *opt)
 			.dual_color = 1,
 			.max_memory = RANGE_DIFF_MAX_MEMORY_DEFAULT,
 			.diffopt = &opts,
-			.log_arg = &opt->rdiff_notes_arg
+			.log_arg = &opt->rdiff_log_arg
 		};
 
 		memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));

index 1e21f2861cc..50dc8b19913 100644
@@ -2625,19 +2625,6 @@ 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);

index e8dbf774b00..acf6d06b241 100644
@@ -351,12 +351,7 @@ 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;
+	struct strvec rdiff_log_arg;
 	int creation_factor;
 	const char *rdiff_title;
 

@@ -437,7 +432,7 @@ struct rev_info {
 	.expand_tabs_in_log = -1, \
 	.commit_format = CMIT_FMT_DEFAULT, \
 	.expand_tabs_in_log_default = 8, \
-	.rdiff_notes_arg = STRVEC_INIT, \
+	.rdiff_log_arg = STRVEC_INIT, \
 }
 
 /**

Range-diff against v1:

1:  977f9c2e97a = 1:  977f9c2e97a format-patch: simplify get_notes_arg parameters
2:  2a555d40ced < -:  ----------- revision.h: rename struct member to reflect notes role
3:  058f5fdc8da ! 2:  bf66e94e376 format-patch: learn --[no-]range-diff-notes
    @@ Commit message
         • No such options given
         • `--no-range-diff-notes`
     
    -    Well, we can’t. Therefore we need `rdiff_override_notes` to set whenever
    +    Well, we can’t. Therefore we need `rdiff_notes.override` 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:
     
    @@ Commit message
     
         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.
    +    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.[2]
    +
    +    † 2: 155986b4 (format-patch: handle range-diff on notes correctly for
    +         single patches, 2025-09-25)
     
         Signed-off-by: Kristoffer Haugsbakk <[email protected]>
     
     
      ## Notes (testing) ##
    -    CI: https://github.com/LemmingAvalanche/git/actions/runs/32762207178
    +    CI: https://github.com/LemmingAvalanche/git/actions/runs/36231842902
    +
    +    This run is on a previous iteration where v1 patch/commit 2/3 was still
    +    there. But that is just a rename. So I compiled and tested
    +    `t/t3206-range-diff.sh` and took that as proof that the full CI/build run
    +    is still valid.
     
      ## Documentation/git-format-patch.adoc ##
     @@ Documentation/git-format-patch.adoc: 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>]`::
    ++`--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:
    ++	range diff.
     ++
    -+----
    -+--no-range-diff-notes --range-diff-notes
    -+----
    -++
    -+Now the range diff is back to displaying the same notes as the patches.
    ++The default behavior is to display the same notes in the range diff as
    ++on the patches; see `--notes`. But you can use these options to use a
    ++different list of notes. For example, say you have given three notes
    ++refs to `--notes`. At this point those same three notes will be
    ++displayed in the range diff. But then you pass
    ++`--range-diff-notes=<ref>`. Now the range diff will only display
    ++_<ref>_. You can of course pass more refs to this option, just like
    ++`--notes`. And you can also turn off all range diff notes with
    ++`--no-range-diff-notes`.
     +
      `--notes[=<ref>]`::
      `--no-notes`::
      	Append the notes (see linkgit:git-notes[1]) for the commit
     
      ## builtin/log.c ##
    -@@ builtin/log.c: static int get_notes_refs(struct string_list_item *item, void *arg)
    +@@ builtin/log.c: 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 rdiff_notes *rdiff_notes = option->value;
    ++
    ++	rdiff_notes->override = 1;
    ++
    ++	/*
    ++	 * The rest is the same as
    ++	 * parse-options-cb.c:parse_opt_string_list
    ++	 */
    ++	if (unset) {
    ++		string_list_clear(&rdiff_notes->notes, 0);
    ++		return 0;
    ++	}
    ++
    ++	if (!arg)
    ++		return -1;
    ++
    ++	string_list_append(&rdiff_notes->notes, arg);
    ++	return 0;
    ++}
    ++
    + 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 rev_info *rev)
    ++static void get_notes_args(struct rdiff_notes *rdiff_notes,
    ++			   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");
    ++	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_notes_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 &&
    -
    - ## revision.c ##
    -@@ revision.c: 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);
    -
    - ## revision.h ##
    -@@ revision.h: 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;
    +@@ builtin/log.c: 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;
    +@@ builtin/log.c: 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,
    +@@ builtin/log.c: 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);
    + 	}
    + 
    + 	/*
    +@@ builtin/log.c: 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;
    + }
    + 
     
      ## t/t3206-range-diff.sh ##
     @@ t/t3206-range-diff.sh: 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_when_finished "rm -f 000?-*" &&
    ++	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=$prev --notes=custom \
    ++	git format-patch --range-diff=main..topic --notes=custom \
     +		--no-range-diff-notes --cover-letter \
    -+		main..unmodified >actual &&
    ++		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 --range-diff-notes uses --notes behavior' '
    ++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=$prev --notes=custom \
    -+		--range-diff-notes --cover-letter \
    -+		main..unmodified >actual &&
    -+	test_grep "^Notes (custom):" 0004-* &&
    ++	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*
     +'
    @@ t/t3206-range-diff.sh: test_expect_success 'format-patch --range-diff with multi
     +	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 \
    ++	git format-patch --range-diff=main..topic --notes=patch \
     +		--range-diff-notes=rdiff --cover-letter \
    -+		main..unmodified >actual &&
    ++		main..unmodified &&
     +	test_grep "^Notes (patch):" 0004-* &&
     +	test_grep ! "^Notes (rdiff):" 0004-* &&
     +	test_grep "^Range-diff:" 0000-cover-letter* &&
    @@ t/t3206-range-diff.sh: test_expect_success 'format-patch --range-diff with multi
     +	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 \
    ++	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 &&
    @@ t/t3206-range-diff.sh: test_expect_success 'format-patch --range-diff with multi
     +	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 \
    ++	git format-patch --range-diff=main..topic \
     +		--range-diff-notes=custom -1 --stdout >actual &&
     +	test_grep ! "Notes (custom):" actual &&
     +	test_grep "^Range-diff:" actual &&

base-commit: 1a3e64c6c4a623626ff0687008732a8e007e2a1c

2.55.0.793.gc667de3f2c5

From: Kristoffer Haugsbakk <[email protected]>

85bd88a7 (revision: add rdiff_log_arg to rev_info, 2025-09-25) added rdiff_log_arg to struct rev_info. I changed get_notes_arg by simply replacing the first argument with an access on this struct member. But the second argument was already struct rev_info. So I should have just simplified to *only* passing that parameter. Let’s do that now.

Now is also a good time to format this for_each... line since it’s gotten quite long.

---

Notes (testing):
just compile tested

 builtin/log.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

index 350b35c5563..560af00e2fd 100644
@@ -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);
 	}
 
 	/*

2.55.0.793.gc667de3f2c5

From: Kristoffer Haugsbakk <[email protected]>

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_notes.override to set whenever any of these options are given.

† 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; 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.[2]

† 2: 155986b4 (format-patch: handle range-diff on notes correctly for

     single patches, 2025-09-25)

---

Notes (series):
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/[email protected]/#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):
CI: https://github.com/LemmingAvalanche/git/actions/runs/36231842902

This run is on a previous iteration where v1 patch/commit 2/3 was still there. But that is just a rename. So I compiled and tested t/t3206-range-diff.sh and took that as proof that the full CI/build run is still valid.

 Documentation/git-format-patch.adoc | 15 +++++
 builtin/log.c                       | 54 +++++++++++++++++-
 t/t3206-range-diff.sh               | 87 +++++++++++++++++++++++++++++
 3 files changed, 153 insertions(+), 3 deletions(-)

index 191f64b77d1..5907f299a8d 100644
@@ -378,6 +378,21 @@ 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`. But you can use these options to use a
+different list of notes. For example, say you have given three notes
+refs to `--notes`. At this point those same three notes will be
+displayed in the range diff. But then you pass
+`--range-diff-notes=<ref>`. Now the range diff will only display
+_<ref>_. You can of course pass more refs to this option, just like
+`--notes`. And you can also turn off all range diff notes with
+`--no-range-diff-notes`.
+
 `--notes[=<ref>]`::
 `--no-notes`::
 	Append the notes (see linkgit:git-notes[1]) for the commit

2.55.0.793.gc667de3f2c5

Oh yeah. If there is no Range-diff then
there won't be a notes section. I'll fix that
in the next version.

I do not understand that comment. I was merely saying that you are registering the same clean-up-when-we-are-done handler twice. Having the earlier invocation of "test_when_finished rm -f 000?-*" shoud be sufficient. It does not make a difference whether we have notes in the range-diff or not.

Yeah. For some reason in my head I jumped
to assuming that second test_grep was in question. x)

Yeah that cleanup is redundant. It happens to be
placed where I have the Notes cleanup in the
other tests.

From: Kristoffer Haugsbakk <[email protected]>

Topic name (applied): kh/format-patch-range-diff-notes

Topic summary: Teach 'format-patch' options to tweak notes output in the range diff independent of what notes are output in the patches.

See patch 2/2 for details.

This is motivated by wanting to turn off range diff notes, but the goal here is to implement it in full generality.

(How many of us git format-patch --notes users are there out there? More than a dozen? Maybe just D. Ben Knoble and me?)

I have implemented this behavior for myself and used it for many months. But that was hacky and only suitable for one person’s use. So this is a completely new implementation. In other words: this is new code, *not* tested for months.

§ Changes in v3

From patch 2/2:

Remove repeated and redundant test_when_finished on patch files:

https://lore.kernel.org/git/[email protected]/T/#m06803e233a2e385e694432d45ecf402f7a67e482

§ Link to v2

https://lore.kernel.org/git/[email protected]/

[1/2] format-patch: simplify get_notes_arg parameters [2/2] format-patch: learn --[no-]range-diff-notes

 Documentation/git-format-patch.adoc | 15 +++++
 builtin/log.c                       | 62 +++++++++++++++++++--
 t/t3206-range-diff.sh               | 86 +++++++++++++++++++++++++++++
 3 files changed, 157 insertions(+), 6 deletions(-)

Interdiff against v2:

index 640c5dec52e..679a707c873 100755
@@ -860,7 +860,6 @@ 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_when_finished "rm -f 000?-*" &&
 	test_file_not_empty 0000-cover-letter* &&
 	test_grep ! "^Range-diff:" 0000-cover-letter* &&
 	test_grep ! "## Notes " 0000-cover-letter*

Range-diff against v2:

1:  977f9c2e97a = 1:  977f9c2e97a format-patch: simplify get_notes_arg parameters
2:  bf66e94e376 ! 2:  748759ca021 format-patch: learn --[no-]range-diff-notes
    @@ Commit message
     
     
      ## Notes (testing) ##
    -    CI: https://github.com/LemmingAvalanche/git/actions/runs/36231842902
    -
    -    This run is on a previous iteration where v1 patch/commit 2/3 was still
    -    there. But that is just a rename. So I compiled and tested
    -    `t/t3206-range-diff.sh` and took that as proof that the full CI/build run
    -    is still valid.
    +    For v3: only compiled and ran `t3206-range-diff`.
     
      ## Documentation/git-format-patch.adoc ##
     @@ Documentation/git-format-patch.adoc: case is to show comparison with an older iteration of the same
    @@ t/t3206-range-diff.sh: test_expect_success 'format-patch --range-diff with multi
     +	test_when_finished "rm -f 000?-*" &&
     +	git format-patch --range-diff-notes=not-a-note --cover-letter \
     +		main..unmodified &&
    -+	test_when_finished "rm -f 000?-*" &&
     +	test_file_not_empty 0000-cover-letter* &&
     +	test_grep ! "^Range-diff:" 0000-cover-letter* &&
     +	test_grep ! "## Notes " 0000-cover-letter*

base-commit: 1a3e64c6c4a623626ff0687008732a8e007e2a1c

2.55.0.793.gc667de3f2c5

From: Kristoffer Haugsbakk <[email protected]>

85bd88a7 (revision: add rdiff_log_arg to rev_info, 2025-09-25) added rdiff_log_arg to struct rev_info. I changed get_notes_arg by simply replacing the first argument with an access on this struct member. But the second argument was already struct rev_info. So I should have just simplified to *only* passing that parameter. Let’s do that now.

Now is also a good time to format this for_each... line since it’s gotten quite long.

---

Notes (testing):
just compile tested

 builtin/log.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

index 350b35c5563..560af00e2fd 100644
@@ -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);
 	}
 
 	/*

2.55.0.793.gc667de3f2c5

The readers do not necessarily want to read the "author's journey" narrative in log messages. Let's be more detached and objective, like

85bd88a7e8 (revision: add rdiff_log_arg to rev_info, 2025-09-25) updated get_notes_args() to push into rev->rdiff_log_arg instead of an explicit strvec, but left the rev argument as the second parameter and strvec *arg as the first. Simplify the signature of get_notes_args() to take only struct rev_info *rev, dropping the redundant strvec *arg parameter.

perhaps?

The code change looks good. As long as this stays as a static helper function, this is not a loss of flexibility but a simplification of the calling convention.

I don’t get what objective improvement there is by replacing “I did” with “it happened”. This is not a gratuitous incidental biography but just says what your alternative says, only with a personal pronoun, less technical diction, and one word longer.

But I think we can shorten it with a little show-don’t-tell:

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.

That’s 16 words less than my first version.

Thanks for reviewing.

Sure.

Fine. The only thing I was concerned about was someone jumping to the conclusion that the --range-diff-notes=<ref> would be additive to the --notes options. But this says “different notes ref” which clearly means that the intent is to discard the --notes for the range diff.

I think that version of yours is better.

Obviously better.

Sorry about this duplicate that message that replied to the wrong email as well.

From: Kristoffer Haugsbakk <[email protected]>

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_notes.override to set whenever any of these options are given.

† 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; 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.[2]

† 2: 155986b4 (format-patch: handle range-diff on notes correctly for

     single patches, 2025-09-25)

---

Notes (series):
v3:
• Remove repeated and redundant test_when_finished on patch files[1]

🔗 1: https://lore.kernel.org/git/[email protected]/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/[email protected]/#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):
For v3: only compiled and ran t3206-range-diff.

 Documentation/git-format-patch.adoc | 15 +++++
 builtin/log.c                       | 54 +++++++++++++++++-
 t/t3206-range-diff.sh               | 86 +++++++++++++++++++++++++++++
 3 files changed, 152 insertions(+), 3 deletions(-)

index 191f64b77d1..5907f299a8d 100644
@@ -378,6 +378,21 @@ 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`. But you can use these options to use a
+different list of notes. For example, say you have given three notes
+refs to `--notes`. At this point those same three notes will be
+displayed in the range diff. But then you pass
+`--range-diff-notes=<ref>`. Now the range diff will only display
+_<ref>_. You can of course pass more refs to this option, just like
+`--notes`. And you can also turn off all range diff notes with
+`--no-range-diff-notes`.
+
 `--notes[=<ref>]`::
 `--no-notes`::
 	Append the notes (see linkgit:git-notes[1]) for the commit

2.55.0.793.gc667de3f2c5

This is somewhat shared with the next step, but the commit message includes a lengthy narrative of the author's thought process ("An off/on switch is enough for this behavior...", "But now we are faced with a problem...", "Well, we can't. Therefore we need...").

Can we strip out the conversational journey? The log message should be a concise, permanent technical reference explaining the problem (range diff notes inherit patch notes, which may contain irrelevant iteration changelogs) and the solution (the new options and the .override flag).

        static int rdiff_notes_cb(const struct option *option,
                               const char *arg,
                               int unset)
        {
                struct option opt = *option;
                struct rdiff_notes *rdiff_notes = opt.value;

                rdiff_notes->override = 1;
                opt.value = &rdiff_notes->notes;
                return parse_opt_string_list(&opt, arg, unset);
        }

Sure.

Fine. The only thing I was concerned about was someone jumping to the conclusion that the --range-diff-notes=<ref> would be additive to the --notes options. But this says “different notes ref” which clearly means that the intent is to discard the --notes for the range diff.

I think that version of yours is better.

Obviously better.

From: Kristoffer Haugsbakk <[email protected]>

Topic name (applied): kh/format-patch-range-diff-notes

Topic summary: Teach 'format-patch' options to tweak notes output in the range diff independent of what notes are output in the patches.

See patch 2/2 for details.

This is motivated by wanting to turn off range diff notes, but the goal here is to implement it in full generality.

(How many of us git format-patch --notes users are there out there? More than a dozen? Maybe just D. Ben Knoble and me?)

I have implemented this behavior for myself and used it for many months. But that was hacky and only suitable for one person’s use. So this is a completely new implementation. In other words: this is new code, *not* tested for months.

§ Changes in v4

Mostly trim expository fat. Also one code refactor. See the patch *notes* for details.

§ Link to v3

https://lore.kernel.org/git/[email protected]/

[1/2] format-patch: simplify get_notes_arg parameters [2/2] format-patch: learn --[no-]range-diff-notes

 Documentation/git-format-patch.adoc | 11 ++++
 builtin/log.c                       | 50 +++++++++++++++--
 t/t3206-range-diff.sh               | 86 +++++++++++++++++++++++++++++
 3 files changed, 141 insertions(+), 6 deletions(-)

Interdiff against v3:

index 5907f299a8d..2399ba24454 100644
@@ -384,14 +384,10 @@ sets of patches.
 	range diff.
 +
 The default behavior is to display the same notes in the range diff as
-on the patches; see `--notes`. But you can use these options to use a
-different list of notes. For example, say you have given three notes
-refs to `--notes`. At this point those same three notes will be
-displayed in the range diff. But then you pass
-`--range-diff-notes=<ref>`. Now the range diff will only display
-_<ref>_. You can of course pass more refs to this option, just like
-`--notes`. And you can also turn off all range diff notes with
-`--no-range-diff-notes`.
+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`::

index d70101f0755..445400ba782 100644
@@ -1337,27 +1337,15 @@ struct rdiff_notes {
 };
 
 static int rdiff_notes_cb(const struct option *option,
-		       const char *arg,
-		       int unset)
+			  const char *arg,
+			  int unset)
 {
+	struct option opt = *option;
 	struct rdiff_notes *rdiff_notes = option->value;
 
 	rdiff_notes->override = 1;
-
-	/*
-	 * The rest is the same as
-	 * parse-options-cb.c:parse_opt_string_list
-	 */
-	if (unset) {
-		string_list_clear(&rdiff_notes->notes, 0);
-		return 0;
-	}
-
-	if (!arg)
-		return -1;
-
-	string_list_append(&rdiff_notes->notes, arg);
-	return 0;
+	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)

Range-diff against v3:
1: 977f9c2e97a ! 1: bb60f300d3f format-patch: simplify get_notes_arg parameters
@@ Commit message
format-patch: simplify get_notes_arg parameters

85bd88a7 (revision: add rdiff_log_arg to rev_info, 2025-09-25) added
- rdiff_log_arg to struct rev_info. I changed get_notes_arg by
- simply replacing the first argument with an access on this struct
- member. But the second argument was already struct rev_info. So I
- should have just simplified to *only* passing that parameter. Let’s do
- that now.
+ 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.
@@ Commit message

## Notes (testing) ##
+ v1:
just compile tested

## builtin/log.c ##
2: 748759ca021 ! 2: 4cbd312fec6 format-patch: learn --[no-]range-diff-notes
@@ Commit message
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.
+ 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.

- 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.
+ In addition to storing the list of notes, we also need a boolean
+ override to distinguish these two cases:

- 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_notes.override to set whenever
- any of these options are given.
-
- † 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.
+ 1. No such options were given and empty list (use --notes)
+ 2. Options were given and empty list (--no-... given; don’t use notes)

***

@@ Commit message
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.[2]
+ notes handling bug.[1]

- † 2: 155986b4 (format-patch: handle range-diff on notes correctly for
+ † 1: 155986b4 (format-patch: handle range-diff on notes correctly for
single patches, 2025-09-25)

+ Helped-by: Junio C Hamano <[email protected]>

## Notes (testing) ##
- For v3: only compiled and ran t3206-range-diff.
+ v4:
+ • Compiled and ran t3206-range-diff.
+ • Ran make html and looked at git-format-patch(1).

## Documentation/git-format-patch.adoc ##
@@ Documentation/git-format-patch.adoc: case is to show comparison with an older iteration of the same
@@ Documentation/git-format-patch.adoc: case is to show comparison with an older it
+ range diff.
++
+The default behavior is to display the same notes in the range diff as
-+on the patches; see --notes. But you can use these options to use a
-+different list of notes. For example, say you have given three notes
-+refs to --notes. At this point those same three notes will be
-+displayed in the range diff. But then you pass
-+--range-diff-notes=<ref>. Now the range diff will only display
-+_<ref>_. You can of course pass more refs to this option, just like
-+--notes. And you can also turn off all range diff notes with -+--no-range-diff-notes.
++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::
@@ builtin/log.c: static void prepare_cover_text(struct pretty_print_context *pp,
+};
+
+static int rdiff_notes_cb(const struct option *option,
-+ const char *arg,
-+ int unset)
++ const char *arg,
++ int unset)
+{
++ struct option opt = *option;
+ struct rdiff_notes *rdiff_notes = option->value;
+
+ rdiff_notes->override = 1;
-+
-+ /*
-+ * The rest is the same as
-+ * parse-options-cb.c:parse_opt_string_list
-+ */
-+ if (unset) {
-+ string_list_clear(&rdiff_notes->notes, 0);
-+ return 0;
-+ }
-+
-+ if (!arg)
-+ return -1;
-+
-+ string_list_append(&rdiff_notes->notes, arg);
-+ return 0;
++ 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)

base-commit: 1a3e64c6c4a623626ff0687008732a8e007e2a1c

2.55.0.793.gc667de3f2c5

From: Kristoffer Haugsbakk <[email protected]>

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.

---

Notes (series):

    v4:
    • Shorter commit message. No I.[1]
      🔗 1: https://lore.kernel.org/git/[email protected]/T/#mfbb107570d497be5bfe54fe209014b607f5d5830

Notes (testing):
v1:
just compile tested

 builtin/log.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

index 350b35c5563..560af00e2fd 100644
@@ -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);
 	}
 
 	/*

2.55.0.793.gc667de3f2c5

From: Kristoffer Haugsbakk <[email protected]>

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)

---

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/[email protected]/#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/[email protected]/

---

v3:
• Remove repeated and redundant test_when_finished on patch files[1]

🔗 1: https://lore.kernel.org/git/[email protected]/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/[email protected]/#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(-)

index 191f64b77d1..2399ba24454 100644
@@ -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

2.55.0.793.gc667de3f2c5

Nicely described.

In short, do you mean something like this?

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.

I suspect that erroring out when only creation-factor is given, perhaps via an alias, was a design mistake. A user who wants to use a setting customized for their workflow must resort to an alias because there is no configuration variable to control its default. In that light, the same argument for --[no-]range-diff-notes applies here. On the other hand, perhaps if we had a configuration variable to control which notes are compared in range-diff and shown in the output, we would not have to worry about these things. I do not know.

Other than that (no, not the "shall we also add a configuration?", which I consider is outside the topic, but the overly verbose log message that gives wandering thought process that does not help the readers with crisp reasoning that leads to the decision which they may or may not agree with), it looks good.

That’s a better way to describe it. I think I will use it pretty much verbatim.

Now in hindsight, with your version on display in front of me, I don’t know why I couldn’t make that paragraph more straighforward. Sometimes I go on a narrative journey because I think it is clearer (but never shorter), but here I didn’t want to do that at all. I just wanted to lay out the motivation. Stumped.

Yeah it can prevent some workflows while not really helping prevent any errors, I think.

I think I can make this next version right now. I have tried to give more time to each version (like the last one, intentionally waiting more than a day) in order to give other people time to react to them. However at this point most of the changes in this series are so stable that I don’t think there are any points to interject to for some hypotethetical person that already had two days or so to speak up.

From: Kristoffer Haugsbakk <[email protected]>

Topic name (applied): kh/format-patch-range-diff-notes

Topic summary: Teach 'format-patch' options to tweak notes output in the range diff independent of what notes are output in the patches.

See patch 2/2 for details.

This is motivated by wanting to turn off range diff notes, but the goal here is to implement it in full generality.

(How many of us git format-patch --notes users are there out there? More than a dozen? Maybe just D. Ben Knoble and me?)

I have implemented this behavior for myself and used it for many months. But that was hacky and only suitable for one person’s use. So this is a completely new implementation. In other words: this is new code, *not* tested for months.

§ Changes in v5

Patch 2/2:

• Msg: Shorten paragraph about “why not error out like --creation-factor...” while keeping the exact same information.[1]
🔗 1: https://lore.kernel.org/git/[email protected]/
• Msg: ... Also drop the thematic breaks (***). I think the paragraphs flow well enough now to the point that they are not needed.

§ Link to v4

https://lore.kernel.org/git/[email protected]/

[1/2] format-patch: simplify get_notes_arg parameters [2/2] format-patch: learn --[no-]range-diff-notes

 Documentation/git-format-patch.adoc | 11 ++++
 builtin/log.c                       | 50 +++++++++++++++--
 t/t3206-range-diff.sh               | 86 +++++++++++++++++++++++++++++
 3 files changed, 141 insertions(+), 6 deletions(-)

Interdiff against v4:
Range-diff against v4:

1:  bb60f300d3f = 1:  bb60f300d3f format-patch: simplify get_notes_arg parameters
2:  4cbd312fec6 ! 2:  676361b383e format-patch: learn --[no-]range-diff-notes

@@ Commit message
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.
-
- ***
+ 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

base-commit: 1a3e64c6c4a623626ff0687008732a8e007e2a1c

2.55.0.793.gc667de3f2c5

From: Kristoffer Haugsbakk <[email protected]>

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.

---

Notes (series):

    v4:
    • Shorter commit message. No I.[1]
      🔗 1: https://lore.kernel.org/git/[email protected]/T/#mfbb107570d497be5bfe54fe209014b607f5d5830

Notes (testing):
v1:
just compile tested

 builtin/log.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

index 350b35c5563..560af00e2fd 100644
@@ -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);
 	}
 
 	/*

2.55.0.793.gc667de3f2c5

From: Kristoffer Haugsbakk <[email protected]>

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)

---

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/[email protected]/
• 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/[email protected]/#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/[email protected]/
---
v3:
• Remove repeated and redundant test_when_finished on patch files[1]

🔗 1: https://lore.kernel.org/git/[email protected]/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/[email protected]/#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(-)

index 191f64b77d1..2399ba24454 100644
@@ -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

2.55.0.793.gc667de3f2c5