Thread (20 messages) flat view 20 messages, 3 authors, 8h ago

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

From: Kristoffer Haugsbakk <hidden>
Date: 2026-09-09 18:08:41

On Sun, Sep 6, 2026, at 19:12, Junio C Hamano wrote:
"Kristoffer Haugsbakk" [off-list ref] writes:
quoted
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`.
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=
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]
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 ...
quoted
    +
    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.
... 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.
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.
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?
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.
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?
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help