Hi Junio,
On Mon, 11 Sep 2023, Junio C Hamano wrote:
> Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx> writes:
>
> >> To fix this explicitly set the output format of the internally executed
> >> `git log` with `--pretty=medium`. Because that cancels `--notes`, add
> >> explicitly `--notes` at the end.
> >
> > § Authors
> >
> > • Fix by Johannes
> > • Tests by Kristoffer
> >
> > † 1: See e.g. 66b2ed09c2 (Fix "log" family not to be too agressive about
> > showing notes, 2010-01-20).
> >
> > Co-authored-by: Johannes Schindelin <Johannes.Schindelin@xxxxxx>
> > Signed-off-by: Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx>
> > ---
>
> OK, Dscho, does this round look acceptable to you?
>
> It feels UGLY to iterate over args _without_ actually parsing them,
> at least to me. Such a non-parsing look breaks at least in two ways
> over time. (1) a mechanism may be introduced laster, similar to
> "--", that allows other_arg->v[] array to mark "here is where the
> dashed options end". Now the existing loop keeps reading to the end
> and finds "--notes" that is not a dashed option but is part of the
> normal command line arguments in "other arg". (2) Among the dashed
> options passed in the other_arg->v[], there may be an option that
> takes a string value, and a value that happens to be "--notes" is
> mistaken as asking for "notes" (iow "git log -G --notes" is looking
> for commits with changes that contain "double dash followed by en oh
> tee ee es").
>
> I think "git range-diff -G --notes" (or "-S --notes") shows that
> this new non-parsing loop is already broken. It looks for a change
> that has "--notes" correctly, but at the same time, triggers that
> "ah, we have an explicit --notes so drop the implicit_notes_arg
> flag" logic.
Right, `-G --notes` is a good argument to rethink this.
A much more surgical way to address the issue at hand might be this
(Kristoffer, what do you think? It's missing documentation for the
newly-introduced `--show-notes-by-default` option, but you get the idea):
-- snipsnap --
diff --git a/range-diff.c b/range-diff.c
index fbb81a92cc0..56f6870ff91 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -41,20 +41,12 @@ static int read_patches(const char *range, struct string_list *list,
struct child_process cp = CHILD_PROCESS_INIT;
struct strbuf buf = STRBUF_INIT, contents = STRBUF_INIT;
struct patch_util *util = NULL;
- int i, implicit_notes_arg = 1, in_header = 1;
+ int in_header = 1;
char *line, *current_filename = NULL;
ssize_t len;
size_t size;
int ret = -1;
- for (i = 0; other_arg && i < other_arg->nr; i++)
- if (!strcmp(other_arg->v[i], "--notes") ||
- starts_with(other_arg->v[i], "--notes=") ||
- !strcmp(other_arg->v[i], "--no-notes")) {
- implicit_notes_arg = 0;
- break;
- }
-
strvec_pushl(&cp.args, "log", "--no-color", "-p", "--no-merges",
"--reverse", "--date-order", "--decorate=no",
"--no-prefix", "--submodule=short",
@@ -68,9 +60,8 @@ static int read_patches(const char *range, struct string_list *list,
"--output-indicator-context=#",
"--no-abbrev-commit",
"--pretty=medium",
+ "--show-notes-by-default",
NULL);
- if (implicit_notes_arg)
- strvec_push(&cp.args, "--notes");
strvec_push(&cp.args, range);
if (other_arg)
strvec_pushv(&cp.args, other_arg->v);
diff --git a/revision.c b/revision.c
index 2f4c53ea207..49d385257ac 100644
--- a/revision.c
+++ b/revision.c
@@ -2484,6 +2484,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
revs->break_bar = xstrdup(optarg);
revs->track_linear = 1;
revs->track_first_time = 1;
+ } else if (!strcmp(arg, "--show-notes-by-default")) {
+ revs->show_notes_by_default = 1;
} else if (skip_prefix(arg, "--show-notes=", &optarg) ||
skip_prefix(arg, "--notes=", &optarg)) {
if (starts_with(arg, "--show-notes=") &&
@@ -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;
}
diff --git a/revision.h b/revision.h
index 82ab400139d..50091bbd13f 100644
--- a/revision.h
+++ b/revision.h
@@ -253,6 +253,7 @@ struct rev_info {
shown_dashes:1,
show_merge:1,
show_notes_given:1,
+ show_notes_by_default:1,
show_signature:1,
pretty_given:1,
abbrev_commit:1,
