{"thread":{"id":"39382","subject":"[WIP] [PATCH 0/4] Unifying git branch -l, git tag -l, and git for-each-ref","startedAt":"2015-05-20T13:14:42Z","lastAt":"2015-05-23T22:33:33Z","messageCount":24,"participants":["karthik nayak","Karthik Nayak","Matthieu Moy","Eric Sunshine","Junio C Hamano","Christian Couder"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"261655","messageId":"555C88C2.8060902@gmail.com","threadId":"39382","inReplyTo":null,"subject":"[WIP] [PATCH 0/4] Unifying git branch -l, git tag -l, and git for-each-ref","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-05-20T13:14:42Z","receivedAt":"2015-05-20T13:14:42Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Hello All,\n\nJust updating on my progress for my GSoC project on \"Unifying git \nbranch- l, git tag -l and git for-each-ref.\"\n\nI have been going through the code for the above commands, I started off \nfirst with 'for-each-ref' because it seemed to be the most basic command \nof the three, I have been building the common library\n'ref-filter' based on the requirements for 'for-each-ref', eventually\nI plan to merge 'tag -l' and 'branch -l' and extend its functionality.\n\nThe '--format' and '--sort' option provided by for-each-ref seemed to be \nthe most fundamental and I plan on using this implementation and \nextending the option to 'tag -l' and 'branch -l'. So this left me with \nmoving most of the code from 'for-each-ref' to 'ref-filter' and leaving \n'for-each-ref' with nothing but the command call from the main git command.\n\nThis is still a WIP and hope to get some suggestions/feedback on my \nprogress.\n"},{"id":"261659","messageId":"1432127904-21070-1-git-send-email-karthik.188@gmail.com","threadId":"39382","inReplyTo":"555C88C2.8060902@gmail.com","subject":"[PATCH 1/4] for-each-ref: rename refinfo members to match similar structures","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-05-20T13:18:21Z","receivedAt":"2015-05-20T13:18:21Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Jeff King <peff@peff.net>\n\nWritten-by: Jeff King <peff@peff.net>\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/for-each-ref.c | 40 ++++++++++++++++++++--------------------\n 1 file changed, 20 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 83f9cf9..2721228 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -32,9 +32,9 @@ struct ref_sort {\n };\n \n struct refinfo {\n-\tchar *refname;\n-\tunsigned char objectname[20];\n-\tint flag;\n+\tchar *name;\n+\tunsigned char sha1[20];\n+\tint flags;\n \tconst char *symref;\n \tstruct atom_value *value;\n };\n@@ -632,9 +632,9 @@ static void populate_value(struct refinfo *ref)\n \n \tref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));\n \n-\tif (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {\n+\tif (need_symref && (ref->flags & REF_ISSYMREF) && !ref->symref) {\n \t\tunsigned char unused1[20];\n-\t\tref->symref = resolve_refdup(ref->refname, RESOLVE_REF_READING,\n+\t\tref->symref = resolve_refdup(ref->name, RESOLVE_REF_READING,\n \t\t\t\t\t     unused1, NULL);\n \t\tif (!ref->symref)\n \t\t\tref->symref = \"\";\n@@ -655,14 +655,14 @@ static void populate_value(struct refinfo *ref)\n \t\t}\n \n \t\tif (starts_with(name, \"refname\"))\n-\t\t\trefname = ref->refname;\n+\t\t\trefname = ref->name;\n \t\telse if (starts_with(name, \"symref\"))\n \t\t\trefname = ref->symref ? ref->symref : \"\";\n \t\telse if (starts_with(name, \"upstream\")) {\n \t\t\t/* only local branches may have an upstream */\n-\t\t\tif (!starts_with(ref->refname, \"refs/heads/\"))\n+\t\t\tif (!starts_with(ref->name, \"refs/heads/\"))\n \t\t\t\tcontinue;\n-\t\t\tbranch = branch_get(ref->refname + 11);\n+\t\t\tbranch = branch_get(ref->name + 11);\n \n \t\t\tif (!branch || !branch->merge || !branch->merge[0] ||\n \t\t\t    !branch->merge[0]->dst)\n@@ -677,9 +677,9 @@ static void populate_value(struct refinfo *ref)\n \t\t\tcontinue;\n \t\t} else if (!strcmp(name, \"flag\")) {\n \t\t\tchar buf[256], *cp = buf;\n-\t\t\tif (ref->flag & REF_ISSYMREF)\n+\t\t\tif (ref->flags & REF_ISSYMREF)\n \t\t\t\tcp = copy_advance(cp, \",symref\");\n-\t\t\tif (ref->flag & REF_ISPACKED)\n+\t\t\tif (ref->flags & REF_ISPACKED)\n \t\t\t\tcp = copy_advance(cp, \",packed\");\n \t\t\tif (cp == buf)\n \t\t\t\tv->s = \"\";\n@@ -688,7 +688,7 @@ static void populate_value(struct refinfo *ref)\n \t\t\t\tv->s = xstrdup(buf + 1);\n \t\t\t}\n \t\t\tcontinue;\n-\t\t} else if (!deref && grab_objectname(name, ref->objectname, v)) {\n+\t\t} else if (!deref && grab_objectname(name, ref->sha1, v)) {\n \t\t\tcontinue;\n \t\t} else if (!strcmp(name, \"HEAD\")) {\n \t\t\tconst char *head;\n@@ -696,7 +696,7 @@ static void populate_value(struct refinfo *ref)\n \n \t\t\thead = resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n \t\t\t\t\t\t  sha1, NULL);\n-\t\t\tif (!strcmp(ref->refname, head))\n+\t\t\tif (!strcmp(ref->name, head))\n \t\t\t\tv->s = \"*\";\n \t\t\telse\n \t\t\t\tv->s = \" \";\n@@ -774,13 +774,13 @@ static void populate_value(struct refinfo *ref)\n \treturn;\n \n  need_obj:\n-\tbuf = get_obj(ref->objectname, &obj, &size, &eaten);\n+\tbuf = get_obj(ref->sha1, &obj, &size, &eaten);\n \tif (!buf)\n \t\tdie(\"missing object %s for %s\",\n-\t\t    sha1_to_hex(ref->objectname), ref->refname);\n+\t\t    sha1_to_hex(ref->sha1), ref->name);\n \tif (!obj)\n \t\tdie(\"parse_object_buffer failed on %s for %s\",\n-\t\t    sha1_to_hex(ref->objectname), ref->refname);\n+\t\t    sha1_to_hex(ref->sha1), ref->name);\n \n \tgrab_values(ref->value, 0, obj, buf, size);\n \tif (!eaten)\n@@ -808,10 +808,10 @@ static void populate_value(struct refinfo *ref)\n \tbuf = get_obj(tagged, &obj, &size, &eaten);\n \tif (!buf)\n \t\tdie(\"missing object %s for %s\",\n-\t\t    sha1_to_hex(tagged), ref->refname);\n+\t\t    sha1_to_hex(tagged), ref->name);\n \tif (!obj)\n \t\tdie(\"parse_object_buffer failed on %s for %s\",\n-\t\t    sha1_to_hex(tagged), ref->refname);\n+\t\t    sha1_to_hex(tagged), ref->name);\n \tgrab_values(ref->value, 1, obj, buf, size);\n \tif (!eaten)\n \t\tfree(buf);\n@@ -877,9 +877,9 @@ static int grab_single_ref(const char *refname, const unsigned char *sha1, int f\n \t * by maxcount logic.\n \t */\n \tref = xcalloc(1, sizeof(*ref));\n-\tref->refname = xstrdup(refname);\n-\thashcpy(ref->objectname, sha1);\n-\tref->flag = flag;\n+\tref->name = xstrdup(refname);\n+\thashcpy(ref->sha1, sha1);\n+\tref->flags = flag;\n \n \tcnt = cb->grab_cnt;\n \tREALLOC_ARRAY(cb->grab_array, cnt + 1);\n-- \n2.4.1\n"},{"id":"261656","messageId":"1432127904-21070-2-git-send-email-karthik.188@gmail.com","threadId":"39382","inReplyTo":"555C88C2.8060902@gmail.com","subject":"[PATCH 2/4] ref-filter: add ref-filter API","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-05-20T13:18:22Z","receivedAt":"2015-05-20T13:18:22Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"add a ref-filter API to provide functions to filter refs for listing.\nThis will act as a common library for commands like 'tag -l',\n'branch -l' and 'for-each-ref'. ref-filter will enable each of these\ncommands to benefit from the features of the others.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n Makefile     |  1 +\n ref-filter.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n ref-filter.h | 47 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 121 insertions(+)\n create mode 100644 ref-filter.c\n create mode 100644 ref-filter.h\n\ndiff --git a/Makefile b/Makefile\nindex 5f3987f..1e03bda 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -759,6 +759,7 @@ LIB_OBJS += reachable.o\n LIB_OBJS += read-cache.o\n LIB_OBJS += reflog-walk.o\n LIB_OBJS += refs.o\n+LIB_OBJS += ref-filter.o\n LIB_OBJS += remote.o\n LIB_OBJS += replace_object.o\n LIB_OBJS += rerere.o\ndiff --git a/ref-filter.c b/ref-filter.c\nnew file mode 100644\nindex 0000000..df571a6\n--- /dev/null\n+++ b/ref-filter.c\n@@ -0,0 +1,73 @@\n+#include \"builtin.h\"\n+#include \"cache.h\"\n+#include \"refs.h\"\n+#include \"ref-filter.h\"\n+#include \"wildmatch.h\"\n+#include \"commit.h\"\n+\n+static int match_name_as_path(const char **pattern, const char *refname)\n+{\n+\tint namelen = strlen(refname);\n+\tfor (; *pattern; pattern++) {\n+\t\tconst char *p = *pattern;\n+\t\tint plen = strlen(p);\n+\n+\t\tif ((plen <= namelen) &&\n+\t\t    !strncmp(refname, p, plen) &&\n+\t\t    (refname[plen] == '\\0' ||\n+\t\t     refname[plen] == '/' ||\n+\t\t     p[plen-1] == '/'))\n+\t\t\treturn 1;\n+\t\tif (!wildmatch(p, refname, WM_PATHNAME, NULL))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+static struct ref_filter_item *new_ref_filter_item(const char *refname,\n+\t\t\t\t\t\t   const unsigned char *sha1,\n+\t\t\t\t\t\t   int flag)\n+{\n+\tstruct ref_filter_item *ref =  xcalloc(1, sizeof(struct ref_filter_item));\n+\tref->name = xstrdup(refname);\n+\thashcpy(ref->sha1, sha1);\n+\tref->flags = flag;\n+\n+\treturn ref;\n+}\n+\n+/*\n+ * A call-back given to for_each_ref().  Filter refs and keep them for\n+ * later object processing.\n+ */\n+int ref_filter_add(const char *refname, const unsigned char *sha1, int flag, void *data)\n+{\n+\tstruct ref_filter *ref_list = data;\n+\tstruct ref_filter_item *ref;\n+\n+\tif (flag & REF_BAD_NAME) {\n+\t\twarning(\"ignoring ref with broken name %s\", refname);\n+\t\treturn 0;\n+\t}\n+\n+\tif (*ref_list->name_patterns && !match_name_as_path(ref_list->name_patterns, refname))\n+\t\treturn 0;\n+\n+\tref = new_ref_filter_item(refname, sha1, flag);\n+\n+\tREALLOC_ARRAY(ref_list->items, ref_list->count + 1);\n+\tref_list->items[ref_list->count++] = ref;\n+\n+\treturn 0;\n+}\n+\n+void ref_filter_clear(struct ref_filter *refs)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < refs->count; i++)\n+\t\tfree(refs->items[i]);\n+\tfree(refs->items);\n+\trefs->items = NULL;\n+\trefs->count = refs->alloc = 0;\n+}\ndiff --git a/ref-filter.h b/ref-filter.h\nnew file mode 100644\nindex 0000000..3010d13\n--- /dev/null\n+++ b/ref-filter.h\n@@ -0,0 +1,47 @@\n+#ifndef REF_FILTER_H\n+#define REF_FILTER_H\n+\n+#include \"sha1-array.h\"\n+#include \"refs.h\"\n+#include \"commit.h\"\n+\n+/*\n+ * ref-filter is meant to act as a common provider of API's for\n+ * 'tag -l', 'branch -l' and 'for-each-ref'. ref-filter is the attempt\n+ * at unification of these three commands so that they ay benefit from\n+ * the functionality of each other.\n+ */\n+\n+/* An atom is a valid field atom used for sorting and formatting of refs.*/\n+struct atom_value {\n+\tconst char *s;\n+\tunsigned long ul; /* used for sorting when not FIELD_STR */\n+};\n+\n+struct ref_sort {\n+\tstruct ref_sort *next;\n+\tint atom; /* index into used_atom array */\n+\tunsigned reverse : 1;\n+};\n+\n+/* ref_filter_item will hold the data pertaining to a particular ref. */\n+struct ref_filter_item {\n+\tunsigned char sha1[20];\n+\tint flags;\n+\tconst char *symref;\n+\tstruct atom_value *value;\n+\tchar *name;\n+};\n+\n+/*  ref_filter will hold data pertaining to a list of refs. */\n+struct ref_filter {\n+\tint count, alloc;\n+\tstruct ref_filter_item **items;\n+\tconst char **name_patterns;\n+};\n+\n+/*  ref_filter_add is used to add refs to the ref_filter list. */\n+int ref_filter_add(const char *refname, const unsigned char *sha1, int flags, void *data);\n+void ref_filter_clear(struct ref_filter *refs);\n+\n+#endif /*  REF_FILTER_H  */\n-- \n2.4.1\n"},{"id":"261658","messageId":"1432127904-21070-3-git-send-email-karthik.188@gmail.com","threadId":"39382","inReplyTo":"555C88C2.8060902@gmail.com","subject":"[PATCH 3/4] for-each-ref: convert to ref-filter","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-05-20T13:18:23Z","receivedAt":"2015-05-20T13:18:23Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"convert 'for-each-ref' to use the common API provided by 'ref-filter'.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/for-each-ref.c | 118 +++++++++----------------------------------------\n 1 file changed, 20 insertions(+), 98 deletions(-)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 2721228..c9875d1 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -10,6 +10,7 @@\n #include \"parse-options.h\"\n #include \"remote.h\"\n #include \"color.h\"\n+#include \"ref-filter.h\"\n \n /* Quoting styles */\n #define QUOTE_NONE 0\n@@ -20,25 +21,6 @@\n \n typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n \n-struct atom_value {\n-\tconst char *s;\n-\tunsigned long ul; /* used for sorting when not FIELD_STR */\n-};\n-\n-struct ref_sort {\n-\tstruct ref_sort *next;\n-\tint atom; /* index into used_atom array */\n-\tunsigned reverse : 1;\n-};\n-\n-struct refinfo {\n-\tchar *name;\n-\tunsigned char sha1[20];\n-\tint flags;\n-\tconst char *symref;\n-\tstruct atom_value *value;\n-};\n-\n static struct {\n \tconst char *name;\n \tcmp_type cmp_type;\n@@ -85,7 +67,7 @@ static struct {\n  * a \"*\" to denote deref_tag().\n  *\n  * We parse given format string and sort specifiers, and make a list\n- * of properties that we need to extract out of objects.  refinfo\n+ * of properties that we need to extract out of objects.  ref_filter_item\n  * structure will hold an array of values extracted that can be\n  * indexed with the \"atom number\", which is an index into this\n  * array.\n@@ -622,7 +604,7 @@ static inline char *copy_advance(char *dst, const char *src)\n /*\n  * Parse the object referred by ref, and grab needed value.\n  */\n-static void populate_value(struct refinfo *ref)\n+static void populate_value(struct ref_filter_item *ref)\n {\n \tvoid *buf;\n \tstruct object *obj;\n@@ -821,7 +803,7 @@ static void populate_value(struct refinfo *ref)\n  * Given a ref, return the value for the atom.  This lazily gets value\n  * out of the object by calling populate value.\n  */\n-static void get_value(struct refinfo *ref, int atom, struct atom_value **v)\n+static void get_value(struct ref_filter_item *ref, int atom, struct atom_value **v)\n {\n \tif (!ref->value) {\n \t\tpopulate_value(ref);\n@@ -830,65 +812,7 @@ static void get_value(struct refinfo *ref, int atom, struct atom_value **v)\n \t*v = &ref->value[atom];\n }\n \n-struct grab_ref_cbdata {\n-\tstruct refinfo **grab_array;\n-\tconst char **grab_pattern;\n-\tint grab_cnt;\n-};\n-\n-/*\n- * A call-back given to for_each_ref().  Filter refs and keep them for\n- * later object processing.\n- */\n-static int grab_single_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)\n-{\n-\tstruct grab_ref_cbdata *cb = cb_data;\n-\tstruct refinfo *ref;\n-\tint cnt;\n-\n-\tif (flag & REF_BAD_NAME) {\n-\t\t  warning(\"ignoring ref with broken name %s\", refname);\n-\t\t  return 0;\n-\t}\n-\n-\tif (*cb->grab_pattern) {\n-\t\tconst char **pattern;\n-\t\tint namelen = strlen(refname);\n-\t\tfor (pattern = cb->grab_pattern; *pattern; pattern++) {\n-\t\t\tconst char *p = *pattern;\n-\t\t\tint plen = strlen(p);\n-\n-\t\t\tif ((plen <= namelen) &&\n-\t\t\t    !strncmp(refname, p, plen) &&\n-\t\t\t    (refname[plen] == '\\0' ||\n-\t\t\t     refname[plen] == '/' ||\n-\t\t\t     p[plen-1] == '/'))\n-\t\t\t\tbreak;\n-\t\t\tif (!wildmatch(p, refname, WM_PATHNAME, NULL))\n-\t\t\t\tbreak;\n-\t\t}\n-\t\tif (!*pattern)\n-\t\t\treturn 0;\n-\t}\n-\n-\t/*\n-\t * We do not open the object yet; sort may only need refname\n-\t * to do its job and the resulting list may yet to be pruned\n-\t * by maxcount logic.\n-\t */\n-\tref = xcalloc(1, sizeof(*ref));\n-\tref->name = xstrdup(refname);\n-\thashcpy(ref->sha1, sha1);\n-\tref->flags = flag;\n-\n-\tcnt = cb->grab_cnt;\n-\tREALLOC_ARRAY(cb->grab_array, cnt + 1);\n-\tcb->grab_array[cnt++] = ref;\n-\tcb->grab_cnt = cnt;\n-\treturn 0;\n-}\n-\n-static int cmp_ref_sort(struct ref_sort *s, struct refinfo *a, struct refinfo *b)\n+static int cmp_ref_sort(struct ref_sort *s, struct ref_filter_item *a, struct ref_filter_item *b)\n {\n \tstruct atom_value *va, *vb;\n \tint cmp;\n@@ -915,8 +839,8 @@ static int cmp_ref_sort(struct ref_sort *s, struct refinfo *a, struct refinfo *b\n static struct ref_sort *ref_sort;\n static int compare_refs(const void *a_, const void *b_)\n {\n-\tstruct refinfo *a = *((struct refinfo **)a_);\n-\tstruct refinfo *b = *((struct refinfo **)b_);\n+\tstruct ref_filter_item *a = *((struct ref_filter_item **)a_);\n+\tstruct ref_filter_item *b = *((struct ref_filter_item **)b_);\n \tstruct ref_sort *s;\n \n \tfor (s = ref_sort; s; s = s->next) {\n@@ -927,10 +851,10 @@ static int compare_refs(const void *a_, const void *b_)\n \treturn 0;\n }\n \n-static void sort_refs(struct ref_sort *sort, struct refinfo **refs, int num_refs)\n+static void sort_refs(struct ref_sort *sort, struct ref_filter *refs)\n {\n \tref_sort = sort;\n-\tqsort(refs, num_refs, sizeof(struct refinfo *), compare_refs);\n+\tqsort(refs->items, refs->count, sizeof(struct ref_filter_item *), compare_refs);\n }\n \n static void print_value(struct atom_value *v, int quote_style)\n@@ -997,7 +921,7 @@ static void emit(const char *cp, const char *ep)\n \t}\n }\n \n-static void show_ref(struct refinfo *info, const char *format, int quote_style)\n+static void show_ref(struct ref_filter_item *info, const char *format, int quote_style)\n {\n \tconst char *cp, *sp, *ep;\n \n@@ -1066,12 +990,12 @@ static char const * const for_each_ref_usage[] = {\n \n int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n {\n-\tint i, num_refs;\n+\tint i;\n \tconst char *format = \"%(objectname) %(objecttype)\\t%(refname)\";\n \tstruct ref_sort *sort = NULL, **sort_tail = &sort;\n \tint maxcount = 0, quote_style = 0;\n-\tstruct refinfo **refs;\n-\tstruct grab_ref_cbdata cbdata;\n+\tstruct ref_filter refs;\n+\tmemset(&refs, 0, sizeof(refs));\n \n \tstruct option opts[] = {\n \t\tOPT_BIT('s', \"shell\", &quote_style,\n@@ -1109,17 +1033,15 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \t/* for warn_ambiguous_refs */\n \tgit_config(git_default_config, NULL);\n \n-\tmemset(&cbdata, 0, sizeof(cbdata));\n-\tcbdata.grab_pattern = argv;\n-\tfor_each_rawref(grab_single_ref, &cbdata);\n-\trefs = cbdata.grab_array;\n-\tnum_refs = cbdata.grab_cnt;\n+\trefs.name_patterns = argv;\n+\tfor_each_rawref(ref_filter_add, &refs);\n \n-\tsort_refs(sort, refs, num_refs);\n+\tsort_refs(sort, &refs);\n \n-\tif (!maxcount || num_refs < maxcount)\n-\t\tmaxcount = num_refs;\n+\tif (!maxcount || refs.count < maxcount)\n+\t\tmaxcount = refs.count;\n \tfor (i = 0; i < maxcount; i++)\n-\t\tshow_ref(refs[i], format, quote_style);\n+\t\tshow_ref(refs.items[i], format, quote_style);\n+\tref_filter_clear(&refs);\n \treturn 0;\n }\n-- \n2.4.1\n"},{"id":"261657","messageId":"1432127904-21070-4-git-send-email-karthik.188@gmail.com","threadId":"39382","inReplyTo":"555C88C2.8060902@gmail.com","subject":"[PATCH 4/4] ref-filter: move formatting/sorting options from 'for-each-ref'","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-05-20T13:18:24Z","receivedAt":"2015-05-20T13:18:24Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"'for-each-ref' uses structures like 'atom_value' and 'ref_sort' to\nformat or sort the given list of refs, move these functions to\n'ref-filter' and make them available as an API so that they can be\nused by others.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/for-each-ref.c | 985 +------------------------------------------------\n ref-filter.c           | 970 ++++++++++++++++++++++++++++++++++++++++++++++++\n ref-filter.h           |  16 +\n 3 files changed, 989 insertions(+), 982 deletions(-)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex c9875d1..6a928cd 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -1,987 +1,8 @@\n #include \"builtin.h\"\n #include \"cache.h\"\n #include \"refs.h\"\n-#include \"object.h\"\n-#include \"tag.h\"\n-#include \"commit.h\"\n-#include \"tree.h\"\n-#include \"blob.h\"\n-#include \"quote.h\"\n-#include \"parse-options.h\"\n-#include \"remote.h\"\n-#include \"color.h\"\n #include \"ref-filter.h\"\n-\n-/* Quoting styles */\n-#define QUOTE_NONE 0\n-#define QUOTE_SHELL 1\n-#define QUOTE_PERL 2\n-#define QUOTE_PYTHON 4\n-#define QUOTE_TCL 8\n-\n-typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n-\n-static struct {\n-\tconst char *name;\n-\tcmp_type cmp_type;\n-} valid_atom[] = {\n-\t{ \"refname\" },\n-\t{ \"objecttype\" },\n-\t{ \"objectsize\", FIELD_ULONG },\n-\t{ \"objectname\" },\n-\t{ \"tree\" },\n-\t{ \"parent\" },\n-\t{ \"numparent\", FIELD_ULONG },\n-\t{ \"object\" },\n-\t{ \"type\" },\n-\t{ \"tag\" },\n-\t{ \"author\" },\n-\t{ \"authorname\" },\n-\t{ \"authoremail\" },\n-\t{ \"authordate\", FIELD_TIME },\n-\t{ \"committer\" },\n-\t{ \"committername\" },\n-\t{ \"committeremail\" },\n-\t{ \"committerdate\", FIELD_TIME },\n-\t{ \"tagger\" },\n-\t{ \"taggername\" },\n-\t{ \"taggeremail\" },\n-\t{ \"taggerdate\", FIELD_TIME },\n-\t{ \"creator\" },\n-\t{ \"creatordate\", FIELD_TIME },\n-\t{ \"subject\" },\n-\t{ \"body\" },\n-\t{ \"contents\" },\n-\t{ \"contents:subject\" },\n-\t{ \"contents:body\" },\n-\t{ \"contents:signature\" },\n-\t{ \"upstream\" },\n-\t{ \"symref\" },\n-\t{ \"flag\" },\n-\t{ \"HEAD\" },\n-\t{ \"color\" },\n-};\n-\n-/*\n- * An atom is a valid field atom listed above, possibly prefixed with\n- * a \"*\" to denote deref_tag().\n- *\n- * We parse given format string and sort specifiers, and make a list\n- * of properties that we need to extract out of objects.  ref_filter_item\n- * structure will hold an array of values extracted that can be\n- * indexed with the \"atom number\", which is an index into this\n- * array.\n- */\n-static const char **used_atom;\n-static cmp_type *used_atom_type;\n-static int used_atom_cnt, need_tagged, need_symref;\n-static int need_color_reset_at_eol;\n-\n-/*\n- * Used to parse format string and sort specifiers\n- */\n-static int parse_atom(const char *atom, const char *ep)\n-{\n-\tconst char *sp;\n-\tint i, at;\n-\n-\tsp = atom;\n-\tif (*sp == '*' && sp < ep)\n-\t\tsp++; /* deref */\n-\tif (ep <= sp)\n-\t\tdie(\"malformed field name: %.*s\", (int)(ep-atom), atom);\n-\n-\t/* Do we have the atom already used elsewhere? */\n-\tfor (i = 0; i < used_atom_cnt; i++) {\n-\t\tint len = strlen(used_atom[i]);\n-\t\tif (len == ep - atom && !memcmp(used_atom[i], atom, len))\n-\t\t\treturn i;\n-\t}\n-\n-\t/* Is the atom a valid one? */\n-\tfor (i = 0; i < ARRAY_SIZE(valid_atom); i++) {\n-\t\tint len = strlen(valid_atom[i].name);\n-\t\t/*\n-\t\t * If the atom name has a colon, strip it and everything after\n-\t\t * it off - it specifies the format for this entry, and\n-\t\t * shouldn't be used for checking against the valid_atom\n-\t\t * table.\n-\t\t */\n-\t\tconst char *formatp = strchr(sp, ':');\n-\t\tif (!formatp || ep < formatp)\n-\t\t\tformatp = ep;\n-\t\tif (len == formatp - sp && !memcmp(valid_atom[i].name, sp, len))\n-\t\t\tbreak;\n-\t}\n-\n-\tif (ARRAY_SIZE(valid_atom) <= i)\n-\t\tdie(\"unknown field name: %.*s\", (int)(ep-atom), atom);\n-\n-\t/* Add it in, including the deref prefix */\n-\tat = used_atom_cnt;\n-\tused_atom_cnt++;\n-\tREALLOC_ARRAY(used_atom, used_atom_cnt);\n-\tREALLOC_ARRAY(used_atom_type, used_atom_cnt);\n-\tused_atom[at] = xmemdupz(atom, ep - atom);\n-\tused_atom_type[at] = valid_atom[i].cmp_type;\n-\tif (*atom == '*')\n-\t\tneed_tagged = 1;\n-\tif (!strcmp(used_atom[at], \"symref\"))\n-\t\tneed_symref = 1;\n-\treturn at;\n-}\n-\n-/*\n- * In a format string, find the next occurrence of %(atom).\n- */\n-static const char *find_next(const char *cp)\n-{\n-\twhile (*cp) {\n-\t\tif (*cp == '%') {\n-\t\t\t/*\n-\t\t\t * %( is the start of an atom;\n-\t\t\t * %% is a quoted per-cent.\n-\t\t\t */\n-\t\t\tif (cp[1] == '(')\n-\t\t\t\treturn cp;\n-\t\t\telse if (cp[1] == '%')\n-\t\t\t\tcp++; /* skip over two % */\n-\t\t\t/* otherwise this is a singleton, literal % */\n-\t\t}\n-\t\tcp++;\n-\t}\n-\treturn NULL;\n-}\n-\n-/*\n- * Make sure the format string is well formed, and parse out\n- * the used atoms.\n- */\n-static int verify_format(const char *format)\n-{\n-\tconst char *cp, *sp;\n-\n-\tneed_color_reset_at_eol = 0;\n-\tfor (cp = format; *cp && (sp = find_next(cp)); ) {\n-\t\tconst char *color, *ep = strchr(sp, ')');\n-\t\tint at;\n-\n-\t\tif (!ep)\n-\t\t\treturn error(\"malformed format string %s\", sp);\n-\t\t/* sp points at \"%(\" and ep points at the closing \")\" */\n-\t\tat = parse_atom(sp + 2, ep);\n-\t\tcp = ep + 1;\n-\n-\t\tif (skip_prefix(used_atom[at], \"color:\", &color))\n-\t\t\tneed_color_reset_at_eol = !!strcmp(color, \"reset\");\n-\t}\n-\treturn 0;\n-}\n-\n-/*\n- * Given an object name, read the object data and size, and return a\n- * \"struct object\".  If the object data we are returning is also borrowed\n- * by the \"struct object\" representation, set *eaten as well---it is a\n- * signal from parse_object_buffer to us not to free the buffer.\n- */\n-static void *get_obj(const unsigned char *sha1, struct object **obj, unsigned long *sz, int *eaten)\n-{\n-\tenum object_type type;\n-\tvoid *buf = read_sha1_file(sha1, &type, sz);\n-\n-\tif (buf)\n-\t\t*obj = parse_object_buffer(sha1, type, *sz, buf, eaten);\n-\telse\n-\t\t*obj = NULL;\n-\treturn buf;\n-}\n-\n-static int grab_objectname(const char *name, const unsigned char *sha1,\n-\t\t\t    struct atom_value *v)\n-{\n-\tif (!strcmp(name, \"objectname\")) {\n-\t\tchar *s = xmalloc(41);\n-\t\tstrcpy(s, sha1_to_hex(sha1));\n-\t\tv->s = s;\n-\t\treturn 1;\n-\t}\n-\tif (!strcmp(name, \"objectname:short\")) {\n-\t\tv->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));\n-\t\treturn 1;\n-\t}\n-\treturn 0;\n-}\n-\n-/* See grab_values */\n-static void grab_common_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < used_atom_cnt; i++) {\n-\t\tconst char *name = used_atom[i];\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\t\tv->s = typename(obj->type);\n-\t\telse if (!strcmp(name, \"objectsize\")) {\n-\t\t\tchar *s = xmalloc(40);\n-\t\t\tsprintf(s, \"%lu\", sz);\n-\t\t\tv->ul = sz;\n-\t\t\tv->s = s;\n-\t\t}\n-\t\telse if (deref)\n-\t\t\tgrab_objectname(name, obj->sha1, v);\n-\t}\n-}\n-\n-/* See grab_values */\n-static void grab_tag_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n-{\n-\tint i;\n-\tstruct tag *tag = (struct tag *) obj;\n-\n-\tfor (i = 0; i < used_atom_cnt; i++) {\n-\t\tconst char *name = used_atom[i];\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\t\tv->s = tag->tag;\n-\t\telse if (!strcmp(name, \"type\") && tag->tagged)\n-\t\t\tv->s = typename(tag->tagged->type);\n-\t\telse if (!strcmp(name, \"object\") && tag->tagged) {\n-\t\t\tchar *s = xmalloc(41);\n-\t\t\tstrcpy(s, sha1_to_hex(tag->tagged->sha1));\n-\t\t\tv->s = s;\n-\t\t}\n-\t}\n-}\n-\n-/* See grab_values */\n-static void grab_commit_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n-{\n-\tint i;\n-\tstruct commit *commit = (struct commit *) obj;\n-\n-\tfor (i = 0; i < used_atom_cnt; i++) {\n-\t\tconst char *name = used_atom[i];\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, \"tree\")) {\n-\t\t\tchar *s = xmalloc(41);\n-\t\t\tstrcpy(s, sha1_to_hex(commit->tree->object.sha1));\n-\t\t\tv->s = s;\n-\t\t}\n-\t\tif (!strcmp(name, \"numparent\")) {\n-\t\t\tchar *s = xmalloc(40);\n-\t\t\tv->ul = commit_list_count(commit->parents);\n-\t\t\tsprintf(s, \"%lu\", v->ul);\n-\t\t\tv->s = s;\n-\t\t}\n-\t\telse if (!strcmp(name, \"parent\")) {\n-\t\t\tint num = commit_list_count(commit->parents);\n-\t\t\tint i;\n-\t\t\tstruct commit_list *parents;\n-\t\t\tchar *s = xmalloc(41 * num + 1);\n-\t\t\tv->s = s;\n-\t\t\tfor (i = 0, parents = commit->parents;\n-\t\t\t     parents;\n-\t\t\t     parents = parents->next, i = i + 41) {\n-\t\t\t\tstruct commit *parent = parents->item;\n-\t\t\t\tstrcpy(s+i, sha1_to_hex(parent->object.sha1));\n-\t\t\t\tif (parents->next)\n-\t\t\t\t\ts[i+40] = ' ';\n-\t\t\t}\n-\t\t\tif (!i)\n-\t\t\t\t*s = '\\0';\n-\t\t}\n-\t}\n-}\n-\n-static const char *find_wholine(const char *who, int wholen, const char *buf, unsigned long sz)\n-{\n-\tconst char *eol;\n-\twhile (*buf) {\n-\t\tif (!strncmp(buf, who, wholen) &&\n-\t\t    buf[wholen] == ' ')\n-\t\t\treturn buf + wholen + 1;\n-\t\teol = strchr(buf, '\\n');\n-\t\tif (!eol)\n-\t\t\treturn \"\";\n-\t\teol++;\n-\t\tif (*eol == '\\n')\n-\t\t\treturn \"\"; /* end of header */\n-\t\tbuf = eol;\n-\t}\n-\treturn \"\";\n-}\n-\n-static const char *copy_line(const char *buf)\n-{\n-\tconst char *eol = strchrnul(buf, '\\n');\n-\treturn xmemdupz(buf, eol - buf);\n-}\n-\n-static const char *copy_name(const char *buf)\n-{\n-\tconst char *cp;\n-\tfor (cp = buf; *cp && *cp != '\\n'; cp++) {\n-\t\tif (!strncmp(cp, \" <\", 2))\n-\t\t\treturn xmemdupz(buf, cp - buf);\n-\t}\n-\treturn \"\";\n-}\n-\n-static const char *copy_email(const char *buf)\n-{\n-\tconst char *email = strchr(buf, '<');\n-\tconst char *eoemail;\n-\tif (!email)\n-\t\treturn \"\";\n-\teoemail = strchr(email, '>');\n-\tif (!eoemail)\n-\t\treturn \"\";\n-\treturn xmemdupz(email, eoemail + 1 - email);\n-}\n-\n-static char *copy_subject(const char *buf, unsigned long len)\n-{\n-\tchar *r = xmemdupz(buf, len);\n-\tint i;\n-\n-\tfor (i = 0; i < len; i++)\n-\t\tif (r[i] == '\\n')\n-\t\t\tr[i] = ' ';\n-\n-\treturn r;\n-}\n-\n-static void grab_date(const char *buf, struct atom_value *v, const char *atomname)\n-{\n-\tconst char *eoemail = strstr(buf, \"> \");\n-\tchar *zone;\n-\tunsigned long timestamp;\n-\tlong tz;\n-\tenum date_mode date_mode = DATE_NORMAL;\n-\tconst char *formatp;\n-\n-\t/*\n-\t * We got here because atomname ends in \"date\" or \"date<something>\";\n-\t * it's not possible that <something> is not \":<format>\" because\n-\t * parse_atom() wouldn't have allowed it, so we can assume that no\n-\t * \":\" means no format is specified, and use the default.\n-\t */\n-\tformatp = strchr(atomname, ':');\n-\tif (formatp != NULL) {\n-\t\tformatp++;\n-\t\tdate_mode = parse_date_format(formatp);\n-\t}\n-\n-\tif (!eoemail)\n-\t\tgoto bad;\n-\ttimestamp = strtoul(eoemail + 2, &zone, 10);\n-\tif (timestamp == ULONG_MAX)\n-\t\tgoto bad;\n-\ttz = strtol(zone, NULL, 10);\n-\tif ((tz == LONG_MIN || tz == LONG_MAX) && errno == ERANGE)\n-\t\tgoto bad;\n-\tv->s = xstrdup(show_date(timestamp, tz, date_mode));\n-\tv->ul = timestamp;\n-\treturn;\n- bad:\n-\tv->s = \"\";\n-\tv->ul = 0;\n-}\n-\n-/* See grab_values */\n-static void grab_person(const char *who, struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n-{\n-\tint i;\n-\tint wholen = strlen(who);\n-\tconst char *wholine = NULL;\n-\n-\tfor (i = 0; i < used_atom_cnt; i++) {\n-\t\tconst char *name = used_atom[i];\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 (strncmp(who, name, wholen))\n-\t\t\tcontinue;\n-\t\tif (name[wholen] != 0 &&\n-\t\t    strcmp(name + wholen, \"name\") &&\n-\t\t    strcmp(name + wholen, \"email\") &&\n-\t\t    !starts_with(name + wholen, \"date\"))\n-\t\t\tcontinue;\n-\t\tif (!wholine)\n-\t\t\twholine = find_wholine(who, wholen, buf, sz);\n-\t\tif (!wholine)\n-\t\t\treturn; /* no point looking for it */\n-\t\tif (name[wholen] == 0)\n-\t\t\tv->s = copy_line(wholine);\n-\t\telse if (!strcmp(name + wholen, \"name\"))\n-\t\t\tv->s = copy_name(wholine);\n-\t\telse if (!strcmp(name + wholen, \"email\"))\n-\t\t\tv->s = copy_email(wholine);\n-\t\telse if (starts_with(name + wholen, \"date\"))\n-\t\t\tgrab_date(wholine, v, name);\n-\t}\n-\n-\t/*\n-\t * For a tag or a commit object, if \"creator\" or \"creatordate\" is\n-\t * requested, do something special.\n-\t */\n-\tif (strcmp(who, \"tagger\") && strcmp(who, \"committer\"))\n-\t\treturn; /* \"author\" for commit object is not wanted */\n-\tif (!wholine)\n-\t\twholine = find_wholine(who, wholen, buf, sz);\n-\tif (!wholine)\n-\t\treturn;\n-\tfor (i = 0; i < used_atom_cnt; i++) {\n-\t\tconst char *name = used_atom[i];\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\t\tgrab_date(wholine, v, name);\n-\t\telse if (!strcmp(name, \"creator\"))\n-\t\t\tv->s = copy_line(wholine);\n-\t}\n-}\n-\n-static void find_subpos(const char *buf, unsigned long sz,\n-\t\t\tconst char **sub, unsigned long *sublen,\n-\t\t\tconst char **body, unsigned long *bodylen,\n-\t\t\tunsigned long *nonsiglen,\n-\t\t\tconst char **sig, unsigned long *siglen)\n-{\n-\tconst char *eol;\n-\t/* skip past header until we hit empty line */\n-\twhile (*buf && *buf != '\\n') {\n-\t\teol = strchrnul(buf, '\\n');\n-\t\tif (*eol)\n-\t\t\teol++;\n-\t\tbuf = eol;\n-\t}\n-\t/* skip any empty lines */\n-\twhile (*buf == '\\n')\n-\t\tbuf++;\n-\n-\t/* parse signature first; we might not even have a subject line */\n-\t*sig = buf + parse_signature(buf, strlen(buf));\n-\t*siglen = strlen(*sig);\n-\n-\t/* subject is first non-empty line */\n-\t*sub = buf;\n-\t/* subject goes to first empty line */\n-\twhile (buf < *sig && *buf && *buf != '\\n') {\n-\t\teol = strchrnul(buf, '\\n');\n-\t\tif (*eol)\n-\t\t\teol++;\n-\t\tbuf = eol;\n-\t}\n-\t*sublen = buf - *sub;\n-\t/* drop trailing newline, if present */\n-\tif (*sublen && (*sub)[*sublen - 1] == '\\n')\n-\t\t*sublen -= 1;\n-\n-\t/* skip any empty lines */\n-\twhile (*buf == '\\n')\n-\t\tbuf++;\n-\t*body = buf;\n-\t*bodylen = strlen(buf);\n-\t*nonsiglen = *sig - buf;\n-}\n-\n-/* See grab_values */\n-static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n-{\n-\tint i;\n-\tconst char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;\n-\tunsigned long sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;\n-\n-\tfor (i = 0; i < used_atom_cnt; i++) {\n-\t\tconst char *name = used_atom[i];\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, \"subject\") &&\n-\t\t    strcmp(name, \"body\") &&\n-\t\t    strcmp(name, \"contents\") &&\n-\t\t    strcmp(name, \"contents:subject\") &&\n-\t\t    strcmp(name, \"contents:body\") &&\n-\t\t    strcmp(name, \"contents:signature\"))\n-\t\t\tcontinue;\n-\t\tif (!subpos)\n-\t\t\tfind_subpos(buf, sz,\n-\t\t\t\t    &subpos, &sublen,\n-\t\t\t\t    &bodypos, &bodylen, &nonsiglen,\n-\t\t\t\t    &sigpos, &siglen);\n-\n-\t\tif (!strcmp(name, \"subject\"))\n-\t\t\tv->s = copy_subject(subpos, sublen);\n-\t\telse if (!strcmp(name, \"contents:subject\"))\n-\t\t\tv->s = copy_subject(subpos, sublen);\n-\t\telse if (!strcmp(name, \"body\"))\n-\t\t\tv->s = xmemdupz(bodypos, bodylen);\n-\t\telse if (!strcmp(name, \"contents:body\"))\n-\t\t\tv->s = xmemdupz(bodypos, nonsiglen);\n-\t\telse if (!strcmp(name, \"contents:signature\"))\n-\t\t\tv->s = xmemdupz(sigpos, siglen);\n-\t\telse if (!strcmp(name, \"contents\"))\n-\t\t\tv->s = xstrdup(subpos);\n-\t}\n-}\n-\n-/*\n- * We want to have empty print-string for field requests\n- * that do not apply (e.g. \"authordate\" for a tag object)\n- */\n-static void fill_missing_values(struct atom_value *val)\n-{\n-\tint i;\n-\tfor (i = 0; i < used_atom_cnt; i++) {\n-\t\tstruct atom_value *v = &val[i];\n-\t\tif (v->s == NULL)\n-\t\t\tv->s = \"\";\n-\t}\n-}\n-\n-/*\n- * val is a list of atom_value to hold returned values.  Extract\n- * the values for atoms in used_atom array out of (obj, buf, sz).\n- * when deref is false, (obj, buf, sz) is the object that is\n- * pointed at by the ref itself; otherwise it is the object the\n- * ref (which is a tag) refers to.\n- */\n-static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n-{\n-\tgrab_common_values(val, deref, obj, buf, sz);\n-\tswitch (obj->type) {\n-\tcase OBJ_TAG:\n-\t\tgrab_tag_values(val, deref, obj, buf, sz);\n-\t\tgrab_sub_body_contents(val, deref, obj, buf, sz);\n-\t\tgrab_person(\"tagger\", val, deref, obj, buf, sz);\n-\t\tbreak;\n-\tcase OBJ_COMMIT:\n-\t\tgrab_commit_values(val, deref, obj, buf, sz);\n-\t\tgrab_sub_body_contents(val, deref, obj, buf, sz);\n-\t\tgrab_person(\"author\", val, deref, obj, buf, sz);\n-\t\tgrab_person(\"committer\", val, deref, obj, buf, sz);\n-\t\tbreak;\n-\tcase OBJ_TREE:\n-\t\t/* grab_tree_values(val, deref, obj, buf, sz); */\n-\t\tbreak;\n-\tcase OBJ_BLOB:\n-\t\t/* grab_blob_values(val, deref, obj, buf, sz); */\n-\t\tbreak;\n-\tdefault:\n-\t\tdie(\"Eh?  Object of type %d?\", obj->type);\n-\t}\n-}\n-\n-static inline char *copy_advance(char *dst, const char *src)\n-{\n-\twhile (*src)\n-\t\t*dst++ = *src++;\n-\treturn dst;\n-}\n-\n-/*\n- * Parse the object referred by ref, and grab needed value.\n- */\n-static void populate_value(struct ref_filter_item *ref)\n-{\n-\tvoid *buf;\n-\tstruct object *obj;\n-\tint eaten, i;\n-\tunsigned long size;\n-\tconst unsigned char *tagged;\n-\n-\tref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));\n-\n-\tif (need_symref && (ref->flags & REF_ISSYMREF) && !ref->symref) {\n-\t\tunsigned char unused1[20];\n-\t\tref->symref = resolve_refdup(ref->name, RESOLVE_REF_READING,\n-\t\t\t\t\t     unused1, NULL);\n-\t\tif (!ref->symref)\n-\t\t\tref->symref = \"\";\n-\t}\n-\n-\t/* Fill in specials first */\n-\tfor (i = 0; i < used_atom_cnt; i++) {\n-\t\tconst char *name = used_atom[i];\n-\t\tstruct atom_value *v = &ref->value[i];\n-\t\tint deref = 0;\n-\t\tconst char *refname;\n-\t\tconst char *formatp;\n-\t\tstruct branch *branch = NULL;\n-\n-\t\tif (*name == '*') {\n-\t\t\tderef = 1;\n-\t\t\tname++;\n-\t\t}\n-\n-\t\tif (starts_with(name, \"refname\"))\n-\t\t\trefname = ref->name;\n-\t\telse if (starts_with(name, \"symref\"))\n-\t\t\trefname = ref->symref ? ref->symref : \"\";\n-\t\telse if (starts_with(name, \"upstream\")) {\n-\t\t\t/* only local branches may have an upstream */\n-\t\t\tif (!starts_with(ref->name, \"refs/heads/\"))\n-\t\t\t\tcontinue;\n-\t\t\tbranch = branch_get(ref->name + 11);\n-\n-\t\t\tif (!branch || !branch->merge || !branch->merge[0] ||\n-\t\t\t    !branch->merge[0]->dst)\n-\t\t\t\tcontinue;\n-\t\t\trefname = branch->merge[0]->dst;\n-\t\t} else if (starts_with(name, \"color:\")) {\n-\t\t\tchar color[COLOR_MAXLEN] = \"\";\n-\n-\t\t\tif (color_parse(name + 6, color) < 0)\n-\t\t\t\tdie(_(\"unable to parse format\"));\n-\t\t\tv->s = xstrdup(color);\n-\t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"flag\")) {\n-\t\t\tchar buf[256], *cp = buf;\n-\t\t\tif (ref->flags & REF_ISSYMREF)\n-\t\t\t\tcp = copy_advance(cp, \",symref\");\n-\t\t\tif (ref->flags & REF_ISPACKED)\n-\t\t\t\tcp = copy_advance(cp, \",packed\");\n-\t\t\tif (cp == buf)\n-\t\t\t\tv->s = \"\";\n-\t\t\telse {\n-\t\t\t\t*cp = '\\0';\n-\t\t\t\tv->s = xstrdup(buf + 1);\n-\t\t\t}\n-\t\t\tcontinue;\n-\t\t} else if (!deref && grab_objectname(name, ref->sha1, v)) {\n-\t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"HEAD\")) {\n-\t\t\tconst char *head;\n-\t\t\tunsigned char sha1[20];\n-\n-\t\t\thead = resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n-\t\t\t\t\t\t  sha1, NULL);\n-\t\t\tif (!strcmp(ref->name, head))\n-\t\t\t\tv->s = \"*\";\n-\t\t\telse\n-\t\t\t\tv->s = \" \";\n-\t\t\tcontinue;\n-\t\t} else\n-\t\t\tcontinue;\n-\n-\t\tformatp = strchr(name, ':');\n-\t\tif (formatp) {\n-\t\t\tint num_ours, num_theirs;\n-\n-\t\t\tformatp++;\n-\t\t\tif (!strcmp(formatp, \"short\"))\n-\t\t\t\trefname = shorten_unambiguous_ref(refname,\n-\t\t\t\t\t\t      warn_ambiguous_refs);\n-\t\t\telse if (!strcmp(formatp, \"track\") &&\n-\t\t\t\t starts_with(name, \"upstream\")) {\n-\t\t\t\tchar buf[40];\n-\n-\t\t\t\tif (stat_tracking_info(branch, &num_ours,\n-\t\t\t\t\t\t       &num_theirs) != 1)\n-\t\t\t\t\tcontinue;\n-\n-\t\t\t\tif (!num_ours && !num_theirs)\n-\t\t\t\t\tv->s = \"\";\n-\t\t\t\telse if (!num_ours) {\n-\t\t\t\t\tsprintf(buf, \"[behind %d]\", num_theirs);\n-\t\t\t\t\tv->s = xstrdup(buf);\n-\t\t\t\t} else if (!num_theirs) {\n-\t\t\t\t\tsprintf(buf, \"[ahead %d]\", num_ours);\n-\t\t\t\t\tv->s = xstrdup(buf);\n-\t\t\t\t} else {\n-\t\t\t\t\tsprintf(buf, \"[ahead %d, behind %d]\",\n-\t\t\t\t\t\tnum_ours, num_theirs);\n-\t\t\t\t\tv->s = xstrdup(buf);\n-\t\t\t\t}\n-\t\t\t\tcontinue;\n-\t\t\t} else if (!strcmp(formatp, \"trackshort\") &&\n-\t\t\t\t   starts_with(name, \"upstream\")) {\n-\t\t\t\tassert(branch);\n-\n-\t\t\t\tif (stat_tracking_info(branch, &num_ours,\n-\t\t\t\t\t\t\t&num_theirs) != 1)\n-\t\t\t\t\tcontinue;\n-\n-\t\t\t\tif (!num_ours && !num_theirs)\n-\t\t\t\t\tv->s = \"=\";\n-\t\t\t\telse if (!num_ours)\n-\t\t\t\t\tv->s = \"<\";\n-\t\t\t\telse if (!num_theirs)\n-\t\t\t\t\tv->s = \">\";\n-\t\t\t\telse\n-\t\t\t\t\tv->s = \"<>\";\n-\t\t\t\tcontinue;\n-\t\t\t} else\n-\t\t\t\tdie(\"unknown %.*s format %s\",\n-\t\t\t\t    (int)(formatp - name), name, formatp);\n-\t\t}\n-\n-\t\tif (!deref)\n-\t\t\tv->s = refname;\n-\t\telse {\n-\t\t\tint len = strlen(refname);\n-\t\t\tchar *s = xmalloc(len + 4);\n-\t\t\tsprintf(s, \"%s^{}\", refname);\n-\t\t\tv->s = s;\n-\t\t}\n-\t}\n-\n-\tfor (i = 0; i < used_atom_cnt; i++) {\n-\t\tstruct atom_value *v = &ref->value[i];\n-\t\tif (v->s == NULL)\n-\t\t\tgoto need_obj;\n-\t}\n-\treturn;\n-\n- need_obj:\n-\tbuf = get_obj(ref->sha1, &obj, &size, &eaten);\n-\tif (!buf)\n-\t\tdie(\"missing object %s for %s\",\n-\t\t    sha1_to_hex(ref->sha1), ref->name);\n-\tif (!obj)\n-\t\tdie(\"parse_object_buffer failed on %s for %s\",\n-\t\t    sha1_to_hex(ref->sha1), ref->name);\n-\n-\tgrab_values(ref->value, 0, obj, buf, size);\n-\tif (!eaten)\n-\t\tfree(buf);\n-\n-\t/*\n-\t * If there is no atom that wants to know about tagged\n-\t * object, we are done.\n-\t */\n-\tif (!need_tagged || (obj->type != OBJ_TAG))\n-\t\treturn;\n-\n-\t/*\n-\t * If it is a tag object, see if we use a value that derefs\n-\t * the object, and if we do grab the object it refers to.\n-\t */\n-\ttagged = ((struct tag *)obj)->tagged->sha1;\n-\n-\t/*\n-\t * NEEDSWORK: This derefs tag only once, which\n-\t * is good to deal with chains of trust, but\n-\t * is not consistent with what deref_tag() does\n-\t * which peels the onion to the core.\n-\t */\n-\tbuf = get_obj(tagged, &obj, &size, &eaten);\n-\tif (!buf)\n-\t\tdie(\"missing object %s for %s\",\n-\t\t    sha1_to_hex(tagged), ref->name);\n-\tif (!obj)\n-\t\tdie(\"parse_object_buffer failed on %s for %s\",\n-\t\t    sha1_to_hex(tagged), ref->name);\n-\tgrab_values(ref->value, 1, obj, buf, size);\n-\tif (!eaten)\n-\t\tfree(buf);\n-}\n-\n-/*\n- * Given a ref, return the value for the atom.  This lazily gets value\n- * out of the object by calling populate value.\n- */\n-static void get_value(struct ref_filter_item *ref, int atom, struct atom_value **v)\n-{\n-\tif (!ref->value) {\n-\t\tpopulate_value(ref);\n-\t\tfill_missing_values(ref->value);\n-\t}\n-\t*v = &ref->value[atom];\n-}\n-\n-static int cmp_ref_sort(struct ref_sort *s, struct ref_filter_item *a, struct ref_filter_item *b)\n-{\n-\tstruct atom_value *va, *vb;\n-\tint cmp;\n-\tcmp_type cmp_type = used_atom_type[s->atom];\n-\n-\tget_value(a, s->atom, &va);\n-\tget_value(b, s->atom, &vb);\n-\tswitch (cmp_type) {\n-\tcase FIELD_STR:\n-\t\tcmp = strcmp(va->s, vb->s);\n-\t\tbreak;\n-\tdefault:\n-\t\tif (va->ul < vb->ul)\n-\t\t\tcmp = -1;\n-\t\telse if (va->ul == vb->ul)\n-\t\t\tcmp = 0;\n-\t\telse\n-\t\t\tcmp = 1;\n-\t\tbreak;\n-\t}\n-\treturn (s->reverse) ? -cmp : cmp;\n-}\n-\n-static struct ref_sort *ref_sort;\n-static int compare_refs(const void *a_, const void *b_)\n-{\n-\tstruct ref_filter_item *a = *((struct ref_filter_item **)a_);\n-\tstruct ref_filter_item *b = *((struct ref_filter_item **)b_);\n-\tstruct ref_sort *s;\n-\n-\tfor (s = ref_sort; s; s = s->next) {\n-\t\tint cmp = cmp_ref_sort(s, a, b);\n-\t\tif (cmp)\n-\t\t\treturn cmp;\n-\t}\n-\treturn 0;\n-}\n-\n-static void sort_refs(struct ref_sort *sort, struct ref_filter *refs)\n-{\n-\tref_sort = sort;\n-\tqsort(refs->items, refs->count, sizeof(struct ref_filter_item *), compare_refs);\n-}\n-\n-static void print_value(struct atom_value *v, int quote_style)\n-{\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tswitch (quote_style) {\n-\tcase QUOTE_NONE:\n-\t\tfputs(v->s, stdout);\n-\t\tbreak;\n-\tcase QUOTE_SHELL:\n-\t\tsq_quote_buf(&sb, v->s);\n-\t\tbreak;\n-\tcase QUOTE_PERL:\n-\t\tperl_quote_buf(&sb, v->s);\n-\t\tbreak;\n-\tcase QUOTE_PYTHON:\n-\t\tpython_quote_buf(&sb, v->s);\n-\t\tbreak;\n-\tcase QUOTE_TCL:\n-\t\ttcl_quote_buf(&sb, v->s);\n-\t\tbreak;\n-\t}\n-\tif (quote_style != QUOTE_NONE) {\n-\t\tfputs(sb.buf, stdout);\n-\t\tstrbuf_release(&sb);\n-\t}\n-}\n-\n-static int hex1(char ch)\n-{\n-\tif ('0' <= ch && ch <= '9')\n-\t\treturn ch - '0';\n-\telse if ('a' <= ch && ch <= 'f')\n-\t\treturn ch - 'a' + 10;\n-\telse if ('A' <= ch && ch <= 'F')\n-\t\treturn ch - 'A' + 10;\n-\treturn -1;\n-}\n-static int hex2(const char *cp)\n-{\n-\tif (cp[0] && cp[1])\n-\t\treturn (hex1(cp[0]) << 4) | hex1(cp[1]);\n-\telse\n-\t\treturn -1;\n-}\n-\n-static void emit(const char *cp, const char *ep)\n-{\n-\twhile (*cp && (!ep || cp < ep)) {\n-\t\tif (*cp == '%') {\n-\t\t\tif (cp[1] == '%')\n-\t\t\t\tcp++;\n-\t\t\telse {\n-\t\t\t\tint ch = hex2(cp + 1);\n-\t\t\t\tif (0 <= ch) {\n-\t\t\t\t\tputchar(ch);\n-\t\t\t\t\tcp += 3;\n-\t\t\t\t\tcontinue;\n-\t\t\t\t}\n-\t\t\t}\n-\t\t}\n-\t\tputchar(*cp);\n-\t\tcp++;\n-\t}\n-}\n-\n-static void show_ref(struct ref_filter_item *info, const char *format, int quote_style)\n-{\n-\tconst char *cp, *sp, *ep;\n-\n-\tfor (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {\n-\t\tstruct atom_value *atomv;\n-\n-\t\tep = strchr(sp, ')');\n-\t\tif (cp < sp)\n-\t\t\temit(cp, sp);\n-\t\tget_value(info, parse_atom(sp + 2, ep), &atomv);\n-\t\tprint_value(atomv, quote_style);\n-\t}\n-\tif (*cp) {\n-\t\tsp = cp + strlen(cp);\n-\t\temit(cp, sp);\n-\t}\n-\tif (need_color_reset_at_eol) {\n-\t\tstruct atom_value resetv;\n-\t\tchar color[COLOR_MAXLEN] = \"\";\n-\n-\t\tif (color_parse(\"reset\", color) < 0)\n-\t\t\tdie(\"BUG: couldn't parse 'reset' as a color\");\n-\t\tresetv.s = color;\n-\t\tprint_value(&resetv, quote_style);\n-\t}\n-\tputchar('\\n');\n-}\n-\n-static struct ref_sort *default_sort(void)\n-{\n-\tstatic const char cstr_name[] = \"refname\";\n-\n-\tstruct ref_sort *sort = xcalloc(1, sizeof(*sort));\n-\n-\tsort->next = NULL;\n-\tsort->atom = parse_atom(cstr_name, cstr_name + strlen(cstr_name));\n-\treturn sort;\n-}\n-\n-static int opt_parse_sort(const struct option *opt, const char *arg, int unset)\n-{\n-\tstruct ref_sort **sort_tail = opt->value;\n-\tstruct ref_sort *s;\n-\tint len;\n-\n-\tif (!arg) /* should --no-sort void the list ? */\n-\t\treturn -1;\n-\n-\ts = xcalloc(1, sizeof(*s));\n-\ts->next = *sort_tail;\n-\t*sort_tail = s;\n-\n-\tif (*arg == '-') {\n-\t\ts->reverse = 1;\n-\t\targ++;\n-\t}\n-\tlen = strlen(arg);\n-\ts->atom = parse_atom(arg, arg+len);\n-\treturn 0;\n-}\n+#include \"parse-options.h\"\n \n static char const * const for_each_ref_usage[] = {\n \tN_(\"git for-each-ref [<options>] [<pattern>]\"),\n@@ -1011,7 +32,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \t\tOPT_INTEGER( 0 , \"count\", &maxcount, N_(\"show only <n> matched refs\")),\n \t\tOPT_STRING(  0 , \"format\", &format, N_(\"format\"), N_(\"format to use for the output\")),\n \t\tOPT_CALLBACK(0 , \"sort\", sort_tail, N_(\"key\"),\n-\t\t\t    N_(\"field name to sort on\"), &opt_parse_sort),\n+\t\t\t     N_(\"field name to sort on\"), &ref_opt_parse_sort),\n \t\tOPT_END(),\n \t};\n \n@@ -1028,7 +49,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(for_each_ref_usage, opts);\n \n \tif (!sort)\n-\t\tsort = default_sort();\n+\t\tsort = ref_default_sort();\n \n \t/* for warn_ambiguous_refs */\n \tgit_config(git_default_config, NULL);\ndiff --git a/ref-filter.c b/ref-filter.c\nindex df571a6..42f0f35 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1,9 +1,979 @@\n #include \"builtin.h\"\n #include \"cache.h\"\n+#include \"parse-options.h\"\n #include \"refs.h\"\n #include \"ref-filter.h\"\n #include \"wildmatch.h\"\n #include \"commit.h\"\n+#include \"remote.h\"\n+#include \"color.h\"\n+#include \"tag.h\"\n+#include \"quote.h\"\n+\n+typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n+\n+static struct {\n+\tconst char *name;\n+\tcmp_type cmp_type;\n+} valid_atom[] = {\n+\t{ \"refname\" },\n+\t{ \"objecttype\" },\n+\t{ \"objectsize\", FIELD_ULONG },\n+\t{ \"objectname\" },\n+\t{ \"tree\" },\n+\t{ \"parent\" },\n+\t{ \"numparent\", FIELD_ULONG },\n+\t{ \"object\" },\n+\t{ \"type\" },\n+\t{ \"tag\" },\n+\t{ \"author\" },\n+\t{ \"authorname\" },\n+\t{ \"authoremail\" },\n+\t{ \"authordate\", FIELD_TIME },\n+\t{ \"committer\" },\n+\t{ \"committername\" },\n+\t{ \"committeremail\" },\n+\t{ \"committerdate\", FIELD_TIME },\n+\t{ \"tagger\" },\n+\t{ \"taggername\" },\n+\t{ \"taggeremail\" },\n+\t{ \"taggerdate\", FIELD_TIME },\n+\t{ \"creator\" },\n+\t{ \"creatordate\", FIELD_TIME },\n+\t{ \"subject\" },\n+\t{ \"body\" },\n+\t{ \"contents\" },\n+\t{ \"contents:subject\" },\n+\t{ \"contents:body\" },\n+\t{ \"contents:signature\" },\n+\t{ \"upstream\" },\n+\t{ \"symref\" },\n+\t{ \"flag\" },\n+\t{ \"HEAD\" },\n+\t{ \"color\" },\n+};\n+\n+/*\n+ * An atom is a valid field atom listed above, possibly prefixed with\n+ * a \"*\" to denote deref_tag().\n+ *\n+ * We parse given format string and sort specifiers, and make a list\n+ * of properties that we need to extract out of objects.  ref_filter_item\n+ * structure will hold an array of values extracted that can be\n+ * indexed with the \"atom number\", which is an index into this\n+ * array.\n+ */\n+static const char **used_atom;\n+static cmp_type *used_atom_type;\n+static int used_atom_cnt, need_tagged, need_symref;\n+int need_color_reset_at_eol;\n+\n+/*\n+ * Used to parse format string and sort specifiers\n+ */\n+int parse_atom(const char *atom, const char *ep)\n+{\n+\tconst char *sp;\n+\tint i, at;\n+\n+\tsp = atom;\n+\tif (*sp == '*' && sp < ep)\n+\t\tsp++; /* deref */\n+\tif (ep <= sp)\n+\t\tdie(\"malformed field name: %.*s\", (int)(ep-atom), atom);\n+\n+\t/* Do we have the atom already used elsewhere? */\n+\tfor (i = 0; i < used_atom_cnt; i++) {\n+\t\tint len = strlen(used_atom[i]);\n+\t\tif (len == ep - atom && !memcmp(used_atom[i], atom, len))\n+\t\t\treturn i;\n+\t}\n+\n+\t/* Is the atom a valid one? */\n+\tfor (i = 0; i < ARRAY_SIZE(valid_atom); i++) {\n+\t\tint len = strlen(valid_atom[i].name);\n+\t\t/*\n+\t\t * If the atom name has a colon, strip it and everything after\n+\t\t * it off - it specifies the format for this entry, and\n+\t\t * shouldn't be used for checking against the valid_atom\n+\t\t * table.\n+\t\t */\n+\t\tconst char *formatp = strchr(sp, ':');\n+\t\tif (!formatp || ep < formatp)\n+\t\t\tformatp = ep;\n+\t\tif (len == formatp - sp && !memcmp(valid_atom[i].name, sp, len))\n+\t\t\tbreak;\n+\t}\n+\n+\tif (ARRAY_SIZE(valid_atom) <= i)\n+\t\tdie(\"unknown field name: %.*s\", (int)(ep-atom), atom);\n+\n+\t/* Add it in, including the deref prefix */\n+\tat = used_atom_cnt;\n+\tused_atom_cnt++;\n+\tREALLOC_ARRAY(used_atom, used_atom_cnt);\n+\tREALLOC_ARRAY(used_atom_type, used_atom_cnt);\n+\tused_atom[at] = xmemdupz(atom, ep - atom);\n+\tused_atom_type[at] = valid_atom[i].cmp_type;\n+\tif (*atom == '*')\n+\t\tneed_tagged = 1;\n+\tif (!strcmp(used_atom[at], \"symref\"))\n+\t\tneed_symref = 1;\n+\treturn at;\n+}\n+\n+/*\n+ * In a format string, find the next occurrence of %(atom).\n+ */\n+static const char *find_next(const char *cp)\n+{\n+\twhile (*cp) {\n+\t\tif (*cp == '%') {\n+\t\t\t/*\n+\t\t\t * %( is the start of an atom;\n+\t\t\t * %% is a quoted per-cent.\n+\t\t\t */\n+\t\t\tif (cp[1] == '(')\n+\t\t\t\treturn cp;\n+\t\t\telse if (cp[1] == '%')\n+\t\t\t\tcp++; /* skip over two % */\n+\t\t\t/* otherwise this is a singleton, literal % */\n+\t\t}\n+\t\tcp++;\n+\t}\n+\treturn NULL;\n+}\n+\n+/*\n+ * Make sure the format string is well formed, and parse out\n+ * the used atoms.\n+ */\n+int verify_format(const char *format)\n+{\n+\tconst char *cp, *sp;\n+\n+\tneed_color_reset_at_eol = 0;\n+\tfor (cp = format; *cp && (sp = find_next(cp)); ) {\n+\t\tconst char *color, *ep = strchr(sp, ')');\n+\t\tint at;\n+\n+\t\tif (!ep)\n+\t\t\treturn error(\"malformed format string %s\", sp);\n+\t\t/* sp points at \"%(\" and ep points at the closing \")\" */\n+\t\tat = parse_atom(sp + 2, ep);\n+\t\tcp = ep + 1;\n+\n+\t\tif (skip_prefix(used_atom[at], \"color:\", &color))\n+\t\t\tneed_color_reset_at_eol = !!strcmp(color, \"reset\");\n+\t}\n+\treturn 0;\n+}\n+\n+/*\n+ * Given an object name, read the object data and size, and return a\n+ * \"struct object\".  If the object data we are returning is also borrowed\n+ * by the \"struct object\" representation, set *eaten as well---it is a\n+ * signal from parse_object_buffer to us not to free the buffer.\n+ */\n+static void *get_obj(const unsigned char *sha1, struct object **obj, unsigned long *sz, int *eaten)\n+{\n+\tenum object_type type;\n+\tvoid *buf = read_sha1_file(sha1, &type, sz);\n+\n+\tif (buf)\n+\t\t*obj = parse_object_buffer(sha1, type, *sz, buf, eaten);\n+\telse\n+\t\t*obj = NULL;\n+\treturn buf;\n+}\n+\n+static int grab_objectname(const char *name, const unsigned char *sha1,\n+\t\t\t    struct atom_value *v)\n+{\n+\tif (!strcmp(name, \"objectname\")) {\n+\t\tchar *s = xmalloc(41);\n+\t\tstrcpy(s, sha1_to_hex(sha1));\n+\t\tv->s = s;\n+\t\treturn 1;\n+\t}\n+\tif (!strcmp(name, \"objectname:short\")) {\n+\t\tv->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+/* See grab_values */\n+static void grab_common_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < used_atom_cnt; i++) {\n+\t\tconst char *name = used_atom[i];\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\t\tv->s = typename(obj->type);\n+\t\telse if (!strcmp(name, \"objectsize\")) {\n+\t\t\tchar *s = xmalloc(40);\n+\t\t\tsprintf(s, \"%lu\", sz);\n+\t\t\tv->ul = sz;\n+\t\t\tv->s = s;\n+\t\t}\n+\t\telse if (deref)\n+\t\t\tgrab_objectname(name, obj->sha1, v);\n+\t}\n+}\n+\n+/* See grab_values */\n+static void grab_tag_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n+{\n+\tint i;\n+\tstruct tag *tag = (struct tag *) obj;\n+\n+\tfor (i = 0; i < used_atom_cnt; i++) {\n+\t\tconst char *name = used_atom[i];\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\t\tv->s = tag->tag;\n+\t\telse if (!strcmp(name, \"type\") && tag->tagged)\n+\t\t\tv->s = typename(tag->tagged->type);\n+\t\telse if (!strcmp(name, \"object\") && tag->tagged) {\n+\t\t\tchar *s = xmalloc(41);\n+\t\t\tstrcpy(s, sha1_to_hex(tag->tagged->sha1));\n+\t\t\tv->s = s;\n+\t\t}\n+\t}\n+}\n+\n+/* See grab_values */\n+static void grab_commit_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n+{\n+\tint i;\n+\tstruct commit *commit = (struct commit *) obj;\n+\n+\tfor (i = 0; i < used_atom_cnt; i++) {\n+\t\tconst char *name = used_atom[i];\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, \"tree\")) {\n+\t\t\tchar *s = xmalloc(41);\n+\t\t\tstrcpy(s, sha1_to_hex(commit->tree->object.sha1));\n+\t\t\tv->s = s;\n+\t\t}\n+\t\tif (!strcmp(name, \"numparent\")) {\n+\t\t\tchar *s = xmalloc(40);\n+\t\t\tv->ul = commit_list_count(commit->parents);\n+\t\t\tsprintf(s, \"%lu\", v->ul);\n+\t\t\tv->s = s;\n+\t\t}\n+\t\telse if (!strcmp(name, \"parent\")) {\n+\t\t\tint num = commit_list_count(commit->parents);\n+\t\t\tint i;\n+\t\t\tstruct commit_list *parents;\n+\t\t\tchar *s = xmalloc(41 * num + 1);\n+\t\t\tv->s = s;\n+\t\t\tfor (i = 0, parents = commit->parents;\n+\t\t\t     parents;\n+\t\t\t     parents = parents->next, i = i + 41) {\n+\t\t\t\tstruct commit *parent = parents->item;\n+\t\t\t\tstrcpy(s+i, sha1_to_hex(parent->object.sha1));\n+\t\t\t\tif (parents->next)\n+\t\t\t\t\ts[i+40] = ' ';\n+\t\t\t}\n+\t\t\tif (!i)\n+\t\t\t\t*s = '\\0';\n+\t\t}\n+\t}\n+}\n+\n+static const char *find_wholine(const char *who, int wholen, const char *buf, unsigned long sz)\n+{\n+\tconst char *eol;\n+\twhile (*buf) {\n+\t\tif (!strncmp(buf, who, wholen) &&\n+\t\t    buf[wholen] == ' ')\n+\t\t\treturn buf + wholen + 1;\n+\t\teol = strchr(buf, '\\n');\n+\t\tif (!eol)\n+\t\t\treturn \"\";\n+\t\teol++;\n+\t\tif (*eol == '\\n')\n+\t\t\treturn \"\"; /* end of header */\n+\t\tbuf = eol;\n+\t}\n+\treturn \"\";\n+}\n+\n+static const char *copy_line(const char *buf)\n+{\n+\tconst char *eol = strchrnul(buf, '\\n');\n+\treturn xmemdupz(buf, eol - buf);\n+}\n+\n+static const char *copy_name(const char *buf)\n+{\n+\tconst char *cp;\n+\tfor (cp = buf; *cp && *cp != '\\n'; cp++) {\n+\t\tif (!strncmp(cp, \" <\", 2))\n+\t\t\treturn xmemdupz(buf, cp - buf);\n+\t}\n+\treturn \"\";\n+}\n+\n+static const char *copy_email(const char *buf)\n+{\n+\tconst char *email = strchr(buf, '<');\n+\tconst char *eoemail;\n+\tif (!email)\n+\t\treturn \"\";\n+\teoemail = strchr(email, '>');\n+\tif (!eoemail)\n+\t\treturn \"\";\n+\treturn xmemdupz(email, eoemail + 1 - email);\n+}\n+\n+static char *copy_subject(const char *buf, unsigned long len)\n+{\n+\tchar *r = xmemdupz(buf, len);\n+\tint i;\n+\n+\tfor (i = 0; i < len; i++)\n+\t\tif (r[i] == '\\n')\n+\t\t\tr[i] = ' ';\n+\n+\treturn r;\n+}\n+\n+static void grab_date(const char *buf, struct atom_value *v, const char *atomname)\n+{\n+\tconst char *eoemail = strstr(buf, \"> \");\n+\tchar *zone;\n+\tunsigned long timestamp;\n+\tlong tz;\n+\tenum date_mode date_mode = DATE_NORMAL;\n+\tconst char *formatp;\n+\n+\t/*\n+\t * We got here because atomname ends in \"date\" or \"date<something>\";\n+\t * it's not possible that <something> is not \":<format>\" because\n+\t * parse_atom() wouldn't have allowed it, so we can assume that no\n+\t * \":\" means no format is specified, and use the default.\n+\t */\n+\tformatp = strchr(atomname, ':');\n+\tif (formatp != NULL) {\n+\t\tformatp++;\n+\t\tdate_mode = parse_date_format(formatp);\n+\t}\n+\n+\tif (!eoemail)\n+\t\tgoto bad;\n+\ttimestamp = strtoul(eoemail + 2, &zone, 10);\n+\tif (timestamp == ULONG_MAX)\n+\t\tgoto bad;\n+\ttz = strtol(zone, NULL, 10);\n+\tif ((tz == LONG_MIN || tz == LONG_MAX) && errno == ERANGE)\n+\t\tgoto bad;\n+\tv->s = xstrdup(show_date(timestamp, tz, date_mode));\n+\tv->ul = timestamp;\n+\treturn;\n+ bad:\n+\tv->s = \"\";\n+\tv->ul = 0;\n+}\n+\n+/* See grab_values */\n+static void grab_person(const char *who, struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n+{\n+\tint i;\n+\tint wholen = strlen(who);\n+\tconst char *wholine = NULL;\n+\n+\tfor (i = 0; i < used_atom_cnt; i++) {\n+\t\tconst char *name = used_atom[i];\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 (strncmp(who, name, wholen))\n+\t\t\tcontinue;\n+\t\tif (name[wholen] != 0 &&\n+\t\t    strcmp(name + wholen, \"name\") &&\n+\t\t    strcmp(name + wholen, \"email\") &&\n+\t\t    !starts_with(name + wholen, \"date\"))\n+\t\t\tcontinue;\n+\t\tif (!wholine)\n+\t\t\twholine = find_wholine(who, wholen, buf, sz);\n+\t\tif (!wholine)\n+\t\t\treturn; /* no point looking for it */\n+\t\tif (name[wholen] == 0)\n+\t\t\tv->s = copy_line(wholine);\n+\t\telse if (!strcmp(name + wholen, \"name\"))\n+\t\t\tv->s = copy_name(wholine);\n+\t\telse if (!strcmp(name + wholen, \"email\"))\n+\t\t\tv->s = copy_email(wholine);\n+\t\telse if (starts_with(name + wholen, \"date\"))\n+\t\t\tgrab_date(wholine, v, name);\n+\t}\n+\n+\t/*\n+\t * For a tag or a commit object, if \"creator\" or \"creatordate\" is\n+\t * requested, do something special.\n+\t */\n+\tif (strcmp(who, \"tagger\") && strcmp(who, \"committer\"))\n+\t\treturn; /* \"author\" for commit object is not wanted */\n+\tif (!wholine)\n+\t\twholine = find_wholine(who, wholen, buf, sz);\n+\tif (!wholine)\n+\t\treturn;\n+\tfor (i = 0; i < used_atom_cnt; i++) {\n+\t\tconst char *name = used_atom[i];\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\t\tgrab_date(wholine, v, name);\n+\t\telse if (!strcmp(name, \"creator\"))\n+\t\t\tv->s = copy_line(wholine);\n+\t}\n+}\n+\n+static void find_subpos(const char *buf, unsigned long sz,\n+\t\t\tconst char **sub, unsigned long *sublen,\n+\t\t\tconst char **body, unsigned long *bodylen,\n+\t\t\tunsigned long *nonsiglen,\n+\t\t\tconst char **sig, unsigned long *siglen)\n+{\n+\tconst char *eol;\n+\t/* skip past header until we hit empty line */\n+\twhile (*buf && *buf != '\\n') {\n+\t\teol = strchrnul(buf, '\\n');\n+\t\tif (*eol)\n+\t\t\teol++;\n+\t\tbuf = eol;\n+\t}\n+\t/* skip any empty lines */\n+\twhile (*buf == '\\n')\n+\t\tbuf++;\n+\n+\t/* parse signature first; we might not even have a subject line */\n+\t*sig = buf + parse_signature(buf, strlen(buf));\n+\t*siglen = strlen(*sig);\n+\n+\t/* subject is first non-empty line */\n+\t*sub = buf;\n+\t/* subject goes to first empty line */\n+\twhile (buf < *sig && *buf && *buf != '\\n') {\n+\t\teol = strchrnul(buf, '\\n');\n+\t\tif (*eol)\n+\t\t\teol++;\n+\t\tbuf = eol;\n+\t}\n+\t*sublen = buf - *sub;\n+\t/* drop trailing newline, if present */\n+\tif (*sublen && (*sub)[*sublen - 1] == '\\n')\n+\t\t*sublen -= 1;\n+\n+\t/* skip any empty lines */\n+\twhile (*buf == '\\n')\n+\t\tbuf++;\n+\t*body = buf;\n+\t*bodylen = strlen(buf);\n+\t*nonsiglen = *sig - buf;\n+}\n+\n+/* See grab_values */\n+static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n+{\n+\tint i;\n+\tconst char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;\n+\tunsigned long sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;\n+\n+\tfor (i = 0; i < used_atom_cnt; i++) {\n+\t\tconst char *name = used_atom[i];\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, \"subject\") &&\n+\t\t    strcmp(name, \"body\") &&\n+\t\t    strcmp(name, \"contents\") &&\n+\t\t    strcmp(name, \"contents:subject\") &&\n+\t\t    strcmp(name, \"contents:body\") &&\n+\t\t    strcmp(name, \"contents:signature\"))\n+\t\t\tcontinue;\n+\t\tif (!subpos)\n+\t\t\tfind_subpos(buf, sz,\n+\t\t\t\t    &subpos, &sublen,\n+\t\t\t\t    &bodypos, &bodylen, &nonsiglen,\n+\t\t\t\t    &sigpos, &siglen);\n+\n+\t\tif (!strcmp(name, \"subject\"))\n+\t\t\tv->s = copy_subject(subpos, sublen);\n+\t\telse if (!strcmp(name, \"contents:subject\"))\n+\t\t\tv->s = copy_subject(subpos, sublen);\n+\t\telse if (!strcmp(name, \"body\"))\n+\t\t\tv->s = xmemdupz(bodypos, bodylen);\n+\t\telse if (!strcmp(name, \"contents:body\"))\n+\t\t\tv->s = xmemdupz(bodypos, nonsiglen);\n+\t\telse if (!strcmp(name, \"contents:signature\"))\n+\t\t\tv->s = xmemdupz(sigpos, siglen);\n+\t\telse if (!strcmp(name, \"contents\"))\n+\t\t\tv->s = xstrdup(subpos);\n+\t}\n+}\n+\n+/*\n+ * We want to have empty print-string for field requests\n+ * that do not apply (e.g. \"authordate\" for a tag object)\n+ */\n+static void fill_missing_values(struct atom_value *val)\n+{\n+\tint i;\n+\tfor (i = 0; i < used_atom_cnt; i++) {\n+\t\tstruct atom_value *v = &val[i];\n+\t\tif (v->s == NULL)\n+\t\t\tv->s = \"\";\n+\t}\n+}\n+\n+/*\n+ * val is a list of atom_value to hold returned values.  Extract\n+ * the values for atoms in used_atom array out of (obj, buf, sz).\n+ * when deref is false, (obj, buf, sz) is the object that is\n+ * pointed at by the ref itself; otherwise it is the object the\n+ * ref (which is a tag) refers to.\n+ */\n+static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n+{\n+\tgrab_common_values(val, deref, obj, buf, sz);\n+\tswitch (obj->type) {\n+\tcase OBJ_TAG:\n+\t\tgrab_tag_values(val, deref, obj, buf, sz);\n+\t\tgrab_sub_body_contents(val, deref, obj, buf, sz);\n+\t\tgrab_person(\"tagger\", val, deref, obj, buf, sz);\n+\t\tbreak;\n+\tcase OBJ_COMMIT:\n+\t\tgrab_commit_values(val, deref, obj, buf, sz);\n+\t\tgrab_sub_body_contents(val, deref, obj, buf, sz);\n+\t\tgrab_person(\"author\", val, deref, obj, buf, sz);\n+\t\tgrab_person(\"committer\", val, deref, obj, buf, sz);\n+\t\tbreak;\n+\tcase OBJ_TREE:\n+\t\t/* grab_tree_values(val, deref, obj, buf, sz); */\n+\t\tbreak;\n+\tcase OBJ_BLOB:\n+\t\t/* grab_blob_values(val, deref, obj, buf, sz); */\n+\t\tbreak;\n+\tdefault:\n+\t\tdie(\"Eh?  Object of type %d?\", obj->type);\n+\t}\n+}\n+\n+static inline char *copy_advance(char *dst, const char *src)\n+{\n+\twhile (*src)\n+\t\t*dst++ = *src++;\n+\treturn dst;\n+}\n+\n+/*\n+ * Parse the object referred by ref, and grab needed value.\n+ */\n+static void populate_value(struct ref_filter_item *ref)\n+{\n+\tvoid *buf;\n+\tstruct object *obj;\n+\tint eaten, i;\n+\tunsigned long size;\n+\tconst unsigned char *tagged;\n+\n+\tref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));\n+\n+\tif (need_symref && (ref->flags & REF_ISSYMREF) && !ref->symref) {\n+\t\tunsigned char unused1[20];\n+\t\tref->symref = resolve_refdup(ref->name, RESOLVE_REF_READING,\n+\t\t\t\t\t     unused1, NULL);\n+\t\tif (!ref->symref)\n+\t\t\tref->symref = \"\";\n+\t}\n+\n+\t/* Fill in specials first */\n+\tfor (i = 0; i < used_atom_cnt; i++) {\n+\t\tconst char *name = used_atom[i];\n+\t\tstruct atom_value *v = &ref->value[i];\n+\t\tint deref = 0;\n+\t\tconst char *refname;\n+\t\tconst char *formatp;\n+\t\tstruct branch *branch = NULL;\n+\n+\t\tif (*name == '*') {\n+\t\t\tderef = 1;\n+\t\t\tname++;\n+\t\t}\n+\n+\t\tif (starts_with(name, \"refname\"))\n+\t\t\trefname = ref->name;\n+\t\telse if (starts_with(name, \"symref\"))\n+\t\t\trefname = ref->symref ? ref->symref : \"\";\n+\t\telse if (starts_with(name, \"upstream\")) {\n+\t\t\t/* only local branches may have an upstream */\n+\t\t\tif (!starts_with(ref->name, \"refs/heads/\"))\n+\t\t\t\tcontinue;\n+\t\t\tbranch = branch_get(ref->name + 11);\n+\n+\t\t\tif (!branch || !branch->merge || !branch->merge[0] ||\n+\t\t\t    !branch->merge[0]->dst)\n+\t\t\t\tcontinue;\n+\t\t\trefname = branch->merge[0]->dst;\n+\t\t} else if (starts_with(name, \"color:\")) {\n+\t\t\tchar color[COLOR_MAXLEN] = \"\";\n+\n+\t\t\tif (color_parse(name + 6, color) < 0)\n+\t\t\t\tdie(_(\"unable to parse format\"));\n+\t\t\tv->s = xstrdup(color);\n+\t\t\tcontinue;\n+\t\t} else if (!strcmp(name, \"flag\")) {\n+\t\t\tchar buf[256], *cp = buf;\n+\t\t\tif (ref->flags & REF_ISSYMREF)\n+\t\t\t\tcp = copy_advance(cp, \",symref\");\n+\t\t\tif (ref->flags & REF_ISPACKED)\n+\t\t\t\tcp = copy_advance(cp, \",packed\");\n+\t\t\tif (cp == buf)\n+\t\t\t\tv->s = \"\";\n+\t\t\telse {\n+\t\t\t\t*cp = '\\0';\n+\t\t\t\tv->s = xstrdup(buf + 1);\n+\t\t\t}\n+\t\t\tcontinue;\n+\t\t} else if (!deref && grab_objectname(name, ref->sha1, v)) {\n+\t\t\tcontinue;\n+\t\t} else if (!strcmp(name, \"HEAD\")) {\n+\t\t\tconst char *head;\n+\t\t\tunsigned char sha1[20];\n+\n+\t\t\thead = resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n+\t\t\t\t\t\t  sha1, NULL);\n+\t\t\tif (!strcmp(ref->name, head))\n+\t\t\t\tv->s = \"*\";\n+\t\t\telse\n+\t\t\t\tv->s = \" \";\n+\t\t\tcontinue;\n+\t\t} else\n+\t\t\tcontinue;\n+\n+\t\tformatp = strchr(name, ':');\n+\t\tif (formatp) {\n+\t\t\tint num_ours, num_theirs;\n+\n+\t\t\tformatp++;\n+\t\t\tif (!strcmp(formatp, \"short\"))\n+\t\t\t\trefname = shorten_unambiguous_ref(refname,\n+\t\t\t\t\t\t      warn_ambiguous_refs);\n+\t\t\telse if (!strcmp(formatp, \"track\") &&\n+\t\t\t\t starts_with(name, \"upstream\")) {\n+\t\t\t\tchar buf[40];\n+\n+\t\t\t\tif (stat_tracking_info(branch, &num_ours,\n+\t\t\t\t\t\t       &num_theirs) != 1)\n+\t\t\t\t\tcontinue;\n+\n+\t\t\t\tif (!num_ours && !num_theirs)\n+\t\t\t\t\tv->s = \"\";\n+\t\t\t\telse if (!num_ours) {\n+\t\t\t\t\tsprintf(buf, \"[behind %d]\", num_theirs);\n+\t\t\t\t\tv->s = xstrdup(buf);\n+\t\t\t\t} else if (!num_theirs) {\n+\t\t\t\t\tsprintf(buf, \"[ahead %d]\", num_ours);\n+\t\t\t\t\tv->s = xstrdup(buf);\n+\t\t\t\t} else {\n+\t\t\t\t\tsprintf(buf, \"[ahead %d, behind %d]\",\n+\t\t\t\t\t\tnum_ours, num_theirs);\n+\t\t\t\t\tv->s = xstrdup(buf);\n+\t\t\t\t}\n+\t\t\t\tcontinue;\n+\t\t\t} else if (!strcmp(formatp, \"trackshort\") &&\n+\t\t\t\t   starts_with(name, \"upstream\")) {\n+\t\t\t\tassert(branch);\n+\n+\t\t\t\tif (stat_tracking_info(branch, &num_ours,\n+\t\t\t\t\t\t\t&num_theirs) != 1)\n+\t\t\t\t\tcontinue;\n+\n+\t\t\t\tif (!num_ours && !num_theirs)\n+\t\t\t\t\tv->s = \"=\";\n+\t\t\t\telse if (!num_ours)\n+\t\t\t\t\tv->s = \"<\";\n+\t\t\t\telse if (!num_theirs)\n+\t\t\t\t\tv->s = \">\";\n+\t\t\t\telse\n+\t\t\t\t\tv->s = \"<>\";\n+\t\t\t\tcontinue;\n+\t\t\t} else\n+\t\t\t\tdie(\"unknown %.*s format %s\",\n+\t\t\t\t    (int)(formatp - name), name, formatp);\n+\t\t}\n+\n+\t\tif (!deref)\n+\t\t\tv->s = refname;\n+\t\telse {\n+\t\t\tint len = strlen(refname);\n+\t\t\tchar *s = xmalloc(len + 4);\n+\t\t\tsprintf(s, \"%s^{}\", refname);\n+\t\t\tv->s = s;\n+\t\t}\n+\t}\n+\n+\tfor (i = 0; i < used_atom_cnt; i++) {\n+\t\tstruct atom_value *v = &ref->value[i];\n+\t\tif (v->s == NULL)\n+\t\t\tgoto need_obj;\n+\t}\n+\treturn;\n+\n+ need_obj:\n+\tbuf = get_obj(ref->sha1, &obj, &size, &eaten);\n+\tif (!buf)\n+\t\tdie(\"missing object %s for %s\",\n+\t\t    sha1_to_hex(ref->sha1), ref->name);\n+\tif (!obj)\n+\t\tdie(\"parse_object_buffer failed on %s for %s\",\n+\t\t    sha1_to_hex(ref->sha1), ref->name);\n+\n+\tgrab_values(ref->value, 0, obj, buf, size);\n+\tif (!eaten)\n+\t\tfree(buf);\n+\n+\t/*\n+\t * If there is no atom that wants to know about tagged\n+\t * object, we are done.\n+\t */\n+\tif (!need_tagged || (obj->type != OBJ_TAG))\n+\t\treturn;\n+\n+\t/*\n+\t * If it is a tag object, see if we use a value that derefs\n+\t * the object, and if we do grab the object it refers to.\n+\t */\n+\ttagged = ((struct tag *)obj)->tagged->sha1;\n+\n+\t/*\n+\t * NEEDSWORK: This derefs tag only once, which\n+\t * is good to deal with chains of trust, but\n+\t * is not consistent with what deref_tag() does\n+\t * which peels the onion to the core.\n+\t */\n+\tbuf = get_obj(tagged, &obj, &size, &eaten);\n+\tif (!buf)\n+\t\tdie(\"missing object %s for %s\",\n+\t\t    sha1_to_hex(tagged), ref->name);\n+\tif (!obj)\n+\t\tdie(\"parse_object_buffer failed on %s for %s\",\n+\t\t    sha1_to_hex(tagged), ref->name);\n+\tgrab_values(ref->value, 1, obj, buf, size);\n+\tif (!eaten)\n+\t\tfree(buf);\n+}\n+\n+/*\n+ * Given a ref, return the value for the atom.  This lazily gets value\n+ * out of the object by calling populate value.\n+ */\n+static void get_value(struct ref_filter_item *ref, int atom, struct atom_value **v)\n+{\n+\tif (!ref->value) {\n+\t\tpopulate_value(ref);\n+\t\tfill_missing_values(ref->value);\n+\t}\n+\t*v = &ref->value[atom];\n+}\n+\n+static int cmp_ref_sort(struct ref_sort *s, struct ref_filter_item *a, struct ref_filter_item *b)\n+{\n+\tstruct atom_value *va, *vb;\n+\tint cmp;\n+\tcmp_type cmp_type = used_atom_type[s->atom];\n+\n+\tget_value(a, s->atom, &va);\n+\tget_value(b, s->atom, &vb);\n+\tswitch (cmp_type) {\n+\tcase FIELD_STR:\n+\t\tcmp = strcmp(va->s, vb->s);\n+\t\tbreak;\n+\tdefault:\n+\t\tif (va->ul < vb->ul)\n+\t\t\tcmp = -1;\n+\t\telse if (va->ul == vb->ul)\n+\t\t\tcmp = 0;\n+\t\telse\n+\t\t\tcmp = 1;\n+\t\tbreak;\n+\t}\n+\treturn (s->reverse) ? -cmp : cmp;\n+}\n+\n+static struct ref_sort *ref_sort;\n+static int compare_refs(const void *a_, const void *b_)\n+{\n+\tstruct ref_filter_item *a = *((struct ref_filter_item **)a_);\n+\tstruct ref_filter_item *b = *((struct ref_filter_item **)b_);\n+\tstruct ref_sort *s;\n+\n+\tfor (s = ref_sort; s; s = s->next) {\n+\t\tint cmp = cmp_ref_sort(s, a, b);\n+\t\tif (cmp)\n+\t\t\treturn cmp;\n+\t}\n+\treturn 0;\n+}\n+\n+void sort_refs(struct ref_sort *sort, struct ref_filter *refs)\n+{\n+\tref_sort = sort;\n+\tqsort(refs->items, refs->count, sizeof(struct ref_filter_item *), compare_refs);\n+}\n+\n+static void print_value(struct atom_value *v, int quote_style)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tswitch (quote_style) {\n+\tcase QUOTE_NONE:\n+\t\tfputs(v->s, stdout);\n+\t\tbreak;\n+\tcase QUOTE_SHELL:\n+\t\tsq_quote_buf(&sb, v->s);\n+\t\tbreak;\n+\tcase QUOTE_PERL:\n+\t\tperl_quote_buf(&sb, v->s);\n+\t\tbreak;\n+\tcase QUOTE_PYTHON:\n+\t\tpython_quote_buf(&sb, v->s);\n+\t\tbreak;\n+\tcase QUOTE_TCL:\n+\t\ttcl_quote_buf(&sb, v->s);\n+\t\tbreak;\n+\t}\n+\tif (quote_style != QUOTE_NONE) {\n+\t\tfputs(sb.buf, stdout);\n+\t\tstrbuf_release(&sb);\n+\t}\n+}\n+\n+static int hex1(char ch)\n+{\n+\tif ('0' <= ch && ch <= '9')\n+\t\treturn ch - '0';\n+\telse if ('a' <= ch && ch <= 'f')\n+\t\treturn ch - 'a' + 10;\n+\telse if ('A' <= ch && ch <= 'F')\n+\t\treturn ch - 'A' + 10;\n+\treturn -1;\n+}\n+static int hex2(const char *cp)\n+{\n+\tif (cp[0] && cp[1])\n+\t\treturn (hex1(cp[0]) << 4) | hex1(cp[1]);\n+\telse\n+\t\treturn -1;\n+}\n+\n+static void emit(const char *cp, const char *ep)\n+{\n+\twhile (*cp && (!ep || cp < ep)) {\n+\t\tif (*cp == '%') {\n+\t\t\tif (cp[1] == '%')\n+\t\t\t\tcp++;\n+\t\t\telse {\n+\t\t\t\tint ch = hex2(cp + 1);\n+\t\t\t\tif (0 <= ch) {\n+\t\t\t\t\tputchar(ch);\n+\t\t\t\t\tcp += 3;\n+\t\t\t\t\tcontinue;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t\tputchar(*cp);\n+\t\tcp++;\n+\t}\n+}\n+\n+void show_ref(struct ref_filter_item *info, const char *format, int quote_style)\n+{\n+\tconst char *cp, *sp, *ep;\n+\textern int need_color_reset_at_eol;\n+\n+\tfor (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {\n+\t\tstruct atom_value *atomv;\n+\n+\t\tep = strchr(sp, ')');\n+\t\tif (cp < sp)\n+\t\t\temit(cp, sp);\n+\t\tget_value(info, parse_atom(sp + 2, ep), &atomv);\n+\t\tprint_value(atomv, quote_style);\n+\t}\n+\tif (*cp) {\n+\t\tsp = cp + strlen(cp);\n+\t\temit(cp, sp);\n+\t}\n+\tif (need_color_reset_at_eol) {\n+\t\tstruct atom_value resetv;\n+\t\tchar color[COLOR_MAXLEN] = \"\";\n+\n+\t\tif (color_parse(\"reset\", color) < 0)\n+\t\t\tdie(\"BUG: couldn't parse 'reset' as a color\");\n+\t\tresetv.s = color;\n+\t\tprint_value(&resetv, quote_style);\n+\t}\n+\tputchar('\\n');\n+}\n+\n+struct ref_sort *ref_default_sort(void)\n+{\n+\tstatic const char cstr_name[] = \"refname\";\n+\n+\tstruct ref_sort *sort = xcalloc(1, sizeof(*sort));\n+\n+\tsort->next = NULL;\n+\tsort->atom = parse_atom(cstr_name, cstr_name + strlen(cstr_name));\n+\treturn sort;\n+}\n+\n+int ref_opt_parse_sort(const struct option *opt, const char *arg, int unset)\n+{\n+\tstruct ref_sort **sort_tail = opt->value;\n+\tstruct ref_sort *s;\n+\tint len;\n+\n+\tif (!arg) /* should --no-sort void the list ? */\n+\t\treturn -1;\n+\n+\ts = xcalloc(1, sizeof(*s));\n+\ts->next = *sort_tail;\n+\t*sort_tail = s;\n+\n+\tif (*arg == '-') {\n+\t\ts->reverse = 1;\n+\t\targ++;\n+\t}\n+\tlen = strlen(arg);\n+\ts->atom = parse_atom(arg, arg+len);\n+\treturn 0;\n+}\n \n static int match_name_as_path(const char **pattern, const char *refname)\n {\ndiff --git a/ref-filter.h b/ref-filter.h\nindex 3010d13..f16978a 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -4,6 +4,7 @@\n #include \"sha1-array.h\"\n #include \"refs.h\"\n #include \"commit.h\"\n+#include \"parse-options.h\"\n \n /*\n  * ref-filter is meant to act as a common provider of API's for\n@@ -12,6 +13,13 @@\n  * the functionality of each other.\n  */\n \n+/* Quoting styles */\n+#define QUOTE_NONE 0\n+#define QUOTE_SHELL 1\n+#define QUOTE_PERL 2\n+#define QUOTE_PYTHON 4\n+#define QUOTE_TCL 8\n+\n /* An atom is a valid field atom used for sorting and formatting of refs.*/\n struct atom_value {\n \tconst char *s;\n@@ -44,4 +52,12 @@ struct ref_filter {\n int ref_filter_add(const char *refname, const unsigned char *sha1, int flags, void *data);\n void ref_filter_clear(struct ref_filter *refs);\n \n+/*  formatting and sorting functions */\n+int parse_atom(const char *atom, const char *ep);\n+int verify_format(const char *format);\n+void sort_refs(struct ref_sort *sort, struct ref_filter *refs);\n+void show_ref(struct ref_filter_item *info, const char *format, int quote_style);\n+int ref_opt_parse_sort(const struct option *opt, const char *arg, int unset);\n+struct ref_sort *ref_default_sort(void);\n+\n #endif /*  REF_FILTER_H  */\n-- \n2.4.1\n"},{"id":"261665","messageId":"vpqtwv7qj92.fsf@anie.imag.fr","threadId":"39382","inReplyTo":"1432127904-21070-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH 1/4] for-each-ref: rename refinfo members to match similar structures","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-05-20T16:57:45Z","receivedAt":"2015-05-20T16:57:45Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\n> From: Jeff King <peff@peff.net>\n\nThis means that \"git am\" will consider Peff as the author ...\n\n> Written-by: Jeff King <peff@peff.net>\n\n... hence this is not needed: in the final history, it will appear as if\nPeff wrote this Written-by: himself, which would be weird.\n\nIf it is the case, you should add in the commit message that there's no\nactual changs, and perhaps which renames were done. This makes the\nreview straightforward.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"261686","messageId":"CAPig+cTiYk_oGj98QttMMDjyJDFV256yOwu9b8_znOMnaMt7-A@mail.gmail.com","threadId":"39382","inReplyTo":"1432127904-21070-2-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH 2/4] ref-filter: add ref-filter API","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-20T19:07:32Z","receivedAt":"2015-05-20T19:07:32Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, May 20, 2015 at 9:18 AM, Karthik Nayak <karthik.188@gmail.com> wrote:\n> add a ref-filter API to provide functions to filter refs for listing.\n> This will act as a common library for commands like 'tag -l',\n> 'branch -l' and 'for-each-ref'. ref-filter will enable each of these\n> commands to benefit from the features of the others.\n>\n> Mentored-by: Christian Couder <christian.couder@gmail.com>\n> Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>\n> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n> ---\n>  Makefile     |  1 +\n>  ref-filter.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>  ref-filter.h | 47 ++++++++++++++++++++++++++++++++++++++\n>  3 files changed, 121 insertions(+)\n>  create mode 100644 ref-filter.c\n>  create mode 100644 ref-filter.h\n\nA shortcoming of this approach is that it's not blame-friendly.\nAlthough those of us following this patch series know that much of the\ncode in this patch was copied from for-each-ref.c, git-blame will not\nrecognize this unless invoked in the very expensive \"git blame -C -C\n-C\" fashion (if I understand correctly). The most blame-friendly way\nto perform this re-organization is to have the code relocation (line\nremovals and line additions) occur in one patch.\n\nThere are multiple ways you could arrange to do so. One would be to\nfirst have a patch which introduces just a skeleton of the intended\nAPI, with do-nothing function implementations. A subsequent patch\nwould then relocate the code from for-each-ref.c to ref-filter.c, and\nupdate for-each-ref.c to call into the new (now fleshed-out) API.\n"},{"id":"261728","messageId":"xmqqvbfmlsfu.fsf@gitster.dls.corp.google.com","threadId":"39382","inReplyTo":"1432127904-21070-3-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH 3/4] for-each-ref: convert to ref-filter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-20T23:50:29Z","receivedAt":"2015-05-20T23:50:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\n> convert 'for-each-ref' to use the common API provided by 'ref-filter'.\n\nStart a sentence with capital?\n\nMore importantly, the above is misleading, as if you invented a new\nref-filter API and made for-each-ref build on that new\ninfrastructure.\n\nThis series is in a form that is very unfriendly to reviewers.  The\nprevious step did not introduce any callers to ref-filter, so for\nthe purpose of review, it needs to be read together with this step\nanyway.\n\nAnd when reading these patches that way, what this half is really\ndoing is to move the code from for-each-ref to ref-filter, but it\ndoes unnecessary or unrelated renaming of a handful of symbols.  It\nmakes it even harder to compare and contrast the original code that\nwas in the original for-each-ref and moved code that ends up in the\nnew ref-filter.  Don't do that.\n\nYou would probably want to organize them in these two steps instead:\n\n * Rename symbols as necessary while all the code is still in\n   for-each-ref. Do not create ref-filter in this step. Justify it\n   along the lines of \"some symbol names were fine while they were\n   file scope static implementation detail of for-each-ref, but we\n   will make the machinery available from other commands by moving\n   it to a library-ish place, so rename X to foo_X to clarify that\n   this is about foo (which is now necessary as it is not specific\n   to for-each-ref\"...\n\n * If you want to do other tweaks like wrapping refs & num_refs into\n   a single structure, do so while the code is still in\n   for-each-ref.  You can do that in the same patch as the above\n   (i.e. it's just part of preparatory step for a move).\n\n * Create ref-filter by _moving_ code from for-each-ref. Do not\n   touch these moved lines in this step. You would need to add\n   include at the top of for-each-ref and ref-filter, of course.\n\n\n> -\tfor_each_rawref(grab_single_ref, &cbdata);\n> -\trefs = cbdata.grab_array;\n> -\tnum_refs = cbdata.grab_cnt;\n> +\trefs.name_patterns = argv;\n> +\tfor_each_rawref(ref_filter_add, &refs);\n\nI think ref_filter_add() may be misnamed as a public API function.\ngrab_single_ref() was OK only because it was an implementation\ndtail, but if you are making it public, the name should make it\nclear that it is meant to be used as a for_each_*ref() callback\nfunction.  Otherwise people may be tempted to add random parameter\nto it in the future, but the signature of that function is dictated\nby for_each_*ref() API.\n"},{"id":"261754","messageId":"555D7AEE.7080201@gmail.com","threadId":"39382","inReplyTo":"vpqtwv7qj92.fsf@anie.imag.fr","subject":"Re: [PATCH 1/4] for-each-ref: rename refinfo members to match similar structures","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-05-21T06:27:58Z","receivedAt":"2015-05-21T06:27:58Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\n\nOn 05/20/2015 10:27 PM, Matthieu Moy wrote:\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> From: Jeff King <peff@peff.net>\n>\n> This means that \"git am\" will consider Peff as the author ...\n>\n>> Written-by: Jeff King <peff@peff.net>\n>\n> ... hence this is not needed: in the final history, it will appear as if\n> Peff wrote this Written-by: himself, which would be weird.\n>\n> If it is the case, you should add in the commit message that there's no\n> actual changs, and perhaps which renames were done. This makes the\n> review straightforward.\n>\n\nNoted, will change it.\n"},{"id":"261755","messageId":"555D8080.1020504@gmail.com","threadId":"39382","inReplyTo":"xmqqvbfmlsfu.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 3/4] for-each-ref: convert to ref-filter","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-05-21T06:51:44Z","receivedAt":"2015-05-21T06:51:44Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\n\nOn 05/21/2015 05:20 AM, Junio C Hamano wrote:\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> convert 'for-each-ref' to use the common API provided by 'ref-filter'.\n>\n> Start a sentence with capital?\n>\n> More importantly, the above is misleading, as if you invented a new\n> ref-filter API and made for-each-ref build on that new\n> infrastructure.\n>\n> This series is in a form that is very unfriendly to reviewers.  The\n> previous step did not introduce any callers to ref-filter, so for\n> the purpose of review, it needs to be read together with this step\n> anyway.\n>\n> And when reading these patches that way, what this half is really\n> doing is to move the code from for-each-ref to ref-filter, but it\n> does unnecessary or unrelated renaming of a handful of symbols.  It\n> makes it even harder to compare and contrast the original code that\n> was in the original for-each-ref and moved code that ends up in the\n> new ref-filter.  Don't do that.\n>\n> You would probably want to organize them in these two steps instead:\n>\n>   * Rename symbols as necessary while all the code is still in\n>     for-each-ref. Do not create ref-filter in this step. Justify it\n>     along the lines of \"some symbol names were fine while they were\n>     file scope static implementation detail of for-each-ref, but we\n>     will make the machinery available from other commands by moving\n>     it to a library-ish place, so rename X to foo_X to clarify that\n>     this is about foo (which is now necessary as it is not specific\n>     to for-each-ref\"...\n>\n>   * If you want to do other tweaks like wrapping refs & num_refs into\n>     a single structure, do so while the code is still in\n>     for-each-ref.  You can do that in the same patch as the above\n>     (i.e. it's just part of preparatory step for a move).\n>\n>   * Create ref-filter by _moving_ code from for-each-ref. Do not\n>     touch these moved lines in this step. You would need to add\n>     include at the top of for-each-ref and ref-filter, of course.\n>\nThanks for the suggestion's Junio, will follow with the path you've \nmentioned.\n>\n>> -\tfor_each_rawref(grab_single_ref, &cbdata);\n>> -\trefs = cbdata.grab_array;\n>> -\tnum_refs = cbdata.grab_cnt;\n>> +\trefs.name_patterns = argv;\n>> +\tfor_each_rawref(ref_filter_add, &refs);\n>\n> I think ref_filter_add() may be misnamed as a public API function.\n> grab_single_ref() was OK only because it was an implementation\n> dtail, but if you are making it public, the name should make it\n> clear that it is meant to be used as a for_each_*ref() callback\n> function.  Otherwise people may be tempted to add random parameter\n> to it in the future, but the signature of that function is dictated\n> by for_each_*ref() API.\n>\n\nLooking through the current each_ref_f() names, I thought I could use\n'ref_filter_handler()' instead of 'ref_filter_add()', as per your \nsuggestion. What do you think?\n"},{"id":"261765","messageId":"vpqr3qagvv6.fsf@anie.imag.fr","threadId":"39382","inReplyTo":"1432127904-21070-2-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH 2/4] ref-filter: add ref-filter API","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-05-21T08:47:41Z","receivedAt":"2015-05-21T08:47:41Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\n> +static int match_name_as_path(const char **pattern, const char *refname)\n\nI would have appreciated a short docstring. The full doc would probably\nbe as long as the code, but a few examples of what matches and what\ndoesn't can help the reader.\n\n> +static struct ref_filter_item *new_ref_filter_item(const char *refname,\n> +\t\t\t\t\t\t   const unsigned char *sha1,\n> +\t\t\t\t\t\t   int flag)\n> +{\n> +\tstruct ref_filter_item *ref =  xcalloc(1, sizeof(struct ref_filter_item));\n\ndouble-space after =.\n\n> +++ b/ref-filter.h\n> @@ -0,0 +1,47 @@\n> +#ifndef REF_FILTER_H\n> +#define REF_FILTER_H\n> +\n> +#include \"sha1-array.h\"\n> +#include \"refs.h\"\n> +#include \"commit.h\"\n> +\n> +/*\n> + * ref-filter is meant to act as a common provider of API's for\n> + * 'tag -l', 'branch -l' and 'for-each-ref'. ref-filter is the attempt\n\nDon't be shy: attempt at unification -> unification. This message may be\nan attempt, but we'll polish it until it is more than that.\n\n> + * at unification of these three commands so that they ay benefit from\n\nthey *may*?\n\n> + * the functionality of each other.\n> + */\n\nI miss a high-level description of what the code is doing. Essentially,\nthere's the complete repository list of refs, and you want to filter\nonly some of them, right?\n\n>From the name, I would guess that ref_filter is the structure describing\nhow you are filtering, but from the code it seems to be the list you're\nfiltering, not the filter.\n\n> +/* An atom is a valid field atom used for sorting and formatting of refs.*/\n\n\"used for\" is very vague. Be more precise, say how it will be involved\nin sorting & formatting.\n\n> +/*  ref_filter will hold data pertaining to a list of refs. */\n\nThis is the answer to the \"what?\" question, which is not very hard to\ninfer from the code. That's not anwsering \"what for?\" or \"why?\", which\nare much harder to infer for the reader.\n\n(plus you have a double-space after /*)\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"261809","messageId":"555E1473.1010703@gmail.com","threadId":"39382","inReplyTo":"vpqr3qagvv6.fsf@anie.imag.fr","subject":"Re: [PATCH 2/4] ref-filter: add ref-filter API","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-05-21T17:22:59Z","receivedAt":"2015-05-21T17:22:59Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\n\nOn 05/21/2015 02:17 PM, Matthieu Moy wrote:\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> +static int match_name_as_path(const char **pattern, const char *refname)\n>\n> I would have appreciated a short docstring. The full doc would probably\n> be as long as the code, but a few examples of what matches and what\n> doesn't can help the reader.\n>\n\nWill patch with an explanation and some examples.\n\n>> +static struct ref_filter_item *new_ref_filter_item(const char *refname,\n>> +\t\t\t\t\t\t   const unsigned char *sha1,\n>> +\t\t\t\t\t\t   int flag)\n>> +{\n>> +\tstruct ref_filter_item *ref =  xcalloc(1, sizeof(struct ref_filter_item));\n>\n> double-space after =.\n\nNoted.\n\n>\n>> +++ b/ref-filter.h\n>> @@ -0,0 +1,47 @@\n>> +#ifndef REF_FILTER_H\n>> +#define REF_FILTER_H\n>> +\n>> +#include \"sha1-array.h\"\n>> +#include \"refs.h\"\n>> +#include \"commit.h\"\n>> +\n>> +/*\n>> + * ref-filter is meant to act as a common provider of API's for\n>> + * 'tag -l', 'branch -l' and 'for-each-ref'. ref-filter is the attempt\n>\n> Don't be shy: attempt at unification -> unification. This message may be\n> an attempt, but we'll polish it until it is more than that.\n\nHaha, OK, will change that.\n\n>\n>> + * at unification of these three commands so that they ay benefit from\n>\n> they *may*?\n\nYes. Will change\n\n>\n>> + * the functionality of each other.\n>> + */\n>\n> I miss a high-level description of what the code is doing. Essentially,\n> there's the complete repository list of refs, and you want to filter\n> only some of them, right?\n>\n>  From the name, I would guess that ref_filter is the structure describing\n> how you are filtering, but from the code it seems to be the list you're\n> filtering, not the filter.\n\nWill write a better explanation and description.\n>\n>> +/* An atom is a valid field atom used for sorting and formatting of refs.*/\n>\n> \"used for\" is very vague. Be more precise, say how it will be involved\n> in sorting & formatting.\n\nNoted.\n\n>\n>> +/*  ref_filter will hold data pertaining to a list of refs. */\n>\n> This is the answer to the \"what?\" question, which is not very hard to\n> infer from the code. That's not anwsering \"what for?\" or \"why?\", which\n> are much harder to infer for the reader.\n>\n> (plus you have a double-space after /*)\n>\n\nNoted! Thanks for the suggestions :)\n"},{"id":"261812","messageId":"555E161C.1080504@gmail.com","threadId":"39382","inReplyTo":"CAPig+cTiYk_oGj98QttMMDjyJDFV256yOwu9b8_znOMnaMt7-A@mail.gmail.com","subject":"Re: [PATCH 2/4] ref-filter: add ref-filter API","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-05-21T17:30:04Z","receivedAt":"2015-05-21T17:30:04Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On 05/21/2015 12:37 AM, Eric Sunshine wrote:\n> On Wed, May 20, 2015 at 9:18 AM, Karthik Nayak <karthik.188@gmail.com> wrote:\n>> add a ref-filter API to provide functions to filter refs for listing.\n>> This will act as a common library for commands like 'tag -l',\n>> 'branch -l' and 'for-each-ref'. ref-filter will enable each of these\n>> commands to benefit from the features of the others.\n>>\n>> Mentored-by: Christian Couder <christian.couder@gmail.com>\n>> Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>\n>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n>> ---\n>>   Makefile     |  1 +\n>>   ref-filter.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>>   ref-filter.h | 47 ++++++++++++++++++++++++++++++++++++++\n>>   3 files changed, 121 insertions(+)\n>>   create mode 100644 ref-filter.c\n>>   create mode 100644 ref-filter.h\n>\n> A shortcoming of this approach is that it's not blame-friendly.\n> Although those of us following this patch series know that much of the\n> code in this patch was copied from for-each-ref.c, git-blame will not\n> recognize this unless invoked in the very expensive \"git blame -C -C\n> -C\" fashion (if I understand correctly). The most blame-friendly way\n> to perform this re-organization is to have the code relocation (line\n> removals and line additions) occur in one patch.\n>\n> There are multiple ways you could arrange to do so. One would be to\n> first have a patch which introduces just a skeleton of the intended\n> API, with do-nothing function implementations. A subsequent patch\n> would then relocate the code from for-each-ref.c to ref-filter.c, and\n> update for-each-ref.c to call into the new (now fleshed-out) API.\n>\n\nDid you read Junio's suggestion on how I should re-order this WIP patch \nseries ?\nThat's somewhat on the lines of what you're suggesting. I'll probably be \ngoing ahead with that, not really sure about how blame works entirely so \nwhat do you think about that?\n"},{"id":"261821","messageId":"555E1CE7.8090507@gmail.com","threadId":"39382","inReplyTo":"vpqr3qagvv6.fsf@anie.imag.fr","subject":"Re: [PATCH 2/4] ref-filter: add ref-filter API","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-05-21T17:59:03Z","receivedAt":"2015-05-21T17:59:03Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\n> I miss a high-level description of what the code is doing. Essentially,\n> there's the complete repository list of refs, and you want to filter\n> only some of them, right?\n>\n>  From the name, I would guess that ref_filter is the structure describing\n> how you are filtering, but from the code it seems to be the list you're\n> filtering, not the filter.\n\nReading this again, A bit confused by what you're trying to imply. Could \nyou rephrase please?\n"},{"id":"261833","messageId":"CAPig+cTT+MEUtRJ6=sf5x-q0S2neL=xeNq_gEQAEy4VSRZga7A@mail.gmail.com","threadId":"39382","inReplyTo":"555E161C.1080504@gmail.com","subject":"Re: [PATCH 2/4] ref-filter: add ref-filter API","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-21T18:40:38Z","receivedAt":"2015-05-21T18:40:38Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, May 21, 2015 at 1:30 PM, karthik nayak <karthik.188@gmail.com> wrote:\n> On 05/21/2015 12:37 AM, Eric Sunshine wrote:\n>> On Wed, May 20, 2015 at 9:18 AM, Karthik Nayak <karthik.188@gmail.com>\n>> wrote:\n>>>   Makefile     |  1 +\n>>>   ref-filter.c | 73\n>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>>>   ref-filter.h | 47 ++++++++++++++++++++++++++++++++++++++\n>>>   3 files changed, 121 insertions(+)\n>>>   create mode 100644 ref-filter.c\n>>>   create mode 100644 ref-filter.h\n>>\n>> A shortcoming of this approach is that it's not blame-friendly.\n>> Although those of us following this patch series know that much of the\n>> code in this patch was copied from for-each-ref.c, git-blame will not\n>> recognize this unless invoked in the very expensive \"git blame -C -C\n>> -C\" fashion (if I understand correctly). The most blame-friendly way\n>> to perform this re-organization is to have the code relocation (line\n>> removals and line additions) occur in one patch.\n>>\n>> There are multiple ways you could arrange to do so. One would be to\n>> first have a patch which introduces just a skeleton of the intended\n>> API, with do-nothing function implementations. A subsequent patch\n>> would then relocate the code from for-each-ref.c to ref-filter.c, and\n>> update for-each-ref.c to call into the new (now fleshed-out) API.\n>\n> Did you read Junio's suggestion on how I should re-order this WIP patch\n> series ?\n> That's somewhat on the lines of what you're suggesting. I'll probably be\n> going ahead with that, not really sure about how blame works entirely so\n> what do you think about that?\n\nYes, Junio's response did a much better job of saying what I intended.\nAlso, his response said something I meant to mention but forgot:\nnamely that, to ease the review task, code movement should be pure\nmovement, and not involve other changes.\n\nAnyhow, follow Junio's advice. He knows what he's talking about. ;-)\n"},{"id":"261885","messageId":"vpqbnhd157k.fsf@anie.imag.fr","threadId":"39382","inReplyTo":"555E1CE7.8090507@gmail.com","subject":"Re: [PATCH 2/4] ref-filter: add ref-filter API","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-05-22T06:44:47Z","receivedAt":"2015-05-22T06:44:47Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"karthik nayak <karthik.188@gmail.com> writes:\n\n>> I miss a high-level description of what the code is doing. Essentially,\n>> there's the complete repository list of refs, and you want to filter\n>> only some of them, right?\n>>\n>>  From the name, I would guess that ref_filter is the structure describing\n>> how you are filtering, but from the code it seems to be the list you're\n>> filtering, not the filter.\n>\n> Reading this again, A bit confused by what you're trying to imply.\n> Could you rephrase please?\n\nAt some point, I'd expect something like\n\n  filtered_list_of_refs = filer(full_list_of_refs, description_of_filter);\n\nThat would remove some refs from full_list_of_refs according to\ndescription_of_filter.\n\n(totally invented code, only to show the idea)\n\nIf there's a piece of code looking like this, then you need a data\nstructure to store list of refs (full_list_of_refs and\nfiltered_list_of_refs) and another to describe what you're doing with it\n(description_of_filter).\n\nThe name ref_filter implies to me that it contains the description of\nthe filter, but looking at the code it doesn't seem to be the case.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"261897","messageId":"555F215E.2050607@gmail.com","threadId":"39382","inReplyTo":"CAPig+cTT+MEUtRJ6=sf5x-q0S2neL=xeNq_gEQAEy4VSRZga7A@mail.gmail.com","subject":"Re: [PATCH 2/4] ref-filter: add ref-filter API","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-05-22T12:30:22Z","receivedAt":"2015-05-22T12:30:22Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\n\nOn 05/22/2015 12:10 AM, Eric Sunshine wrote:\n> On Thu, May 21, 2015 at 1:30 PM, karthik nayak <karthik.188@gmail.com> wrote:\n>> On 05/21/2015 12:37 AM, Eric Sunshine wrote:\n>>> On Wed, May 20, 2015 at 9:18 AM, Karthik Nayak <karthik.188@gmail.com>\n>>> wrote:\n>>>>    Makefile     |  1 +\n>>>>    ref-filter.c | 73\n>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>>>>    ref-filter.h | 47 ++++++++++++++++++++++++++++++++++++++\n>>>>    3 files changed, 121 insertions(+)\n>>>>    create mode 100644 ref-filter.c\n>>>>    create mode 100644 ref-filter.h\n>>>\n>>> A shortcoming of this approach is that it's not blame-friendly.\n>>> Although those of us following this patch series know that much of the\n>>> code in this patch was copied from for-each-ref.c, git-blame will not\n>>> recognize this unless invoked in the very expensive \"git blame -C -C\n>>> -C\" fashion (if I understand correctly). The most blame-friendly way\n>>> to perform this re-organization is to have the code relocation (line\n>>> removals and line additions) occur in one patch.\n>>>\n>>> There are multiple ways you could arrange to do so. One would be to\n>>> first have a patch which introduces just a skeleton of the intended\n>>> API, with do-nothing function implementations. A subsequent patch\n>>> would then relocate the code from for-each-ref.c to ref-filter.c, and\n>>> update for-each-ref.c to call into the new (now fleshed-out) API.\n>>\n>> Did you read Junio's suggestion on how I should re-order this WIP patch\n>> series ?\n>> That's somewhat on the lines of what you're suggesting. I'll probably be\n>> going ahead with that, not really sure about how blame works entirely so\n>> what do you think about that?\n>\n> Yes, Junio's response did a much better job of saying what I intended.\n> Also, his response said something I meant to mention but forgot:\n> namely that, to ease the review task, code movement should be pure\n> movement, and not involve other changes.\n>\n> Anyhow, follow Junio's advice. He knows what he's talking about. ;-)\n>\n\nAlright, Thanks for clearing that out.\n\nRegards,\nKarthik\n"},{"id":"261898","messageId":"555F252C.2060601@gmail.com","threadId":"39382","inReplyTo":"vpqbnhd157k.fsf@anie.imag.fr","subject":"Re: [PATCH 2/4] ref-filter: add ref-filter API","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-05-22T12:46:36Z","receivedAt":"2015-05-22T12:46:36Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\n\nOn 05/22/2015 12:14 PM, Matthieu Moy wrote:\n> karthik nayak <karthik.188@gmail.com> writes:\n>\n>>> I miss a high-level description of what the code is doing. Essentially,\n>>> there's the complete repository list of refs, and you want to filter\n>>> only some of them, right?\n>>>\n>>>   From the name, I would guess that ref_filter is the structure describing\n>>> how you are filtering, but from the code it seems to be the list you're\n>>> filtering, not the filter.\n>>\n>> Reading this again, A bit confused by what you're trying to imply.\n>> Could you rephrase please?\n>\n> At some point, I'd expect something like\n>\n>    filtered_list_of_refs = filer(full_list_of_refs, description_of_filter);\n>\n> That would remove some refs from full_list_of_refs according to\n> description_of_filter.\n>\n> (totally invented code, only to show the idea)\n>\n> If there's a piece of code looking like this, then you need a data\n> structure to store list of refs (full_list_of_refs and\n> filtered_list_of_refs) and another to describe what you're doing with it\n> (description_of_filter).\n>\n> The name ref_filter implies to me that it contains the description of\n> the filter, but looking at the code it doesn't seem to be the case.\n>\n\nBut it does just that, doesn't it?\n\nstrict ref_filter {\n\tint count, alloc;\n\tstruct ref_filter_item **items;\n\tconst char **name_patterns;\n};\n\nIf you see it does contain 'name_patterns' according to which it will \nfilter the given refs, but thats just the start, as 'for-each-ref' only \nsupports filtering based on the given pattern, eventually as I merge the \nfunctionality of 'git tag -l' and 'git branch -l' it will contain more \nfilters like, 'contains_commit', 'merged' and so on. Eventually becoming \nmore of a filter description as you put it. I hope that clears out things :)\n\nRegards,\nKarthik\n"},{"id":"261992","messageId":"vpq8ucffj8h.fsf@anie.imag.fr","threadId":"39382","inReplyTo":"555F252C.2060601@gmail.com","subject":"Re: [PATCH 2/4] ref-filter: add ref-filter API","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-05-23T14:42:38Z","receivedAt":"2015-05-23T14:42:38Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"karthik nayak <karthik.188@gmail.com> writes:\n\n>> At some point, I'd expect something like\n>>\n>>    filtered_list_of_refs = filer(full_list_of_refs, description_of_filter);\n>>\n>> That would remove some refs from full_list_of_refs according to\n>> description_of_filter.\n>>\n>> (totally invented code, only to show the idea)\n>>\n>> If there's a piece of code looking like this, then you need a data\n>> structure to store list of refs (full_list_of_refs and\n>> filtered_list_of_refs) and another to describe what you're doing with it\n>> (description_of_filter).\n>>\n>> The name ref_filter implies to me that it contains the description of\n>> the filter, but looking at the code it doesn't seem to be the case.\n>>\n>\n> But it does just that, doesn't it?\n>\n> struct ref_filter {\n> \tint count, alloc;\n> \tstruct ref_filter_item **items;\n> \tconst char **name_patterns;\n> };\n>\n> If you see it does contain 'name_patterns' according to which it will\n> filter the given refs,\n\nBut it also contains struct ref_filter_item **items, which as I\nunderstand it contains a list of refs (with name, sha1 & such).\n\nThat's the part I do not find natural: the same structure contains both\nthe list of refs and the way it should be filtered.\n\nRe-reading the patch, I seem to understand that you're putting both on\nthe same struct because of the API of for_each_ref() which takes one\n'data' pointer to be casted, so you want both the input (filter\ndescription) and the output (list of refs after filtering) to be\ncontained in the same struct.\n\nBut I think this could be clearer in the code (and/or comment + commit\nmessage). Perhaps stg like:\n\nstruct ref_filter_data /* Probably not the best name */ {\n        struct ref_list list;\n        struct ref_filter filter;\n};\n\nstruct ref_list {\n \tint count, alloc;\n \tstruct ref_filter_item **items;\n \tconst char **name_patterns;\n};\n\nstruct ref_filter {\n\tconst char **name_patterns;\n\t/* There will be more here later */\n};\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"261993","messageId":"CAP8UFD0CPLcTGYTo1Oh_QFxVkORBZF2SmwRYu0O8do66qmakPw@mail.gmail.com","threadId":"39382","inReplyTo":"vpq8ucffj8h.fsf@anie.imag.fr","subject":"Re: [PATCH 2/4] ref-filter: add ref-filter API","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-05-23T16:04:27Z","receivedAt":"2015-05-23T16:04:27Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sat, May 23, 2015 at 4:42 PM, Matthieu Moy\n<Matthieu.Moy@grenoble-inp.fr> wrote:\n> karthik nayak <karthik.188@gmail.com> writes:\n>\n>>> At some point, I'd expect something like\n>>>\n>>>    filtered_list_of_refs = filer(full_list_of_refs, description_of_filter);\n>>>\n>>> That would remove some refs from full_list_of_refs according to\n>>> description_of_filter.\n>>>\n>>> (totally invented code, only to show the idea)\n>>>\n>>> If there's a piece of code looking like this, then you need a data\n>>> structure to store list of refs (full_list_of_refs and\n>>> filtered_list_of_refs) and another to describe what you're doing with it\n>>> (description_of_filter).\n>>>\n>>> The name ref_filter implies to me that it contains the description of\n>>> the filter, but looking at the code it doesn't seem to be the case.\n>>>\n>>\n>> But it does just that, doesn't it?\n>>\n>> struct ref_filter {\n>>       int count, alloc;\n>>       struct ref_filter_item **items;\n>>       const char **name_patterns;\n>> };\n>>\n>> If you see it does contain 'name_patterns' according to which it will\n>> filter the given refs,\n>\n> But it also contains struct ref_filter_item **items, which as I\n> understand it contains a list of refs (with name, sha1 & such).\n>\n> That's the part I do not find natural: the same structure contains both\n> the list of refs and the way it should be filtered.\n>\n> Re-reading the patch, I seem to understand that you're putting both on\n> the same struct because of the API of for_each_ref() which takes one\n> 'data' pointer to be casted, so you want both the input (filter\n> description) and the output (list of refs after filtering) to be\n> contained in the same struct.\n>\n> But I think this could be clearer in the code (and/or comment + commit\n> message). Perhaps stg like:\n>\n> struct ref_filter_data /* Probably not the best name */ {\n>         struct ref_list list;\n>         struct ref_filter filter;\n> };\n>\n> struct ref_list {\n>         int count, alloc;\n>         struct ref_filter_item **items;\n>         const char **name_patterns;\n> };\n\nMatthieu, I think you forgot to remove \"const char **name_patterns;\"\nin the above struct, as you put it in the \"ref_filter\" struct below:\n\n> struct ref_filter {\n>         const char **name_patterns;\n>         /* There will be more here later */\n> };\n\nI agree that it might be clearer to separate both. In this case\ninstead of \"ref_list\" the struct might be called \"ref_filter_array\" as\nwe already have \"argv_array\" in argv-array.h and \"sha1_array\" in\n\"sha1-array.h\".\n"},{"id":"261996","messageId":"vpqr3q7cjpd.fsf@anie.imag.fr","threadId":"39382","inReplyTo":"CAP8UFD0CPLcTGYTo1Oh_QFxVkORBZF2SmwRYu0O8do66qmakPw@mail.gmail.com","subject":"Re: [PATCH 2/4] ref-filter: add ref-filter API","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-05-23T17:00:46Z","receivedAt":"2015-05-23T17:00:46Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n>> struct ref_list {\n>>         int count, alloc;\n>>         struct ref_filter_item **items;\n>>         const char **name_patterns;\n>> };\n>\n> Matthieu, I think you forgot to remove \"const char **name_patterns;\"\n> in the above struct, as you put it in the \"ref_filter\" struct below:\n\nYes, indeed. Too quick cut-and-paste.\n\n> I agree that it might be clearer to separate both. In this case\n> instead of \"ref_list\" the struct might be called \"ref_filter_array\" as\n> we already have \"argv_array\" in argv-array.h and \"sha1_array\" in\n> \"sha1-array.h\".\n\nI'd drop the \"filter\" part and make it ref_array then. There's no reason\nwe could not use it it places other than filter.\n\nBut we also have string_list which is an array underneath, so I think\nboth names (_array and _list) are fine.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"261998","messageId":"xmqqoalbciwj.fsf@gitster.dls.corp.google.com","threadId":"39382","inReplyTo":"vpq8ucffj8h.fsf@anie.imag.fr","subject":"Re: [PATCH 2/4] ref-filter: add ref-filter API","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-23T17:18:04Z","receivedAt":"2015-05-23T17:18:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> But I think this could be clearer in the code (and/or comment + commit\n> message). Perhaps stg like:\n>\n> struct ref_filter_data /* Probably not the best name */ {\n>         struct ref_list list;\n>         struct ref_filter filter;\n> };\n>\n> struct ref_list {\n>  \tint count, alloc;\n>  \tstruct ref_filter_item **items;\n> };\n\nIf you plan to use ALLOC_GROW() API, name the bookkeeping variables\n\"alloc\" & \"nr\", unless you have a compelling reason to deviate from\nthe prevailing practice.\n\n> struct ref_filter {\n> \tconst char **name_patterns;\n> \t/* There will be more here later */\n> };\n\nVery good suggestion.\n\nWhatever the final name would be, it is a good idea to separate the\n\"list of things that are operated on\" and the \"set of operations to\nbe applied\".  That makes things conceptually cleaner; you can have\nmultiple of the former operated on with a singleton of the latter\nand then their results merged, etc. etc.\n\nAnd I do not think an array of things that are operated on should\nnot be named \"ref_filter_item\".\n\nSurely, the latter \"set of operations to be applied\" may currently\nbe only filtering, but who says it has to stay that way?  \"I have a\nset of refs that represent my local branches I am interested\nin. Please map them to their corresponding @{upstream}\" is a\nreasonable request once you have an infrastructure to represent \"set\nof refs to be worked on\" and \"set of operations to apply\", and at\nthat point, the items are no longer filter-items (map-items?).\n"},{"id":"262001","messageId":"5560BE73.8020801@gmail.com","threadId":"39382","inReplyTo":"vpq8ucffj8h.fsf@anie.imag.fr","subject":"Re: [PATCH 2/4] ref-filter: add ref-filter API","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-05-23T17:52:51Z","receivedAt":"2015-05-23T17:52:51Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":">\n> But it also contains struct ref_filter_item **items, which as I\n> understand it contains a list of refs (with name, sha1 & such).\n>\n> That's the part I do not find natural: the same structure contains both\n> the list of refs and the way it should be filtered.\n>\n> Re-reading the patch, I seem to understand that you're putting both on\n> the same struct because of the API of for_each_ref() which takes one\n> 'data' pointer to be casted, so you want both the input (filter\n> description) and the output (list of refs after filtering) to be\n> contained in the same struct.\n\nWas kinda confused, This clears out things, Thanks.\n\n>\n> But I think this could be clearer in the code (and/or comment + commit\n> message). Perhaps stg like:\n>\n> struct ref_filter_data /* Probably not the best name */ {\n>          struct ref_list list;\n>          struct ref_filter filter;\n> };\n>\n> struct ref_list {\n>   \tint count, alloc;\n>   \tstruct ref_filter_item **items;\n>   \tconst char **name_patterns;\n> };\n>\n> struct ref_filter {\n> \tconst char **name_patterns;\n> \t/* There will be more here later */\n> };\n>\n\nThis seems cleaner, agreed.\n\n >\n > I agree that it might be clearer to separate both. In this case\n > instead of \"ref_list\" the struct might be called \"ref_filter_array\" as\n > we already have \"argv_array\" in argv-array.h and \"sha1_array\" in\n > \"sha1-array.h\".\n >\n\nSomehow ref_list seems more real to me, list of refs.\n\n >\n > And I do not think an array of things that are operated on should\n > not be named \"ref_filter_item\".\n >\n > Surely, the latter \"set of operations to be applied\" may currently\n > be only filtering, but who says it has to stay that way?  \"I have a\n > set of refs that represent my local branches I am interested\n > in. Please map them to their corresponding @{upstream}\" is a\n > reasonable request once you have an infrastructure to represent \"set\n > of refs to be worked on\" and \"set of operations to apply\", and at\n > that point, the items are no longer filter-items (map-items?).\n >\n\nThat's also a good point to consider, I shall rename and restructure the \ncode as discussed here, thanks.\n\n-- \nRegards,\nKarthik\n"},{"id":"262007","messageId":"vpqwpzyc4aq.fsf@anie.imag.fr","threadId":"39382","inReplyTo":"xmqqoalbciwj.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/4] ref-filter: add ref-filter API","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-05-23T22:33:33Z","receivedAt":"2015-05-23T22:33:33Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> And I do not think an array of things that are operated on should\n> not be named \"ref_filter_item\".\n\nIs the double-negation intended? It seems contradictory with:\n\n> Surely, the latter \"set of operations to be applied\" may currently\n> be only filtering, but who says it has to stay that way?\n\n(With which I do agree)\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}