log --format existence of notes?

log --format existence of notes?

[Bence Ferdinandy]

Hi,

based on the man pages it doesn't seem possible, but maybe I'm missing something.

I would like to put together a "log --format=" which is similar to --oneline,
but where if there's a note for the commit it's marked with e.g. a notebook
symbol. There's %N, but that prints the entire note, so it doesn't work well
with one commit per line.

E.g. something like this, where Taylor's commit does not have a note, but mine do:

eb12efd995 2024-10-08 Bence Ferdin.. remote set-head: refactor for readability 📓
a189f4e077 2024-09-28 Bence Ferdin.. refs: atomically record overwritten ref in update_symref 📓
ae0b28db8c 2024-10-19 Bence Ferdin.. t/t5505-remote: set default branch to main 📓
15030f9556 2024-10-15 Taylor Blau    The second batch

Is there a way to do this now? If not, maybe a "%N?" might be a nice addition?

Thanks,
Bence

-- 
bence.ferdinandy.com

Re: log --format existence of notes?

[Junio C Hamano]

"Bence Ferdinandy" <bence@ferdinandy.com> writes:

> based on the man pages it doesn't seem possible, but maybe I'm missing something.
>
> I would like to put together a "log --format=" which is similar to --oneline,
> but where if there's a note for the commit it's marked with e.g. a notebook
> symbol. There's %N, but that prints the entire note, so it doesn't work well
> with one commit per line.

I do not think it is doable.  Unlike the format language in the
for-each-ref/branch --list family of commands, the pretty-format
language in the log family of commands lack more involved
conditional formatting features.

Unifying these two formatting languages to port features from one to
the other would be needed, I guess.  If we had a note support in the
latter,  something like

$ git branch -l --format='%(subject)%(if)%(note:amlog)%(then)📓%(end)'

may have worked.

Re: log --format existence of notes?

[Bence Ferdinandy]

On Sat Nov 16, 2024 at 01:06, Junio C Hamano <gitster@pobox.com> wrote:
> "Bence Ferdinandy" <bence@ferdinandy.com> writes:
>
>> based on the man pages it doesn't seem possible, but maybe I'm missing something.
>>
>> I would like to put together a "log --format=" which is similar to --oneline,
>> but where if there's a note for the commit it's marked with e.g. a notebook
>> symbol. There's %N, but that prints the entire note, so it doesn't work well
>> with one commit per line.
>
> I do not think it is doable.  Unlike the format language in the
> for-each-ref/branch --list family of commands, the pretty-format
> language in the log family of commands lack more involved
> conditional formatting features.
>
> Unifying these two formatting languages to port features from one to
> the other would be needed, I guess.  

Is this already on the roadmap with a plan on how to do it? It sounds like
switching log format to the same formatting as the other commands would make
life easier in the long run.

> If we had a note support in the
> latter,  something like
>
> $ git branch -l --format='%(subject)%(if)%(note:amlog)%(then)📓%(end)'
>
> may have worked.

Conditional formatting definitely would sound useful for logs as well :)

On the other hand: would a quick win in analogy to "%G?" be considered? A "%N?"
that either outputs a "N" for note and a "-" for no note? Or something like
that. A bit ugly, but gets the job done.

Thanks,
Bence

[PATCH] ref-filter: add "notes" atom

[Kousik Sanagavarapu]

On Sat, Nov 16, 2024 at 04:11:17PM +0100, Bence Ferdinandy wrote:
> 
> On Sat Nov 16, 2024 at 01:06, Junio C Hamano <gitster@pobox.com> wrote:
> > "Bence Ferdinandy" <bence@ferdinandy.com> writes:
> >
> >> based on the man pages it doesn't seem possible, but maybe I'm missing something.
> >>
> >> I would like to put together a "log --format=" which is similar to --oneline,
> >> but where if there's a note for the commit it's marked with e.g. a notebook
> >> symbol. There's %N, but that prints the entire note, so it doesn't work well
> >> with one commit per line.
> >
> > I do not think it is doable.  Unlike the format language in the
> > for-each-ref/branch --list family of commands, the pretty-format
> > language in the log family of commands lack more involved
> > conditional formatting features.
> >
> > Unifying these two formatting languages to port features from one to
> > the other would be needed, I guess.  
> 
> Is this already on the roadmap with a plan on how to do it? It sounds like
> switching log format to the same formatting as the other commands would make
> life easier in the long run.

Yes, but a fixed plan doesn't exist really.  There has been effort in
the past (and even now) to re-implement formats from pretty in ref-filter
and when that is done, change everything to use ref-filter, but the parsing
mechanism in ref-filter is a bit broken in some really really rare cases [1].

