{"thread":{"id":"39934","subject":"[PATCH v4 0/10] port tag.c to use ref-filter APIs","startedAt":"2015-07-24T19:04:35Z","lastAt":"2015-07-28T10:35:44Z","messageCount":36,"participants":["Karthik Nayak","Junio C Hamano","Eric Sunshine","Duy Nguyen","Jacob Keller","Matthieu Moy"],"isPatch":true,"patchVersion":4,"patchTotal":10},"messages":[{"id":"266737","messageId":"1437764685-8633-1-git-send-email-Karthik.188@gmail.com","threadId":"39934","inReplyTo":null,"subject":"[PATCH v4 0/10] port tag.c to use ref-filter APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-24T19:04:35Z","receivedAt":"2015-07-24T19:04:35Z","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 3 can be found here\nhttp://thread.gmane.org/gmane.comp.version-control.git/274168\n\nChanges from v3:\n\n[01/10]: Changed the whole implementation as per Junio's Suggestion.\nAlso fixed parts where the code was wrong, as per Matthieu's and\nEric's suggestion.\n\n[04/10]: Small changes in comments and line wrapping.\n\n[05/10]: Include more tests, better comments and refactored code.\n\n[06/10]: Improvement in comments.\n\n[09/10]: Change the tests to not fallback to default values.\nIntroduce \"for_each_tag_ref_fullpath()\" which iterates through tags\nwithout stripping the same.\n\n[10/10]: Improvements in comments.\n\nSet of commits:\n\n[PATCH v4 01/10] ref-filter: add option to align atoms to the left\n[PATCH v4 02/10] ref-filter: make the 'color' use\n[PATCH v4 03/10] ref-filter: add option to filter only tags\n[PATCH v4 04/10] ref-filter: support printing N lines from tag\n[PATCH v4 05/10] ref-filter: add support to sort by version\n[PATCH v4 06/10] ref-filter: add option to match literal pattern\n[PATCH v4 07/10] tag.c: use 'ref-filter' data structures\n[PATCH v4 08/10] tag.c: use 'ref-filter' APIs\n[PATCH v4 09/10] tag.c: implement '--format' option\n[PATCH v4 10/10] tag.c: implement '--merged' and '--no-merged'\n"},{"id":"266744","messageId":"1437764685-8633-2-git-send-email-Karthik.188@gmail.com","threadId":"39934","inReplyTo":"1437764685-8633-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v4 01/10] ref-filter: add option to align atoms to the left","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-24T19:04:36Z","receivedAt":"2015-07-24T19:04:36Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nAdd a new atom \"align\" and support %(align:X) where X is a number.\nThis will align the preceeding atom value to the left followed by\nspaces for a total length of X characters. If X is less than the item\nsize, the entire atom value is printed.\n\nHelped-by: Duy Nguyen \nHelped-by: Junio C Hamano \nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n ref-filter.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++--------------\n ref-filter.h | 6 ++++\n 2 files changed, 77 insertions(+), 22 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 7561727..3c90ffc 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 \t{ \"flag\" },\n \t{ \"HEAD\" },\n \t{ \"color\" },\n+\t{ \"align\" },\n };\n \n /*\n@@ -597,7 +600,8 @@ 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_array_item *ref)\n+static void populate_value(struct ref_formatting_state *state,\n+\t\t\t struct ref_array_item *ref)\n {\n \tvoid *buf;\n \tstruct object *obj;\n@@ -620,7 +624,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@@ -687,6 +691,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, \"align:\")) {\n+\t\t\tconst char *valp = NULL;\n+\n+\t\t\tskip_prefix(name, \"align:\", &valp);\n+\t\t\tif (!valp[0])\n+\t\t\t\tdie(_(\"no value given with 'align:'\"));\n+\t\t\tif (strtoul_ui(valp, 10, &state->pad_to_right))\n+\t\t\t\tdie(_(\"positive integer expected after ':' in align:%u\\n\"),\n+\t\t\t\t state->pad_to_right);\n+\t\t\tv->pseudo_atom = 1;\n+\t\t\tv->s = \"\";\n+\t\t\tcontinue;\n \t\t} else\n \t\t\tcontinue;\n \n@@ -809,10 +825,15 @@ static void populate_value(struct ref_array_item *ref)\n * Given a ref, return the value for the atom. This lazily gets value\n * out of the object by calling populate value.\n */\n-static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v)\n+static void get_ref_atom_value(struct ref_formatting_state *state,\n+\t\t\t struct ref_array_item *ref, int atom, struct atom_value **v)\n {\n-\tif (!ref->value) {\n-\t\tpopulate_value(ref);\n+\t/*\n+\t * If the atom is a pseudo_atom then we re-populate the value\n+\t * into the ref_formatting_state stucture.\n+\t */\n+\tif (!ref->value || ref->value[atom].pseudo_atom) {\n+\t\tpopulate_value(state, ref);\n \t\tfill_missing_values(ref->value);\n \t}\n \t*v = &ref->value[atom];\n@@ -1150,9 +1171,12 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru\n \tstruct atom_value *va, *vb;\n \tint cmp;\n \tcmp_type cmp_type = used_atom_type[s->atom];\n+\tstruct ref_formatting_state state;\n \n-\tget_ref_atom_value(a, s->atom, &va);\n-\tget_ref_atom_value(b, s->atom, &vb);\n+\tmemset(&state, 0, sizeof(state));\n+\n+\tget_ref_atom_value(&state, a, s->atom, &va);\n+\tget_ref_atom_value(&state, b, s->atom, &vb);\n \tswitch (cmp_type) {\n \tcase FIELD_STR:\n \t\tcmp = strcmp(va->s, vb->s);\n@@ -1190,30 +1214,50 @@ 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, struct atom_value *v,\n+\t\t\t struct strbuf *value)\n {\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tswitch (quote_style) {\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 len = strlen(v->s) - utf8_strwidth(v->s);\n+\t\t\tstrbuf_addf(value, \"%-*s\", state->pad_to_right + len, v->s);\n+\t\t}\n+\t} else\n+\t\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+\tif (v->pseudo_atom)\n+\t\treturn;\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 +1301,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@@ -1264,8 +1312,9 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n \t\tep = strchr(sp, ')');\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\tget_ref_atom_value(&state, info,\n+\t\t\t\t parse_ref_filter_atom(sp + 2, ep), &atomv);\n+\t\tprint_value(&state, atomv);\n \t}\n \tif (*cp) {\n \t\tsp = cp + strlen(cp);\n@@ -1278,7 +1327,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..ea2d0e6 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -19,6 +19,7 @@\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_sorting {\n@@ -27,6 +28,11 @@ struct ref_sorting {\n \tunsigned reverse : 1;\n };\n \n+struct ref_formatting_state {\n+\tunsigned int pad_to_right; /*pad atoms to the right*/\n+\tint quote_style;\n+};\n+\n struct ref_array_item {\n \tunsigned char objectname[20];\n \tint flag;\n-- \n2.4.6\n"},{"id":"266738","messageId":"1437764685-8633-3-git-send-email-Karthik.188@gmail.com","threadId":"39934","inReplyTo":"1437764685-8633-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v4 02/10] ref-filter: make the 'color' use ref_formatting_state","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-24T19:04:37Z","receivedAt":"2015-07-24T19:04:37Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Make color which was considered as an atom, to use\nref_formatting_state and act as a pseudo atom. This allows\ninterchangeability between 'align' and 'color'.\n\nHelped-by: Junio C Hamano \nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n ref-filter.c | 16 ++++++++++++----\n ref-filter.h | 1 +\n 2 files changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 3c90ffc..fd13a23 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -663,7 +663,8 @@ static void populate_value(struct ref_formatting_state *state,\n \n \t\t\tif (color_parse(name + 6, color) < 0)\n \t\t\t\tdie(_(\"unable to parse format\"));\n-\t\t\tv->s = xstrdup(color);\n+\t\t\tstate->color = xstrdup(color);\n+\t\t\tv->pseudo_atom = 1;\n \t\t\tcontinue;\n \t\t} else if (!strcmp(name, \"flag\")) {\n \t\t\tchar buf[256], *cp = buf;\n@@ -1217,6 +1218,11 @@ void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)\n static void ref_formatting(struct ref_formatting_state *state, struct atom_value *v,\n \t\t\t struct strbuf *value)\n {\n+\tif (state->color) {\n+\t\tstrbuf_addstr(value, state->color);\n+\t\tfree((void *)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@@ -1224,8 +1230,9 @@ static void ref_formatting(struct ref_formatting_state *state, struct atom_value\n \t\t\tint len = strlen(v->s) - utf8_strwidth(v->s);\n \t\t\tstrbuf_addf(value, \"%-*s\", state->pad_to_right + len, v->s);\n \t\t}\n-\t} else\n-\t\tstrbuf_addf(value, \"%s\", v->s);\n+\t\treturn;\n+\t}\n+\tstrbuf_addf(value, \"%s\", v->s);\n }\n \n static void print_value(struct ref_formatting_state *state, struct atom_value *v)\n@@ -1326,7 +1333,8 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n \n \t\tif (color_parse(\"reset\", color) < 0)\n \t\t\tdie(\"BUG: couldn't parse 'reset' as a color\");\n-\t\tresetv.s = color;\n+\t\tresetv.s = \"\";\n+\t\tstate.color = xstrdup(color);\n \t\tprint_value(&state, &resetv);\n \t}\n \tputchar('\\n');\ndiff --git a/ref-filter.h b/ref-filter.h\nindex ea2d0e6..bacbb23 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -31,6 +31,7 @@ struct ref_sorting {\n struct ref_formatting_state {\n \tunsigned int pad_to_right; /*pad atoms to the right*/\n \tint quote_style;\n+\tconst char *color;\n };\n \n struct ref_array_item {\n-- \n2.4.6\n"},{"id":"266739","messageId":"1437764685-8633-4-git-send-email-Karthik.188@gmail.com","threadId":"39934","inReplyTo":"1437764685-8633-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v4 03/10] ref-filter: add option to filter only tags","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-24T19:04:38Z","receivedAt":"2015-07-24T19:04:38Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \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 \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \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 fd13a23..34e6603 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1157,6 +1157,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 bacbb23..729dece 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 ce8cd8d..b679fce 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2103,6 +2103,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 e82fca5..e5d53d3 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":"266742","messageId":"1437764685-8633-5-git-send-email-Karthik.188@gmail.com","threadId":"39934","inReplyTo":"1437764685-8633-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v4 04/10] ref-filter: support printing N lines from tag annotation","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-24T19:04:39Z","receivedAt":"2015-07-24T19:04:39Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nIn 'tag.c' we can print N lines from the annotation of the tag using\nthe '-n' 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 \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n builtin/for-each-ref.c | 2 +-\n builtin/tag.c | 4 +\n ref-filter.c | 51 ++++++++-\n ref-filter.h | 9 +-\n ...ter-add-option-to-align-atoms-to-the-left.patch | 124 +++++++++++++++++++++\n 5 files changed, 186 insertions(+), 4 deletions(-)\n create mode 100644 v3-0001-ref-filter-add-option-to-align-atoms-to-the-left.patch\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 071d001..10b11ce 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 34e6603..08ecce5 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1307,7 +1307,51 @@ static void emit(const char *cp, const char *ep)\n \t}\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\tstate.color = xstrdup(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 729dece..1e2ee65 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -62,6 +62,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@@ -93,8 +94,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 */\ndiff --git a/v3-0001-ref-filter-add-option-to-align-atoms-to-the-left.patch b/v3-0001-ref-filter-add-option-to-align-atoms-to-the-left.patch\nnew file mode 100644\nindex 0000000..350acae\n--- /dev/null\n+++ b/v3-0001-ref-filter-add-option-to-align-atoms-to-the-left.patch\n@@ -0,0 +1,124 @@\n+From 3a07ca1f56f74ca54b7f3c30e3dfd9fe2fed1cc3 Mon Sep 17 00:00:00 2001\n+From: Karthik Nayak \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 \n+Mentored-by: Christian Couder \n+Mentored-by: Matthieu Moy \n+Signed-off-by: Karthik Nayak \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+ \t{ \"flag\" },\n+ \t{ \"HEAD\" },\n+ \t{ \"color\" },\n++\t{ \"align\" },\n+ };\n+ \n+ /*\n+@@ -620,7 +623,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+@@ -687,6 +690,17 @@ 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, \"align:\")) {\n++\t\t\tconst char *valp = NULL;\n++\n++\t\t\tskip_prefix(name, \"align:\", &valp);\n++\t\t\tif (!valp[0])\n++\t\t\t\tdie(_(\"no value given with 'align:'\"));\n++\t\t\tstrtoul_ui(valp, 10, &ref->align_value);\n++\t\t\tif (ref->align_value < 1)\n++\t\t\t\tdie(_(\"value should be greater than zero: align:%u\"), ref->align_value);\n++\t\t\tv->s = \"\";\n++\t\t\tcontinue;\n+ \t\t} else\n+ \t\t\tcontinue;\n+ \n+@@ -1254,17 +1268,40 @@ static void emit(const char *cp, const char *ep)\n+ \t}\n+ }\n+ \n++static void assign_formating(struct ref_array_item *ref, int parsed_atom, struct atom_value *v)\n++{\n++\tif (ref->align_value && !starts_with(used_atom[parsed_atom], \"align\")) {\n++\t\tunsigned int len = 0;\n++\n++\t\tif (*v->s)\n++\t\t\tlen = utf8_strwidth(v->s);\n++\t\tif (ref->align_value > len) {\n++\t\t\tstruct strbuf buf = STRBUF_INIT;\n++\t\t\tif (*v->s)\n++\t\t\t\tstrbuf_addstr(&buf, v->s);\n++\t\t\tif (*v->s && v->s[0] == '\\0')\n++\t\t\t\tfree((char *)v->s);\n++\t\t\tstrbuf_addchars(&buf, ' ', ref->align_value - len);\n++\t\t\tv->s = strbuf_detach(&buf, NULL);\n++\t\t}\n++\t\tref->align_value = 0;\n++\t}\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+ \n+ \tfor (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {\n+ \t\tstruct atom_value *atomv;\n++\t\tint parsed_atom;\n+ \n+ \t\tep = strchr(sp, ')');\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\tparsed_atom = parse_ref_filter_atom(sp + 2, ep);\n++\t\tget_ref_atom_value(info, parsed_atom, &atomv);\n++\t\tassign_formating(info, parsed_atom, atomv);\n+ \t\tprint_value(atomv, quote_style);\n+ \t}\n+ \tif (*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+ \tunsigned char objectname[20];\n+ \tint flag;\n++\tunsigned int align_value;\n+ \tconst char *symref;\n+ \tstruct commit *commit;\n+ \tstruct atom_value *value;\n+-- \n+2.4.6\n+\n-- \n2.4.6\n"},{"id":"266740","messageId":"1437764685-8633-6-git-send-email-Karthik.188@gmail.com","threadId":"39934","inReplyTo":"1437764685-8633-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v4 05/10] ref-filter: add support to sort by version","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-24T19:04:40Z","receivedAt":"2015-07-24T19:04:40Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \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 \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n Documentation/git-for-each-ref.txt | 3 +++\n ref-filter.c | 26 ++++++++++++++------------\n ref-filter.h | 3 ++-\n t/t6302-for-each-ref-filter.sh | 36 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 55 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex e49d578..224dc8c 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -145,6 +145,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 08ecce5..8f2148f 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@@ -1180,19 +1181,17 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru\n \n \tget_ref_atom_value(&state, a, s->atom, &va);\n \tget_ref_atom_value(&state, 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-\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+\telse if (va->ul < vb->ul)\n+\t\tcmp = -1;\n+\telse if (va->ul == vb->ul)\n+\t\tcmp = 0;\n+\telse\n+\t\tcmp = 1;\n+\n \treturn (s->reverse) ? -cmp : cmp;\n }\n \n@@ -1420,6 +1419,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 1e2ee65..a12fe0c 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -26,7 +26,8 @@ struct atom_value {\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_formatting_state {\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex 505a360..5017032 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -81,4 +81,40 @@ test_expect_success 'filtering with --contains' '\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 tag -l --sort=version:refname | 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 tag -l --sort=v:refname | 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 tag -l --sort=-version:refname | 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":"266741","messageId":"1437764685-8633-7-git-send-email-Karthik.188@gmail.com","threadId":"39934","inReplyTo":"1437764685-8633-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v4 06/10] ref-filter: add option to match literal pattern","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-24T19:04:41Z","receivedAt":"2015-07-24T19:04:41Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \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 \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n builtin/for-each-ref.c | 1 +\n ref-filter.c | 37 ++++++++++++++++++++++++++++++++++++-\n ref-filter.h | 3 ++-\n 3 files changed, 39 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 8f2148f..0a64b84 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -951,6 +951,31 @@ 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+\tfor (; *patterns; patterns++) {\n+\t\t/*\n+\t\t * When no '--format' option is given we need to skip the prefix\n+\t\t * for matching refs of tags and branches.\n+\t\t */\n+\t\tif (!starts_with(*patterns, \"refs/tags/\"))\n+\t\t\tskip_prefix(refname, \"refs/tags/\", &refname);\n+\t\tif (!starts_with(*patterns, \"refs/heads/\"))\n+\t\t\tskip_prefix(refname, \"refs/heads/\", &refname);\n+\t\tif (!starts_with(*patterns, \"refs/remotes/\"))\n+\t\t\tskip_prefix(refname, \"refs/remotes/\", &refname);\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@@ -974,6 +999,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@@ -1042,7 +1077,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 a12fe0c..f1933e0 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -62,7 +62,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":"266743","messageId":"1437764685-8633-8-git-send-email-Karthik.188@gmail.com","threadId":"39934","inReplyTo":"1437764685-8633-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v4 07/10] tag.c: use 'ref-filter' data structures","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-24T19:04:42Z","receivedAt":"2015-07-24T19:04:42Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \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 \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \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 10b11ce..dadc686 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 ] [-f] [-m | -F ] []\"),\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,16 +579,16 @@ 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 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 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@@ -604,14 +609,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@@ -620,6 +625,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@@ -636,7 +643,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@@ -649,18 +656,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":"266745","messageId":"1437765582-12537-1-git-send-email-Karthik.188@gmail.com","threadId":"39934","inReplyTo":"1437764685-8633-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v4 08/10] tag.c: use 'ref-filter' APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-24T19:19:40Z","receivedAt":"2015-07-24T19:19:40Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \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 \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \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 4b04c2b..02fb363 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n \t [ | ]\n 'git tag' -d ...\n 'git tag' [-n[]] -l [--contains ] [--points-at ]\n-\t[--column[=] | --no-column] [...]\n+\t[--column[=] | --no-column] [--sort=] [...]\n 'git tag' -v ...\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=::\n-\tSort in a specific order. Supported type is \"refname\"\n-\t(lexicographic order), \"version:refname\" or \"v:refname\" (tag\n+--sort=::\n+\tSort based on the key given. Prefix `-` to sort in\n+\tdescending order of the value. You may use the --sort= 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 dadc686..d396872 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 = \"%(align: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@@ -586,6 +325,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@@ -611,10 +351,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@@ -622,7 +360,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@@ -648,6 +386,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@@ -656,10 +396,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 d1ff5c9..51a233f 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1450,13 +1450,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":"266747","messageId":"1437765582-12537-2-git-send-email-Karthik.188@gmail.com","threadId":"39934","inReplyTo":"1437765582-12537-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v4 09/10] tag.c: implement '--format' option","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-24T19:19:41Z","receivedAt":"2015-07-24T19:19:41Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \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 \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \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 02fb363..727d653 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -13,7 +13,8 @@ SYNOPSIS\n \t [ | ]\n 'git tag' -d ...\n 'git tag' [-n[]] -l [--contains ] [--points-at ]\n-\t[--column[=] | --no-column] [--sort=] [...]\n+\t[--column[=] | --no-column] [--sort=] [--format=]\n+\t[...]\n 'git tag' -v ...\n \n DESCRIPTION\n@@ -155,6 +156,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+::\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 d396872..a8052b4 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 = \"%(align:16)%(refname:short)\";\n-\telse\n+\telse if (!format)\n \t\tformat = \"%(refname:short)\";\n \n \tverify_ref_format(format);\n@@ -326,6 +325,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@@ -357,6 +357,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@@ -396,8 +397,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 51a233f..884c0d3 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1507,4 +1507,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":"266746","messageId":"1437765582-12537-3-git-send-email-Karthik.188@gmail.com","threadId":"39934","inReplyTo":"1437765582-12537-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v4 10/10] tag.c: implement '--merged' and '--no-merged' options","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-24T19:19:42Z","receivedAt":"2015-07-24T19:19:42Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \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 \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \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 727d653..2a07d67 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -14,7 +14,7 @@ SYNOPSIS\n 'git tag' -d ...\n 'git tag' [-n[]] -l [--contains ] [--points-at ]\n \t[--column[=] | --no-column] [--sort=] [--format=]\n-\t[...]\n+\t[--[no-]merged []] [...]\n 'git tag' -v ...\n \n DESCRIPTION\n@@ -168,6 +168,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 []::\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 a8052b4..23256fc 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 ] [-f] [-m | -F ] []\"),\n \tN_(\"git tag -d ...\"),\n \tN_(\"git tag -l [-n[]] [--contains ] [--points-at ]\"\n-\t\t\"\\n\\t\\t[...]\"),\n+\t\t\"\\n\\t\\t[--[no-]merged []] [...]\"),\n \tN_(\"git tag -v ...\"),\n \tNULL\n };\n@@ -351,6 +351,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@@ -411,6 +413,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 884c0d3..e154a32 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1523,4 +1523,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":"266762","messageId":"xmqq1tfxkz44.fsf@gitster.dls.corp.google.com","threadId":"39934","inReplyTo":"1437764685-8633-3-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v4 02/10] ref-filter: make the 'color' use ref_formatting_state","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-24T21:46:35Z","receivedAt":"2015-07-24T21:46:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> Make color which was considered as an atom, to use\n> ref_formatting_state and act as a pseudo atom. This allows\n> interchangeability between 'align' and 'color'.\n>\n> Helped-by: Junio C Hamano \n> Mentored-by: Christian Couder \n> Mentored-by: Matthieu Moy \n> Signed-off-by: Karthik Nayak \n> ---\n\nI think 1/10 and 2/10 are the other way around. Preferably, these\nwould be three patches, in this order:\n\n (1) prepare the \"formatting state\" and pass it around; no other\n code change.\n\n (2) have \"color\" use that facility.\n\n (3) add a new feature using that facility.\n\nThe first two may want to be combined into a single patch, if it\ndoes not make the patch too noisy.\n\n> ref-filter.c | 16 ++++++++++++----\n> ref-filter.h | 1 +\n> 2 files changed, 13 insertions(+), 4 deletions(-)\n>\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 3c90ffc..fd13a23 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -663,7 +663,8 @@ static void populate_value(struct ref_formatting_state *state,\n> \n> \t\t\tif (color_parse(name + 6, color) < 0)\n> \t\t\t\tdie(_(\"unable to parse format\"));\n> -\t\t\tv->s = xstrdup(color);\n> +\t\t\tstate->color = xstrdup(color);\n> +\t\t\tv->pseudo_atom = 1;\n> \t\t\tcontinue;\n> \t\t} else if (!strcmp(name, \"flag\")) {\n> \t\t\tchar buf[256], *cp = buf;\n> @@ -1217,6 +1218,11 @@ void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)\n> static void ref_formatting(struct ref_formatting_state *state, struct atom_value *v,\n> \t\t\t struct strbuf *value)\n> {\n> +\tif (state->color) {\n> +\t\tstrbuf_addstr(value, state->color);\n> +\t\tfree((void *)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> @@ -1224,8 +1230,9 @@ static void ref_formatting(struct ref_formatting_state *state, struct atom_value\n> \t\t\tint len = strlen(v->s) - utf8_strwidth(v->s);\n> \t\t\tstrbuf_addf(value, \"%-*s\", state->pad_to_right + len, v->s);\n> \t\t}\n> -\t} else\n> -\t\tstrbuf_addf(value, \"%s\", v->s);\n> +\t\treturn;\n> +\t}\n> +\tstrbuf_addf(value, \"%s\", v->s);\n> }\n> \n> static void print_value(struct ref_formatting_state *state, struct atom_value *v)\n> @@ -1326,7 +1333,8 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n> \n> \t\tif (color_parse(\"reset\", color) < 0)\n> \t\t\tdie(\"BUG: couldn't parse 'reset' as a color\");\n> -\t\tresetv.s = color;\n> +\t\tresetv.s = \"\";\n> +\t\tstate.color = xstrdup(color);\n> \t\tprint_value(&state, &resetv);\n> \t}\n> \tputchar('\\n');\n> diff --git a/ref-filter.h b/ref-filter.h\n> index ea2d0e6..bacbb23 100644\n> --- a/ref-filter.h\n> +++ b/ref-filter.h\n> @@ -31,6 +31,7 @@ struct ref_sorting {\n> struct ref_formatting_state {\n> \tunsigned int pad_to_right; /*pad atoms to the right*/\n> \tint quote_style;\n> +\tconst char *color;\n> };\n> \n> struct ref_array_item {\n"},{"id":"266763","messageId":"xmqqwpxpjk6p.fsf@gitster.dls.corp.google.com","threadId":"39934","inReplyTo":"1437764685-8633-2-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v4 01/10] ref-filter: add option to align atoms to the left","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-24T21:54:22Z","receivedAt":"2015-07-24T21:54:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> -\tif (!ref->value) {\n> -\t\tpopulate_value(ref);\n> +\t/*\n> +\t * If the atom is a pseudo_atom then we re-populate the value\n> +\t * into the ref_formatting_state stucture.\n> +\t */\n> +\tif (!ref->value || ref->value[atom].pseudo_atom) {\n> +\t\tpopulate_value(state, ref);\n> \t\tfill_missing_values(ref->value);\n\nI am not sure why you need to do this. populate_value() and\nfill_missing_values() are fairly heavy-weight operations that are\nexpected to grab everything necessary from scratch, and that is why\nwe ensure that we do not call them more than once for each \"ref\"\nwith by guarding the calls with \"if (!ref->value)\".\n\nThis change is breaking that basic arrangement, and worse yet, it\nforces us re-read everything about that ref, leaking old ref->value.\n\nWhy could this be a good idea?\n"},{"id":"266764","messageId":"xmqqpp3hjh3r.fsf@gitster.dls.corp.google.com","threadId":"39934","inReplyTo":"xmqqwpxpjk6p.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4 01/10] ref-filter: add option to align atoms to the left","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-24T23:00:56Z","receivedAt":"2015-07-24T23:00:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Karthik Nayak writes:\n>\n>> -\tif (!ref->value) {\n>> -\t\tpopulate_value(ref);\n>> +\t/*\n>> +\t * If the atom is a pseudo_atom then we re-populate the value\n>> +\t * into the ref_formatting_state stucture.\n>> +\t */\n>> +\tif (!ref->value || ref->value[atom].pseudo_atom) {\n>> +\t\tpopulate_value(state, ref);\n>> \t\tfill_missing_values(ref->value);\n>\n> I am not sure why you need to do this. populate_value() and\n> fill_missing_values() are fairly heavy-weight operations that are\n> expected to grab everything necessary from scratch, and that is why\n> we ensure that we do not call them more than once for each \"ref\"\n> with by guarding the calls with \"if (!ref->value)\".\n>\n> This change is breaking that basic arrangement, and worse yet, it\n> forces us re-read everything about that ref, leaking old ref->value.\n>\n> Why could this be a good idea?\n\nI think populate_value() should not take state; that is the root\ncause of this mistake.\n\nThe flow should be:\n\n - verify_format() looks at the format string and enumerates all\n atoms that will ever be used in the output by calling\n parse_atom() and letting it to fill used_atom[];\n\n - when ref->value is not yet populated, populate_value() is\n called, just once. This uses the enumeration in used_atom[]\n and stores computed value to refs->value[atom];\n\n - show_ref() repeatedly calls find_next() to find the next\n reference to %(...), emits everything before it, and then\n uses the atom value (i.e. ref->value[atom]).\n\nI would expect that the atom value for pseudos like color and align\nto be parsed and stored in ref->value in populate_value() when it is\ncalled for the first time for each ref _just once_.\n\n\"color:blue\" may choose to store \"blue\" as v->s, and \"align:4\" may\nchoose to do \"v->ul = 4\".\n\nAnd the code that uses these values should look more like:\n\n\tfor (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {\n\t\tstruct atom_value *atomv;\n\n\t\tep = strchr(sp, ')');\n\t\tif (cp < sp)\n\t\t\temit(cp, sp);\n\t\tget_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);\n\t\tif (atomv->is_pseudo)\n \tapply_pseudo_state(&state, atomv);\n\t\telse\n\t\t\tprint_value(&state, atomv);\n\t}\n\nwhere apply_pseudo_state() would switch on what kind of pseudo the\natom is and update the state accordingly, i.e. the \"state\" munging\ncode you added to populate_value() in this patch.\n"},{"id":"266768","messageId":"CAOLa=ZRBwErMLhcg=STymECrfGG3WQpfFvPXzDCcjBTQb9JmCA@mail.gmail.com","threadId":"39934","inReplyTo":"xmqqpp3hjh3r.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4 01/10] ref-filter: add option to align atoms to the left","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-25T04:14:19Z","receivedAt":"2015-07-25T04:14:19Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sat, Jul 25, 2015 at 4:30 AM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Karthik Nayak writes:\n>>\n>>> - if (!ref->value) {\n>>> - populate_value(ref);\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>>> fill_missing_values(ref->value);\n>>\n> I am not sure why you need to do this. populate_value() and\n> fill_missing_values() are fairly heavy-weight operations that are\n> expected to grab everything necessary from scratch, and that is why\n> we ensure that we do not call them more than once for each \"ref\"\n> with by guarding the calls with \"if (!ref->value)\".\n>\n> This change is breaking that basic arrangement, and worse yet, it\n> forces us re-read everything about that ref, leaking old ref->value.\n>\n> Why could this be a good idea?\n>\n\nThis was required as populate_value() would fill the 'state' but the 'state'\nbeing a static variable would be lost before printing and hence we would\nnot have the correct values of the 'state' when printing.\n\n> I think populate_value() should not take state; that is the root\n> cause of this mistake.\n\nYes! agreed. atomv should be a better candidate to hold the formatting values.\n\n>\n> The flow should be:\n>\n> - verify_format() looks at the format string and enumerates all\n> atoms that will ever be used in the output by calling\n> parse_atom() and letting it to fill used_atom[];\n>\n> - when ref->value is not yet populated, populate_value() is\n> called, just once. This uses the enumeration in used_atom[]\n> and stores computed value to refs->value[atom];\n>\n> - show_ref() repeatedly calls find_next() to find the next\n> reference to %(...), emits everything before it, and then\n> uses the atom value (i.e. ref->value[atom]).\n>\n> I would expect that the atom value for pseudos like color and align\n> to be parsed and stored in ref->value in populate_value() when it is\n> called for the first time for each ref _just once_.\n>\n> \"color:blue\" may choose to store \"blue\" as v->s, and \"align:4\" may\n> choose to do \"v->ul = 4\".\n>\n> And the code that uses these values should look more like:\n>\n> for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {\n> struct atom_value *atomv;\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> if (atomv->is_pseudo)\n> apply_pseudo_state(&state, atomv);\n> else\n> print_value(&state, atomv);\n> }\n>\n> where apply_pseudo_state() would switch on what kind of pseudo the\n> atom is and update the state accordingly, i.e. the \"state\" munging\n> code you added to populate_value() in this patch.\n\nThis makes more sense, and avoids the repetitive call to populate_value().\nWill implement this.\nThanks\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266769","messageId":"CAOLa=ZT0LALR0gQaRfVnpyhScSKrSLP+QPQioe+uEH+KJYs=Ow@mail.gmail.com","threadId":"39934","inReplyTo":"xmqq1tfxkz44.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4 02/10] ref-filter: make the 'color' use ref_formatting_state","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-25T04:15:21Z","receivedAt":"2015-07-25T04:15:21Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sat, Jul 25, 2015 at 3:16 AM, Junio C Hamano wrote:\n> Karthik Nayak writes:\n>\n>> Make color which was considered as an atom, to use\n>> ref_formatting_state and act as a pseudo atom. This allows\n>> interchangeability between 'align' and 'color'.\n>>\n>> Helped-by: Junio C Hamano \n>> Mentored-by: Christian Couder \n>> Mentored-by: Matthieu Moy \n>> Signed-off-by: Karthik Nayak \n>> ---\n>\n> I think 1/10 and 2/10 are the other way around. Preferably, these\n> would be three patches, in this order:\n>\n> (1) prepare the \"formatting state\" and pass it around; no other\n> code change.\n>\n> (2) have \"color\" use that facility.\n>\n> (3) add a new feature using that facility.\n>\n> The first two may want to be combined into a single patch, if it\n> does not make the patch too noisy.\n>\n\nIll reverse the patches and merge the first two as you suggested.\nThanks\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266775","messageId":"xmqqd1zfkgi3.fsf@gitster.dls.corp.google.com","threadId":"39934","inReplyTo":"1437764685-8633-6-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v4 05/10] ref-filter: add support to sort by version","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-25T22:40:52Z","receivedAt":"2015-07-25T22:40:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> @@ -1180,19 +1181,17 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru\n> \n> \tget_ref_atom_value(&state, a, s->atom, &va);\n> \tget_ref_atom_value(&state, 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> -\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> +\telse if (va->ul < vb->ul)\n> +\t\tcmp = -1;\n> +\telse if (va->ul == vb->ul)\n> +\t\tcmp = 0;\n> +\telse\n> +\t\tcmp = 1;\n> +\n\nSo there are generally three kinds of comparison possible:\n\n - if it is to be compared as versions, do versioncmp\n - if it is to be compared as strings, do strcmp\n - if it is to be compared as numbers, do <=> but because\n we are writing in C, not in Perl, do so as if/else/else\n\nHaving understood that, the above is not really easy to read and\nextend. We should structure the above more like this:\n\n\tif (s->version)\n \t... versioncmp\n\telse if (... FIELD_STR)\n \t... strcmp\n\telse {\n\t\tif (a < b)\n \t...\n\t\telse if (a == b)\n \t...\n\t\telse\n \t...\n }\n\nso that it would be obvious how this code need to be updated\nwhen we need to add yet another kind of comparison.\n\nWithout looking at the callers, s->version looks like a misdesign\nthat should be updated to use the same cmp_type mechanism? That\nwould lead to even more obvious construct that is easy to enhance,\ni.e.\n\n\tswitch (cmp_type) {\n case CMP_VERSION:\n \t...\n\tcase CMP_STRING:\n \t...\n\tcase CMP_NUMBER:\n \t...\n }\n \nI dunno.\n\nOther than that (and the structure of that \"format-state\" stuff we\ndiscussed separately), the series was a pleasant read.\n\nThanks.\n"},{"id":"266779","messageId":"CAPig+cQSy6imD_z+a9tvprJtn2-20awLJOcUQAksyv6HceVzxQ@mail.gmail.com","threadId":"39934","inReplyTo":"1437764685-8633-2-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v4 01/10] ref-filter: add option to align atoms to the left","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-26T04:08:40Z","receivedAt":"2015-07-26T04:08:40Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 24, 2015 at 3:04 PM, Karthik Nayak wrote:\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\nDo you mean \"succeeding\" or \"following\" or \"next\" (or something)\nrather than \"preceding\"?\n\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\nIsn't this a pad-right operation? If so, should this be called\n%(padright:X) or %(pad:right:X)?\n\n> Signed-off-by: Karthik Nayak \n\nAlso, it is helpful to reviewers if you include an interdiff at the\nbottom of your cover letter showing the changes from one version to\nanother. You can generate an interdiff with \"git diff branchname-v4\nbranchname-v5\", for instance.\n"},{"id":"266780","messageId":"CAPig+cQEKrjePskjSacrf3SbLZR7_ADxTLwcV7vBPcjUxBLiEg@mail.gmail.com","threadId":"39934","inReplyTo":"1437764685-8633-3-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v4 02/10] ref-filter: make the 'color' use ref_formatting_state","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-26T04:12:39Z","receivedAt":"2015-07-26T04:12:39Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 24, 2015 at 3:04 PM, Karthik Nayak wrote:\n> Make color which was considered as an atom, to use\n> ref_formatting_state and act as a pseudo atom. This allows\n> interchangeability between 'align' and 'color'.\n>\n> Signed-off-by: Karthik Nayak \n> ---\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 3c90ffc..fd13a23 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -663,7 +663,8 @@ static void populate_value(struct ref_formatting_state *state,\n> - v->s = xstrdup(color);\n> + state->color = xstrdup(color);\n> + v->pseudo_atom = 1;\n> @@ -1217,6 +1218,11 @@ void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)\n> + if (state->color) {\n> + strbuf_addstr(value, state->color);\n> + free((void *)state->color);\n> + state->color = NULL;\n> + }\n> @@ -1326,7 +1333,8 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n> - resetv.s = color;\n> + resetv.s = \"\";\n> + state.color = xstrdup(color);\n> diff --git a/ref-filter.h b/ref-filter.h\n> index ea2d0e6..bacbb23 100644\n> --- a/ref-filter.h\n> +++ b/ref-filter.h\n> @@ -31,6 +31,7 @@ struct ref_sorting {\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\nShould 'color' should be declared 'char *' rather than 'const char *'?\nIt's always assigned via xstrdup(), and if declared 'char *', you\nwouldn't have to cast away the 'const' when freeing it.\n"},{"id":"266781","messageId":"CAPig+cQ3tFFC8MMNfwo0V_hyYi8EoSfaQLei0+S+Tpz=2sMJ8g@mail.gmail.com","threadId":"39934","inReplyTo":"CAOLa=ZT0LALR0gQaRfVnpyhScSKrSLP+QPQioe+uEH+KJYs=Ow@mail.gmail.com","subject":"Re: [PATCH v4 02/10] ref-filter: make the 'color' use ref_formatting_state","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-26T04:28:39Z","receivedAt":"2015-07-26T04:28:39Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jul 25, 2015 at 12:15 AM, Karthik Nayak wrote:\n> On Sat, Jul 25, 2015 at 3:16 AM, Junio C Hamano wrote:\n>> Karthik Nayak writes:\n>>> Make color which was considered as an atom, to use\n>>> ref_formatting_state and act as a pseudo atom. This allows\n>>> interchangeability between 'align' and 'color'.\n>>>\n>>> Signed-off-by: Karthik Nayak \n>>> ---\n>> I think 1/10 and 2/10 are the other way around. Preferably, these\n>> would be three patches, in this order:\n>>\n>> (1) prepare the \"formatting state\" and pass it around; no other\n>> code change.\n>>\n>> (2) have \"color\" use that facility.\n>>\n>> (3) add a new feature using that facility.\n>>\n>> The first two may want to be combined into a single patch, if it\n>> does not make the patch too noisy.\n>\n> Ill reverse the patches and merge the first two as you suggested.\n\nWhile reviewing patch 1/10, it required more effort and was\ndistracting to have to separate out (mentally) changes which were\nspecific to the new %(align:X) pseudo-atom and those which introduced\nthe \"formatting state\". As such, it probably would be a good idea to\naim for the three distinct patches suggested by Junio rather than\naiming, up front, to merge the first two. After all, they are\nconceptually distinct changes, and keeping them separate is friendlier\nto reviewers. In the end, you may find that patch 1 is so trivial that\nit can be merged with patch 2 without making review more difficult,\nhowever, keeping them distinct during development helps you avoid\nconflating unrelated changes.\n"},{"id":"266782","messageId":"CAOLa=ZQvMV8C6LV-+B3m74x209Fr1X7W=piaL-Eftu_PdRW64w@mail.gmail.com","threadId":"39934","inReplyTo":"CAPig+cQSy6imD_z+a9tvprJtn2-20awLJOcUQAksyv6HceVzxQ@mail.gmail.com","subject":"Re: [PATCH v4 01/10] ref-filter: add option to align atoms to the left","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-26T04:36:15Z","receivedAt":"2015-07-26T04:36:15Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sun, Jul 26, 2015 at 9:38 AM, Eric Sunshine wrote:\n> On Fri, Jul 24, 2015 at 3:04 PM, Karthik Nayak wrote:\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>\n> Do you mean \"succeeding\" or \"following\" or \"next\" (or something)\n> rather than \"preceding\"?\n\nI meant succeeding, I had just changed that, thanks for telling\n\n>\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> Isn't this a pad-right operation? If so, should this be called\n> %(padright:X) or %(pad:right:X)?\n>\n\nI guess \"padright\" makes more sense, thanks.\n\n>> Signed-off-by: Karthik Nayak \n>\n> Also, it is helpful to reviewers if you include an interdiff at the\n> bottom of your cover letter showing the changes from one version to\n> another. You can generate an interdiff with \"git diff branchname-v4\n> branchname-v5\", for instance.\n\nI've been working on the same branch, and that's why I didn't really\nprovide interdiff's, and I kinda worked on the same branch again,\nso I wont be giving an interdiff for the next series either, but I'll keep this\nin mind and follow it from the forthcoming patch series. Thanks\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266783","messageId":"CAPig+cS92cJ=OmaG2_HWNXNq9-D5V4f032WqeU0SVh_XCtxpXA@mail.gmail.com","threadId":"39934","inReplyTo":"1437764685-8633-5-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v4 04/10] ref-filter: support printing N lines from tag annotation","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-26T04:46:34Z","receivedAt":"2015-07-26T04:46:34Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 24, 2015 at 3:04 PM, Karthik Nayak wrote:\n> From: Karthik Nayak \n>\n> In 'tag.c' we can print N lines from the annotation of the tag using\n> the '-n' option. Copy code from 'tag.c' to 'ref-filter' and\n> modify 'ref-filter' to support printing of N lines from the annotation\n> of tags.\n>\n> Mentored-by: Christian Couder \n> Mentored-by: Matthieu Moy \n> Signed-off-by: Karthik Nayak \n> ---\n> builtin/for-each-ref.c | 2 +-\n> builtin/tag.c | 4 +\n> ref-filter.c | 51 ++++++++-\n> ref-filter.h | 9 +-\n> ...ter-add-option-to-align-atoms-to-the-left.patch | 124 +++++++++++++++++++++\n> 5 files changed, 186 insertions(+), 4 deletions(-)\n> create mode 100644 v3-0001-ref-filter-add-option-to-align-atoms-to-the-left.patch\n\nSomehow you managed to \"git add\" and \"git commit\" your v3 patch 1 file\ninside this v4 patch 4/10.\n\n> diff --git a/v3-0001-ref-filter-add-option-to-align-atoms-to-the-left.patch b/v3-0001-ref-filter-add-option-to-align-atoms-to-the-left.patch\n> new file mode 100644\n> index 0000000..350acae\n> --- /dev/null\n> +++ b/v3-0001-ref-filter-add-option-to-align-atoms-to-the-left.patch\n> @@ -0,0 +1,124 @@\n> +From 3a07ca1f56f74ca54b7f3c30e3dfd9fe2fed1cc3 Mon Sep 17 00:00:00 2001\n> +From: Karthik Nayak \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 \n> +Mentored-by: Christian Couder \n> +Mentored-by: Matthieu Moy \n> +Signed-off-by: Karthik Nayak \n> +---\n> + ref-filter.c | 41 +++++++++++++++++++++++++++++++++++++++--\n> + ref-filter.h | 1 +\n> + 2 files changed, 40 insertions(+), 2 deletions(-)\n"},{"id":"266785","messageId":"CAOLa=ZQ89Vz82ro_V8mTgYKZ0UUgbsbAW6zgSgyiZ1gpwjTOHA@mail.gmail.com","threadId":"39934","inReplyTo":"xmqqd1zfkgi3.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4 05/10] ref-filter: add support to sort by version","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-26T05:07:57Z","receivedAt":"2015-07-26T05:07:57Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sun, Jul 26, 2015 at 4:10 AM, Junio C Hamano wrote:\n> Karthik Nayak writes:\n>\n>> @@ -1180,19 +1181,17 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru\n>>\n>> get_ref_atom_value(&state, a, s->atom, &va);\n>> get_ref_atom_value(&state, b, s->atom, &vb);\n>> - switch (cmp_type) {\n>> - case FIELD_STR:\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>> - break;\n>> - default:\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>> - break;\n>> - }\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>> +\n>\n> So there are generally three kinds of comparison possible:\n>\n> - if it is to be compared as versions, do versioncmp\n> - if it is to be compared as strings, do strcmp\n> - if it is to be compared as numbers, do <=> but because\n> we are writing in C, not in Perl, do so as if/else/else\n>\n> Having understood that, the above is not really easy to read and\n> extend. We should structure the above more like this:\n>\n> if (s->version)\n> ... versioncmp\n> else if (... FIELD_STR)\n> ... strcmp\n> else {\n> if (a < b)\n> ...\n> else if (a == b)\n> ...\n> else\n> ...\n> }\n>\n> so that it would be obvious how this code need to be updated\n> when we need to add yet another kind of comparison.\n>\n\nI find the current version more pleasing to read, The way you've explained\nit though, it seems that its better to structure it the way you've\nmentioned as this\nactually shows the code flow of the three kinds of comparison possible.\n\n> Without looking at the callers, s->version looks like a misdesign\n> that should be updated to use the same cmp_type mechanism? That\n> would lead to even more obvious construct that is easy to enhance,\n> i.e.\n>\n> switch (cmp_type) {\n> case CMP_VERSION:\n> ...\n> case CMP_STRING:\n> ...\n> case CMP_NUMBER:\n> ...\n> }\n>\n> I dunno.\n>\n> Other than that (and the structure of that \"format-state\" stuff we\n> discussed separately), the series was a pleasant read.\n>\n> Thanks.\n\nThat was the previous design, but Duy asked me to do this so\nthat we could support all atoms. And I agree with him on this.\n\nhttp://article.gmane.org/gmane.comp.version-control.git/273888\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266787","messageId":"CAOLa=ZRGNPBA6bOOG+KQafW02bjcTEJdU4BJ7hVzNDL7n9Hf7Q@mail.gmail.com","threadId":"39934","inReplyTo":"CAPig+cS92cJ=OmaG2_HWNXNq9-D5V4f032WqeU0SVh_XCtxpXA@mail.gmail.com","subject":"Re: [PATCH v4 04/10] ref-filter: support printing N lines from tag annotation","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-26T05:15:09Z","receivedAt":"2015-07-26T05:15:09Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sun, Jul 26, 2015 at 10:16 AM, Eric Sunshine wrote:\n> On Fri, Jul 24, 2015 at 3:04 PM, Karthik Nayak wrote:\n>> From: Karthik Nayak \n>>\n>> In 'tag.c' we can print N lines from the annotation of the tag using\n>> the '-n' option. Copy code from 'tag.c' to 'ref-filter' and\n>> modify 'ref-filter' to support printing of N lines from the annotation\n>> of tags.\n>>\n>> Mentored-by: Christian Couder \n>> Mentored-by: Matthieu Moy \n>> Signed-off-by: Karthik Nayak \n>> ---\n>> builtin/for-each-ref.c | 2 +-\n>> builtin/tag.c | 4 +\n>> ref-filter.c | 51 ++++++++-\n>> ref-filter.h | 9 +-\n>> ...ter-add-option-to-align-atoms-to-the-left.patch | 124 +++++++++++++++++++++\n>> 5 files changed, 186 insertions(+), 4 deletions(-)\n>> create mode 100644 v3-0001-ref-filter-add-option-to-align-atoms-to-the-left.patch\n>\n> Somehow you managed to \"git add\" and \"git commit\" your v3 patch 1 file\n> inside this v4 patch 4/10.\n\nYa! removed it, thanks.\n\n>\n>> diff --git a/v3-0001-ref-filter-add-option-to-align-atoms-to-the-left.patch b/v3-0001-ref-filter-add-option-to-align-atoms-to-the-left.patch\n>> new file mode 100644\n>> index 0000000..350acae\n>> --- /dev/null\n>> +++ b/v3-0001-ref-filter-add-option-to-align-atoms-to-the-left.patch\n>> @@ -0,0 +1,124 @@\n>> +From 3a07ca1f56f74ca54b7f3c30e3dfd9fe2fed1cc3 Mon Sep 17 00:00:00 2001\n>> +From: Karthik Nayak \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 \n>> +Mentored-by: Christian Couder \n>> +Mentored-by: Matthieu Moy \n>> +Signed-off-by: Karthik Nayak \n>> +---\n>> + ref-filter.c | 41 +++++++++++++++++++++++++++++++++++++++--\n>> + ref-filter.h | 1 +\n>> + 2 files changed, 40 insertions(+), 2 deletions(-)\n\n\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266786","messageId":"CAPig+cTAYAxg4aWpaPrHe6Hfz3wCEiypoXZxS_Lir4yNOFzSNA@mail.gmail.com","threadId":"39934","inReplyTo":"1437764685-8633-7-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v4 06/10] ref-filter: add option to match literal pattern","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-26T05:15:35Z","receivedAt":"2015-07-26T05:15:35Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 24, 2015 at 3:04 PM, Karthik Nayak wrote:\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 \n> ---\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 8f2148f..0a64b84 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -951,6 +951,31 @@ 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> + for (; *patterns; patterns++) {\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 (!starts_with(*patterns, \"refs/tags/\"))\n> + skip_prefix(refname, \"refs/tags/\", &refname);\n> + if (!starts_with(*patterns, \"refs/heads/\"))\n> + skip_prefix(refname, \"refs/heads/\", &refname);\n> + if (!starts_with(*patterns, \"refs/remotes/\"))\n> + skip_prefix(refname, \"refs/remotes/\", &refname);\n\nNote the inefficiency here: You're performing an \"expensive\"\nstarts_with() on each pattern for each refname even though the\npatterns will never change. You could instead compute starts_with()\nfor each pattern just once, in a preprocessing step, and remember each\nresult as a boolean, and then use the computed booleans here in place\nof starts_with(). For a few refnames, this may not make a difference,\nbut for a project with a huge number, it could. Whether such an\noptimization is worth the complexity (at this time or ever) is\nsomething that can be answered by taking measurements.\n\nAlso, the repetition in the code is not all that pretty. You could\ninstead place \"refs/tags/\", \"refs/heads/\", and \"refs/remotes/\" in a\ntable and then loop over them rather than repeating the code for each\none, though whether that would be an improvement is likely a judgment\ncall (so not something I'd insist upon).\n\n> + if (!wildmatch(*patterns, refname, 0, NULL))\n> + return 1;\n> + }\n> + return 0;\n> +}\n"},{"id":"266789","messageId":"CAOLa=ZRz9LL5heW2FOhZje+=5W_Ybtm+Rcsm-RH=eKXoh5yw2Q@mail.gmail.com","threadId":"39934","inReplyTo":"CAPig+cQEKrjePskjSacrf3SbLZR7_ADxTLwcV7vBPcjUxBLiEg@mail.gmail.com","subject":"Re: [PATCH v4 02/10] ref-filter: make the 'color' use ref_formatting_state","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-26T05:40:10Z","receivedAt":"2015-07-26T05:40:10Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sun, Jul 26, 2015 at 9:42 AM, Eric Sunshine wrote:\n>\n> Should 'color' should be declared 'char *' rather than 'const char *'?\n> It's always assigned via xstrdup(), and if declared 'char *', you\n> wouldn't have to cast away the 'const' when freeing it.\n>\n\nyes, will change.\n\n>\n> While reviewing patch 1/10, it required more effort and was\n> distracting to have to separate out (mentally) changes which were\n> specific to the new %(align:X) pseudo-atom and those which introduced\n> the \"formatting state\". As such, it probably would be a good idea to\n> aim for the three distinct patches suggested by Junio rather than\n> aiming, up front, to merge the first two. After all, they are\n> conceptually distinct changes, and keeping them separate is friendlier\n> to reviewers. In the end, you may find that patch 1 is so trivial that\n> it can be merged with patch 2 without making review more difficult,\n> however, keeping them distinct during development helps you avoid\n> conflating unrelated changes.\n>\n\nYes! I'm doing that like Junio suggested. Thanks for bringing it up :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266790","messageId":"CAPig+cQD3tMmOrYFb=iWEq7CptDgwarZWW3WDGh7QGKVavgWGA@mail.gmail.com","threadId":"39934","inReplyTo":"CAOLa=ZQvMV8C6LV-+B3m74x209Fr1X7W=piaL-Eftu_PdRW64w@mail.gmail.com","subject":"Re: [PATCH v4 01/10] ref-filter: add option to align atoms to the left","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-26T05:58:23Z","receivedAt":"2015-07-26T05:58:23Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jul 26, 2015 at 12:36 AM, Karthik Nayak wrote:\n> On Sun, Jul 26, 2015 at 9:38 AM, Eric Sunshine wrote:\n>> Also, it is helpful to reviewers if you include an interdiff at the\n>> bottom of your cover letter showing the changes from one version to\n>> another. You can generate an interdiff with \"git diff branchname-v4\n>> branchname-v5\", for instance.\n>\n> I've been working on the same branch, and that's why I didn't really\n> provide interdiff's, and I kinda worked on the same branch again,\n> so I wont be giving an interdiff for the next series either, but I'll keep this\n> in mind and follow it from the forthcoming patch series. Thanks\n\nYou can still provide an interdiff. It is quite likely that you can\nfind the commit corresponding to v4 in the reflog for that branch (see\ngit-reflog). Failing that, an easy way to recover the state at v4 is\nto create a new branch (from the parent of your current working\nbranch) and apply the v4 patches which you sent to the mailing list.\nIf Junio queued v4 in his 'pu' branch, then you can also recover from\nthere. Once you've recovered the state of v4 using one of the methods\nmentioned here (or some other), you can make an interdiff when sending\nout v5.\n\nReviewers do appreciate that you provide a URL to the previous version\nand take the time to explain in your cover letter what changed (and\nwhy). Including an interdiff is one more way to ease the review\nprocess, and is also appreciated, so it may be worthwhile to put in\nthe effort to recover the state of v4 so that you can include an\ninterdiff with v5. Doing so does require a bit of extra work for you,\nbut that's work you only need to do *once*, whereas if you don't do\nit, then you place the burden on *each* reviewer for *each* version,\nwhich quickly adds up to a lot more work for those reviewing your\nsubmissions.\n"},{"id":"266791","messageId":"CAOLa=ZQi-_8FOmJRCwRHutfHD=Sqk7K=Xga6gmfjfnYF7veySg@mail.gmail.com","threadId":"39934","inReplyTo":"CAPig+cQD3tMmOrYFb=iWEq7CptDgwarZWW3WDGh7QGKVavgWGA@mail.gmail.com","subject":"Re: [PATCH v4 01/10] ref-filter: add option to align atoms to the left","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-26T06:03:52Z","receivedAt":"2015-07-26T06:03:52Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sun, Jul 26, 2015 at 11:28 AM, Eric Sunshine wrote:\n> On Sun, Jul 26, 2015 at 12:36 AM, Karthik Nayak wrote:\n>> On Sun, Jul 26, 2015 at 9:38 AM, Eric Sunshine wrote:\n>>> Also, it is helpful to reviewers if you include an interdiff at the\n>>> bottom of your cover letter showing the changes from one version to\n>>> another. You can generate an interdiff with \"git diff branchname-v4\n>>> branchname-v5\", for instance.\n>>\n>> I've been working on the same branch, and that's why I didn't really\n>> provide interdiff's, and I kinda worked on the same branch again,\n>> so I wont be giving an interdiff for the next series either, but I'll keep this\n>> in mind and follow it from the forthcoming patch series. Thanks\n>\n> You can still provide an interdiff. It is quite likely that you can\n> find the commit corresponding to v4 in the reflog for that branch (see\n> git-reflog). Failing that, an easy way to recover the state at v4 is\n> to create a new branch (from the parent of your current working\n> branch) and apply the v4 patches which you sent to the mailing list.\n> If Junio queued v4 in his 'pu' branch, then you can also recover from\n> there. Once you've recovered the state of v4 using one of the methods\n> mentioned here (or some other), you can make an interdiff when sending\n> out v5.\n>\n\nHaha, I was just thinking about applying my patches and doing it, will\ndefinitely\nprovide an interdiff.\n\n> Reviewers do appreciate that you provide a URL to the previous version\n> and take the time to explain in your cover letter what changed (and\n> why). Including an interdiff is one more way to ease the review\n> process, and is also appreciated, so it may be worthwhile to put in\n> the effort to recover the state of v4 so that you can include an\n> interdiff with v5. Doing so does require a bit of extra work for you,\n> but that's work you only need to do *once*, whereas if you don't do\n> it, then you place the burden on *each* reviewer for *each* version,\n> which quickly adds up to a lot more work for those reviewing your\n> submissions.\n\nYes! definitely, I'll make sure that I provide the interdiff, I'll\nlook at reflog,\nthanks a lot. I appreciate how reviewers take time to review code, its a\nwonderful process. I will be glad to put in any time to make their process\neasier\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266798","messageId":"CAOLa=ZQQ1F=DibGW=McxORH9CMkOZ8n5fnMOi0_D++Du-zWStw@mail.gmail.com","threadId":"39934","inReplyTo":"CAPig+cTAYAxg4aWpaPrHe6Hfz3wCEiypoXZxS_Lir4yNOFzSNA@mail.gmail.com","subject":"Re: [PATCH v4 06/10] ref-filter: add option to match literal pattern","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-26T18:21:34Z","receivedAt":"2015-07-26T18:21:34Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sun, Jul 26, 2015 at 10:45 AM, Eric Sunshine wrote:\n> On Fri, Jul 24, 2015 at 3:04 PM, Karthik Nayak wrote:\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 \n>> ---\n>> diff --git a/ref-filter.c b/ref-filter.c\n>> index 8f2148f..0a64b84 100644\n>> --- a/ref-filter.c\n>> +++ b/ref-filter.c\n>> @@ -951,6 +951,31 @@ 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>> + for (; *patterns; patterns++) {\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 (!starts_with(*patterns, \"refs/tags/\"))\n>> + skip_prefix(refname, \"refs/tags/\", &refname);\n>> + if (!starts_with(*patterns, \"refs/heads/\"))\n>> + skip_prefix(refname, \"refs/heads/\", &refname);\n>> + if (!starts_with(*patterns, \"refs/remotes/\"))\n>> + skip_prefix(refname, \"refs/remotes/\", &refname);\n>\n> Note the inefficiency here: You're performing an \"expensive\"\n> starts_with() on each pattern for each refname even though the\n> patterns will never change. You could instead compute starts_with()\n> for each pattern just once, in a preprocessing step, and remember each\n> result as a boolean, and then use the computed booleans here in place\n> of starts_with(). For a few refnames, this may not make a difference,\n> but for a project with a huge number, it could. Whether such an\n> optimization is worth the complexity (at this time or ever) is\n> something that can be answered by taking measurements.\n>\n> Also, the repetition in the code is not all that pretty. You could\n> instead place \"refs/tags/\", \"refs/heads/\", and \"refs/remotes/\" in a\n> table and then loop over them rather than repeating the code for each\n> one, though whether that would be an improvement is likely a judgment\n> call (so not something I'd insist upon).\n>\n\nI just put the \"starts_with()\" code so as to ensure that its only used when\nwe don't say \"refs/heads/\", \"refs/remotes/\" or \"refs/tags/\" in the pattern.\nBut looking at the tag.c and branch.c implementations this should always be\ndone. Hence I think Ill move it outside the loop and make it mandatory as this\npattern matching is only used by tag and branch and as they by default\nremove the\npath of the ref. I think it'd make sense to remove it here also.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266799","messageId":"CACsJy8Bq3y25QsQbntpNcVz=TQziiXY-XQ88Y_Cps10YF8zrJA@mail.gmail.com","threadId":"39934","inReplyTo":"CAPig+cQSy6imD_z+a9tvprJtn2-20awLJOcUQAksyv6HceVzxQ@mail.gmail.com","subject":"Re: [PATCH v4 01/10] ref-filter: add option to align atoms to the left","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-07-27T00:39:58Z","receivedAt":"2015-07-27T00:39:58Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Jul 26, 2015 at 11:08 AM, Eric Sunshine wrote:\n> You can generate an interdiff with \"git diff branchname-v4\n> branchname-v5\", for instance.\n\nOff topic. But what stops me from doing this often is it creates a big\nmess in \"git tag -l\". Do we have an option to hide away some\n\"insignificant:\" tags? reflog might be an option if we have something\nlike foo@{/v2} to quickly retrieve the reflog entry whose message\ncontains \"v2\".\n-- \nDuy\n"},{"id":"266815","messageId":"CA+P7+xob8eUf=oJHYN3fBB3k+XX+Jj8Vy0fKLtVQUGdsWDyaKA@mail.gmail.com","threadId":"39934","inReplyTo":"CACsJy8Bq3y25QsQbntpNcVz=TQziiXY-XQ88Y_Cps10YF8zrJA@mail.gmail.com","subject":"Re: [PATCH v4 01/10] ref-filter: add option to align atoms to the left","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2015-07-27T07:39:10Z","receivedAt":"2015-07-27T07:39:10Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Sun, Jul 26, 2015 at 5:39 PM, Duy Nguyen wrote:\n> On Sun, Jul 26, 2015 at 11:08 AM, Eric Sunshine wrote:\n>> You can generate an interdiff with \"git diff branchname-v4\n>> branchname-v5\", for instance.\n>\n> Off topic. But what stops me from doing this often is it creates a big\n> mess in \"git tag -l\". Do we have an option to hide away some\n> \"insignificant:\" tags? reflog might be an option if we have something\n> like foo@{/v2} to quickly retrieve the reflog entry whose message\n> contains \"v2\".\n> --\n> Duy\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n\nYou can normally find the previous commit via the reflog. Various\nchanges to the settings can make the reflog be maintained for longer\nif you have longer lived patch series. That's how I would suggest it,\nrather than branches, as I tend not to keep old versions around on\nseparate tags or branches.\n\nThe problem with \"foo@{/v2}\" is that people don't always keep values\ninside the message it self, but maybe \"foo@{/pattern}\" would be a\nuseful extension?\n\nRegards,\nJake\n"},{"id":"266819","messageId":"CACsJy8Dr29B7Lei-jFqgPViZwd6BZcJde0jkcAwySYcY2v8QqA@mail.gmail.com","threadId":"39934","inReplyTo":"CA+P7+xob8eUf=oJHYN3fBB3k+XX+Jj8Vy0fKLtVQUGdsWDyaKA@mail.gmail.com","subject":"Re: [PATCH v4 01/10] ref-filter: add option to align atoms to the left","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-07-27T10:18:38Z","receivedAt":"2015-07-27T10:18:38Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Jul 27, 2015 at 2:39 PM, Jacob Keller wrote:\n> On Sun, Jul 26, 2015 at 5:39 PM, Duy Nguyen wrote:\n>> On Sun, Jul 26, 2015 at 11:08 AM, Eric Sunshine wrote:\n>>> You can generate an interdiff with \"git diff branchname-v4\n>>> branchname-v5\", for instance.\n>>\n>> Off topic. But what stops me from doing this often is it creates a big\n>> mess in \"git tag -l\". Do we have an option to hide away some\n>> \"insignificant:\" tags? reflog might be an option if we have something\n>> like foo@{/v2} to quickly retrieve the reflog entry whose message\n>> contains \"v2\".\n>\n> You can normally find the previous commit via the reflog. Various\n> changes to the settings can make the reflog be maintained for longer\n> if you have longer lived patch series. That's how I would suggest it,\n> rather than branches, as I tend not to keep old versions around on\n> separate tags or branches.\n>\n> The problem with \"foo@{/v2}\" is that people don't always keep values\n> inside the message it self, but maybe \"foo@{/pattern}\" would be a\n> useful extension?\n\n\"reflog message\" is different from \"commit message\". I was referring\nto the first one (which is out of user control), perhaps you were\nreferring to the second one? I don't expect people to add v2 to\nmanually. If we have something like foo@{/v2} then we can teach\nsend-email (or format-patch) to add a reflog entry with \"v2\" in it.\nBut maybe we're abusing reflog..\n-- \nDuy\n"},{"id":"266823","messageId":"vpqa8uhbyhq.fsf@anie.imag.fr","threadId":"39934","inReplyTo":"CAOLa=ZQvMV8C6LV-+B3m74x209Fr1X7W=piaL-Eftu_PdRW64w@mail.gmail.com","subject":"Re: [PATCH v4 01/10] ref-filter: add option to align atoms to the left","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-27T12:01:37Z","receivedAt":"2015-07-27T12:01:37Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Karthik Nayak writes:\n\n> I've been working on the same branch, and that's why I didn't really\n> provide interdiff's,\n\nYou can keep working on the same branch and tag versions you send to the\nlist. \"This state is what I sent to the list as vX\" is something that does not\nchange in time hence a tag avoids mistakenly changing it.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"266835","messageId":"xmqqvbd5ipy1.fsf@gitster.dls.corp.google.com","threadId":"39934","inReplyTo":"CAOLa=ZQ89Vz82ro_V8mTgYKZ0UUgbsbAW6zgSgyiZ1gpwjTOHA@mail.gmail.com","subject":"Re: [PATCH v4 05/10] ref-filter: add support to sort by version","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-27T15:24:22Z","receivedAt":"2015-07-27T15:24:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> On Sun, Jul 26, 2015 at 4:10 AM, Junio C Hamano wrote:\n>\n>> Without looking at the callers, s->version looks like a misdesign\n>> that should be updated to use the same cmp_type mechanism? That\n>> would lead to even more obvious construct that is easy to enhance,\n>> i.e.\n>>\n>> switch (cmp_type) {\n>> case CMP_VERSION:\n>> ...\n>> case CMP_STRING:\n>> ...\n>> case CMP_NUMBER:\n>> ...\n>> }\n>>\n>> I dunno.\n>>\n>> Other than that (and the structure of that \"format-state\" stuff we\n>> discussed separately), the series was a pleasant read.\n>>\n>> Thanks.\n>\n> That was the previous design, but Duy asked me to do this so\n> that we could support all atoms. And I agree with him on this.\n>\n> http://article.gmane.org/gmane.comp.version-control.git/273888\n\nI am not objecting, but $gmane/273888 does not immediately read, at\nleast to me, as suggesting using a mechanism different from cmp_type\nbut a dedicated field s->version. Puzzled...\n"},{"id":"266844","messageId":"CAOLa=ZQrBbgeYjnVaLf4EV+S1aRYBjJ-G==K_1J8XbGUCp5mmg@mail.gmail.com","threadId":"39934","inReplyTo":"xmqqvbd5ipy1.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4 05/10] ref-filter: add support to sort by version","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-27T17:03:59Z","receivedAt":"2015-07-27T17:03:59Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Jul 27, 2015 at 8:54 PM, Junio C Hamano wrote:\n> Karthik Nayak writes:\n>\n>> On Sun, Jul 26, 2015 at 4:10 AM, Junio C Hamano wrote:\n>>\n>>> Without looking at the callers, s->version looks like a misdesign\n>>> that should be updated to use the same cmp_type mechanism? That\n>>> would lead to even more obvious construct that is easy to enhance,\n>>> i.e.\n>>>\n>>> switch (cmp_type) {\n>>> case CMP_VERSION:\n>>> ...\n>>> case CMP_STRING:\n>>> ...\n>>> case CMP_NUMBER:\n>>> ...\n>>> }\n>>>\n>>> I dunno.\n>>>\n>>> Other than that (and the structure of that \"format-state\" stuff we\n>>> discussed separately), the series was a pleasant read.\n>>>\n>>> Thanks.\n>>\n>> That was the previous design, but Duy asked me to do this so\n>> that we could support all atoms. And I agree with him on this.\n>>\n>> http://article.gmane.org/gmane.comp.version-control.git/273888\n>\n> I am not objecting, but $gmane/273888 does not immediately read, at\n> least to me, as suggesting using a mechanism different from cmp_type\n> but a dedicated field s->version. Puzzled...\n>\n\nWhat I mean was since \"version\"/\"v\" aren't atoms as such, their processing is\ndone before we parse through atoms and fill used_atoms and used_atom_type.\ncmp_type is obtained from the used_atom_type, which isn't filled by\n\"version\"/\"v\".\nHence the dedicated field, just like reverse.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266918","messageId":"CACsJy8DMAUfOPHRZuOhf1dLABqhb_aP3LOLqBE7xo3_aMMSyYg@mail.gmail.com","threadId":"39934","inReplyTo":"CACsJy8Dr29B7Lei-jFqgPViZwd6BZcJde0jkcAwySYcY2v8QqA@mail.gmail.com","subject":"Re: [PATCH v4 01/10] ref-filter: add option to align atoms to the left","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-07-28T10:35:44Z","receivedAt":"2015-07-28T10:35:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Jul 27, 2015 at 5:18 PM, Duy Nguyen wrote:\n> On Mon, Jul 27, 2015 at 2:39 PM, Jacob Keller wrote:\n>> On Sun, Jul 26, 2015 at 5:39 PM, Duy Nguyen wrote:\n>>> On Sun, Jul 26, 2015 at 11:08 AM, Eric Sunshine wrote:\n>>>> You can generate an interdiff with \"git diff branchname-v4\n>>>> branchname-v5\", for instance.\n>>>\n>>> Off topic. But what stops me from doing this often is it creates a big\n>>> mess in \"git tag -l\". Do we have an option to hide away some\n>>> \"insignificant:\" tags? reflog might be an option if we have something\n>>> like foo@{/v2} to quickly retrieve the reflog entry whose message\n>>> contains \"v2\".\n> ...\n>\n> But maybe we're abusing reflog..\n\nActually a good place for this stuff is \"git branch\n--edit-description\". A lot of manual steps to save old refs, do\ninter-diff.. but it's probably good enough.\n-- \nDuy\n"}]}