Skip to contentSkip to conversation

ref-filter: add %(notes) atom and --notes

RFC PATCH New ZheNing Hu via GitGitGadget 1 participant 1 message May 15, 2021 Documentation

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 ref-filter: add %(notes) atom and --notes

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. New atom %(notes) can show the notes associated with the object pointed at by the ref. At the same time we can use %(*notes) to show the notes of the object which dereferenced by the ref.

Add --notes=<ref> option and --no-notes option to git for-each-ref. Multiple --notes=<ref> options can be combined to control which notes are being displayed. --no-notes will reset the list of notes refs from which notes are shown.

By default, git for-each-ref --format="%(notes)" will output notes of "refs/notes/commits", and in the case of using --notes=foo, will suppress the output notes of refs/notes/commits, unless use --notes=refs/notes/commits at the same time, "refs/notes/commit"'s notes will output. Therefore, the semantics of --notes=<ref> here are different from that of git log --notes=<ref>.

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

In https://lore.kernel.org/git/[email protected]/

Junio think that %(notes) atom can be used with --notes. René Scharfe think %(notes) can make use of load_display_notes() and format_display_notes() to reduce overhead.

So in this new version of this patch, I may have implemented this feature in some troublesome ways.

First, I parsed --notes and --no-notes in for-each-ref.c. use new struct notes_info as a carrier for parsing --notes, The parsed notes_info is passed to ref-filter.c through a global variable "struct notes_info notes_info".

Then in ref-filter.c I use notes_atom_parser() to parse %(notes), the parsing of all notes atoms only calls load_display_notes() once.

Finally, use grab_notes() to get the notes corresponding to the object.

However, the current implementation is still not elegant enough,

First, can we directly learn the processing of --notes and --no-notes in revision.c? It is very cumbersome to make new --notes and --no-notes in for-each-ref.c. Since we need to output "refs/notes/commits" notes by default when using --format="%(notes)", should we reject the default notes output of "refs/notes/commits" when using --notes=<ref> as it has been done now? And here --notes=<ref> does not support --notes (no ref), is there any good way to implement it?

Second, I can not easily make struct notes_info notes_info a member of struct ref_format format, because as a parameter of *_atom_parser in ref-flter.c, const struct ref_format *format is used to prevent us from modifying it. However, struct notes_info notes_info is a variable that needs to be modified. So here I have to use a global variable to pass it (very urgly).

Third, I am not sure whether the "raw" parameter of format_display_notes() should be passed 0 or 1.

So, I feel very confused, any good suggestions?

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

 Documentation/git-for-each-ref.txt | 14 +++++++
 builtin/for-each-ref.c             | 26 ++++++++++++
 ref-filter.c                       | 35 +++++++++++++++-
 ref-filter.h                       |  7 ++++
 t/t6300-for-each-ref.sh            | 64 ++++++++++++++++++++++++++++++
 5 files changed, 144 insertions(+), 2 deletions(-)
Documentation/git-for-each-ref.txt +14 -0
index 2ae2478de706..1157e8dda63d 100644
@@ -13,6 +13,7 @@ SYNOPSIS
 		   [--points-at=<object>]
 		   [--merged[=<object>]] [--no-merged[=<object>]]
 		   [--contains[=<object>]] [--no-contains[=<object>]]
+		   [--no-notes | --notes=<ref>]
 
 DESCRIPTION
 -----------
@@ -57,6 +58,15 @@ OPTIONS
 	`xx`; for example `%00` interpolates to `\0` (NUL),
 	`%09` to `\t` (TAB) and `%0a` to `\n` (LF).
 
