Thread (41 messages) 41 messages, 4 authors, 7d ago

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

flat view

From: Kristoffer Haugsbakk <hidden>
Date: 2026-10-02 18:56:29

On Fri, Oct 2, 2026, at 19:28, Junio C Hamano wrote:
kristofferhaugsbakk@fastmail.com writes:
quoted
From: Kristoffer Haugsbakk <redacted>

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.
quoted
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.
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.
[snip]
quoted
+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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help