{"thread":{"id":"39939","subject":"[PATCH v5 00/11] port tag.c to use ref-filter APIs","startedAt":"2015-07-27T07:26:08Z","lastAt":"2015-07-30T11:21:21Z","messageCount":38,"participants":["Karthik Nayak","Matthieu Moy","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":5,"patchTotal":11},"messages":[{"id":"266803","messageId":"CAOLa=ZTtAepdO5U8zo62VBn_z4-LcKoguPxVGaAnNZULEwzrQQ@mail.gmail.com","threadId":"39939","inReplyTo":null,"subject":"[PATCH v5 00/11] port tag.c to use ref-filter APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T07:26:08Z","receivedAt":"2015-07-27T07:26:08Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"This is part of my GSoC project to unify git tag -l, git branch -l,\ngit for-each-ref.  This patch series is continued from: Git (next)\nhttps://github.com/git/git/commit/bf5418f49ff0cebc6e5ce04ad1417e1a47c81b61\n\nVersion 4 can be found here\nhttp://article.gmane.org/gmane.comp.version-control.git/274579\n\nChanges in this version (interdiff at the end):\n* Change the way ref_formatting_state is used and the sequence of its\nusage in the\nseries.\n* Separate the introduction of ref_formatting_state from its usage.\n* Removed the inefficient removal of path in match_pattern().\n* Improve coding style in the implementation of sorting based on version.\n* Changed the tests for sorting based on version, to use\n'for-each-ref' rather than 'tag.c'\nwhich already has a working sort based on version.\n* Other small grammatical changes.\n* Change the atom name from 'align' to 'padright'.\n\n[PATCH v5 01/11] ref-filter: introduce 'ref_formatting_state'\n[PATCH v5 02/11] ref-filter: make `color` use `ref_formatting_state`\n[PATCH v5 03/11] ref-filter: add option to pad atoms to the right\n[PATCH v5 04/11] ref-filter: add option to filter only tags\n[PATCH v5 05/11] ref-filter: support printing N lines from tag\n[PATCH v5 06/11] ref-filter: add support to sort by version\n[PATCH v5 07/11] ref-filter: add option to match literal pattern\n[PATCH v5 08/11] tag.c: use 'ref-filter' data structures\n[PATCH v5 09/11] tag.c: use 'ref-filter' APIs\n[PATCH v5 10/11] tag.c: implement '--format' option\n[PATCH v5 11/11] tag.c: implement '--merged' and '--no-merged'\n\nInterdiff:\n\ndiff --git a/Documentation/git-for-each-ref.txt\nb/Documentation/git-for-each-ref.txt\nindex 224dc8c..2b60aee 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -127,6 +127,12 @@ color::\n     Change output color.  Followed by `:<colorname>`, where names\n     are described in `color.branch.*`.\n\n+padright::\n+    Pad succeeding atom to the right. Followed by `:<value>`,\n+    where `value` states the total length of atom including the\n+    padding. If the `value` is greater than the atom length, then\n+    no padding is performed.\n+\n In addition to the above, for commit and tag objects, the header\n field names (`tree`, `parent`, `object`, `type`, and `tag`) can\n be used to specify the value in the header field.\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 8a7f684..fc01117 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -41,13 +41,11 @@ static int list_tags(struct ref_filter *filter,\nstruct ref_sorting *sorting, con\n         filter->lines = 0;\n\n     if (filter->lines)\n-        format = \"%(align:16)%(refname:short)\";\n+        format = \"%(padright:16)%(refname:short)\";\n     else if (!format)\n         format = \"%(refname:short)\";\n\n     verify_ref_format(format);\n-    if (!sorting)\n-        sorting = ref_default_sorting();\n     filter_refs(&array, filter, FILTER_REFS_TAGS);\n     ref_array_sort(sorting, &array);\n\n@@ -134,7 +132,7 @@ static int parse_sorting_string(const char *arg,\nstruct ref_sorting **sorting_ta\n     if (*arg == '-') {\n         s->reverse = 1;\n         arg++;\n-     }\n+    }\n     if (skip_prefix(arg, \"version:\", &arg) ||\n         skip_prefix(arg, \"v:\", &arg))\n         s->version = 1;\ndiff --git a/ref-filter.c b/ref-filter.c\nindex c686c03..597b189 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -56,7 +56,7 @@ static struct {\n     { \"flag\" },\n     { \"HEAD\" },\n     { \"color\" },\n-    { \"align\" },\n+    { \"padright\" },\n };\n\n /*\n@@ -601,8 +601,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 ref_formatting_state *state,\n-               struct ref_array_item *ref)\n+static void populate_value(struct ref_array_item *ref)\n {\n     void *buf;\n     struct object *obj;\n@@ -664,8 +663,9 @@ static void populate_value(struct\nref_formatting_state *state,\n\n             if (color_parse(name + 6, color) < 0)\n                 die(_(\"unable to parse format\"));\n-            state->color = xstrdup(color);\n+            v->s = xstrdup(color);\n             v->pseudo_atom = 1;\n+            v->color = 1;\n             continue;\n         } else if (!strcmp(name, \"flag\")) {\n             char buf[256], *cp = buf;\n@@ -693,17 +693,17 @@ static void populate_value(struct\nref_formatting_state *state,\n             else\n                 v->s = \" \";\n             continue;\n-        } else if (starts_with(name, \"align:\")) {\n+        } else if (starts_with(name, \"padright:\")) {\n             const char *valp = NULL;\n\n-            skip_prefix(name, \"align:\", &valp);\n+            skip_prefix(name, \"padright:\", &valp);\n             if (!valp[0])\n-                die(_(\"no value given with 'align:'\"));\n-            if (strtoul_ui(valp, 10, &state->pad_to_right))\n-                die(_(\"positive integer expected after ':' in align:%u\\n\"),\n-                    state->pad_to_right);\n+                die(_(\"no value given with 'padright:'\"));\n+            if (strtoul_ui(valp, 10, (unsigned int *)&v->ul))\n+                die(_(\"positive integer expected after ':' in padright:%u\\n\"),\n+                    (unsigned int)v->ul);\n             v->pseudo_atom = 1;\n-            v->s = \"\";\n+            v->pad_to_right = 1;\n             continue;\n         } else\n             continue;\n@@ -827,15 +827,10 @@ static void populate_value(struct\nref_formatting_state *state,\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_ref_atom_value(struct ref_formatting_state *state,\n-                   struct ref_array_item *ref, int atom, struct atom_value **v)\n+static void get_ref_atom_value(struct ref_array_item *ref, int atom,\nstruct atom_value **v)\n {\n-    /*\n-     * If the atom is a pseudo_atom then we re-populate the value\n-     * into the ref_formatting_state stucture.\n-     */\n-    if (!ref->value || ref->value[atom].pseudo_atom) {\n-        populate_value(state, ref);\n+    if (!ref->value) {\n+        populate_value(ref);\n         fill_missing_values(ref->value);\n     }\n     *v = &ref->value[atom];\n@@ -957,17 +952,18 @@ static int commit_contains(struct ref_filter\n*filter, struct commit *commit)\n  */\n static int match_pattern(const char **patterns, const char *refname)\n {\n+    /*\n+     * When no '--format' option is given we need to skip the prefix\n+     * for matching refs of tags and branches.\n+     */\n+    if (skip_prefix(refname, \"refs/tags/\", &refname))\n+        ;\n+    else if (skip_prefix(refname, \"refs/heads/\", &refname))\n+        ;\n+    else if (skip_prefix(refname, \"refs/remotes/\", &refname))\n+        ;\n+\n     for (; *patterns; patterns++) {\n-        /*\n-         * When no '--format' option is given we need to skip the prefix\n-         * for listing tags and branches.\n-         */\n-        if (!starts_with(*pattern, \"refs/tags/\"))\n-            skip_prefix(refname, \"refs/tags/\", &refname);\n-        else if (!starts_with(*pattern, \"refs/heads/\"))\n-            skip_prefix(refname, \"refs/heads/\", &refname);\n-        else if (!starts_with(refname, \"refs/remotes/\"))\n-            skip_prefix(refname, \"refs/remotes/\", &refname);\n         if (!wildmatch(*patterns, refname, 0, NULL))\n             return 1;\n     }\n@@ -978,7 +974,7 @@ static int match_pattern(const char **patterns,\nconst char *refname)\n  * Return 1 if the refname matches one of the patterns, otherwise 0.\n  * A pattern can be path prefix (e.g. a refname \"refs/heads/master\"\n  * matches a pattern \"refs/heads/\") or a wildcard (e.g. the same ref\n- * matches \"refs/heads/m*\", too).\n+ * matches \"refs/heads/m*\",too).\n  */\n static int match_name_as_path(const char **pattern, const char *refname)\n {\n@@ -1210,22 +1206,21 @@ static int cmp_ref_sorting(struct ref_sorting\n*s, struct ref_array_item *a, stru\n     struct atom_value *va, *vb;\n     int cmp;\n     cmp_type cmp_type = used_atom_type[s->atom];\n-    struct ref_formatting_state state;\n\n-    memset(&state, 0, sizeof(state));\n-\n-    get_ref_atom_value(&state, a, s->atom, &va);\n-    get_ref_atom_value(&state, b, s->atom, &vb);\n+    get_ref_atom_value(a, s->atom, &va);\n+    get_ref_atom_value(b, s->atom, &vb);\n     if (s->version)\n         cmp = versioncmp(va->s, vb->s);\n     else if (cmp_type == FIELD_STR)\n         cmp = strcmp(va->s, vb->s);\n-    else if (va->ul < vb->ul)\n-        cmp = -1;\n-    else if (va->ul == vb->ul)\n-        cmp = 0;\n-    else\n-        cmp = 1;\n+    else {\n+        if (va->ul < vb->ul)\n+            cmp = -1;\n+        else if (va->ul == vb->ul)\n+            cmp = 0;\n+        else\n+            cmp = 1;\n+    }\n\n     return (s->reverse) ? -cmp : cmp;\n }\n@@ -1251,20 +1246,20 @@ void ref_array_sort(struct ref_sorting\n*sorting, struct ref_array *array)\n     qsort(array->items, array->nr, sizeof(struct ref_array_item *),\ncompare_refs);\n }\n\n-static void ref_formatting(struct ref_formatting_state *state, struct\natom_value *v,\n-               struct strbuf *value)\n+static void ref_formatting(struct ref_formatting_state *state,\n+               struct atom_value *v, struct strbuf *value)\n {\n     if (state->color) {\n         strbuf_addstr(value, state->color);\n-        free((void *)state->color);\n+        free(state->color);\n         state->color = NULL;\n     }\n     if (state->pad_to_right) {\n         if (!is_utf8(v->s))\n             strbuf_addf(value, \"%-*s\", state->pad_to_right, v->s);\n         else {\n-            int len = strlen(v->s) - utf8_strwidth(v->s);\n-            strbuf_addf(value, \"%-*s\", state->pad_to_right + len, v->s);\n+            int utf8_compensation = strlen(v->s) - utf8_strwidth(v->s);\n+            strbuf_addf(value, \"%-*s\", state->pad_to_right +\nutf8_compensation, v->s);\n         }\n         return;\n     }\n@@ -1276,8 +1271,6 @@ static void print_value(struct\nref_formatting_state *state, struct atom_value *v\n     struct strbuf value = STRBUF_INIT;\n     struct strbuf formatted = STRBUF_INIT;\n\n-    if (v->pseudo_atom)\n-        return;\n     ref_formatting(state, v, &value);\n\n     switch (state->quote_style) {\n@@ -1341,6 +1334,15 @@ static void emit(const char *cp, const char *ep)\n     }\n }\n\n+static void apply_pseudo_state(struct ref_formatting_state *state,\n+                   struct atom_value *v)\n+{\n+    if (v->color)\n+        state->color = (char *)v->s;\n+    if (v->pad_to_right)\n+        state->pad_to_right = v->ul;\n+}\n+\n /*\n  * If 'lines' is greater than 0, print that many lines from the given\n  * object_id 'oid'.\n@@ -1399,9 +1401,11 @@ void show_ref_array_item(struct ref_array_item\n*info, const char *format,\n         ep = strchr(sp, ')');\n         if (cp < sp)\n             emit(cp, sp);\n-        get_ref_atom_value(&state, info,\n-                   parse_ref_filter_atom(sp + 2, ep), &atomv);\n-        print_value(&state, atomv);\n+        get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);\n+        if (atomv->pseudo_atom)\n+            apply_pseudo_state(&state, atomv);\n+        else\n+            print_value(&state, atomv);\n     }\n     if (*cp) {\n         sp = cp + strlen(cp);\n@@ -1413,8 +1417,7 @@ void show_ref_array_item(struct ref_array_item\n*info, const char *format,\n\n         if (color_parse(\"reset\", color) < 0)\n             die(\"BUG: couldn't parse 'reset' as a color\");\n-        resetv.s = \"\";\n-        state.color = xstrdup(color);\n+        resetv.s = color;\n         print_value(&state, &resetv);\n     }\n     if (lines > 0) {\ndiff --git a/ref-filter.h b/ref-filter.h\nindex f1933e0..a27745f 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -20,7 +20,15 @@\n struct atom_value {\n     const char *s;\n     unsigned long ul; /* used for sorting when not FIELD_STR */\n-    unsigned int pseudo_atom : 1; /*  atoms which aren't placeholders\nfor ref attributes */\n+    unsigned int pseudo_atom : 1, /*  atoms which aren't placeholders\nfor ref attributes */\n+        color : 1,\n+        pad_to_right : 1;\n+};\n+\n+struct ref_formatting_state {\n+    int quote_style;\n+    char *color;\n+    unsigned int pad_to_right;\n };\n\n struct ref_sorting {\n@@ -30,12 +38,6 @@ struct ref_sorting {\n         version : 1;\n };\n\n-struct ref_formatting_state {\n-    unsigned int pad_to_right; /*pad atoms to the right*/\n-    int quote_style;\n-    const char *color;\n-};\n-\n struct ref_array_item {\n     unsigned char objectname[20];\n     int flag;\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex 5017032..de872db 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -81,6 +81,22 @@ test_expect_success 'filtering with --contains' '\n     test_cmp expect actual\n '\n\n+test_expect_success 'padding to the right using `padright`' '\n+    cat >expect <<-\\EOF &&\n+    refs/heads/master        |\n+    refs/heads/side          |\n+    refs/odd/spot            |\n+    refs/tags/double-tag     |\n+    refs/tags/four           |\n+    refs/tags/one            |\n+    refs/tags/signed-tag     |\n+    refs/tags/three          |\n+    refs/tags/two            |\n+    EOF\n+    git for-each-ref --format=\"%(padright:25)%(refname)|\" >actual &&\n+    test_cmp expect actual\n+'\n+\n test_expect_success 'setup for version sort' '\n     test_commit foo1.3 &&\n     test_commit foo1.6 &&\n@@ -88,7 +104,7 @@ test_expect_success 'setup for version sort' '\n '\n\n test_expect_success 'version sort' '\n-    git tag -l --sort=version:refname | grep \"foo\" >actual &&\n+    git for-each-ref --sort=version:refname\n--format=\"%(refname:short)\" refs/tags/ | grep \"foo\" >actual &&\n     cat >expect <<-\\EOF &&\n     foo1.3\n     foo1.6\n@@ -98,7 +114,7 @@ test_expect_success 'version sort' '\n '\n\n test_expect_success 'version sort (shortened)' '\n-    git tag -l --sort=v:refname | grep \"foo\" >actual &&\n+    git for-each-ref --sort=v:refname --format=\"%(refname:short)\"\nrefs/tags/ | grep \"foo\" >actual &&\n     cat >expect <<-\\EOF &&\n     foo1.3\n     foo1.6\n@@ -108,7 +124,7 @@ test_expect_success 'version sort (shortened)' '\n '\n\n test_expect_success 'reverse version sort' '\n-    git tag -l --sort=-version:refname | grep \"foo\" >actual &&\n+    git for-each-ref --sort=-version:refname\n--format=\"%(refname:short)\" refs/tags/ | grep \"foo\" >actual &&\n     cat >expect <<-\\EOF &&\n     foo1.10\n     foo1.6\ndiff --git a/v3-0001-ref-filter-add-option-to-align-atoms-to-the-left.patch\nb/v3-0001-ref-filter-add-option-to-align-atoms-to-the-left.patch\ndeleted file mode 100644\nindex 350acae..0000000\n--- a/v3-0001-ref-filter-add-option-to-align-atoms-to-the-left.patch\n+++ /dev/null\n@@ -1,124 +0,0 @@\n-From 3a07ca1f56f74ca54b7f3c30e3dfd9fe2fed1cc3 Mon Sep 17 00:00:00 2001\n-From: Karthik Nayak <karthik.188@gmail.com>\n-Date: Wed, 10 Jun 2015 17:19:55 +0530\n-Subject: [PATCH v3 1/9] ref-filter: add option to align atoms to the left\n-\n-Add a new atom \"align\" and support %(align:X) where X is a number.\n-This will align the preceeding atom value to the left followed by\n-spaces for a total length of X characters. If X is less than the item\n-size, the entire atom value is printed.\n-\n-Helped-by: Duy Nguyen <pclouds@gmail.com>\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- ref-filter.c | 41 +++++++++++++++++++++++++++++++++++++++--\n- ref-filter.h |  1 +\n- 2 files changed, 40 insertions(+), 2 deletions(-)\n-\n-diff --git a/ref-filter.c b/ref-filter.c\n-index 7561727..93f59aa 100644\n---- a/ref-filter.c\n-+++ b/ref-filter.c\n-@@ -10,6 +10,8 @@\n- #include \"quote.h\"\n- #include \"ref-filter.h\"\n- #include \"revision.h\"\n-+#include \"utf8.h\"\n-+#include \"git-compat-util.h\"\n-\n- typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n-\n-@@ -53,6 +55,7 @@ static struct {\n-     { \"flag\" },\n-     { \"HEAD\" },\n-     { \"color\" },\n-+    { \"align\" },\n- };\n-\n- /*\n-@@ -620,7 +623,7 @@ static void populate_value(struct ref_array_item *ref)\n-         const char *name = used_atom[i];\n-         struct atom_value *v = &ref->value[i];\n-         int deref = 0;\n--        const char *refname;\n-+        const char *refname = NULL;\n-         const char *formatp;\n-         struct branch *branch = NULL;\n-\n-@@ -687,6 +690,17 @@ static void populate_value(struct ref_array_item *ref)\n-             else\n-                 v->s = \" \";\n-             continue;\n-+        } else if (starts_with(name, \"align:\")) {\n-+            const char *valp = NULL;\n-+\n-+            skip_prefix(name, \"align:\", &valp);\n-+            if (!valp[0])\n-+                die(_(\"no value given with 'align:'\"));\n-+            strtoul_ui(valp, 10, &ref->align_value);\n-+            if (ref->align_value < 1)\n-+                die(_(\"value should be greater than zero:\nalign:%u\"), ref->align_value);\n-+            v->s = \"\";\n-+            continue;\n-         } else\n-             continue;\n-\n-@@ -1254,17 +1268,40 @@ static void emit(const char *cp, const char *ep)\n-     }\n- }\n-\n-+static void assign_formating(struct ref_array_item *ref, int\nparsed_atom, struct atom_value *v)\n-+{\n-+    if (ref->align_value && !starts_with(used_atom[parsed_atom], \"align\")) {\n-+        unsigned int len = 0;\n-+\n-+        if (*v->s)\n-+            len = utf8_strwidth(v->s);\n-+        if (ref->align_value > len) {\n-+            struct strbuf buf = STRBUF_INIT;\n-+            if (*v->s)\n-+                strbuf_addstr(&buf, v->s);\n-+            if (*v->s && v->s[0] == '\\0')\n-+                free((char *)v->s);\n-+            strbuf_addchars(&buf, ' ', ref->align_value - len);\n-+            v->s = strbuf_detach(&buf, NULL);\n-+        }\n-+        ref->align_value = 0;\n-+    }\n-+}\n-+\n- void show_ref_array_item(struct ref_array_item *info, const char\n*format, int quote_style)\n- {\n-     const char *cp, *sp, *ep;\n-\n-     for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {\n-         struct atom_value *atomv;\n-+        int parsed_atom;\n-\n-         ep = strchr(sp, ')');\n-         if (cp < sp)\n-             emit(cp, sp);\n--        get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);\n-+        parsed_atom = parse_ref_filter_atom(sp + 2, ep);\n-+        get_ref_atom_value(info, parsed_atom, &atomv);\n-+        assign_formating(info, parsed_atom, atomv);\n-         print_value(atomv, quote_style);\n-     }\n-     if (*cp) {\n-diff --git a/ref-filter.h b/ref-filter.h\n-index 6bf27d8..12ffbc5 100644\n---- a/ref-filter.h\n-+++ b/ref-filter.h\n-@@ -30,6 +30,7 @@ struct ref_sorting {\n- struct ref_array_item {\n-     unsigned char objectname[20];\n-     int flag;\n-+    unsigned int align_value;\n-     const char *symref;\n-     struct commit *commit;\n-     struct atom_value *value;\n---\n-2.4.6\n-\n\nThanks to everyone for suggestions on the previous version.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266804","messageId":"1437982035-6658-1-git-send-email-Karthik.188@gmail.com","threadId":"39939","inReplyTo":"CAOLa=ZTtAepdO5U8zo62VBn_z4-LcKoguPxVGaAnNZULEwzrQQ@mail.gmail.com","subject":"[PATCH v5 01/11] ref-filter: introduce 'ref_formatting_state'","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T07:27:05Z","receivedAt":"2015-07-27T07:27:05Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Introduce 'ref_formatting' structure to hold values of pseudo atoms\nwhich help only in formatting. This will eventually be used by atoms\nlike `color` and the `padright` atom which will be introduced in a\nlater patch.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\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 ref-filter.c | 46 +++++++++++++++++++++++++++++++---------------\n ref-filter.h |  5 +++++\n 2 files changed, 36 insertions(+), 15 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 7561727..a3625e8 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -10,6 +10,8 @@\n #include \"quote.h\"\n #include \"ref-filter.h\"\n #include \"revision.h\"\n+#include \"utf8.h\"\n+#include \"git-compat-util.h\"\n \n typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n \n@@ -620,7 +622,7 @@ static void populate_value(struct ref_array_item *ref)\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 *refname = NULL;\n \t\tconst char *formatp;\n \t\tstruct branch *branch = NULL;\n \n@@ -1190,30 +1192,40 @@ void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)\n \tqsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);\n }\n \n-static void print_value(struct atom_value *v, int quote_style)\n+static void ref_formatting(struct ref_formatting_state *state,\n+\t\t\t   struct atom_value *v, struct strbuf *value)\n {\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tswitch (quote_style) {\n+\tstrbuf_addf(value, \"%s\", v->s);\n+}\n+\n+static void print_value(struct ref_formatting_state *state, struct atom_value *v)\n+{\n+\tstruct strbuf value = STRBUF_INIT;\n+\tstruct strbuf formatted = STRBUF_INIT;\n+\n+\tref_formatting(state, v, &value);\n+\n+\tswitch (state->quote_style) {\n \tcase QUOTE_NONE:\n-\t\tfputs(v->s, stdout);\n+\t\tfputs(value.buf, stdout);\n \t\tbreak;\n \tcase QUOTE_SHELL:\n-\t\tsq_quote_buf(&sb, v->s);\n+\t\tsq_quote_buf(&formatted, value.buf);\n \t\tbreak;\n \tcase QUOTE_PERL:\n-\t\tperl_quote_buf(&sb, v->s);\n+\t\tperl_quote_buf(&formatted, value.buf);\n \t\tbreak;\n \tcase QUOTE_PYTHON:\n-\t\tpython_quote_buf(&sb, v->s);\n+\t\tpython_quote_buf(&formatted, value.buf);\n \t\tbreak;\n \tcase QUOTE_TCL:\n-\t\ttcl_quote_buf(&sb, v->s);\n+\t\ttcl_quote_buf(&formatted, value.buf);\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+\tif (state->quote_style != QUOTE_NONE)\n+\t\tfputs(formatted.buf, stdout);\n+\tstrbuf_release(&formatted);\n+\tstrbuf_release(&value);\n }\n \n static int hex1(char ch)\n@@ -1257,6 +1269,10 @@ static void emit(const char *cp, const char *ep)\n void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)\n {\n \tconst char *cp, *sp, *ep;\n+\tstruct ref_formatting_state state;\n+\n+\tmemset(&state, 0, sizeof(state));\n+\tstate.quote_style = quote_style;\n \n \tfor (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {\n \t\tstruct atom_value *atomv;\n@@ -1265,7 +1281,7 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n \t\tif (cp < sp)\n \t\t\temit(cp, sp);\n \t\tget_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);\n-\t\tprint_value(atomv, quote_style);\n+\t\tprint_value(&state, atomv);\n \t}\n \tif (*cp) {\n \t\tsp = cp + strlen(cp);\n@@ -1278,7 +1294,7 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\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\tprint_value(&state, &resetv);\n \t}\n \tputchar('\\n');\n }\ndiff --git a/ref-filter.h b/ref-filter.h\nindex 6bf27d8..f24e3ef 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -19,6 +19,11 @@\n struct atom_value {\n \tconst char *s;\n \tunsigned long ul; /* used for sorting when not FIELD_STR */\n+\tunsigned int pseudo_atom : 1; /*  atoms which aren't placeholders for ref attributes */\n+};\n+\n+struct ref_formatting_state {\n+\tint quote_style;\n };\n \n struct ref_sorting {\n-- \n2.4.6\n"},{"id":"266805","messageId":"1437982035-6658-2-git-send-email-Karthik.188@gmail.com","threadId":"39939","inReplyTo":"1437982035-6658-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v5 02/11] ref-filter: make `color` use `ref_formatting_state`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T07:27:06Z","receivedAt":"2015-07-27T07:27:06Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Make the `color` atom a pseudo atom and ensure that it uses\n`ref_formatting_state`.\n---\n ref-filter.c | 19 ++++++++++++++++++-\n ref-filter.h |  4 +++-\n 2 files changed, 21 insertions(+), 2 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex a3625e8..cc25c85 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -662,6 +662,8 @@ static void populate_value(struct ref_array_item *ref)\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\tv->pseudo_atom = 1;\n+\t\t\tv->color = 1;\n \t\t\tcontinue;\n \t\t} else if (!strcmp(name, \"flag\")) {\n \t\t\tchar buf[256], *cp = buf;\n@@ -1195,6 +1197,11 @@ void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)\n static void ref_formatting(struct ref_formatting_state *state,\n \t\t\t   struct atom_value *v, struct strbuf *value)\n {\n+\tif (state->color) {\n+\t\tstrbuf_addstr(value, state->color);\n+\t\tfree(state->color);\n+\t\tstate->color = NULL;\n+\t}\n \tstrbuf_addf(value, \"%s\", v->s);\n }\n \n@@ -1266,6 +1273,13 @@ static void emit(const char *cp, const char *ep)\n \t}\n }\n \n+static void apply_pseudo_state(struct ref_formatting_state *state,\n+\t\t\t       struct atom_value *v)\n+{\n+\tif (v->color)\n+\t\tstate->color = (char *)v->s;\n+}\n+\n void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)\n {\n \tconst char *cp, *sp, *ep;\n@@ -1281,7 +1295,10 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n \t\tif (cp < sp)\n \t\t\temit(cp, sp);\n \t\tget_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);\n-\t\tprint_value(&state, atomv);\n+\t\tif (atomv->pseudo_atom)\n+\t\t\tapply_pseudo_state(&state, atomv);\n+\t\telse\n+\t\t\tprint_value(&state, atomv);\n \t}\n \tif (*cp) {\n \t\tsp = cp + strlen(cp);\ndiff --git a/ref-filter.h b/ref-filter.h\nindex f24e3ef..7687879 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -19,11 +19,13 @@\n struct atom_value {\n \tconst char *s;\n \tunsigned long ul; /* used for sorting when not FIELD_STR */\n-\tunsigned int pseudo_atom : 1; /*  atoms which aren't placeholders for ref attributes */\n+\tunsigned int pseudo_atom : 1, /*  atoms which aren't placeholders for ref attributes */\n+\t\tcolor : 1;\n };\n \n struct ref_formatting_state {\n \tint quote_style;\n+\tchar *color;\n };\n \n struct ref_sorting {\n-- \n2.4.6\n"},{"id":"266806","messageId":"1437982035-6658-3-git-send-email-Karthik.188@gmail.com","threadId":"39939","inReplyTo":"1437982035-6658-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v5 03/11] ref-filter: add option to pad atoms to the right","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T07:27:07Z","receivedAt":"2015-07-27T07:27:07Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Add a new atom \"padright\" and support %(padright:X) where X is a\nnumber.  This will align the succeeding atom value to the left\nfollowed by spaces for a total length of X characters. If X is less\nthan the item size, the entire atom value is printed.\n\nAdd tests and documentation for the same.\n\nHelped-by: Duy Nguyen <pclouds@gmail.com>\nHelped-by: Junio C Hamano <gitster@pobox.com>\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 Documentation/git-for-each-ref.txt |  6 ++++++\n ref-filter.c                       | 24 ++++++++++++++++++++++++\n ref-filter.h                       |  4 +++-\n t/t6302-for-each-ref-filter.sh     | 16 ++++++++++++++++\n 4 files changed, 49 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex e49d578..45dd7f8 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -127,6 +127,12 @@ color::\n \tChange output color.  Followed by `:<colorname>`, where names\n \tare described in `color.branch.*`.\n \n+padright::\n+\tPad succeeding atom to the right. Followed by `:<value>`,\n+\twhere `value` states the total length of atom including the\n+\tpadding. If the `value` is greater than the atom length, then\n+\tno padding is performed.\n+\n In addition to the above, for commit and tag objects, the header\n field names (`tree`, `parent`, `object`, `type`, and `tag`) can\n be used to specify the value in the header field.\ndiff --git a/ref-filter.c b/ref-filter.c\nindex cc25c85..7ab34be 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -55,6 +55,7 @@ static struct {\n \t{ \"flag\" },\n \t{ \"HEAD\" },\n \t{ \"color\" },\n+\t{ \"padright\" },\n };\n \n /*\n@@ -691,6 +692,18 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\telse\n \t\t\t\tv->s = \" \";\n \t\t\tcontinue;\n+\t\t} else if (starts_with(name, \"padright:\")) {\n+\t\t\tconst char *valp = NULL;\n+\n+\t\t\tskip_prefix(name, \"padright:\", &valp);\n+\t\t\tif (!valp[0])\n+\t\t\t\tdie(_(\"no value given with 'padright:'\"));\n+\t\t\tif (strtoul_ui(valp, 10, (unsigned int *)&v->ul))\n+\t\t\t\tdie(_(\"positive integer expected after ':' in padright:%u\\n\"),\n+\t\t\t\t    (unsigned int)v->ul);\n+\t\t\tv->pseudo_atom = 1;\n+\t\t\tv->pad_to_right = 1;\n+\t\t\tcontinue;\n \t\t} else\n \t\t\tcontinue;\n \n@@ -1202,6 +1215,15 @@ static void ref_formatting(struct ref_formatting_state *state,\n \t\tfree(state->color);\n \t\tstate->color = NULL;\n \t}\n+\tif (state->pad_to_right) {\n+\t\tif (!is_utf8(v->s))\n+\t\t\tstrbuf_addf(value, \"%-*s\", state->pad_to_right, v->s);\n+\t\telse {\n+\t\t\tint utf8_compensation = strlen(v->s) - utf8_strwidth(v->s);\n+\t\t\tstrbuf_addf(value, \"%-*s\", state->pad_to_right + utf8_compensation, v->s);\n+\t\t}\n+\t\treturn;\n+\t}\n \tstrbuf_addf(value, \"%s\", v->s);\n }\n \n@@ -1278,6 +1300,8 @@ static void apply_pseudo_state(struct ref_formatting_state *state,\n {\n \tif (v->color)\n \t\tstate->color = (char *)v->s;\n+\tif (v->pad_to_right)\n+\t\tstate->pad_to_right = v->ul;\n }\n \n void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)\ndiff --git a/ref-filter.h b/ref-filter.h\nindex 7687879..63b8175 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -20,12 +20,14 @@ struct atom_value {\n \tconst char *s;\n \tunsigned long ul; /* used for sorting when not FIELD_STR */\n \tunsigned int pseudo_atom : 1, /*  atoms which aren't placeholders for ref attributes */\n-\t\tcolor : 1;\n+\t\tcolor : 1,\n+\t\tpad_to_right : 1;\n };\n \n struct ref_formatting_state {\n \tint quote_style;\n \tchar *color;\n+\tunsigned int pad_to_right;\n };\n \n struct ref_sorting {\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex 505a360..daaa27a 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -81,4 +81,20 @@ test_expect_success 'filtering with --contains' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'padding to the right using `padright`' '\n+\tcat >expect <<-\\EOF &&\n+\trefs/heads/master        |\n+\trefs/heads/side          |\n+\trefs/odd/spot            |\n+\trefs/tags/double-tag     |\n+\trefs/tags/four           |\n+\trefs/tags/one            |\n+\trefs/tags/signed-tag     |\n+\trefs/tags/three          |\n+\trefs/tags/two            |\n+\tEOF\n+\tgit for-each-ref --format=\"%(padright:25)%(refname)|\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.4.6\n"},{"id":"266810","messageId":"1437982035-6658-4-git-send-email-Karthik.188@gmail.com","threadId":"39939","inReplyTo":"1437982035-6658-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v5 04/11] ref-filter: add option to filter only tags","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T07:27:08Z","receivedAt":"2015-07-27T07:27:08Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak <karthik.188@gmail.com>\n\nAdd a functions called 'for_each_tag_ref_fullpath()' to refs.{c,h}\nwhich iterates through each tag ref without trimming the path.\n\nAdd an option in 'filter_refs()' to use 'for_each_tag_ref_fullpath()'\nand filter refs. This type checking is done by adding a\n'FILTER_REFS_TAGS' in 'ref-filter.h'\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 ref-filter.c | 2 ++\n ref-filter.h | 1 +\n refs.c       | 5 +++++\n refs.h       | 1 +\n 4 files changed, 9 insertions(+)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 7ab34be..6e01b44 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1152,6 +1152,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n \t\tret = for_each_rawref(ref_filter_handler, &ref_cbdata);\n \telse if (type & FILTER_REFS_ALL)\n \t\tret = for_each_ref(ref_filter_handler, &ref_cbdata);\n+\telse if (type & FILTER_REFS_TAGS)\n+\t\tret = for_each_tag_ref_fullpath(ref_filter_handler, &ref_cbdata);\n \telse if (type)\n \t\tdie(\"filter_refs: invalid type\");\n \ndiff --git a/ref-filter.h b/ref-filter.h\nindex 63b8175..62ad979 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -15,6 +15,7 @@\n \n #define FILTER_REFS_INCLUDE_BROKEN 0x1\n #define FILTER_REFS_ALL 0x2\n+#define FILTER_REFS_TAGS 0x4\n \n struct atom_value {\n \tconst char *s;\ndiff --git a/refs.c b/refs.c\nindex 0b96ece..23ce483 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2108,6 +2108,11 @@ int for_each_tag_ref(each_ref_fn fn, void *cb_data)\n \treturn for_each_ref_in(\"refs/tags/\", fn, cb_data);\n }\n \n+int for_each_tag_ref_fullpath(each_ref_fn fn, void *cb_data)\n+{\n+\treturn do_for_each_ref(&ref_cache, \"refs/tags/\", fn, 0, 0, cb_data);\n+}\n+\n int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)\n {\n \treturn for_each_ref_in_submodule(submodule, \"refs/tags/\", fn, cb_data);\ndiff --git a/refs.h b/refs.h\nindex e4e46c3..9eee2de 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -174,6 +174,7 @@ extern int head_ref(each_ref_fn fn, void *cb_data);\n extern int for_each_ref(each_ref_fn fn, void *cb_data);\n extern int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data);\n extern int for_each_tag_ref(each_ref_fn fn, void *cb_data);\n+extern int for_each_tag_ref_fullpath(each_ref_fn fn, void *cb_data);\n extern int for_each_branch_ref(each_ref_fn fn, void *cb_data);\n extern int for_each_remote_ref(each_ref_fn fn, void *cb_data);\n extern int for_each_replace_ref(each_ref_fn fn, void *cb_data);\n-- \n2.4.6\n"},{"id":"266808","messageId":"1437982035-6658-5-git-send-email-Karthik.188@gmail.com","threadId":"39939","inReplyTo":"1437982035-6658-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v5 05/11] ref-filter: support printing N lines from tag annotation","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T07:27:09Z","receivedAt":"2015-07-27T07:27:09Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak <karthik.188@gmail.com>\n\nIn 'tag.c' we can print N lines from the annotation of the tag using\nthe '-n<num>' option. Copy code from 'tag.c' to 'ref-filter' and\nmodify 'ref-filter' to support printing of N lines from the annotation\nof tags.\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 |  2 +-\n builtin/tag.c          |  4 ++++\n ref-filter.c           | 51 +++++++++++++++++++++++++++++++++++++++++++++++++-\n ref-filter.h           |  9 +++++++--\n 4 files changed, 62 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 40f343b..e4a4f8a 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -74,7 +74,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \tif (!maxcount || array.nr < maxcount)\n \t\tmaxcount = array.nr;\n \tfor (i = 0; i < maxcount; i++)\n-\t\tshow_ref_array_item(array.items[i], format, quote_style);\n+\t\tshow_ref_array_item(array.items[i], format, quote_style, 0);\n \tref_array_clear(&array);\n \treturn 0;\n }\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 471d6b1..0fc7557 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -185,6 +185,10 @@ static enum contains_result contains(struct commit *candidate,\n \treturn contains_test(candidate, want);\n }\n \n+/*\n+ * Currently duplicated in ref-filter, will eventually be removed as\n+ * we port tag.c to use ref-filter APIs.\n+ */\n static void show_tag_lines(const struct object_id *oid, int lines)\n {\n \tint i;\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 6e01b44..1605252 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1306,7 +1306,51 @@ static void apply_pseudo_state(struct ref_formatting_state *state,\n \t\tstate->pad_to_right = v->ul;\n }\n \n-void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)\n+/*\n+ * If 'lines' is greater than 0, print that many lines from the given\n+ * object_id 'oid'.\n+ */\n+static void show_tag_lines(const struct object_id *oid, int lines)\n+{\n+\tint i;\n+\tunsigned long size;\n+\tenum object_type type;\n+\tchar *buf, *sp, *eol;\n+\tsize_t len;\n+\n+\tbuf = read_sha1_file(oid->hash, &type, &size);\n+\tif (!buf)\n+\t\tdie_errno(\"unable to read object %s\", oid_to_hex(oid));\n+\tif (type != OBJ_COMMIT && type != OBJ_TAG)\n+\t\tgoto free_return;\n+\tif (!size)\n+\t\tdie(\"an empty %s object %s?\",\n+\t\t    typename(type), oid_to_hex(oid));\n+\n+\t/* skip header */\n+\tsp = strstr(buf, \"\\n\\n\");\n+\tif (!sp)\n+\t\tgoto free_return;\n+\n+\t/* only take up to \"lines\" lines, and strip the signature from a tag */\n+\tif (type == OBJ_TAG)\n+\t\tsize = parse_signature(buf, size);\n+\tfor (i = 0, sp += 2; i < lines && sp < buf + size; i++) {\n+\t\tif (i)\n+\t\t\tprintf(\"\\n    \");\n+\t\teol = memchr(sp, '\\n', size - (sp - buf));\n+\t\tlen = eol ? eol - sp : size - (sp - buf);\n+\t\tfwrite(sp, len, 1, stdout);\n+\t\tif (!eol)\n+\t\t\tbreak;\n+\t\tsp = eol + 1;\n+\t}\n+free_return:\n+\tfree(buf);\n+}\n+\n+void show_ref_array_item(struct ref_array_item *info, const char *format,\n+\t\t\t int quote_style, unsigned int lines)\n {\n \tconst char *cp, *sp, *ep;\n \tstruct ref_formatting_state state;\n@@ -1339,6 +1383,11 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n \t\tresetv.s = color;\n \t\tprint_value(&state, &resetv);\n \t}\n+\tif (lines > 0) {\n+\t\tstruct object_id oid;\n+\t\thashcpy(oid.hash, info->objectname);\n+\t\tshow_tag_lines(&oid, lines);\n+\t}\n \tputchar('\\n');\n }\n \ndiff --git a/ref-filter.h b/ref-filter.h\nindex 62ad979..570fc3a 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -64,6 +64,7 @@ struct ref_filter {\n \tstruct commit *merge_commit;\n \n \tunsigned int with_commit_tag_algo : 1;\n+\tunsigned int lines;\n };\n \n struct ref_filter_cbdata {\n@@ -95,8 +96,12 @@ int parse_ref_filter_atom(const char *atom, const char *ep);\n int verify_ref_format(const char *format);\n /*  Sort the given ref_array as per the ref_sorting provided */\n void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);\n-/*  Print the ref using the given format and quote_style */\n-void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style);\n+/*\n+ * Print the ref using the given format and quote_style. If 'lines' > 0,\n+ * print that many lines of the the given ref.\n+ */\n+void show_ref_array_item(struct ref_array_item *info, const char *format,\n+\t\t\t int quote_style, unsigned int lines);\n /*  Callback function for parsing the sort option */\n int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset);\n /*  Default sort option based on refname */\n-- \n2.4.6\n"},{"id":"266809","messageId":"1437982035-6658-6-git-send-email-Karthik.188@gmail.com","threadId":"39939","inReplyTo":"1437982035-6658-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v5 06/11] ref-filter: add support to sort by version","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T07:27:10Z","receivedAt":"2015-07-27T07:27:10Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak <karthik.188@gmail.com>\n\nAdd support to sort by version using the \"v:refname\" and\n\"version:refname\" option. This is achieved by using the 'versioncmp()'\nfunction as the comparing function for qsort.\n\nThis option is included to support sorting by versions in `git tag -l`\nwhich will eventaully be ported to use ref-filter APIs.\n\nAdd documentation and tests for the same.\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 Documentation/git-for-each-ref.txt |  3 +++\n ref-filter.c                       | 14 +++++++++-----\n ref-filter.h                       |  3 ++-\n t/t6302-for-each-ref-filter.sh     | 36 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 50 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 45dd7f8..2b60aee 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -151,6 +151,9 @@ For sorting purposes, fields with numeric values sort in numeric\n order (`objectsize`, `authordate`, `committerdate`, `taggerdate`).\n All other fields are used to sort in their byte-value order.\n \n+There is also an option to sort by versions, this can be done by using\n+the fieldname `version:refname` or in short `v:refname`.\n+\n In any case, a field name that refers to a field inapplicable to\n the object referred by the ref does not cause an error.  It\n returns an empty string instead.\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 1605252..26eb26c 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -12,6 +12,7 @@\n #include \"revision.h\"\n #include \"utf8.h\"\n #include \"git-compat-util.h\"\n+#include \"version.h\"\n \n typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n \n@@ -1172,19 +1173,19 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru\n \n \tget_ref_atom_value(a, s->atom, &va);\n \tget_ref_atom_value(b, s->atom, &vb);\n-\tswitch (cmp_type) {\n-\tcase FIELD_STR:\n+\tif (s->version)\n+\t\tcmp = versioncmp(va->s, vb->s);\n+\telse if (cmp_type == FIELD_STR)\n \t\tcmp = strcmp(va->s, vb->s);\n-\t\tbreak;\n-\tdefault:\n+\telse {\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+\n \treturn (s->reverse) ? -cmp : cmp;\n }\n \n@@ -1420,6 +1421,9 @@ int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)\n \t\ts->reverse = 1;\n \t\targ++;\n \t}\n+\tif (skip_prefix(arg, \"version:\", &arg) ||\n+\t    skip_prefix(arg, \"v:\", &arg))\n+\t\ts->version = 1;\n \tlen = strlen(arg);\n \ts->atom = parse_ref_filter_atom(arg, arg+len);\n \treturn 0;\ndiff --git a/ref-filter.h b/ref-filter.h\nindex 570fc3a..5406721 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -34,7 +34,8 @@ struct ref_formatting_state {\n struct ref_sorting {\n \tstruct ref_sorting *next;\n \tint atom; /* index into used_atom array (internal) */\n-\tunsigned reverse : 1;\n+\tunsigned reverse : 1,\n+\t\tversion : 1;\n };\n \n struct ref_array_item {\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex daaa27a..de872db 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -97,4 +97,40 @@ test_expect_success 'padding to the right using `padright`' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'setup for version sort' '\n+\ttest_commit foo1.3 &&\n+\ttest_commit foo1.6 &&\n+\ttest_commit foo1.10\n+'\n+\n+test_expect_success 'version sort' '\n+\tgit for-each-ref --sort=version:refname --format=\"%(refname:short)\" refs/tags/ | grep \"foo\" >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tfoo1.3\n+\tfoo1.6\n+\tfoo1.10\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'version sort (shortened)' '\n+\tgit for-each-ref --sort=v:refname --format=\"%(refname:short)\" refs/tags/ | grep \"foo\" >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tfoo1.3\n+\tfoo1.6\n+\tfoo1.10\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'reverse version sort' '\n+\tgit for-each-ref --sort=-version:refname --format=\"%(refname:short)\" refs/tags/ | grep \"foo\" >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tfoo1.10\n+\tfoo1.6\n+\tfoo1.3\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.4.6\n"},{"id":"266807","messageId":"1437982035-6658-7-git-send-email-Karthik.188@gmail.com","threadId":"39939","inReplyTo":"1437982035-6658-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v5 07/11] ref-filter: add option to match literal pattern","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T07:27:11Z","receivedAt":"2015-07-27T07:27:11Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak <karthik.188@gmail.com>\n\nSince 'ref-filter' only has an option to match path names add an\noption for plain fnmatch pattern-matching.\n\nThis is to support the pattern matching options which are used in `git\ntag -l` and `git branch -l` where we can match patterns like `git tag\n-l foo*` which would match all tags which has a \"foo*\" pattern.\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 |  1 +\n ref-filter.c           | 38 +++++++++++++++++++++++++++++++++++++-\n ref-filter.h           |  3 ++-\n 3 files changed, 40 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex e4a4f8a..3ad6a64 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -68,6 +68,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \tgit_config(git_default_config, NULL);\n \n \tfilter.name_patterns = argv;\n+\tfilter.match_as_path = 1;\n \tfilter_refs(&array, &filter, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN);\n \tref_array_sort(sorting, &array);\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex 26eb26c..597b189 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -946,6 +946,32 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)\n \n /*\n  * Return 1 if the refname matches one of the patterns, otherwise 0.\n+ * A pattern can be a literal prefix (e.g. a refname \"refs/heads/master\"\n+ * matches a pattern \"refs/heads/mas\") or a wildcard (e.g. the same ref\n+ * matches \"refs/heads/mas*\", too).\n+ */\n+static int match_pattern(const char **patterns, const char *refname)\n+{\n+\t/*\n+\t * When no '--format' option is given we need to skip the prefix\n+\t * for matching refs of tags and branches.\n+\t */\n+\tif (skip_prefix(refname, \"refs/tags/\", &refname))\n+\t\t;\n+\telse if (skip_prefix(refname, \"refs/heads/\", &refname))\n+\t\t;\n+\telse if (skip_prefix(refname, \"refs/remotes/\", &refname))\n+\t\t;\n+\n+\tfor (; *patterns; patterns++) {\n+\t\tif (!wildmatch(*patterns, refname, 0, NULL))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+/*\n+ * Return 1 if the refname matches one of the patterns, otherwise 0.\n  * A pattern can be path prefix (e.g. a refname \"refs/heads/master\"\n  * matches a pattern \"refs/heads/\") or a wildcard (e.g. the same ref\n  * matches \"refs/heads/m*\",too).\n@@ -969,6 +995,16 @@ static int match_name_as_path(const char **pattern, const char *refname)\n \treturn 0;\n }\n \n+/* Return 1 if the refname matches one of the patterns, otherwise 0. */\n+static int filter_pattern_match(struct ref_filter *filter, const char *refname)\n+{\n+\tif (!*filter->name_patterns)\n+\t\treturn 1; /* No pattern always matches */\n+\tif (filter->match_as_path)\n+\t\treturn match_name_as_path(filter->name_patterns, refname);\n+\treturn match_pattern(filter->name_patterns, refname);\n+}\n+\n /*\n  * Given a ref (sha1, refname), check if the ref belongs to the array\n  * of sha1s. If the given ref is a tag, check if the given tag points\n@@ -1037,7 +1073,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,\n \t\treturn 0;\n \t}\n \n-\tif (*filter->name_patterns && !match_name_as_path(filter->name_patterns, refname))\n+\tif (!filter_pattern_match(filter, refname))\n \t\treturn 0;\n \n \tif (filter->points_at.nr && !match_points_at(&filter->points_at, oid->hash, refname))\ndiff --git a/ref-filter.h b/ref-filter.h\nindex 5406721..a27745f 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -64,7 +64,8 @@ struct ref_filter {\n \t} merge;\n \tstruct commit *merge_commit;\n \n-\tunsigned int with_commit_tag_algo : 1;\n+\tunsigned int with_commit_tag_algo : 1,\n+\t\tmatch_as_path : 1;\n \tunsigned int lines;\n };\n \n-- \n2.4.6\n"},{"id":"266811","messageId":"1437982035-6658-8-git-send-email-Karthik.188@gmail.com","threadId":"39939","inReplyTo":"1437982035-6658-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v5 08/11] tag.c: use 'ref-filter' data structures","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T07:27:12Z","receivedAt":"2015-07-27T07:27:12Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak <karthik.188@gmail.com>\n\nMake 'tag.c' use 'ref-filter' data structures and make changes to\nsupport the new data structures. This is a part of the process\nof porting 'tag.c' to use 'ref-filter' APIs.\n\nThis is a temporary step before porting 'tag.c' to use 'ref-filter'\ncompletely. As this is a temporary step, most of the code\nintroduced here will be removed when 'tag.c' is ported over to use\n'ref-filter' APIs\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/tag.c | 106 +++++++++++++++++++++++++++++++---------------------------\n 1 file changed, 57 insertions(+), 49 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 0fc7557..e96bae2 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -17,6 +17,7 @@\n #include \"gpg-interface.h\"\n #include \"sha1-array.h\"\n #include \"column.h\"\n+#include \"ref-filter.h\"\n \n static const char * const git_tag_usage[] = {\n \tN_(\"git tag [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>] <tagname> [<head>]\"),\n@@ -34,15 +35,6 @@ static const char * const git_tag_usage[] = {\n \n static int tag_sort;\n \n-struct tag_filter {\n-\tconst char **patterns;\n-\tint lines;\n-\tint sort;\n-\tstruct string_list tags;\n-\tstruct commit_list *with_commit;\n-};\n-\n-static struct sha1_array points_at;\n static unsigned int colopts;\n \n static int match_pattern(const char **patterns, const char *ref)\n@@ -61,19 +53,20 @@ static int match_pattern(const char **patterns, const char *ref)\n  * removed as we port tag.c to use the ref-filter APIs.\n  */\n static const unsigned char *match_points_at(const char *refname,\n-\t\t\t\t\t    const unsigned char *sha1)\n+\t\t\t\t\t    const unsigned char *sha1,\n+\t\t\t\t\t    struct sha1_array *points_at)\n {\n \tconst unsigned char *tagged_sha1 = NULL;\n \tstruct object *obj;\n \n-\tif (sha1_array_lookup(&points_at, sha1) >= 0)\n+\tif (sha1_array_lookup(points_at, sha1) >= 0)\n \t\treturn sha1;\n \tobj = parse_object(sha1);\n \tif (!obj)\n \t\tdie(_(\"malformed object at '%s'\"), refname);\n \tif (obj->type == OBJ_TAG)\n \t\ttagged_sha1 = ((struct tag *)obj)->tagged->sha1;\n-\tif (tagged_sha1 && sha1_array_lookup(&points_at, tagged_sha1) >= 0)\n+\tif (tagged_sha1 && sha1_array_lookup(points_at, tagged_sha1) >= 0)\n \t\treturn tagged_sha1;\n \treturn NULL;\n }\n@@ -228,12 +221,24 @@ free_return:\n \tfree(buf);\n }\n \n+static void ref_array_append(struct ref_array *array, const char *refname)\n+{\n+\tsize_t len = strlen(refname);\n+\tstruct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) + len + 1);\n+\tmemcpy(ref->refname, refname, len);\n+\tref->refname[len] = '\\0';\n+\tREALLOC_ARRAY(array->items, array->nr + 1);\n+\tarray->items[array->nr++] = ref;\n+}\n+\n static int show_reference(const char *refname, const struct object_id *oid,\n \t\t\t  int flag, void *cb_data)\n {\n-\tstruct tag_filter *filter = cb_data;\n+\tstruct ref_filter_cbdata *data = cb_data;\n+\tstruct ref_array *array = data->array;\n+\tstruct ref_filter *filter = data->filter;\n \n-\tif (match_pattern(filter->patterns, refname)) {\n+\tif (match_pattern(filter->name_patterns, refname)) {\n \t\tif (filter->with_commit) {\n \t\t\tstruct commit *commit;\n \n@@ -244,12 +249,12 @@ static int show_reference(const char *refname, const struct object_id *oid,\n \t\t\t\treturn 0;\n \t\t}\n \n-\t\tif (points_at.nr && !match_points_at(refname, oid->hash))\n+\t\tif (filter->points_at.nr && !match_points_at(refname, oid->hash, &filter->points_at))\n \t\t\treturn 0;\n \n \t\tif (!filter->lines) {\n-\t\t\tif (filter->sort)\n-\t\t\t\tstring_list_append(&filter->tags, refname);\n+\t\t\tif (tag_sort)\n+\t\t\t\tref_array_append(array, refname);\n \t\t\telse\n \t\t\t\tprintf(\"%s\\n\", refname);\n \t\t\treturn 0;\n@@ -264,36 +269,36 @@ static int show_reference(const char *refname, const struct object_id *oid,\n \n static int sort_by_version(const void *a_, const void *b_)\n {\n-\tconst struct string_list_item *a = a_;\n-\tconst struct string_list_item *b = b_;\n-\treturn versioncmp(a->string, b->string);\n+\tconst struct ref_array_item *a = *((struct ref_array_item **)a_);\n+\tconst struct ref_array_item *b = *((struct ref_array_item **)b_);\n+\treturn versioncmp(a->refname, b->refname);\n }\n \n-static int list_tags(const char **patterns, int lines,\n-\t\t     struct commit_list *with_commit, int sort)\n+static int list_tags(struct ref_filter *filter, int sort)\n {\n-\tstruct tag_filter filter;\n+\tstruct ref_array array;\n+\tstruct ref_filter_cbdata data;\n+\n+\tmemset(&array, 0, sizeof(array));\n+\tdata.array = &array;\n+\tdata.filter = filter;\n \n-\tfilter.patterns = patterns;\n-\tfilter.lines = lines;\n-\tfilter.sort = sort;\n-\tfilter.with_commit = with_commit;\n-\tmemset(&filter.tags, 0, sizeof(filter.tags));\n-\tfilter.tags.strdup_strings = 1;\n+\tif (filter->lines == -1)\n+\t\tfilter->lines = 0;\n \n-\tfor_each_tag_ref(show_reference, (void *)&filter);\n+\tfor_each_tag_ref(show_reference, &data);\n \tif (sort) {\n \t\tint i;\n \t\tif ((sort & SORT_MASK) == VERCMP_SORT)\n-\t\t\tqsort(filter.tags.items, filter.tags.nr,\n-\t\t\t      sizeof(struct string_list_item), sort_by_version);\n+\t\t\tqsort(array.items, array.nr,\n+\t\t\t      sizeof(struct ref_array_item *), sort_by_version);\n \t\tif (sort & REVERSE_SORT)\n-\t\t\tfor (i = filter.tags.nr - 1; i >= 0; i--)\n-\t\t\t\tprintf(\"%s\\n\", filter.tags.items[i].string);\n+\t\t\tfor (i = array.nr - 1; i >= 0; i--)\n+\t\t\t\tprintf(\"%s\\n\", array.items[i]->refname);\n \t\telse\n-\t\t\tfor (i = 0; i < filter.tags.nr; i++)\n-\t\t\t\tprintf(\"%s\\n\", filter.tags.items[i].string);\n-\t\tstring_list_clear(&filter.tags, 0);\n+\t\t\tfor (i = 0; i < array.nr; i++)\n+\t\t\t\tprintf(\"%s\\n\", array.items[i]->refname);\n+\t\tref_array_clear(&array);\n \t}\n \treturn 0;\n }\n@@ -574,17 +579,17 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tconst char *object_ref, *tag;\n \tstruct create_tag_options opt;\n \tchar *cleanup_arg = NULL;\n-\tint annotate = 0, force = 0, lines = -1;\n \tint create_reflog = 0;\n+\tint annotate = 0, force = 0;\n \tint cmdmode = 0;\n \tconst char *msgfile = NULL, *keyid = NULL;\n \tstruct msg_arg msg = { 0, STRBUF_INIT };\n-\tstruct commit_list *with_commit = NULL;\n \tstruct ref_transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n+\tstruct ref_filter filter;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE('l', \"list\", &cmdmode, N_(\"list tag names\"), 'l'),\n-\t\t{ OPTION_INTEGER, 'n', NULL, &lines, N_(\"n\"),\n+\t\t{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_(\"n\"),\n \t\t\t\tN_(\"print <n> lines of each tag message\"),\n \t\t\t\tPARSE_OPT_OPTARG, NULL, 1 },\n \t\tOPT_CMDMODE('d', \"delete\", &cmdmode, N_(\"delete tags\"), 'd'),\n@@ -606,14 +611,14 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \n \t\tOPT_GROUP(N_(\"Tag listing options\")),\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\n-\t\tOPT_CONTAINS(&with_commit, N_(\"print only tags that contain the commit\")),\n-\t\tOPT_WITH(&with_commit, N_(\"print only tags that contain the commit\")),\n+\t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n+\t\tOPT_WITH(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n \t\t{\n \t\t\tOPTION_CALLBACK, 0, \"sort\", &tag_sort, N_(\"type\"), N_(\"sort tags\"),\n \t\t\tPARSE_OPT_NONEG, parse_opt_sort\n \t\t},\n \t\t{\n-\t\t\tOPTION_CALLBACK, 0, \"points-at\", &points_at, N_(\"object\"),\n+\t\t\tOPTION_CALLBACK, 0, \"points-at\", &filter.points_at, N_(\"object\"),\n \t\t\tN_(\"print only tags of the object\"), 0, parse_opt_object_name\n \t\t},\n \t\tOPT_END()\n@@ -622,6 +627,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tgit_config(git_tag_config, NULL);\n \n \tmemset(&opt, 0, sizeof(opt));\n+\tmemset(&filter, 0, sizeof(filter));\n+\tfilter.lines = -1;\n \n \targc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);\n \n@@ -638,7 +645,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(git_tag_usage, options);\n \n \tfinalize_colopts(&colopts, -1);\n-\tif (cmdmode == 'l' && lines != -1) {\n+\tif (cmdmode == 'l' && filter.lines != -1) {\n \t\tif (explicitly_enable_column(colopts))\n \t\t\tdie(_(\"--column and -n are incompatible\"));\n \t\tcolopts = 0;\n@@ -651,18 +658,19 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\tcopts.padding = 2;\n \t\t\trun_column_filter(colopts, &copts);\n \t\t}\n-\t\tif (lines != -1 && tag_sort)\n+\t\tif (filter.lines != -1 && tag_sort)\n \t\t\tdie(_(\"--sort and -n are incompatible\"));\n-\t\tret = list_tags(argv, lines == -1 ? 0 : lines, with_commit, tag_sort);\n+\t\tfilter.name_patterns = argv;\n+\t\tret = list_tags(&filter, tag_sort);\n \t\tif (column_active(colopts))\n \t\t\tstop_column_filter();\n \t\treturn ret;\n \t}\n-\tif (lines != -1)\n+\tif (filter.lines != -1)\n \t\tdie(_(\"-n option is only allowed with -l.\"));\n-\tif (with_commit)\n+\tif (filter.with_commit)\n \t\tdie(_(\"--contains option is only allowed with -l.\"));\n-\tif (points_at.nr)\n+\tif (filter.points_at.nr)\n \t\tdie(_(\"--points-at option is only allowed with -l.\"));\n \tif (cmdmode == 'd')\n \t\treturn for_each_tag_name(argv, delete_tag);\n-- \n2.4.6\n"},{"id":"266814","messageId":"1437982035-6658-9-git-send-email-Karthik.188@gmail.com","threadId":"39939","inReplyTo":"1437982035-6658-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v5 09/11] tag.c: use 'ref-filter' APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T07:27:13Z","receivedAt":"2015-07-27T07:27:13Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak <karthik.188@gmail.com>\n\nMake 'tag.c' use 'ref-filter' APIs for iterating through refs sorting\nand printing of refs. This removes most of the code used in 'tag.c'\nreplacing it with calls to the 'ref-filter' library.\n\nMake 'tag.c' use the 'filter_refs()' function provided by 'ref-filter'\nto filter out tags based on the options set.\n\nFor printing tags we use 'show_ref_array_item()' function provided by\n'ref-filter'.\n\nWe improve the sorting option provided by 'tag.c' by using the sorting\noptions provided by 'ref-filter'. This causes the test 'invalid sort\nparameter on command line' in t7004 to fail, as 'ref-filter' throws an\nerror for all sorting fields which are incorrect. The test is changed\nto reflect the same.\n\nModify documentation for the same.\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 Documentation/git-tag.txt |  16 ++-\n builtin/tag.c             | 342 ++++++----------------------------------------\n t/t7004-tag.sh            |   8 +-\n 3 files changed, 50 insertions(+), 316 deletions(-)\n\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 84f6496..3ac4a96 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n \t<tagname> [<commit> | <object>]\n 'git tag' -d <tagname>...\n 'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]\n-\t[--column[=<options>] | --no-column] [--create-reflog] [<pattern>...]\n+\t[--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>] [<pattern>...]\n 'git tag' -v <tagname>...\n \n DESCRIPTION\n@@ -94,14 +94,16 @@ OPTIONS\n \tusing fnmatch(3)).  Multiple patterns may be given; if any of\n \tthem matches, the tag is shown.\n \n---sort=<type>::\n-\tSort in a specific order. Supported type is \"refname\"\n-\t(lexicographic order), \"version:refname\" or \"v:refname\" (tag\n+--sort=<key>::\n+\tSort based on the key given.  Prefix `-` to sort in\n+\tdescending order of the value. You may use the --sort=<key> option\n+\tmultiple times, in which case the last key becomes the primary\n+\tkey. Also supports \"version:refname\" or \"v:refname\" (tag\n \tnames are treated as versions). The \"version:refname\" sort\n \torder can also be affected by the\n-\t\"versionsort.prereleaseSuffix\" configuration variable. Prepend\n-\t\"-\" to reverse sort order. When this option is not given, the\n-\tsort order defaults to the value configured for the 'tag.sort'\n+\t\"versionsort.prereleaseSuffix\" configuration variable.\n+\tThe keys supported are the same as those in `git for-each-ref`.\n+\tSort order defaults to the value configured for the 'tag.sort'\n \tvariable if it exists, or lexicographic order otherwise. See\n \tlinkgit:git-config[1].\n \ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex e96bae2..1e8d1b2 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -28,278 +28,32 @@ static const char * const git_tag_usage[] = {\n \tNULL\n };\n \n-#define STRCMP_SORT     0\t/* must be zero */\n-#define VERCMP_SORT     1\n-#define SORT_MASK       0x7fff\n-#define REVERSE_SORT    0x8000\n-\n-static int tag_sort;\n-\n static unsigned int colopts;\n \n-static int match_pattern(const char **patterns, const char *ref)\n-{\n-\t/* no pattern means match everything */\n-\tif (!*patterns)\n-\t\treturn 1;\n-\tfor (; *patterns; patterns++)\n-\t\tif (!wildmatch(*patterns, ref, 0, NULL))\n-\t\t\treturn 1;\n-\treturn 0;\n-}\n-\n-/*\n- * This is currently duplicated in ref-filter.c, and will eventually be\n- * removed as we port tag.c to use the ref-filter APIs.\n- */\n-static const unsigned char *match_points_at(const char *refname,\n-\t\t\t\t\t    const unsigned char *sha1,\n-\t\t\t\t\t    struct sha1_array *points_at)\n-{\n-\tconst unsigned char *tagged_sha1 = NULL;\n-\tstruct object *obj;\n-\n-\tif (sha1_array_lookup(points_at, sha1) >= 0)\n-\t\treturn sha1;\n-\tobj = parse_object(sha1);\n-\tif (!obj)\n-\t\tdie(_(\"malformed object at '%s'\"), refname);\n-\tif (obj->type == OBJ_TAG)\n-\t\ttagged_sha1 = ((struct tag *)obj)->tagged->sha1;\n-\tif (tagged_sha1 && sha1_array_lookup(points_at, tagged_sha1) >= 0)\n-\t\treturn tagged_sha1;\n-\treturn NULL;\n-}\n-\n-static int in_commit_list(const struct commit_list *want, struct commit *c)\n-{\n-\tfor (; want; want = want->next)\n-\t\tif (!hashcmp(want->item->object.sha1, c->object.sha1))\n-\t\t\treturn 1;\n-\treturn 0;\n-}\n-\n-/*\n- * The entire code segment for supporting the --contains option has been\n- * copied over to ref-filter.{c,h}. This will be deleted evetually when\n- * we port tag.c to use ref-filter APIs.\n- */\n-enum contains_result {\n-\tCONTAINS_UNKNOWN = -1,\n-\tCONTAINS_NO = 0,\n-\tCONTAINS_YES = 1\n-};\n-\n-/*\n- * Test whether the candidate or one of its parents is contained in the list.\n- * Do not recurse to find out, though, but return -1 if inconclusive.\n- */\n-static enum contains_result contains_test(struct commit *candidate,\n-\t\t\t    const struct commit_list *want)\n-{\n-\t/* was it previously marked as containing a want commit? */\n-\tif (candidate->object.flags & TMP_MARK)\n-\t\treturn 1;\n-\t/* or marked as not possibly containing a want commit? */\n-\tif (candidate->object.flags & UNINTERESTING)\n-\t\treturn 0;\n-\t/* or are we it? */\n-\tif (in_commit_list(want, candidate)) {\n-\t\tcandidate->object.flags |= TMP_MARK;\n-\t\treturn 1;\n-\t}\n-\n-\tif (parse_commit(candidate) < 0)\n-\t\treturn 0;\n-\n-\treturn -1;\n-}\n-\n-/*\n- * Mimicking the real stack, this stack lives on the heap, avoiding stack\n- * overflows.\n- *\n- * At each recursion step, the stack items points to the commits whose\n- * ancestors are to be inspected.\n- */\n-struct stack {\n-\tint nr, alloc;\n-\tstruct stack_entry {\n-\t\tstruct commit *commit;\n-\t\tstruct commit_list *parents;\n-\t} *stack;\n-};\n-\n-static void push_to_stack(struct commit *candidate, struct stack *stack)\n-{\n-\tint index = stack->nr++;\n-\tALLOC_GROW(stack->stack, stack->nr, stack->alloc);\n-\tstack->stack[index].commit = candidate;\n-\tstack->stack[index].parents = candidate->parents;\n-}\n-\n-static enum contains_result contains(struct commit *candidate,\n-\t\tconst struct commit_list *want)\n-{\n-\tstruct stack stack = { 0, 0, NULL };\n-\tint result = contains_test(candidate, want);\n-\n-\tif (result != CONTAINS_UNKNOWN)\n-\t\treturn result;\n-\n-\tpush_to_stack(candidate, &stack);\n-\twhile (stack.nr) {\n-\t\tstruct stack_entry *entry = &stack.stack[stack.nr - 1];\n-\t\tstruct commit *commit = entry->commit;\n-\t\tstruct commit_list *parents = entry->parents;\n-\n-\t\tif (!parents) {\n-\t\t\tcommit->object.flags |= UNINTERESTING;\n-\t\t\tstack.nr--;\n-\t\t}\n-\t\t/*\n-\t\t * If we just popped the stack, parents->item has been marked,\n-\t\t * therefore contains_test will return a meaningful 0 or 1.\n-\t\t */\n-\t\telse switch (contains_test(parents->item, want)) {\n-\t\tcase CONTAINS_YES:\n-\t\t\tcommit->object.flags |= TMP_MARK;\n-\t\t\tstack.nr--;\n-\t\t\tbreak;\n-\t\tcase CONTAINS_NO:\n-\t\t\tentry->parents = parents->next;\n-\t\t\tbreak;\n-\t\tcase CONTAINS_UNKNOWN:\n-\t\t\tpush_to_stack(parents->item, &stack);\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-\tfree(stack.stack);\n-\treturn contains_test(candidate, want);\n-}\n-\n-/*\n- * Currently duplicated in ref-filter, will eventually be removed as\n- * we port tag.c to use ref-filter APIs.\n- */\n-static void show_tag_lines(const struct object_id *oid, int lines)\n-{\n-\tint i;\n-\tunsigned long size;\n-\tenum object_type type;\n-\tchar *buf, *sp, *eol;\n-\tsize_t len;\n-\n-\tbuf = read_sha1_file(oid->hash, &type, &size);\n-\tif (!buf)\n-\t\tdie_errno(\"unable to read object %s\", oid_to_hex(oid));\n-\tif (type != OBJ_COMMIT && type != OBJ_TAG)\n-\t\tgoto free_return;\n-\tif (!size)\n-\t\tdie(\"an empty %s object %s?\",\n-\t\t    typename(type), oid_to_hex(oid));\n-\n-\t/* skip header */\n-\tsp = strstr(buf, \"\\n\\n\");\n-\tif (!sp)\n-\t\tgoto free_return;\n-\n-\t/* only take up to \"lines\" lines, and strip the signature from a tag */\n-\tif (type == OBJ_TAG)\n-\t\tsize = parse_signature(buf, size);\n-\tfor (i = 0, sp += 2; i < lines && sp < buf + size; i++) {\n-\t\tif (i)\n-\t\t\tprintf(\"\\n    \");\n-\t\teol = memchr(sp, '\\n', size - (sp - buf));\n-\t\tlen = eol ? eol - sp : size - (sp - buf);\n-\t\tfwrite(sp, len, 1, stdout);\n-\t\tif (!eol)\n-\t\t\tbreak;\n-\t\tsp = eol + 1;\n-\t}\n-free_return:\n-\tfree(buf);\n-}\n-\n-static void ref_array_append(struct ref_array *array, const char *refname)\n-{\n-\tsize_t len = strlen(refname);\n-\tstruct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) + len + 1);\n-\tmemcpy(ref->refname, refname, len);\n-\tref->refname[len] = '\\0';\n-\tREALLOC_ARRAY(array->items, array->nr + 1);\n-\tarray->items[array->nr++] = ref;\n-}\n-\n-static int show_reference(const char *refname, const struct object_id *oid,\n-\t\t\t  int flag, void *cb_data)\n-{\n-\tstruct ref_filter_cbdata *data = cb_data;\n-\tstruct ref_array *array = data->array;\n-\tstruct ref_filter *filter = data->filter;\n-\n-\tif (match_pattern(filter->name_patterns, refname)) {\n-\t\tif (filter->with_commit) {\n-\t\t\tstruct commit *commit;\n-\n-\t\t\tcommit = lookup_commit_reference_gently(oid->hash, 1);\n-\t\t\tif (!commit)\n-\t\t\t\treturn 0;\n-\t\t\tif (!contains(commit, filter->with_commit))\n-\t\t\t\treturn 0;\n-\t\t}\n-\n-\t\tif (filter->points_at.nr && !match_points_at(refname, oid->hash, &filter->points_at))\n-\t\t\treturn 0;\n-\n-\t\tif (!filter->lines) {\n-\t\t\tif (tag_sort)\n-\t\t\t\tref_array_append(array, refname);\n-\t\t\telse\n-\t\t\t\tprintf(\"%s\\n\", refname);\n-\t\t\treturn 0;\n-\t\t}\n-\t\tprintf(\"%-15s \", refname);\n-\t\tshow_tag_lines(oid, filter->lines);\n-\t\tputchar('\\n');\n-\t}\n-\n-\treturn 0;\n-}\n-\n-static int sort_by_version(const void *a_, const void *b_)\n-{\n-\tconst struct ref_array_item *a = *((struct ref_array_item **)a_);\n-\tconst struct ref_array_item *b = *((struct ref_array_item **)b_);\n-\treturn versioncmp(a->refname, b->refname);\n-}\n-\n-static int list_tags(struct ref_filter *filter, int sort)\n+static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting)\n {\n \tstruct ref_array array;\n-\tstruct ref_filter_cbdata data;\n+\tchar *format;\n+\tint i;\n \n \tmemset(&array, 0, sizeof(array));\n-\tdata.array = &array;\n-\tdata.filter = filter;\n \n \tif (filter->lines == -1)\n \t\tfilter->lines = 0;\n \n-\tfor_each_tag_ref(show_reference, &data);\n-\tif (sort) {\n-\t\tint i;\n-\t\tif ((sort & SORT_MASK) == VERCMP_SORT)\n-\t\t\tqsort(array.items, array.nr,\n-\t\t\t      sizeof(struct ref_array_item *), sort_by_version);\n-\t\tif (sort & REVERSE_SORT)\n-\t\t\tfor (i = array.nr - 1; i >= 0; i--)\n-\t\t\t\tprintf(\"%s\\n\", array.items[i]->refname);\n-\t\telse\n-\t\t\tfor (i = 0; i < array.nr; i++)\n-\t\t\t\tprintf(\"%s\\n\", array.items[i]->refname);\n-\t\tref_array_clear(&array);\n-\t}\n+\tif (filter->lines)\n+\t\tformat = \"%(padright:16)%(refname:short)\";\n+\telse\n+\t\tformat = \"%(refname:short)\";\n+\n+\tverify_ref_format(format);\n+\tfilter_refs(&array, filter, FILTER_REFS_TAGS);\n+\tref_array_sort(sorting, &array);\n+\n+\tfor (i = 0; i < array.nr; i++)\n+\t\tshow_ref_array_item(array.items[i], format, QUOTE_NONE, filter->lines);\n+\tref_array_clear(&array);\n+\n \treturn 0;\n }\n \n@@ -366,35 +120,26 @@ static const char tag_template_nocleanup[] =\n \t\"Lines starting with '%c' will be kept; you may remove them\"\n \t\" yourself if you want to.\\n\");\n \n-/*\n- * Parse a sort string, and return 0 if parsed successfully. Will return\n- * non-zero when the sort string does not parse into a known type. If var is\n- * given, the error message becomes a warning and includes information about\n- * the configuration value.\n- */\n-static int parse_sort_string(const char *var, const char *arg, int *sort)\n+/* Parse arg given and add it the ref_sorting array */\n+static int parse_sorting_string(const char *arg, struct ref_sorting **sorting_tail)\n {\n-\tint type = 0, flags = 0;\n-\n-\tif (skip_prefix(arg, \"-\", &arg))\n-\t\tflags |= REVERSE_SORT;\n+\tstruct ref_sorting *s;\n+\tint len;\n \n-\tif (skip_prefix(arg, \"version:\", &arg) || skip_prefix(arg, \"v:\", &arg))\n-\t\ttype = VERCMP_SORT;\n-\telse\n-\t\ttype = STRCMP_SORT;\n+\ts = xcalloc(1, sizeof(*s));\n+\ts->next = *sorting_tail;\n+\t*sorting_tail = s;\n \n-\tif (strcmp(arg, \"refname\")) {\n-\t\tif (!var)\n-\t\t\treturn error(_(\"unsupported sort specification '%s'\"), arg);\n-\t\telse {\n-\t\t\twarning(_(\"unsupported sort specification '%s' in variable '%s'\"),\n-\t\t\t\tvar, arg);\n-\t\t\treturn -1;\n-\t\t}\n+\tif (*arg == '-') {\n+\t\ts->reverse = 1;\n+\t\targ++;\n \t}\n+\tif (skip_prefix(arg, \"version:\", &arg) ||\n+\t    skip_prefix(arg, \"v:\", &arg))\n+\t\ts->version = 1;\n \n-\t*sort = (type | flags);\n+\tlen = strlen(arg);\n+\ts->atom = parse_ref_filter_atom(arg, arg+len);\n \n \treturn 0;\n }\n@@ -402,11 +147,12 @@ static int parse_sort_string(const char *var, const char *arg, int *sort)\n static int git_tag_config(const char *var, const char *value, void *cb)\n {\n \tint status;\n+\tstruct ref_sorting **sorting_tail = (struct ref_sorting **)cb;\n \n \tif (!strcmp(var, \"tag.sort\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n-\t\tparse_sort_string(var, value, &tag_sort);\n+\t\tparse_sorting_string(value, sorting_tail);\n \t\treturn 0;\n \t}\n \n@@ -564,13 +310,6 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)\n \treturn check_refname_format(sb->buf, 0);\n }\n \n-static int parse_opt_sort(const struct option *opt, const char *arg, int unset)\n-{\n-\tint *sort = opt->value;\n-\n-\treturn parse_sort_string(NULL, arg, sort);\n-}\n-\n int cmd_tag(int argc, const char **argv, const char *prefix)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -587,6 +326,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tstruct ref_transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \tstruct ref_filter filter;\n+\tstatic struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE('l', \"list\", &cmdmode, N_(\"list tag names\"), 'l'),\n \t\t{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_(\"n\"),\n@@ -613,10 +353,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\n \t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n \t\tOPT_WITH(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n-\t\t{\n-\t\t\tOPTION_CALLBACK, 0, \"sort\", &tag_sort, N_(\"type\"), N_(\"sort tags\"),\n-\t\t\tPARSE_OPT_NONEG, parse_opt_sort\n-\t\t},\n+\t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n+\t\t\t     N_(\"field name to sort on\"), &parse_opt_ref_sorting),\n \t\t{\n \t\t\tOPTION_CALLBACK, 0, \"points-at\", &filter.points_at, N_(\"object\"),\n \t\t\tN_(\"print only tags of the object\"), 0, parse_opt_object_name\n@@ -624,7 +362,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \n-\tgit_config(git_tag_config, NULL);\n+\tgit_config(git_tag_config, sorting_tail);\n \n \tmemset(&opt, 0, sizeof(opt));\n \tmemset(&filter, 0, sizeof(filter));\n@@ -650,6 +388,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"--column and -n are incompatible\"));\n \t\tcolopts = 0;\n \t}\n+\tif (!sorting)\n+\t\tsorting = ref_default_sorting();\n \tif (cmdmode == 'l') {\n \t\tint ret;\n \t\tif (column_active(colopts)) {\n@@ -658,10 +398,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\tcopts.padding = 2;\n \t\t\trun_column_filter(colopts, &copts);\n \t\t}\n-\t\tif (filter.lines != -1 && tag_sort)\n-\t\t\tdie(_(\"--sort and -n are incompatible\"));\n \t\tfilter.name_patterns = argv;\n-\t\tret = list_tags(&filter, tag_sort);\n+\t\tret = list_tags(&filter, sorting);\n \t\tif (column_active(colopts))\n \t\t\tstop_column_filter();\n \t\treturn ret;\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex d31788c..1f066aa 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1462,13 +1462,7 @@ test_expect_success 'invalid sort parameter on command line' '\n \n test_expect_success 'invalid sort parameter in configuratoin' '\n \tgit config tag.sort \"v:notvalid\" &&\n-\tgit tag -l \"foo*\" >actual &&\n-\tcat >expect <<-\\EOF &&\n-\tfoo1.10\n-\tfoo1.3\n-\tfoo1.6\n-\tEOF\n-\ttest_cmp expect actual\n+\ttest_must_fail git tag -l \"foo*\" >actual\n '\n \n test_expect_success 'version sort with prerelease reordering' '\n-- \n2.4.6\n"},{"id":"266813","messageId":"1437982035-6658-10-git-send-email-Karthik.188@gmail.com","threadId":"39939","inReplyTo":"1437982035-6658-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v5 10/11] tag.c: implement '--format' option","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T07:27:14Z","receivedAt":"2015-07-27T07:27:14Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak <karthik.188@gmail.com>\n\nImplement the '--format' option provided by 'ref-filter'.\nThis lets the user list tags as per desired format similar\nto the implementation in 'git for-each-ref'.\n\nAdd tests and documentation for the same.\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 Documentation/git-tag.txt | 15 ++++++++++++++-\n builtin/tag.c             | 11 +++++++----\n t/t7004-tag.sh            | 16 ++++++++++++++++\n 3 files changed, 37 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 3ac4a96..75703c5 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -13,7 +13,8 @@ SYNOPSIS\n \t<tagname> [<commit> | <object>]\n 'git tag' -d <tagname>...\n 'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]\n-\t[--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>] [<pattern>...]\n+\t[--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]\n+\t[--format=<format>] [<pattern>...]\n 'git tag' -v <tagname>...\n \n DESCRIPTION\n@@ -158,6 +159,18 @@ This option is only applicable when listing tags without annotation lines.\n \tThe object that the new tag will refer to, usually a commit.\n \tDefaults to HEAD.\n \n+<format>::\n+\tA string that interpolates `%(fieldname)` from the object\n+\tpointed at by a ref being shown.  If `fieldname` is prefixed\n+\twith an asterisk (`*`) and the ref points at a tag object, the\n+\tvalue for the field in the object tag refers is used.  When\n+\tunspecified, defaults to `%(refname:short)`.  It also\n+\tinterpolates `%%` to `%`, and `%xx` where `xx` are hex digits\n+\tinterpolates to character with hex code `xx`; for example\n+\t`%00` interpolates to `\\0` (NUL), `%09` to `\\t` (TAB) and\n+\t`%0a` to `\\n` (LF).  The fields are same as those in `git\n+\tfor-each-ref`.\n+\n \n CONFIGURATION\n -------------\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 1e8d1b2..7de49c4 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -30,10 +30,9 @@ static const char * const git_tag_usage[] = {\n \n static unsigned int colopts;\n \n-static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting)\n+static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)\n {\n \tstruct ref_array array;\n-\tchar *format;\n \tint i;\n \n \tmemset(&array, 0, sizeof(array));\n@@ -43,7 +42,7 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting)\n \n \tif (filter->lines)\n \t\tformat = \"%(padright:16)%(refname:short)\";\n-\telse\n+\telse if (!format)\n \t\tformat = \"%(refname:short)\";\n \n \tverify_ref_format(format);\n@@ -327,6 +326,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tstruct strbuf err = STRBUF_INIT;\n \tstruct ref_filter filter;\n \tstatic struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;\n+\tconst char *format = NULL;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE('l', \"list\", &cmdmode, N_(\"list tag names\"), 'l'),\n \t\t{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_(\"n\"),\n@@ -359,6 +359,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\tOPTION_CALLBACK, 0, \"points-at\", &filter.points_at, N_(\"object\"),\n \t\t\tN_(\"print only tags of the object\"), 0, parse_opt_object_name\n \t\t},\n+\t\tOPT_STRING(  0 , \"format\", &format, N_(\"format\"), N_(\"format to use for the output\")),\n \t\tOPT_END()\n \t};\n \n@@ -398,8 +399,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\tcopts.padding = 2;\n \t\t\trun_column_filter(colopts, &copts);\n \t\t}\n+\t\tif (format && (filter.lines != -1))\n+\t\t\tdie(_(\"--format and -n are incompatible\"));\n \t\tfilter.name_patterns = argv;\n-\t\tret = list_tags(&filter, sorting);\n+\t\tret = list_tags(&filter, sorting, format);\n \t\tif (column_active(colopts))\n \t\t\tstop_column_filter();\n \t\treturn ret;\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 1f066aa..1809011 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1519,4 +1519,20 @@ EOF\"\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--format cannot be used with -n' '\n+\ttest_must_fail git tag -l -n4 --format=\"%(refname)\"\n+'\n+\n+test_expect_success '--format should list tags as per format given' '\n+\tcat >expect <<-\\EOF &&\n+\trefname : refs/tags/foo1.10\n+\trefname : refs/tags/foo1.3\n+\trefname : refs/tags/foo1.6\n+\trefname : refs/tags/foo1.6-rc1\n+\trefname : refs/tags/foo1.6-rc2\n+\tEOF\n+\tgit tag -l --format=\"refname : %(refname)\" \"foo*\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.4.6\n"},{"id":"266812","messageId":"1437982035-6658-11-git-send-email-Karthik.188@gmail.com","threadId":"39939","inReplyTo":"1437982035-6658-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v5 11/11] tag.c: implement '--merged' and '--no-merged' options","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T07:27:15Z","receivedAt":"2015-07-27T07:27:15Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak <karthik.188@gmail.com>\n\nUsing 'ref-filter' APIs implement the '--merged' and '--no-merged'\noptions into 'tag.c'. The '--merged' option lets the user to only\nlist tags merged into the named commit. The '--no-merged' option\nlets the user to only list tags not merged into the named commit.\nIf no object is provided it assumes HEAD as the object.\n\nAdd documentation and tests for the same.\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 Documentation/git-tag.txt |  7 ++++++-\n builtin/tag.c             |  6 +++++-\n t/t7004-tag.sh            | 27 +++++++++++++++++++++++++++\n 3 files changed, 38 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 75703c5..c2785d9 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -14,7 +14,7 @@ SYNOPSIS\n 'git tag' -d <tagname>...\n 'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]\n \t[--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]\n-\t[--format=<format>] [<pattern>...]\n+\t[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]\n 'git tag' -v <tagname>...\n \n DESCRIPTION\n@@ -171,6 +171,11 @@ This option is only applicable when listing tags without annotation lines.\n \t`%0a` to `\\n` (LF).  The fields are same as those in `git\n \tfor-each-ref`.\n \n+--[no-]merged [<commit>]::\n+\tOnly list tags whose tips are reachable, or not reachable\n+\tif '--no-merged' is used, from the specified commit ('HEAD'\n+\tif not specified).\n+\n \n CONFIGURATION\n -------------\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 7de49c4..fc01117 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -23,7 +23,7 @@ static const char * const git_tag_usage[] = {\n \tN_(\"git tag [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>] <tagname> [<head>]\"),\n \tN_(\"git tag -d <tagname>...\"),\n \tN_(\"git tag -l [-n[<num>]] [--contains <commit>] [--points-at <object>]\"\n-\t\t\"\\n\\t\\t[<pattern>...]\"),\n+\t\t\"\\n\\t\\t[--[no-]merged [<commit>]] [<pattern>...]\"),\n \tN_(\"git tag -v <tagname>...\"),\n \tNULL\n };\n@@ -353,6 +353,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\n \t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n \t\tOPT_WITH(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n+\t\tOPT_MERGED(&filter, N_(\"print only tags that are merged\")),\n+\t\tOPT_NO_MERGED(&filter, N_(\"print only tags that are not merged\")),\n \t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n \t\t\t     N_(\"field name to sort on\"), &parse_opt_ref_sorting),\n \t\t{\n@@ -413,6 +415,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"--contains option is only allowed with -l.\"));\n \tif (filter.points_at.nr)\n \t\tdie(_(\"--points-at option is only allowed with -l.\"));\n+\tif (filter.merge_commit)\n+\t\tdie(_(\"--merged and --no-merged option are only allowed with -l\"));\n \tif (cmdmode == 'd')\n \t\treturn for_each_tag_name(argv, delete_tag);\n \tif (cmdmode == 'v')\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 1809011..5b73539 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1535,4 +1535,31 @@ test_expect_success '--format should list tags as per format given' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'setup --merged test tags' '\n+\tgit tag mergetest-1 HEAD~2 &&\n+\tgit tag mergetest-2 HEAD~1 &&\n+\tgit tag mergetest-3 HEAD\n+'\n+\n+test_expect_success '--merged cannot be used in non-list mode' '\n+\ttest_must_fail git tag --merged=mergetest-2 foo\n+'\n+\n+test_expect_success '--merged shows merged tags' '\n+\tcat >expect <<-\\EOF &&\n+\tmergetest-1\n+\tmergetest-2\n+\tEOF\n+\tgit tag -l --merged=mergetest-2 mergetest-* >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--no-merged show unmerged tags' '\n+\tcat >expect <<-\\EOF &&\n+\tmergetest-3\n+\tEOF\n+\tgit tag -l --no-merged=mergetest-2 mergetest-* >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.4.6\n"},{"id":"266824","messageId":"vpqmvyhai1k.fsf@anie.imag.fr","threadId":"39939","inReplyTo":"1437982035-6658-1-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v5 01/11] ref-filter: introduce 'ref_formatting_state'","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-27T12:42:15Z","receivedAt":"2015-07-27T12:42:15Z","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 void ref_formatting(struct ref_formatting_state *state,\n> +\t\t\t   struct atom_value *v, struct strbuf *value)\n>  {\n> -\tstruct strbuf sb = STRBUF_INIT;\n> -\tswitch (quote_style) {\n> +\tstrbuf_addf(value, \"%s\", v->s);\n> +}\n\nYou're taking 'state' as argument, but you're not using it in the\nfunction for now. Perhaps add a temporary comment like:\n\nstatic void ref_formatting(...)\n{\n\t/* Formatting according to 'state' will be applied here */\n\tstrbuf_addf(...)\n}\n\nOr perhaps it's OK like this.\n\n> -static void print_value(struct atom_value *v, int quote_style)\n> +static void print_value(struct ref_formatting_state *state, struct atom_value *v)\n\nChanging the position of the v parameter makes the patch a bit harder to\nread. I would have written in this order:\n\nstatic void print_value(struct atom_value *v, struct ref_formatting_state *state)\n\nSo the patch reads as \"encapsulate quote_style in a struct\" more\nstraightforwardly.\n\n> @@ -1257,6 +1269,10 @@ static void emit(const char *cp, const char *ep)\n>  void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)\n>  {\n>  \tconst char *cp, *sp, *ep;\n> +\tstruct ref_formatting_state state;\n\nI still found it a bit hard to read, and I would have appreciated a\ncomment here, like\n\n\t/*\n\t * Some (pseudo) atom have no immediate side effect, but only\n\t * affect the next atom. Store the relevant information from \t\n\t * these atoms in the 'state' variable for use when displaying\n\t * the next atom.\n\t */\n\nWith this in mind, it becomes more obvious that you also need to reset\nthe state after using it, which you forgot to do. See:\n\n$ ./git for-each-ref --format '%(padright:30)|%(refname)|%(refname)|' refs/tags/v2.4.\\*\n|refs/tags/v2.4.0              |refs/tags/v2.4.0              |\n|refs/tags/v2.4.0-rc0          |refs/tags/v2.4.0-rc0          |\n|refs/tags/v2.4.0-rc1          |refs/tags/v2.4.0-rc1          |\n|refs/tags/v2.4.0-rc2          |refs/tags/v2.4.0-rc2          |\n|refs/tags/v2.4.0-rc3          |refs/tags/v2.4.0-rc3          |\n|refs/tags/v2.4.1              |refs/tags/v2.4.1              |\n|refs/tags/v2.4.2              |refs/tags/v2.4.2              |\n|refs/tags/v2.4.3              |refs/tags/v2.4.3              |\n|refs/tags/v2.4.4              |refs/tags/v2.4.4              |\n|refs/tags/v2.4.5              |refs/tags/v2.4.5              |\n|refs/tags/v2.4.6              |refs/tags/v2.4.6              |\n\nI think only the first column should have padding, not the second. You\ncan fix this with a patch like this:\n\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1431,6 +1431,14 @@ static void apply_pseudo_state(struct ref_formatting_state *state,\n                state->ifexists = v->s;\n }\n \n+static void reset_formatting_state(struct ref_formatting_state *state)\n+{\n+       int quote_style = state->quote_style;\n+       memset(state, 0, sizeof(*state));\n+       state->quote_style = quote_style;\n+}\n+\n+\n /*\n  * If 'lines' is greater than 0, print that many lines from the given\n  * object_id 'oid'.\n@@ -1492,8 +1500,11 @@ void show_ref_array_item(struct ref_array_item *info, const char *format,\n                get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);\n                if (atomv->pseudo_atom)\n                        apply_pseudo_state(&state, atomv);\n-               else\n+               else {\n                        print_value(&state, atomv);\n+                       reset_formatting_state(&state);\n+               }\n+\n        }\n        if (*cp) {\n                sp = cp + strlen(cp);\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"266826","messageId":"vpqa8uhaht3.fsf@anie.imag.fr","threadId":"39939","inReplyTo":"1437982035-6658-2-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v5 02/11] ref-filter: make `color` use `ref_formatting_state`","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-27T12:47:20Z","receivedAt":"2015-07-27T12:47:20Z","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> Make the `color` atom a pseudo atom and ensure that it uses\n> `ref_formatting_state`.\n\nActually, I think this is an incorrect change.\n\nPreviously, %(color:...) was taking effect immediately, and after this\npatch, it takes effect starting from the next atom.\n\nTry this:\n\ngit for-each-ref --format '%(color:red)red %(color:reset)normal'\n\nBefore your patch, I get 'red' as red, and 'normal' as normal. After\nyour patch, I get no color at all, since %(color:...) just stores\ninformation that is never used because I have no real atom.\n\n> --- a/ref-filter.h\n> +++ b/ref-filter.h\n> @@ -19,11 +19,13 @@\n>  struct atom_value {\n>  \tconst char *s;\n>  \tunsigned long ul; /* used for sorting when not FIELD_STR */\n> -\tunsigned int pseudo_atom : 1; /*  atoms which aren't placeholders for ref attributes */\n> +\tunsigned int pseudo_atom : 1, /*  atoms which aren't placeholders for ref attributes */\n> +\t\tcolor : 1;\n>  };\n\nAs a consequence of the remark above, I think the name and comment of\nthe field are misleading. There are 3 kinds of atoms:\n\n* Placeholders for ref attributes\n\n* Atoms that take action immediately, but that are not ref attributes\n  like %(color)\n\n* Atoms that only act as modifiers for the next atom. Perhaps they could\n  be called \"prefix atoms\" or \"modifier atoms\".\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"266827","messageId":"vpq3809aho9.fsf@anie.imag.fr","threadId":"39939","inReplyTo":"1437982035-6658-3-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v5 03/11] ref-filter: add option to pad atoms to the right","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-27T12:50:14Z","receivedAt":"2015-07-27T12:50:14Z","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> --- a/t/t6302-for-each-ref-filter.sh\n> +++ b/t/t6302-for-each-ref-filter.sh\n> @@ -81,4 +81,20 @@ test_expect_success 'filtering with --contains' '\n>  \ttest_cmp expect actual\n>  '\n>  \n> +test_expect_success 'padding to the right using `padright`' '\n> +\tcat >expect <<-\\EOF &&\n> +\trefs/heads/master        |\n> +\trefs/heads/side          |\n> +\trefs/odd/spot            |\n> +\trefs/tags/double-tag     |\n> +\trefs/tags/four           |\n> +\trefs/tags/one            |\n> +\trefs/tags/signed-tag     |\n> +\trefs/tags/three          |\n> +\trefs/tags/two            |\n> +\tEOF\n> +\tgit for-each-ref --format=\"%(padright:25)%(refname)|\" >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n>  test_done\n\nSee my remark on previous patch: this test is not sufficient. You do\nnot only need to check that %(padright) is taken into account, but you\nalso need to check that it is taken into account for the right atom and\nonly this one. I'd suggest\n\n--format '%(refname)%(padright:25)|%(refname)|%(refname)|'\n\nOnly the middle column should be padded.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"266829","messageId":"vpqtwsp92wf.fsf@anie.imag.fr","threadId":"39939","inReplyTo":"1437982035-6658-7-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v5 07/11] ref-filter: add option to match literal pattern","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-27T12:54:40Z","receivedAt":"2015-07-27T12:54:40Z","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> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -946,6 +946,32 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)\n\n> +/*\n> + * Return 1 if the refname matches one of the patterns, otherwise 0.\n>   * A pattern can be path prefix (e.g. a refname \"refs/heads/master\"\n>   * matches a pattern \"refs/heads/\") or a wildcard (e.g. the same ref\n>   * matches \"refs/heads/m*\",too).\n\nNit: you used to s/,too/, too/ in the comment in a previous version.\n\nI think I already suggested saying explicitly \"... matches a pattern\n\"refs/heads/\" but not \"refs/heads/m\")\", but I won't insist on that. Just\na reminder in case you missed it.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"266831","messageId":"vpqlhe192d5.fsf@anie.imag.fr","threadId":"39939","inReplyTo":"1437982035-6658-2-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v5 02/11] ref-filter: make `color` use `ref_formatting_state`","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-27T13:06:14Z","receivedAt":"2015-07-27T13:06:14Z","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> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1195,6 +1197,11 @@ void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)\n>  static void ref_formatting(struct ref_formatting_state *state,\n>  \t\t\t   struct atom_value *v, struct strbuf *value)\n>  {\n> +\tif (state->color) {\n> +\t\tstrbuf_addstr(value, state->color);\n> +\t\tfree(state->color);\n> +\t\tstate->color = NULL;\n> +\t}\n>  \tstrbuf_addf(value, \"%s\", v->s);\n>  }\n>  \n> @@ -1266,6 +1273,13 @@ static void emit(const char *cp, const char *ep)\n>  \t}\n>  }\n>  \n> +static void apply_pseudo_state(struct ref_formatting_state *state,\n> +\t\t\t       struct atom_value *v)\n> +{\n> +\tif (v->color)\n> +\t\tstate->color = (char *)v->s;\n> +}\n> +\n>  void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)\n>  {\n>  \tconst char *cp, *sp, *ep;\n\nIt's not clear enough in the code and history that these these two\nfunctions are symmetrical.\n\nYou can find better names. 'apply_pseudo_state' seems wrong it two ways:\nit does not _apply_ the state, but it stores it. And it's a \"pseudo-atom\nrelated state\", but not a \"pseudo-state\".\n\nHow about\n\nref_formatting -> apply_formatting_state\napply_pseudo_state -> store_formatting_state\n\n?\n\nActually, I would even call these functions right from\nshow_ref_array_item, so that the result look like this:\n\n\t\tif (atomv->pseudo_atom)\n\t\t\tstore_formatting_state(&state, atomv);\n\t\telse {\n\t\t\tapply_formatting_state(&state, atomv);\n\t\t\treset_formatting_state(&state);\n\t\t\tprint_value(&state, atomv);\n\t\t}\n\nIn the history, if you are to introduce a dumb version of ref_formatting\nin PATCH 1, I think you should also introduce a dumb (actually, totally\nempty) version of apply_pseudo_state. Then, further patches would just\nadd a few lines in each function, and ...\n\n> @@ -1281,7 +1295,10 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n>  \t\tif (cp < sp)\n>  \t\t\temit(cp, sp);\n>  \t\tget_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);\n> -\t\tprint_value(&state, atomv);\n> +\t\tif (atomv->pseudo_atom)\n> +\t\t\tapply_pseudo_state(&state, atomv);\n> +\t\telse\n> +\t\t\tprint_value(&state, atomv);\n>  \t}\n\n... this hunk would belong to PATCH 1.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"266833","messageId":"xmqq4mkpk73h.fsf@gitster.dls.corp.google.com","threadId":"39939","inReplyTo":"vpqa8uhaht3.fsf@anie.imag.fr","subject":"Re: [PATCH v5 02/11] ref-filter: make `color` use `ref_formatting_state`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-27T14:28:34Z","receivedAt":"2015-07-27T14:28:34Z","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> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> Make the `color` atom a pseudo atom and ensure that it uses\n>> `ref_formatting_state`.\n>\n> Actually, I think this is an incorrect change.\n>\n> Previously, %(color:...) was taking effect immediately, and after this\n> patch, it takes effect starting from the next atom.\n> ...\n> As a consequence of the remark above, I think the name and comment of\n> the field are misleading. There are 3 kinds of atoms:\n>\n> * Placeholders for ref attributes\n>\n> * Atoms that take action immediately, but that are not ref attributes\n>   like %(color)\n>\n> * Atoms that only act as modifiers for the next atom. Perhaps they could\n>   be called \"prefix atoms\" or \"modifier atoms\".\n\nMy fault.\n\nI briefly thought that it may be simpler to treat %(color) just as a\ndifferent way to express a literal that cannot be typed from the\nkeyboard, but your \"different kind of atom\" is a much better way to\nthink about it.\n\nWhat is necessary is that, just like the updated \"print_value()\"\nknows about the formatting state, \"emit()\" needs to be told about\nthe same formatting state.  Some of these \"state affecting\" atoms\nmay take effect on what is output by \"emit()\" (e.g. \"color\" is\nobviously an example, the hypotethical one that counts the current\noutput column and uses that knowledge to \"align\" the output to\ncertain columns, instead of \"right pad to make the next column\n30-columns wide\" one, which is in this series, is another).\n\nThanks for finding this, and Karthik, sorry for an incomplete\nsuggestion based on a faulty thinking.\n"},{"id":"266836","messageId":"CAOLa=ZRRw9RCRTL1UaeDMoc683P370eCPD1VcVhMKhju_jfnNg@mail.gmail.com","threadId":"39939","inReplyTo":"vpqmvyhai1k.fsf@anie.imag.fr","subject":"Re: [PATCH v5 01/11] ref-filter: introduce 'ref_formatting_state'","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T15:28:30Z","receivedAt":"2015-07-27T15:28:30Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Jul 27, 2015 at 6:12 PM, Matthieu Moy\n<Matthieu.Moy@grenoble-inp.fr> wrote:\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> +static void ref_formatting(struct ref_formatting_state *state,\n>> +                        struct atom_value *v, struct strbuf *value)\n>>  {\n>> -     struct strbuf sb = STRBUF_INIT;\n>> -     switch (quote_style) {\n>> +     strbuf_addf(value, \"%s\", v->s);\n>> +}\n>\n> You're taking 'state' as argument, but you're not using it in the\n> function for now. Perhaps add a temporary comment like:\n>\n> static void ref_formatting(...)\n> {\n>         /* Formatting according to 'state' will be applied here */\n>         strbuf_addf(...)\n> }\n>\n> Or perhaps it's OK like this.\n\nI thought it'd be OK since it doesn't have any adverse effect, but I added\nthe comment you suggested nonetheless.\n\n>\n>> -static void print_value(struct atom_value *v, int quote_style)\n>> +static void print_value(struct ref_formatting_state *state, struct atom_value *v)\n>\n> Changing the position of the v parameter makes the patch a bit harder to\n> read. I would have written in this order:\n>\n> static void print_value(struct atom_value *v, struct ref_formatting_state *state)\n>\n> So the patch reads as \"encapsulate quote_style in a struct\" more\n> straightforwardly.\n>\n\nI need to be more careful about things like this, thanks.\n\n>> @@ -1257,6 +1269,10 @@ static void emit(const char *cp, const char *ep)\n>>  void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)\n>>  {\n>>       const char *cp, *sp, *ep;\n>> +     struct ref_formatting_state state;\n>\n> I still found it a bit hard to read, and I would have appreciated a\n> comment here, like\n>\n>         /*\n>          * Some (pseudo) atom have no immediate side effect, but only\n>          * affect the next atom. Store the relevant information from\n>          * these atoms in the 'state' variable for use when displaying\n>          * the next atom.\n>          */\n>\n\nThis seems good, will add this.\n\n> With this in mind, it becomes more obvious that you also need to reset\n> the state after using it, which you forgot to do. See:\n>\n> $ ./git for-each-ref --format '%(padright:30)|%(refname)|%(refname)|' refs/tags/v2.4.\\*\n> |refs/tags/v2.4.0              |refs/tags/v2.4.0              |\n> |refs/tags/v2.4.0-rc0          |refs/tags/v2.4.0-rc0          |\n> |refs/tags/v2.4.0-rc1          |refs/tags/v2.4.0-rc1          |\n> |refs/tags/v2.4.0-rc2          |refs/tags/v2.4.0-rc2          |\n> |refs/tags/v2.4.0-rc3          |refs/tags/v2.4.0-rc3          |\n> |refs/tags/v2.4.1              |refs/tags/v2.4.1              |\n> |refs/tags/v2.4.2              |refs/tags/v2.4.2              |\n> |refs/tags/v2.4.3              |refs/tags/v2.4.3              |\n> |refs/tags/v2.4.4              |refs/tags/v2.4.4              |\n> |refs/tags/v2.4.5              |refs/tags/v2.4.5              |\n> |refs/tags/v2.4.6              |refs/tags/v2.4.6              |\n>\n> I think only the first column should have padding, not the second. You\n> can fix this with a patch like this:\n>\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1431,6 +1431,14 @@ static void apply_pseudo_state(struct ref_formatting_state *state,\n>                 state->ifexists = v->s;\n>  }\n>\n> +static void reset_formatting_state(struct ref_formatting_state *state)\n> +{\n> +       int quote_style = state->quote_style;\n> +       memset(state, 0, sizeof(*state));\n> +       state->quote_style = quote_style;\n> +}\n> +\n> +\n>  /*\n>   * If 'lines' is greater than 0, print that many lines from the given\n>   * object_id 'oid'.\n> @@ -1492,8 +1500,11 @@ void show_ref_array_item(struct ref_array_item *info, const char *format,\n>                 get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);\n>                 if (atomv->pseudo_atom)\n>                         apply_pseudo_state(&state, atomv);\n> -               else\n> +               else {\n>                         print_value(&state, atomv);\n> +                       reset_formatting_state(&state);\n> +               }\n> +\n>         }\n>         if (*cp) {\n>                 sp = cp + strlen(cp);\n>\n\nIf you saw the last patch series, i had them reset individually, I\nmissed that by mistake.\nBut what you're doing seems good, will integrate this. thanks :D\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266837","messageId":"xmqqr3ntioyh.fsf@gitster.dls.corp.google.com","threadId":"39939","inReplyTo":"vpq3809aho9.fsf@anie.imag.fr","subject":"Re: [PATCH v5 03/11] ref-filter: add option to pad atoms to the right","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-27T15:45:42Z","receivedAt":"2015-07-27T15:45:42Z","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> See my remark on previous patch: this test is not sufficient. You do\n> not only need to check that %(padright) is taken into account, but you\n> also need to check that it is taken into account for the right atom and\n> only this one. I'd suggest\n>\n> --format '%(refname)%(padright:25)|%(refname)|%(refname)|'\n>\n> Only the middle column should be padded.\n\nThis actually raises an interesting point.  It is equally valid to\nview that format string as requesting to pad the first \"|\" with 24\nspaces; in other words, %(padright:N) would apply to the next span,\nbe it a literal string up to the next %(atom), or the %(atom) that\ncomes immediately after it.\n\nThe thing is, the above _might_ be an OK way to ask the middle\nrefname to be padded, but I somehow find it very unnatural and\nunintuitive to expect that this:\n\n\t--format '%(padright:25)A Very Long Irrelevancy%(refname)'\n\nwould do nothing to \"A Very Long Irrelevancy\" and affects the\nrefname that comes much later in the format string.\n\nOr we could simply forbid certain atoms followed by an non-empty\nliteral as an error.\n\nMy preference between the three is \"%(padright:4)\", etc. to apply to\nthe \"next span\", but I can live with \"it is an error to pad-right\na far-away %(atom)\".\n"},{"id":"266838","messageId":"vpqk2tl4mvx.fsf@anie.imag.fr","threadId":"39939","inReplyTo":"xmqqr3ntioyh.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 03/11] ref-filter: add option to pad atoms to the right","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-27T15:54:10Z","receivedAt":"2015-07-27T15:54:10Z","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> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>\n>> See my remark on previous patch: this test is not sufficient. You do\n>> not only need to check that %(padright) is taken into account, but you\n>> also need to check that it is taken into account for the right atom and\n>> only this one. I'd suggest\n>>\n>> --format '%(refname)%(padright:25)|%(refname)|%(refname)|'\n>>\n>> Only the middle column should be padded.\n>\n> This actually raises an interesting point.  It is equally valid to\n> view that format string as requesting to pad the first \"|\" with 24\n> spaces; in other words, %(padright:N) would apply to the next span,\n> be it a literal string up to the next %(atom), or the %(atom) that\n> comes immediately after it.\n\nFor an arbitrary %(modifier), I'd agree, but in the particular case of\n%(padright), I think it only makes sense if applied to something of\nvariable length.\n\n> The thing is, the above _might_ be an OK way to ask the middle\n> refname to be padded, but I somehow find it very unnatural and\n> unintuitive to expect that this:\n>\n> \t--format '%(padright:25)A Very Long Irrelevancy%(refname)'\n\nYes, but on the other hand we already have:\n\n  git log --format='%<|(50)A very long irrevlevancy|%an|'\n\nthat pads/truncate %an. So, consistancy would dictate that Karthik's\nversion is the right one.\n\n> My preference between the three is \"%(padright:4)\", etc. to apply to\n> the \"next span\", but I can live with \"it is an error to pad-right\n> a far-away %(atom)\".\n\nI think there are valid argument for the 3 versions. I'm fine with any\nof them, as long as there's a test that shows which one is picked.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"266839","messageId":"CAOLa=ZSXD-pjgYCOt+ZcYS4uHS0p2uBYX8Rw80xpL+9pNUvm+w@mail.gmail.com","threadId":"39939","inReplyTo":"vpqtwsp92wf.fsf@anie.imag.fr","subject":"Re: [PATCH v5 07/11] ref-filter: add option to match literal pattern","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T15:57:29Z","receivedAt":"2015-07-27T15:57:29Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Jul 27, 2015 at 6:24 PM, Matthieu Moy\n<Matthieu.Moy@grenoble-inp.fr> wrote:\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> --- a/ref-filter.c\n>> +++ b/ref-filter.c\n>> @@ -946,6 +946,32 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)\n>\n>> +/*\n>> + * Return 1 if the refname matches one of the patterns, otherwise 0.\n>>   * A pattern can be path prefix (e.g. a refname \"refs/heads/master\"\n>>   * matches a pattern \"refs/heads/\") or a wildcard (e.g. the same ref\n>>   * matches \"refs/heads/m*\",too).\n>\n> Nit: you used to s/,too/, too/ in the comment in a previous version.\n>\n\nThat's carried over from a previous patch, ill change it.\n\n> I think I already suggested saying explicitly \"... matches a pattern\n> \"refs/heads/\" but not \"refs/heads/m\")\", but I won't insist on that. Just\n> a reminder in case you missed it.\n>\n\nSorry, I missed that out. Thanks for reminding.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266840","messageId":"CAOLa=ZT58Fai++91AUH2VcerH5OLkbC3thoGtqVYC70R8gSkaA@mail.gmail.com","threadId":"39939","inReplyTo":"CAOLa=ZSXD-pjgYCOt+ZcYS4uHS0p2uBYX8Rw80xpL+9pNUvm+w@mail.gmail.com","subject":"Re: [PATCH v5 07/11] ref-filter: add option to match literal pattern","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T15:59:37Z","receivedAt":"2015-07-27T15:59:37Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Jul 27, 2015 at 9:27 PM, Karthik Nayak <karthik.188@gmail.com> wrote:\n> On Mon, Jul 27, 2015 at 6:24 PM, Matthieu Moy\n> <Matthieu.Moy@grenoble-inp.fr> wrote:\n>> Karthik Nayak <karthik.188@gmail.com> writes:\n>>\n>>> --- a/ref-filter.c\n>>> +++ b/ref-filter.c\n>>> @@ -946,6 +946,32 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)\n>>\n>>> +/*\n>>> + * Return 1 if the refname matches one of the patterns, otherwise 0.\n>>>   * A pattern can be path prefix (e.g. a refname \"refs/heads/master\"\n>>>   * matches a pattern \"refs/heads/\") or a wildcard (e.g. the same ref\n>>>   * matches \"refs/heads/m*\",too).\n>>\n>> Nit: you used to s/,too/, too/ in the comment in a previous version.\n>>\n>\n> That's carried over from a previous patch, ill change it.\n>\n>> I think I already suggested saying explicitly \"... matches a pattern\n>> \"refs/heads/\" but not \"refs/heads/m\")\", but I won't insist on that. Just\n>> a reminder in case you missed it.\n>>\n>\n> Sorry, I missed that out. Thanks for reminding.\n>\n\nIf I remember right, I didn't change that cause It didn't pertain to\nthis commit.\nI forgot to mention it in your previous mail.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266841","messageId":"CAOLa=ZS2xEegWfoObXub8KMrvjw_7K-Wd9x8E7kKAZtdt8M+=Q@mail.gmail.com","threadId":"39939","inReplyTo":"vpqa8uhaht3.fsf@anie.imag.fr","subject":"Re: [PATCH v5 02/11] ref-filter: make `color` use `ref_formatting_state`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T16:02:21Z","receivedAt":"2015-07-27T16:02:21Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Jul 27, 2015 at 6:17 PM, Matthieu Moy\n<Matthieu.Moy@grenoble-inp.fr> wrote:\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> Make the `color` atom a pseudo atom and ensure that it uses\n>> `ref_formatting_state`.\n>\n> Actually, I think this is an incorrect change.\n>\n> Previously, %(color:...) was taking effect immediately, and after this\n> patch, it takes effect starting from the next atom.\n>\n> Try this:\n>\n> git for-each-ref --format '%(color:red)red %(color:reset)normal'\n>\n> Before your patch, I get 'red' as red, and 'normal' as normal. After\n> your patch, I get no color at all, since %(color:...) just stores\n> information that is never used because I have no real atom.\n>\n>> --- a/ref-filter.h\n>> +++ b/ref-filter.h\n>> @@ -19,11 +19,13 @@\n>>  struct atom_value {\n>>       const char *s;\n>>       unsigned long ul; /* used for sorting when not FIELD_STR */\n>> -     unsigned int pseudo_atom : 1; /*  atoms which aren't placeholders for ref attributes */\n>> +     unsigned int pseudo_atom : 1, /*  atoms which aren't placeholders for ref attributes */\n>> +             color : 1;\n>>  };\n>\n> As a consequence of the remark above, I think the name and comment of\n> the field are misleading. There are 3 kinds of atoms:\n>\n> * Placeholders for ref attributes\n>\n> * Atoms that take action immediately, but that are not ref attributes\n>   like %(color)\n>\n> * Atoms that only act as modifiers for the next atom. Perhaps they could\n>   be called \"prefix atoms\" or \"modifier atoms\".\n>\n\nThats right, Junio and I missed out the second part and overlapped it\nover with the\nthird part. Thanks for seeing it through.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266842","messageId":"vpqd1zd4mbp.fsf@anie.imag.fr","threadId":"39939","inReplyTo":"CAOLa=ZT58Fai++91AUH2VcerH5OLkbC3thoGtqVYC70R8gSkaA@mail.gmail.com","subject":"Re: [PATCH v5 07/11] ref-filter: add option to match literal pattern","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-27T16:06:18Z","receivedAt":"2015-07-27T16:06:18Z","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> On Mon, Jul 27, 2015 at 9:27 PM, Karthik Nayak <karthik.188@gmail.com> wrote:\n>> On Mon, Jul 27, 2015 at 6:24 PM, Matthieu Moy\n>> <Matthieu.Moy@grenoble-inp.fr> wrote:\n>>> Karthik Nayak <karthik.188@gmail.com> writes:\n>>>\n>>>> --- a/ref-filter.c\n>>>> +++ b/ref-filter.c\n>>>> @@ -946,6 +946,32 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)\n>>>\n>>>> +/*\n>>>> + * Return 1 if the refname matches one of the patterns, otherwise 0.\n>>>>   * A pattern can be path prefix (e.g. a refname \"refs/heads/master\"\n>>>>   * matches a pattern \"refs/heads/\") or a wildcard (e.g. the same ref\n>>>>   * matches \"refs/heads/m*\",too).\n>>>\n>>> Nit: you used to s/,too/, too/ in the comment in a previous version.\n>>>\n>>\n>> That's carried over from a previous patch, ill change it.\n>>\n>>> I think I already suggested saying explicitly \"... matches a pattern\n>>> \"refs/heads/\" but not \"refs/heads/m\")\", but I won't insist on that. Just\n>>> a reminder in case you missed it.\n>>>\n>>\n>> Sorry, I missed that out. Thanks for reminding.\n>>\n>\n> If I remember right, I didn't change that cause It didn't pertain to\n> this commit. I forgot to mention it in your previous mail.\n\nThe \"but not \"refs/heads/m\"\" part makes sense in this patch to document\nexplicitly the difference with the other function. But again, it's just\na suggestion, you chose whether to apply it or not.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"266843","messageId":"CAOLa=ZS555Y31Lh70XDgumQv==JFk1rJKmpBWvrBO8CjPuY13g@mail.gmail.com","threadId":"39939","inReplyTo":"vpqd1zd4mbp.fsf@anie.imag.fr","subject":"Re: [PATCH v5 07/11] ref-filter: add option to match literal pattern","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T16:48:23Z","receivedAt":"2015-07-27T16:48:23Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Jul 27, 2015 at 9:36 PM, Matthieu Moy\n<Matthieu.Moy@grenoble-inp.fr> wrote:\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> On Mon, Jul 27, 2015 at 9:27 PM, Karthik Nayak <karthik.188@gmail.com> wrote:\n>>> On Mon, Jul 27, 2015 at 6:24 PM, Matthieu Moy\n>>> <Matthieu.Moy@grenoble-inp.fr> wrote:\n>>>> Karthik Nayak <karthik.188@gmail.com> writes:\n>>>>\n>>>>> --- a/ref-filter.c\n>>>>> +++ b/ref-filter.c\n>>>>> @@ -946,6 +946,32 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)\n>>>>\n>>>>> +/*\n>>>>> + * Return 1 if the refname matches one of the patterns, otherwise 0.\n>>>>>   * A pattern can be path prefix (e.g. a refname \"refs/heads/master\"\n>>>>>   * matches a pattern \"refs/heads/\") or a wildcard (e.g. the same ref\n>>>>>   * matches \"refs/heads/m*\",too).\n>>>>\n>>>> Nit: you used to s/,too/, too/ in the comment in a previous version.\n>>>>\n>>>\n>>> That's carried over from a previous patch, ill change it.\n>>>\n>>>> I think I already suggested saying explicitly \"... matches a pattern\n>>>> \"refs/heads/\" but not \"refs/heads/m\")\", but I won't insist on that. Just\n>>>> a reminder in case you missed it.\n>>>>\n>>>\n>>> Sorry, I missed that out. Thanks for reminding.\n>>>\n>>\n>> If I remember right, I didn't change that cause It didn't pertain to\n>> this commit. I forgot to mention it in your previous mail.\n>\n> The \"but not \"refs/heads/m\"\" part makes sense in this patch to document\n> explicitly the difference with the other function. But again, it's just\n> a suggestion, you chose whether to apply it or not.\n>\n\nI think it's a valid add-on, will include it :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266845","messageId":"CAOLa=ZQ6tO+n+V=iVZQi0+UOPDQvfGSNjxCyOTZCOFic-1wHaw@mail.gmail.com","threadId":"39939","inReplyTo":"vpqlhe192d5.fsf@anie.imag.fr","subject":"Re: [PATCH v5 02/11] ref-filter: make `color` use `ref_formatting_state`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T17:16:40Z","receivedAt":"2015-07-27T17:16:40Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Jul 27, 2015 at 6:36 PM, Matthieu Moy\n<Matthieu.Moy@grenoble-inp.fr> wrote:\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> --- a/ref-filter.c\n>> +++ b/ref-filter.c\n>> @@ -1195,6 +1197,11 @@ void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)\n>>  static void ref_formatting(struct ref_formatting_state *state,\n>>                          struct atom_value *v, struct strbuf *value)\n>>  {\n>> +     if (state->color) {\n>> +             strbuf_addstr(value, state->color);\n>> +             free(state->color);\n>> +             state->color = NULL;\n>> +     }\n>>       strbuf_addf(value, \"%s\", v->s);\n>>  }\n>>\n>> @@ -1266,6 +1273,13 @@ static void emit(const char *cp, const char *ep)\n>>       }\n>>  }\n>>\n>> +static void apply_pseudo_state(struct ref_formatting_state *state,\n>> +                            struct atom_value *v)\n>> +{\n>> +     if (v->color)\n>> +             state->color = (char *)v->s;\n>> +}\n>> +\n>>  void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)\n>>  {\n>>       const char *cp, *sp, *ep;\n>\n> It's not clear enough in the code and history that these these two\n> functions are symmetrical.\n>\n> You can find better names. 'apply_pseudo_state' seems wrong it two ways:\n> it does not _apply_ the state, but it stores it. And it's a \"pseudo-atom\n> related state\", but not a \"pseudo-state\".\n>\n> How about\n>\n> ref_formatting -> apply_formatting_state\n> apply_pseudo_state -> store_formatting_state\n>\n> ?\n\nYes, your suggested naming scheme is better. Ill adopt this.\n\n>\n> Actually, I would even call these functions right from\n> show_ref_array_item, so that the result look like this:\n>\n>                 if (atomv->pseudo_atom)\n>                         store_formatting_state(&state, atomv);\n>                 else {\n>                         apply_formatting_state(&state, atomv);\n>                         reset_formatting_state(&state);\n>                         print_value(&state, atomv);\n>                 }\n\nThis would eliminate that extra strbuf in print_value() wouldn't it,\nbut this would also mean\nthat we replace the value in atomv to hold the new formatted value.\nSounds good to me.\nThanks :)\n\n>\n> In the history, if you are to introduce a dumb version of ref_formatting\n> in PATCH 1, I think you should also introduce a dumb (actually, totally\n> empty) version of apply_pseudo_state. Then, further patches would just\n> add a few lines in each function, and ...\n>\n>> @@ -1281,7 +1295,10 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n>>               if (cp < sp)\n>>                       emit(cp, sp);\n>>               get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);\n>> -             print_value(&state, atomv);\n>> +             if (atomv->pseudo_atom)\n>> +                     apply_pseudo_state(&state, atomv);\n>> +             else\n>> +                     print_value(&state, atomv);\n>>       }\n>\n> ... this hunk would belong to PATCH 1.\n>\n\nI'll add a placeholder for this in the PATCH 1. Thanks :D\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266846","messageId":"CAOLa=ZQABLyaKVXub_dzCZem=aK-pFr5odM4aXaKjnMN6Rk=kQ@mail.gmail.com","threadId":"39939","inReplyTo":"xmqq4mkpk73h.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 02/11] ref-filter: make `color` use `ref_formatting_state`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T17:23:06Z","receivedAt":"2015-07-27T17:23:06Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Jul 27, 2015 at 7:58 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>\n>> Karthik Nayak <karthik.188@gmail.com> writes:\n>>\n>>> Make the `color` atom a pseudo atom and ensure that it uses\n>>> `ref_formatting_state`.\n>>\n>> Actually, I think this is an incorrect change.\n>>\n>> Previously, %(color:...) was taking effect immediately, and after this\n>> patch, it takes effect starting from the next atom.\n>> ...\n>> As a consequence of the remark above, I think the name and comment of\n>> the field are misleading. There are 3 kinds of atoms:\n>>\n>> * Placeholders for ref attributes\n>>\n>> * Atoms that take action immediately, but that are not ref attributes\n>>   like %(color)\n>>\n>> * Atoms that only act as modifiers for the next atom. Perhaps they could\n>>   be called \"prefix atoms\" or \"modifier atoms\".\n>\n> My fault.\n>\n> I briefly thought that it may be simpler to treat %(color) just as a\n> different way to express a literal that cannot be typed from the\n> keyboard, but your \"different kind of atom\" is a much better way to\n> think about it.\n\nYes even I agree with this.\n\n>\n> What is necessary is that, just like the updated \"print_value()\"\n> knows about the formatting state, \"emit()\" needs to be told about\n> the same formatting state.  Some of these \"state affecting\" atoms\n> may take effect on what is output by \"emit()\" (e.g. \"color\" is\n> obviously an example, the hypotethical one that counts the current\n> output column and uses that knowledge to \"align\" the output to\n> certain columns, instead of \"right pad to make the next column\n> 30-columns wide\" one, which is in this series, is another).\n\nThis depends on whether these modifier atoms should effect only succeeding\natoms or any string. Since there is a conversation about this topic on the other\npatch, let's continue this discussion there.\n\n>\n> Thanks for finding this, and Karthik, sorry for an incomplete\n> suggestion based on a faulty thinking.\n\nI don't see why you have to be sorry, you were trying to help :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266848","messageId":"CAOLa=ZRVMnqxb-E29f_53igzJDTWNU0yjVACSq1CK8TCbaM-9w@mail.gmail.com","threadId":"39939","inReplyTo":"vpqk2tl4mvx.fsf@anie.imag.fr","subject":"Re: [PATCH v5 03/11] ref-filter: add option to pad atoms to the right","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T18:42:24Z","receivedAt":"2015-07-27T18:42:24Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Jul 27, 2015 at 9:24 PM, Matthieu Moy\n<Matthieu.Moy@grenoble-inp.fr> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>>\n>>> See my remark on previous patch: this test is not sufficient. You do\n>>> not only need to check that %(padright) is taken into account, but you\n>>> also need to check that it is taken into account for the right atom and\n>>> only this one. I'd suggest\n>>>\n>>> --format '%(refname)%(padright:25)|%(refname)|%(refname)|'\n>>>\n>>> Only the middle column should be padded.\n>>\n>> This actually raises an interesting point.  It is equally valid to\n>> view that format string as requesting to pad the first \"|\" with 24\n>> spaces; in other words, %(padright:N) would apply to the next span,\n>> be it a literal string up to the next %(atom), or the %(atom) that\n>> comes immediately after it.\n>\n> For an arbitrary %(modifier), I'd agree, but in the particular case of\n> %(padright), I think it only makes sense if applied to something of\n> variable length\n>\n\nI agree with Matthieu here, Other than atom values, any user defined string\nwould be of known size, hence padding for such things would make no sense.\n\n>> The thing is, the above _might_ be an OK way to ask the middle\n>> refname to be padded, but I somehow find it very unnatural and\n>> unintuitive to expect that this:\n>>\n>>       --format '%(padright:25)A Very Long Irrelevancy%(refname)'\n>\n> Yes, but on the other hand we already have:\n>\n>   git log --format='%<|(50)A very long irrevlevancy|%an|'\n>\n> that pads/truncate %an. So, consistancy would dictate that Karthik's\n> version is the right one.\n\nSorry but I didn't understand what you're trying to say here, Matthieu.\n\n>\n>> My preference between the three is \"%(padright:4)\", etc. to apply to\n>> the \"next span\", but I can live with \"it is an error to pad-right\n>> a far-away %(atom)\".\n>\n> I think there are valid argument for the 3 versions. I'm fine with any\n> of them, as long as there's a test that shows which one is picked.\n>\n\nMakes sense, thanks both of you :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266849","messageId":"CAOLa=ZR05e-sx51ne==2+t+rS6mGxXAX44Vd2vHCvBYMM16K3A@mail.gmail.com","threadId":"39939","inReplyTo":"vpq3809aho9.fsf@anie.imag.fr","subject":"Re: [PATCH v5 03/11] ref-filter: add option to pad atoms to the right","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T18:43:57Z","receivedAt":"2015-07-27T18:43:57Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Jul 27, 2015 at 6:20 PM, Matthieu Moy\n<Matthieu.Moy@grenoble-inp.fr> wrote:\n> u\n> also need to check that it is taken into account for the right atom and\n> only this one. I'd suggest\n>\n> --format '%(refname)%(padright:25)|%(refname)|%(refname)|'\n\nI guess this is more accurate, Thanks.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266850","messageId":"vpqwpxl30aj.fsf@anie.imag.fr","threadId":"39939","inReplyTo":"CAOLa=ZRVMnqxb-E29f_53igzJDTWNU0yjVACSq1CK8TCbaM-9w@mail.gmail.com","subject":"Re: [PATCH v5 03/11] ref-filter: add option to pad atoms to the right","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-27T18:47:32Z","receivedAt":"2015-07-27T18:47:32Z","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> On Mon, Jul 27, 2015 at 9:24 PM, Matthieu Moy\n> <Matthieu.Moy@grenoble-inp.fr> wrote:\n>> Yes, but on the other hand we already have:\n>>\n>>   git log --format='%<|(50)A very long irrevlevancy|%an|'\n>>\n>> that pads/truncate %an. So, consistancy would dictate that Karthik's\n>> version is the right one.\n>\n> Sorry but I didn't understand what you're trying to say here, Matthieu.\n\nThe \"git log\" equivalent of %(padright:N) is %<|(N), and it behaves the\nsame way as your current implementation of %(padright) (except for the\nmissing reset in your v5).\n\nSo, if we want to be consistant with \"git log\", we should keep the\n\"apply to next atom, even if it's far away in the format string\"\nsemantics.\n\nNote that consistancy is not the only criterion for choice, so I'm not\nsaying we should absolutely do this, just that there's an argument in\nfavor of it.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"266851","messageId":"CAOLa=ZT9_0ZZQwKwObgndLezcOWOk3LBTjAM7mMU2cyXu7--MQ@mail.gmail.com","threadId":"39939","inReplyTo":"vpqwpxl30aj.fsf@anie.imag.fr","subject":"Re: [PATCH v5 03/11] ref-filter: add option to pad atoms to the right","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T18:54:33Z","receivedAt":"2015-07-27T18:54:33Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Tue, Jul 28, 2015 at 12:17 AM, Matthieu Moy\n<Matthieu.Moy@grenoble-inp.fr> wrote:\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> On Mon, Jul 27, 2015 at 9:24 PM, Matthieu Moy\n>> <Matthieu.Moy@grenoble-inp.fr> wrote:\n>>> Yes, but on the other hand we already have:\n>>>\n>>>   git log --format='%<|(50)A very long irrevlevancy|%an|'\n>>>\n>>> that pads/truncate %an. So, consistancy would dictate that Karthik's\n>>> version is the right one.\n>>\n>> Sorry but I didn't understand what you're trying to say here, Matthieu.\n>\n> The \"git log\" equivalent of %(padright:N) is %<|(N), and it behaves the\n> same way as your current implementation of %(padright) (except for the\n> missing reset in your v5).\n>\n> So, if we want to be consistant with \"git log\", we should keep the\n> \"apply to next atom, even if it's far away in the format string\"\n> semantics.\n>\n> Note that consistancy is not the only criterion for choice, so I'm not\n> saying we should absolutely do this, just that there's an argument in\n> favor of it.\n>\n\nI didn't know that, thanks, I think I'll let Junio make the call here :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266852","messageId":"xmqqmvyhig4n.fsf@gitster.dls.corp.google.com","threadId":"39939","inReplyTo":"vpqk2tl4mvx.fsf@anie.imag.fr","subject":"Re: [PATCH v5 03/11] ref-filter: add option to pad atoms to the right","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-27T18:56:24Z","receivedAt":"2015-07-27T18:56:24Z","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> Yes, but on the other hand we already have:\n>\n>   git log --format='%<|(50)A very long irrevlevancy|%an|'\n>\n> that pads/truncate %an. So, consistancy would dictate that Karthik's\n> version is the right one.\n\nInteresting.  Although that %<(50) looks simply a bug to me which we\nmay want to fix.\n\nI wonder if there is a good reason why the above should not be\nspelled as --format=\"A very long irrelevancy%<(50)%an\".\n"},{"id":"267006","messageId":"CAPig+cQ4PmKc0J1_X2vxn+yJ=pVd5AjM3j7j15T8ojrTPsEY_Q@mail.gmail.com","threadId":"39939","inReplyTo":"1437982035-6658-3-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v5 03/11] ref-filter: add option to pad atoms to the right","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-28T21:41:34Z","receivedAt":"2015-07-28T21:41:34Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jul 27, 2015 at 3:27 AM, Karthik Nayak <karthik.188@gmail.com> wrote:\n> Add a new atom \"padright\" and support %(padright:X) where X is a\n> number.  This will align the succeeding atom value to the left\n> followed by spaces for a total length of X characters. If X is less\n> than the item size, the entire atom value is printed.\n>\n> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n> ---\n> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> index e49d578..45dd7f8 100644\n> --- a/Documentation/git-for-each-ref.txt\n> +++ b/Documentation/git-for-each-ref.txt\n> @@ -127,6 +127,12 @@ color::\n>         Change output color.  Followed by `:<colorname>`, where names\n>         are described in `color.branch.*`.\n>\n> +padright::\n> +       Pad succeeding atom to the right. Followed by `:<value>`,\n> +       where `value` states the total length of atom including the\n> +       padding. If the `value` is greater than the atom length, then\n> +       no padding is performed.\n\nIsn't this backward? Don't you mean\n\n    ... If the atom length is greater than `value`, then\n    no padding is performed.\n\n?\n"},{"id":"267008","messageId":"CAPig+cSiCR3OdYf=4BnaiS8PuSKxd7OPPxOUQYyZpBjkQ15LQQ@mail.gmail.com","threadId":"39939","inReplyTo":"1437982035-6658-7-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v5 07/11] ref-filter: add option to match literal pattern","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-28T21:49:30Z","receivedAt":"2015-07-28T21:49:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jul 27, 2015 at 3:27 AM, Karthik Nayak <karthik.188@gmail.com> wrote:\n> From: Karthik Nayak <karthik.188@gmail.com>\n>\n> Since 'ref-filter' only has an option to match path names add an\n> option for plain fnmatch pattern-matching.\n>\n> This is to support the pattern matching options which are used in `git\n> tag -l` and `git branch -l` where we can match patterns like `git tag\n> -l foo*` which would match all tags which has a \"foo*\" pattern.\n>\n> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n> ---\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 26eb26c..597b189 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -946,6 +946,32 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)\n>\n>  /*\n>   * Return 1 if the refname matches one of the patterns, otherwise 0.\n> + * A pattern can be a literal prefix (e.g. a refname \"refs/heads/master\"\n> + * matches a pattern \"refs/heads/mas\") or a wildcard (e.g. the same ref\n> + * matches \"refs/heads/mas*\", too).\n> + */\n> +static int match_pattern(const char **patterns, const char *refname)\n> +{\n> +       /*\n> +        * When no '--format' option is given we need to skip the prefix\n> +        * for matching refs of tags and branches.\n> +        */\n> +       if (skip_prefix(refname, \"refs/tags/\", &refname))\n> +               ;\n> +       else if (skip_prefix(refname, \"refs/heads/\", &refname))\n> +               ;\n> +       else if (skip_prefix(refname, \"refs/remotes/\", &refname))\n> +               ;\n\nOr, more concisely:\n\n    skip_prefix(refname, \"refs/tags/\", &refname) ||\n    skip_prefix(refname, \"refs/heads/\", &refname) ||\n    skip_prefix(refname, \"refs/remotes/\", &refname);\n\nsince || short-circuits. No need for the 'if' or cascading 'else if's.\n\n> +       for (; *patterns; patterns++) {\n> +               if (!wildmatch(*patterns, refname, 0, NULL))\n> +                       return 1;\n> +       }\n> +       return 0;\n> +}\n"},{"id":"267068","messageId":"CAOLa=ZSLkfO7enWEouGyRuRNb+f47vYnR-QM1+50yBdqqtFHFw@mail.gmail.com","threadId":"39939","inReplyTo":"CAPig+cQ4PmKc0J1_X2vxn+yJ=pVd5AjM3j7j15T8ojrTPsEY_Q@mail.gmail.com","subject":"Re: [PATCH v5 03/11] ref-filter: add option to pad atoms to the right","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-29T16:16:40Z","receivedAt":"2015-07-29T16:16:40Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Wed, Jul 29, 2015 at 3:11 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Mon, Jul 27, 2015 at 3:27 AM, Karthik Nayak <karthik.188@gmail.com> wrote:\n>> Add a new atom \"padright\" and support %(padright:X) where X is a\n>> number.  This will align the succeeding atom value to the left\n>> followed by spaces for a total length of X characters. If X is less\n>> than the item size, the entire atom value is printed.\n>>\n>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n>> ---\n>> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n>> index e49d578..45dd7f8 100644\n>> --- a/Documentation/git-for-each-ref.txt\n>> +++ b/Documentation/git-for-each-ref.txt\n>> @@ -127,6 +127,12 @@ color::\n>>         Change output color.  Followed by `:<colorname>`, where names\n>>         are described in `color.branch.*`.\n>>\n>> +padright::\n>> +       Pad succeeding atom to the right. Followed by `:<value>`,\n>> +       where `value` states the total length of atom including the\n>> +       padding. If the `value` is greater than the atom length, then\n>> +       no padding is performed.\n>\n> Isn't this backward? Don't you mean\n>\n>     ... If the atom length is greater than `value`, then\n>     no padding is performed.\n>\n> ?\n\nYes, silly mistake.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"267069","messageId":"CAOLa=ZS0jV-8-BROSjPwZZHe9gVSa-tgs+79Bxdqb1pETSVFdw@mail.gmail.com","threadId":"39939","inReplyTo":"CAPig+cSiCR3OdYf=4BnaiS8PuSKxd7OPPxOUQYyZpBjkQ15LQQ@mail.gmail.com","subject":"Re: [PATCH v5 07/11] ref-filter: add option to match literal pattern","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-29T16:17:14Z","receivedAt":"2015-07-29T16:17:14Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Wed, Jul 29, 2015 at 3:19 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Mon, Jul 27, 2015 at 3:27 AM, Karthik Nayak <karthik.188@gmail.com> wrote:\n>> From: Karthik Nayak <karthik.188@gmail.com>\n>>\n>> Since 'ref-filter' only has an option to match path names add an\n>> option for plain fnmatch pattern-matching.\n>>\n>> This is to support the pattern matching options which are used in `git\n>> tag -l` and `git branch -l` where we can match patterns like `git tag\n>> -l foo*` which would match all tags which has a \"foo*\" pattern.\n>>\n>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n>> ---\n>> diff --git a/ref-filter.c b/ref-filter.c\n>> index 26eb26c..597b189 100644\n>> --- a/ref-filter.c\n>> +++ b/ref-filter.c\n>> @@ -946,6 +946,32 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)\n>>\n>>  /*\n>>   * Return 1 if the refname matches one of the patterns, otherwise 0.\n>> + * A pattern can be a literal prefix (e.g. a refname \"refs/heads/master\"\n>> + * matches a pattern \"refs/heads/mas\") or a wildcard (e.g. the same ref\n>> + * matches \"refs/heads/mas*\", too).\n>> + */\n>> +static int match_pattern(const char **patterns, const char *refname)\n>> +{\n>> +       /*\n>> +        * When no '--format' option is given we need to skip the prefix\n>> +        * for matching refs of tags and branches.\n>> +        */\n>> +       if (skip_prefix(refname, \"refs/tags/\", &refname))\n>> +               ;\n>> +       else if (skip_prefix(refname, \"refs/heads/\", &refname))\n>> +               ;\n>> +       else if (skip_prefix(refname, \"refs/remotes/\", &refname))\n>> +               ;\n>\n> Or, more concisely:\n>\n>     skip_prefix(refname, \"refs/tags/\", &refname) ||\n>     skip_prefix(refname, \"refs/heads/\", &refname) ||\n>     skip_prefix(refname, \"refs/remotes/\", &refname);\n>\n> since || short-circuits. No need for the 'if' or cascading 'else if's.\n>\n>> +       for (; *patterns; patterns++) {\n>> +               if (!wildmatch(*patterns, refname, 0, NULL))\n>> +                       return 1;\n>> +       }\n>> +       return 0;\n>> +}\n\nLooks better thanks :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"267133","messageId":"CAOLa=ZRsX-xxJyD1OZ47wzZg2qbZWvvR6qpvfp_M-xtqLBZRsw@mail.gmail.com","threadId":"39939","inReplyTo":"CAPig+cSiCR3OdYf=4BnaiS8PuSKxd7OPPxOUQYyZpBjkQ15LQQ@mail.gmail.com","subject":"Re: [PATCH v5 07/11] ref-filter: add option to match literal pattern","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-30T11:21:21Z","receivedAt":"2015-07-30T11:21:21Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Wed, Jul 29, 2015 at 3:19 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Mon, Jul 27, 2015 at 3:27 AM, Karthik Nayak <karthik.188@gmail.com> wrote:\n>> From: Karthik Nayak <karthik.188@gmail.com>\n>>\n>> Since 'ref-filter' only has an option to match path names add an\n>> option for plain fnmatch pattern-matching.\n>>\n>> This is to support the pattern matching options which are used in `git\n>> tag -l` and `git branch -l` where we can match patterns like `git tag\n>> -l foo*` which would match all tags which has a \"foo*\" pattern.\n>>\n>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n>> ---\n>> diff --git a/ref-filter.c b/ref-filter.c\n>> index 26eb26c..597b189 100644\n>> --- a/ref-filter.c\n>> +++ b/ref-filter.c\n>> @@ -946,6 +946,32 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)\n>>\n>>  /*\n>>   * Return 1 if the refname matches one of the patterns, otherwise 0.\n>> + * A pattern can be a literal prefix (e.g. a refname \"refs/heads/master\"\n>> + * matches a pattern \"refs/heads/mas\") or a wildcard (e.g. the same ref\n>> + * matches \"refs/heads/mas*\", too).\n>> + */\n>> +static int match_pattern(const char **patterns, const char *refname)\n>> +{\n>> +       /*\n>> +        * When no '--format' option is given we need to skip the prefix\n>> +        * for matching refs of tags and branches.\n>> +        */\n>> +       if (skip_prefix(refname, \"refs/tags/\", &refname))\n>> +               ;\n>> +       else if (skip_prefix(refname, \"refs/heads/\", &refname))\n>> +               ;\n>> +       else if (skip_prefix(refname, \"refs/remotes/\", &refname))\n>> +               ;\n>\n> Or, more concisely:\n>\n>     skip_prefix(refname, \"refs/tags/\", &refname) ||\n>     skip_prefix(refname, \"refs/heads/\", &refname) ||\n>     skip_prefix(refname, \"refs/remotes/\", &refname);\n>\n\nGives a \"warning: value computed is not used [-Wunused-value]\"\n\nso I typecasted the output as:\n\n      (void)(skip_prefix(refname, \"refs/tags/\", &refname) ||\n                skip_prefix(refname, \"refs/heads/\", &refname) ||\n                skip_prefix(refname, \"refs/remotes/\", &refname));\n\nJust wondering if that's alright.\n\n-- \nRegards,\nKarthik Nayak\n"}]}