Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx> writes:
> Currently, `range-diff` shows the default notes if no notes-related
> arguments are given. This is also how `log` behaves. But unlike
> `range-diff`, `log` does *not* show the default notes if
> `--notes=<custom>` are given. In other words, this:
>
> git log --notes=custom
>
> is equivalent to this:
>
> git log --no-notes --notes=custom
>
> While:
>
> git range-diff --notes=custom
>
> acts like this:
>
> git log --notes --notes-custom
>
> This can’t be how the user expects `range-diff` to behave given that the
> man page for `range-diff` under `--[no-]notes[=<ref>]` says:
>
>> This flag is passed to the `git log` program (see git-log(1)) that
>> generates the patches.
>
> This behavior also affects `format-patch` since it uses `range-diff` for
> the cover letter. Unlike `log`, though, `format-patch` is not supposed
> to show the default notes if no notes-related arguments are given.[1]
> But this promise is broken when the range-diff happens to have something
> to say about the changes to the default notes, since that will be shown
> in the cover letter.
>
> Remedy this by introducing `--show-notes-by-default` that `range-diff` can
> use to tell the `log` subprocess what to do.
Very well described. I think the rest of the proposed log message
is redundant now we have quite a good write-up above.
> ifndef::git-rev-list[]
> +--show-notes-by-default::
> + Show the default notes (see `--notes`) unless subsequent arguments
> + are used to display specific notes.
> +
> --notes[=<ref>]::
> Show the notes (see linkgit:git-notes[1]) that annotate the
> commit, when showing the commit log message. This is the default
I think the new entry should come after the description of `--notes`,
which is the primary option around the "notes" feature.
In the description, I think "subsequent" is misphrased. It makes it
sound as if
$ git log --show-notes-by-default --notes=amlog
would stop showing the notes from the default notes tree (because
the notes from the .git/refs/notes/amlog is explicitly asked for),
while
$ git log --notes=amlog --show-notes-by-default
would show both the default and the custom notes, which is not what
the code does, I think, in this hunk:
> @@ -3054,6 +3056,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
> if (revs->expand_tabs_in_log < 0)
> revs->expand_tabs_in_log = revs->expand_tabs_in_log_default;
>
> + if (!revs->show_notes_given && revs->show_notes_by_default) {
> + enable_default_display_notes(&revs->notes_opt, &revs->show_notes);
> + revs->show_notes_given = 1;
> + }
> +
> return left;
> }
Other than the above minor nits, looks very good.
Thanks.
