{"thread":{"id":"55647","subject":"[PATCH 0/2] [GSOC][RFC] ref-filter: introduce enum atom_type","startedAt":"2021-05-08T15:22:37Z","lastAt":"2021-05-13T15:15:51Z","messageCount":27,"participants":["ZheNing Hu via GitGitGadget","Christian Couder","Junio C Hamano","ZheNing Hu"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"423931","messageId":"pull.951.git.1620487353.gitgitgadget@gmail.com","threadId":"55647","inReplyTo":null,"subject":"[PATCH 0/2] [GSOC][RFC] ref-filter: introduce enum atom_type","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-08T15:22:30Z","receivedAt":"2021-05-08T15:22:37Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Firstly, let the union used_atom.u add a member \"objectsize\",which learn to\nrecord the attributes of \"objectsize\", this will bring an extension line for\natom \"%(objectsize)\". Next patch will base on this feature to better support\nits functions.\n\nSecondly, In\nhttps://lore.kernel.org/git/CAOLTT8RhVZQJX8z1gY5UM1jv0imZ4K9UnD14MgJFfvqBBiAZQg@mail.gmail.com/\nI and Junio discussed the use of enum atom_type in used_atom to represent\nthe specific type of union used_atom.u, which can correctly distinguish atom\ntypes and reduce the overhead of string matching.\n\nBut after my attempts, I found that our used_atom is better using enum to\nremember the type of valid_atom item instead of remember type of\nused_atom.u, which can be used throughout the ref-filter, easily distinguish\ndifferent types of atoms through a used_atom[i].atom_type, and reduce the\noverhead of string matching.\n\nWhat's left: some function like grab_oid() or grab_person(), they can also\ntake advantage of this feature, but require some additional modifications.\n\nWhat's for future: Let used_atom entry can index directly by enum atom_type,\nIn order to reduce the very many loops in ref-filter.\n\nE.g.\n\nstatic struct used_atom_cache { enum atom_type atom_type; struct used_atom\n*used_atom; } used_atom_cache[] = { {ATOM_REFNAME, NULL}, {ATOM_OBJECTTYPE,\nNULL}, ... };\n\nIf want check whether used_atom entries have a atom \"%(refname)\", we can\njust check if\nused_atom_cache[ATOM_REFNAME].used_atom is equal to NULL, the time\ncomplexity is O(1) and the complexity of the original method is O(n). This\nwill once again get a performance boost for ref-filter.\n\nZheNing Hu (2):\n [GSOC] ref-filter: add objectsize to used_atom\n [GSOC][RFC] ref-filter: introduce enum atom_type\n\n ref-filter.c | 209 +++++++++++++++++++++++++++++++++------------------\n 1 file changed, 135 insertions(+), 74 deletions(-)\n\n\nbase-commit: 7e391989789db82983665667013a46eabc6fc570\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-951%2Fadlternative%2Fref-filter-atom-type-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-951/adlternative/ref-filter-atom-type-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/951\n-- \ngitgitgadget\n"},{"id":"423932","messageId":"91ca57c9d04a822aa4955dbfe3962a6fb2444e81.1620487353.git.gitgitgadget@gmail.com","threadId":"55647","inReplyTo":"pull.951.git.1620487353.gitgitgadget@gmail.com","subject":"[PATCH 1/2] [GSOC] ref-filter: add objectsize to used_atom","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-08T15:22:31Z","receivedAt":"2021-05-08T15:22:40Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nSince \"objectsize:size\" is composed of two parts,\n\"type:attribute\". However, the original implementation\ndid not decouple the two parts \"type\" and \"attribute\" well,\nwe still need to judge separately whether the atom is\n\"objectsize\" or \"objectsize:disk\" in `grab_common_values()`.\n\nAdd a new member `objectsize` to the union `used_atom.u`,\nso that we can separate the judgment of atom type from the\njudgment of atom attribute, This will bring scalability to\natom `%(objectsize)`.\n\nSigned-off-by: ZheNing Hu \n---\n ref-filter.c | 19 +++++++++++++------\n 1 file changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex a0adb4551d87..f420bae6e5ba 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -146,6 +146,9 @@ static struct used_atom {\n \t\t\tenum { O_FULL, O_LENGTH, O_SHORT } option;\n \t\t\tunsigned int length;\n \t\t} oid;\n+\t\tstruct {\n+\t\t\tenum { O_SIZE, O_SIZE_DISK } option;\n+\t\t} objectsize;\n \t\tstruct email_option {\n \t\t\tenum { EO_RAW, EO_TRIM, EO_LOCALPART } option;\n \t\t} email_option;\n@@ -269,11 +272,13 @@ static int objectsize_atom_parser(const struct ref_format *format, struct used_a\n \t\t\t\t const char *arg, struct strbuf *err)\n {\n \tif (!arg) {\n+\t\tatom->u.objectsize.option = O_SIZE;\n \t\tif (*atom->name == '*')\n \t\t\toi_deref.info.sizep = &oi_deref.size;\n \t\telse\n \t\t\toi.info.sizep = &oi.size;\n \t} else if (!strcmp(arg, \"disk\")) {\n+\t\tatom->u.objectsize.option = O_SIZE_DISK;\n \t\tif (*atom->name == '*')\n \t\t\toi_deref.info.disk_sizep = &oi_deref.disk_size;\n \t\telse\n@@ -967,12 +972,14 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \t\t\tname++;\n \t\tif (!strcmp(name, \"objecttype\"))\n \t\t\tv->s = xstrdup(type_name(oi->type));\n-\t\telse if (!strcmp(name, \"objectsize:disk\")) {\n-\t\t\tv->value = oi->disk_size;\n-\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n-\t\t} else if (!strcmp(name, \"objectsize\")) {\n-\t\t\tv->value = oi->size;\n-\t\t\tv->s = xstrfmt(\"%\"PRIuMAX , (uintmax_t)oi->size);\n+\t\telse if (starts_with(name, \"objectsize\")) {\n+\t\t\tif (used_atom[i].u.objectsize.option == O_SIZE_DISK) {\n+\t\t\t\tv->value = oi->disk_size;\n+\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n+\t\t\t} else if (used_atom[i].u.objectsize.option == O_SIZE) {\n+\t\t\t\tv->value = oi->size;\n+\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX , (uintmax_t)oi->size);\n+\t\t\t}\n \t\t} else if (!strcmp(name, \"deltabase\"))\n \t\t\tv->s = xstrdup(oid_to_hex(&oi->delta_base_oid));\n \t\telse if (deref)\n-- \ngitgitgadget\n\n"},{"id":"423933","messageId":"3770df1829830229e768607b699730d2a7c21c5e.1620487353.git.gitgitgadget@gmail.com","threadId":"55647","inReplyTo":"pull.951.git.1620487353.gitgitgadget@gmail.com","subject":"[PATCH 2/2] [GSOC][RFC] ref-filter: introduce enum atom_type","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-08T15:22:32Z","receivedAt":"2021-05-08T15:22:41Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nIn the original ref-filter design, it will copy the parsed\natom's name and attributes to `used_atom[i].name` in the\natom's parsing step, and use it again for string matching\nin the later specific ref attributes filling step. It use\na lot of string matching to determine which atom we need.\n\nIntroduce the enum member `valid_atom.atom_type` which\nrecord type of each valid_atom, in the first step of the\natom parsing, `used_atom.atom_type` will record corresponding\nenum value from `valid_atom.atom_type`, and then in specific\nreference attribute filling step, only need to compare the\nvalue of the `used_atom.atom_type` to judge the atom type.\n\nAt the same time, The value of an atom_type is the coordinate\nof its corresponding valid_atom entry, we can quickly index\nto the corresponding valid_atom entry by the atom_type value.\n\nSigned-off-by: ZheNing Hu \n---\n ref-filter.c | 192 +++++++++++++++++++++++++++++++++------------------\n 1 file changed, 123 insertions(+), 69 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex f420bae6e5ba..4ce46e70dc99 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -108,6 +108,50 @@ static struct ref_to_worktree_map {\n \tstruct worktree **worktrees;\n } ref_to_worktree_map;\n \n+enum atom_type {\n+ATOM_REFNAME,\n+ATOM_OBJECTTYPE,\n+ATOM_OBJECTSIZE,\n+ATOM_OBJECTNAME,\n+ATOM_DELTABASE,\n+ATOM_TREE,\n+ATOM_PARENT,\n+ATOM_NUMPARENT,\n+ATOM_OBJECT,\n+ATOM_TYPE,\n+ATOM_TAG,\n+ATOM_AUTHOR,\n+ATOM_AUTHORNAME,\n+ATOM_AUTHOREMAIL,\n+ATOM_AUTHORDATE,\n+ATOM_COMMITTER,\n+ATOM_COMMITTERNAME,\n+ATOM_COMMITTEREMAIL,\n+ATOM_COMMITTERDATE,\n+ATOM_TAGGER,\n+ATOM_TAGGERNAME,\n+ATOM_TAGGEREMAIL,\n+ATOM_TAGGERDATE,\n+ATOM_CREATOR,\n+ATOM_CREATORDATE,\n+ATOM_SUBJECT,\n+ATOM_BODY,\n+ATOM_TRAILERS,\n+ATOM_CONTENTS,\n+ATOM_UPSTREAM,\n+ATOM_PUSH,\n+ATOM_SYMREF,\n+ATOM_FLAG,\n+ATOM_HEAD,\n+ATOM_COLOR,\n+ATOM_WORKTREEPATH,\n+ATOM_ALIGN,\n+ATOM_END,\n+ATOM_IF,\n+ATOM_THEN,\n+ATOM_ELSE,\n+};\n+\n /*\n * An atom is a valid field atom listed below, possibly prefixed with\n * a \"*\" to denote deref_tag().\n@@ -122,6 +166,7 @@ static struct used_atom {\n \tconst char *name;\n \tcmp_type type;\n \tinfo_source source;\n+\tenum atom_type atom_type;\n \tunion {\n \t\tchar color[COLOR_MAXLEN];\n \t\tstruct align align;\n@@ -500,53 +545,54 @@ static int head_atom_parser(const struct ref_format *format, struct used_atom *a\n }\n \n static struct {\n+\tenum atom_type atom_type;\n \tconst char *name;\n \tinfo_source source;\n \tcmp_type cmp_type;\n \tint (*parser)(const struct ref_format *format, struct used_atom *atom,\n \t\t const char *arg, struct strbuf *err);\n } valid_atom[] = {\n-\t{ \"refname\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n-\t{ \"objecttype\", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser },\n-\t{ \"objectsize\", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },\n-\t{ \"objectname\", SOURCE_OTHER, FIELD_STR, oid_atom_parser },\n-\t{ \"deltabase\", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },\n-\t{ \"tree\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n-\t{ \"parent\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n-\t{ \"numparent\", SOURCE_OBJ, FIELD_ULONG },\n-\t{ \"object\", SOURCE_OBJ },\n-\t{ \"type\", SOURCE_OBJ },\n-\t{ \"tag\", SOURCE_OBJ },\n-\t{ \"author\", SOURCE_OBJ },\n-\t{ \"authorname\", SOURCE_OBJ },\n-\t{ \"authoremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n-\t{ \"authordate\", SOURCE_OBJ, FIELD_TIME },\n-\t{ \"committer\", SOURCE_OBJ },\n-\t{ \"committername\", SOURCE_OBJ },\n-\t{ \"committeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n-\t{ \"committerdate\", SOURCE_OBJ, FIELD_TIME },\n-\t{ \"tagger\", SOURCE_OBJ },\n-\t{ \"taggername\", SOURCE_OBJ },\n-\t{ \"taggeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n-\t{ \"taggerdate\", SOURCE_OBJ, FIELD_TIME },\n-\t{ \"creator\", SOURCE_OBJ },\n-\t{ \"creatordate\", SOURCE_OBJ, FIELD_TIME },\n-\t{ \"subject\", SOURCE_OBJ, FIELD_STR, subject_atom_parser },\n-\t{ \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n-\t{ \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n-\t{ \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n-\t{ \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n-\t{ \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n-\t{ \"symref\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n-\t{ \"flag\", SOURCE_NONE },\n-\t{ \"HEAD\", SOURCE_NONE, FIELD_STR, head_atom_parser },\n-\t{ \"color\", SOURCE_NONE, FIELD_STR, color_atom_parser },\n-\t{ \"worktreepath\", SOURCE_NONE },\n-\t{ \"align\", SOURCE_NONE, FIELD_STR, align_atom_parser },\n-\t{ \"end\", SOURCE_NONE },\n-\t{ \"if\", SOURCE_NONE, FIELD_STR, if_atom_parser },\n-\t{ \"then\", SOURCE_NONE },\n-\t{ \"else\", SOURCE_NONE },\n+\t{ ATOM_REFNAME, \"refname\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n+\t{ ATOM_OBJECTTYPE, \"objecttype\", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser },\n+\t{ ATOM_OBJECTSIZE, \"objectsize\", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },\n+\t{ ATOM_OBJECTNAME, \"objectname\", SOURCE_OTHER, FIELD_STR, oid_atom_parser },\n+\t{ ATOM_DELTABASE, \"deltabase\", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },\n+\t{ ATOM_TREE, \"tree\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n+\t{ ATOM_PARENT, \"parent\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n+\t{ ATOM_NUMPARENT, \"numparent\", SOURCE_OBJ, FIELD_ULONG },\n+\t{ ATOM_OBJECT, \"object\", SOURCE_OBJ },\n+\t{ ATOM_TYPE, \"type\", SOURCE_OBJ },\n+\t{ ATOM_TAG, \"tag\", SOURCE_OBJ },\n+\t{ ATOM_AUTHOR, \"author\", SOURCE_OBJ },\n+\t{ ATOM_AUTHORNAME, \"authorname\", SOURCE_OBJ },\n+\t{ ATOM_AUTHOREMAIL, \"authoremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n+\t{ ATOM_AUTHORDATE, \"authordate\", SOURCE_OBJ, FIELD_TIME },\n+\t{ ATOM_COMMITTER, \"committer\", SOURCE_OBJ },\n+\t{ ATOM_COMMITTERNAME, \"committername\", SOURCE_OBJ },\n+\t{ ATOM_COMMITTEREMAIL, \"committeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n+\t{ ATOM_COMMITTERDATE, \"committerdate\", SOURCE_OBJ, FIELD_TIME },\n+\t{ ATOM_TAGGER, \"tagger\", SOURCE_OBJ },\n+\t{ ATOM_TAGGERNAME, \"taggername\", SOURCE_OBJ },\n+\t{ ATOM_TAGGEREMAIL, \"taggeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n+\t{ ATOM_TAGGERDATE, \"taggerdate\", SOURCE_OBJ, FIELD_TIME },\n+\t{ ATOM_CREATOR, \"creator\", SOURCE_OBJ },\n+\t{ ATOM_CREATORDATE, \"creatordate\", SOURCE_OBJ, FIELD_TIME },\n+\t{ ATOM_SUBJECT, \"subject\", SOURCE_OBJ, FIELD_STR, subject_atom_parser },\n+\t{ ATOM_BODY, \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n+\t{ ATOM_TRAILERS, \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n+\t{ ATOM_CONTENTS, \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n+\t{ ATOM_UPSTREAM, \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n+\t{ ATOM_PUSH, \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n+\t{ ATOM_SYMREF, \"symref\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n+\t{ ATOM_FLAG, \"flag\", SOURCE_NONE },\n+\t{ ATOM_HEAD, \"HEAD\", SOURCE_NONE, FIELD_STR, head_atom_parser },\n+\t{ ATOM_COLOR, \"color\", SOURCE_NONE, FIELD_STR, color_atom_parser },\n+\t{ ATOM_WORKTREEPATH, \"worktreepath\", SOURCE_NONE },\n+\t{ ATOM_ALIGN, \"align\", SOURCE_NONE, FIELD_STR, align_atom_parser },\n+\t{ ATOM_END, \"end\", SOURCE_NONE },\n+\t{ ATOM_IF, \"if\", SOURCE_NONE, FIELD_STR, if_atom_parser },\n+\t{ ATOM_THEN, \"then\", SOURCE_NONE },\n+\t{ ATOM_ELSE, \"else\", SOURCE_NONE },\n \t/*\n \t * Please update $__git_ref_fieldlist in git-completion.bash\n \t * when you add new atoms\n@@ -628,6 +674,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n \tat = used_atom_cnt;\n \tused_atom_cnt++;\n \tREALLOC_ARRAY(used_atom, used_atom_cnt);\n+\tused_atom[at].atom_type = valid_atom[i].atom_type;\n \tused_atom[at].name = xmemdupz(atom, ep - atom);\n \tused_atom[at].type = valid_atom[i].cmp_type;\n \tused_atom[at].source = valid_atom[i].source;\n@@ -652,7 +699,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n \t\treturn -1;\n \tif (*atom == '*')\n \t\tneed_tagged = 1;\n-\tif (!strcmp(valid_atom[i].name, \"symref\"))\n+\tif (valid_atom[i].atom_type == ATOM_SYMREF)\n \t\tneed_symref = 1;\n \treturn at;\n }\n@@ -965,14 +1012,15 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n-\t\tif (!strcmp(name, \"objecttype\"))\n+\t\tif (atom_type == ATOM_OBJECTTYPE)\n \t\t\tv->s = xstrdup(type_name(oi->type));\n-\t\telse if (starts_with(name, \"objectsize\")) {\n+\t\telse if (atom_type == ATOM_OBJECTSIZE) {\n \t\t\tif (used_atom[i].u.objectsize.option == O_SIZE_DISK) {\n \t\t\t\tv->value = oi->disk_size;\n \t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n@@ -980,9 +1028,9 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \t\t\t\tv->value = oi->size;\n \t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX , (uintmax_t)oi->size);\n \t\t\t}\n-\t\t} else if (!strcmp(name, \"deltabase\"))\n+\t\t} else if (atom_type == ATOM_DELTABASE)\n \t\t\tv->s = xstrdup(oid_to_hex(&oi->delta_base_oid));\n-\t\telse if (deref)\n+\t\telse if (atom_type == ATOM_OBJECTNAME && deref)\n \t\t\tgrab_oid(name, \"objectname\", &oi->oid, v, &used_atom[i]);\n \t}\n }\n@@ -995,16 +1043,17 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n-\t\tif (!strcmp(name, \"tag\"))\n+\t\tif (atom_type == ATOM_TAG)\n \t\t\tv->s = xstrdup(tag->tag);\n-\t\telse if (!strcmp(name, \"type\") && tag->tagged)\n+\t\telse if (atom_type == ATOM_TYPE && tag->tagged)\n \t\t\tv->s = xstrdup(type_name(tag->tagged->type));\n-\t\telse if (!strcmp(name, \"object\") && tag->tagged)\n+\t\telse if (atom_type == ATOM_OBJECT && tag->tagged)\n \t\t\tv->s = xstrdup(oid_to_hex(&tag->tagged->oid));\n \t}\n }\n@@ -1017,18 +1066,20 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n-\t\tif (grab_oid(name, \"tree\", get_commit_tree_oid(commit), v, &used_atom[i]))\n+\t\tif (atom_type == ATOM_TREE &&\n+\t\t grab_oid(name, \"tree\", get_commit_tree_oid(commit), v, &used_atom[i]))\n \t\t\tcontinue;\n-\t\tif (!strcmp(name, \"numparent\")) {\n+\t\tif (atom_type == ATOM_NUMPARENT) {\n \t\t\tv->value = commit_list_count(commit->parents);\n \t\t\tv->s = xstrfmt(\"%lu\", (unsigned long)v->value);\n \t\t}\n-\t\telse if (starts_with(name, \"parent\")) {\n+\t\telse if (atom_type == ATOM_PARENT) {\n \t\t\tstruct commit_list *parents;\n \t\t\tstruct strbuf s = STRBUF_INIT;\n \t\t\tfor (parents = commit->parents; parents; parents = parents->next) {\n@@ -1208,15 +1259,16 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n \t\treturn;\n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n \n-\t\tif (starts_with(name, \"creatordate\"))\n+\t\tif (atom_type == ATOM_CREATORDATE)\n \t\t\tgrab_date(wholine, v, name);\n-\t\telse if (!strcmp(name, \"creator\"))\n+\t\telse if (atom_type == ATOM_CREATOR)\n \t\t\tv->s = copy_line(wholine);\n \t}\n }\n@@ -1696,6 +1748,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t/* Fill in specials first */\n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tstruct used_atom *atom = &used_atom[i];\n+\t\tenum atom_type atom_type = atom->atom_type;\n \t\tconst char *name = used_atom[i].name;\n \t\tstruct atom_value *v = &ref->value[i];\n \t\tint deref = 0;\n@@ -1710,18 +1763,18 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\tname++;\n \t\t}\n \n-\t\tif (starts_with(name, \"refname\"))\n+\t\tif (atom_type == ATOM_REFNAME)\n \t\t\trefname = get_refname(atom, ref);\n-\t\telse if (!strcmp(name, \"worktreepath\")) {\n+\t\telse if (atom_type == ATOM_WORKTREEPATH) {\n \t\t\tif (ref->kind == FILTER_REFS_BRANCHES)\n \t\t\t\tv->s = get_worktree_path(atom, ref);\n \t\t\telse\n \t\t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n \t\t}\n-\t\telse if (starts_with(name, \"symref\"))\n+\t\telse if (atom_type == ATOM_SYMREF)\n \t\t\trefname = get_symref(atom, ref);\n-\t\telse if (starts_with(name, \"upstream\")) {\n+\t\telse if (atom_type == ATOM_UPSTREAM) {\n \t\t\tconst char *branch_name;\n \t\t\t/* only local branches may have an upstream */\n \t\t\tif (!skip_prefix(ref->refname, \"refs/heads/\",\n@@ -1737,7 +1790,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\telse\n \t\t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-\t\t} else if (atom->u.remote_ref.push) {\n+\t\t} else if (atom_type == ATOM_PUSH && atom->u.remote_ref.push) {\n \t\t\tconst char *branch_name;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tif (!skip_prefix(ref->refname, \"refs/heads/\",\n@@ -1756,10 +1809,10 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\tfree((char *)v->s);\n \t\t\tfill_remote_ref_details(atom, refname, branch, &v->s);\n \t\t\tcontinue;\n-\t\t} else if (starts_with(name, \"color:\")) {\n+\t\t} else if (atom_type == ATOM_COLOR) {\n \t\t\tv->s = xstrdup(atom->u.color);\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"flag\")) {\n+\t\t} else if (atom_type == ATOM_FLAG) {\n \t\t\tchar buf[256], *cp = buf;\n \t\t\tif (ref->flag & REF_ISSYMREF)\n \t\t\t\tcp = copy_advance(cp, \",symref\");\n@@ -1772,23 +1825,24 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\t\tv->s = xstrdup(buf + 1);\n \t\t\t}\n \t\t\tcontinue;\n-\t\t} else if (!deref && grab_oid(name, \"objectname\", &ref->objectname, v, atom)) {\n-\t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"HEAD\")) {\n+\t\t} else if (!deref && atom_type == ATOM_OBJECTNAME &&\n+\t\t\t grab_oid(name, \"objectname\", &ref->objectname, v, atom)) {\n+\t\t\t\tcontinue;\n+\t\t} else if (atom_type == ATOM_HEAD) {\n \t\t\tif (atom->u.head && !strcmp(ref->refname, atom->u.head))\n \t\t\t\tv->s = xstrdup(\"*\");\n \t\t\telse\n \t\t\t\tv->s = xstrdup(\" \");\n \t\t\tcontinue;\n-\t\t} else if (starts_with(name, \"align\")) {\n+\t\t} else if (atom_type == ATOM_ALIGN) {\n \t\t\tv->handler = align_atom_handler;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"end\")) {\n+\t\t} else if (atom_type == ATOM_END) {\n \t\t\tv->handler = end_atom_handler;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-\t\t} else if (starts_with(name, \"if\")) {\n+\t\t} else if (atom_type == ATOM_IF) {\n \t\t\tconst char *s;\n \t\t\tif (skip_prefix(name, \"if:\", &s))\n \t\t\t\tv->s = xstrdup(s);\n@@ -1796,11 +1850,11 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\t\tv->s = xstrdup(\"\");\n \t\t\tv->handler = if_atom_handler;\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"then\")) {\n+\t\t} else if (atom_type == ATOM_THEN) {\n \t\t\tv->handler = then_atom_handler;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"else\")) {\n+\t\t} else if (atom_type == ATOM_ELSE) {\n \t\t\tv->handler = else_atom_handler;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-- \ngitgitgadget\n"},{"id":"423954","messageId":"CAP8UFD0mOmqWUS49wywDVoWOyAkCRSUY5+i7Gqq-ZZSNCr-jvg@mail.gmail.com","threadId":"55647","inReplyTo":"3770df1829830229e768607b699730d2a7c21c5e.1620487353.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] [GSOC][RFC] ref-filter: introduce enum atom_type","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2021-05-09T06:21:41Z","receivedAt":"2021-05-09T06:21:56Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sat, May 8, 2021 at 5:22 PM ZheNing Hu via GitGitGadget\n wrote:\n>\n> From: ZheNing Hu \n>\n> In the original ref-filter design, it will copy the parsed\n> atom's name and attributes to `used_atom[i].name` in the\n> atom's parsing step, and use it again for string matching\n> in the later specific ref attributes filling step. It use\n> a lot of string matching to determine which atom we need.\n>\n> Introduce the enum member `valid_atom.atom_type` which\n> record type of each valid_atom, in the first step of the\n> atom parsing, `used_atom.atom_type` will record corresponding\n> enum value from `valid_atom.atom_type`, and then in specific\n> reference attribute filling step, only need to compare the\n> value of the `used_atom.atom_type` to judge the atom type.\n>\n> At the same time, The value of an atom_type is the coordinate\n> of its corresponding valid_atom entry, we can quickly index\n> to the corresponding valid_atom entry by the atom_type value.\n\nI am not sure it's worth having an atom_type field for each valid_atom\nelement if the value of that field is already the index of the\nelement, because then one would always be able to replace\n`valid_atom[i].atom_type` with just `i`. Or is it for some kind of\ntype safety issue?\n\n> Signed-off-by: ZheNing Hu \n> ---\n> ref-filter.c | 192 +++++++++++++++++++++++++++++++++------------------\n> 1 file changed, 123 insertions(+), 69 deletions(-)\n>\n> diff --git a/ref-filter.c b/ref-filter.c\n> index f420bae6e5ba..4ce46e70dc99 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -108,6 +108,50 @@ static struct ref_to_worktree_map {\n> struct worktree **worktrees;\n> } ref_to_worktree_map;\n>\n> +enum atom_type {\n> +ATOM_REFNAME,\n...\n+ATOM_ELSE,\n+};\n\nI wonder if the enum should be instead defined like this:\n\nenum atom_type {\nATOM_UNKNOWN = 0,\nATOM_REFNAME,\n...\nATOM_ELSE,\nATOM_INVALID, /* should be last */\n};\n\nAs a struct containing an atom_type would typically be initialized\nwith 0 after being allocated, `ATOM_UNKNOWN = 0` could ensure that we\ncan easily distinguish such a struct where the atom_type is known from\nsuch a struct where it is unknown yet.\n\nHaving ATOM_INVALID as always the last enum value (even if some new\nones are added later) could help us iterate over the valid atoms using\nsomething like:\n\nfor (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++)\n /* do something with valid_atom[i] */;\n\n> +\n> /*\n> * An atom is a valid field atom listed below, possibly prefixed with\n> * a \"*\" to denote deref_tag().\n> @@ -122,6 +166,7 @@ static struct used_atom {\n> const char *name;\n> cmp_type type;\n> info_source source;\n> + enum atom_type atom_type;\n> union {\n> char color[COLOR_MAXLEN];\n> struct align align;\n> @@ -500,53 +545,54 @@ static int head_atom_parser(const struct ref_format *format, struct used_atom *a\n> }\n>\n> static struct {\n> + enum atom_type atom_type;\n> const char *name;\n> info_source source;\n> cmp_type cmp_type;\n\nI can see that the fields are already not in the same order as in\nstruct used_atom, but my opinion is that it would be better if they\nwould we as much as possible in the same order. Maybe one day we could\neven unify these structs in some way.\n\nAlso as discussed above we might not even need to add an atom_type to\nvalid_atom[].\n\n> int (*parser)(const struct ref_format *format, struct used_atom *atom,\n> const char *arg, struct strbuf *err);\n> } valid_atom[] = {\n\n> @@ -628,6 +674,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n> at = used_atom_cnt;\n> used_atom_cnt++;\n> REALLOC_ARRAY(used_atom, used_atom_cnt);\n> + used_atom[at].atom_type = valid_atom[i].atom_type;\n\nAs discussed above, if the value of an atom_type is the coordinate of\nits corresponding valid_atom entry, then here the following would be\nsimpler:\n\n used_atom[at].atom_type = i;\n\n> used_atom[at].name = xmemdupz(atom, ep - atom);\n> used_atom[at].type = valid_atom[i].cmp_type;\n> used_atom[at].source = valid_atom[i].source;\n> @@ -652,7 +699,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n> return -1;\n> if (*atom == '*')\n> need_tagged = 1;\n> - if (!strcmp(valid_atom[i].name, \"symref\"))\n> + if (valid_atom[i].atom_type == ATOM_SYMREF)\n\nIn the same way as above, the above line might be replaced with the simpler:\n\n if (i == ATOM_SYMREF)\n\n> need_symref = 1;\n> return at;\n> }\n"},{"id":"423957","messageId":"xmqqh7jctiru.fsf@gitster.g","threadId":"55647","inReplyTo":"CAP8UFD0mOmqWUS49wywDVoWOyAkCRSUY5+i7Gqq-ZZSNCr-jvg@mail.gmail.com","subject":"Re: [PATCH 2/2] [GSOC][RFC] ref-filter: introduce enum atom_type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-09T08:26:29Z","receivedAt":"2021-05-09T08:26:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> I am not sure it's worth having an atom_type field for each valid_atom\n> element if the value of that field is already the index of the\n> element, because then one would always be able to replace\n> `valid_atom[i].atom_type` with just `i`. Or is it for some kind of\n> type safety issue?\n\n> I wonder if the enum should be instead defined like this:\n>\n> enum atom_type {\n> ATOM_UNKNOWN = 0,\n> ATOM_REFNAME,\n> ...\n> ATOM_ELSE,\n> ATOM_INVALID, /* should be last */\n> };\n>\n> As a struct containing an atom_type would typically be initialized\n> with 0 after being allocated, `ATOM_UNKNOWN = 0` could ensure that we\n> can easily distinguish such a struct where the atom_type is known from\n> such a struct where it is unknown yet.\n>\n> Having ATOM_INVALID as always the last enum value (even if some new\n> ones are added later) could help us iterate over the valid atoms using\n> something like:\n>\n> for (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++)\n> /* do something with valid_atom[i] */;\n\nOK.\n\nAs to \"safety\", I think it still makes sense to declare \"enum\", but\nI agree that we do not necessarily have to have it in the valid_atom[]\nstruct. We could do something like this instead:\n\n static struct {\n const char *name;\n info_source source;\n cmp_type cmp_type;\n int (*parser)(const struct ref_format *format, struct used_atom *atom,\n const char *arg, struct strbuf *err);\n } valid_atom[] = {\n [ATOM_REFNAME] = { \"refname\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n [ATOM_OBJECTTYPE] = { \"objecttype\", SOURCE_OTHER, FIELD_STR, objecttype...\n [ATOM_OBJECTSIZE] = { \"objectsize\", SOURCE_OTHER, FIELD_ULONG, objectsi...\n ...\n"},{"id":"423963","messageId":"CAOLTT8TGa2U=O6jAOcmqVwiKv6RiSt5j7FuA_XTO=JVi73ts2w@mail.gmail.com","threadId":"55647","inReplyTo":"CAP8UFD0mOmqWUS49wywDVoWOyAkCRSUY5+i7Gqq-ZZSNCr-jvg@mail.gmail.com","subject":"Re: [PATCH 2/2] [GSOC][RFC] ref-filter: introduce enum atom_type","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-05-09T13:40:37Z","receivedAt":"2021-05-09T13:40:56Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Christian Couder 于2021年5月9日周日 下午2:21写道：\n>\n> > At the same time, The value of an atom_type is the coordinate\n> > of its corresponding valid_atom entry, we can quickly index\n> > to the corresponding valid_atom entry by the atom_type value.\n>\n> I am not sure it's worth having an atom_type field for each valid_atom\n> element if the value of that field is already the index of the\n> element, because then one would always be able to replace\n> `valid_atom[i].atom_type` with just `i`. Or is it for some kind of\n> type safety issue?\n>\n\nWell, I think the security issue here is just to allow used_atom and valid_atom\nto be correctly mapped through atom_type. We don’t want the coder to forget to\nupdate \"enum atom_type\" when adding new atoms to valid_atom in the future.\nSo maybe Junio's suggestion is reasonable, we delete the member atom_type in\nvalid_atom, but maintain the connection between atom_type and valid_atom items\nby specifying atom_type as array coordinates.\n\n> I wonder if the enum should be instead defined like this:\n>\n> enum atom_type {\n> ATOM_UNKNOWN = 0,\n> ATOM_REFNAME,\n> ...\n> ATOM_ELSE,\n> ATOM_INVALID, /* should be last */\n> };\n>\n> As a struct containing an atom_type would typically be initialized\n> with 0 after being allocated, `ATOM_UNKNOWN = 0` could ensure that we\n> can easily distinguish such a struct where the atom_type is known from\n> such a struct where it is unknown yet.\n>\n> Having ATOM_INVALID as always the last enum value (even if some new\n> ones are added later) could help us iterate over the valid atoms using\n> something like:\n>\n> for (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++)\n> /* do something with valid_atom[i] */;\n>\n\nThanks, this suggestion is good!\n\n> > +\n> > /*\n> > * An atom is a valid field atom listed below, possibly prefixed with\n> > * a \"*\" to denote deref_tag().\n> > @@ -122,6 +166,7 @@ static struct used_atom {\n> > const char *name;\n> > cmp_type type;\n> > info_source source;\n> > + enum atom_type atom_type;\n> > union {\n> > char color[COLOR_MAXLEN];\n> > struct align align;\n> > @@ -500,53 +545,54 @@ static int head_atom_parser(const struct ref_format *format, struct used_atom *a\n> > }\n> >\n> > static struct {\n> > + enum atom_type atom_type;\n> > const char *name;\n> > info_source source;\n> > cmp_type cmp_type;\n>\n> I can see that the fields are already not in the same order as in\n> struct used_atom, but my opinion is that it would be better if they\n> would we as much as possible in the same order. Maybe one day we could\n> even unify these structs in some way.\n>\n\nYes, atom_value, valid_atom, used_atom, It may be difficult to read for the\nfirst time. Maybe unifying them is a good direction for the future.\n\n> Also as discussed above we might not even need to add an atom_type to\n> valid_atom[].\n>\n\nOK.\n\n> > int (*parser)(const struct ref_format *format, struct used_atom *atom,\n> > const char *arg, struct strbuf *err);\n> > } valid_atom[] = {\n>\n> > @@ -628,6 +674,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n> > at = used_atom_cnt;\n> > used_atom_cnt++;\n> > REALLOC_ARRAY(used_atom, used_atom_cnt);\n> > + used_atom[at].atom_type = valid_atom[i].atom_type;\n>\n> As discussed above, if the value of an atom_type is the coordinate of\n> its corresponding valid_atom entry, then here the following would be\n> simpler:\n>\n> used_atom[at].atom_type = i;\n>\n\nI agree.\n\n> > used_atom[at].name = xmemdupz(atom, ep - atom);\n> > used_atom[at].type = valid_atom[i].cmp_type;\n> > used_atom[at].source = valid_atom[i].source;\n> > @@ -652,7 +699,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n> > return -1;\n> > if (*atom == '*')\n> > need_tagged = 1;\n> > - if (!strcmp(valid_atom[i].name, \"symref\"))\n> > + if (valid_atom[i].atom_type == ATOM_SYMREF)\n>\n> In the same way as above, the above line might be replaced with the simpler:\n>\n> if (i == ATOM_SYMREF)\n>\n> > need_symref = 1;\n> > return at;\n> > }\n\nThanks!\n--\nZheNing Hu\n"},{"id":"423964","messageId":"CAOLTT8Q3QLsD=yy8OBYcHh7FGhfmOebLVk71fidoih-X5HgNsQ@mail.gmail.com","threadId":"55647","inReplyTo":"xmqqh7jctiru.fsf@gitster.g","subject":"Re: [PATCH 2/2] [GSOC][RFC] ref-filter: introduce enum atom_type","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-05-09T13:44:58Z","receivedAt":"2021-05-09T13:45:18Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano 于2021年5月9日周日 下午4:26写道：\n>\n> Christian Couder writes:\n>\n> > I am not sure it's worth having an atom_type field for each valid_atom\n> > element if the value of that field is already the index of the\n> > element, because then one would always be able to replace\n> > `valid_atom[i].atom_type` with just `i`. Or is it for some kind of\n> > type safety issue?\n>\n> > I wonder if the enum should be instead defined like this:\n> >\n> > enum atom_type {\n> > ATOM_UNKNOWN = 0,\n> > ATOM_REFNAME,\n> > ...\n> > ATOM_ELSE,\n> > ATOM_INVALID, /* should be last */\n> > };\n> >\n> > As a struct containing an atom_type would typically be initialized\n> > with 0 after being allocated, `ATOM_UNKNOWN = 0` could ensure that we\n> > can easily distinguish such a struct where the atom_type is known from\n> > such a struct where it is unknown yet.\n> >\n> > Having ATOM_INVALID as always the last enum value (even if some new\n> > ones are added later) could help us iterate over the valid atoms using\n> > something like:\n> >\n> > for (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++)\n> > /* do something with valid_atom[i] */;\n>\n> OK.\n>\n> As to \"safety\", I think it still makes sense to declare \"enum\", but\n> I agree that we do not necessarily have to have it in the valid_atom[]\n> struct. We could do something like this instead:\n>\n> static struct {\n> const char *name;\n> info_source source;\n> cmp_type cmp_type;\n> int (*parser)(const struct ref_format *format, struct used_atom *atom,\n> const char *arg, struct strbuf *err);\n> } valid_atom[] = {\n> [ATOM_REFNAME] = { \"refname\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n> [ATOM_OBJECTTYPE] = { \"objecttype\", SOURCE_OTHER, FIELD_STR, objecttype...\n> [ATOM_OBJECTSIZE] = { \"objectsize\", SOURCE_OTHER, FIELD_ULONG, objectsi...\n> ...\n\nThank! Good suggection. We hope that the atom_type and\nvalid_atom items establish a clear connection. Maybe we\nshould add some comments before the definition of the\n`enum atom_type` to remind the coder of the connection\nbetween atom_type and valid_atom.\n\n--\nZheNing Hu\n"},{"id":"424045","messageId":"pull.951.v2.git.1620659000.gitgitgadget@gmail.com","threadId":"55647","inReplyTo":"pull.951.git.1620487353.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] [GSOC][RFC] ref-filter: introduce enum atom_type","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-10T15:03:17Z","receivedAt":"2021-05-10T15:24:18Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Change from last version:\n\n * Remove the atom_type member from valid_atom at the suggestion of\n Christian and Junio, and use ATOM_* as the coordinates of valid_atom.\n * Add ATOM_INVALID and ATOM_UNKNOWN under Christian's suggestion, we can\n traverse valid_atom through them.\n * Add a note about enum atom_type to remind readers of its role.\n\nZheNing Hu (2):\n [GSOC] ref-filter: add objectsize to used_atom\n [GSOC] ref-filter: introduce enum atom_type\n\n ref-filter.c | 231 ++++++++++++++++++++++++++++++++++-----------------\n 1 file changed, 155 insertions(+), 76 deletions(-)\n\n\nbase-commit: 7e391989789db82983665667013a46eabc6fc570\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-951%2Fadlternative%2Fref-filter-atom-type-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-951/adlternative/ref-filter-atom-type-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/951\n\nRange-diff vs v1:\n\n 1: 91ca57c9d04a = 1: 91ca57c9d04a [GSOC] ref-filter: add objectsize to used_atom\n 2: 3770df182983 ! 2: a1f70b39b7ef [GSOC][RFC] ref-filter: introduce enum atom_type\n @@ Metadata\n Author: ZheNing Hu \n \n ## Commit message ##\n - [GSOC][RFC] ref-filter: introduce enum atom_type\n + [GSOC] ref-filter: introduce enum atom_type\n \n In the original ref-filter design, it will copy the parsed\n atom's name and attributes to `used_atom[i].name` in the\n @@ Commit message\n in the later specific ref attributes filling step. It use\n a lot of string matching to determine which atom we need.\n \n - Introduce the enum member `valid_atom.atom_type` which\n - record type of each valid_atom, in the first step of the\n - atom parsing, `used_atom.atom_type` will record corresponding\n - enum value from `valid_atom.atom_type`, and then in specific\n - reference attribute filling step, only need to compare the\n - value of the `used_atom.atom_type` to judge the atom type.\n + Introduce the enum \"atom_type\", each enum value is named\n + as `ATOM_*`, which is the index of each corresponding\n + valid_atom entry. In the first step of the atom parsing,\n + `used_atom.atom_type` will record corresponding enum value\n + from valid_atom entry index, and then in specific reference\n + attribute filling step, only need to compare the value of\n + the `used_atom.atom_type` to judge the atom type.\n \n - At the same time, The value of an atom_type is the coordinate\n - of its corresponding valid_atom entry, we can quickly index\n - to the corresponding valid_atom entry by the atom_type value.\n + the enum value of `ATOM_UNKNOWN` is equals to zero, which\n + could ensure that we can easily distinguish such a struct\n + where the atom_type is known from such a struct where it\n + is unknown yet.\n +\n + the enum value of `ATOM_INVALID` is equals to the size of\n + valid_atom array, which could help us iterate over\n + valid_atom array using something like:\n +\n + for (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++)\n + /* do something with valid_atom[i] */;\n \n Signed-off-by: ZheNing Hu \n \n @@ ref-filter.c: static struct ref_to_worktree_map {\n \tstruct worktree **worktrees;\n } ref_to_worktree_map;\n \n ++/*\n ++ * The enum atom_type is used as the coordinates of valid_atom entry.\n ++ * In the atom parsing stage, it will be passed to used_atom.atom_type\n ++ * as the identifier of the atom type. We can judge the type of used_atom\n ++ * entry by `if (used_atom[i].atom_type == ATOM_*)`.\n ++ *\n ++ * ATOM_UNKNOWN equals to 0, used as an enumeration value of uninitialized\n ++ * atom_type.\n ++ * ATOM_INVALID equals to the size of valid_atom array, which could help us\n ++ * iterate over valid_atom array like this:\n ++ *\n ++ * \tfor (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++) {\n ++ *\t\tint len = strlen(valid_atom[i].name);\n ++ *\t\tif (len == atom_len && !memcmp(valid_atom[i].name, sp, len))\n ++ *\t\t\tbreak;\n ++ *\t}\n ++ */\n +enum atom_type {\n ++ATOM_UNKNOWN,\n +ATOM_REFNAME,\n +ATOM_OBJECTTYPE,\n +ATOM_OBJECTSIZE,\n @@ ref-filter.c: static struct ref_to_worktree_map {\n +ATOM_IF,\n +ATOM_THEN,\n +ATOM_ELSE,\n ++ATOM_INVALID,\n +};\n +\n /*\n * An atom is a valid field atom listed below, possibly prefixed with\n * a \"*\" to denote deref_tag().\n -@@ ref-filter.c: static struct used_atom {\n - \tconst char *name;\n - \tcmp_type type;\n - \tinfo_source source;\n -+\tenum atom_type atom_type;\n - \tunion {\n - \t\tchar color[COLOR_MAXLEN];\n - \t\tstruct align align;\n -@@ ref-filter.c: static int head_atom_parser(const struct ref_format *format, struct used_atom *a\n - }\n - \n - static struct {\n +@@ ref-filter.c: static struct ref_to_worktree_map {\n + * array.\n + */\n + static struct used_atom {\n +\tenum atom_type atom_type;\n \tconst char *name;\n + \tcmp_type type;\n \tinfo_source source;\n - \tcmp_type cmp_type;\n +@@ ref-filter.c: static struct {\n \tint (*parser)(const struct ref_format *format, struct used_atom *atom,\n \t\t const char *arg, struct strbuf *err);\n } valid_atom[] = {\n @@ ref-filter.c: static int head_atom_parser(const struct ref_format *format, struc\n -\t{ \"if\", SOURCE_NONE, FIELD_STR, if_atom_parser },\n -\t{ \"then\", SOURCE_NONE },\n -\t{ \"else\", SOURCE_NONE },\n -+\t{ ATOM_REFNAME, \"refname\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n -+\t{ ATOM_OBJECTTYPE, \"objecttype\", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser },\n -+\t{ ATOM_OBJECTSIZE, \"objectsize\", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },\n -+\t{ ATOM_OBJECTNAME, \"objectname\", SOURCE_OTHER, FIELD_STR, oid_atom_parser },\n -+\t{ ATOM_DELTABASE, \"deltabase\", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },\n -+\t{ ATOM_TREE, \"tree\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n -+\t{ ATOM_PARENT, \"parent\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n -+\t{ ATOM_NUMPARENT, \"numparent\", SOURCE_OBJ, FIELD_ULONG },\n -+\t{ ATOM_OBJECT, \"object\", SOURCE_OBJ },\n -+\t{ ATOM_TYPE, \"type\", SOURCE_OBJ },\n -+\t{ ATOM_TAG, \"tag\", SOURCE_OBJ },\n -+\t{ ATOM_AUTHOR, \"author\", SOURCE_OBJ },\n -+\t{ ATOM_AUTHORNAME, \"authorname\", SOURCE_OBJ },\n -+\t{ ATOM_AUTHOREMAIL, \"authoremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n -+\t{ ATOM_AUTHORDATE, \"authordate\", SOURCE_OBJ, FIELD_TIME },\n -+\t{ ATOM_COMMITTER, \"committer\", SOURCE_OBJ },\n -+\t{ ATOM_COMMITTERNAME, \"committername\", SOURCE_OBJ },\n -+\t{ ATOM_COMMITTEREMAIL, \"committeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n -+\t{ ATOM_COMMITTERDATE, \"committerdate\", SOURCE_OBJ, FIELD_TIME },\n -+\t{ ATOM_TAGGER, \"tagger\", SOURCE_OBJ },\n -+\t{ ATOM_TAGGERNAME, \"taggername\", SOURCE_OBJ },\n -+\t{ ATOM_TAGGEREMAIL, \"taggeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n -+\t{ ATOM_TAGGERDATE, \"taggerdate\", SOURCE_OBJ, FIELD_TIME },\n -+\t{ ATOM_CREATOR, \"creator\", SOURCE_OBJ },\n -+\t{ ATOM_CREATORDATE, \"creatordate\", SOURCE_OBJ, FIELD_TIME },\n -+\t{ ATOM_SUBJECT, \"subject\", SOURCE_OBJ, FIELD_STR, subject_atom_parser },\n -+\t{ ATOM_BODY, \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n -+\t{ ATOM_TRAILERS, \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n -+\t{ ATOM_CONTENTS, \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n -+\t{ ATOM_UPSTREAM, \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n -+\t{ ATOM_PUSH, \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n -+\t{ ATOM_SYMREF, \"symref\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n -+\t{ ATOM_FLAG, \"flag\", SOURCE_NONE },\n -+\t{ ATOM_HEAD, \"HEAD\", SOURCE_NONE, FIELD_STR, head_atom_parser },\n -+\t{ ATOM_COLOR, \"color\", SOURCE_NONE, FIELD_STR, color_atom_parser },\n -+\t{ ATOM_WORKTREEPATH, \"worktreepath\", SOURCE_NONE },\n -+\t{ ATOM_ALIGN, \"align\", SOURCE_NONE, FIELD_STR, align_atom_parser },\n -+\t{ ATOM_END, \"end\", SOURCE_NONE },\n -+\t{ ATOM_IF, \"if\", SOURCE_NONE, FIELD_STR, if_atom_parser },\n -+\t{ ATOM_THEN, \"then\", SOURCE_NONE },\n -+\t{ ATOM_ELSE, \"else\", SOURCE_NONE },\n ++\t[ATOM_REFNAME] = { \"refname\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n ++\t[ATOM_OBJECTTYPE] = { \"objecttype\", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser },\n ++\t[ATOM_OBJECTSIZE] = { \"objectsize\", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },\n ++\t[ATOM_OBJECTNAME] = { \"objectname\", SOURCE_OTHER, FIELD_STR, oid_atom_parser },\n ++\t[ATOM_DELTABASE] = { \"deltabase\", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },\n ++\t[ATOM_TREE] = { \"tree\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n ++\t[ATOM_PARENT] = { \"parent\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n ++\t[ATOM_NUMPARENT] = { \"numparent\", SOURCE_OBJ, FIELD_ULONG },\n ++\t[ATOM_OBJECT] = { \"object\", SOURCE_OBJ },\n ++\t[ATOM_TYPE] = { \"type\", SOURCE_OBJ },\n ++\t[ATOM_TAG] = { \"tag\", SOURCE_OBJ },\n ++\t[ATOM_AUTHOR] = { \"author\", SOURCE_OBJ },\n ++\t[ATOM_AUTHORNAME] = { \"authorname\", SOURCE_OBJ },\n ++\t[ATOM_AUTHOREMAIL] = { \"authoremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n ++\t[ATOM_AUTHORDATE] = { \"authordate\", SOURCE_OBJ, FIELD_TIME },\n ++\t[ATOM_COMMITTER] = { \"committer\", SOURCE_OBJ },\n ++\t[ATOM_COMMITTERNAME] = { \"committername\", SOURCE_OBJ },\n ++\t[ATOM_COMMITTEREMAIL] = { \"committeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n ++\t[ATOM_COMMITTERDATE] = { \"committerdate\", SOURCE_OBJ, FIELD_TIME },\n ++\t[ATOM_TAGGER] = { \"tagger\", SOURCE_OBJ },\n ++\t[ATOM_TAGGERNAME] = { \"taggername\", SOURCE_OBJ },\n ++\t[ATOM_TAGGEREMAIL] = { \"taggeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n ++\t[ATOM_TAGGERDATE] = { \"taggerdate\", SOURCE_OBJ, FIELD_TIME },\n ++\t[ATOM_CREATOR] = { \"creator\", SOURCE_OBJ },\n ++\t[ATOM_CREATORDATE] = { \"creatordate\", SOURCE_OBJ, FIELD_TIME },\n ++\t[ATOM_SUBJECT] = { \"subject\", SOURCE_OBJ, FIELD_STR, subject_atom_parser },\n ++\t[ATOM_BODY] = { \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n ++\t[ATOM_TRAILERS] = { \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n ++\t[ATOM_CONTENTS] = { \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n ++\t[ATOM_UPSTREAM] = { \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n ++\t[ATOM_PUSH] = { \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n ++\t[ATOM_SYMREF] = { \"symref\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n ++\t[ATOM_FLAG] = { \"flag\", SOURCE_NONE },\n ++\t[ATOM_HEAD] = { \"HEAD\", SOURCE_NONE, FIELD_STR, head_atom_parser },\n ++\t[ATOM_COLOR] = { \"color\", SOURCE_NONE, FIELD_STR, color_atom_parser },\n ++\t[ATOM_WORKTREEPATH] = { \"worktreepath\", SOURCE_NONE },\n ++\t[ATOM_ALIGN] = { \"align\", SOURCE_NONE, FIELD_STR, align_atom_parser },\n ++\t[ATOM_END] = { \"end\", SOURCE_NONE },\n ++\t[ATOM_IF] = { \"if\", SOURCE_NONE, FIELD_STR, if_atom_parser },\n ++\t[ATOM_THEN] = { \"then\", SOURCE_NONE },\n ++\t[ATOM_ELSE] = { \"else\", SOURCE_NONE },\n \t/*\n \t * Please update $__git_ref_fieldlist in git-completion.bash\n \t * when you add new atoms\n +@@ ref-filter.c: static int parse_ref_filter_atom(const struct ref_format *format,\n + \tatom_len = (arg ? arg : ep) - sp;\n + \n + \t/* Is the atom a valid one? */\n +-\tfor (i = 0; i < ARRAY_SIZE(valid_atom); i++) {\n ++\tfor (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++) {\n + \t\tint len = strlen(valid_atom[i].name);\n + \t\tif (len == atom_len && !memcmp(valid_atom[i].name, sp, len))\n + \t\t\tbreak;\n + \t}\n + \n +-\tif (ARRAY_SIZE(valid_atom) <= i)\n ++\tif (i == ATOM_INVALID)\n + \t\treturn strbuf_addf_ret(err, -1, _(\"unknown field name: %.*s\"),\n + \t\t\t\t (int)(ep-atom), atom);\n + \tif (valid_atom[i].source != SOURCE_NONE && !have_git_dir())\n @@ ref-filter.c: static int parse_ref_filter_atom(const struct ref_format *format,\n \tat = used_atom_cnt;\n \tused_atom_cnt++;\n \tREALLOC_ARRAY(used_atom, used_atom_cnt);\n -+\tused_atom[at].atom_type = valid_atom[i].atom_type;\n ++\tused_atom[at].atom_type = i;\n \tused_atom[at].name = xmemdupz(atom, ep - atom);\n \tused_atom[at].type = valid_atom[i].cmp_type;\n \tused_atom[at].source = valid_atom[i].source;\n @@ ref-filter.c: static int parse_ref_filter_atom(const struct ref_format *format,\n \tif (*atom == '*')\n \t\tneed_tagged = 1;\n -\tif (!strcmp(valid_atom[i].name, \"symref\"))\n -+\tif (valid_atom[i].atom_type == ATOM_SYMREF)\n ++\tif (i == ATOM_SYMREF)\n \t\tneed_symref = 1;\n \treturn at;\n }\n\n-- \ngitgitgadget\n"},{"id":"424046","messageId":"91ca57c9d04a822aa4955dbfe3962a6fb2444e81.1620659000.git.gitgitgadget@gmail.com","threadId":"55647","inReplyTo":"pull.951.v2.git.1620659000.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] [GSOC] ref-filter: add objectsize to used_atom","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-10T15:03:18Z","receivedAt":"2021-05-10T15:24:19Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nSince \"objectsize:size\" is composed of two parts,\n\"type:attribute\". However, the original implementation\ndid not decouple the two parts \"type\" and \"attribute\" well,\nwe still need to judge separately whether the atom is\n\"objectsize\" or \"objectsize:disk\" in `grab_common_values()`.\n\nAdd a new member `objectsize` to the union `used_atom.u`,\nso that we can separate the judgment of atom type from the\njudgment of atom attribute, This will bring scalability to\natom `%(objectsize)`.\n\nSigned-off-by: ZheNing Hu \n---\n ref-filter.c | 19 +++++++++++++------\n 1 file changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex a0adb4551d87..f420bae6e5ba 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -146,6 +146,9 @@ static struct used_atom {\n \t\t\tenum { O_FULL, O_LENGTH, O_SHORT } option;\n \t\t\tunsigned int length;\n \t\t} oid;\n+\t\tstruct {\n+\t\t\tenum { O_SIZE, O_SIZE_DISK } option;\n+\t\t} objectsize;\n \t\tstruct email_option {\n \t\t\tenum { EO_RAW, EO_TRIM, EO_LOCALPART } option;\n \t\t} email_option;\n@@ -269,11 +272,13 @@ static int objectsize_atom_parser(const struct ref_format *format, struct used_a\n \t\t\t\t const char *arg, struct strbuf *err)\n {\n \tif (!arg) {\n+\t\tatom->u.objectsize.option = O_SIZE;\n \t\tif (*atom->name == '*')\n \t\t\toi_deref.info.sizep = &oi_deref.size;\n \t\telse\n \t\t\toi.info.sizep = &oi.size;\n \t} else if (!strcmp(arg, \"disk\")) {\n+\t\tatom->u.objectsize.option = O_SIZE_DISK;\n \t\tif (*atom->name == '*')\n \t\t\toi_deref.info.disk_sizep = &oi_deref.disk_size;\n \t\telse\n@@ -967,12 +972,14 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \t\t\tname++;\n \t\tif (!strcmp(name, \"objecttype\"))\n \t\t\tv->s = xstrdup(type_name(oi->type));\n-\t\telse if (!strcmp(name, \"objectsize:disk\")) {\n-\t\t\tv->value = oi->disk_size;\n-\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n-\t\t} else if (!strcmp(name, \"objectsize\")) {\n-\t\t\tv->value = oi->size;\n-\t\t\tv->s = xstrfmt(\"%\"PRIuMAX , (uintmax_t)oi->size);\n+\t\telse if (starts_with(name, \"objectsize\")) {\n+\t\t\tif (used_atom[i].u.objectsize.option == O_SIZE_DISK) {\n+\t\t\t\tv->value = oi->disk_size;\n+\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n+\t\t\t} else if (used_atom[i].u.objectsize.option == O_SIZE) {\n+\t\t\t\tv->value = oi->size;\n+\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX , (uintmax_t)oi->size);\n+\t\t\t}\n \t\t} else if (!strcmp(name, \"deltabase\"))\n \t\t\tv->s = xstrdup(oid_to_hex(&oi->delta_base_oid));\n \t\telse if (deref)\n-- \ngitgitgadget\n\n"},{"id":"424047","messageId":"a1f70b39b7efbadff9e2202968dd1ca65ea3c1b4.1620659000.git.gitgitgadget@gmail.com","threadId":"55647","inReplyTo":"pull.951.v2.git.1620659000.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] [GSOC] ref-filter: introduce enum atom_type","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-10T15:03:19Z","receivedAt":"2021-05-10T15:24:21Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nIn the original ref-filter design, it will copy the parsed\natom's name and attributes to `used_atom[i].name` in the\natom's parsing step, and use it again for string matching\nin the later specific ref attributes filling step. It use\na lot of string matching to determine which atom we need.\n\nIntroduce the enum \"atom_type\", each enum value is named\nas `ATOM_*`, which is the index of each corresponding\nvalid_atom entry. In the first step of the atom parsing,\n`used_atom.atom_type` will record corresponding enum value\nfrom valid_atom entry index, and then in specific reference\nattribute filling step, only need to compare the value of\nthe `used_atom.atom_type` to judge the atom type.\n\nthe enum value of `ATOM_UNKNOWN` is equals to zero, which\ncould ensure that we can easily distinguish such a struct\nwhere the atom_type is known from such a struct where it\nis unknown yet.\n\nthe enum value of `ATOM_INVALID` is equals to the size of\nvalid_atom array, which could help us iterate over\nvalid_atom array using something like:\n\nfor (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++)\n /* do something with valid_atom[i] */;\n\nSigned-off-by: ZheNing Hu \n---\n ref-filter.c | 214 ++++++++++++++++++++++++++++++++++-----------------\n 1 file changed, 143 insertions(+), 71 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex f420bae6e5ba..e3bf2cd1aaec 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -108,6 +108,69 @@ static struct ref_to_worktree_map {\n \tstruct worktree **worktrees;\n } ref_to_worktree_map;\n \n+/*\n+ * The enum atom_type is used as the coordinates of valid_atom entry.\n+ * In the atom parsing stage, it will be passed to used_atom.atom_type\n+ * as the identifier of the atom type. We can judge the type of used_atom\n+ * entry by `if (used_atom[i].atom_type == ATOM_*)`.\n+ *\n+ * ATOM_UNKNOWN equals to 0, used as an enumeration value of uninitialized\n+ * atom_type.\n+ * ATOM_INVALID equals to the size of valid_atom array, which could help us\n+ * iterate over valid_atom array like this:\n+ *\n+ * \tfor (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++) {\n+ *\t\tint len = strlen(valid_atom[i].name);\n+ *\t\tif (len == atom_len && !memcmp(valid_atom[i].name, sp, len))\n+ *\t\t\tbreak;\n+ *\t}\n+ */\n+enum atom_type {\n+ATOM_UNKNOWN,\n+ATOM_REFNAME,\n+ATOM_OBJECTTYPE,\n+ATOM_OBJECTSIZE,\n+ATOM_OBJECTNAME,\n+ATOM_DELTABASE,\n+ATOM_TREE,\n+ATOM_PARENT,\n+ATOM_NUMPARENT,\n+ATOM_OBJECT,\n+ATOM_TYPE,\n+ATOM_TAG,\n+ATOM_AUTHOR,\n+ATOM_AUTHORNAME,\n+ATOM_AUTHOREMAIL,\n+ATOM_AUTHORDATE,\n+ATOM_COMMITTER,\n+ATOM_COMMITTERNAME,\n+ATOM_COMMITTEREMAIL,\n+ATOM_COMMITTERDATE,\n+ATOM_TAGGER,\n+ATOM_TAGGERNAME,\n+ATOM_TAGGEREMAIL,\n+ATOM_TAGGERDATE,\n+ATOM_CREATOR,\n+ATOM_CREATORDATE,\n+ATOM_SUBJECT,\n+ATOM_BODY,\n+ATOM_TRAILERS,\n+ATOM_CONTENTS,\n+ATOM_UPSTREAM,\n+ATOM_PUSH,\n+ATOM_SYMREF,\n+ATOM_FLAG,\n+ATOM_HEAD,\n+ATOM_COLOR,\n+ATOM_WORKTREEPATH,\n+ATOM_ALIGN,\n+ATOM_END,\n+ATOM_IF,\n+ATOM_THEN,\n+ATOM_ELSE,\n+ATOM_INVALID,\n+};\n+\n /*\n * An atom is a valid field atom listed below, possibly prefixed with\n * a \"*\" to denote deref_tag().\n@@ -119,6 +182,7 @@ static struct ref_to_worktree_map {\n * array.\n */\n static struct used_atom {\n+\tenum atom_type atom_type;\n \tconst char *name;\n \tcmp_type type;\n \tinfo_source source;\n@@ -506,47 +570,47 @@ static struct {\n \tint (*parser)(const struct ref_format *format, struct used_atom *atom,\n \t\t const char *arg, struct strbuf *err);\n } valid_atom[] = {\n-\t{ \"refname\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n-\t{ \"objecttype\", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser },\n-\t{ \"objectsize\", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },\n-\t{ \"objectname\", SOURCE_OTHER, FIELD_STR, oid_atom_parser },\n-\t{ \"deltabase\", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },\n-\t{ \"tree\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n-\t{ \"parent\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n-\t{ \"numparent\", SOURCE_OBJ, FIELD_ULONG },\n-\t{ \"object\", SOURCE_OBJ },\n-\t{ \"type\", SOURCE_OBJ },\n-\t{ \"tag\", SOURCE_OBJ },\n-\t{ \"author\", SOURCE_OBJ },\n-\t{ \"authorname\", SOURCE_OBJ },\n-\t{ \"authoremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n-\t{ \"authordate\", SOURCE_OBJ, FIELD_TIME },\n-\t{ \"committer\", SOURCE_OBJ },\n-\t{ \"committername\", SOURCE_OBJ },\n-\t{ \"committeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n-\t{ \"committerdate\", SOURCE_OBJ, FIELD_TIME },\n-\t{ \"tagger\", SOURCE_OBJ },\n-\t{ \"taggername\", SOURCE_OBJ },\n-\t{ \"taggeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n-\t{ \"taggerdate\", SOURCE_OBJ, FIELD_TIME },\n-\t{ \"creator\", SOURCE_OBJ },\n-\t{ \"creatordate\", SOURCE_OBJ, FIELD_TIME },\n-\t{ \"subject\", SOURCE_OBJ, FIELD_STR, subject_atom_parser },\n-\t{ \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n-\t{ \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n-\t{ \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n-\t{ \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n-\t{ \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n-\t{ \"symref\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n-\t{ \"flag\", SOURCE_NONE },\n-\t{ \"HEAD\", SOURCE_NONE, FIELD_STR, head_atom_parser },\n-\t{ \"color\", SOURCE_NONE, FIELD_STR, color_atom_parser },\n-\t{ \"worktreepath\", SOURCE_NONE },\n-\t{ \"align\", SOURCE_NONE, FIELD_STR, align_atom_parser },\n-\t{ \"end\", SOURCE_NONE },\n-\t{ \"if\", SOURCE_NONE, FIELD_STR, if_atom_parser },\n-\t{ \"then\", SOURCE_NONE },\n-\t{ \"else\", SOURCE_NONE },\n+\t[ATOM_REFNAME] = { \"refname\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n+\t[ATOM_OBJECTTYPE] = { \"objecttype\", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser },\n+\t[ATOM_OBJECTSIZE] = { \"objectsize\", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },\n+\t[ATOM_OBJECTNAME] = { \"objectname\", SOURCE_OTHER, FIELD_STR, oid_atom_parser },\n+\t[ATOM_DELTABASE] = { \"deltabase\", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },\n+\t[ATOM_TREE] = { \"tree\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n+\t[ATOM_PARENT] = { \"parent\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n+\t[ATOM_NUMPARENT] = { \"numparent\", SOURCE_OBJ, FIELD_ULONG },\n+\t[ATOM_OBJECT] = { \"object\", SOURCE_OBJ },\n+\t[ATOM_TYPE] = { \"type\", SOURCE_OBJ },\n+\t[ATOM_TAG] = { \"tag\", SOURCE_OBJ },\n+\t[ATOM_AUTHOR] = { \"author\", SOURCE_OBJ },\n+\t[ATOM_AUTHORNAME] = { \"authorname\", SOURCE_OBJ },\n+\t[ATOM_AUTHOREMAIL] = { \"authoremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n+\t[ATOM_AUTHORDATE] = { \"authordate\", SOURCE_OBJ, FIELD_TIME },\n+\t[ATOM_COMMITTER] = { \"committer\", SOURCE_OBJ },\n+\t[ATOM_COMMITTERNAME] = { \"committername\", SOURCE_OBJ },\n+\t[ATOM_COMMITTEREMAIL] = { \"committeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n+\t[ATOM_COMMITTERDATE] = { \"committerdate\", SOURCE_OBJ, FIELD_TIME },\n+\t[ATOM_TAGGER] = { \"tagger\", SOURCE_OBJ },\n+\t[ATOM_TAGGERNAME] = { \"taggername\", SOURCE_OBJ },\n+\t[ATOM_TAGGEREMAIL] = { \"taggeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n+\t[ATOM_TAGGERDATE] = { \"taggerdate\", SOURCE_OBJ, FIELD_TIME },\n+\t[ATOM_CREATOR] = { \"creator\", SOURCE_OBJ },\n+\t[ATOM_CREATORDATE] = { \"creatordate\", SOURCE_OBJ, FIELD_TIME },\n+\t[ATOM_SUBJECT] = { \"subject\", SOURCE_OBJ, FIELD_STR, subject_atom_parser },\n+\t[ATOM_BODY] = { \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n+\t[ATOM_TRAILERS] = { \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n+\t[ATOM_CONTENTS] = { \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n+\t[ATOM_UPSTREAM] = { \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n+\t[ATOM_PUSH] = { \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n+\t[ATOM_SYMREF] = { \"symref\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n+\t[ATOM_FLAG] = { \"flag\", SOURCE_NONE },\n+\t[ATOM_HEAD] = { \"HEAD\", SOURCE_NONE, FIELD_STR, head_atom_parser },\n+\t[ATOM_COLOR] = { \"color\", SOURCE_NONE, FIELD_STR, color_atom_parser },\n+\t[ATOM_WORKTREEPATH] = { \"worktreepath\", SOURCE_NONE },\n+\t[ATOM_ALIGN] = { \"align\", SOURCE_NONE, FIELD_STR, align_atom_parser },\n+\t[ATOM_END] = { \"end\", SOURCE_NONE },\n+\t[ATOM_IF] = { \"if\", SOURCE_NONE, FIELD_STR, if_atom_parser },\n+\t[ATOM_THEN] = { \"then\", SOURCE_NONE },\n+\t[ATOM_ELSE] = { \"else\", SOURCE_NONE },\n \t/*\n \t * Please update $__git_ref_fieldlist in git-completion.bash\n \t * when you add new atoms\n@@ -610,13 +674,13 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n \tatom_len = (arg ? arg : ep) - sp;\n \n \t/* Is the atom a valid one? */\n-\tfor (i = 0; i < ARRAY_SIZE(valid_atom); i++) {\n+\tfor (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++) {\n \t\tint len = strlen(valid_atom[i].name);\n \t\tif (len == atom_len && !memcmp(valid_atom[i].name, sp, len))\n \t\t\tbreak;\n \t}\n \n-\tif (ARRAY_SIZE(valid_atom) <= i)\n+\tif (i == ATOM_INVALID)\n \t\treturn strbuf_addf_ret(err, -1, _(\"unknown field name: %.*s\"),\n \t\t\t\t (int)(ep-atom), atom);\n \tif (valid_atom[i].source != SOURCE_NONE && !have_git_dir())\n@@ -628,6 +692,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n \tat = used_atom_cnt;\n \tused_atom_cnt++;\n \tREALLOC_ARRAY(used_atom, used_atom_cnt);\n+\tused_atom[at].atom_type = i;\n \tused_atom[at].name = xmemdupz(atom, ep - atom);\n \tused_atom[at].type = valid_atom[i].cmp_type;\n \tused_atom[at].source = valid_atom[i].source;\n@@ -652,7 +717,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n \t\treturn -1;\n \tif (*atom == '*')\n \t\tneed_tagged = 1;\n-\tif (!strcmp(valid_atom[i].name, \"symref\"))\n+\tif (i == ATOM_SYMREF)\n \t\tneed_symref = 1;\n \treturn at;\n }\n@@ -965,14 +1030,15 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n-\t\tif (!strcmp(name, \"objecttype\"))\n+\t\tif (atom_type == ATOM_OBJECTTYPE)\n \t\t\tv->s = xstrdup(type_name(oi->type));\n-\t\telse if (starts_with(name, \"objectsize\")) {\n+\t\telse if (atom_type == ATOM_OBJECTSIZE) {\n \t\t\tif (used_atom[i].u.objectsize.option == O_SIZE_DISK) {\n \t\t\t\tv->value = oi->disk_size;\n \t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n@@ -980,9 +1046,9 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \t\t\t\tv->value = oi->size;\n \t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX , (uintmax_t)oi->size);\n \t\t\t}\n-\t\t} else if (!strcmp(name, \"deltabase\"))\n+\t\t} else if (atom_type == ATOM_DELTABASE)\n \t\t\tv->s = xstrdup(oid_to_hex(&oi->delta_base_oid));\n-\t\telse if (deref)\n+\t\telse if (atom_type == ATOM_OBJECTNAME && deref)\n \t\t\tgrab_oid(name, \"objectname\", &oi->oid, v, &used_atom[i]);\n \t}\n }\n@@ -995,16 +1061,17 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n-\t\tif (!strcmp(name, \"tag\"))\n+\t\tif (atom_type == ATOM_TAG)\n \t\t\tv->s = xstrdup(tag->tag);\n-\t\telse if (!strcmp(name, \"type\") && tag->tagged)\n+\t\telse if (atom_type == ATOM_TYPE && tag->tagged)\n \t\t\tv->s = xstrdup(type_name(tag->tagged->type));\n-\t\telse if (!strcmp(name, \"object\") && tag->tagged)\n+\t\telse if (atom_type == ATOM_OBJECT && tag->tagged)\n \t\t\tv->s = xstrdup(oid_to_hex(&tag->tagged->oid));\n \t}\n }\n@@ -1017,18 +1084,20 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n-\t\tif (grab_oid(name, \"tree\", get_commit_tree_oid(commit), v, &used_atom[i]))\n+\t\tif (atom_type == ATOM_TREE &&\n+\t\t grab_oid(name, \"tree\", get_commit_tree_oid(commit), v, &used_atom[i]))\n \t\t\tcontinue;\n-\t\tif (!strcmp(name, \"numparent\")) {\n+\t\tif (atom_type == ATOM_NUMPARENT) {\n \t\t\tv->value = commit_list_count(commit->parents);\n \t\t\tv->s = xstrfmt(\"%lu\", (unsigned long)v->value);\n \t\t}\n-\t\telse if (starts_with(name, \"parent\")) {\n+\t\telse if (atom_type == ATOM_PARENT) {\n \t\t\tstruct commit_list *parents;\n \t\t\tstruct strbuf s = STRBUF_INIT;\n \t\t\tfor (parents = commit->parents; parents; parents = parents->next) {\n@@ -1208,15 +1277,16 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n \t\treturn;\n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n \n-\t\tif (starts_with(name, \"creatordate\"))\n+\t\tif (atom_type == ATOM_CREATORDATE)\n \t\t\tgrab_date(wholine, v, name);\n-\t\telse if (!strcmp(name, \"creator\"))\n+\t\telse if (atom_type == ATOM_CREATOR)\n \t\t\tv->s = copy_line(wholine);\n \t}\n }\n@@ -1696,6 +1766,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t/* Fill in specials first */\n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tstruct used_atom *atom = &used_atom[i];\n+\t\tenum atom_type atom_type = atom->atom_type;\n \t\tconst char *name = used_atom[i].name;\n \t\tstruct atom_value *v = &ref->value[i];\n \t\tint deref = 0;\n@@ -1710,18 +1781,18 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\tname++;\n \t\t}\n \n-\t\tif (starts_with(name, \"refname\"))\n+\t\tif (atom_type == ATOM_REFNAME)\n \t\t\trefname = get_refname(atom, ref);\n-\t\telse if (!strcmp(name, \"worktreepath\")) {\n+\t\telse if (atom_type == ATOM_WORKTREEPATH) {\n \t\t\tif (ref->kind == FILTER_REFS_BRANCHES)\n \t\t\t\tv->s = get_worktree_path(atom, ref);\n \t\t\telse\n \t\t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n \t\t}\n-\t\telse if (starts_with(name, \"symref\"))\n+\t\telse if (atom_type == ATOM_SYMREF)\n \t\t\trefname = get_symref(atom, ref);\n-\t\telse if (starts_with(name, \"upstream\")) {\n+\t\telse if (atom_type == ATOM_UPSTREAM) {\n \t\t\tconst char *branch_name;\n \t\t\t/* only local branches may have an upstream */\n \t\t\tif (!skip_prefix(ref->refname, \"refs/heads/\",\n@@ -1737,7 +1808,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\telse\n \t\t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-\t\t} else if (atom->u.remote_ref.push) {\n+\t\t} else if (atom_type == ATOM_PUSH && atom->u.remote_ref.push) {\n \t\t\tconst char *branch_name;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tif (!skip_prefix(ref->refname, \"refs/heads/\",\n@@ -1756,10 +1827,10 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\tfree((char *)v->s);\n \t\t\tfill_remote_ref_details(atom, refname, branch, &v->s);\n \t\t\tcontinue;\n-\t\t} else if (starts_with(name, \"color:\")) {\n+\t\t} else if (atom_type == ATOM_COLOR) {\n \t\t\tv->s = xstrdup(atom->u.color);\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"flag\")) {\n+\t\t} else if (atom_type == ATOM_FLAG) {\n \t\t\tchar buf[256], *cp = buf;\n \t\t\tif (ref->flag & REF_ISSYMREF)\n \t\t\t\tcp = copy_advance(cp, \",symref\");\n@@ -1772,23 +1843,24 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\t\tv->s = xstrdup(buf + 1);\n \t\t\t}\n \t\t\tcontinue;\n-\t\t} else if (!deref && grab_oid(name, \"objectname\", &ref->objectname, v, atom)) {\n-\t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"HEAD\")) {\n+\t\t} else if (!deref && atom_type == ATOM_OBJECTNAME &&\n+\t\t\t grab_oid(name, \"objectname\", &ref->objectname, v, atom)) {\n+\t\t\t\tcontinue;\n+\t\t} else if (atom_type == ATOM_HEAD) {\n \t\t\tif (atom->u.head && !strcmp(ref->refname, atom->u.head))\n \t\t\t\tv->s = xstrdup(\"*\");\n \t\t\telse\n \t\t\t\tv->s = xstrdup(\" \");\n \t\t\tcontinue;\n-\t\t} else if (starts_with(name, \"align\")) {\n+\t\t} else if (atom_type == ATOM_ALIGN) {\n \t\t\tv->handler = align_atom_handler;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"end\")) {\n+\t\t} else if (atom_type == ATOM_END) {\n \t\t\tv->handler = end_atom_handler;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-\t\t} else if (starts_with(name, \"if\")) {\n+\t\t} else if (atom_type == ATOM_IF) {\n \t\t\tconst char *s;\n \t\t\tif (skip_prefix(name, \"if:\", &s))\n \t\t\t\tv->s = xstrdup(s);\n@@ -1796,11 +1868,11 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\t\tv->s = xstrdup(\"\");\n \t\t\tv->handler = if_atom_handler;\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"then\")) {\n+\t\t} else if (atom_type == ATOM_THEN) {\n \t\t\tv->handler = then_atom_handler;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"else\")) {\n+\t\t} else if (atom_type == ATOM_ELSE) {\n \t\t\tv->handler = else_atom_handler;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-- \ngitgitgadget\n"},{"id":"424088","messageId":"xmqqh7jaow31.fsf@gitster.g","threadId":"55647","inReplyTo":"a1f70b39b7efbadff9e2202968dd1ca65ea3c1b4.1620659000.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/2] [GSOC] ref-filter: introduce enum atom_type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-11T02:14:42Z","receivedAt":"2021-05-11T02:14:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" writes:\n\n> the enum value of `ATOM_UNKNOWN` is equals to zero, which\n\ns/the/The/;\n\n> could ensure that we can easily distinguish such a struct\n> where the atom_type is known from such a struct where it\n> is unknown yet.\n>\n> the enum value of `ATOM_INVALID` is equals to the size of\n\nDitto.\n\n> +/*\n> + * The enum atom_type is used as the coordinates of valid_atom entry.\n> + * In the atom parsing stage, it will be passed to used_atom.atom_type\n> + * as the identifier of the atom type. We can judge the type of used_atom\n> + * entry by `if (used_atom[i].atom_type == ATOM_*)`.\n> + *\n> + * ATOM_UNKNOWN equals to 0, used as an enumeration value of uninitialized\n> + * atom_type.\n\nShouldn't it be (-1)?\n\nAnd I'd assume I am right in the following.\n\n> + * ATOM_INVALID equals to the size of valid_atom array, which could help us\n> + * iterate over valid_atom array like this:\n> + *\n> + * \tfor (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++) {\n\nI find it far more intuitive to say\n\n\tfor (i = 0; i < ATOM_INVALID; i++)\n\nthan having to say UNKNOWN+1.\n\nIn any case, the values should be indented, and a comment should\nensure that the final one stays at the end, perhaps like this.\n\n\tenum atom_type {\n\t\tATOM_INVALID = -2,\n\t\tATOM_UNKNOWN = -1,\n\t\tATOM_REFNAME,\n\t\t...\n\t\tATOM_ELSE,\n\t\tATOM_MAX /* MUST BE AT THE END */\n\t}\n\n(note that the trailing comma is deliberately omitted).\n\nIt would allow people to say\n\n\tfor (i = 0; i < ATOM_MAX; i++)\n\ninstead, which would be even nicer.\n"},{"id":"424105","messageId":"CAP8UFD3m0TA3nPDaPhaU3=FVF_r5cPM8xksVL6MpCOhNC4dT3g@mail.gmail.com","threadId":"55647","inReplyTo":"xmqqh7jaow31.fsf@gitster.g","subject":"Re: [PATCH v2 2/2] [GSOC] ref-filter: introduce enum atom_type","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2021-05-11T05:51:44Z","receivedAt":"2021-05-11T05:56:53Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, May 11, 2021 at 4:14 AM Junio C Hamano wrote:\n>\n> \"ZheNing Hu via GitGitGadget\" writes:\n\n> > +/*\n> > + * The enum atom_type is used as the coordinates of valid_atom entry.\n> > + * In the atom parsing stage, it will be passed to used_atom.atom_type\n> > + * as the identifier of the atom type. We can judge the type of used_atom\n> > + * entry by `if (used_atom[i].atom_type == ATOM_*)`.\n> > + *\n> > + * ATOM_UNKNOWN equals to 0, used as an enumeration value of uninitialized\n> > + * atom_type.\n>\n> Shouldn't it be (-1)?\n\nIf it's -1 instead of 0, then it might be a bit more complex to\ninitialize structs that contain such a field, as it cannot be done\nwith only xcalloc().\n\n> And I'd assume I am right in the following.\n>\n> > + * ATOM_INVALID equals to the size of valid_atom array, which could help us\n> > + * iterate over valid_atom array like this:\n> > + *\n> > + * for (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++) {\n>\n> I find it far more intuitive to say\n>\n> for (i = 0; i < ATOM_INVALID; i++)\n>\n> than having to say UNKNOWN+1.\n\nYeah, that's more intuitive. But in my opinion, using `ATOM_UNKNOWN +\n1` instead of `0` at least shouldn't often result in more lines of\ncode, and should be a bit easier to get right, compared to having to\ninitialize the field with ATOM_UNKNOWN.\n\n> In any case, the values should be indented, and a comment should\n> ensure that the final one stays at the end, perhaps like this.\n>\n> enum atom_type {\n> ATOM_INVALID = -2,\n> ATOM_UNKNOWN = -1,\n> ATOM_REFNAME,\n> ...\n> ATOM_ELSE,\n> ATOM_MAX /* MUST BE AT THE END */\n\nI agree that a comment telling people that it must be at the end is good.\n\n> }\n>\n> (note that the trailing comma is deliberately omitted).\n\nYeah.\n\n> It would allow people to say\n>\n> for (i = 0; i < ATOM_MAX; i++)\n>\n> instead, which would be even nicer.\n\nYeah, it's also a tradeoff to have the last one called ATOM_MAX\ninstead of ATOM_INVALID, and to have a separate ATOM_INVALID if it's\nneeded.\n"},{"id":"424107","messageId":"xmqq7dk5ol3c.fsf@gitster.g","threadId":"55647","inReplyTo":"CAP8UFD3m0TA3nPDaPhaU3=FVF_r5cPM8xksVL6MpCOhNC4dT3g@mail.gmail.com","subject":"Re: [PATCH v2 2/2] [GSOC] ref-filter: introduce enum atom_type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-11T06:12:07Z","receivedAt":"2021-05-11T06:12:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n>> I find it far more intuitive to say\n>>\n>> for (i = 0; i < ATOM_INVALID; i++)\n>>\n>> than having to say UNKNOWN+1.\n>\n> Yeah, that's more intuitive. But in my opinion, using `ATOM_UNKNOWN +\n> 1` instead of `0` at least shouldn't often result in more lines of\n> code, and should be a bit easier to get right, compared to having to\n> initialize the field with ATOM_UNKNOWN.\n\nNumber of lines is not all that important.\n\nBut the developers must remember that UNKNOWN is at the bottom end\nand INVALID is at the top end, which is very taxing. Tying UNKNOWN\nto the top end and INVALID to the bottom end would equally be\nplausible and there is no memory aid to help us remember which one\nis which. Compare it to \"array indices begin at 0, and the upper\nend is MAX\". Your scheme is much easier for developers to screw up.\n\n"},{"id":"424123","messageId":"CAOLTT8Sd=zxCDPjHQm3M_xsHafG2pkFbNgdibpzBJuTzAu8ECQ@mail.gmail.com","threadId":"55647","inReplyTo":"xmqqh7jaow31.fsf@gitster.g","subject":"Re: [PATCH v2 2/2] [GSOC] ref-filter: introduce enum atom_type","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-05-11T12:18:24Z","receivedAt":"2021-05-11T12:18:50Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"> And I'd assume I am right in the following.\n>\n> > + * ATOM_INVALID equals to the size of valid_atom array, which could help us\n> > + * iterate over valid_atom array like this:\n> > + *\n> > + * for (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++) {\n>\n> I find it far more intuitive to say\n>\n> for (i = 0; i < ATOM_INVALID; i++)\n>\n> than having to say UNKNOWN+1.\n>\n> In any case, the values should be indented, and a comment should\n> ensure that the final one stays at the end, perhaps like this.\n>\n> enum atom_type {\n> ATOM_INVALID = -2,\n> ATOM_UNKNOWN = -1,\n> ATOM_REFNAME,\n> ...\n> ATOM_ELSE,\n> ATOM_MAX /* MUST BE AT THE END */\n> }\n>\n> (note that the trailing comma is deliberately omitted).\n>\n> It would allow people to say\n>\n> for (i = 0; i < ATOM_MAX; i++)\n>\n> instead, which would be even nicer.\n\nI think ATOM_INVALID and ATOM_MAX all will have a\nsimilar effort. Why don't we omit one of them?\n\nFor the time being, all the used_atom entry create in\n`parse_ref_filter_atom()`, we directly use\n`used_atom[at].atom_type = i;` after realloc the used_atom.\nThere is no time for \"ATOM_UNKNOWN\" at all.\n\nI don’t know if it makes a lot of sense use \"ATOM_UNKNOWN\"\nat the moment.\n\n--\nZheNing Hu\n"},{"id":"424124","messageId":"CAOLTT8RP9B7mmP+zfgdjrP+jjoBgJyeWmxnJitOFM87JraXCFQ@mail.gmail.com","threadId":"55647","inReplyTo":"CAP8UFD3m0TA3nPDaPhaU3=FVF_r5cPM8xksVL6MpCOhNC4dT3g@mail.gmail.com","subject":"Re: [PATCH v2 2/2] [GSOC] ref-filter: introduce enum atom_type","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-05-11T12:37:33Z","receivedAt":"2021-05-11T12:38:03Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Christian Couder 于2021年5月11日周二 下午1:51写道：\n>\n> >\n> > Shouldn't it be (-1)?\n>\n> If it's -1 instead of 0, then it might be a bit more complex to\n> initialize structs that contain such a field, as it cannot be done\n> with only xcalloc().\n>\n\nI agree. If the traverse start from 0, an init atom_type will have\n\"ATOM_REFNAME\" junk value. If let users manually adjust it to\nATOM_UNKNOWN, it seems to be very troublesome.\n\n> > And I'd assume I am right in the following.\n> >\n> > > + * ATOM_INVALID equals to the size of valid_atom array, which could help us\n> > > + * iterate over valid_atom array like this:\n> > > + *\n> > > + * for (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++) {\n> >\n> > I find it far more intuitive to say\n> >\n> > for (i = 0; i < ATOM_INVALID; i++)\n> >\n> > than having to say UNKNOWN+1.\n>\n> Yeah, that's more intuitive. But in my opinion, using `ATOM_UNKNOWN +\n> 1` instead of `0` at least shouldn't often result in more lines of\n> code, and should be a bit easier to get right, compared to having to\n> initialize the field with ATOM_UNKNOWN.\n>\n\nThere will be a trade-off. Traverse from 0 or does not need to adjust the\ninitialized atom_type = UNKNOWN.\n\n>\n> > It would allow people to say\n> >\n> > for (i = 0; i < ATOM_MAX; i++)\n> >\n> > instead, which would be even nicer.\n>\n> Yeah, it's also a tradeoff to have the last one called ATOM_MAX\n> instead of ATOM_INVALID, and to have a separate ATOM_INVALID if it's\n> needed.\n\nAbout ATOM_MAX or ATOM_INVALID, I have a idea:\n\nenum atom_type {\nATOM_UNKNOWN,\n...\nATOM_ELSE,\nATOM_INVALID,\n+ATOM_MAX = ATOM_INVALID\n };\n\nThis might be able to do both.\n\nThanks.\n--\nZheNing Hu\n"},{"id":"424126","messageId":"CAOLTT8T-pUc7SOaivdydXHXm+_-hY=ARpWqGA_OOnej8gRrMCA@mail.gmail.com","threadId":"55647","inReplyTo":"xmqq7dk5ol3c.fsf@gitster.g","subject":"Re: [PATCH v2 2/2] [GSOC] ref-filter: introduce enum atom_type","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-05-11T12:53:11Z","receivedAt":"2021-05-11T12:53:31Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano 于2021年5月11日周二 下午2:12写道：\n>\n> Christian Couder writes:\n>\n> >> I find it far more intuitive to say\n> >>\n> >> for (i = 0; i < ATOM_INVALID; i++)\n> >>\n> >> than having to say UNKNOWN+1.\n> >\n> > Yeah, that's more intuitive. But in my opinion, using `ATOM_UNKNOWN +\n> > 1` instead of `0` at least shouldn't often result in more lines of\n> > code, and should be a bit easier to get right, compared to having to\n> > initialize the field with ATOM_UNKNOWN.\n>\n> Number of lines is not all that important.\n>\n> But the developers must remember that UNKNOWN is at the bottom end\n> and INVALID is at the top end, which is very taxing. Tying UNKNOWN\n> to the top end and INVALID to the bottom end would equally be\n> plausible and there is no memory aid to help us remember which one\n> is which. Compare it to \"array indices begin at 0, and the upper\n> end is MAX\". Your scheme is much easier for developers to screw up.\n>\n\nYes, UNKNOWN + 1 is difficult to use. But using UNKNOWN = -1,\nthis means that the coder may indirectly use an init atom_type with\njunk value \"ATOM_REFNAME\", they maybe did't notice they need\nreinitialize the value to UNKNOWN.\n\nI thought that perhaps such a naming would be better:\n\nATOM_BEGIN = ATOM_UNKNOWN + 1,\nATOM_END = ATOM_INVALID\n\n for (i = ATOM_BEGIN; i < ATOM_END; i++) {\n }\n\nBut ATOM_END has been used...\n\n--\nZheNing Hu\n"},{"id":"424128","messageId":"xmqqbl9hmne2.fsf@gitster.g","threadId":"55647","inReplyTo":"CAP8UFD3m0TA3nPDaPhaU3=FVF_r5cPM8xksVL6MpCOhNC4dT3g@mail.gmail.com","subject":"Re: [PATCH v2 2/2] [GSOC] ref-filter: introduce enum atom_type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-11T13:05:25Z","receivedAt":"2021-05-11T13:05:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> If it's -1 instead of 0, then it might be a bit more complex to\n> initialize structs that contain such a field, as it cannot be done\n> with only xcalloc().\n\nIn general, yes, but I would have thought that the codepath that\nallocates used_atom[] elements are pretty isolated---it is not like\nthere are random xcalloc() all over the code. In fact, there is\nonly one \"used_atom_cnt++\" in the whole file, which means there is\nonly one place that (re)allocates the array.\n\n\t...\n\tat = used_atom_cnt;\n\tused_atom_cnt++;\n\tREALLOC_ARRAY(used_atom, used_atom_cnt);\n\tused_atom[at].name = xmemdupz(atom, ep - atom);\n\tused_atom[at].type = valid_atom[i].cmp_type;\n\tused_atom[at].source = valid_atom[i].source;\n\t...\n\nSo, I do not think there is even any need to worry about \"initialize\nto invalid and fill it in as we discover what it really is\"; if\nthere were such a use pattern, UNKNOWN would be handy, but that is\nnot what we are dealing with here. In the above snippet, we already\nhave found from which valid_atom[] element to instantiate the new\nelement in used_atom[] array.\n\n>> > + * ATOM_INVALID equals to the size of valid_atom array, which could help us\n>> > + * iterate over valid_atom array like this:\n>> > + *\n>> > + * for (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++) {\n>>\n>> I find it far more intuitive to say\n>>\n>> for (i = 0; i < ATOM_INVALID; i++)\n>>\n>> than having to say UNKNOWN+1.\n\nAnd here I was being embarrassingly silly.\n\nAs long as we do not waste any entry in valid_atom[] with leading\ngap, trailing gap or gap in the middle, the way to iterate over such\nan array is\n\n\tfor (i = 0; i < ARRAY_SIZE(valid_atom); i++)\n\nhence, there is no need for ATOM_MAX, and no need to burden us to\nremember that UNKNOWN is near the bottom of the range, and INVALID\nis near the top of the range.\n\n"},{"id":"424316","messageId":"pull.951.v3.git.1620821464.gitgitgadget@gmail.com","threadId":"55647","inReplyTo":"pull.951.v2.git.1620659000.gitgitgadget@gmail.com","subject":"[PATCH v3 0/2] [GSOC][RFC] ref-filter: introduce enum atom_type","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-12T12:11:02Z","receivedAt":"2021-05-12T12:11:12Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Change from last version: Since used_atom obtains the index from valid_atom\nas its atom_type after init in parse_ref_filter_atom(), here is the only\nplace where used_atom is initialized in the global, ATOM_INVALID and\nATOM_UNKNOWN seem to cause unnecessary trouble.\n\nSo remove ATOM_INVALID and ATOM_UNKNOWN, Use the original method of\ntraversing valid_atom.\n\nZheNing Hu (2):\n [GSOC] ref-filter: add objectsize to used_atom\n [GSOC] ref-filter: introduce enum atom_type\n\n ref-filter.c | 214 +++++++++++++++++++++++++++++++++------------------\n 1 file changed, 140 insertions(+), 74 deletions(-)\n\n\nbase-commit: 7e391989789db82983665667013a46eabc6fc570\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-951%2Fadlternative%2Fref-filter-atom-type-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-951/adlternative/ref-filter-atom-type-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/951\n\nRange-diff vs v2:\n\n 1: 91ca57c9d04a = 1: 91ca57c9d04a [GSOC] ref-filter: add objectsize to used_atom\n 2: a1f70b39b7ef ! 2: 43400cac58e7 [GSOC] ref-filter: introduce enum atom_type\n @@ Commit message\n `used_atom.atom_type` will record corresponding enum value\n from valid_atom entry index, and then in specific reference\n attribute filling step, only need to compare the value of\n - the `used_atom.atom_type` to judge the atom type.\n -\n - the enum value of `ATOM_UNKNOWN` is equals to zero, which\n - could ensure that we can easily distinguish such a struct\n - where the atom_type is known from such a struct where it\n - is unknown yet.\n -\n - the enum value of `ATOM_INVALID` is equals to the size of\n - valid_atom array, which could help us iterate over\n - valid_atom array using something like:\n -\n - for (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++)\n - /* do something with valid_atom[i] */;\n + the `used_atom[i].atom_type` to judge the atom type.\n \n + Helped-by: Junio C Hamano \n + Helped-by: Christian Couder \n Signed-off-by: ZheNing Hu \n \n ## ref-filter.c ##\n @@ ref-filter.c: static struct ref_to_worktree_map {\n + * In the atom parsing stage, it will be passed to used_atom.atom_type\n + * as the identifier of the atom type. We can judge the type of used_atom\n + * entry by `if (used_atom[i].atom_type == ATOM_*)`.\n -+ *\n -+ * ATOM_UNKNOWN equals to 0, used as an enumeration value of uninitialized\n -+ * atom_type.\n -+ * ATOM_INVALID equals to the size of valid_atom array, which could help us\n -+ * iterate over valid_atom array like this:\n -+ *\n -+ * \tfor (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++) {\n -+ *\t\tint len = strlen(valid_atom[i].name);\n -+ *\t\tif (len == atom_len && !memcmp(valid_atom[i].name, sp, len))\n -+ *\t\t\tbreak;\n -+ *\t}\n + */\n +enum atom_type {\n -+ATOM_UNKNOWN,\n -+ATOM_REFNAME,\n -+ATOM_OBJECTTYPE,\n -+ATOM_OBJECTSIZE,\n -+ATOM_OBJECTNAME,\n -+ATOM_DELTABASE,\n -+ATOM_TREE,\n -+ATOM_PARENT,\n -+ATOM_NUMPARENT,\n -+ATOM_OBJECT,\n -+ATOM_TYPE,\n -+ATOM_TAG,\n -+ATOM_AUTHOR,\n -+ATOM_AUTHORNAME,\n -+ATOM_AUTHOREMAIL,\n -+ATOM_AUTHORDATE,\n -+ATOM_COMMITTER,\n -+ATOM_COMMITTERNAME,\n -+ATOM_COMMITTEREMAIL,\n -+ATOM_COMMITTERDATE,\n -+ATOM_TAGGER,\n -+ATOM_TAGGERNAME,\n -+ATOM_TAGGEREMAIL,\n -+ATOM_TAGGERDATE,\n -+ATOM_CREATOR,\n -+ATOM_CREATORDATE,\n -+ATOM_SUBJECT,\n -+ATOM_BODY,\n -+ATOM_TRAILERS,\n -+ATOM_CONTENTS,\n -+ATOM_UPSTREAM,\n -+ATOM_PUSH,\n -+ATOM_SYMREF,\n -+ATOM_FLAG,\n -+ATOM_HEAD,\n -+ATOM_COLOR,\n -+ATOM_WORKTREEPATH,\n -+ATOM_ALIGN,\n -+ATOM_END,\n -+ATOM_IF,\n -+ATOM_THEN,\n -+ATOM_ELSE,\n -+ATOM_INVALID,\n ++\tATOM_REFNAME,\n ++\tATOM_OBJECTTYPE,\n ++\tATOM_OBJECTSIZE,\n ++\tATOM_OBJECTNAME,\n ++\tATOM_DELTABASE,\n ++\tATOM_TREE,\n ++\tATOM_PARENT,\n ++\tATOM_NUMPARENT,\n ++\tATOM_OBJECT,\n ++\tATOM_TYPE,\n ++\tATOM_TAG,\n ++\tATOM_AUTHOR,\n ++\tATOM_AUTHORNAME,\n ++\tATOM_AUTHOREMAIL,\n ++\tATOM_AUTHORDATE,\n ++\tATOM_COMMITTER,\n ++\tATOM_COMMITTERNAME,\n ++\tATOM_COMMITTEREMAIL,\n ++\tATOM_COMMITTERDATE,\n ++\tATOM_TAGGER,\n ++\tATOM_TAGGERNAME,\n ++\tATOM_TAGGEREMAIL,\n ++\tATOM_TAGGERDATE,\n ++\tATOM_CREATOR,\n ++\tATOM_CREATORDATE,\n ++\tATOM_SUBJECT,\n ++\tATOM_BODY,\n ++\tATOM_TRAILERS,\n ++\tATOM_CONTENTS,\n ++\tATOM_UPSTREAM,\n ++\tATOM_PUSH,\n ++\tATOM_SYMREF,\n ++\tATOM_FLAG,\n ++\tATOM_HEAD,\n ++\tATOM_COLOR,\n ++\tATOM_WORKTREEPATH,\n ++\tATOM_ALIGN,\n ++\tATOM_END,\n ++\tATOM_IF,\n ++\tATOM_THEN,\n ++\tATOM_ELSE,\n +};\n +\n /*\n @@ ref-filter.c: static struct {\n \t/*\n \t * Please update $__git_ref_fieldlist in git-completion.bash\n \t * when you add new atoms\n -@@ ref-filter.c: static int parse_ref_filter_atom(const struct ref_format *format,\n - \tatom_len = (arg ? arg : ep) - sp;\n - \n - \t/* Is the atom a valid one? */\n --\tfor (i = 0; i < ARRAY_SIZE(valid_atom); i++) {\n -+\tfor (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++) {\n - \t\tint len = strlen(valid_atom[i].name);\n - \t\tif (len == atom_len && !memcmp(valid_atom[i].name, sp, len))\n - \t\t\tbreak;\n - \t}\n - \n --\tif (ARRAY_SIZE(valid_atom) <= i)\n -+\tif (i == ATOM_INVALID)\n - \t\treturn strbuf_addf_ret(err, -1, _(\"unknown field name: %.*s\"),\n - \t\t\t\t (int)(ep-atom), atom);\n - \tif (valid_atom[i].source != SOURCE_NONE && !have_git_dir())\n @@ ref-filter.c: static int parse_ref_filter_atom(const struct ref_format *format,\n \tat = used_atom_cnt;\n \tused_atom_cnt++;\n\n-- \ngitgitgadget\n"},{"id":"424317","messageId":"91ca57c9d04a822aa4955dbfe3962a6fb2444e81.1620821464.git.gitgitgadget@gmail.com","threadId":"55647","inReplyTo":"pull.951.v3.git.1620821464.gitgitgadget@gmail.com","subject":"[PATCH v3 1/2] [GSOC] ref-filter: add objectsize to used_atom","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-12T12:11:03Z","receivedAt":"2021-05-12T12:11:15Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nSince \"objectsize:size\" is composed of two parts,\n\"type:attribute\". However, the original implementation\ndid not decouple the two parts \"type\" and \"attribute\" well,\nwe still need to judge separately whether the atom is\n\"objectsize\" or \"objectsize:disk\" in `grab_common_values()`.\n\nAdd a new member `objectsize` to the union `used_atom.u`,\nso that we can separate the judgment of atom type from the\njudgment of atom attribute, This will bring scalability to\natom `%(objectsize)`.\n\nSigned-off-by: ZheNing Hu \n---\n ref-filter.c | 19 +++++++++++++------\n 1 file changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex a0adb4551d87..f420bae6e5ba 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -146,6 +146,9 @@ static struct used_atom {\n \t\t\tenum { O_FULL, O_LENGTH, O_SHORT } option;\n \t\t\tunsigned int length;\n \t\t} oid;\n+\t\tstruct {\n+\t\t\tenum { O_SIZE, O_SIZE_DISK } option;\n+\t\t} objectsize;\n \t\tstruct email_option {\n \t\t\tenum { EO_RAW, EO_TRIM, EO_LOCALPART } option;\n \t\t} email_option;\n@@ -269,11 +272,13 @@ static int objectsize_atom_parser(const struct ref_format *format, struct used_a\n \t\t\t\t const char *arg, struct strbuf *err)\n {\n \tif (!arg) {\n+\t\tatom->u.objectsize.option = O_SIZE;\n \t\tif (*atom->name == '*')\n \t\t\toi_deref.info.sizep = &oi_deref.size;\n \t\telse\n \t\t\toi.info.sizep = &oi.size;\n \t} else if (!strcmp(arg, \"disk\")) {\n+\t\tatom->u.objectsize.option = O_SIZE_DISK;\n \t\tif (*atom->name == '*')\n \t\t\toi_deref.info.disk_sizep = &oi_deref.disk_size;\n \t\telse\n@@ -967,12 +972,14 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \t\t\tname++;\n \t\tif (!strcmp(name, \"objecttype\"))\n \t\t\tv->s = xstrdup(type_name(oi->type));\n-\t\telse if (!strcmp(name, \"objectsize:disk\")) {\n-\t\t\tv->value = oi->disk_size;\n-\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n-\t\t} else if (!strcmp(name, \"objectsize\")) {\n-\t\t\tv->value = oi->size;\n-\t\t\tv->s = xstrfmt(\"%\"PRIuMAX , (uintmax_t)oi->size);\n+\t\telse if (starts_with(name, \"objectsize\")) {\n+\t\t\tif (used_atom[i].u.objectsize.option == O_SIZE_DISK) {\n+\t\t\t\tv->value = oi->disk_size;\n+\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n+\t\t\t} else if (used_atom[i].u.objectsize.option == O_SIZE) {\n+\t\t\t\tv->value = oi->size;\n+\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX , (uintmax_t)oi->size);\n+\t\t\t}\n \t\t} else if (!strcmp(name, \"deltabase\"))\n \t\t\tv->s = xstrdup(oid_to_hex(&oi->delta_base_oid));\n \t\telse if (deref)\n-- \ngitgitgadget\n\n"},{"id":"424318","messageId":"43400cac58e74a2acab15ace929481a9efb7978f.1620821464.git.gitgitgadget@gmail.com","threadId":"55647","inReplyTo":"pull.951.v3.git.1620821464.gitgitgadget@gmail.com","subject":"[PATCH v3 2/2] [GSOC] ref-filter: introduce enum atom_type","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-12T12:11:04Z","receivedAt":"2021-05-12T12:11:15Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nIn the original ref-filter design, it will copy the parsed\natom's name and attributes to `used_atom[i].name` in the\natom's parsing step, and use it again for string matching\nin the later specific ref attributes filling step. It use\na lot of string matching to determine which atom we need.\n\nIntroduce the enum \"atom_type\", each enum value is named\nas `ATOM_*`, which is the index of each corresponding\nvalid_atom entry. In the first step of the atom parsing,\n`used_atom.atom_type` will record corresponding enum value\nfrom valid_atom entry index, and then in specific reference\nattribute filling step, only need to compare the value of\nthe `used_atom[i].atom_type` to judge the atom type.\n\nHelped-by: Junio C Hamano \nHelped-by: Christian Couder \nSigned-off-by: ZheNing Hu \n---\n ref-filter.c | 197 +++++++++++++++++++++++++++++++++------------------\n 1 file changed, 128 insertions(+), 69 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex f420bae6e5ba..cf36716b76e3 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -108,6 +108,56 @@ static struct ref_to_worktree_map {\n \tstruct worktree **worktrees;\n } ref_to_worktree_map;\n \n+/*\n+ * The enum atom_type is used as the coordinates of valid_atom entry.\n+ * In the atom parsing stage, it will be passed to used_atom.atom_type\n+ * as the identifier of the atom type. We can judge the type of used_atom\n+ * entry by `if (used_atom[i].atom_type == ATOM_*)`.\n+ */\n+enum atom_type {\n+\tATOM_REFNAME,\n+\tATOM_OBJECTTYPE,\n+\tATOM_OBJECTSIZE,\n+\tATOM_OBJECTNAME,\n+\tATOM_DELTABASE,\n+\tATOM_TREE,\n+\tATOM_PARENT,\n+\tATOM_NUMPARENT,\n+\tATOM_OBJECT,\n+\tATOM_TYPE,\n+\tATOM_TAG,\n+\tATOM_AUTHOR,\n+\tATOM_AUTHORNAME,\n+\tATOM_AUTHOREMAIL,\n+\tATOM_AUTHORDATE,\n+\tATOM_COMMITTER,\n+\tATOM_COMMITTERNAME,\n+\tATOM_COMMITTEREMAIL,\n+\tATOM_COMMITTERDATE,\n+\tATOM_TAGGER,\n+\tATOM_TAGGERNAME,\n+\tATOM_TAGGEREMAIL,\n+\tATOM_TAGGERDATE,\n+\tATOM_CREATOR,\n+\tATOM_CREATORDATE,\n+\tATOM_SUBJECT,\n+\tATOM_BODY,\n+\tATOM_TRAILERS,\n+\tATOM_CONTENTS,\n+\tATOM_UPSTREAM,\n+\tATOM_PUSH,\n+\tATOM_SYMREF,\n+\tATOM_FLAG,\n+\tATOM_HEAD,\n+\tATOM_COLOR,\n+\tATOM_WORKTREEPATH,\n+\tATOM_ALIGN,\n+\tATOM_END,\n+\tATOM_IF,\n+\tATOM_THEN,\n+\tATOM_ELSE,\n+};\n+\n /*\n * An atom is a valid field atom listed below, possibly prefixed with\n * a \"*\" to denote deref_tag().\n@@ -119,6 +169,7 @@ static struct ref_to_worktree_map {\n * array.\n */\n static struct used_atom {\n+\tenum atom_type atom_type;\n \tconst char *name;\n \tcmp_type type;\n \tinfo_source source;\n@@ -506,47 +557,47 @@ static struct {\n \tint (*parser)(const struct ref_format *format, struct used_atom *atom,\n \t\t const char *arg, struct strbuf *err);\n } valid_atom[] = {\n-\t{ \"refname\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n-\t{ \"objecttype\", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser },\n-\t{ \"objectsize\", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },\n-\t{ \"objectname\", SOURCE_OTHER, FIELD_STR, oid_atom_parser },\n-\t{ \"deltabase\", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },\n-\t{ \"tree\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n-\t{ \"parent\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n-\t{ \"numparent\", SOURCE_OBJ, FIELD_ULONG },\n-\t{ \"object\", SOURCE_OBJ },\n-\t{ \"type\", SOURCE_OBJ },\n-\t{ \"tag\", SOURCE_OBJ },\n-\t{ \"author\", SOURCE_OBJ },\n-\t{ \"authorname\", SOURCE_OBJ },\n-\t{ \"authoremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n-\t{ \"authordate\", SOURCE_OBJ, FIELD_TIME },\n-\t{ \"committer\", SOURCE_OBJ },\n-\t{ \"committername\", SOURCE_OBJ },\n-\t{ \"committeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n-\t{ \"committerdate\", SOURCE_OBJ, FIELD_TIME },\n-\t{ \"tagger\", SOURCE_OBJ },\n-\t{ \"taggername\", SOURCE_OBJ },\n-\t{ \"taggeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n-\t{ \"taggerdate\", SOURCE_OBJ, FIELD_TIME },\n-\t{ \"creator\", SOURCE_OBJ },\n-\t{ \"creatordate\", SOURCE_OBJ, FIELD_TIME },\n-\t{ \"subject\", SOURCE_OBJ, FIELD_STR, subject_atom_parser },\n-\t{ \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n-\t{ \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n-\t{ \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n-\t{ \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n-\t{ \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n-\t{ \"symref\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n-\t{ \"flag\", SOURCE_NONE },\n-\t{ \"HEAD\", SOURCE_NONE, FIELD_STR, head_atom_parser },\n-\t{ \"color\", SOURCE_NONE, FIELD_STR, color_atom_parser },\n-\t{ \"worktreepath\", SOURCE_NONE },\n-\t{ \"align\", SOURCE_NONE, FIELD_STR, align_atom_parser },\n-\t{ \"end\", SOURCE_NONE },\n-\t{ \"if\", SOURCE_NONE, FIELD_STR, if_atom_parser },\n-\t{ \"then\", SOURCE_NONE },\n-\t{ \"else\", SOURCE_NONE },\n+\t[ATOM_REFNAME] = { \"refname\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n+\t[ATOM_OBJECTTYPE] = { \"objecttype\", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser },\n+\t[ATOM_OBJECTSIZE] = { \"objectsize\", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },\n+\t[ATOM_OBJECTNAME] = { \"objectname\", SOURCE_OTHER, FIELD_STR, oid_atom_parser },\n+\t[ATOM_DELTABASE] = { \"deltabase\", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },\n+\t[ATOM_TREE] = { \"tree\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n+\t[ATOM_PARENT] = { \"parent\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n+\t[ATOM_NUMPARENT] = { \"numparent\", SOURCE_OBJ, FIELD_ULONG },\n+\t[ATOM_OBJECT] = { \"object\", SOURCE_OBJ },\n+\t[ATOM_TYPE] = { \"type\", SOURCE_OBJ },\n+\t[ATOM_TAG] = { \"tag\", SOURCE_OBJ },\n+\t[ATOM_AUTHOR] = { \"author\", SOURCE_OBJ },\n+\t[ATOM_AUTHORNAME] = { \"authorname\", SOURCE_OBJ },\n+\t[ATOM_AUTHOREMAIL] = { \"authoremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n+\t[ATOM_AUTHORDATE] = { \"authordate\", SOURCE_OBJ, FIELD_TIME },\n+\t[ATOM_COMMITTER] = { \"committer\", SOURCE_OBJ },\n+\t[ATOM_COMMITTERNAME] = { \"committername\", SOURCE_OBJ },\n+\t[ATOM_COMMITTEREMAIL] = { \"committeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n+\t[ATOM_COMMITTERDATE] = { \"committerdate\", SOURCE_OBJ, FIELD_TIME },\n+\t[ATOM_TAGGER] = { \"tagger\", SOURCE_OBJ },\n+\t[ATOM_TAGGERNAME] = { \"taggername\", SOURCE_OBJ },\n+\t[ATOM_TAGGEREMAIL] = { \"taggeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n+\t[ATOM_TAGGERDATE] = { \"taggerdate\", SOURCE_OBJ, FIELD_TIME },\n+\t[ATOM_CREATOR] = { \"creator\", SOURCE_OBJ },\n+\t[ATOM_CREATORDATE] = { \"creatordate\", SOURCE_OBJ, FIELD_TIME },\n+\t[ATOM_SUBJECT] = { \"subject\", SOURCE_OBJ, FIELD_STR, subject_atom_parser },\n+\t[ATOM_BODY] = { \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n+\t[ATOM_TRAILERS] = { \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n+\t[ATOM_CONTENTS] = { \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n+\t[ATOM_UPSTREAM] = { \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n+\t[ATOM_PUSH] = { \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n+\t[ATOM_SYMREF] = { \"symref\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n+\t[ATOM_FLAG] = { \"flag\", SOURCE_NONE },\n+\t[ATOM_HEAD] = { \"HEAD\", SOURCE_NONE, FIELD_STR, head_atom_parser },\n+\t[ATOM_COLOR] = { \"color\", SOURCE_NONE, FIELD_STR, color_atom_parser },\n+\t[ATOM_WORKTREEPATH] = { \"worktreepath\", SOURCE_NONE },\n+\t[ATOM_ALIGN] = { \"align\", SOURCE_NONE, FIELD_STR, align_atom_parser },\n+\t[ATOM_END] = { \"end\", SOURCE_NONE },\n+\t[ATOM_IF] = { \"if\", SOURCE_NONE, FIELD_STR, if_atom_parser },\n+\t[ATOM_THEN] = { \"then\", SOURCE_NONE },\n+\t[ATOM_ELSE] = { \"else\", SOURCE_NONE },\n \t/*\n \t * Please update $__git_ref_fieldlist in git-completion.bash\n \t * when you add new atoms\n@@ -628,6 +679,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n \tat = used_atom_cnt;\n \tused_atom_cnt++;\n \tREALLOC_ARRAY(used_atom, used_atom_cnt);\n+\tused_atom[at].atom_type = i;\n \tused_atom[at].name = xmemdupz(atom, ep - atom);\n \tused_atom[at].type = valid_atom[i].cmp_type;\n \tused_atom[at].source = valid_atom[i].source;\n@@ -652,7 +704,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n \t\treturn -1;\n \tif (*atom == '*')\n \t\tneed_tagged = 1;\n-\tif (!strcmp(valid_atom[i].name, \"symref\"))\n+\tif (i == ATOM_SYMREF)\n \t\tneed_symref = 1;\n \treturn at;\n }\n@@ -965,14 +1017,15 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n-\t\tif (!strcmp(name, \"objecttype\"))\n+\t\tif (atom_type == ATOM_OBJECTTYPE)\n \t\t\tv->s = xstrdup(type_name(oi->type));\n-\t\telse if (starts_with(name, \"objectsize\")) {\n+\t\telse if (atom_type == ATOM_OBJECTSIZE) {\n \t\t\tif (used_atom[i].u.objectsize.option == O_SIZE_DISK) {\n \t\t\t\tv->value = oi->disk_size;\n \t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n@@ -980,9 +1033,9 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \t\t\t\tv->value = oi->size;\n \t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX , (uintmax_t)oi->size);\n \t\t\t}\n-\t\t} else if (!strcmp(name, \"deltabase\"))\n+\t\t} else if (atom_type == ATOM_DELTABASE)\n \t\t\tv->s = xstrdup(oid_to_hex(&oi->delta_base_oid));\n-\t\telse if (deref)\n+\t\telse if (atom_type == ATOM_OBJECTNAME && deref)\n \t\t\tgrab_oid(name, \"objectname\", &oi->oid, v, &used_atom[i]);\n \t}\n }\n@@ -995,16 +1048,17 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n-\t\tif (!strcmp(name, \"tag\"))\n+\t\tif (atom_type == ATOM_TAG)\n \t\t\tv->s = xstrdup(tag->tag);\n-\t\telse if (!strcmp(name, \"type\") && tag->tagged)\n+\t\telse if (atom_type == ATOM_TYPE && tag->tagged)\n \t\t\tv->s = xstrdup(type_name(tag->tagged->type));\n-\t\telse if (!strcmp(name, \"object\") && tag->tagged)\n+\t\telse if (atom_type == ATOM_OBJECT && tag->tagged)\n \t\t\tv->s = xstrdup(oid_to_hex(&tag->tagged->oid));\n \t}\n }\n@@ -1017,18 +1071,20 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n-\t\tif (grab_oid(name, \"tree\", get_commit_tree_oid(commit), v, &used_atom[i]))\n+\t\tif (atom_type == ATOM_TREE &&\n+\t\t grab_oid(name, \"tree\", get_commit_tree_oid(commit), v, &used_atom[i]))\n \t\t\tcontinue;\n-\t\tif (!strcmp(name, \"numparent\")) {\n+\t\tif (atom_type == ATOM_NUMPARENT) {\n \t\t\tv->value = commit_list_count(commit->parents);\n \t\t\tv->s = xstrfmt(\"%lu\", (unsigned long)v->value);\n \t\t}\n-\t\telse if (starts_with(name, \"parent\")) {\n+\t\telse if (atom_type == ATOM_PARENT) {\n \t\t\tstruct commit_list *parents;\n \t\t\tstruct strbuf s = STRBUF_INIT;\n \t\t\tfor (parents = commit->parents; parents; parents = parents->next) {\n@@ -1208,15 +1264,16 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n \t\treturn;\n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n \n-\t\tif (starts_with(name, \"creatordate\"))\n+\t\tif (atom_type == ATOM_CREATORDATE)\n \t\t\tgrab_date(wholine, v, name);\n-\t\telse if (!strcmp(name, \"creator\"))\n+\t\telse if (atom_type == ATOM_CREATOR)\n \t\t\tv->s = copy_line(wholine);\n \t}\n }\n@@ -1696,6 +1753,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t/* Fill in specials first */\n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tstruct used_atom *atom = &used_atom[i];\n+\t\tenum atom_type atom_type = atom->atom_type;\n \t\tconst char *name = used_atom[i].name;\n \t\tstruct atom_value *v = &ref->value[i];\n \t\tint deref = 0;\n@@ -1710,18 +1768,18 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\tname++;\n \t\t}\n \n-\t\tif (starts_with(name, \"refname\"))\n+\t\tif (atom_type == ATOM_REFNAME)\n \t\t\trefname = get_refname(atom, ref);\n-\t\telse if (!strcmp(name, \"worktreepath\")) {\n+\t\telse if (atom_type == ATOM_WORKTREEPATH) {\n \t\t\tif (ref->kind == FILTER_REFS_BRANCHES)\n \t\t\t\tv->s = get_worktree_path(atom, ref);\n \t\t\telse\n \t\t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n \t\t}\n-\t\telse if (starts_with(name, \"symref\"))\n+\t\telse if (atom_type == ATOM_SYMREF)\n \t\t\trefname = get_symref(atom, ref);\n-\t\telse if (starts_with(name, \"upstream\")) {\n+\t\telse if (atom_type == ATOM_UPSTREAM) {\n \t\t\tconst char *branch_name;\n \t\t\t/* only local branches may have an upstream */\n \t\t\tif (!skip_prefix(ref->refname, \"refs/heads/\",\n@@ -1737,7 +1795,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\telse\n \t\t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-\t\t} else if (atom->u.remote_ref.push) {\n+\t\t} else if (atom_type == ATOM_PUSH && atom->u.remote_ref.push) {\n \t\t\tconst char *branch_name;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tif (!skip_prefix(ref->refname, \"refs/heads/\",\n@@ -1756,10 +1814,10 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\tfree((char *)v->s);\n \t\t\tfill_remote_ref_details(atom, refname, branch, &v->s);\n \t\t\tcontinue;\n-\t\t} else if (starts_with(name, \"color:\")) {\n+\t\t} else if (atom_type == ATOM_COLOR) {\n \t\t\tv->s = xstrdup(atom->u.color);\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"flag\")) {\n+\t\t} else if (atom_type == ATOM_FLAG) {\n \t\t\tchar buf[256], *cp = buf;\n \t\t\tif (ref->flag & REF_ISSYMREF)\n \t\t\t\tcp = copy_advance(cp, \",symref\");\n@@ -1772,23 +1830,24 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\t\tv->s = xstrdup(buf + 1);\n \t\t\t}\n \t\t\tcontinue;\n-\t\t} else if (!deref && grab_oid(name, \"objectname\", &ref->objectname, v, atom)) {\n-\t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"HEAD\")) {\n+\t\t} else if (!deref && atom_type == ATOM_OBJECTNAME &&\n+\t\t\t grab_oid(name, \"objectname\", &ref->objectname, v, atom)) {\n+\t\t\t\tcontinue;\n+\t\t} else if (atom_type == ATOM_HEAD) {\n \t\t\tif (atom->u.head && !strcmp(ref->refname, atom->u.head))\n \t\t\t\tv->s = xstrdup(\"*\");\n \t\t\telse\n \t\t\t\tv->s = xstrdup(\" \");\n \t\t\tcontinue;\n-\t\t} else if (starts_with(name, \"align\")) {\n+\t\t} else if (atom_type == ATOM_ALIGN) {\n \t\t\tv->handler = align_atom_handler;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"end\")) {\n+\t\t} else if (atom_type == ATOM_END) {\n \t\t\tv->handler = end_atom_handler;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-\t\t} else if (starts_with(name, \"if\")) {\n+\t\t} else if (atom_type == ATOM_IF) {\n \t\t\tconst char *s;\n \t\t\tif (skip_prefix(name, \"if:\", &s))\n \t\t\t\tv->s = xstrdup(s);\n@@ -1796,11 +1855,11 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\t\tv->s = xstrdup(\"\");\n \t\t\tv->handler = if_atom_handler;\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"then\")) {\n+\t\t} else if (atom_type == ATOM_THEN) {\n \t\t\tv->handler = then_atom_handler;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"else\")) {\n+\t\t} else if (atom_type == ATOM_ELSE) {\n \t\t\tv->handler = else_atom_handler;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-- \ngitgitgadget\n"},{"id":"424395","messageId":"xmqqpmxveee9.fsf@gitster.g","threadId":"55647","inReplyTo":"91ca57c9d04a822aa4955dbfe3962a6fb2444e81.1620821464.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 1/2] [GSOC] ref-filter: add objectsize to used_atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-12T23:11:26Z","receivedAt":"2021-05-12T23:28:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" writes:\n\n> From: ZheNing Hu \n>\n> Since \"objectsize:size\" is composed of two parts,\n> \"type:attribute\". However, the original implementation\n\nY is missing in the above \"Since X, Y\". Because it is composed of\ntwo parts, what?\n\nDropping \"Since \" would make the sentence work better. Also, I\nthink it should be \"objectsize:disk\".\n\n> did not decouple the two parts \"type\" and \"attribute\" well,\n> we still need to judge separately whether the atom is\n> \"objectsize\" or \"objectsize:disk\" in `grab_common_values()`.\n\nPerhaps\n\n\tWhen the support for \"objectsize:disk\" was bolted onto the\n\texisting support for \"objectsize\", it didn't follow the\n\tusual pattern for handling \"atomtype:modifier\", which reads\n\tthe part just once while parsing the format\n\tstring, and store the parsed result in the union in the\n\tused_atom structure, so that the string form of it does not\n\thave to be parsed over and over at runtime (e.g. in\n\tgrab_common_values()).\n\n> Add a new member `objectsize` to the union `used_atom.u`,\n> so that we can separate the judgment of atom type from the\n> judgment of atom attribute, This will bring scalability to\n> atom `%(objectsize)`.\n\nOK.\n\n> +\t\tstruct {\n> +\t\t\tenum { O_SIZE, O_SIZE_DISK } option;\n> +\t\t} objectsize;\n\nOK.\n\n> @@ -967,12 +972,14 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n> \t\t\tname++;\n> \t\tif (!strcmp(name, \"objecttype\"))\n> \t\t\tv->s = xstrdup(type_name(oi->type));\n> +\t\telse if (starts_with(name, \"objectsize\")) {\n> +\t\t\tif (used_atom[i].u.objectsize.option == O_SIZE_DISK) {\n> +\t\t\t\tv->value = oi->disk_size;\n> +\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n> +\t\t\t} else if (used_atom[i].u.objectsize.option == O_SIZE) {\n> +\t\t\t\tv->value = oi->size;\n> +\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX , (uintmax_t)oi->size);\n> +\t\t\t}\n\nOK.\n"},{"id":"424397","messageId":"xmqqk0o3edxw.fsf@gitster.g","threadId":"55647","inReplyTo":"43400cac58e74a2acab15ace929481a9efb7978f.1620821464.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 2/2] [GSOC] ref-filter: introduce enum atom_type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-12T23:21:15Z","receivedAt":"2021-05-12T23:32:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" writes:\n\n> +/*\n> + * The enum atom_type is used as the coordinates of valid_atom entry.\n\nUsually we do not say \"coordinates\" when we talk about X of an array\nelement A[X]. \"... used as an index in the valid_atom[] array.\" perhaps.\n\n> + * In the atom parsing stage, it will be passed to used_atom.atom_type\n> + * as the identifier of the atom type. We can judge the type of used_atom\n\nYou seem to like the verb \"judge\" (it was also seen in the proposed\nlog message for 1/2) and tend to overuse it when we use other verbs;\nin this particular case, probably the right verb is to \"check\".\n\n> + * entry by `if (used_atom[i].atom_type == ATOM_*)`.\n> + */\n> +enum atom_type {\n> +\tATOM_REFNAME,\n> +...\n> +\tATOM_ELSE,\n> +};\n> +\n> @@ -119,6 +169,7 @@ static struct ref_to_worktree_map {\n> * array.\n> */\n> static struct used_atom {\n> +\tenum atom_type atom_type;\n> \tconst char *name;\n> \tcmp_type type;\n> \tinfo_source source;\n\nOK.\n\n> @@ -506,47 +557,47 @@ static struct {\n> \tint (*parser)(const struct ref_format *format, struct used_atom *atom,\n> \t\t const char *arg, struct strbuf *err);\n> } valid_atom[] = {\n> -\t{ \"refname\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n> ...\n> -\t{ \"else\", SOURCE_NONE },\n> +\t[ATOM_REFNAME] = { \"refname\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n> ...\n> +\t[ATOM_ELSE] = { \"else\", SOURCE_NONE },\n\nMakes sense.\n\n> @@ -628,6 +679,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n> \tat = used_atom_cnt;\n> \tused_atom_cnt++;\n> \tREALLOC_ARRAY(used_atom, used_atom_cnt);\n> +\tused_atom[at].atom_type = i;\n\nMakes sense.\n\n> \tused_atom[at].name = xmemdupz(atom, ep - atom);\n> \tused_atom[at].type = valid_atom[i].cmp_type;\n> \tused_atom[at].source = valid_atom[i].source;\n> @@ -652,7 +704,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n> \t\treturn -1;\n> \tif (*atom == '*')\n> \t\tneed_tagged = 1;\n> -\tif (!strcmp(valid_atom[i].name, \"symref\"))\n> +\tif (i == ATOM_SYMREF)\n> \t\tneed_symref = 1;\n> \treturn at;\n> }\n\nNice.\n\n> @@ -965,14 +1017,15 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n> \n> \tfor (i = 0; i < used_atom_cnt; i++) {\n> \t\tconst char *name = used_atom[i].name;\n> +\t\tenum atom_type atom_type = used_atom[i].atom_type;\n> \t\tstruct atom_value *v = &val[i];\n> \t\tif (!!deref != (*name == '*'))\n> \t\t\tcontinue;\n> \t\tif (deref)\n> \t\t\tname++;\n> -\t\tif (!strcmp(name, \"objecttype\"))\n> +\t\tif (atom_type == ATOM_OBJECTTYPE)\n> \t\t\tv->s = xstrdup(type_name(oi->type));\n> -\t\telse if (starts_with(name, \"objectsize\")) {\n> +\t\telse if (atom_type == ATOM_OBJECTSIZE) {\n> \t\t\tif (used_atom[i].u.objectsize.option == O_SIZE_DISK) {\n> \t\t\t\tv->value = oi->disk_size;\n> \t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n\nReplacing !strcmp() with comparison with ATOM_* like the above is\nthe best solution for this step, but I wonder if this part (or any\nother part that this patch touches) would benefit from using a\nswitch statement on atom_type. Something to think about in the\nfuture, after the dust settles.\n\nThanks.\n"},{"id":"424449","messageId":"CAOLTT8T9iwAk1vyQVa_kM5zdym6-UJ0Fi6gWJp=YnSAVOiCDsw@mail.gmail.com","threadId":"55647","inReplyTo":"xmqqpmxveee9.fsf@gitster.g","subject":"Re: [PATCH v3 1/2] [GSOC] ref-filter: add objectsize to used_atom","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-05-13T09:04:36Z","receivedAt":"2021-05-13T09:04:57Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano 于2021年5月13日周四 上午7:11写道：\n>\n> \"ZheNing Hu via GitGitGadget\" writes:\n>\n> > From: ZheNing Hu \n> >\n> > Since \"objectsize:size\" is composed of two parts,\n> > \"type:attribute\". However, the original implementation\n>\n> Y is missing in the above \"Since X, Y\". Because it is composed of\n> two parts, what?\n>\n> Dropping \"Since \" would make the sentence work better. Also, I\n> think it should be \"objectsize:disk\".\n>\n\nOh, here is my typo.\n\n> > did not decouple the two parts \"type\" and \"attribute\" well,\n> > we still need to judge separately whether the atom is\n> > \"objectsize\" or \"objectsize:disk\" in `grab_common_values()`.\n>\n> Perhaps\n>\n> When the support for \"objectsize:disk\" was bolted onto the\n> existing support for \"objectsize\", it didn't follow the\n> usual pattern for handling \"atomtype:modifier\", which reads\n> the part just once while parsing the format\n> string, and store the parsed result in the union in the\n> used_atom structure, so that the string form of it does not\n> have to be parsed over and over at runtime (e.g. in\n> grab_common_values()).\n>\n\nThat’s great.\n\nThanks.\n--\nZheNing Hu\n"},{"id":"424450","messageId":"CAOLTT8TuMJgETg7F7XG2FhFTC318Ag7dJ=8Zi1CG2AwQXtgHvA@mail.gmail.com","threadId":"55647","inReplyTo":"xmqqk0o3edxw.fsf@gitster.g","subject":"Re: [PATCH v3 2/2] [GSOC] ref-filter: introduce enum atom_type","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-05-13T09:25:57Z","receivedAt":"2021-05-13T09:26:13Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano 于2021年5月13日周四 上午7:21写道：\n>\n> \"ZheNing Hu via GitGitGadget\" writes:\n>\n> > +/*\n> > + * The enum atom_type is used as the coordinates of valid_atom entry.\n>\n> Usually we do not say \"coordinates\" when we talk about X of an array\n> element A[X]. \"... used as an index in the valid_atom[] array.\" perhaps.\n>\n\nOk, use index instead of coordinates.\n\n> > + * In the atom parsing stage, it will be passed to used_atom.atom_type\n> > + * as the identifier of the atom type. We can judge the type of used_atom\n>\n> You seem to like the verb \"judge\" (it was also seen in the proposed\n> log message for 1/2) and tend to overuse it when we use other verbs;\n> in this particular case, probably the right verb is to \"check\".\n>\n\nYes, \"judge\" closer to my personal language expression, I will use \"check\".\n\n> > @@ -965,14 +1017,15 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n> >\n> > for (i = 0; i < used_atom_cnt; i++) {\n> > const char *name = used_atom[i].name;\n> > + enum atom_type atom_type = used_atom[i].atom_type;\n> > struct atom_value *v = &val[i];\n> > if (!!deref != (*name == '*'))\n> > continue;\n> > if (deref)\n> > name++;\n> > - if (!strcmp(name, \"objecttype\"))\n> > + if (atom_type == ATOM_OBJECTTYPE)\n> > v->s = xstrdup(type_name(oi->type));\n> > - else if (starts_with(name, \"objectsize\")) {\n> > + else if (atom_type == ATOM_OBJECTSIZE) {\n> > if (used_atom[i].u.objectsize.option == O_SIZE_DISK) {\n> > v->value = oi->disk_size;\n> > v->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n>\n> Replacing !strcmp() with comparison with ATOM_* like the above is\n> the best solution for this step, but I wonder if this part (or any\n> other part that this patch touches) would benefit from using a\n> switch statement on atom_type. Something to think about in the\n> future, after the dust settles.\n>\n> Thanks.\n\nWell, that's right, use switch can increase readability and maintainability.\nThis can be used as a future direction.\n\nThanks.\n--\nZheNing Hu\n"},{"id":"424477","messageId":"pull.951.v4.git.1620918939.gitgitgadget@gmail.com","threadId":"55647","inReplyTo":"pull.951.v3.git.1620821464.gitgitgadget@gmail.com","subject":"[PATCH v4 0/2] [GSOC][RFC] ref-filter: introduce enum atom_type","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-13T15:15:36Z","receivedAt":"2021-05-13T15:15:48Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Change from last version: fix some typo error and change commit message with\nJunio's help.\n\nZheNing Hu (2):\n [GSOC] ref-filter: add objectsize to used_atom\n [GSOC] ref-filter: introduce enum atom_type\n\n ref-filter.c | 214 +++++++++++++++++++++++++++++++++------------------\n 1 file changed, 140 insertions(+), 74 deletions(-)\n\n\nbase-commit: 7e391989789db82983665667013a46eabc6fc570\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-951%2Fadlternative%2Fref-filter-atom-type-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-951/adlternative/ref-filter-atom-type-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/951\n\nRange-diff vs v3:\n\n 1: 91ca57c9d04a ! 1: 23fc04f7eb23 [GSOC] ref-filter: add objectsize to used_atom\n @@ Metadata\n ## Commit message ##\n [GSOC] ref-filter: add objectsize to used_atom\n \n - Since \"objectsize:size\" is composed of two parts,\n - \"type:attribute\". However, the original implementation\n - did not decouple the two parts \"type\" and \"attribute\" well,\n - we still need to judge separately whether the atom is\n - \"objectsize\" or \"objectsize:disk\" in `grab_common_values()`.\n + When the support for \"objectsize:disk\" was bolted onto the\n + existing support for \"objectsize\", it didn't follow the\n + usual pattern for handling \"atomtype:modifier\", which reads\n + the part just once while parsing the format\n + string, and store the parsed result in the union in the\n + used_atom structure, so that the string form of it does not\n + have to be parsed over and over at runtime (e.g. in\n + grab_common_values()).\n \n Add a new member `objectsize` to the union `used_atom.u`,\n - so that we can separate the judgment of atom type from the\n - judgment of atom attribute, This will bring scalability to\n - atom `%(objectsize)`.\n + so that we can separate the check of from the\n + check of , this will bring scalability to atom\n + `%(objectsize)`.\n \n Signed-off-by: ZheNing Hu \n \n 2: 43400cac58e7 ! 2: 50cfe1f0c6c1 [GSOC] ref-filter: introduce enum atom_type\n @@ Commit message\n `used_atom.atom_type` will record corresponding enum value\n from valid_atom entry index, and then in specific reference\n attribute filling step, only need to compare the value of\n - the `used_atom[i].atom_type` to judge the atom type.\n + the `used_atom[i].atom_type` to check the atom type.\n \n Helped-by: Junio C Hamano \n Helped-by: Christian Couder \n @@ ref-filter.c: static struct ref_to_worktree_map {\n } ref_to_worktree_map;\n \n +/*\n -+ * The enum atom_type is used as the coordinates of valid_atom entry.\n ++ * The enum atom_type is used as the index of valid_atom array.\n + * In the atom parsing stage, it will be passed to used_atom.atom_type\n -+ * as the identifier of the atom type. We can judge the type of used_atom\n ++ * as the identifier of the atom type. We can check the type of used_atom\n + * entry by `if (used_atom[i].atom_type == ATOM_*)`.\n + */\n +enum atom_type {\n\n-- \ngitgitgadget\n"},{"id":"424478","messageId":"23fc04f7eb23819473364d8392de95b1e77ddb03.1620918939.git.gitgitgadget@gmail.com","threadId":"55647","inReplyTo":"pull.951.v4.git.1620918939.gitgitgadget@gmail.com","subject":"[PATCH v4 1/2] [GSOC] ref-filter: add objectsize to used_atom","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-13T15:15:37Z","receivedAt":"2021-05-13T15:15:49Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nWhen the support for \"objectsize:disk\" was bolted onto the\nexisting support for \"objectsize\", it didn't follow the\nusual pattern for handling \"atomtype:modifier\", which reads\nthe part just once while parsing the format\nstring, and store the parsed result in the union in the\nused_atom structure, so that the string form of it does not\nhave to be parsed over and over at runtime (e.g. in\ngrab_common_values()).\n\nAdd a new member `objectsize` to the union `used_atom.u`,\nso that we can separate the check of from the\ncheck of , this will bring scalability to atom\n`%(objectsize)`.\n\nSigned-off-by: ZheNing Hu \n---\n ref-filter.c | 19 +++++++++++++------\n 1 file changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex a0adb4551d87..f420bae6e5ba 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -146,6 +146,9 @@ static struct used_atom {\n \t\t\tenum { O_FULL, O_LENGTH, O_SHORT } option;\n \t\t\tunsigned int length;\n \t\t} oid;\n+\t\tstruct {\n+\t\t\tenum { O_SIZE, O_SIZE_DISK } option;\n+\t\t} objectsize;\n \t\tstruct email_option {\n \t\t\tenum { EO_RAW, EO_TRIM, EO_LOCALPART } option;\n \t\t} email_option;\n@@ -269,11 +272,13 @@ static int objectsize_atom_parser(const struct ref_format *format, struct used_a\n \t\t\t\t const char *arg, struct strbuf *err)\n {\n \tif (!arg) {\n+\t\tatom->u.objectsize.option = O_SIZE;\n \t\tif (*atom->name == '*')\n \t\t\toi_deref.info.sizep = &oi_deref.size;\n \t\telse\n \t\t\toi.info.sizep = &oi.size;\n \t} else if (!strcmp(arg, \"disk\")) {\n+\t\tatom->u.objectsize.option = O_SIZE_DISK;\n \t\tif (*atom->name == '*')\n \t\t\toi_deref.info.disk_sizep = &oi_deref.disk_size;\n \t\telse\n@@ -967,12 +972,14 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \t\t\tname++;\n \t\tif (!strcmp(name, \"objecttype\"))\n \t\t\tv->s = xstrdup(type_name(oi->type));\n-\t\telse if (!strcmp(name, \"objectsize:disk\")) {\n-\t\t\tv->value = oi->disk_size;\n-\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n-\t\t} else if (!strcmp(name, \"objectsize\")) {\n-\t\t\tv->value = oi->size;\n-\t\t\tv->s = xstrfmt(\"%\"PRIuMAX , (uintmax_t)oi->size);\n+\t\telse if (starts_with(name, \"objectsize\")) {\n+\t\t\tif (used_atom[i].u.objectsize.option == O_SIZE_DISK) {\n+\t\t\t\tv->value = oi->disk_size;\n+\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n+\t\t\t} else if (used_atom[i].u.objectsize.option == O_SIZE) {\n+\t\t\t\tv->value = oi->size;\n+\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX , (uintmax_t)oi->size);\n+\t\t\t}\n \t\t} else if (!strcmp(name, \"deltabase\"))\n \t\t\tv->s = xstrdup(oid_to_hex(&oi->delta_base_oid));\n \t\telse if (deref)\n-- \ngitgitgadget\n\n"},{"id":"424479","messageId":"50cfe1f0c6c1394c8b3729cb603ea7ab071f9676.1620918939.git.gitgitgadget@gmail.com","threadId":"55647","inReplyTo":"pull.951.v4.git.1620918939.gitgitgadget@gmail.com","subject":"[PATCH v4 2/2] [GSOC] ref-filter: introduce enum atom_type","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-13T15:15:38Z","receivedAt":"2021-05-13T15:15:51Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nIn the original ref-filter design, it will copy the parsed\natom's name and attributes to `used_atom[i].name` in the\natom's parsing step, and use it again for string matching\nin the later specific ref attributes filling step. It use\na lot of string matching to determine which atom we need.\n\nIntroduce the enum \"atom_type\", each enum value is named\nas `ATOM_*`, which is the index of each corresponding\nvalid_atom entry. In the first step of the atom parsing,\n`used_atom.atom_type` will record corresponding enum value\nfrom valid_atom entry index, and then in specific reference\nattribute filling step, only need to compare the value of\nthe `used_atom[i].atom_type` to check the atom type.\n\nHelped-by: Junio C Hamano \nHelped-by: Christian Couder \nSigned-off-by: ZheNing Hu \n---\n ref-filter.c | 197 +++++++++++++++++++++++++++++++++------------------\n 1 file changed, 128 insertions(+), 69 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex f420bae6e5ba..e7a48d428547 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -108,6 +108,56 @@ static struct ref_to_worktree_map {\n \tstruct worktree **worktrees;\n } ref_to_worktree_map;\n \n+/*\n+ * The enum atom_type is used as the index of valid_atom array.\n+ * In the atom parsing stage, it will be passed to used_atom.atom_type\n+ * as the identifier of the atom type. We can check the type of used_atom\n+ * entry by `if (used_atom[i].atom_type == ATOM_*)`.\n+ */\n+enum atom_type {\n+\tATOM_REFNAME,\n+\tATOM_OBJECTTYPE,\n+\tATOM_OBJECTSIZE,\n+\tATOM_OBJECTNAME,\n+\tATOM_DELTABASE,\n+\tATOM_TREE,\n+\tATOM_PARENT,\n+\tATOM_NUMPARENT,\n+\tATOM_OBJECT,\n+\tATOM_TYPE,\n+\tATOM_TAG,\n+\tATOM_AUTHOR,\n+\tATOM_AUTHORNAME,\n+\tATOM_AUTHOREMAIL,\n+\tATOM_AUTHORDATE,\n+\tATOM_COMMITTER,\n+\tATOM_COMMITTERNAME,\n+\tATOM_COMMITTEREMAIL,\n+\tATOM_COMMITTERDATE,\n+\tATOM_TAGGER,\n+\tATOM_TAGGERNAME,\n+\tATOM_TAGGEREMAIL,\n+\tATOM_TAGGERDATE,\n+\tATOM_CREATOR,\n+\tATOM_CREATORDATE,\n+\tATOM_SUBJECT,\n+\tATOM_BODY,\n+\tATOM_TRAILERS,\n+\tATOM_CONTENTS,\n+\tATOM_UPSTREAM,\n+\tATOM_PUSH,\n+\tATOM_SYMREF,\n+\tATOM_FLAG,\n+\tATOM_HEAD,\n+\tATOM_COLOR,\n+\tATOM_WORKTREEPATH,\n+\tATOM_ALIGN,\n+\tATOM_END,\n+\tATOM_IF,\n+\tATOM_THEN,\n+\tATOM_ELSE,\n+};\n+\n /*\n * An atom is a valid field atom listed below, possibly prefixed with\n * a \"*\" to denote deref_tag().\n@@ -119,6 +169,7 @@ static struct ref_to_worktree_map {\n * array.\n */\n static struct used_atom {\n+\tenum atom_type atom_type;\n \tconst char *name;\n \tcmp_type type;\n \tinfo_source source;\n@@ -506,47 +557,47 @@ static struct {\n \tint (*parser)(const struct ref_format *format, struct used_atom *atom,\n \t\t const char *arg, struct strbuf *err);\n } valid_atom[] = {\n-\t{ \"refname\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n-\t{ \"objecttype\", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser },\n-\t{ \"objectsize\", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },\n-\t{ \"objectname\", SOURCE_OTHER, FIELD_STR, oid_atom_parser },\n-\t{ \"deltabase\", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },\n-\t{ \"tree\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n-\t{ \"parent\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n-\t{ \"numparent\", SOURCE_OBJ, FIELD_ULONG },\n-\t{ \"object\", SOURCE_OBJ },\n-\t{ \"type\", SOURCE_OBJ },\n-\t{ \"tag\", SOURCE_OBJ },\n-\t{ \"author\", SOURCE_OBJ },\n-\t{ \"authorname\", SOURCE_OBJ },\n-\t{ \"authoremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n-\t{ \"authordate\", SOURCE_OBJ, FIELD_TIME },\n-\t{ \"committer\", SOURCE_OBJ },\n-\t{ \"committername\", SOURCE_OBJ },\n-\t{ \"committeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n-\t{ \"committerdate\", SOURCE_OBJ, FIELD_TIME },\n-\t{ \"tagger\", SOURCE_OBJ },\n-\t{ \"taggername\", SOURCE_OBJ },\n-\t{ \"taggeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n-\t{ \"taggerdate\", SOURCE_OBJ, FIELD_TIME },\n-\t{ \"creator\", SOURCE_OBJ },\n-\t{ \"creatordate\", SOURCE_OBJ, FIELD_TIME },\n-\t{ \"subject\", SOURCE_OBJ, FIELD_STR, subject_atom_parser },\n-\t{ \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n-\t{ \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n-\t{ \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n-\t{ \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n-\t{ \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n-\t{ \"symref\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n-\t{ \"flag\", SOURCE_NONE },\n-\t{ \"HEAD\", SOURCE_NONE, FIELD_STR, head_atom_parser },\n-\t{ \"color\", SOURCE_NONE, FIELD_STR, color_atom_parser },\n-\t{ \"worktreepath\", SOURCE_NONE },\n-\t{ \"align\", SOURCE_NONE, FIELD_STR, align_atom_parser },\n-\t{ \"end\", SOURCE_NONE },\n-\t{ \"if\", SOURCE_NONE, FIELD_STR, if_atom_parser },\n-\t{ \"then\", SOURCE_NONE },\n-\t{ \"else\", SOURCE_NONE },\n+\t[ATOM_REFNAME] = { \"refname\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n+\t[ATOM_OBJECTTYPE] = { \"objecttype\", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser },\n+\t[ATOM_OBJECTSIZE] = { \"objectsize\", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },\n+\t[ATOM_OBJECTNAME] = { \"objectname\", SOURCE_OTHER, FIELD_STR, oid_atom_parser },\n+\t[ATOM_DELTABASE] = { \"deltabase\", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },\n+\t[ATOM_TREE] = { \"tree\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n+\t[ATOM_PARENT] = { \"parent\", SOURCE_OBJ, FIELD_STR, oid_atom_parser },\n+\t[ATOM_NUMPARENT] = { \"numparent\", SOURCE_OBJ, FIELD_ULONG },\n+\t[ATOM_OBJECT] = { \"object\", SOURCE_OBJ },\n+\t[ATOM_TYPE] = { \"type\", SOURCE_OBJ },\n+\t[ATOM_TAG] = { \"tag\", SOURCE_OBJ },\n+\t[ATOM_AUTHOR] = { \"author\", SOURCE_OBJ },\n+\t[ATOM_AUTHORNAME] = { \"authorname\", SOURCE_OBJ },\n+\t[ATOM_AUTHOREMAIL] = { \"authoremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n+\t[ATOM_AUTHORDATE] = { \"authordate\", SOURCE_OBJ, FIELD_TIME },\n+\t[ATOM_COMMITTER] = { \"committer\", SOURCE_OBJ },\n+\t[ATOM_COMMITTERNAME] = { \"committername\", SOURCE_OBJ },\n+\t[ATOM_COMMITTEREMAIL] = { \"committeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n+\t[ATOM_COMMITTERDATE] = { \"committerdate\", SOURCE_OBJ, FIELD_TIME },\n+\t[ATOM_TAGGER] = { \"tagger\", SOURCE_OBJ },\n+\t[ATOM_TAGGERNAME] = { \"taggername\", SOURCE_OBJ },\n+\t[ATOM_TAGGEREMAIL] = { \"taggeremail\", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },\n+\t[ATOM_TAGGERDATE] = { \"taggerdate\", SOURCE_OBJ, FIELD_TIME },\n+\t[ATOM_CREATOR] = { \"creator\", SOURCE_OBJ },\n+\t[ATOM_CREATORDATE] = { \"creatordate\", SOURCE_OBJ, FIELD_TIME },\n+\t[ATOM_SUBJECT] = { \"subject\", SOURCE_OBJ, FIELD_STR, subject_atom_parser },\n+\t[ATOM_BODY] = { \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n+\t[ATOM_TRAILERS] = { \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n+\t[ATOM_CONTENTS] = { \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n+\t[ATOM_UPSTREAM] = { \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n+\t[ATOM_PUSH] = { \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n+\t[ATOM_SYMREF] = { \"symref\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n+\t[ATOM_FLAG] = { \"flag\", SOURCE_NONE },\n+\t[ATOM_HEAD] = { \"HEAD\", SOURCE_NONE, FIELD_STR, head_atom_parser },\n+\t[ATOM_COLOR] = { \"color\", SOURCE_NONE, FIELD_STR, color_atom_parser },\n+\t[ATOM_WORKTREEPATH] = { \"worktreepath\", SOURCE_NONE },\n+\t[ATOM_ALIGN] = { \"align\", SOURCE_NONE, FIELD_STR, align_atom_parser },\n+\t[ATOM_END] = { \"end\", SOURCE_NONE },\n+\t[ATOM_IF] = { \"if\", SOURCE_NONE, FIELD_STR, if_atom_parser },\n+\t[ATOM_THEN] = { \"then\", SOURCE_NONE },\n+\t[ATOM_ELSE] = { \"else\", SOURCE_NONE },\n \t/*\n \t * Please update $__git_ref_fieldlist in git-completion.bash\n \t * when you add new atoms\n@@ -628,6 +679,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n \tat = used_atom_cnt;\n \tused_atom_cnt++;\n \tREALLOC_ARRAY(used_atom, used_atom_cnt);\n+\tused_atom[at].atom_type = i;\n \tused_atom[at].name = xmemdupz(atom, ep - atom);\n \tused_atom[at].type = valid_atom[i].cmp_type;\n \tused_atom[at].source = valid_atom[i].source;\n@@ -652,7 +704,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n \t\treturn -1;\n \tif (*atom == '*')\n \t\tneed_tagged = 1;\n-\tif (!strcmp(valid_atom[i].name, \"symref\"))\n+\tif (i == ATOM_SYMREF)\n \t\tneed_symref = 1;\n \treturn at;\n }\n@@ -965,14 +1017,15 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n-\t\tif (!strcmp(name, \"objecttype\"))\n+\t\tif (atom_type == ATOM_OBJECTTYPE)\n \t\t\tv->s = xstrdup(type_name(oi->type));\n-\t\telse if (starts_with(name, \"objectsize\")) {\n+\t\telse if (atom_type == ATOM_OBJECTSIZE) {\n \t\t\tif (used_atom[i].u.objectsize.option == O_SIZE_DISK) {\n \t\t\t\tv->value = oi->disk_size;\n \t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n@@ -980,9 +1033,9 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \t\t\t\tv->value = oi->size;\n \t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX , (uintmax_t)oi->size);\n \t\t\t}\n-\t\t} else if (!strcmp(name, \"deltabase\"))\n+\t\t} else if (atom_type == ATOM_DELTABASE)\n \t\t\tv->s = xstrdup(oid_to_hex(&oi->delta_base_oid));\n-\t\telse if (deref)\n+\t\telse if (atom_type == ATOM_OBJECTNAME && deref)\n \t\t\tgrab_oid(name, \"objectname\", &oi->oid, v, &used_atom[i]);\n \t}\n }\n@@ -995,16 +1048,17 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n-\t\tif (!strcmp(name, \"tag\"))\n+\t\tif (atom_type == ATOM_TAG)\n \t\t\tv->s = xstrdup(tag->tag);\n-\t\telse if (!strcmp(name, \"type\") && tag->tagged)\n+\t\telse if (atom_type == ATOM_TYPE && tag->tagged)\n \t\t\tv->s = xstrdup(type_name(tag->tagged->type));\n-\t\telse if (!strcmp(name, \"object\") && tag->tagged)\n+\t\telse if (atom_type == ATOM_OBJECT && tag->tagged)\n \t\t\tv->s = xstrdup(oid_to_hex(&tag->tagged->oid));\n \t}\n }\n@@ -1017,18 +1071,20 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n-\t\tif (grab_oid(name, \"tree\", get_commit_tree_oid(commit), v, &used_atom[i]))\n+\t\tif (atom_type == ATOM_TREE &&\n+\t\t grab_oid(name, \"tree\", get_commit_tree_oid(commit), v, &used_atom[i]))\n \t\t\tcontinue;\n-\t\tif (!strcmp(name, \"numparent\")) {\n+\t\tif (atom_type == ATOM_NUMPARENT) {\n \t\t\tv->value = commit_list_count(commit->parents);\n \t\t\tv->s = xstrfmt(\"%lu\", (unsigned long)v->value);\n \t\t}\n-\t\telse if (starts_with(name, \"parent\")) {\n+\t\telse if (atom_type == ATOM_PARENT) {\n \t\t\tstruct commit_list *parents;\n \t\t\tstruct strbuf s = STRBUF_INIT;\n \t\t\tfor (parents = commit->parents; parents; parents = parents->next) {\n@@ -1208,15 +1264,16 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n \t\treturn;\n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n \n-\t\tif (starts_with(name, \"creatordate\"))\n+\t\tif (atom_type == ATOM_CREATORDATE)\n \t\t\tgrab_date(wholine, v, name);\n-\t\telse if (!strcmp(name, \"creator\"))\n+\t\telse if (atom_type == ATOM_CREATOR)\n \t\t\tv->s = copy_line(wholine);\n \t}\n }\n@@ -1696,6 +1753,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t/* Fill in specials first */\n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tstruct used_atom *atom = &used_atom[i];\n+\t\tenum atom_type atom_type = atom->atom_type;\n \t\tconst char *name = used_atom[i].name;\n \t\tstruct atom_value *v = &ref->value[i];\n \t\tint deref = 0;\n@@ -1710,18 +1768,18 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\tname++;\n \t\t}\n \n-\t\tif (starts_with(name, \"refname\"))\n+\t\tif (atom_type == ATOM_REFNAME)\n \t\t\trefname = get_refname(atom, ref);\n-\t\telse if (!strcmp(name, \"worktreepath\")) {\n+\t\telse if (atom_type == ATOM_WORKTREEPATH) {\n \t\t\tif (ref->kind == FILTER_REFS_BRANCHES)\n \t\t\t\tv->s = get_worktree_path(atom, ref);\n \t\t\telse\n \t\t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n \t\t}\n-\t\telse if (starts_with(name, \"symref\"))\n+\t\telse if (atom_type == ATOM_SYMREF)\n \t\t\trefname = get_symref(atom, ref);\n-\t\telse if (starts_with(name, \"upstream\")) {\n+\t\telse if (atom_type == ATOM_UPSTREAM) {\n \t\t\tconst char *branch_name;\n \t\t\t/* only local branches may have an upstream */\n \t\t\tif (!skip_prefix(ref->refname, \"refs/heads/\",\n@@ -1737,7 +1795,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\telse\n \t\t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-\t\t} else if (atom->u.remote_ref.push) {\n+\t\t} else if (atom_type == ATOM_PUSH && atom->u.remote_ref.push) {\n \t\t\tconst char *branch_name;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tif (!skip_prefix(ref->refname, \"refs/heads/\",\n@@ -1756,10 +1814,10 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\tfree((char *)v->s);\n \t\t\tfill_remote_ref_details(atom, refname, branch, &v->s);\n \t\t\tcontinue;\n-\t\t} else if (starts_with(name, \"color:\")) {\n+\t\t} else if (atom_type == ATOM_COLOR) {\n \t\t\tv->s = xstrdup(atom->u.color);\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"flag\")) {\n+\t\t} else if (atom_type == ATOM_FLAG) {\n \t\t\tchar buf[256], *cp = buf;\n \t\t\tif (ref->flag & REF_ISSYMREF)\n \t\t\t\tcp = copy_advance(cp, \",symref\");\n@@ -1772,23 +1830,24 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\t\tv->s = xstrdup(buf + 1);\n \t\t\t}\n \t\t\tcontinue;\n-\t\t} else if (!deref && grab_oid(name, \"objectname\", &ref->objectname, v, atom)) {\n-\t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"HEAD\")) {\n+\t\t} else if (!deref && atom_type == ATOM_OBJECTNAME &&\n+\t\t\t grab_oid(name, \"objectname\", &ref->objectname, v, atom)) {\n+\t\t\t\tcontinue;\n+\t\t} else if (atom_type == ATOM_HEAD) {\n \t\t\tif (atom->u.head && !strcmp(ref->refname, atom->u.head))\n \t\t\t\tv->s = xstrdup(\"*\");\n \t\t\telse\n \t\t\t\tv->s = xstrdup(\" \");\n \t\t\tcontinue;\n-\t\t} else if (starts_with(name, \"align\")) {\n+\t\t} else if (atom_type == ATOM_ALIGN) {\n \t\t\tv->handler = align_atom_handler;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"end\")) {\n+\t\t} else if (atom_type == ATOM_END) {\n \t\t\tv->handler = end_atom_handler;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-\t\t} else if (starts_with(name, \"if\")) {\n+\t\t} else if (atom_type == ATOM_IF) {\n \t\t\tconst char *s;\n \t\t\tif (skip_prefix(name, \"if:\", &s))\n \t\t\t\tv->s = xstrdup(s);\n@@ -1796,11 +1855,11 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\t\tv->s = xstrdup(\"\");\n \t\t\tv->handler = if_atom_handler;\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"then\")) {\n+\t\t} else if (atom_type == ATOM_THEN) {\n \t\t\tv->handler = then_atom_handler;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"else\")) {\n+\t\t} else if (atom_type == ATOM_ELSE) {\n \t\t\tv->handler = else_atom_handler;\n \t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n-- \ngitgitgadget\n"}]}