From: Kristoffer Haugsbakk Date: Fri, 02 Oct 2026 18:56:05 GMT Subject: Re: [PATCH v3 2/2] format-patch: learn --[no-]range-diff-notes Message-ID: In-Reply-To: On Fri, Oct 2, 2026, at 19:28, Junio C Hamano wrote: > kristofferhaugsbakk@fastmail.com writes: > >> From: Kristoffer Haugsbakk >> >> 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 >> ... >> something like an alias set up with it. But why spend code closing >> that door? There is no usability upside to erroring out. > > 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). Sure. > >> diff --git a/Documentation/git-format-patch.adoc b/Documentation/git-format-patch.adoc >> index 191f64b77d1..5907f299a8d 100644 >> --- a/Documentation/git-format-patch.adoc >> +++ b/Documentation/git-format-patch.adoc >> @@ -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=`:: >> +`--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=`. Now the range diff will only display >> +__. 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`. > > Very chatty and colloquial. A technical reference manual should be > concise and direct. Here is my attempt to condense it down to make > it more readable: > > By default, '--range-diff' displays the same notes as the patches > (see '--notes'). Use '--range-diff-notes=' to specify a > different notes ref for the range diff. 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. Fine. The only thing I was concerned about was someone jumping to the conclusion that the `--range-diff-notes=` 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. >[snip] >> +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 >> + */ > > Hmph, I wonder if it is more future-proof to wrap the string-list > callback like so ... > > 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); > } > > ... than copying and letting the code drift apart. Obviously better.