> > If we had a note support in the
> > latter,  something like
> >
> > $ git branch -l --format='%(subject)%(if)%(note:amlog)%(then)📓%(end)'
> >
> > may have worked.
> 
> Conditional formatting definitely would sound useful for logs as well :)

I agree that having the "%(if)..." would be good in pretty.

[1] See thread

	https://lore.kernel.org/git/20241105190235.13502-1-five231003@gmail.com/



In the meantime, here is an implementation of "%(notes)" atom in
ref-filter.  I know this doesn't really address the "log" family since
we are not adding this format in pretty, but we can now do

	$ git branch -l --format='%(subject)%(if)%(notes:amlog)%(then)📓%(end)'

------------------------ >8 ------------------------
Subject: [PATCH] ref-filter: add "notes" atom

Introduce new "%(notes)" format which is capable of showing the notes
associated with any particular ref.  This new atom may also be given an
argument specifying where to look for the notes, for example
refs/notes/amlog, associated with the ref.

The behavior is the same as doing

	$ git notes show <refname>

for the bare "%(notes)" format and

	$ git notes --ref <notes-ref> show <refname>

for the "%(notes:notes-ref)" format.

Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
 Documentation/git-for-each-ref.txt |  7 ++++
 ref-filter.c                       | 54 ++++++++++++++++++++++++++++++
 t/t6300-for-each-ref.sh            | 28 ++++++++++++++++
 3 files changed, 89 insertions(+)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index d3764401a2..406e4a0390 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -329,6 +329,13 @@ exclude=<pattern>;;
 	in linkgit:git-describe[1] for details.
 --
 
+notes:<notes-ref>::
+	Show notes associated with a ref.  Defaults to getting notes
+	from `refs/notes/commits`.  If `"notes-ref"` is given then notes
+	retrieved from that ref are shown; for the given ref.  For
+	example, `%(notes:amlog)` will retrieve the notes from
+	`refs/notes/amlog` for the given ref.  See linkgit:git-notes[1].
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/ref-filter.c b/ref-filter.c
index 84c6036107..76c06178f2 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -155,6 +155,7 @@ enum atom_type {
 	ATOM_TRAILERS,
 	ATOM_CONTENTS,
 	ATOM_SIGNATURE,
+	ATOM_NOTES,
 	ATOM_RAW,
 	ATOM_UPSTREAM,
 	ATOM_PUSH,
@@ -235,6 +236,7 @@ static struct used_atom {
 			       S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL } option;
 		} signature;
 		struct strvec describe_args;
+		const char *notes_refname;
 		struct refname_atom refname;
 		char *head;
 	} u;
@@ -712,6 +714,15 @@ static int describe_atom_parser(struct ref_format *format UNUSED,
 	return 0;
 }
 
+static int notes_atom_parser(struct ref_format *format UNUSED,
+			     struct used_atom *atom,
+			     const char *arg, struct strbuf *err UNUSED)
+{
+	if (arg)
+		atom->u.notes_refname = arg;
+	return 0;
+}
+
 static int raw_atom_parser(struct ref_format *format UNUSED,
 			   struct used_atom *atom,
 			   const char *arg, struct strbuf *err)
