Git development
 help / color / mirror / Atom feed
From: "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com>
To: "Junio C Hamano" <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 3/3] format-patch: learn --[no-]range-diff-notes
Date: Wed, 09 Sep 2026 20:08:17 +0200	[thread overview]
Message-ID: <8f0a076b-4822-44e2-a842-cc1e39ae1c1d@app.fastmail.com> (raw)
In-Reply-To: <xmqqcxuq483g.fsf@gitster.g>

On Sun, Sep 6, 2026, at 19:12, Junio C Hamano wrote:
> "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
>
>> 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 ...
>
>>     +
>>     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.

  reply	other threads:[~2026-09-09 18:08 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-24 20:35 [PATCH 0/3] format-patch: learn --[no-]range-diff-notes kristofferhaugsbakk
2026-08-24 20:35 ` [PATCH 1/3] format-patch: simplify get_notes_arg parameters kristofferhaugsbakk
2026-08-24 20:35 ` [PATCH 2/3] revision.h: rename struct member to reflect notes role kristofferhaugsbakk
2026-08-24 20:35 ` [PATCH 3/3] format-patch: learn --[no-]range-diff-notes kristofferhaugsbakk
2026-08-24 22:31   ` Junio C Hamano
2026-08-25 18:36     ` Kristoffer Haugsbakk
2026-08-28  0:31       ` Junio C Hamano
2026-08-28 13:48         ` Kristoffer Haugsbakk
2026-08-28 17:13           ` Junio C Hamano
2026-09-02 13:19             ` Kristoffer Haugsbakk
2026-09-06  7:22               ` Kristoffer Haugsbakk
2026-09-06 13:37                 ` D. Ben Knoble
2026-09-06 16:44                   ` Kristoffer Haugsbakk
2026-09-06 17:57                     ` D. Ben Knoble
2026-09-06 17:12                 ` Junio C Hamano
2026-09-09 18:08                   ` Kristoffer Haugsbakk [this message]
2026-09-09 19:04                     ` Junio C Hamano
2026-09-26 18:27 ` [PATCH v2 0/2] " kristofferhaugsbakk
2026-09-26 18:27   ` [PATCH v2 1/2] format-patch: simplify get_notes_arg parameters kristofferhaugsbakk
2026-09-26 18:27   ` [PATCH v2 2/2] format-patch: learn --[no-]range-diff-notes kristofferhaugsbakk
2026-09-27 12:50     ` Junio C Hamano
2026-09-27 19:42       ` Kristoffer Haugsbakk
2026-09-28 15:35         ` Junio C Hamano
2026-09-28 15:53           ` Kristoffer Haugsbakk
2026-10-02 10:56 ` [PATCH v3 0/2] " kristofferhaugsbakk
2026-10-02 10:56   ` [PATCH v3 1/2] format-patch: simplify get_notes_arg parameters kristofferhaugsbakk
2026-10-02 16:50     ` Junio C Hamano
2026-10-02 18:51       ` Kristoffer Haugsbakk
2026-10-02 19:07       ` Kristoffer Haugsbakk
2026-10-02 19:13         ` Kristoffer Haugsbakk
2026-10-02 10:56   ` [PATCH v3 2/2] format-patch: learn --[no-]range-diff-notes kristofferhaugsbakk
2026-10-02 17:28     ` Junio C Hamano
2026-10-02 18:56       ` Kristoffer Haugsbakk
2026-10-04 10:17 ` [PATCH v4 0/2] " kristofferhaugsbakk
2026-10-04 10:17   ` [PATCH v4 1/2] format-patch: simplify get_notes_arg parameters kristofferhaugsbakk
2026-10-04 10:17   ` [PATCH v4 2/2] format-patch: learn --[no-]range-diff-notes kristofferhaugsbakk
2026-10-04 16:25     ` Junio C Hamano
2026-10-04 17:30       ` Kristoffer Haugsbakk
2026-10-04 17:58 ` [PATCH v5 0/2] " kristofferhaugsbakk
2026-10-04 17:58   ` [PATCH v5 1/2] format-patch: simplify get_notes_arg parameters kristofferhaugsbakk
2026-10-04 17:58   ` [PATCH v5 2/2] format-patch: learn --[no-]range-diff-notes kristofferhaugsbakk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8f0a076b-4822-44e2-a842-cc1e39ae1c1d@app.fastmail.com \
    --to=kristofferhaugsbakk@fastmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox