Re: [PATCH v3 2/2] format-patch: learn --[no-]range-diff-notes
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 2, 2026, 17:28 UTC
- Message-ID
- <xmqqy0cgvwpi.fsf@gitster.g>
- In-Reply-To
- <V3_format-patch_learn_--range-diff-notes.d3b@m5gid.xyz>
kristofferhaugsbakk@fastmail.com writes:
Show 11 quoted lines
> From: Kristoffer Haugsbakk <code@khaugsbakk.name> > > git-format-patch(1) passes on the notes behavior that it is using for > the patches to git-range-diff(1). In turn you get the same Git notes > displayed in the range diff as the ones you used to generate the > patches. And that makes sense in most cases. > > However, I often make notes between series versions that mostly prepend > ... > 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).
Show 22 quoted lines
> 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=<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`.
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=<ref>' 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.
Show 29 quoted lines
> diff --git a/builtin/log.c b/builtin/log.c
> index 560af00e2fd..d70101f0755 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1327,15 +1327,56 @@ 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
> + */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.