+--notes=<ref>::
+	Show the notes that annotate the object. With an optional '<ref>' argument,
+	use the ref to find the notes to display.
+	(see linkgit:git-notes[1] and linkgit:pretty-options.txt[])
+--no-notes::
+	Do not show notes. This negates the above `--notes` option, by
+	resetting the list of notes refs from which notes are shown.
+	(see linkgit:git-notes[1] and linkgit:pretty-options.txt[])
+
 --color[=<when>]::
 	Respect any colors specified in the `--format` option. The
 	`<when>` field must be one of `always`, `never`, or `auto` (if
@@ -139,6 +149,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 object pointed at by the ref.
+
 upstream::
 	The name of a local ref which can be considered ``upstream''
 	from the displayed ref. Respects `:short`, `:lstrip` and
@@ -302,6 +315,7 @@ git for-each-ref --count=3 --sort='-*authordate' \
 Subject: %(*subject)
 Date: %(*authordate)
 Ref: %(*refname)
+Notes: %(*notes)
 
 %(*body)
 ' 'refs/tags'
builtin/for-each-ref.c +26 -0
index 89cb6307d46f..12e6e673d48e 100644
@@ -14,6 +14,26 @@ static char const * const for_each_ref_usage[] = {
 	NULL
 };
 
+extern struct notes_info notes_info;
+
+int parse_opt_notes(const struct option *opt, const char *arg, int unset)
+{
+	struct notes_info *ni = opt->value;
+
+	if (unset) {
+		disable_display_notes(&ni->notes_option, &ni->show_notes);
+		return 0;
+	}
+
+	if (!arg)
+		return -1;
+
+	enable_ref_display_notes(&ni->notes_option, &ni->show_notes, arg);
+	ni->notes_option.use_default_notes = 0;
+
+	return 0;
+}
+
 int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 {
 	int i;
@@ -38,6 +58,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(""),
 		OPT_INTEGER( 0 , "count", &maxcount, N_("show only <n> matched refs")),
 		OPT_STRING(  0 , "format", &format.format, N_("format"), N_("format to use for the output")),
+		OPT_CALLBACK(0, "notes", &notes_info, N_("notes"), N_("the notes associated"
+			     "with the object pointed at by the ref"), parse_opt_notes),
 		OPT__COLOR(&format.use_color, N_("respect format colors")),
 		OPT_REF_SORT(sorting_tail),
 		OPT_CALLBACK(0, "points-at", &filter.points_at,
@@ -51,6 +73,9 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_END(),
 	};
 
+	init_display_notes(&notes_info.notes_option);
+	enable_default_display_notes(&notes_info.notes_option, &notes_info.show_notes);
+
 	memset(&array, 0, sizeof(array));
 	memset(&filter, 0, sizeof(filter));
 
@@ -97,5 +122,6 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	free_commit_list(filter.with_commit);
 	free_commit_list(filter.no_commit);
 	UNLEAK(sorting);
+	string_list_clear(&notes_info.notes_option.extra_notes_refs, 0);
 	return 0;
 }
ref-filter.c +33 -2
index e2eac50d9508..ffcc1e500ed3 100644
@@ -84,6 +84,8 @@ static struct expand_data {
 	struct object_info info;
 } oi, oi_deref;
 
+struct notes_info notes_info;
+
 struct ref_to_worktree_entry {
 	struct hashmap_entry ent;
 	struct worktree *wt; /* key is wt->head_ref */
@@ -295,6 +297,16 @@ static int deltabase_atom_parser(const struct ref_format *format, struct used_at
 	return 0;
 }
 
+static int notes_atom_parser(const struct ref_format *format, struct used_atom *atom,
+				 const char *arg, struct strbuf *err)
+{
+	if(!notes_info.show_notes_given && notes_info.show_notes) {
+			load_display_notes(&notes_info.notes_option);
+			notes_info.show_notes_given = 1;
+	}
+	return 0;
+}
+
 static int body_atom_parser(const struct ref_format *format, struct used_atom *atom,
 			    const char *arg, struct strbuf *err)
 {
@@ -506,6 +518,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, notes_atom_parser },
 	{ "tree", SOURCE_OBJ, FIELD_STR, oid_atom_parser },
 	{ "parent", SOURCE_OBJ, FIELD_STR, oid_atom_parser },
 	{ "numparent", SOURCE_OBJ, FIELD_ULONG },
@@ -953,6 +966,17 @@ static int grab_oid(const char *name, const char *field, const struct object_id
 	return 0;
 }
 
+static void grab_notes(const struct object_id *oid, struct atom_value *v)
+{
+	struct strbuf notebuf = STRBUF_INIT;
+
+	if (notes_info.show_notes)
+			format_display_notes(oid, &notebuf,
+			     get_log_output_encoding(), 1);
+	strbuf_trim_trailing_newline(&notebuf);
+	v->s = strbuf_detach(&notebuf, NULL);
+}
+
 /* See grab_values */
 static void grab_common_values(struct atom_value *val, int deref, struct expand_data *oi)
 {
@@ -975,8 +999,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 +1795,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("*");
ref-filter.h +7 -0
index baf72a718965..b2f97bbd164e 100644
@@ -5,6 +5,7 @@
 #include "refs.h"
 #include "commit.h"
 #include "parse-options.h"
+#include "notes.h"
 
 /* Quoting styles */
 #define QUOTE_NONE 0
@@ -85,6 +86,12 @@ struct ref_format {
 
 #define REF_FORMAT_INIT { NULL, 0, -1 }
 
+struct notes_info {
+	int show_notes;
+	int show_notes_given;
+	struct display_notes_opt notes_option;
+};
+
 /*  Macros for checking --merged and --no-merged options */
 #define _OPT_MERGED_NO_MERGED(option, filter, h) \
 	{ OPTION_CALLBACK, 0, option, (filter), N_("commit"), (h), \
t/t6300-for-each-ref.sh +64 -0

120 lines not on this page · raw message

base-commit: 97eea85a0a1ec66d356567808a1e4ca2367e0ce7

Signature · 1 line
gitgitgadget