ref-filter: add %(notes) format atom

From: ZheNing Hu <[email protected]>

Note that --pretty="%N" can view notes related to the commit. So add %(notes) to ref-filter seem is a good choice. This atom %(notes) view the notes associated with the ref, and support dereference.

Signed-off-by ZheNing Hu <[email protected]>

---
[GSOC] ref-filter: add %(notes) format atom

An important step in the GSOC project Use ref-filter formats in git cat-file is to integrate different format atoms into the ref-filter. Olga and Hariom have also made a lot of efforts in this area.

Currently, I noticed that there may be some format atoms in "pretty.c" that have not been migrated to ref-filter, such as --pretty="%N", --pretty="%(describe)".

So in this patch, I tried to migrate --pretty=%N to --format=%(notes).

Hope this will be hopeful !!!

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-944%2Fadlternative%2Fformat-notes-atom-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-944/adlternative/format-notes-atom-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/944

 Documentation/git-for-each-ref.txt |  4 ++++
 ref-filter.c                       | 31 ++++++++++++++++++++++++++++--
 t/t6300-for-each-ref.sh            | 10 ++++++++++
 3 files changed, 43 insertions(+), 2 deletions(-)

Documentation/git-for-each-ref.txt +4 -0

index 2ae2478de706..07f037a16e13 100644
@@ -139,6 +139,9 @@ deltabase::
 	given object, if it is stored as a delta.  Otherwise it
 	expands to the null object name (all zeroes).
 
+notes::
+	The notes associated with the ref.
+
 upstream::
 	The name of a local ref which can be considered ``upstream''
 	from the displayed ref. Respects `:short`, `:lstrip` and

@@ -302,6 +305,7 @@ git for-each-ref --count=3 --sort='-*authordate' \
 Subject: %(*subject)
 Date: %(*authordate)
 Ref: %(*refname)
+Notes: %(*notes)
 
 %(*body)
 ' 'refs/tags'

ref-filter.c +4 -0

index a0adb4551d87..42a5608a3056 100644
@@ -23,6 +23,7 @@
 #include "worktree.h"
 #include "hashmap.h"
 #include "strvec.h"
+#include "run-command.h"
 
 static struct ref_msg {
 	const char *gone;

@@ -506,6 +507,7 @@ static struct {
 	{ "objectsize", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },
 	{ "objectname", SOURCE_OTHER, FIELD_STR, oid_atom_parser },
 	{ "deltabase", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },
+	{ "notes", SOURCE_OTHER, FIELD_STR },
 	{ "tree", SOURCE_OBJ, FIELD_STR, oid_atom_parser },
 	{ "parent", SOURCE_OBJ, FIELD_STR, oid_atom_parser },
 	{ "numparent", SOURCE_OBJ, FIELD_ULONG },

@@ -953,6 +955,24 @@ static int grab_oid(const char *name, const char *field, const struct object_id
 	return 0;
 }
 
+static int grab_notes(const struct object_id *oid, struct atom_value *v)
+{

ref-filter.c +15 -2

+	strbuf_trim_trailing_newline(&out);
+	v->s = strbuf_detach(&out, NULL);
+	strbuf_release(&err);
+	return ret;
+}
+
 /* See grab_values */
 static void grab_common_values(struct atom_value *val, int deref, struct expand_data *oi)
 {

@@ -975,8 +995,12 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
 			v->s = xstrfmt("%"PRIuMAX , (uintmax_t)oi->size);
 		} else if (!strcmp(name, "deltabase"))
 			v->s = xstrdup(oid_to_hex(&oi->delta_base_oid));
-		else if (deref)
-			grab_oid(name, "objectname", &oi->oid, v, &used_atom[i]);
+		else if (deref) {
+			if (!strcmp(name, "notes"))
+				grab_notes(&oi->oid, v);
+			else
+				grab_oid(name, "objectname", &oi->oid, v, &used_atom[i]);
+		}
 	}
 }
 