@@ -974,6 +985,7 @@ static struct {
 	[ATOM_TRAILERS] = { "trailers", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },
 	[ATOM_CONTENTS] = { "contents", SOURCE_OBJ, FIELD_STR, contents_atom_parser },
 	[ATOM_SIGNATURE] = { "signature", SOURCE_OBJ, FIELD_STR, signature_atom_parser },
+	[ATOM_NOTES] = { "notes", SOURCE_OBJ, FIELD_STR, notes_atom_parser },
 	[ATOM_RAW] = { "raw", SOURCE_OBJ, FIELD_STR, raw_atom_parser },
 	[ATOM_UPSTREAM] = { "upstream", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
 	[ATOM_PUSH] = { "push", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
@@ -1957,6 +1969,44 @@ static void grab_describe_values(struct atom_value *val, int deref,
 	}
 }
 
+static void grab_notes_values(struct atom_value *val, int deref,
+			      struct object *obj)
+{
+	for (int i = 0; i < used_atom_cnt; i++) {
+		struct used_atom *atom = &used_atom[i];
+		const char *name = atom->name;
+		struct atom_value *v = &val[i];
+
+		struct child_process cmd = CHILD_PROCESS_INIT;
+		struct strbuf out = STRBUF_INIT;
+		struct strbuf err = STRBUF_INIT;
+
+		if (atom->atom_type != ATOM_NOTES)
+			continue;
+
+		if (!!deref != (*name == '*'))
+			continue;
+
+		cmd.git_cmd = 1;
+		strvec_push(&cmd.args, "notes");
+		if (atom->u.notes_refname) {
+			strvec_push(&cmd.args, "--ref");
+			strvec_push(&cmd.args, atom->u.notes_refname);
+		}
+		strvec_push(&cmd.args, "show");
+		strvec_push(&cmd.args, oid_to_hex(&obj->oid));
+		if (pipe_command(&cmd, NULL, 0, &out, 0, &err, 0) < 0) {
+			error(_("failed to run 'notes'"));
+			v->s = xstrdup("");
+			continue;
+		}
+		strbuf_rtrim(&out);
+		v->s = strbuf_detach(&out, NULL);
+
+		strbuf_release(&err);
+	}
+}
+
 /* See grab_values */
 static void grab_sub_body_contents(struct atom_value *val, int deref, struct expand_data *data)
 {
@@ -2076,6 +2126,7 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s
 		grab_sub_body_contents(val, deref, data);
 		grab_person("tagger", val, deref, buf);
 		grab_describe_values(val, deref, obj);
+		grab_notes_values(val, deref, obj);
 		break;
 	case OBJ_COMMIT:
 		grab_commit_values(val, deref, obj);
@@ -2084,14 +2135,17 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s
 		grab_person("committer", val, deref, buf);
 		grab_signature(val, deref, obj);
 		grab_describe_values(val, deref, obj);
+		grab_notes_values(val, deref, obj);
 		break;
 	case OBJ_TREE:
 		/* grab_tree_values(val, deref, obj, buf, sz); */
 		grab_sub_body_contents(val, deref, data);
+		grab_notes_values(val, deref, obj);
 		break;
 	case OBJ_BLOB:
 		/* grab_blob_values(val, deref, obj, buf, sz); */
 		grab_sub_body_contents(val, deref, data);
+		grab_notes_values(val, deref, obj);
 		break;
 	default:
 		die("Eh?  Object of type %d?", obj->type);
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index c39d4e7e9c..cf2ff26fd1 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -847,6 +847,34 @@ test_expect_success 'err on bad describe atom arg' '
 	)
 '
 
+test_expect_success 'bare notes atom' '
+	test_when_finished "git checkout main && git branch -D notes-atom " &&
+
+	git checkout -b notes-atom &&
+	test_commit --no-tag "commit on notes-atom" &&
+
+	git notes add -m "some msg" refs/heads/notes-atom &&
+	git notes show refs/heads/notes-atom >expect &&
+	git for-each-ref --format="%(notes)" refs/heads/notes-atom >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'notes atom with notes ref' '
+	test_when_finished \
+		"git checkout main && git branch -D notes-atom-refname" &&
+
+	git checkout -b notes-atom-refname &&
+	test_commit --no-tag "commit on notes-atom-refname" &&
+
+	git notes --ref notes-ref add -m "some msg" \
+		 refs/heads/notes-atom-refname &&
+	git notes --ref notes-ref show \
+		refs/heads/notes-atom-refname >expect &&
+	git for-each-ref --format="%(notes:notes-ref)" \
+		refs/heads/notes-atom-refname >actual &&
+	test_cmp expect actual
+'
+
 cat >expected <<\EOF
 heads/main
 tags/main
-- 
2.47.0.289.g3d75e57c9c

Re: [PATCH] ref-filter: add "notes" atom

[Junio C Hamano]

Kousik Sanagavarapu <five231003@gmail.com> writes:

> +static void grab_notes_values(struct atom_value *val, int deref,
> +			      struct object *obj)
> +{
> +	for (int i = 0; i < used_atom_cnt; i++) {
> +		struct used_atom *atom = &used_atom[i];
> +		const char *name = atom->name;
> +		struct atom_value *v = &val[i];
> +
> +		struct child_process cmd = CHILD_PROCESS_INIT;
> +		struct strbuf out = STRBUF_INIT;
> +		struct strbuf err = STRBUF_INIT;
> +
> +		if (atom->atom_type != ATOM_NOTES)
> +			continue;
> +
> +		if (!!deref != (*name == '*'))
> +			continue;
> +
> +		cmd.git_cmd = 1;
> +		strvec_push(&cmd.args, "notes");
> +		if (atom->u.notes_refname) {
> +			strvec_push(&cmd.args, "--ref");
> +			strvec_push(&cmd.args, atom->u.notes_refname);
> +		}
> +		strvec_push(&cmd.args, "show");
> +		strvec_push(&cmd.args, oid_to_hex(&obj->oid));
> +		if (pipe_command(&cmd, NULL, 0, &out, 0, &err, 0) < 0) {
> +			error(_("failed to run 'notes'"));
> +			v->s = xstrdup("");
> +			continue;
> +		}
> +		strbuf_rtrim(&out);
> +		v->s = strbuf_detach(&out, NULL);
> +
> +		strbuf_release(&err);
> +	}
> +}

I suspect that this was written to mimick what is done for describe.

The describe codepath has a (semi-)valid reason to fork out to a
subprocess, as computation of describe smudges the object flags of
in-core object database and it is not trivial to call into the
helper functions twice.

But showing notes for a single commit is merely an internal call to
get_note() away, so unless the note object is not a blob (which
should be absolutely rare), spawning a subprocess for each and every
ref tip feels a bit heavier than acceptable.  We'd probably need to
maintain a table of notes_trees, one per <note-ref> used as
%(notes:<note-ref>) in the format string, and init_notes() on them
while parsing the atoms, and in this codepath it would be a look-up
of notes_tree from the table based on the u.notes_refname by calling
get_note() to learn the object name, plus reading the object
contents into the v->s member when the note object is a blob (and
fallback the above code when it is not a blob, which is a rare-case,
if we really want to handle them).

Re: log --format existence of notes?

[Simon Richter]

Hi,

On 11/17/24 00:11, Bence Ferdinandy wrote:

> On the other hand: would a quick win in analogy to "%G?" be considered? A "%N?"
> that either outputs a "N" for note and a "-" for no note? Or something like
> that. A bit ugly, but gets the job done.

FWIW my main use case for notes is to record build/test results.

Having formats for "first line of notes" or "last line of notes" (with 
newline removed) would be really useful for building a poor man's CI 
that can display checkmarks next to commit messages.

    Simon

Re: [PATCH] ref-filter: add "notes" atom

[Kousik Sanagavarapu]

On Thu, Nov 21, 2024 at 08:51:47AM +0900, Junio C Hamano wrote:
> Kousik Sanagavarapu <five231003@gmail.com> writes:
> 
> > +static void grab_notes_values(struct atom_value *val, int deref,
> > +			      struct object *obj)
> > +{
> > +	for (int i = 0; i < used_atom_cnt; i++) {
> > +		struct used_atom *atom = &used_atom[i];
> > +		const char *name = atom->name;
> > +		struct atom_value *v = &val[i];
> > +
> > +		struct child_process cmd = CHILD_PROCESS_INIT;
> > +		struct strbuf out = STRBUF_INIT;
> > +		struct strbuf err = STRBUF_INIT;
> > +
> > +		if (atom->atom_type != ATOM_NOTES)
> > +			continue;
> > +
> > +		if (!!deref != (*name == '*'))
> > +			continue;
> > +
> > +		cmd.git_cmd = 1;
> > +		strvec_push(&cmd.args, "notes");
> > +		if (atom->u.notes_refname) {
> > +			strvec_push(&cmd.args, "--ref");
> > +			strvec_push(&cmd.args, atom->u.notes_refname);
> > +		}
> > +		strvec_push(&cmd.args, "show");
> > +		strvec_push(&cmd.args, oid_to_hex(&obj->oid));
> > +		if (pipe_command(&cmd, NULL, 0, &out, 0, &err, 0) < 0) {
> > +			error(_("failed to run 'notes'"));
> > +			v->s = xstrdup("");
> > +			continue;
> > +		}
> > +		strbuf_rtrim(&out);
> > +		v->s = strbuf_detach(&out, NULL);
> > +
> > +		strbuf_release(&err);
> > +	}
> > +}
> 
> I suspect that this was written to mimick what is done for describe.
> 
> The describe codepath has a (semi-)valid reason to fork out to a
> subprocess, as computation of describe smudges the object flags of
> in-core object database and it is not trivial to call into the
> helper functions twice.
> 
> But showing notes for a single commit is merely an internal call to
> get_note() away, so unless the note object is not a blob (which
> should be absolutely rare), spawning a subprocess for each and every
> ref tip feels a bit heavier than acceptable.  We'd probably need to
> maintain a table of notes_trees, one per <note-ref> used as
> %(notes:<note-ref>) in the format string, and init_notes() on them
> while parsing the atoms, and in this codepath it would be a look-up
> of notes_tree from the table based on the u.notes_refname by calling
> get_note() to learn the object name, plus reading the object
> contents into the v->s member when the note object is a blob (and
> fallback the above code when it is not a blob, which is a rare-case,
> if we really want to handle them).

Hi,
I was supposed to send a v2 a lot earlier but due to time constraints
(my semester is ending and there's work related to that) I've not been
able to work on this.  So if anyone is interested, they may work on this
and submit v2.  I personally think this format would be a nice addition
to the ref-filter framework and Junio's msg above gives a really nice
explanation on how to do things.

I will work on it in the earliest when I find time but just in case
someone else is interested, they may pick this up and work on it.

Thanks