{"thread":{"id":"39818","subject":"[PATCH v2 00/10] Port tag.c to use ref-filter APIs","startedAt":"2015-07-09T10:27:13Z","lastAt":"2015-07-13T20:36:15Z","messageCount":48,"participants":["Karthik Nayak","Christian Couder","Matthieu Moy","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":2,"patchTotal":10},"messages":[{"id":"265898","messageId":"CAOLa=ZQyHwza6L9r6iFX1GkVrC+F-XNwegO=bGyxafjY3JoYpw@mail.gmail.com","threadId":"39818","inReplyTo":null,"subject":"[PATCH v2 00/10] Port tag.c to use ref-filter APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-09T10:27:13Z","receivedAt":"2015-07-09T10:27:13Z","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\nThis patch series is continued from:\nhttp://article.gmane.org/gmane.comp.version-control.git/273569\n\nThe previous RFC version is here:\nhttp://thread.gmane.org/gmane.comp.version-control.git/272654\n\nChanges in this version:\n* Cleanup Documentation/tag\n* Fixed grammatical errors\n* Fixed a small merge conflict\n* Other small changes\n\n Documentation/git-tag.txt | 39 ++++++++++---\n builtin/for-each-ref.c | 3 +-\n builtin/tag.c | 368\n++++++++++++++++++++------------------------------------------------------------------------------------------------\n ref-filter.c | 95 ++++++++++++++++++++++++++++--\n ref-filter.h | 7 ++-\n t/t7004-tag.sh | 51 +++++++++++++---\n 6 files changed, 234 insertions(+), 329 deletions(-)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"265899","messageId":"1436437671-25600-1-git-send-email-karthik.188@gmail.com","threadId":"39818","inReplyTo":"CAOLa=ZQyHwza6L9r6iFX1GkVrC+F-XNwegO=bGyxafjY3JoYpw@mail.gmail.com","subject":"[PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-09T10:27:42Z","receivedAt":"2015-07-09T10:27:42Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Add support for %(refname:shortalign=X) where X is a number.\nThis will print a shortened refname aligned to the left\nfollowed by spaces for a total length of X characters.\nIf X is less than the shortened refname size, the entire\nshortened refname is printed.\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n ref-filter.c | 19 ++++++++++++++++++-\n 1 file changed, 18 insertions(+), 1 deletion(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex dd0709d..3098497 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -10,6 +10,7 @@\n #include \"quote.h\"\n #include \"ref-filter.h\"\n #include \"revision.h\"\n+#include \"utf8.h\"\n \n typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n \n@@ -695,7 +696,23 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\tint num_ours, num_theirs;\n \n \t\t\tformatp++;\n-\t\t\tif (!strcmp(formatp, \"short\"))\n+\t\t\tif (starts_with(formatp, \"shortalign=\")) {\n+\t\t\t\tconst char *valp, *short_refname = NULL;\n+\t\t\t\tint val, len;\n+\n+\t\t\t\tskip_prefix(formatp, \"shortalign=\", &valp);\n+\t\t\t\tval = atoi(valp);\n+\t\t\t\trefname = short_refname = shorten_unambiguous_ref(refname,\n+\t\t\t\t\t\t\t\t\t\t warn_ambiguous_refs);\n+\t\t\t\tlen = utf8_strwidth(refname);\n+\t\t\t\tif (val > len) {\n+\t\t\t\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\t\t\t\tstrbuf_addstr(&buf, refname);\n+\t\t\t\t\tstrbuf_addchars(&buf, ' ', val - len);\n+\t\t\t\t\tfree((char *)short_refname);\n+\t\t\t\t\trefname = strbuf_detach(&buf, NULL);\n+\t\t\t\t}\n+\t\t\t} else if (!strcmp(formatp, \"short\"))\n \t\t\t\trefname = shorten_unambiguous_ref(refname,\n \t\t\t\t\t\t warn_ambiguous_refs);\n \t\t\telse if (!strcmp(formatp, \"track\") &&\n-- \n2.4.5\n"},{"id":"265904","messageId":"1436437671-25600-2-git-send-email-karthik.188@gmail.com","threadId":"39818","inReplyTo":"1436437671-25600-1-git-send-email-karthik.188@gmail.com","subject":"[PATCH v2 02/10] ref-filter: add option to filter only tags","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-09T10:27:43Z","receivedAt":"2015-07-09T10:27:43Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Add an option in 'filter_refs()' to use 'for_each_tag_ref()'\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 2 files changed, 3 insertions(+)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 3098497..e690177 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1152,6 +1152,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n \t\tret = for_each_rawref(ref_filter_handler, &ref_cbdata);\n \telse if (type & FILTER_REFS_ALL)\n \t\tret = for_each_ref(ref_filter_handler, &ref_cbdata);\n+\telse if (type & FILTER_REFS_TAGS)\n+\t\tret = for_each_tag_ref(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 6bf27d8..449ddb8 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;\n-- \n2.4.5\n"},{"id":"265905","messageId":"1436437671-25600-3-git-send-email-karthik.188@gmail.com","threadId":"39818","inReplyTo":"1436437671-25600-1-git-send-email-karthik.188@gmail.com","subject":"[PATCH v2 03/10] ref-filter: support printing N lines from tag annotation","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-09T10:27:44Z","receivedAt":"2015-07-09T10:27:44Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"In 'tag.c' we can print N lines from the annotation of the tag\nusing the '-n' option. Copy code from 'tag.c' to 'ref-filter'\nand modify 'ref-filter' to support printing of N lines from the\nannotation of 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 | 48 +++++++++++++++++++++++++++++++++++++++++++++++-\n ref-filter.h | 3 ++-\n 4 files changed, 54 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 40f343b..e4a4f8a 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -74,7 +74,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \tif (!maxcount || array.nr < maxcount)\n \t\tmaxcount = array.nr;\n \tfor (i = 0; i < maxcount; i++)\n-\t\tshow_ref_array_item(array.items[i], format, quote_style);\n+\t\tshow_ref_array_item(array.items[i], format, quote_style, 0);\n \tref_array_clear(&array);\n \treturn 0;\n }\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 071d001..3b6a6cc 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 dupplicated 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 e690177..1b088b1 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1273,7 +1273,48 @@ 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+/* Print 'lines' no of lines of a given oid */\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 \n@@ -1299,6 +1340,11 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n \t\tresetv.s = color;\n \t\tprint_value(&resetv, quote_style);\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 449ddb8..5247475 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -55,6 +55,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@@ -87,7 +88,7 @@ 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+void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style, unsigned int lines);\n /* Callback function for parsing the sort option */\n int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset);\n /* Default sort option based on refname */\n-- \n2.4.5\n"},{"id":"265902","messageId":"1436437671-25600-4-git-send-email-karthik.188@gmail.com","threadId":"39818","inReplyTo":"1436437671-25600-1-git-send-email-karthik.188@gmail.com","subject":"[PATCH v2 04/10] ref-filter: add support to sort by version","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-09T10:27:45Z","receivedAt":"2015-07-09T10:27:45Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Add support to sort by version using the \"v:refname\" and\n\"version:refname\" option. This is achieved by using the\n'version_cmp()' function as the comparing function for qsort.\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n ref-filter.c | 20 +++++++++++++++++---\n 1 file changed, 17 insertions(+), 3 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 1b088b1..1516cd1 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -11,8 +11,9 @@\n #include \"ref-filter.h\"\n #include \"revision.h\"\n #include \"utf8.h\"\n+#include \"version.h\"\n \n-typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n+typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME, FIELD_VER } cmp_type;\n \n static struct {\n \tconst char *name;\n@@ -54,6 +55,8 @@ static struct {\n \t{ \"flag\" },\n \t{ \"HEAD\" },\n \t{ \"color\" },\n+\t{ \"version\", FIELD_VER },\n+\t{ \"v\", FIELD_VER },\n };\n \n /*\n@@ -630,7 +633,9 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\tname++;\n \t\t}\n \n-\t\tif (starts_with(name, \"refname\"))\n+\t\tif (starts_with(name, \"refname\") ||\n+\t\t starts_with(name, \"version:\") ||\n+\t\t starts_with(name, \"v:\"))\n \t\t\trefname = ref->refname;\n \t\telse if (starts_with(name, \"symref\"))\n \t\t\trefname = ref->symref ? ref->symref : \"\";\n@@ -696,7 +701,13 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\tint num_ours, num_theirs;\n \n \t\t\tformatp++;\n-\t\t\tif (starts_with(formatp, \"shortalign=\")) {\n+\t\t\tif (starts_with(name, \"version\") || starts_with(name, \"v\")) {\n+\t\t\t\tif (strcmp(formatp, \"refname\"))\n+\t\t\t\t\tdie(\"unknown %.*s format %s\",\n+\t\t\t\t\t (int)(formatp - name), name, formatp);\n+\t\t\t\tv->s = refname;\n+\t\t\t\tcontinue;\n+\t\t\t} else if (starts_with(formatp, \"shortalign=\")) {\n \t\t\t\tconst char *valp, *short_refname = NULL;\n \t\t\t\tint val, len;\n \n@@ -1176,6 +1187,9 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru\n \tcase FIELD_STR:\n \t\tcmp = strcmp(va->s, vb->s);\n \t\tbreak;\n+\tcase FIELD_VER:\n+\t\tcmp = versioncmp(va->s, vb->s);\n+\t\tbreak;\n \tdefault:\n \t\tif (va->ul < vb->ul)\n \t\t\tcmp = -1;\n-- \n2.4.5\n"},{"id":"265900","messageId":"1436437671-25600-5-git-send-email-karthik.188@gmail.com","threadId":"39818","inReplyTo":"1436437671-25600-1-git-send-email-karthik.188@gmail.com","subject":"[PATCH v2 05/10] ref-filter: add option to match literal pattern","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-09T10:27:46Z","receivedAt":"2015-07-09T10:27:46Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Since 'ref-filter' only has an option to match path names\nadd an option for regular pattern matching.\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 | 25 ++++++++++++++++++++++++-\n ref-filter.h | 3 ++-\n 3 files changed, 27 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 1516cd1..0e19f44 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -956,6 +956,20 @@ 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/m\") or a wildcard (e.g. the same ref\n+ * matches \"refs/heads/m*\",too).\n+ */\n+static int match_pattern(const char **patterns, const char *refname)\n+{\n+\tfor (; *patterns; patterns++)\n+\t\tif (!wildmatch(*patterns, refname, 0, NULL))\n+\t\t\treturn 1;\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@@ -979,6 +993,15 @@ static int match_name_as_path(const char **pattern, const char *refname)\n \treturn 0;\n }\n \n+static int filter_pattern_match(struct ref_filter *filter, const char *refname)\n+{\n+\tif (!*filter->name_patterns)\n+\t\treturn 1;\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@@ -1047,7 +1070,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 5247475..633108c 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -54,7 +54,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.5\n"},{"id":"265901","messageId":"1436437671-25600-6-git-send-email-karthik.188@gmail.com","threadId":"39818","inReplyTo":"1436437671-25600-1-git-send-email-karthik.188@gmail.com","subject":"[PATCH v2 06/10] Documentation/tag: remove double occurance of \"\"","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-09T10:27:47Z","receivedAt":"2015-07-09T10:27:47Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Mentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n Documentation/git-tag.txt | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 034d10d..4b04c2b 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -14,7 +14,6 @@ SYNOPSIS\n 'git tag' -d ...\n 'git tag' [-n[]] -l [--contains ] [--points-at ]\n \t[--column[=] | --no-column] [...]\n-\t[...]\n 'git tag' -v ...\n \n DESCRIPTION\n-- \n2.4.5\n"},{"id":"265903","messageId":"1436437671-25600-7-git-send-email-karthik.188@gmail.com","threadId":"39818","inReplyTo":"1436437671-25600-1-git-send-email-karthik.188@gmail.com","subject":"[PATCH v2 07/10] tag.c: use 'ref-filter' data structures","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-09T10:27:48Z","receivedAt":"2015-07-09T10:27:48Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Make '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 3b6a6cc..bbda0c6 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.5\n"},{"id":"265906","messageId":"559E5333.7080901@gmail.com","threadId":"39818","inReplyTo":"1436437671-25600-1-git-send-email-karthik.188@gmail.com","subject":"[PATCH v2 08/10] tag.c: use 'ref-filter' APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-09T10:55:47Z","receivedAt":"2015-07-09T10:55:47Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Make 'tag.c' use 'ref-filter' APIs for iterating through refs\nsorting and printing of refs. This removes most of the code\nused in 'tag.c' replacing it with calls to the 'ref-filter'\nlibrary.\n\nMake 'tag.c' use the 'filter_refs()' function provided by\n'ref-filter' to filter out tags based on the options set.\n\nFor printing tags we use 'show_ref_array_item()' function\nprovided by 'ref-filter'.\n\nWe improve the sorting option provided by 'tag.c' by using the\nsorting options provided by 'ref-filter'. This causes the test\n'invalid sort parameter on command line' in t7004 to fail, as\n'ref-filter' throws an error for all sorting fields which are\nincorrect. The test is changed to 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 | 343 \n++++++----------------------------------------\n t/t7004-tag.sh | 8 +-\n 3 files changed, 50 insertions(+), 317 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 bbda0c6..36f8019 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -28,278 +28,34 @@ 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 \nlist.\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 dupplicated 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) \n+ 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, \n&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 \n*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 = \"%(refname:shortalign=16)\";\n+\telse\n+\t\tformat = \"%(refname:short)\";\n+\n+\tverify_ref_format(format);\n+\tif (!sorting)\n+\t\tsorting = ref_default_sorting();\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 +122,23 @@ 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 \nvar is\n- * given, the error message becomes a warning and includes information \nabout\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 \n**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-\t}\n+\tif (*arg == '-') {\n+\t\ts->reverse = 1;\n+\t\targ++;\n+ \t}\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 +146,12 @@ static int parse_sort_string(const char *var, \nconst 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 +309,6 @@ static int strbuf_check_tag_ref(struct strbuf *sb, \nconst 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, \nint 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 +324,7 @@ int cmd_tag(int argc, const char **argv, const char \n*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 +350,8 @@ int cmd_tag(int argc, const char **argv, const char \n*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 \nthe commit\")),\n \t\tOPT_WITH(&filter.with_commit, N_(\"print only tags that contain the \ncommit\")),\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 +359,7 @@ int cmd_tag(int argc, const char **argv, const char \n*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 +385,8 @@ int cmd_tag(int argc, const char **argv, const char \n*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 +395,8 @@ int cmd_tag(int argc, const char **argv, const char \n*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 \ncommand 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.5\n"},{"id":"265907","messageId":"559E53DC.2040804@gmail.com","threadId":"39818","inReplyTo":"1436437671-25600-1-git-send-email-karthik.188@gmail.com","subject":"[PATCH v2 08/10] tag.c: use 'ref-filter' APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-09T10:58:36Z","receivedAt":"2015-07-09T10:58:36Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Make 'tag.c' use 'ref-filter' APIs for iterating through refs\nsorting and printing of refs. This removes most of the code\nused in 'tag.c' replacing it with calls to the 'ref-filter'\nlibrary.\n\nMake 'tag.c' use the 'filter_refs()' function provided by\n'ref-filter' to filter out tags based on the options set.\n\nFor printing tags we use 'show_ref_array_item()' function\nprovided by 'ref-filter'.\n\nWe improve the sorting option provided by 'tag.c' by using the\nsorting options provided by 'ref-filter'. This causes the test\n'invalid sort parameter on command line' in t7004 to fail, as\n'ref-filter' throws an error for all sorting fields which are\nincorrect. The test is changed to 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 | 343 \n++++++----------------------------------------\n t/t7004-tag.sh | 8 +-\n 3 files changed, 50 insertions(+), 317 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 bbda0c6..36f8019 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -28,278 +28,34 @@ 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 \nlist.\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 dupplicated 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) \n+ 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, \n&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 \n*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 = \"%(refname:shortalign=16)\";\n+\telse\n+\t\tformat = \"%(refname:short)\";\n+\n+\tverify_ref_format(format);\n+\tif (!sorting)\n+\t\tsorting = ref_default_sorting();\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 +122,23 @@ 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 \nvar is\n- * given, the error message becomes a warning and includes information \nabout\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 \n**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-\t}\n+\tif (*arg == '-') {\n+\t\ts->reverse = 1;\n+\t\targ++;\n+ \t}\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 +146,12 @@ static int parse_sort_string(const char *var, \nconst 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 +309,6 @@ static int strbuf_check_tag_ref(struct strbuf *sb, \nconst 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, \nint 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 +324,7 @@ int cmd_tag(int argc, const char **argv, const char \n*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 +350,8 @@ int cmd_tag(int argc, const char **argv, const char \n*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 \nthe commit\")),\n \t\tOPT_WITH(&filter.with_commit, N_(\"print only tags that contain the \ncommit\")),\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 +359,7 @@ int cmd_tag(int argc, const char **argv, const char \n*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 +385,8 @@ int cmd_tag(int argc, const char **argv, const char \n*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 +395,8 @@ int cmd_tag(int argc, const char **argv, const char \n*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 \ncommand 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.5\n"},{"id":"265908","messageId":"559E5422.4070802@gmail.com","threadId":"39818","inReplyTo":"1436437671-25600-1-git-send-email-karthik.188@gmail.com","subject":"[PATCH v2 09/10] tag.c: implement '--format' option","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-09T10:59:46Z","receivedAt":"2015-07-09T10:59:46Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Implement 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 | 16 +++++++++++++++-\n builtin/tag.c | 11 +++++++----\n t/t7004-tag.sh | 16 ++++++++++++++++\n 3 files changed, 38 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 02fb363..16e396c 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,19 @@ This option is only applicable when listing tags \nwithout 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\n+\tobject pointed at by a ref being shown. If `fieldname`\n+\tis prefixed with an asterisk (`*`) and the ref points\n+\tat a tag object, the value for the field in the object\n+\ttag refers is used. When unspecified, defaults to\n+\t`%(objectname) SPC %(objecttype) TAB %(refname)`.\n+\tIt also interpolates `%%` to `%`, and `%xx` where `xx`\n+\tare hex digits interpolates to character with hex code\n+\t`xx`; for example `%00` interpolates to `\\0` (NUL),\n+\t`%09` to `\\t` (TAB) and `%0a` to `\\n` (LF).\n+\tThe fields are same as those in `git for-each-ref`.\n+\n\n CONFIGURATION\n -------------\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 36f8019..601b293 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 \n*sorting)\n+static int list_tags(struct ref_filter *filter, struct ref_sorting \n*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 \nref_sorting *sorting)\n\n \tif (filter->lines)\n \t\tformat = \"%(refname:shortalign=16)\";\n-\telse\n+\telse if (!format)\n \t\tformat = \"%(refname:short)\";\n\n \tverify_ref_format(format);\n@@ -325,6 +324,7 @@ int cmd_tag(int argc, const char **argv, const char \n*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@@ -356,6 +356,7 @@ int cmd_tag(int argc, const char **argv, const char \n*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 \nfor the output\")),\n \t\tOPT_END()\n \t};\n\n@@ -395,8 +396,10 @@ int cmd_tag(int argc, const char **argv, const char \n*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..e8cebb6 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+\tfoo1.10\n+\tfoo1.3\n+\tfoo1.6\n+\tfoo1.6-rc1\n+\tfoo1.6-rc2\n+\tEOF\n+\tgit tag -l --format=\"%(refname)\" \"foo*\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.4.5\n"},{"id":"265909","messageId":"559E544E.2060508@gmail.com","threadId":"39818","inReplyTo":"1436437671-25600-1-git-send-email-karthik.188@gmail.com","subject":"[PATCH v2 10/10] tag.c: implement '--merged' and '--no-merged' options","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-09T11:00:30Z","receivedAt":"2015-07-09T11:00:30Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Using '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 | 10 +++++++++-\n builtin/tag.c | 6 +++++-\n t/t7004-tag.sh | 27 +++++++++++++++++++++++++++\n 3 files changed, 41 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 16e396c..74ed157 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[(--merged | --no-merged) []] [...]\n 'git tag' -v ...\n\n DESCRIPTION\n@@ -169,6 +169,14 @@ This option is only applicable when listing tags \nwithout annotation lines.\n \t`%09` to `\\t` (TAB) and `%0a` to `\\n` (LF).\n \tThe fields are same as those in `git for-each-ref`.\n\n+--merged []::\n+\tOnly list tags whose tips are reachable from the\n+\tspecified commit (HEAD if not specified).\n+\n+--no-merged []::\n+\tOnly list tags whose tips are not reachable from the\n+\tspecified commit (HEAD if not specified).\n+\n\n CONFIGURATION\n -------------\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 601b293..abd42a0 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 []\"),\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[--merged []] [--no-merged []] [...]\"),\n \tN_(\"git tag -v ...\"),\n \tNULL\n };\n@@ -350,6 +350,8 @@ int cmd_tag(int argc, const char **argv, const char \n*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 \nthe commit\")),\n \t\tOPT_WITH(&filter.with_commit, N_(\"print only tags that contain the \ncommit\")),\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@@ -410,6 +412,8 @@ int cmd_tag(int argc, const char **argv, const char \n*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 e8cebb6..873aad3 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 \nper 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.5\n"},{"id":"265911","messageId":"CAP8UFD0mSZ_FyPcaMoWRbrR=i9vFJvVYyqM+0B-3L3dQpA6SsA@mail.gmail.com","threadId":"39818","inReplyTo":"1436437671-25600-6-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v2 06/10] Documentation/tag: remove double occurance of \"\"","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-07-09T12:19:01Z","receivedAt":"2015-07-09T12:19:01Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Jul 9, 2015 at 12:27 PM, Karthik Nayak wrote:\n> Mentored-by: Christian Couder \n> Mentored-by: Matthieu Moy \n> Signed-off-by: Karthik Nayak \n> ---\n> Documentation/git-tag.txt | 1 -\n> 1 file changed, 1 deletion(-)\n>\n> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n> index 034d10d..4b04c2b 100644\n> --- a/Documentation/git-tag.txt\n> +++ b/Documentation/git-tag.txt\n> @@ -14,7 +14,6 @@ SYNOPSIS\n> 'git tag' -d ...\n> 'git tag' [-n[]] -l [--contains ] [--points-at ]\n> [--column[=] | --no-column] [...]\n> - [...]\n> 'git tag' -v ...\n\nAs this patch could be applied directly to master and to maint maybe\nyou could send it at the top of this patch series or alone outside of\nthis patch series.\n"},{"id":"265912","messageId":"vpqa8v58pka.fsf@anie.imag.fr","threadId":"39818","inReplyTo":"559E5333.7080901@gmail.com","subject":"Re: [PATCH v2 08/10] tag.c: use 'ref-filter' APIs","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-09T12:48:21Z","receivedAt":"2015-07-09T12:48:21Z","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> Documentation/git-tag.txt | 16 ++-\n> builtin/tag.c | 343\n> ++++++----------------------------------------\n> t/t7004-tag.sh | 8 +-\n\nThis patch was sent with Thunderbird unlike the others in the series. It\nhas some line wrapping which make it unapplicable.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265913","messageId":"CAOLa=ZSp-X0Mo5BGWghdk56sM2AEAmnXZt7Y3VkfXWvf8rh96w@mail.gmail.com","threadId":"39818","inReplyTo":"vpqa8v58pka.fsf@anie.imag.fr","subject":"Re: [PATCH v2 08/10] tag.c: use 'ref-filter' APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-09T12:55:57Z","receivedAt":"2015-07-09T12:55:57Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Thu, Jul 9, 2015 at 6:18 PM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> Documentation/git-tag.txt | 16 ++-\n>> builtin/tag.c | 343\n>> ++++++----------------------------------------\n>> t/t7004-tag.sh | 8 +-\n>\n> This patch was sent with Thunderbird unlike the others in the series. It\n> has some line wrapping which make it unapplicable.\n>\n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n\nYes, 'git send-email' started failing, hence 08/10, 09/10 and 10/10\nwere sent by thunderbird.\nI'm trying to figure out why.\nIf anyone can help, this is what it's saying.\n\"[Net::SMTP::SSL] Connection closed at\n/usr/lib/git-core/git-send-email line 1320.\nfatal: 'send-email' appears to be a git command, but we were not\nable to execute it. Maybe git-send-email is broken?\"\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"265914","messageId":"CAOLa=ZQpjXv3UBnnPpqpgDPw_tUzCQQ4EgdhW46QOfLd+3VGWQ@mail.gmail.com","threadId":"39818","inReplyTo":"CAP8UFD0mSZ_FyPcaMoWRbrR=i9vFJvVYyqM+0B-3L3dQpA6SsA@mail.gmail.com","subject":"Re: [PATCH v2 06/10] Documentation/tag: remove double occurance of \"\"","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-09T12:56:34Z","receivedAt":"2015-07-09T12:56:34Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Thu, Jul 9, 2015 at 5:49 PM, Christian Couder\n wrote:\n> On Thu, Jul 9, 2015 at 12:27 PM, Karthik Nayak wrote:\n>> Mentored-by: Christian Couder \n>> Mentored-by: Matthieu Moy \n>> Signed-off-by: Karthik Nayak \n>> ---\n>> Documentation/git-tag.txt | 1 -\n>> 1 file changed, 1 deletion(-)\n>>\n>> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n>> index 034d10d..4b04c2b 100644\n>> --- a/Documentation/git-tag.txt\n>> +++ b/Documentation/git-tag.txt\n>> @@ -14,7 +14,6 @@ SYNOPSIS\n>> 'git tag' -d ...\n>> 'git tag' [-n[]] -l [--contains ] [--points-at ]\n>> [--column[=] | --no-column] [...]\n>> - [...]\n>> 'git tag' -v ...\n>\n> As this patch could be applied directly to master and to maint maybe\n> you could send it at the top of this patch series or alone outside of\n> this patch series.\n\nI'll do that :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"265915","messageId":"vpq380x8p3a.fsf@anie.imag.fr","threadId":"39818","inReplyTo":"1436437671-25600-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-09T12:58:33Z","receivedAt":"2015-07-09T12:58:33Z","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> Add support for %(refname:shortalign=X) where X is a number.\n> This will print a shortened refname aligned to the left\n> followed by spaces for a total length of X characters.\n> If X is less than the shortened refname size, the entire\n> shortened refname is printed.\n\nNot really an issue, but you're wrapping your text at ~60 characters.\nThe common use is to wrap around 70 to 80. Using Emacs, auto-fill-mode\nor M-q does this automatically. If you use another text editor, it can\nprobably do that for you too.\n\n> Mentored-by: Christian Couder \n> Mentored-by: Matthieu Moy \n> Signed-off-by: Karthik Nayak \n> ---\n> ref-filter.c | 19 ++++++++++++++++++-\n\nI think this would deserve a test and documentation. Even though your\nmotivation is for an internal implementation, some users may want to use\nthe feature in 'git for-each-ref --format=...'.\n\n> 1 file changed, 18 insertions(+), 1 deletion(-)\n>\n> diff --git a/ref-filter.c b/ref-filter.c\n> index dd0709d..3098497 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -10,6 +10,7 @@\n> #include \"quote.h\"\n> #include \"ref-filter.h\"\n> #include \"revision.h\"\n> +#include \"utf8.h\"\n> \n> typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n> \n> @@ -695,7 +696,23 @@ static void populate_value(struct ref_array_item *ref)\n> \t\t\tint num_ours, num_theirs;\n> \n> \t\t\tformatp++;\n> -\t\t\tif (!strcmp(formatp, \"short\"))\n> +\t\t\tif (starts_with(formatp, \"shortalign=\")) {\n> +\t\t\t\tconst char *valp, *short_refname = NULL;\n> +\t\t\t\tint val, len;\n> +\n> +\t\t\t\tskip_prefix(formatp, \"shortalign=\", &valp);\n> +\t\t\t\tval = atoi(valp);\n\nYou're silently accepting %(refname:shortalign=foo) and\n%(refname:shortalign=). I think it would be better to reject such cases\nexplicitly.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265916","messageId":"vpqpp417a4i.fsf@anie.imag.fr","threadId":"39818","inReplyTo":"1436437671-25600-3-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v2 03/10] ref-filter: support printing N lines from tag annotation","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-09T13:07:09Z","receivedAt":"2015-07-09T13:07:09Z","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> In 'tag.c' we can print N lines from the annotation of the tag\n> using the '-n' option.\n\nNot only annotation of the tag, but also from the commit message for\nlightweight tags.\n\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 dupplicated in ref-filter, will eventually be removed as\n\ndupplicated -> duplicated.\n\n> --- a/ref-filter.h\n> +++ b/ref-filter.h\n> @@ -55,6 +55,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> @@ -87,7 +88,7 @@ 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> +void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style, unsigned int lines);\n\nI would add \"If lines > 0, prints the first 'lines' no of lines of the\nobject pointed to\" or so to the docstring.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265917","messageId":"vpqfv4x792z.fsf@anie.imag.fr","threadId":"39818","inReplyTo":"1436437671-25600-4-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v2 04/10] ref-filter: add support to sort by version","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-09T13:29:40Z","receivedAt":"2015-07-09T13:29:40Z","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> Add support to sort by version using the \"v:refname\" and\n> \"version:refname\" option. This is achieved by using the\n> 'version_cmp()' function as the comparing function for qsort.\n\nYou should elaborate on why you need this. Given the context, I can\nguess that you will need this to implement tag, but for example I first\nwondered why you needed both version: and v:, but I guess it comes from\nthe fact that 'git tag --sort' can take version:refname or v:refname.\n\nI think this deserves a test and documentation in for-each-ref.txt.\n\nAs-is, the code is a bit hard to understand. I first saw you were\nallowing\n\n git for-each-ref --format '%(version:refname)'\n\n(which you are, but only as a side effect), and then understood that\nthis was also allowing\n\n git for-each-ref --sort version:refname\n\nA test would have shown me this immediately. Some hints in the commit\nmessage would clearly have helped too.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265918","messageId":"vpq8uap78xu.fsf@anie.imag.fr","threadId":"39818","inReplyTo":"1436437671-25600-5-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v2 05/10] ref-filter: add option to match literal pattern","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-09T13:32:45Z","receivedAt":"2015-07-09T13:32:45Z","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> Since 'ref-filter' only has an option to match path names\n> add an option for regular pattern matching.\n\nHere also, a hint on why this is needed would be welcome.\n\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -956,6 +956,20 @@ 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/m\") or a wildcard (e.g. the same ref\n> + * matches \"refs/heads/m*\",too).\n\nMissing space after , (same in the hunk context below)\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265919","messageId":"vpq1tgh78k0.fsf@anie.imag.fr","threadId":"39818","inReplyTo":"559E5333.7080901@gmail.com","subject":"Re: [PATCH v2 08/10] tag.c: use 'ref-filter' APIs","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-09T13:41:03Z","receivedAt":"2015-07-09T13:41:03Z","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> +\ts->atom = parse_ref_filter_atom(arg, arg+len);\n\nSpaces around +.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265920","messageId":"vpqvbdt5tv6.fsf@anie.imag.fr","threadId":"39818","inReplyTo":"CAOLa=ZSp-X0Mo5BGWghdk56sM2AEAmnXZt7Y3VkfXWvf8rh96w@mail.gmail.com","subject":"Re: [PATCH v2 08/10] tag.c: use 'ref-filter' APIs","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-09T13:43:41Z","receivedAt":"2015-07-09T13:43:41Z","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> If anyone can help, this is what it's saying.\n> \"[Net::SMTP::SSL] Connection closed at\n\nPerhaps your SMTP server thought you were sending too many emails to too\nmany people and closed the connection thinking you were a spammer.\n\nIf you're having this kind of issues, it may make sense to run\n\"format-patch\" and \"send-email\" as two separate steps. This way, you can\nre-run \"send-email\" on the pieces which failed manually (adjusting\n--in-reply-to).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265977","messageId":"CAOLa=ZTonG-TVwxdcmt3rc_5c5Vm+uGgXo_Mc4mNe6n_VKb7KA@mail.gmail.com","threadId":"39818","inReplyTo":"vpqvbdt5tv6.fsf@anie.imag.fr","subject":"Re: [PATCH v2 08/10] tag.c: use 'ref-filter' APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-10T09:41:49Z","receivedAt":"2015-07-10T09:41:49Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Thu, Jul 9, 2015 at 7:13 PM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> If anyone can help, this is what it's saying.\n>> \"[Net::SMTP::SSL] Connection closed at\n>\n> Perhaps your SMTP server thought you were sending too many emails to too\n> many people and closed the connection thinking you were a spammer.\n>\n> If you're having this kind of issues, it may make sense to run\n> \"format-patch\" and \"send-email\" as two separate steps. This way, you can\n> re-run \"send-email\" on the pieces which failed manually (adjusting\n> --in-reply-to).\n>\n\nI'm guessing the same. That's what I usually do, separate format-patch and\nsend-email.\n\n>Spaces around +.\n\nThanks :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"265979","messageId":"CAOLa=ZTTLHZ_C_9m1ctHkjazhC+6r4U_u6j2xkjsCAXtBDr1RQ@mail.gmail.com","threadId":"39818","inReplyTo":"vpqpp417a4i.fsf@anie.imag.fr","subject":"Re: [PATCH v2 03/10] ref-filter: support printing N lines from tag annotation","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-10T10:38:33Z","receivedAt":"2015-07-10T10:38:33Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Thu, Jul 9, 2015 at 6:37 PM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> In 'tag.c' we can print N lines from the annotation of the tag\n>> using the '-n' option.\n>\n> Not only annotation of the tag, but also from the commit message for\n> lightweight tags.\n\nwill do.\n\n>\n>> --- a/builtin/tag.c\n>> +++ b/builtin/tag.c\n>> @@ -185,6 +185,10 @@ static enum contains_result contains(struct commit *candidate,\n>> return contains_test(candidate, want);\n>> }\n>>\n>> +/*\n>> + * Currently dupplicated in ref-filter, will eventually be removed as\n>\n> dupplicated -> duplicated.\n>\n\nThanks :)\n\n>> --- a/ref-filter.h\n>> +++ b/ref-filter.h\n>> @@ -55,6 +55,7 @@ struct ref_filter {\n>> struct commit *merge_commit;\n>>\n>> unsigned int with_commit_tag_algo : 1;\n>> + unsigned int lines;\n>> };\n>>\n>> struct ref_filter_cbdata {\n>> @@ -87,7 +88,7 @@ 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>> +void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style, unsigned int lines);\n>\n> I would add \"If lines > 0, prints the first 'lines' no of lines of the\n> object pointed to\" or so to the docstring.\n>\n\nWill do, thanks.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"265981","messageId":"CAOLa=ZRs560_YBVMTpfKBvB1BF6ts+X_JGFTzhp7A_+YsjwqqQ@mail.gmail.com","threadId":"39818","inReplyTo":"vpqfv4x792z.fsf@anie.imag.fr","subject":"Re: [PATCH v2 04/10] ref-filter: add support to sort by version","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-10T10:52:35Z","receivedAt":"2015-07-10T10:52:35Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Thu, Jul 9, 2015 at 6:59 PM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> Add support to sort by version using the \"v:refname\" and\n>> \"version:refname\" option. This is achieved by using the\n>> 'version_cmp()' function as the comparing function for qsort.\n>\n> You should elaborate on why you need this. Given the context, I can\n> guess that you will need this to implement tag, but for example I first\n> wondered why you needed both version: and v:, but I guess it comes from\n> the fact that 'git tag --sort' can take version:refname or v:refname.\n>\n> I think this deserves a test and documentation in for-each-ref.txt.\n\nI'll add it to \"for-each-ref.txt\" documentation.\nAbout the tests, there are already tests for the same in git-tag.txt and\nthat's the only reason I did not repeat the tests in for-each-ref.\n\n>\n> As-is, the code is a bit hard to understand. I first saw you were\n> allowing\n>\n> git for-each-ref --format '%(version:refname)'\n>\n> (which you are, but only as a side effect), and then understood that\n> this was also allowing\n>\n> git for-each-ref --sort version:refname\n>\n> A test would have shown me this immediately. Some hints in the commit\n> message would clearly have helped too.\n>\n\nMost of the sorting options have side effects in \"--format\", but yeah will add\ndetails in the commit message.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"265982","messageId":"CAOLa=ZROk=+mW8uVjpuxMVY9NP0eW4yk2Za8xYW5MAFWSGzXRw@mail.gmail.com","threadId":"39818","inReplyTo":"CAOLa=ZRs560_YBVMTpfKBvB1BF6ts+X_JGFTzhp7A_+YsjwqqQ@mail.gmail.com","subject":"Re: [PATCH v2 04/10] ref-filter: add support to sort by version","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-10T11:01:34Z","receivedAt":"2015-07-10T11:01:34Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Fri, Jul 10, 2015 at 4:22 PM, Karthik Nayak wrote:\n> On Thu, Jul 9, 2015 at 6:59 PM, Matthieu Moy\n> wrote:\n>> Karthik Nayak writes:\n>>\n>>> Add support to sort by version using the \"v:refname\" and\n>>> \"version:refname\" option. This is achieved by using the\n>>> 'version_cmp()' function as the comparing function for qsort.\n>>\n>> You should elaborate on why you need this. Given the context, I can\n>> guess that you will need this to implement tag, but for example I first\n>> wondered why you needed both version: and v:, but I guess it comes from\n>> the fact that 'git tag --sort' can take version:refname or v:refname.\n>>\n>> I think this deserves a test and documentation in for-each-ref.txt.\n>\n> I'll add it to \"for-each-ref.txt\" documentation.\n> About the tests, there are already tests for the same in git-tag.txt and\n> that's the only reason I did not repeat the tests in for-each-ref.\n>\n\nBut since the porting is in a later commit, will add tests to for-each-ref.\n\n>>\n>> As-is, the code is a bit hard to understand. I first saw you were\n>> allowing\n>>\n>> git for-each-ref --format '%(version:refname)'\n>>\n>> (which you are, but only as a side effect), and then understood that\n>> this was also allowing\n>>\n>> git for-each-ref --sort version:refname\n>>\n>> A test would have shown me this immediately. Some hints in the commit\n>> message would clearly have helped too.\n>>\n>\n> Most of the sorting options have side effects in \"--format\", but yeah will add\n> details in the commit message.\n>\n> --\n> Regards,\n> Karthik Nayak\n\n\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"265983","messageId":"CAOLa=ZTr=C49K4d_zdD56mGoiiKKO_M35cagvTfCz06paxh=ew@mail.gmail.com","threadId":"39818","inReplyTo":"vpq8uap78xu.fsf@anie.imag.fr","subject":"Re: [PATCH v2 05/10] ref-filter: add option to match literal pattern","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-10T11:11:52Z","receivedAt":"2015-07-10T11:11:52Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Thu, Jul 9, 2015 at 7:02 PM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> Since 'ref-filter' only has an option to match path names\n>> add an option for regular pattern matching.\n>\n> Here also, a hint on why this is needed would be welcome.\n>\n\nWill add.\n\n>> --- a/ref-filter.c\n>> +++ b/ref-filter.c\n>> @@ -956,6 +956,20 @@ 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/m\") or a wildcard (e.g. the same ref\n>> + * matches \"refs/heads/m*\",too).\n>\n> Missing space after , (same in the hunk context below)\n>\n\nNoted. Thanks :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"265984","messageId":"vpqd2005hq4.fsf@anie.imag.fr","threadId":"39818","inReplyTo":"CAOLa=ZROk=+mW8uVjpuxMVY9NP0eW4yk2Za8xYW5MAFWSGzXRw@mail.gmail.com","subject":"Re: [PATCH v2 04/10] ref-filter: add support to sort by version","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-10T12:18:11Z","receivedAt":"2015-07-10T12:18:11Z","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> On Fri, Jul 10, 2015 at 4:22 PM, Karthik Nayak wrote:\n>> On Thu, Jul 9, 2015 at 6:59 PM, Matthieu Moy\n>> wrote:\n>>> Karthik Nayak writes:\n>>>\n>>>> Add support to sort by version using the \"v:refname\" and\n>>>> \"version:refname\" option. This is achieved by using the\n>>>> 'version_cmp()' function as the comparing function for qsort.\n>>>\n>>> You should elaborate on why you need this. Given the context, I can\n>>> guess that you will need this to implement tag, but for example I first\n>>> wondered why you needed both version: and v:, but I guess it comes from\n>>> the fact that 'git tag --sort' can take version:refname or v:refname.\n>>>\n>>> I think this deserves a test and documentation in for-each-ref.txt.\n>>\n>> I'll add it to \"for-each-ref.txt\" documentation.\n>> About the tests, there are already tests for the same in git-tag.txt and\n>> that's the only reason I did not repeat the tests in for-each-ref.\n>>\n>\n> But since the porting is in a later commit, will add tests to for-each-ref.\n\nYes: to me \"it's tested through 'git tag'\" is a good argument to do only\na superficial test, check that 'for-each-ref --sort v:refname' activate\nthe sorting, but no detailed corner-case testing. But not a good\nargument to have no test at all on for-each-ref.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265989","messageId":"xmqqk2u8kmre.fsf@gitster.dls.corp.google.com","threadId":"39818","inReplyTo":"1436437671-25600-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-10T16:20:21Z","receivedAt":"2015-07-10T16:20:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> Add support for %(refname:shortalign=X) where X is a number.\n> This will print a shortened refname aligned to the left\n> followed by spaces for a total length of X characters.\n> If X is less than the shortened refname size, the entire\n> shortened refname is printed.\n>\n> Mentored-by: Christian Couder \n> Mentored-by: Matthieu Moy \n> Signed-off-by: Karthik Nayak \n> ---\n> ref-filter.c | 19 ++++++++++++++++++-\n> 1 file changed, 18 insertions(+), 1 deletion(-)\n\nThis may be enough to support the various existing formats that are\noffered by \"git branch\" and/or \"git tag\", but I do not think if this\nis the right approach in the longer term, or if we are painting\nourselves in a corner we cannot cleanly get out of later [*1*].\nWill the \"refname\" stay to be the only thing that may want alignment\npadding appended in the future? Will it stay true that we want to\nalign only to the left? Etc., etc.\n\nCc'ed Duy as %< in the pretty-format was his invention at around\na5752342 (pretty: support padding placeholders, %< %> and %><,\n2013-04-19).\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index dd0709d..3098497 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -10,6 +10,7 @@\n> #include \"quote.h\"\n> #include \"ref-filter.h\"\n> #include \"revision.h\"\n> +#include \"utf8.h\"\n> \n> typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n> \n> @@ -695,7 +696,23 @@ static void populate_value(struct ref_array_item *ref)\n> \t\t\tint num_ours, num_theirs;\n> \n> \t\t\tformatp++;\n> -\t\t\tif (!strcmp(formatp, \"short\"))\n> +\t\t\tif (starts_with(formatp, \"shortalign=\")) {\n\nWhen adding a new thing to an existing list, we prefer to append it\nat the end of the list, if there is no other reason not to do so\n(e.g. \"the existing list is sorted in this order, and the new\nlocation was chosen to fit the new item to honor the existing\nordering rule\" is a valid reason to put it at the beginning, if the\nexisting sorting rule dictates that the new thing must come at the\nbeginning).\n\n> +\t\t\t\tconst char *valp, *short_refname = NULL;\n> +\t\t\t\tint val, len;\n> +\n> +\t\t\t\tskip_prefix(formatp, \"shortalign=\", &valp);\n> +\t\t\t\tval = atoi(valp);\n\nIn newer code, we would want to avoid atoi() so that \"foo:shortalign=1z\"\nthat is a typo of \"12\" can be caught as an error. Either strtol_i()\nor strtoul_ui() may be better (we would need to adjust it further\nwhen Michael decides to resurrect his numparse thing that has been\nin the stalled bin for quite a while, though).\n\n> +\t\t\t\trefname = short_refname = shorten_unambiguous_ref(refname,\n> +\t\t\t\t\t\t\t\t\t\t warn_ambiguous_refs);\n> +\t\t\t\tlen = utf8_strwidth(refname);\n> +\t\t\t\tif (val > len) {\n> +\t\t\t\t\tstruct strbuf buf = STRBUF_INIT;\n> +\t\t\t\t\tstrbuf_addstr(&buf, refname);\n> +\t\t\t\t\tstrbuf_addchars(&buf, ' ', val - len);\n> +\t\t\t\t\tfree((char *)short_refname);\n> +\t\t\t\t\trefname = strbuf_detach(&buf, NULL);\n> +\t\t\t\t}\n\nWhat should happen when the display column width of the string is\nwider? If a user wants to align the refs that are usually usually\nshort start the next thing at the 8th column, which should she use?\n\n \"%(refname:shorta=7) %(next item)\"\n \"%(refname:shorta=8)%(next item)\"\n\n> +\t\t\t} else if (!strcmp(formatp, \"short\"))\n> \t\t\t\trefname = shorten_unambiguous_ref(refname,\n> \t\t\t\t\t\t warn_ambiguous_refs);\n> \t\t\telse if (!strcmp(formatp, \"track\") &&\n"},{"id":"265990","messageId":"xmqqfv4wklph.fsf@gitster.dls.corp.google.com","threadId":"39818","inReplyTo":"1436437671-25600-5-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v2 05/10] ref-filter: add option to match literal pattern","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-10T16:43:06Z","receivedAt":"2015-07-10T16:43:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> Since 'ref-filter' only has an option to match path names\n> add an option for regular pattern matching.\n\nThere is nothing \"regular\" about the pattern matching you are\nadding.\n\nEverywhere else we use patterns on refs we call wildmatch(), which\nis an enhanced implementation of fnmatch(3), and you are doing the\nsame in this new codepath.\n\nJust drop that word from here (and if you said something similar in\nthe documentation, drop \"regular\" ffrom there as well). It would\ninvite confusion with regular expression matching, which we will not\ndo for refs.\n"},{"id":"265991","messageId":"xmqqbnfkklmp.fsf@gitster.dls.corp.google.com","threadId":"39818","inReplyTo":"CAP8UFD0mSZ_FyPcaMoWRbrR=i9vFJvVYyqM+0B-3L3dQpA6SsA@mail.gmail.com","subject":"Re: [PATCH v2 06/10] Documentation/tag: remove double occurance of \"\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-10T16:44:46Z","receivedAt":"2015-07-10T16:44:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> On Thu, Jul 9, 2015 at 12:27 PM, Karthik Nayak wrote:\n>> Mentored-by: Christian Couder \n>> Mentored-by: Matthieu Moy \n>> Signed-off-by: Karthik Nayak \n>> ---\n>> Documentation/git-tag.txt | 1 -\n>> 1 file changed, 1 deletion(-)\n>>\n>> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n>> index 034d10d..4b04c2b 100644\n>> --- a/Documentation/git-tag.txt\n>> +++ b/Documentation/git-tag.txt\n>> @@ -14,7 +14,6 @@ SYNOPSIS\n>> 'git tag' -d ...\n>> 'git tag' [-n[]] -l [--contains ] [--points-at ]\n>> [--column[=] | --no-column] [...]\n>> - [...]\n>> 'git tag' -v ...\n>\n> As this patch could be applied directly to master and to maint maybe\n> you could send it at the top of this patch series or alone outside of\n> this patch series.\n\nThanks. I'll pick this patch from the remainder of the series and\nqueue it on a separate topic.\n"},{"id":"266009","messageId":"CAOLa=ZSuftULFSCKK=PoaPejA_FXGkmVrOybk1G6OEjDchfSEg@mail.gmail.com","threadId":"39818","inReplyTo":"vpqd2005hq4.fsf@anie.imag.fr","subject":"Re: [PATCH v2 04/10] ref-filter: add support to sort by version","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-11T05:54:53Z","receivedAt":"2015-07-11T05:54:53Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Fri, Jul 10, 2015 at 5:48 PM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> On Fri, Jul 10, 2015 at 4:22 PM, Karthik Nayak wrote:\n>>> On Thu, Jul 9, 2015 at 6:59 PM, Matthieu Moy\n>>> wrote:\n>>>> Karthik Nayak writes:\n>>>>\n>>>>> Add support to sort by version using the \"v:refname\" and\n>>>>> \"version:refname\" option. This is achieved by using the\n>>>>> 'version_cmp()' function as the comparing function for qsort.\n>>>>\n>>>> You should elaborate on why you need this. Given the context, I can\n>>>> guess that you will need this to implement tag, but for example I first\n>>>> wondered why you needed both version: and v:, but I guess it comes from\n>>>> the fact that 'git tag --sort' can take version:refname or v:refname.\n>>>>\n>>>> I think this deserves a test and documentation in for-each-ref.txt.\n>>>\n>>> I'll add it to \"for-each-ref.txt\" documentation.\n>>> About the tests, there are already tests for the same in git-tag.txt and\n>>> that's the only reason I did not repeat the tests in for-each-ref.\n>>>\n>>\n>> But since the porting is in a later commit, will add tests to for-each-ref.\n>\n> Yes: to me \"it's tested through 'git tag'\" is a good argument to do only\n> a superficial test, check that 'for-each-ref --sort v:refname' activate\n> the sorting, but no detailed corner-case testing. But not a good\n> argument to have no test at all on for-each-ref.\n>\n\nAgreed, I've written tests for the same.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266011","messageId":"CAOLa=ZT+59dcCW+QHhvK8Wp5Q_1YhkfCxV0=PirOWtVKWFCvCw@mail.gmail.com","threadId":"39818","inReplyTo":"xmqqfv4wklph.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 05/10] ref-filter: add option to match literal pattern","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-11T05:55:44Z","receivedAt":"2015-07-11T05:55:44Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Fri, Jul 10, 2015 at 10:13 PM, Junio C Hamano wrote:\n> Karthik Nayak writes:\n>\n>> Since 'ref-filter' only has an option to match path names\n>> add an option for regular pattern matching.\n>\n> There is nothing \"regular\" about the pattern matching you are\n> adding.\n>\n> Everywhere else we use patterns on refs we call wildmatch(), which\n> is an enhanced implementation of fnmatch(3), and you are doing the\n> same in this new codepath.\n>\n> Just drop that word from here (and if you said something similar in\n> the documentation, drop \"regular\" ffrom there as well). It would\n> invite confusion with regular expression matching, which we will not\n> do for refs.\n\nOk, will do. Thanks\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266007","messageId":"CAOLa=ZRJSA7hrRatP8GH49eZUrcSYz=UCnAJPaR+yJ2P-zrP4g@mail.gmail.com","threadId":"39818","inReplyTo":"vpq380x8p3a.fsf@anie.imag.fr","subject":"Re: [PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-11T06:07:01Z","receivedAt":"2015-07-11T06:07:01Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Thu, Jul 9, 2015 at 6:28 PM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> Add support for %(refname:shortalign=X) where X is a number.\n>> This will print a shortened refname aligned to the left\n>> followed by spaces for a total length of X characters.\n>> If X is less than the shortened refname size, the entire\n>> shortened refname is printed.\n>\n> Not really an issue, but you're wrapping your text at ~60 characters.\n> The common use is to wrap around 70 to 80. Using Emacs, auto-fill-mode\n> or M-q does this automatically. If you use another text editor, it can\n> probably do that for you too.\n>\n\nThanks, I was just manually clipping it.\n\n>> Mentored-by: Christian Couder \n>> Mentored-by: Matthieu Moy \n>> Signed-off-by: Karthik Nayak \n>> ---\n>> ref-filter.c | 19 ++++++++++++++++++-\n>\n> I think this would deserve a test and documentation. Even though your\n> motivation is for an internal implementation, some users may want to use\n> the feature in 'git for-each-ref --format=...'.\n>\n\nI didn't want to include documentation as this is mostly for internal use,\nbut will add with tests.\n\n>> 1 file changed, 18 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/ref-filter.c b/ref-filter.c\n>> index dd0709d..3098497 100644\n>> --- a/ref-filter.c\n>> +++ b/ref-filter.c\n>> @@ -10,6 +10,7 @@\n>> #include \"quote.h\"\n>> #include \"ref-filter.h\"\n>> #include \"revision.h\"\n>> +#include \"utf8.h\"\n>>\n>> typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n>>\n>> @@ -695,7 +696,23 @@ static void populate_value(struct ref_array_item *ref)\n>> int num_ours, num_theirs;\n>>\n>> formatp++;\n>> - if (!strcmp(formatp, \"short\"))\n>> + if (starts_with(formatp, \"shortalign=\")) {\n>> + const char *valp, *short_refname = NULL;\n>> + int val, len;\n>> +\n>> + skip_prefix(formatp, \"shortalign=\", &valp);\n>> + val = atoi(valp);\n>\n> You're silently accepting %(refname:shortalign=foo) and\n> %(refname:shortalign=). I think it would be better to reject such cases\n> explicitly.\n>\n\nOh yeah! will do.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266037","messageId":"vpqvbdryrhh.fsf@anie.imag.fr","threadId":"39818","inReplyTo":"CAOLa=ZT+59dcCW+QHhvK8Wp5Q_1YhkfCxV0=PirOWtVKWFCvCw@mail.gmail.com","subject":"Re: [PATCH v2 05/10] ref-filter: add option to match literal pattern","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-11T09:26:50Z","receivedAt":"2015-07-11T09:26:50Z","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> On Fri, Jul 10, 2015 at 10:13 PM, Junio C Hamano wrote:\n>> Karthik Nayak writes:\n>>\n>>> Since 'ref-filter' only has an option to match path names\n>>> add an option for regular pattern matching.\n>>\n>> There is nothing \"regular\" about the pattern matching you are\n>> adding.\n>>\n>> Everywhere else we use patterns on refs we call wildmatch(), which\n>> is an enhanced implementation of fnmatch(3), and you are doing the\n>> same in this new codepath.\n>>\n>> Just drop that word from here (and if you said something similar in\n>> the documentation, drop \"regular\" ffrom there as well). It would\n>> invite confusion with regular expression matching, which we will not\n>> do for refs.\n>\n> Ok, will do. Thanks\n\nJust dropping \"regular\" leads to a strange sentence, since the path name\nmatch is also a kind of pattern-matching. I'd write\n\nSince 'ref-filter' only has an option to match path names, add an option\nfor plain fnmatch pattern-matching.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"266038","messageId":"vpqwpy7vvuz.fsf@anie.imag.fr","threadId":"39818","inReplyTo":"CAOLa=ZRJSA7hrRatP8GH49eZUrcSYz=UCnAJPaR+yJ2P-zrP4g@mail.gmail.com","subject":"Re: [PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-11T10:20:36Z","receivedAt":"2015-07-11T10:20:36Z","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> On Thu, Jul 9, 2015 at 6:28 PM, Matthieu Moy\n> wrote:\n>\n>> I think this would deserve a test and documentation. Even though your\n>> motivation is for an internal implementation, some users may want to use\n>> the feature in 'git for-each-ref --format=...'.\n>>\n>\n> I didn't want to include documentation as this is mostly for internal use,\n> but will add with tests.\n\nYou need it for internal use, but it's still available to the users, and\nmay actually turn out to be useful to users.\n\nActually, you are rewritting tag and branch based on the internal C API,\nbut it should be possible to write similar commands based on the\ncommand-line interface and by looking at the documentation. It's part of\nthe philosophy of Git (\"toolkit design\") to have plumbing commands that\nallow writting high-level commands through scripting, or custom commands\nthrough aliases.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"266039","messageId":"CAOLa=ZTNGV=NAXDPB3HX=fanPMQaQQc4ZpFdDFeg+LJ8SFuVOQ@mail.gmail.com","threadId":"39818","inReplyTo":"xmqqk2u8kmre.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-11T12:05:29Z","receivedAt":"2015-07-11T12:05:29Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Fri, Jul 10, 2015 at 9:50 PM, Junio C Hamano wrote:\n>\n> This may be enough to support the various existing formats that are\n> offered by \"git branch\" and/or \"git tag\", but I do not think if this\n> is the right approach in the longer term, or if we are painting\n> ourselves in a corner we cannot cleanly get out of later [*1*].\n> Will the \"refname\" stay to be the only thing that may want alignment\n> padding appended in the future? Will it stay true that we want to\n> align only to the left? Etc., etc.\n>\n> Cc'ed Duy as %< in the pretty-format was his invention at around\n> a5752342 (pretty: support padding placeholders, %< %> and %><,\n> 2013-04-19).\n>\n\nI kinda had the same though, my only justification was that it was only being\ninternally used. I'll have another look if as to see if I can make it\nuniversal somehow.\nLet's see what Duy has to suggest.\n\n>\n> When adding a new thing to an existing list, we prefer to append it\n> at the end of the list, if there is no other reason not to do so\n> (e.g. \"the existing list is sorted in this order, and the new\n> location was chosen to fit the new item to honor the existing\n> ordering rule\" is a valid reason to put it at the beginning, if the\n> existing sorting rule dictates that the new thing must come at the\n> beginning).\n>\n\nmy bad, will change it!\n\n>\n> In newer code, we would want to avoid atoi() so that \"foo:shortalign=1z\"\n> that is a typo of \"12\" can be caught as an error. Either strtol_i()\n> or strtoul_ui() may be better (we would need to adjust it further\n> when Michael decides to resurrect his numparse thing that has been\n> in the stalled bin for quite a while, though).\n>\n\nWill have a look, thanks :)\n\n>\n> What should happen when the display column width of the string is\n> wider? If a user wants to align the refs that are usually usually\n> short start the next thing at the 8th column, which should she use?\n>\n> \"%(refname:shorta=7) %(next item)\"\n> \"%(refname:shorta=8)%(next item)\"\n>\n\nBoth your examples would start the next item at the 8th column\n(starting with 0),\nthe only difference being :\nCase 1: when the refname is 8 columns wide\n\"%(refname:shorta=7) %(next item)\": would give us eight columns of\nrefname + space + next item\n\"%(refname:shorta=8)%(next item)\": would give us eight columns of\nrefname + next item\nCase 2: when the refname < 8 columns wide\nBoth would give: upto 7 columns of refname + space + next item.\n\nThanks for the suggestions :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266040","messageId":"CAOLa=ZTvtQrkUqhq5u9h+RfQiqHJmQRAfzZ9KwT5X=-GMuWv+A@mail.gmail.com","threadId":"39818","inReplyTo":"vpqvbdryrhh.fsf@anie.imag.fr","subject":"Re: [PATCH v2 05/10] ref-filter: add option to match literal pattern","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-11T12:54:00Z","receivedAt":"2015-07-11T12:54:00Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sat, Jul 11, 2015 at 2:56 PM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> On Fri, Jul 10, 2015 at 10:13 PM, Junio C Hamano wrote:\n>>> Karthik Nayak writes:\n>>>\n>>>> Since 'ref-filter' only has an option to match path names\n>>>> add an option for regular pattern matching.\n>>>\n>>> There is nothing \"regular\" about the pattern matching you are\n>>> adding.\n>>>\n>>> Everywhere else we use patterns on refs we call wildmatch(), which\n>>> is an enhanced implementation of fnmatch(3), and you are doing the\n>>> same in this new codepath.\n>>>\n>>> Just drop that word from here (and if you said something similar in\n>>> the documentation, drop \"regular\" ffrom there as well). It would\n>>> invite confusion with regular expression matching, which we will not\n>>> do for refs.\n>>\n>> Ok, will do. Thanks\n>\n> Just dropping \"regular\" leads to a strange sentence, since the path name\n> match is also a kind of pattern-matching. I'd write\n>\n> Since 'ref-filter' only has an option to match path names, add an option\n> for plain fnmatch pattern-matching.\n>\n\nThanks for the heads up :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266047","messageId":"CACsJy8BsLnkPFX04WujMdnBiRGmXacF1QSQudb3T8nxGrZ_1Fg@mail.gmail.com","threadId":"39818","inReplyTo":"CAOLa=ZTNGV=NAXDPB3HX=fanPMQaQQc4ZpFdDFeg+LJ8SFuVOQ@mail.gmail.com","subject":"Re: [PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-07-12T01:47:39Z","receivedAt":"2015-07-12T01:47:39Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Jul 11, 2015 at 7:05 PM, Karthik Nayak wrote:\n> On Fri, Jul 10, 2015 at 9:50 PM, Junio C Hamano wrote:\n>>\n>> This may be enough to support the various existing formats that are\n>> offered by \"git branch\" and/or \"git tag\", but I do not think if this\n>> is the right approach in the longer term, or if we are painting\n>> ourselves in a corner we cannot cleanly get out of later [*1*].\n>> Will the \"refname\" stay to be the only thing that may want alignment\n>> padding appended in the future? Will it stay true that we want to\n>> align only to the left? Etc., etc.\n>>\n>> Cc'ed Duy as %< in the pretty-format was his invention at around\n>> a5752342 (pretty: support padding placeholders, %< %> and %><,\n>> 2013-04-19).\n>>\n>\n> I kinda had the same though, my only justification was that it was only being\n> internally used. I'll have another look if as to see if I can make it\n> universal somehow.\n> Let's see what Duy has to suggest.\n\nI guess if you can have multiple arguments after ':' in an atom, then\nyou have wiggle room for future. But it looks like you only accept one\nargument after ':'.. (I only checked the version on 'pu'). Having an\n\"alignment atom\" to augment the real one (like %< changes the behavior\nof the next placeholder), could also work, but it adds dependency\nbetween atoms, something I don't think ref-filter.c is ready for.\n\nAnother thing, the atom value is also used for sorting. When used for\nsorting, I think these padding spaces should not be generated or it\nmay confuse the sort algorithm. Left alignment may be ok, right or\ncenter alignment (in future?), not so much. Perhaps we should do the\npadding in a separate phase, outside populate_value(). If you go this\nroute, having separate atoms for alignment works better: you don't\nhave to parse them in populate_value() which is for actual values, and\nyou can handle dependency easily (I think).\n\nBy the way, please consider adding _() back to translatable strings,\nusually those die() or warn(), or \"[ahead %s]\"... In the last case,\nbecause you don't really know how long the string is after\ntranslation, avoid hard coding buffer size (to 40).\n-- \nDuy\n"},{"id":"266054","messageId":"CACsJy8BL0ccxNYLE0gqBx5KTpqTOt=Ut6-k3fauxXNt2wpxSEw@mail.gmail.com","threadId":"39818","inReplyTo":"CACsJy8BsLnkPFX04WujMdnBiRGmXacF1QSQudb3T8nxGrZ_1Fg@mail.gmail.com","subject":"Re: [PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-07-12T08:59:45Z","receivedAt":"2015-07-12T08:59:45Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Jul 12, 2015 at 8:47 AM, Duy Nguyen wrote:\n> Another thing, the atom value is also used for sorting. When used for\n> sorting, I think these padding spaces should not be generated or it\n> may confuse the sort algorithm. Left alignment may be ok, right or\n> center alignment (in future?), not so much.\\\n\nCorrection: I'm wrong. This is only true if the atom for sorting is in\nthe same atom list for display. But we create new atoms for sorting in\nparse_opt_ref_sorting(), where we drop everything after ':', so\n\"shortalign=\" shouldn't be there when populate_atom() for sorting is\ncalled.\n-- \nDuy\n"},{"id":"266057","messageId":"CACsJy8AEEDEUE=nBvWVv426MWYzEmRSFFR1xF-cKKD2Lid9n4w@mail.gmail.com","threadId":"39818","inReplyTo":"559E53DC.2040804@gmail.com","subject":"Re: [PATCH v2 08/10] tag.c: use 'ref-filter' APIs","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-07-12T09:45:58Z","receivedAt":"2015-07-12T09:45:58Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jul 9, 2015 at 5:58 PM, Karthik Nayak wrote:\n> -static int show_reference(const char *refname, const struct object_id *oid,\n> - int flag, void *cb_data)\n> -{\n...\n> -\n> - if (match_pattern(filter->name_patterns, refname)) {\n....\n> - printf(\"%-15s \", refname);\n> - show_tag_lines(oid, filter->lines);\n> - putchar('\\n');\n> - }\n> -\n> - return 0;\n> -}\n...\n> + if (filter->lines)\n> + format = \"%(refname:shortalign=16)\";\n> + else\n> + format = \"%(refname:short)\";\n\nI can see this is a faithful conversion, but this looks line an\nopportunity to avoid this special limit 15/16. Even on git.git \"git\ntag -l -n1\" already breaks alignment with *.msysgit.* tags (ok maybe\nmsysgit, not purely git.git) When you get to \"branch -l\", it\ncalculates the max-width automatically so you probably need\n\"%(refname:shortalign)\" any way. \"shortalign\" (i.e. create the \"align\"\nversion for every modifier) does not look good because it could double\nthe number of modifiers and let's not thinking about truncation\noptions or right alignment..\n-- \nDuy\n"},{"id":"266060","messageId":"CAOLa=ZRnmccZq9ZKmMOKNQ+HU8hWoHwvRQFL0MY_Di=B_jz4Vw@mail.gmail.com","threadId":"39818","inReplyTo":"xmqqbnfkklmp.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 06/10] Documentation/tag: remove double occurance of \"\"","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-12T12:39:59Z","receivedAt":"2015-07-12T12:39:59Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Fri, Jul 10, 2015 at 10:14 PM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> On Thu, Jul 9, 2015 at 12:27 PM, Karthik Nayak wrote:\n>>> Mentored-by: Christian Couder \n>>> Mentored-by: Matthieu Moy \n>>> Signed-off-by: Karthik Nayak \n>>> ---\n>>> Documentation/git-tag.txt | 1 -\n>>> 1 file changed, 1 deletion(-)\n>>>\n>>> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n>>> index 034d10d..4b04c2b 100644\n>>> --- a/Documentation/git-tag.txt\n>>> +++ b/Documentation/git-tag.txt\n>>> @@ -14,7 +14,6 @@ SYNOPSIS\n>>> 'git tag' -d ...\n>>> 'git tag' [-n[]] -l [--contains ] [--points-at ]\n>>> [--column[=] | --no-column] [...]\n>>> - [...]\n>>> 'git tag' -v ...\n>>\n>> As this patch could be applied directly to master and to maint maybe\n>> you could send it at the top of this patch series or alone outside of\n>> this patch series.\n>\n> Thanks. I'll pick this patch from the remainder of the series and\n> queue it on a separate topic.\n\nThat'll be awesome :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266069","messageId":"CAOLa=ZSR+QPnE36F=kX5w7q_ANobso+MM2q9-SKBARYghoJvrA@mail.gmail.com","threadId":"39818","inReplyTo":"CACsJy8AEEDEUE=nBvWVv426MWYzEmRSFFR1xF-cKKD2Lid9n4w@mail.gmail.com","subject":"Re: [PATCH v2 08/10] tag.c: use 'ref-filter' APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-12T19:36:10Z","receivedAt":"2015-07-12T19:36:10Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sun, Jul 12, 2015 at 3:15 PM, Duy Nguyen wrote:\n> On Thu, Jul 9, 2015 at 5:58 PM, Karthik Nayak wrote:\n>> -static int show_reference(const char *refname, const struct object_id *oid,\n>> - int flag, void *cb_data)\n>> -{\n> ...\n>> -\n>> - if (match_pattern(filter->name_patterns, refname)) {\n> ....\n>> - printf(\"%-15s \", refname);\n>> - show_tag_lines(oid, filter->lines);\n>> - putchar('\\n');\n>> - }\n>> -\n>> - return 0;\n>> -}\n> ...\n>> + if (filter->lines)\n>> + format = \"%(refname:shortalign=16)\";\n>> + else\n>> + format = \"%(refname:short)\";\n>\n> I can see this is a faithful conversion, but this looks line an\n> opportunity to avoid this special limit 15/16. Even on git.git \"git\n> tag -l -n1\" already breaks alignment with *.msysgit.* tags (ok maybe\n> msysgit, not purely git.git) When you get to \"branch -l\", it\n> calculates the max-width automatically so you probably need\n> \"%(refname:shortalign)\" any way. \"shortalign\" (i.e. create the \"align\"\n> version for every modifier) does not look good because it could double\n> the number of modifiers and let's not thinking about truncation\n> options or right alignment..\n\nWhat I was thinking of was getting rid of the whole \"align\" feature where\nyou provide a value to which it would align.\n\nSomething like: --format=\"%(item:modifieralign)\" which would use something\non the lines of what the max-width calculator in branch -l uses, to get the max\nalignment size. But the problem is that ref-filter goes through the refs using\na function which has no connections with the atoms used. So a more practical\nsolution would be --format=\"%(item:modifieralign=X)\" where we could provide a\nmeans of calculating X via ref-filter. Something like this in tag.c:\n\nint max_width = get_max_width(\"\");\nuse this max_width to then do a\n--format=\"%(item:modifieralign=X)\", where X = max_width\n\nWhat do you think?\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266070","messageId":"CAOLa=ZThDcmM79hL0TUh6+8=NkQBWOWK=Ti+-v+2PCysquw-1w@mail.gmail.com","threadId":"39818","inReplyTo":"CACsJy8BL0ccxNYLE0gqBx5KTpqTOt=Ut6-k3fauxXNt2wpxSEw@mail.gmail.com","subject":"Re: [PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-12T19:56:20Z","receivedAt":"2015-07-12T19:56:20Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sun, Jul 12, 2015 at 7:17 AM, Duy Nguyen wrote:\n>\n> I guess if you can have multiple arguments after ':' in an atom, then\n> you have wiggle room for future. But it looks like you only accept one\n> argument after ':'.. (I only checked the version on 'pu'). Having an\n> \"alignment atom\" to augment the real one (like %< changes the behavior\n> of the next placeholder), could also work, but it adds dependency\n> between atoms, something I don't think ref-filter.c is ready for.\n>\n\nI was thinking of something on the lines of having a function which right\nbefore printing checks if any \"align\" option is given to the end of a given\nitem and aligns it accordingly, this ensures that any item which needs to\nhave such an option can easily do so.\n\nhttps://github.com/KarthikNayak/git/commit/0284320483d6442a6425fc665e740f9f975654a1\n\nThis is what I came up with, you could have a look and let me know if\nyou have any\nsuggestions.\n\n> Another thing, the atom value is also used for sorting. When used for\n> sorting, I think these padding spaces should not be generated or it\n> may confuse the sort algorithm. Left alignment may be ok, right or\n> center alignment (in future?), not so much. Perhaps we should do the\n> padding in a separate phase, outside populate_value(). If you go this\n> route, having separate atoms for alignment works better: you don't\n> have to parse them in populate_value() which is for actual values, and\n> you can handle dependency easily (I think).\n\nThis was cleared out in your other reply.\n\n>\n> By the way, please consider adding _() back to translatable strings,\n> usually those die() or warn(), or \"[ahead %s]\"... In the last case,\n> because you don't really know how long the string is after\n> translation, avoid hard coding buffer size (to 40).\n\nYes, sure, maybe a cleanup patch at the end to do all of cleanup any\nsuch use cases :)\n\nThanks for all the suggestions.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266077","messageId":"CACsJy8DRYGSoiyPRi3hCD54NA1TF2mr5+9xwReX333ppdoAg+A@mail.gmail.com","threadId":"39818","inReplyTo":"CAOLa=ZSR+QPnE36F=kX5w7q_ANobso+MM2q9-SKBARYghoJvrA@mail.gmail.com","subject":"Re: [PATCH v2 08/10] tag.c: use 'ref-filter' APIs","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-07-13T10:46:11Z","receivedAt":"2015-07-13T10:46:11Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Jul 13, 2015 at 2:36 AM, Karthik Nayak wrote:\n> What I was thinking of was getting rid of the whole \"align\" feature where\n> you provide a value to which it would align.\n>\n> Something like: --format=\"%(item:modifieralign)\" which would use something\n> on the lines of what the max-width calculator in branch -l uses, to get the max\n> alignment size. But the problem is that ref-filter goes through the refs using\n> a function which has no connections with the atoms used. So a more practical\n> solution would be --format=\"%(item:modifieralign=X)\" where we could provide a\n> means of calculating X via ref-filter. Something like this in tag.c:\n>\n> int max_width = get_max_width(\"\");\n> use this max_width to then do a\n> --format=\"%(item:modifieralign=X)\", where X = max_width\n>\n> What do you think?\n\nThis is where separate \"alignment atoms\" (instead of alignment\nmodifiers) make sense. Suppose you introduce another function, let's\nsay print_all() for now, to wrap the \"for (i < maxcount)\" loop at the\nend of for-each-ref, you would have total control over display and\nformatting. populate_value() generates empty strings for these\nalignment atoms (because they don't really have true values). Those\nalignment atoms are recognized in print_all() and\nshow_ref_array_item(). In print_all(), if it sees max width needs to\nbe calculated (because the user does not specify the width), it can\ncall populate_value() for an atom for all rows. show_ref_array_item()\ndoes the padding and even truncating if needed. This pattern is\nsimilar to how print_columns() works, first we collect data of the\nwhole \"table\", then we place them line by line.\n\nIt sounds good to me. But it may not be the best option, I don't know.\nAnd it may create unnecessary work. So you and your mentors decide.\n-- \nDuy\n"},{"id":"266078","messageId":"CACsJy8D6XOJbVb9LiDmmNuH9UWmyj9BRc0V5GYq-0wwi2uvN0w@mail.gmail.com","threadId":"39818","inReplyTo":"CAOLa=ZThDcmM79hL0TUh6+8=NkQBWOWK=Ti+-v+2PCysquw-1w@mail.gmail.com","subject":"Re: [PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-07-13T10:51:35Z","receivedAt":"2015-07-13T10:51:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Jul 13, 2015 at 2:56 AM, Karthik Nayak wrote:\n> On Sun, Jul 12, 2015 at 7:17 AM, Duy Nguyen wrote:\n>>\n>> I guess if you can have multiple arguments after ':' in an atom, then\n>> you have wiggle room for future. But it looks like you only accept one\n>> argument after ':'.. (I only checked the version on 'pu'). Having an\n>> \"alignment atom\" to augment the real one (like %< changes the behavior\n>> of the next placeholder), could also work, but it adds dependency\n>> between atoms, something I don't think ref-filter.c is ready for.\n>>\n>\n> I was thinking of something on the lines of having a function which right\n> before printing checks if any \"align\" option is given to the end of a given\n> item and aligns it accordingly, this ensures that any item which needs to\n> have such an option can easily do so.\n>\n> https://github.com/KarthikNayak/git/commit/0284320483d6442a6425fc665e740f9f975654a1\n>\n> This is what I came up with, you could have a look and let me know if\n> you have any\n> suggestions.\n\nYeah, pretty close to what I described in the other mail. Now if you\nmake \"align\" a separate atom, I think it would reduce clutter in\npopulate_value() (my personal opinion is this function looks too messy\nalready) and we can easily add more alignment options in future :)\n-- \nDuy\n"},{"id":"266091","messageId":"CAOLa=ZQjYi=PAddDnFNd-Q3+C43O9ELDqXgynF0kya9SJbPagg@mail.gmail.com","threadId":"39818","inReplyTo":"CACsJy8DRYGSoiyPRi3hCD54NA1TF2mr5+9xwReX333ppdoAg+A@mail.gmail.com","subject":"Re: [PATCH v2 08/10] tag.c: use 'ref-filter' APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-13T20:34:46Z","receivedAt":"2015-07-13T20:34:46Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Jul 13, 2015 at 4:16 PM, Duy Nguyen wrote:\n> On Mon, Jul 13, 2015 at 2:36 AM, Karthik Nayak wrote:\n>> What I was thinking of was getting rid of the whole \"align\" feature where\n>> you provide a value to which it would align.\n>>\n>> Something like: --format=\"%(item:modifieralign)\" which would use something\n>> on the lines of what the max-width calculator in branch -l uses, to get the max\n>> alignment size. But the problem is that ref-filter goes through the refs using\n>> a function which has no connections with the atoms used. So a more practical\n>> solution would be --format=\"%(item:modifieralign=X)\" where we could provide a\n>> means of calculating X via ref-filter. Something like this in tag.c:\n>>\n>> int max_width = get_max_width(\"\");\n>> use this max_width to then do a\n>> --format=\"%(item:modifieralign=X)\", where X = max_width\n>>\n>> What do you think?\n>\n> This is where separate \"alignment atoms\" (instead of alignment\n> modifiers) make sense. Suppose you introduce another function, let's\n> say print_all() for now, to wrap the \"for (i < maxcount)\" loop at the\n> end of for-each-ref, you would have total control over display and\n> formatting. populate_value() generates empty strings for these\n> alignment atoms (because they don't really have true values). Those\n> alignment atoms are recognized in print_all() and\n> show_ref_array_item(). In print_all(), if it sees max width needs to\n> be calculated (because the user does not specify the width), it can\n> call populate_value() for an atom for all rows. show_ref_array_item()\n> does the padding and even truncating if needed. This pattern is\n> similar to how print_columns() works, first we collect data of the\n> whole \"table\", then we place them line by line.\n>\n> It sounds good to me. But it may not be the best option, I don't know.\n> And it may create unnecessary work. So you and your mentors decide.\n> --\n> Duy\n\nSounds good, but what you're saying goes on the lines of interdependence of\natoms, since we would have separate atoms. Not sure we want to do that right\nnow.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"266092","messageId":"CAOLa=ZRuCa=7QzS7K86eikApiGgYsGLeaP0mw5Ax=k5en72jxA@mail.gmail.com","threadId":"39818","inReplyTo":"CACsJy8D6XOJbVb9LiDmmNuH9UWmyj9BRc0V5GYq-0wwi2uvN0w@mail.gmail.com","subject":"Re: [PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-07-13T20:36:15Z","receivedAt":"2015-07-13T20:36:15Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Jul 13, 2015 at 4:21 PM, Duy Nguyen wrote:\n> On Mon, Jul 13, 2015 at 2:56 AM, Karthik Nayak wrote:\n>> On Sun, Jul 12, 2015 at 7:17 AM, Duy Nguyen wrote:\n>>>\n>>> I guess if you can have multiple arguments after ':' in an atom, then\n>>> you have wiggle room for future. But it looks like you only accept one\n>>> argument after ':'.. (I only checked the version on 'pu'). Having an\n>>> \"alignment atom\" to augment the real one (like %< changes the behavior\n>>> of the next placeholder), could also work, but it adds dependency\n>>> between atoms, something I don't think ref-filter.c is ready for.\n>>>\n>>\n>> I was thinking of something on the lines of having a function which right\n>> before printing checks if any \"align\" option is given to the end of a given\n>> item and aligns it accordingly, this ensures that any item which needs to\n>> have such an option can easily do so.\n>>\n>> https://github.com/KarthikNayak/git/commit/0284320483d6442a6425fc665e740f9f975654a1\n>>\n>> This is what I came up with, you could have a look and let me know if\n>> you have any\n>> suggestions.\n>\n> Yeah, pretty close to what I described in the other mail. Now if you\n> make \"align\" a separate atom, I think it would reduce clutter in\n> populate_value() (my personal opinion is this function looks too messy\n> already) and we can easily add more alignment options in future :)\n> --\n> Duy\n\nYeah, that seems like the way to go, eventually :)\n\n-- \nRegards,\nKarthik Nayak\n"}]}