@@ -1767,6 +1791,9 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 			continue;
 		} else if (!deref && grab_oid(name, "objectname", &ref->objectname, v, atom)) {
 			continue;
+		} else if (!deref && !strcmp(name, "notes")) {
+			grab_notes(&ref->objectname, v);
+			continue;
 		} else if (!strcmp(name, "HEAD")) {
 			if (atom->u.head && !strcmp(ref->refname, atom->u.head))
 				v->s = xstrdup("*");

Am 29.04.21 um 12:24 schrieb ZheNing Hu via GitGitGadget:

Quoted 7 lines

From: ZheNing Hu <[email protected]>

Note that --pretty="%N" can view notes related to the commit. So add %(notes) to ref-filter seem is a good choice. This atom %(notes) view the notes associated with the ref, and support dereference.

I can't judge the usefulness of this feature because I don't use notes.

Quoted 7 lines

Signed-off-by ZheNing Hu <[email protected]>

---
[GSOC] ref-filter: add %(notes) format atom

An important step in the GSOC project Use ref-filter formats in git cat-file is to integrate different format atoms into the ref-filter. Olga and Hariom have also made a lot of efforts in this area.

This is done to replace the custom format parser in git cat-file with ref-filter in order to reduce code duplication, right?

Quoted 3 lines

Currently, I noticed that there may be some format atoms in "pretty.c" that have not been migrated to ref-filter, such as --pretty="%N", --pretty="%(describe)".

git cat-file doesn't support pretty format atoms, so I'm not sure I see the connection here.

Quoted 68 lines

So in this patch, I tried to migrate --pretty=%N to --format=%(notes).

Hope this will be hopeful !!!

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-944%2Fadlternative%2Fformat-notes-atom-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-944/adlternative/format-notes-atom-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/944

 Documentation/git-for-each-ref.txt |  4 ++++
 ref-filter.c                       | 31 ++++++++++++++++++++++++++++--
 t/t6300-for-each-ref.sh            | 10 ++++++++++
 3 files changed, 43 insertions(+), 2 deletions(-)

Documentation/git-for-each-ref.txt +4 -0

index 2ae2478de706..07f037a16e13 100644
@@ -139,6 +139,9 @@ deltabase::
      given object, if it is stored as a delta.  Otherwise it
      expands to the null object name (all zeroes).
 
+notes::
+     The notes associated with the ref.
+
 upstream::
      The name of a local ref which can be considered ``upstream''
      from the displayed ref. Respects `:short`, `:lstrip` and

@@ -302,6 +305,7 @@ git for-each-ref --count=3 --sort='-*authordate' \
 Subject: %(*subject)
 Date: %(*authordate)
 Ref: %(*refname)
+Notes: %(*notes)
 
 %(*body)
 ' 'refs/tags'

ref-filter.c +11 -0

index a0adb4551d87..42a5608a3056 100644
@@ -23,6 +23,7 @@
 #include "worktree.h"
 #include "hashmap.h"
 #include "strvec.h"
+#include "run-command.h"
 
 static struct ref_msg {
      const char *gone;

@@ -506,6 +507,7 @@ static struct {
      { "objectsize", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },
      { "objectname", SOURCE_OTHER, FIELD_STR, oid_atom_parser },
      { "deltabase", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },
+     { "notes", SOURCE_OTHER, FIELD_STR },
      { "tree", SOURCE_OBJ, FIELD_STR, oid_atom_parser },
      { "parent", SOURCE_OBJ, FIELD_STR, oid_atom_parser },
      { "numparent", SOURCE_OBJ, FIELD_ULONG },

@@ -953,6 +955,24 @@ static int grab_oid(const char *name, const char *field, const struct object_id
      return 0;
 }
 
+static int grab_notes(const struct object_id *oid, struct atom_value *v)
+{
+     struct child_process cmd = CHILD_PROCESS_INIT;
+     struct strbuf out = STRBUF_INIT;
+     struct strbuf err = STRBUF_INIT;
+     const char *args[] = { "notes", "show", NULL };
+     int ret;
+
+     cmd.git_cmd = 1;

ref-filter.c static int grab_oid(const char *name, const char *field, const struct object_id · line 969

Quoted 3 lines

ref-filter.c +3 -0

+     strvec_pushv(&cmd.args, args);
+     strvec_push(&cmd.args, oid_to_hex(oid));
+     ret = pipe_command(&cmd, NULL, 0, &out, 0, &err, 0);

Nice prototype. Is it possible to avoid the overhead of calling git notes as an external process by calling load_display_notes() once at parse time and format_display_notes() for each item? Would it cause conflicts for code that uses ref-filter and handles notes already?

Note that cat-file --batch should be a superset of ref-filter and --pretty, because
it supports all objects, while ref-filter only supports "commit" and "tag" two kinds
of objects, and --pretty only supports "commit" one kind of object.

So it may be reasonable to provide %(notes) to "commit","tag" objects in cat-file --batch
in the future, now integrate them into ref-filter firstly.

Quoted 1 line

Quoted 1 line

So in this patch, I tried to migrate --pretty=%N to --format=%(notes).

Quoted 10 lines

Documentation/git-for-each-ref.txt deltabase:: · line 143

Quoted 10 lines

Documentation/git-for-each-ref.txt +2 -0

index 2ae2478de706..07f037a16e13 100644
@@ -139,6 +139,9 @@ deltabase::
      given object, if it is stored as a delta.  Otherwise it
      expands to the null object name (all zeroes).
 
+notes::
+     The notes associated with the ref.

Quoted 5 lines · ZheNing Hu

Note that cat-file --batch should be a superset of ref-filter and --pretty, because
it supports all objects, while ref-filter only supports "commit" and "tag" two kinds
of objects, and --pretty only supports "commit" one kind of object.

What? A ref can point at any kind of objects, not necessarily commits and tags. %(objectname), %(objecttype), etc. obviously are applicable to any type of object.

Another thing worth noting is that ref-filter needs to deal with traits that are not tied to any particular object, but to the ref itself, like who its upstream is and where it would be pushed to.

Quoted 3 lines

So it may be reasonable to provide %(notes) to "commit","tag" objects in cat-file --batch
in the future, now integrate them into ref-filter firstly.

And you can attach notes to objects of any type, not limited to commits and tags.

Quoted 1 line

Quoted 1 line

Quoted 1 line

So in this patch, I tried to migrate --pretty=%N to --format=%(notes).

What do you mean by "migrate"? Are you making both available?

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Documentation/git-for-each-ref.txt +0 -0

index 2ae2478de706..07f037a16e13 100644
@@ -139,6 +139,9 @@ deltabase::
      given object, if it is stored as a delta.  Otherwise it
      expands to the null object name (all zeroes).
 

Quoted 2 lines

Quoted 2 lines

Documentation/git-for-each-ref.txt deltabase:: · line 143

Quoted 2 lines

Documentation/git-for-each-ref.txt +2 -0

+notes::
+     The notes associated with the ref.

"The notes associated with the object pointed at by the ref".

A note is never associated with a ref. It can only associated with an object.

If we are going to have %(notes), it may be natural to desire grabbing notes for the given object from specified notes ref, e.g.

    git for-each-ref --format="%(notes:amlog) %(subject)" zh/pretty-date-human

may want to give the same kind of information as

    git show -s --format='%N %s' --notes=amlog zh/pretty-date-human

The underlying notes machinery however may not be prepared to work with more than one notes tree at the same time, so

    git for-each-ref --format="%(notes) %(subject)" --notes=amlog zh/pretty-date-human

might be a better syntax, as --format="%(notes:X) %(notes:Y)" may not be something you can easily support.