{"thread":{"id":"40286","subject":"[PATCH v16 00/14] port tag.c to use ref-filter APIs","startedAt":"2015-09-05T18:52:01Z","lastAt":"2015-09-09T19:18:50Z","messageCount":30,"participants":["Karthik Nayak","Matthieu Moy","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":16,"patchTotal":14},"messages":[{"id":"269503","messageId":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","threadId":"40286","inReplyTo":null,"subject":"[PATCH v16 00/14] port tag.c to use ref-filter APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-05T18:52:01Z","receivedAt":"2015-09-05T18:52:01Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Version 15 can be found here:\nhttp://thread.gmane.org/gmane.comp.version-control.git/277012\n\nChanges in this version:\n* Grammatical changes.\n* rename cb_data to at_end_data.\n* rename align_handler() to end_align_handler() and move it\nnext to align_atom_handler().\n* introduce match_atom_name() for checking atom names.\n\nKarthik Nayak (14):\n ref-filter: move `struct atom_value` to ref-filter.c\n ref-filter: introduce ref_formatting_state and ref_formatting_stack\n utf8: add function to align a string into given strbuf\n ref-filter: introduce handler function for each atom\n ref-filter: introduce match_atom_name()\n ref-filter: implement an `align` atom\n ref-filter: add option to filter out tags, branches and remotes\n ref-filter: add support for %(contents:lines=X)\n ref-filter: add support to sort by version\n ref-filter: add option to match literal pattern\n tag.c: use 'ref-filter' data structures\n tag.c: use 'ref-filter' APIs\n tag.c: implement '--format' option\n tag.c: implement '--merged' and '--no-merged' options\n\n Documentation/git-for-each-ref.txt | 16 +-\n Documentation/git-tag.txt | 27 ++-\n builtin/for-each-ref.c | 1 +\n builtin/tag.c | 368 ++++++--------------------------\n ref-filter.c | 415 ++++++++++++++++++++++++++++++++-----\n ref-filter.h | 25 ++-\n refs.c | 9 +\n refs.h | 1 +\n t/t6302-for-each-ref-filter.sh | 174 ++++++++++++++++\n t/t7004-tag.sh | 47 ++++-\n utf8.c | 21 ++\n utf8.h | 15 ++\n 12 files changed, 743 insertions(+), 376 deletions(-)\n\nInterdiff:\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex d039f40..c5154bb 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -128,13 +128,14 @@ color::\n \tare described in `color.branch.*`.\n \n align::\n-\tLeft-, middle-, or right-align the content between %(align:..)\n+\tLeft-, middle-, or right-align the content between %(align:...)\n \tand %(end). Followed by `:,`, where the\n \t`` is either left, right or middle and `` is\n \tthe total length of the content with alignment. If the\n \tcontents length is more than the width then no alignment is\n \tperformed. If used with '--quote' everything in between\n-\t%(align:..) and %(end) is quoted.\n+\t%(align:...) and %(end) is quoted, but if nested then only the\n+\ttopmost level performs quoting.\n \n In addition to the above, for commit and tag objects, the header\n field names (`tree`, `parent`, `object`, `type`, and `tag`) can\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 9fa1400..f55dfda 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -43,8 +43,8 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con\n \n \tif (!format) {\n \t\tif (filter->lines)\n-\t\t\tformat = to_free = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)\",\n-\t\t\t\t\t\t filter->lines);\n+\t\t\tformat = to_free = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) \"\n+\t\t\t\t\t\t \"%%(contents:lines=%d)\", filter->lines);\n \t\telse\n \t\t\tformat = \"%(refname:short)\";\n \t}\ndiff --git a/ref-filter.c b/ref-filter.c\nindex b37d57a..e3024d3 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -77,7 +77,7 @@ struct ref_formatting_stack {\n \tstruct ref_formatting_stack *prev;\n \tstruct strbuf output;\n \tvoid (*at_end)(struct ref_formatting_stack *stack);\n-\tvoid *cb_data;\n+\tvoid *at_end_data;\n };\n \n struct ref_formatting_state {\n@@ -185,16 +185,6 @@ static void quote_formatting(struct strbuf *s, const char *str, int quote_style)\n \t}\n }\n \n-static void align_handler(struct ref_formatting_stack *stack)\n-{\n-\tstruct align *align = (struct align *)stack->cb_data;\n-\tstruct strbuf s = STRBUF_INIT;\n-\n-\tstrbuf_utf8_align(&s, align->position, align->width, stack->output.buf);\n-\tstrbuf_swap(&stack->output, &s);\n-\tstrbuf_release(&s);\n-}\n-\n static void append_atom(struct atom_value *v, struct ref_formatting_state *state)\n {\n \t/*\n@@ -230,14 +220,24 @@ static void pop_stack_element(struct ref_formatting_stack **stack)\n \t*stack = prev;\n }\n \n+static void end_align_handler(struct ref_formatting_stack *stack)\n+{\n+\tstruct align *align = (struct align *)stack->at_end_data;\n+\tstruct strbuf s = STRBUF_INIT;\n+\n+\tstrbuf_utf8_align(&s, align->position, align->width, stack->output.buf);\n+\tstrbuf_swap(&stack->output, &s);\n+\tstrbuf_release(&s);\n+}\n+\n static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)\n {\n \tstruct ref_formatting_stack *new;\n \n \tpush_stack_element(&state->stack);\n \tnew = state->stack;\n-\tnew->at_end = align_handler;\n-\tnew->cb_data = &atomv->u.align;\n+\tnew->at_end = end_align_handler;\n+\tnew->at_end_data = &atomv->u.align;\n }\n \n static void end_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)\n@@ -246,12 +246,13 @@ static void end_atom_handler(struct atom_value *atomv, struct ref_formatting_sta\n \tstruct strbuf s = STRBUF_INIT;\n \n \tif (!current->at_end)\n-\t\tdie(_(\"format: `end` atom used without a supporting atom\"));\n+\t\tdie(_(\"format: %%(end) atom used without corresponding atom\"));\n \tcurrent->at_end(current);\n \n \t/*\n \t * Perform quote formatting when the stack element is that of\n-\t * a modifier atom and right above the first stack element.\n+\t * a supporting atom. If nested then perform quote formatting\n+\t * only on the topmost supporting atom.\n \t */\n \tif (!state->stack->prev->prev) {\n \t\tquote_formatting(&s, current->output.buf, state->quote_style);\n@@ -261,6 +262,22 @@ static void end_atom_handler(struct atom_value *atomv, struct ref_formatting_sta\n \tpop_stack_element(&state->stack);\n }\n \n+static int match_atom_name(const char *name, const char *atom_name, const char **val)\n+{\n+\tconst char *body;\n+\n+\tif (!skip_prefix(name, atom_name, &body))\n+\t\treturn 0; /* doesn't even begin with \"atom_name\" */\n+\tif (!body[0] || !body[1]) {\n+\t\t*val = NULL; /* %(atom_name) and no customization */\n+\t\treturn 1;\n+\t}\n+\tif (body[0] != ':')\n+\t\treturn 0; /* \"atom_namefoo\" is not \"atom_name\" or \"atom_name:...\" */\n+\t*val = body + 1; /* \"atomname:val\" */\n+\treturn 1;\n+}\n+\n /*\n * In a format string, find the next occurrence of %(atom).\n */\n@@ -645,9 +662,6 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size\n \tconst char *sp, *eol;\n \tsize_t len;\n \n-\tif ((sp = strstr(buf, \"\\n\\n\")) && (sp <= buf + size))\n-\t\tsize += 2;\n-\n \tsp = buf;\n \n \tfor (i = 0; i < lines && sp < buf + size; i++) {\n@@ -705,9 +719,12 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n \t\t\tv->s = xstrdup(subpos);\n \t\telse if (skip_prefix(name, \"contents:lines=\", &valp)) {\n \t\t\tstruct strbuf s = STRBUF_INIT;\n+\t\t\tconst char *contents_end = bodylen + bodypos - siglen;\n+\n \t\t\tif (strtoul_ui(valp, 10, &v->u.contents.lines))\n-\t\t\t\tdie(_(\"positive width expected contents:lines=%s\"), valp);\n-\t\t\tappend_lines(&s, subpos, sublen + bodylen - siglen, v->u.contents.lines);\n+\t\t\t\tdie(_(\"positive value expected contents:lines=%s\"), valp);\n+\t\t\t/* Size is the length of the message after removing the signature */\n+\t\t\tappend_lines(&s, subpos, contents_end - subpos, v->u.contents.lines);\n \t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t}\n \t}\n@@ -830,10 +847,12 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\trefname = branch_get_push(branch, NULL);\n \t\t\tif (!refname)\n \t\t\t\tcontinue;\n-\t\t} else if (starts_with(name, \"color:\")) {\n+\t\t} else if (match_atom_name(name, \"color\", &valp)) {\n \t\t\tchar color[COLOR_MAXLEN] = \"\";\n \n-\t\t\tif (color_parse(name + 6, color) < 0)\n+\t\t\tif (!valp)\n+\t\t\t\tdie(_(\"expected format: %%(color:)\"));\n+\t\t\tif (color_parse(valp, color) < 0)\n \t\t\t\tdie(_(\"unable to parse format\"));\n \t\t\tv->s = xstrdup(color);\n \t\t\tcontinue;\n@@ -863,24 +882,32 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\telse\n \t\t\t\tv->s = \" \";\n \t\t\tcontinue;\n-\t\t} else if (skip_prefix(name, \"align\", &valp)) {\n+\t\t} else if (match_atom_name(name, \"align\", &valp)) {\n \t\t\tstruct align *align = &v->u.align;\n \t\t\tstruct strbuf **s;\n \n-\t\t\tif (valp[0] != ':')\n-\t\t\t\tdie(_(\"format: usage %%(align:,)\"));\n-\t\t\telse\n-\t\t\t\tvalp++;\n+\t\t\tif (!valp)\n+\t\t\t\tdie(_(\"expected format: %%(align:, )\"));\n \n+\t\t\t/*\n+\t\t\t * TODO: Implement a function similar to strbuf_split_str()\n+\t\t\t * which would strip the terminator at the end.\n+\t\t\t */\n \t\t\ts = strbuf_split_str(valp, ',', 0);\n \n \t\t\t/* If the position is given trim the ',' from the first strbuf */\n \t\t\tif (s[1])\n \t\t\t\tstrbuf_setlen(s[0], s[0]->len - 1);\n+\t\t\tif (s[2])\n+\t\t\t\tdie(_(\"align:, followed by garbage: %s\"), s[2]->buf);\n \n \t\t\tif (strtoul_ui(s[0]->buf, 10, &align->width))\n \t\t\t\tdie(_(\"positive width expected align:%s\"), s[0]->buf);\n \n+\t\t\t/*\n+\t\t\t * TODO: Implement a more general check, so that the values\n+\t\t\t * do not always have to be in a specific order.\n+\t\t\t */\n \t\t\tif (!s[1])\n \t\t\t\talign->position = ALIGN_LEFT;\n \t\t\telse if (!strcmp(s[1]->buf, \"left\"))\n@@ -1294,11 +1321,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,\n \t\treturn 0;\n \t}\n \n-\t/*\n-\t * Get the current ref kind. If we're filtering tags, remotes or local branches\n-\t * only then the current ref-kind is nothing but filter->kind and filter_ref_kind()\n-\t * will only return that value.\n-\t */\n+\t/* Obtain the current ref kind from filter_ref_kind() and ignore unwanted refs. */\n \tkind = filter_ref_kind(filter, refname);\n \tif (!(kind & filter->kind))\n \t\treturn 0;\n@@ -1424,6 +1447,12 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n \tif (!filter->kind)\n \t\tdie(\"filter_refs: invalid type\");\n \telse {\n+\t\t/*\n+\t\t * For common cases where we need only branches or remotes or tags,\n+\t\t * we only iterate through those refs. If a mix of refs is needed,\n+\t\t * we iterate over all refs and filter out required refs with the help\n+\t\t * of filter_ref_kind().\n+\t\t */\n \t\tif (filter->kind == FILTER_REFS_BRANCHES)\n \t\t\tret = for_each_fullref_in(\"refs/heads/\", ref_filter_handler, &ref_cbdata, broken);\n \t\telse if (filter->kind == FILTER_REFS_REMOTES)\n@@ -1561,7 +1590,7 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n \t\tappend_atom(&resetv, &state);\n \t}\n \tif (state.stack->prev)\n-\t\tdie(_(\"format: `end` atom missing\"));\n+\t\tdie(_(\"format: %%(end) atom missing\"));\n \tfinal_buf = &state.stack->output;\n \tfwrite(final_buf->buf, 1, final_buf->len, stdout);\n \tpop_stack_element(&state.stack);\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex d95b781..4bc1055 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -81,6 +81,10 @@ test_expect_success 'filtering with --contains' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '%(color) must fail' '\n+\ttest_must_fail git for-each-ref --format=\"%(color)%(refname)\"\n+'\n+\n test_expect_success 'left alignment' '\n \tcat >expect <<-\\EOF &&\n \trefname is refs/heads/master |refs/heads/master\n@@ -129,42 +133,39 @@ test_expect_success 'right alignment' '\n \ttest_cmp expect actual\n '\n \n-# Everything in between the %(align)...%(end) atom must be quoted, hence we test this by\n-# introducing single quote's in %(align)...%(end), which must not be escaped.\n-\n-sq=\"'\"\n+# Individual atoms inside %(align:...) and %(end) must not be quoted.\n \n-test_expect_success 'alignment with format quote' '\n-\tcat >expect <<-EOF &&\n-\trefname is ${sq} ${sq}\\\\${sq}${sq}master${sq}\\\\${sq}${sq} ${sq}|\n-\trefname is ${sq} ${sq}\\\\${sq}${sq}side${sq}\\\\${sq}${sq} ${sq}|\n-\trefname is ${sq} ${sq}\\\\${sq}${sq}odd/spot${sq}\\\\${sq}${sq} ${sq}|\n-\trefname is ${sq} ${sq}\\\\${sq}${sq}double-tag${sq}\\\\${sq}${sq} ${sq}|\n-\trefname is ${sq} ${sq}\\\\${sq}${sq}four${sq}\\\\${sq}${sq} ${sq}|\n-\trefname is ${sq} ${sq}\\\\${sq}${sq}one${sq}\\\\${sq}${sq} ${sq}|\n-\trefname is ${sq} ${sq}\\\\${sq}${sq}signed-tag${sq}\\\\${sq}${sq} ${sq}|\n-\trefname is ${sq} ${sq}\\\\${sq}${sq}three${sq}\\\\${sq}${sq} ${sq}|\n-\trefname is ${sq} ${sq}\\\\${sq}${sq}two${sq}\\\\${sq}${sq} ${sq}|\n+test_expect_success 'alignment with format quote' \"\n+\tcat >expect <<-\\EOF &&\n+\t|' master| A U Thor '|\n+\t|' side| A U Thor '|\n+\t|' odd/spot| A U Thor '|\n+\t|' double-tag| '|\n+\t|' four| A U Thor '|\n+\t|' one| A U Thor '|\n+\t|' signed-tag| '|\n+\t|' three| A U Thor '|\n+\t|' two| A U Thor '|\n \tEOF\n-\tgit for-each-ref --shell --format=\"refname is %(align:30,middle)${sq}%(refname:short)${sq}%(end)|\" >actual &&\n+\tgit for-each-ref --shell --format='|%(align:30,middle)%(refname:short)| %(authorname)%(end)|' >actual &&\n \ttest_cmp expect actual\n-'\n+\"\n \n-test_expect_success 'nested alignment' '\n+test_expect_success 'nested alignment with quote formatting' \"\n \tcat >expect <<-\\EOF &&\n-\t| master |\n-\t| side |\n-\t| odd/spot |\n-\t| double-tag |\n-\t| four |\n-\t| one |\n-\t| signed-tag |\n-\t| three |\n-\t| two |\n+\t|' master '|\n+\t|' side '|\n+\t|' odd/spot '|\n+\t|' double-tag '|\n+\t|' four '|\n+\t|' one '|\n+\t|' signed-tag '|\n+\t|' three '|\n+\t|' two '|\n \tEOF\n-\tgit for-each-ref --format=\"|%(align:30,left)%(align:15,right)%(refname:short)%(end)%(end)|\" >actual &&\n+\tgit for-each-ref --shell --format='|%(align:30,left)%(align:15,right)%(refname:short)%(end)%(end)|' >actual &&\n \ttest_cmp expect actual\n-'\n+\"\n \n test_expect_success 'check `%(contents:lines=1)`' '\n \tcat >expect <<-\\EOF &&\n\n-- \n2.5.1\n"},{"id":"269504","messageId":"1441479135-5285-2-git-send-email-Karthik.188@gmail.com","threadId":"40286","inReplyTo":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v16 01/14] ref-filter: move `struct atom_value` to ref-filter.c","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-05T18:52:02Z","receivedAt":"2015-09-05T18:52:02Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Since atom_value is only required for the internal working of\nref-filter it doesn't belong in the public header.\n\nHelped-by: Eric Sunshine \nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n ref-filter.c | 5 +++++\n ref-filter.h | 5 +----\n 2 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 46963a5..e53c77e 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -55,6 +55,11 @@ static struct {\n \t{ \"color\" },\n };\n \n+struct atom_value {\n+\tconst char *s;\n+\tunsigned long ul; /* used for sorting when not FIELD_STR */\n+};\n+\n /*\n * An atom is a valid field atom listed above, possibly prefixed with\n * a \"*\" to denote deref_tag().\ndiff --git a/ref-filter.h b/ref-filter.h\nindex 6bf27d8..45026d0 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -16,10 +16,7 @@\n #define FILTER_REFS_INCLUDE_BROKEN 0x1\n #define FILTER_REFS_ALL 0x2\n \n-struct atom_value {\n-\tconst char *s;\n-\tunsigned long ul; /* used for sorting when not FIELD_STR */\n-};\n+struct atom_value;\n \n struct ref_sorting {\n \tstruct ref_sorting *next;\n-- \n2.5.1\n"},{"id":"269507","messageId":"1441479135-5285-3-git-send-email-Karthik.188@gmail.com","threadId":"40286","inReplyTo":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v16 02/14] ref-filter: introduce ref_formatting_state and ref_formatting_stack","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-05T18:52:03Z","receivedAt":"2015-09-05T18:52:03Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Introduce ref_formatting_state which will hold the formatted output\nstrbuf instead of directly printing to stdout. This will help us in\ncreating modifier atoms which modify the format specified before\nprinting to stdout.\n\nImplement a stack machinery for ref_formatting_state, this allows us\nto push and pop elements onto the stack. Whenever we pop an element\nfrom the stack, the strbuf from that element is appended to the strbuf\nof the next element on the stack, this will allow us to support\nnesting of modifier atoms.\n\nRename some functions to reflect the changes made:\nprint_value() -> append_atom()\nemit() -> append_literal()\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n ref-filter.c | 78 +++++++++++++++++++++++++++++++++++++++++++++---------------\n 1 file changed, 59 insertions(+), 19 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex e53c77e..432cea0 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -55,6 +55,18 @@ static struct {\n \t{ \"color\" },\n };\n \n+#define REF_FORMATTING_STATE_INIT { 0, NULL }\n+\n+struct ref_formatting_stack {\n+\tstruct ref_formatting_stack *prev;\n+\tstruct strbuf output;\n+};\n+\n+struct ref_formatting_state {\n+\tint quote_style;\n+\tstruct ref_formatting_stack *stack;\n+};\n+\n struct atom_value {\n \tconst char *s;\n \tunsigned long ul; /* used for sorting when not FIELD_STR */\n@@ -129,6 +141,27 @@ int parse_ref_filter_atom(const char *atom, const char *ep)\n \treturn at;\n }\n \n+static void push_stack_element(struct ref_formatting_stack **stack)\n+{\n+\tstruct ref_formatting_stack *s = xcalloc(1, sizeof(struct ref_formatting_stack));\n+\n+\tstrbuf_init(&s->output, 0);\n+\ts->prev = *stack;\n+\t*stack = s;\n+}\n+\n+static void pop_stack_element(struct ref_formatting_stack **stack)\n+{\n+\tstruct ref_formatting_stack *current = *stack;\n+\tstruct ref_formatting_stack *prev = current->prev;\n+\n+\tif (prev)\n+\t\tstrbuf_addbuf(&prev->output, ¤t->output);\n+\tstrbuf_release(¤t->output);\n+\tfree(current);\n+\t*stack = prev;\n+}\n+\n /*\n * In a format string, find the next occurrence of %(atom).\n */\n@@ -1195,30 +1228,27 @@ void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)\n \tqsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);\n }\n \n-static void print_value(struct atom_value *v, int quote_style)\n+static void append_atom(struct atom_value *v, struct ref_formatting_state *state)\n {\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tswitch (quote_style) {\n+\tstruct strbuf *s = &state->stack->output;\n+\n+\tswitch (state->quote_style) {\n \tcase QUOTE_NONE:\n-\t\tfputs(v->s, stdout);\n+\t\tstrbuf_addstr(s, v->s);\n \t\tbreak;\n \tcase QUOTE_SHELL:\n-\t\tsq_quote_buf(&sb, v->s);\n+\t\tsq_quote_buf(s, v->s);\n \t\tbreak;\n \tcase QUOTE_PERL:\n-\t\tperl_quote_buf(&sb, v->s);\n+\t\tperl_quote_buf(s, v->s);\n \t\tbreak;\n \tcase QUOTE_PYTHON:\n-\t\tpython_quote_buf(&sb, v->s);\n+\t\tpython_quote_buf(s, v->s);\n \t\tbreak;\n \tcase QUOTE_TCL:\n-\t\ttcl_quote_buf(&sb, v->s);\n+\t\ttcl_quote_buf(s, v->s);\n \t\tbreak;\n \t}\n-\tif (quote_style != QUOTE_NONE) {\n-\t\tfputs(sb.buf, stdout);\n-\t\tstrbuf_release(&sb);\n-\t}\n }\n \n static int hex1(char ch)\n@@ -1239,8 +1269,10 @@ static int hex2(const char *cp)\n \t\treturn -1;\n }\n \n-static void emit(const char *cp, const char *ep)\n+static void append_literal(const char *cp, const char *ep, struct ref_formatting_state *state)\n {\n+\tstruct strbuf *s = &state->stack->output;\n+\n \twhile (*cp && (!ep || cp < ep)) {\n \t\tif (*cp == '%') {\n \t\t\tif (cp[1] == '%')\n@@ -1248,13 +1280,13 @@ static void emit(const char *cp, const char *ep)\n \t\t\telse {\n \t\t\t\tint ch = hex2(cp + 1);\n \t\t\t\tif (0 <= ch) {\n-\t\t\t\t\tputchar(ch);\n+\t\t\t\t\tstrbuf_addch(s, ch);\n \t\t\t\t\tcp += 3;\n \t\t\t\t\tcontinue;\n \t\t\t\t}\n \t\t\t}\n \t\t}\n-\t\tputchar(*cp);\n+\t\tstrbuf_addch(s, *cp);\n \t\tcp++;\n \t}\n }\n@@ -1262,19 +1294,24 @@ static void emit(const char *cp, const char *ep)\n void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)\n {\n \tconst char *cp, *sp, *ep;\n+\tstruct strbuf *final_buf;\n+\tstruct ref_formatting_state state = REF_FORMATTING_STATE_INIT;\n+\n+\tstate.quote_style = quote_style;\n+\tpush_stack_element(&state.stack);\n \n \tfor (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {\n \t\tstruct atom_value *atomv;\n \n \t\tep = strchr(sp, ')');\n \t\tif (cp < sp)\n-\t\t\temit(cp, sp);\n+\t\t\tappend_literal(cp, sp, &state);\n \t\tget_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);\n-\t\tprint_value(atomv, quote_style);\n+\t\tappend_atom(atomv, &state);\n \t}\n \tif (*cp) {\n \t\tsp = cp + strlen(cp);\n-\t\temit(cp, sp);\n+\t\tappend_literal(cp, sp, &state);\n \t}\n \tif (need_color_reset_at_eol) {\n \t\tstruct atom_value resetv;\n@@ -1283,8 +1320,11 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n \t\tif (color_parse(\"reset\", color) < 0)\n \t\t\tdie(\"BUG: couldn't parse 'reset' as a color\");\n \t\tresetv.s = color;\n-\t\tprint_value(&resetv, quote_style);\n+\t\tappend_atom(&resetv, &state);\n \t}\n+\tfinal_buf = &state.stack->output;\n+\tfwrite(final_buf->buf, 1, final_buf->len, stdout);\n+\tpop_stack_element(&state.stack);\n \tputchar('\\n');\n }\n \n-- \n2.5.1\n"},{"id":"269506","messageId":"1441479135-5285-4-git-send-email-Karthik.188@gmail.com","threadId":"40286","inReplyTo":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v16 03/14] utf8: add function to align a string into given strbuf","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-05T18:52:04Z","receivedAt":"2015-09-05T18:52:04Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Add strbuf_utf8_align() which will align a given string into a strbuf\nas per given align_type and width. If the width is greater than the\nstring length then no alignment is performed.\n\nHelped-by: Eric Sunshine \nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n utf8.c | 21 +++++++++++++++++++++\n utf8.h | 15 +++++++++++++++\n 2 files changed, 36 insertions(+)\n\ndiff --git a/utf8.c b/utf8.c\nindex 28e6d76..00e10c8 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -644,3 +644,24 @@ int skip_utf8_bom(char **text, size_t len)\n \t*text += strlen(utf8_bom);\n \treturn 1;\n }\n+\n+void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int width,\n+\t\t const char *s)\n+{\n+\tint slen = strlen(s);\n+\tint display_len = utf8_strnwidth(s, slen, 0);\n+\tint utf8_compensation = slen - display_len;\n+\n+\tif (display_len >= width) {\n+\t\tstrbuf_addstr(buf, s);\n+\t\treturn;\n+\t}\n+\n+\tif (position == ALIGN_LEFT)\n+\t\tstrbuf_addf(buf, \"%-*s\", width + utf8_compensation, s);\n+\telse if (position == ALIGN_MIDDLE) {\n+\t\tint left = (width - display_len) / 2;\n+\t\tstrbuf_addf(buf, \"%*s%-*s\", left, \"\", width - left + utf8_compensation, s);\n+\t} else if (position == ALIGN_RIGHT)\n+\t\tstrbuf_addf(buf, \"%*s\", width + utf8_compensation, s);\n+}\ndiff --git a/utf8.h b/utf8.h\nindex 5a9e94b..7930b44 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -55,4 +55,19 @@ int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding);\n */\n int is_hfs_dotgit(const char *path);\n \n+typedef enum {\n+\tALIGN_LEFT,\n+\tALIGN_MIDDLE,\n+\tALIGN_RIGHT\n+} align_type;\n+\n+/*\n+ * Align the string given and store it into a strbuf as per the\n+ * 'position' and 'width'. If the given string length is larger than\n+ * 'width' than then the input string is not truncated and no\n+ * alignment is done.\n+ */\n+void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int width,\n+\t\t const char *s);\n+\n #endif\n-- \n2.5.1\n"},{"id":"269505","messageId":"1441479135-5285-5-git-send-email-Karthik.188@gmail.com","threadId":"40286","inReplyTo":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v16 04/14] ref-filter: introduce handler function for each atom","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-05T18:52:05Z","receivedAt":"2015-09-05T18:52:05Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Introduce a handler function for each atom, which is called when the\natom is processed in show_ref_array_item().\n\nIn this context make append_atom() as the default handler function and\nextract quote_formatting() out of append_atom(). Bump this to the top.\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n ref-filter.c | 54 ++++++++++++++++++++++++++++++------------------------\n 1 file changed, 30 insertions(+), 24 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 432cea0..a993216 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -69,6 +69,7 @@ struct ref_formatting_state {\n \n struct atom_value {\n \tconst char *s;\n+\tvoid (*handler)(struct atom_value *atomv, struct ref_formatting_state *state);\n \tunsigned long ul; /* used for sorting when not FIELD_STR */\n };\n \n@@ -141,6 +142,32 @@ int parse_ref_filter_atom(const char *atom, const char *ep)\n \treturn at;\n }\n \n+static void quote_formatting(struct strbuf *s, const char *str, int quote_style)\n+{\n+\tswitch (quote_style) {\n+\tcase QUOTE_NONE:\n+\t\tstrbuf_addstr(s, str);\n+\t\tbreak;\n+\tcase QUOTE_SHELL:\n+\t\tsq_quote_buf(s, str);\n+\t\tbreak;\n+\tcase QUOTE_PERL:\n+\t\tperl_quote_buf(s, str);\n+\t\tbreak;\n+\tcase QUOTE_PYTHON:\n+\t\tpython_quote_buf(s, str);\n+\t\tbreak;\n+\tcase QUOTE_TCL:\n+\t\ttcl_quote_buf(s, str);\n+\t\tbreak;\n+\t}\n+}\n+\n+static void append_atom(struct atom_value *v, struct ref_formatting_state *state)\n+{\n+\tquote_formatting(&state->stack->output, v->s, state->quote_style);\n+}\n+\n static void push_stack_element(struct ref_formatting_stack **stack)\n {\n \tstruct ref_formatting_stack *s = xcalloc(1, sizeof(struct ref_formatting_stack));\n@@ -662,6 +689,8 @@ static void populate_value(struct ref_array_item *ref)\n \t\tconst char *formatp;\n \t\tstruct branch *branch = NULL;\n \n+\t\tv->handler = append_atom;\n+\n \t\tif (*name == '*') {\n \t\t\tderef = 1;\n \t\t\tname++;\n@@ -1228,29 +1257,6 @@ void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)\n \tqsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);\n }\n \n-static void append_atom(struct atom_value *v, struct ref_formatting_state *state)\n-{\n-\tstruct strbuf *s = &state->stack->output;\n-\n-\tswitch (state->quote_style) {\n-\tcase QUOTE_NONE:\n-\t\tstrbuf_addstr(s, v->s);\n-\t\tbreak;\n-\tcase QUOTE_SHELL:\n-\t\tsq_quote_buf(s, v->s);\n-\t\tbreak;\n-\tcase QUOTE_PERL:\n-\t\tperl_quote_buf(s, v->s);\n-\t\tbreak;\n-\tcase QUOTE_PYTHON:\n-\t\tpython_quote_buf(s, v->s);\n-\t\tbreak;\n-\tcase QUOTE_TCL:\n-\t\ttcl_quote_buf(s, v->s);\n-\t\tbreak;\n-\t}\n-}\n-\n static int hex1(char ch)\n {\n \tif ('0' <= ch && ch <= '9')\n@@ -1307,7 +1313,7 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n \t\tif (cp < sp)\n \t\t\tappend_literal(cp, sp, &state);\n \t\tget_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);\n-\t\tappend_atom(atomv, &state);\n+\t\tatomv->handler(atomv, &state);\n \t}\n \tif (*cp) {\n \t\tsp = cp + strlen(cp);\n-- \n2.5.1\n"},{"id":"269508","messageId":"1441479135-5285-6-git-send-email-Karthik.188@gmail.com","threadId":"40286","inReplyTo":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v16 05/14] ref-filter: introduce match_atom_name()","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-05T18:52:06Z","receivedAt":"2015-09-05T18:52:06Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Introduce match_atom_name() which helps in checking if a particular\natom is the atom we're looking for and if it has a value attached to\nit or not.\n\nUse it instead of starts_with() for checking the value of %(color:...)\natom. Write a test for the same.\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nThanks-to: Junio C Hamano \nSigned-off-by: Karthik Nayak \n---\n ref-filter.c | 23 +++++++++++++++++++++--\n t/t6302-for-each-ref-filter.sh | 4 ++++\n 2 files changed, 25 insertions(+), 2 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex a993216..e99c342 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -189,6 +189,22 @@ static void pop_stack_element(struct ref_formatting_stack **stack)\n \t*stack = prev;\n }\n \n+static int match_atom_name(const char *name, const char *atom_name, const char **val)\n+{\n+\tconst char *body;\n+\n+\tif (!skip_prefix(name, atom_name, &body))\n+\t\treturn 0; /* doesn't even begin with \"atom_name\" */\n+\tif (!body[0] || !body[1]) {\n+\t\t*val = NULL; /* %(atom_name) and no customization */\n+\t\treturn 1;\n+\t}\n+\tif (body[0] != ':')\n+\t\treturn 0; /* \"atom_namefoo\" is not \"atom_name\" or \"atom_name:...\" */\n+\t*val = body + 1; /* \"atomname:val\" */\n+\treturn 1;\n+}\n+\n /*\n * In a format string, find the next occurrence of %(atom).\n */\n@@ -687,6 +703,7 @@ static void populate_value(struct ref_array_item *ref)\n \t\tint deref = 0;\n \t\tconst char *refname;\n \t\tconst char *formatp;\n+\t\tconst char *valp;\n \t\tstruct branch *branch = NULL;\n \n \t\tv->handler = append_atom;\n@@ -721,10 +738,12 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\trefname = branch_get_push(branch, NULL);\n \t\t\tif (!refname)\n \t\t\t\tcontinue;\n-\t\t} else if (starts_with(name, \"color:\")) {\n+\t\t} else if (match_atom_name(name, \"color\", &valp)) {\n \t\t\tchar color[COLOR_MAXLEN] = \"\";\n \n-\t\t\tif (color_parse(name + 6, color) < 0)\n+\t\t\tif (!valp)\n+\t\t\t\tdie(_(\"expected format: %%(color:)\"));\n+\t\t\tif (color_parse(valp, color) < 0)\n \t\t\t\tdie(_(\"unable to parse format\"));\n \t\t\tv->s = xstrdup(color);\n \t\t\tcontinue;\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex 505a360..c4f0378 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -81,4 +81,8 @@ test_expect_success 'filtering with --contains' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '%(color) must fail' '\n+\ttest_must_fail git for-each-ref --format=\"%(color)%(refname)\"\n+'\n+\n test_done\n-- \n2.5.1\n"},{"id":"269510","messageId":"1441479135-5285-7-git-send-email-Karthik.188@gmail.com","threadId":"40286","inReplyTo":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v16 06/14] ref-filter: implement an `align` atom","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-05T18:52:07Z","receivedAt":"2015-09-05T18:52:07Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Implement an `align` atom which left-, middle-, or right-aligns the\ncontent between %(align:...) and %(end).\n\nIt is followed by `:,`, where the `` is\neither left, right or middle and `` is the size of the area\ninto which the content will be placed. If the content between\n%(align:...) and %(end) is more than the width then no alignment is\nperformed. e.g. to align a refname atom to the middle with a total\nwidth of 40 we can do: --format=\"%(align:middle,40)%(refname)%(end)\".\n\nWe introduce an `at_end` function for each element of the stack which\nis to be called when the `end` atom is encountered. Using this we\nimplement end_align_handler() for the `align` atom, this aligns the\nfinal strbuf by calling `strbuf_utf8_align()` from utf8.c.\n\nEnsure that quote formatting is performed on the whole of\n%(align:...)...%(end) rather than individual atoms inside. We skip\nquote formatting for individual atoms when the current stack element\nis handling an %(align:...) atom and perform quote formatting at the\nend when we encounter the %(end) atom of the second element of then\nstack.\n\nAdd documentation and tests for the same.\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n Documentation/git-for-each-ref.txt | 10 ++++\n ref-filter.c | 112 ++++++++++++++++++++++++++++++++++++-\n t/t6302-for-each-ref-filter.sh | 82 +++++++++++++++++++++++++++\n 3 files changed, 203 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex e49d578..b23412d 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -127,6 +127,16 @@ color::\n \tChange output color. Followed by `:`, where names\n \tare described in `color.branch.*`.\n \n+align::\n+\tLeft-, middle-, or right-align the content between %(align:...)\n+\tand %(end). Followed by `:,`, where the\n+\t`` is either left, right or middle and `` is\n+\tthe total length of the content with alignment. If the\n+\tcontents length is more than the width then no alignment is\n+\tperformed. If used with '--quote' everything in between\n+\t%(align:...) and %(end) is quoted, but if nested then only the\n+\ttopmost level performs quoting.\n+\n In addition to the above, for commit and tag objects, the header\n field names (`tree`, `parent`, `object`, `type`, and `tag`) can\n be used to specify the value in the header field.\ndiff --git a/ref-filter.c b/ref-filter.c\nindex e99c342..6c9ef08 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@@ -53,13 +54,22 @@ static struct {\n \t{ \"flag\" },\n \t{ \"HEAD\" },\n \t{ \"color\" },\n+\t{ \"align\" },\n+\t{ \"end\" },\n };\n \n #define REF_FORMATTING_STATE_INIT { 0, NULL }\n \n+struct align {\n+\talign_type position;\n+\tunsigned int width;\n+};\n+\n struct ref_formatting_stack {\n \tstruct ref_formatting_stack *prev;\n \tstruct strbuf output;\n+\tvoid (*at_end)(struct ref_formatting_stack *stack);\n+\tvoid *at_end_data;\n };\n \n struct ref_formatting_state {\n@@ -69,6 +79,9 @@ struct ref_formatting_state {\n \n struct atom_value {\n \tconst char *s;\n+\tunion {\n+\t\tstruct align align;\n+\t} u;\n \tvoid (*handler)(struct atom_value *atomv, struct ref_formatting_state *state);\n \tunsigned long ul; /* used for sorting when not FIELD_STR */\n };\n@@ -165,7 +178,16 @@ static void quote_formatting(struct strbuf *s, const char *str, int quote_style)\n \n static void append_atom(struct atom_value *v, struct ref_formatting_state *state)\n {\n-\tquote_formatting(&state->stack->output, v->s, state->quote_style);\n+\t/*\n+\t * Quote formatting is only done when the stack has a single\n+\t * element. Otherwise quote formatting is done on the\n+\t * element's entire output strbuf when the %(end) atom is\n+\t * encountered.\n+\t */\n+\tif (!state->stack->prev)\n+\t\tquote_formatting(&state->stack->output, v->s, state->quote_style);\n+\telse\n+\t\tstrbuf_addstr(&state->stack->output, v->s);\n }\n \n static void push_stack_element(struct ref_formatting_stack **stack)\n@@ -189,6 +211,48 @@ static void pop_stack_element(struct ref_formatting_stack **stack)\n \t*stack = prev;\n }\n \n+static void end_align_handler(struct ref_formatting_stack *stack)\n+{\n+\tstruct align *align = (struct align *)stack->at_end_data;\n+\tstruct strbuf s = STRBUF_INIT;\n+\n+\tstrbuf_utf8_align(&s, align->position, align->width, stack->output.buf);\n+\tstrbuf_swap(&stack->output, &s);\n+\tstrbuf_release(&s);\n+}\n+\n+static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)\n+{\n+\tstruct ref_formatting_stack *new;\n+\n+\tpush_stack_element(&state->stack);\n+\tnew = state->stack;\n+\tnew->at_end = end_align_handler;\n+\tnew->at_end_data = &atomv->u.align;\n+}\n+\n+static void end_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)\n+{\n+\tstruct ref_formatting_stack *current = state->stack;\n+\tstruct strbuf s = STRBUF_INIT;\n+\n+\tif (!current->at_end)\n+\t\tdie(_(\"format: %%(end) atom used without corresponding atom\"));\n+\tcurrent->at_end(current);\n+\n+\t/*\n+\t * Perform quote formatting when the stack element is that of\n+\t * a supporting atom. If nested then perform quote formatting\n+\t * only on the topmost supporting atom.\n+\t */\n+\tif (!state->stack->prev->prev) {\n+\t\tquote_formatting(&s, current->output.buf, state->quote_style);\n+\t\tstrbuf_swap(¤t->output, &s);\n+\t}\n+\tstrbuf_release(&s);\n+\tpop_stack_element(&state->stack);\n+}\n+\n static int match_atom_name(const char *name, const char *atom_name, const char **val)\n {\n \tconst char *body;\n@@ -773,6 +837,50 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\telse\n \t\t\t\tv->s = \" \";\n \t\t\tcontinue;\n+\t\t} else if (match_atom_name(name, \"align\", &valp)) {\n+\t\t\tstruct align *align = &v->u.align;\n+\t\t\tstruct strbuf **s;\n+\n+\t\t\tif (!valp)\n+\t\t\t\tdie(_(\"expected format: %%(align:, )\"));\n+\n+\t\t\t/*\n+\t\t\t * TODO: Implement a function similar to strbuf_split_str()\n+\t\t\t * which would strip the terminator at the end.\n+\t\t\t */\n+\t\t\ts = strbuf_split_str(valp, ',', 0);\n+\n+\t\t\t/* If the position is given trim the ',' from the first strbuf */\n+\t\t\tif (s[1])\n+\t\t\t\tstrbuf_setlen(s[0], s[0]->len - 1);\n+\t\t\tif (s[2])\n+\t\t\t\tdie(_(\"align:, followed by garbage: %s\"), s[2]->buf);\n+\n+\t\t\tif (strtoul_ui(s[0]->buf, 10, &align->width))\n+\t\t\t\tdie(_(\"positive width expected align:%s\"), s[0]->buf);\n+\n+\t\t\t/*\n+\t\t\t * TODO: Implement a more general check, so that the values\n+\t\t\t * do not always have to be in a specific order.\n+\t\t\t */\n+\t\t\tif (!s[1])\n+\t\t\t\talign->position = ALIGN_LEFT;\n+\t\t\telse if (!strcmp(s[1]->buf, \"left\"))\n+\t\t\t\talign->position = ALIGN_LEFT;\n+\t\t\telse if (!strcmp(s[1]->buf, \"right\"))\n+\t\t\t\talign->position = ALIGN_RIGHT;\n+\t\t\telse if (!strcmp(s[1]->buf, \"middle\"))\n+\t\t\t\talign->position = ALIGN_MIDDLE;\n+\t\t\telse\n+\t\t\t\tdie(_(\"improper format entered align:%s\"), s[1]->buf);\n+\n+\t\t\tstrbuf_list_free(s);\n+\n+\t\t\tv->handler = align_atom_handler;\n+\t\t\tcontinue;\n+\t\t} else if (!strcmp(name, \"end\")) {\n+\t\t\tv->handler = end_atom_handler;\n+\t\t\tcontinue;\n \t\t} else\n \t\t\tcontinue;\n \n@@ -1347,6 +1455,8 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n \t\tresetv.s = color;\n \t\tappend_atom(&resetv, &state);\n \t}\n+\tif (state.stack->prev)\n+\t\tdie(_(\"format: %%(end) atom missing\"));\n \tfinal_buf = &state.stack->output;\n \tfwrite(final_buf->buf, 1, final_buf->len, stdout);\n \tpop_stack_element(&state.stack);\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex c4f0378..d0c0139 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -85,4 +85,86 @@ test_expect_success '%(color) must fail' '\n \ttest_must_fail git for-each-ref --format=\"%(color)%(refname)\"\n '\n \n+test_expect_success 'left alignment' '\n+\tcat >expect <<-\\EOF &&\n+\trefname is refs/heads/master |refs/heads/master\n+\trefname is refs/heads/side |refs/heads/side\n+\trefname is refs/odd/spot |refs/odd/spot\n+\trefname is refs/tags/double-tag|refs/tags/double-tag\n+\trefname is refs/tags/four |refs/tags/four\n+\trefname is refs/tags/one |refs/tags/one\n+\trefname is refs/tags/signed-tag|refs/tags/signed-tag\n+\trefname is refs/tags/three |refs/tags/three\n+\trefname is refs/tags/two |refs/tags/two\n+\tEOF\n+\tgit for-each-ref --format=\"%(align:30,left)refname is %(refname)%(end)|%(refname)\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'middle alignment' '\n+\tcat >expect <<-\\EOF &&\n+\t| refname is refs/heads/master |refs/heads/master\n+\t| refname is refs/heads/side |refs/heads/side\n+\t| refname is refs/odd/spot |refs/odd/spot\n+\t|refname is refs/tags/double-tag|refs/tags/double-tag\n+\t| refname is refs/tags/four |refs/tags/four\n+\t| refname is refs/tags/one |refs/tags/one\n+\t|refname is refs/tags/signed-tag|refs/tags/signed-tag\n+\t| refname is refs/tags/three |refs/tags/three\n+\t| refname is refs/tags/two |refs/tags/two\n+\tEOF\n+\tgit for-each-ref --format=\"|%(align:30,middle)refname is %(refname)%(end)|%(refname)\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'right alignment' '\n+\tcat >expect <<-\\EOF &&\n+\t| refname is refs/heads/master|refs/heads/master\n+\t| refname is refs/heads/side|refs/heads/side\n+\t| refname is refs/odd/spot|refs/odd/spot\n+\t|refname is refs/tags/double-tag|refs/tags/double-tag\n+\t| refname is refs/tags/four|refs/tags/four\n+\t| refname is refs/tags/one|refs/tags/one\n+\t|refname is refs/tags/signed-tag|refs/tags/signed-tag\n+\t| refname is refs/tags/three|refs/tags/three\n+\t| refname is refs/tags/two|refs/tags/two\n+\tEOF\n+\tgit for-each-ref --format=\"|%(align:30,right)refname is %(refname)%(end)|%(refname)\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+# Individual atoms inside %(align:...) and %(end) must not be quoted.\n+\n+test_expect_success 'alignment with format quote' \"\n+\tcat >expect <<-\\EOF &&\n+\t|' master| A U Thor '|\n+\t|' side| A U Thor '|\n+\t|' odd/spot| A U Thor '|\n+\t|' double-tag| '|\n+\t|' four| A U Thor '|\n+\t|' one| A U Thor '|\n+\t|' signed-tag| '|\n+\t|' three| A U Thor '|\n+\t|' two| A U Thor '|\n+\tEOF\n+\tgit for-each-ref --shell --format='|%(align:30,middle)%(refname:short)| %(authorname)%(end)|' >actual &&\n+\ttest_cmp expect actual\n+\"\n+\n+test_expect_success 'nested alignment with quote formatting' \"\n+\tcat >expect <<-\\EOF &&\n+\t|' master '|\n+\t|' side '|\n+\t|' odd/spot '|\n+\t|' double-tag '|\n+\t|' four '|\n+\t|' one '|\n+\t|' signed-tag '|\n+\t|' three '|\n+\t|' two '|\n+\tEOF\n+\tgit for-each-ref --shell --format='|%(align:30,left)%(align:15,right)%(refname:short)%(end)%(end)|' >actual &&\n+\ttest_cmp expect actual\n+\"\n+\n test_done\n-- \n2.5.1\n"},{"id":"269509","messageId":"1441479135-5285-8-git-send-email-Karthik.188@gmail.com","threadId":"40286","inReplyTo":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v16 07/14] ref-filter: add option to filter out tags, branches and remotes","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-05T18:52:08Z","receivedAt":"2015-09-05T18:52:08Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nAdd a function called 'for_each_fullref_in()' to refs.{c,h} which\niterates through each ref for the given path without trimming the path\nand also accounting for broken refs, if mentioned.\n\nAdd 'filter_ref_kind()' in ref-filter.c to check the kind of ref being\nhandled and return the kind to 'ref_filter_handler()', where we\ndiscard refs which we do not need and assign the kind to needed refs.\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n ref-filter.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-----\n ref-filter.h | 13 ++++++++++--\n refs.c | 9 +++++++++\n refs.h | 1 +\n 4 files changed, 81 insertions(+), 7 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 6c9ef08..c7a8cf0 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1192,6 +1192,34 @@ static struct ref_array_item *new_ref_array_item(const char *refname,\n \treturn ref;\n }\n \n+static int filter_ref_kind(struct ref_filter *filter, const char *refname)\n+{\n+\tunsigned int i;\n+\n+\tstatic struct {\n+\t\tconst char *prefix;\n+\t\tunsigned int kind;\n+\t} ref_kind[] = {\n+\t\t{ \"refs/heads/\" , FILTER_REFS_BRANCHES },\n+\t\t{ \"refs/remotes/\" , FILTER_REFS_REMOTES },\n+\t\t{ \"refs/tags/\", FILTER_REFS_TAGS}\n+\t};\n+\n+\tif (filter->kind == FILTER_REFS_BRANCHES ||\n+\t filter->kind == FILTER_REFS_REMOTES ||\n+\t filter->kind == FILTER_REFS_TAGS)\n+\t\treturn filter->kind;\n+\telse if (!strcmp(refname, \"HEAD\"))\n+\t\treturn FILTER_REFS_DETACHED_HEAD;\n+\n+\tfor (i = 0; i < ARRAY_SIZE(ref_kind); i++) {\n+\t\tif (starts_with(refname, ref_kind[i].prefix))\n+\t\t\treturn ref_kind[i].kind;\n+\t}\n+\n+\treturn FILTER_REFS_OTHERS;\n+}\n+\n /*\n * A call-back given to for_each_ref(). Filter refs and keep them for\n * later object processing.\n@@ -1202,6 +1230,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,\n \tstruct ref_filter *filter = ref_cbdata->filter;\n \tstruct ref_array_item *ref;\n \tstruct commit *commit = NULL;\n+\tunsigned int kind;\n \n \tif (flag & REF_BAD_NAME) {\n \t\twarning(\"ignoring ref with broken name %s\", refname);\n@@ -1213,6 +1242,11 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,\n \t\treturn 0;\n \t}\n \n+\t/* Obtain the current ref kind from filter_ref_kind() and ignore unwanted refs. */\n+\tkind = filter_ref_kind(filter, refname);\n+\tif (!(kind & filter->kind))\n+\t\treturn 0;\n+\n \tif (*filter->name_patterns && !match_name_as_path(filter->name_patterns, refname))\n \t\treturn 0;\n \n@@ -1244,6 +1278,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,\n \n \tREALLOC_ARRAY(ref_cbdata->array->items, ref_cbdata->array->nr + 1);\n \tref_cbdata->array->items[ref_cbdata->array->nr++] = ref;\n+\tref->kind = kind;\n \treturn 0;\n }\n \n@@ -1320,17 +1355,37 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n {\n \tstruct ref_filter_cbdata ref_cbdata;\n \tint ret = 0;\n+\tunsigned int broken = 0;\n \n \tref_cbdata.array = array;\n \tref_cbdata.filter = filter;\n \n+\tif (type & FILTER_REFS_INCLUDE_BROKEN)\n+\t\tbroken = 1;\n+\tfilter->kind = type & FILTER_REFS_KIND_MASK;\n+\n \t/* Simple per-ref filtering */\n-\tif (type & (FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN))\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)\n+\tif (!filter->kind)\n \t\tdie(\"filter_refs: invalid type\");\n+\telse {\n+\t\t/*\n+\t\t * For common cases where we need only branches or remotes or tags,\n+\t\t * we only iterate through those refs. If a mix of refs is needed,\n+\t\t * we iterate over all refs and filter out required refs with the help\n+\t\t * of filter_ref_kind().\n+\t\t */\n+\t\tif (filter->kind == FILTER_REFS_BRANCHES)\n+\t\t\tret = for_each_fullref_in(\"refs/heads/\", ref_filter_handler, &ref_cbdata, broken);\n+\t\telse if (filter->kind == FILTER_REFS_REMOTES)\n+\t\t\tret = for_each_fullref_in(\"refs/remotes/\", ref_filter_handler, &ref_cbdata, broken);\n+\t\telse if (filter->kind == FILTER_REFS_TAGS)\n+\t\t\tret = for_each_fullref_in(\"refs/tags/\", ref_filter_handler, &ref_cbdata, broken);\n+\t\telse if (filter->kind & FILTER_REFS_ALL)\n+\t\t\tret = for_each_fullref_in(\"\", ref_filter_handler, &ref_cbdata, broken);\n+\t\tif (!ret && (filter->kind & FILTER_REFS_DETACHED_HEAD))\n+\t\t\thead_ref(ref_filter_handler, &ref_cbdata);\n+\t}\n+\n \n \t/* Filters that need revision walking */\n \tif (filter->merge_commit)\ndiff --git a/ref-filter.h b/ref-filter.h\nindex 45026d0..0913ba9 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -13,8 +13,15 @@\n #define QUOTE_PYTHON 4\n #define QUOTE_TCL 8\n \n-#define FILTER_REFS_INCLUDE_BROKEN 0x1\n-#define FILTER_REFS_ALL 0x2\n+#define FILTER_REFS_INCLUDE_BROKEN 0x0001\n+#define FILTER_REFS_TAGS 0x0002\n+#define FILTER_REFS_BRANCHES 0x0004\n+#define FILTER_REFS_REMOTES 0x0008\n+#define FILTER_REFS_OTHERS 0x0010\n+#define FILTER_REFS_ALL (FILTER_REFS_TAGS | FILTER_REFS_BRANCHES | \\\n+\t\t\t\t FILTER_REFS_REMOTES | FILTER_REFS_OTHERS)\n+#define FILTER_REFS_DETACHED_HEAD 0x0020\n+#define FILTER_REFS_KIND_MASK (FILTER_REFS_ALL | FILTER_REFS_DETACHED_HEAD)\n \n struct atom_value;\n \n@@ -27,6 +34,7 @@ struct ref_sorting {\n struct ref_array_item {\n \tunsigned char objectname[20];\n \tint flag;\n+\tunsigned int kind;\n \tconst char *symref;\n \tstruct commit *commit;\n \tstruct atom_value *value;\n@@ -51,6 +59,7 @@ struct ref_filter {\n \tstruct commit *merge_commit;\n \n \tunsigned int with_commit_tag_algo : 1;\n+\tunsigned int kind;\n };\n \n struct ref_filter_cbdata {\ndiff --git a/refs.c b/refs.c\nindex 4e15f60..a9469c2 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2108,6 +2108,15 @@ int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)\n \treturn do_for_each_ref(&ref_cache, prefix, fn, strlen(prefix), 0, cb_data);\n }\n \n+int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken)\n+{\n+\tunsigned int flag = 0;\n+\n+\tif (broken)\n+\t\tflag = DO_FOR_EACH_INCLUDE_BROKEN;\n+\treturn do_for_each_ref(&ref_cache, prefix, fn, 0, flag, cb_data);\n+}\n+\n int for_each_ref_in_submodule(const char *submodule, const char *prefix,\n \t\teach_ref_fn fn, void *cb_data)\n {\ndiff --git a/refs.h b/refs.h\nindex e9a5f32..6d30c98 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -173,6 +173,7 @@ typedef int each_ref_fn(const char *refname,\n extern int head_ref(each_ref_fn fn, void *cb_data);\n extern int for_each_ref(each_ref_fn fn, void *cb_data);\n extern int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data);\n+extern int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken);\n extern int for_each_tag_ref(each_ref_fn fn, void *cb_data);\n extern int for_each_branch_ref(each_ref_fn fn, void *cb_data);\n extern int for_each_remote_ref(each_ref_fn fn, void *cb_data);\n-- \n2.5.1\n"},{"id":"269511","messageId":"1441479135-5285-9-git-send-email-Karthik.188@gmail.com","threadId":"40286","inReplyTo":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v16 08/14] ref-filter: add support for %(contents:lines=X)","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-05T18:52:09Z","receivedAt":"2015-09-05T18:52:09Z","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 using\nthe '-n' option. Copy code from 'tag.c' to 'ref-filter' and\nmodify it to support appending of N lines from the annotation of tags\nto the given strbuf.\n\nImplement %(contents:lines=X) where X lines of the given object are\nobtained.\n\nAdd documentation and test for the same.\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n Documentation/git-for-each-ref.txt | 3 ++-\n builtin/tag.c | 4 +++\n ref-filter.c | 45 ++++++++++++++++++++++++++++++++-\n ref-filter.h | 3 ++-\n t/t6302-for-each-ref-filter.sh | 52 ++++++++++++++++++++++++++++++++++++++\n 5 files changed, 104 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex b23412d..99d108a 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -149,7 +149,8 @@ The complete message in a commit and tag object is `contents`.\n Its first line is `contents:subject`, where subject is the concatenation\n of all lines of the commit message up to the first blank line. The next\n line is 'contents:body', where body is all of the lines after the first\n-blank line. Finally, the optional GPG signature is `contents:signature`.\n+blank line. The optional GPG signature is `contents:signature`. The\n+first `N` lines of the message is obtained using `contents:lines=N`.\n \n For sorting purposes, fields with numeric values sort in numeric\n order (`objectsize`, `authordate`, `committerdate`, `taggerdate`).\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 471d6b1..b0bc1c5 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 modified and used in ref-filter as append_lines(), will\n+ * eventually be removed as 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 c7a8cf0..4435ec8 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -56,6 +56,7 @@ static struct {\n \t{ \"color\" },\n \t{ \"align\" },\n \t{ \"end\" },\n+\t{ \"contents:lines\" },\n };\n \n #define REF_FORMATTING_STATE_INIT { 0, NULL }\n@@ -65,6 +66,11 @@ struct align {\n \tunsigned int width;\n };\n \n+struct contents {\n+\tunsigned int lines;\n+\tstruct object_id oid;\n+};\n+\n struct ref_formatting_stack {\n \tstruct ref_formatting_stack *prev;\n \tstruct strbuf output;\n@@ -81,6 +87,7 @@ struct atom_value {\n \tconst char *s;\n \tunion {\n \t\tstruct align align;\n+\t\tstruct contents contents;\n \t} u;\n \tvoid (*handler)(struct atom_value *atomv, struct ref_formatting_state *state);\n \tunsigned long ul; /* used for sorting when not FIELD_STR */\n@@ -643,6 +650,30 @@ static void find_subpos(const char *buf, unsigned long sz,\n \t*nonsiglen = *sig - buf;\n }\n \n+/*\n+ * If 'lines' is greater than 0, append that many lines from the given\n+ * 'buf' of length 'size' to the given strbuf.\n+ */\n+static void append_lines(struct strbuf *out, const char *buf, unsigned long size, int lines)\n+{\n+\tint i;\n+\tconst char *sp, *eol;\n+\tsize_t len;\n+\n+\tsp = buf;\n+\n+\tfor (i = 0; i < lines && sp < buf + size; i++) {\n+\t\tif (i)\n+\t\t\tstrbuf_addstr(out, \"\\n \");\n+\t\teol = memchr(sp, '\\n', size - (sp - buf));\n+\t\tlen = eol ? eol - sp : size - (sp - buf);\n+\t\tstrbuf_add(out, sp, len);\n+\t\tif (!eol)\n+\t\t\tbreak;\n+\t\tsp = eol + 1;\n+\t}\n+}\n+\n /* See grab_values */\n static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n {\n@@ -653,6 +684,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i];\n \t\tstruct atom_value *v = &val[i];\n+\t\tconst char *valp = NULL;\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n@@ -662,7 +694,8 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n \t\t strcmp(name, \"contents\") &&\n \t\t strcmp(name, \"contents:subject\") &&\n \t\t strcmp(name, \"contents:body\") &&\n-\t\t strcmp(name, \"contents:signature\"))\n+\t\t strcmp(name, \"contents:signature\") &&\n+\t\t !starts_with(name, \"contents:lines=\"))\n \t\t\tcontinue;\n \t\tif (!subpos)\n \t\t\tfind_subpos(buf, sz,\n@@ -682,6 +715,16 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n \t\t\tv->s = xmemdupz(sigpos, siglen);\n \t\telse if (!strcmp(name, \"contents\"))\n \t\t\tv->s = xstrdup(subpos);\n+\t\telse if (skip_prefix(name, \"contents:lines=\", &valp)) {\n+\t\t\tstruct strbuf s = STRBUF_INIT;\n+\t\t\tconst char *contents_end = bodylen + bodypos - siglen;\n+\n+\t\t\tif (strtoul_ui(valp, 10, &v->u.contents.lines))\n+\t\t\t\tdie(_(\"positive value expected contents:lines=%s\"), valp);\n+\t\t\t/* Size is the length of the message after removing the signature */\n+\t\t\tappend_lines(&s, subpos, contents_end - subpos, v->u.contents.lines);\n+\t\t\tv->s = strbuf_detach(&s, NULL);\n+\t\t}\n \t}\n }\n \ndiff --git a/ref-filter.h b/ref-filter.h\nindex 0913ba9..ab76b22 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -59,7 +59,8 @@ struct ref_filter {\n \tstruct commit *merge_commit;\n \n \tunsigned int with_commit_tag_algo : 1;\n-\tunsigned int kind;\n+\tunsigned int kind,\n+\t\tlines;\n };\n \n struct ref_filter_cbdata {\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex d0c0139..428d139 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -167,4 +167,56 @@ test_expect_success 'nested alignment with quote formatting' \"\n \ttest_cmp expect actual\n \"\n \n+test_expect_success 'check `%(contents:lines=1)`' '\n+\tcat >expect <<-\\EOF &&\n+\tmaster |three\n+\tside |four\n+\todd/spot |three\n+\tdouble-tag |Annonated doubly\n+\tfour |four\n+\tone |one\n+\tsigned-tag |A signed tag message\n+\tthree |three\n+\ttwo |two\n+\tEOF\n+\tgit for-each-ref --format=\"%(refname:short) |%(contents:lines=1)\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'check `%(contents:lines=0)`' '\n+\tcat >expect <<-\\EOF &&\n+\tmaster |\n+\tside |\n+\todd/spot |\n+\tdouble-tag |\n+\tfour |\n+\tone |\n+\tsigned-tag |\n+\tthree |\n+\ttwo |\n+\tEOF\n+\tgit for-each-ref --format=\"%(refname:short) |%(contents:lines=0)\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'check `%(contents:lines=99999)`' '\n+\tcat >expect <<-\\EOF &&\n+\tmaster |three\n+\tside |four\n+\todd/spot |three\n+\tdouble-tag |Annonated doubly\n+\tfour |four\n+\tone |one\n+\tsigned-tag |A signed tag message\n+\tthree |three\n+\ttwo |two\n+\tEOF\n+\tgit for-each-ref --format=\"%(refname:short) |%(contents:lines=99999)\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '`%(contents:lines=-1)` should fail' '\n+\ttest_must_fail git for-each-ref --format=\"%(refname:short) |%(contents:lines=-1)\"\n+'\n+\n test_done\n-- \n2.5.1\n"},{"id":"269512","messageId":"1441479135-5285-10-git-send-email-Karthik.188@gmail.com","threadId":"40286","inReplyTo":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v16 09/14] ref-filter: add support to sort by version","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-05T18:52:10Z","receivedAt":"2015-09-05T18:52:10Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nAdd support to sort by version using the \"v:refname\" and\n\"version:refname\" option. This is achieved by using the 'versioncmp()'\nfunction as the comparing function for qsort.\n\nThis option is included to support sorting by versions in `git tag -l`\nwhich will eventually be ported to use ref-filter APIs.\n\nAdd documentation and tests for the same.\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n Documentation/git-for-each-ref.txt | 3 +++\n ref-filter.c | 15 ++++++++++-----\n ref-filter.h | 3 ++-\n t/t6302-for-each-ref-filter.sh | 36 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 51 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 99d108a..c5154bb 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -156,6 +156,9 @@ For sorting purposes, fields with numeric values sort in numeric\n order (`objectsize`, `authordate`, `committerdate`, `taggerdate`).\n All other fields are used to sort in their byte-value order.\n \n+There is also an option to sort by versions, this can be done by using\n+the fieldname `version:refname` or its alias `v:refname`.\n+\n In any case, a field name that refers to a field inapplicable to\n the object referred by the ref does not cause an error. It\n returns an empty string instead.\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 4435ec8..696c0c7 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -11,6 +11,8 @@\n #include \"ref-filter.h\"\n #include \"revision.h\"\n #include \"utf8.h\"\n+#include \"git-compat-util.h\"\n+#include \"version.h\"\n \n typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n \n@@ -1445,19 +1447,19 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru\n \n \tget_ref_atom_value(a, s->atom, &va);\n \tget_ref_atom_value(b, s->atom, &vb);\n-\tswitch (cmp_type) {\n-\tcase FIELD_STR:\n+\tif (s->version)\n+\t\tcmp = versioncmp(va->s, vb->s);\n+\telse if (cmp_type == FIELD_STR)\n \t\tcmp = strcmp(va->s, vb->s);\n-\t\tbreak;\n-\tdefault:\n+\telse {\n \t\tif (va->ul < vb->ul)\n \t\t\tcmp = -1;\n \t\telse if (va->ul == vb->ul)\n \t\t\tcmp = 0;\n \t\telse\n \t\t\tcmp = 1;\n-\t\tbreak;\n \t}\n+\n \treturn (s->reverse) ? -cmp : cmp;\n }\n \n@@ -1590,6 +1592,9 @@ int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)\n \t\ts->reverse = 1;\n \t\targ++;\n \t}\n+\tif (skip_prefix(arg, \"version:\", &arg) ||\n+\t skip_prefix(arg, \"v:\", &arg))\n+\t\ts->version = 1;\n \tlen = strlen(arg);\n \ts->atom = parse_ref_filter_atom(arg, arg+len);\n \treturn 0;\ndiff --git a/ref-filter.h b/ref-filter.h\nindex ab76b22..ef25b6e 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -28,7 +28,8 @@ struct atom_value;\n struct ref_sorting {\n \tstruct ref_sorting *next;\n \tint atom; /* index into used_atom array (internal) */\n-\tunsigned reverse : 1;\n+\tunsigned reverse : 1,\n+\t\tversion : 1;\n };\n \n struct ref_array_item {\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex 428d139..4bc1055 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -219,4 +219,40 @@ test_expect_success '`%(contents:lines=-1)` should fail' '\n \ttest_must_fail git for-each-ref --format=\"%(refname:short) |%(contents:lines=-1)\"\n '\n \n+test_expect_success 'setup for version sort' '\n+\ttest_commit foo1.3 &&\n+\ttest_commit foo1.6 &&\n+\ttest_commit foo1.10\n+'\n+\n+test_expect_success 'version sort' '\n+\tgit for-each-ref --sort=version:refname --format=\"%(refname:short)\" refs/tags/ | grep \"foo\" >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tfoo1.3\n+\tfoo1.6\n+\tfoo1.10\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'version sort (shortened)' '\n+\tgit for-each-ref --sort=v:refname --format=\"%(refname:short)\" refs/tags/ | grep \"foo\" >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tfoo1.3\n+\tfoo1.6\n+\tfoo1.10\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'reverse version sort' '\n+\tgit for-each-ref --sort=-version:refname --format=\"%(refname:short)\" refs/tags/ | grep \"foo\" >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tfoo1.10\n+\tfoo1.6\n+\tfoo1.3\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.5.1\n"},{"id":"269513","messageId":"1441479135-5285-11-git-send-email-Karthik.188@gmail.com","threadId":"40286","inReplyTo":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v16 10/14] ref-filter: add option to match literal pattern","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-05T18:52:11Z","receivedAt":"2015-09-05T18:52:11Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nSince 'ref-filter' only has an option to match path names add an\noption for plain fnmatch pattern-matching.\n\nThis is to support the pattern matching options which are used in `git\ntag -l` and `git branch -l` where we can match patterns like `git tag\n-l foo*` which would match all tags which has a \"foo*\" pattern.\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n builtin/for-each-ref.c | 1 +\n ref-filter.c | 40 +++++++++++++++++++++++++++++++++++++---\n ref-filter.h | 3 ++-\n 3 files changed, 40 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 40f343b..4e9f6c2 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 696c0c7..e3024d3 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1167,9 +1167,33 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)\n \n /*\n * Return 1 if the refname matches one of the patterns, otherwise 0.\n+ * A pattern can be a literal prefix (e.g. a refname \"refs/heads/master\"\n+ * matches a pattern \"refs/heads/mas\") or a wildcard (e.g. the same ref\n+ * matches \"refs/heads/mas*\", too).\n+ */\n+static int match_pattern(const char **patterns, const char *refname)\n+{\n+\t/*\n+\t * When no '--format' option is given we need to skip the prefix\n+\t * for matching refs of tags and branches.\n+\t */\n+\t(void)(skip_prefix(refname, \"refs/tags/\", &refname) ||\n+\t skip_prefix(refname, \"refs/heads/\", &refname) ||\n+\t skip_prefix(refname, \"refs/remotes/\", &refname) ||\n+\t skip_prefix(refname, \"refs/\", &refname));\n+\n+\tfor (; *patterns; patterns++) {\n+\t\tif (!wildmatch(*patterns, refname, 0, NULL))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+/*\n+ * Return 1 if the refname matches one of the patterns, otherwise 0.\n * A pattern can be path prefix (e.g. a refname \"refs/heads/master\"\n- * matches a pattern \"refs/heads/\") or a wildcard (e.g. the same ref\n- * matches \"refs/heads/m*\",too).\n+ * matches a pattern \"refs/heads/\" but not \"refs/heads/m\") or a\n+ * wildcard (e.g. the same ref matches \"refs/heads/m*\", too).\n */\n static int match_name_as_path(const char **pattern, const char *refname)\n {\n@@ -1190,6 +1214,16 @@ static int match_name_as_path(const char **pattern, const char *refname)\n \treturn 0;\n }\n \n+/* Return 1 if the refname matches one of the patterns, otherwise 0. */\n+static int filter_pattern_match(struct ref_filter *filter, const char *refname)\n+{\n+\tif (!*filter->name_patterns)\n+\t\treturn 1; /* No pattern always matches */\n+\tif (filter->match_as_path)\n+\t\treturn match_name_as_path(filter->name_patterns, refname);\n+\treturn match_pattern(filter->name_patterns, refname);\n+}\n+\n /*\n * Given a ref (sha1, refname), check if the ref belongs to the array\n * of sha1s. If the given ref is a tag, check if the given tag points\n@@ -1292,7 +1326,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,\n \tif (!(kind & filter->kind))\n \t\treturn 0;\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 ef25b6e..a5cfa5e 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -59,7 +59,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 kind,\n \t\tlines;\n };\n-- \n2.5.1\n"},{"id":"269514","messageId":"1441479135-5285-12-git-send-email-Karthik.188@gmail.com","threadId":"40286","inReplyTo":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v16 11/14] tag.c: use 'ref-filter' data structures","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-05T18:52:12Z","receivedAt":"2015-09-05T18:52:12Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nMake 'tag.c' use 'ref-filter' data structures and make changes to\nsupport the new data structures. This is a part of the process\nof porting 'tag.c' to use 'ref-filter' APIs.\n\nThis is a temporary step before porting 'tag.c' to use 'ref-filter'\ncompletely. As this is a temporary step, most of the code\nintroduced here will be removed when 'tag.c' is ported over to use\n'ref-filter' APIs.\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n builtin/tag.c | 106 +++++++++++++++++++++++++++++++---------------------------\n 1 file changed, 57 insertions(+), 49 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex b0bc1c5..fe66f7b 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,17 +579,17 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tconst char *object_ref, *tag;\n \tstruct create_tag_options opt;\n \tchar *cleanup_arg = NULL;\n-\tint annotate = 0, force = 0, lines = -1;\n \tint create_reflog = 0;\n+\tint annotate = 0, force = 0;\n \tint cmdmode = 0;\n \tconst char *msgfile = NULL, *keyid = NULL;\n \tstruct msg_arg msg = { 0, STRBUF_INIT };\n-\tstruct commit_list *with_commit = NULL;\n \tstruct ref_transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n+\tstruct ref_filter filter;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE('l', \"list\", &cmdmode, N_(\"list tag names\"), 'l'),\n-\t\t{ OPTION_INTEGER, 'n', NULL, &lines, N_(\"n\"),\n+\t\t{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_(\"n\"),\n \t\t\t\tN_(\"print lines of each tag message\"),\n \t\t\t\tPARSE_OPT_OPTARG, NULL, 1 },\n \t\tOPT_CMDMODE('d', \"delete\", &cmdmode, N_(\"delete tags\"), 'd'),\n@@ -606,14 +611,14 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \n \t\tOPT_GROUP(N_(\"Tag listing options\")),\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\n-\t\tOPT_CONTAINS(&with_commit, N_(\"print only tags that contain the commit\")),\n-\t\tOPT_WITH(&with_commit, N_(\"print only tags that contain the commit\")),\n+\t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n+\t\tOPT_WITH(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n \t\t{\n \t\t\tOPTION_CALLBACK, 0, \"sort\", &tag_sort, N_(\"type\"), N_(\"sort tags\"),\n \t\t\tPARSE_OPT_NONEG, parse_opt_sort\n \t\t},\n \t\t{\n-\t\t\tOPTION_CALLBACK, 0, \"points-at\", &points_at, N_(\"object\"),\n+\t\t\tOPTION_CALLBACK, 0, \"points-at\", &filter.points_at, N_(\"object\"),\n \t\t\tN_(\"print only tags of the object\"), 0, parse_opt_object_name\n \t\t},\n \t\tOPT_END()\n@@ -622,6 +627,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tgit_config(git_tag_config, NULL);\n \n \tmemset(&opt, 0, sizeof(opt));\n+\tmemset(&filter, 0, sizeof(filter));\n+\tfilter.lines = -1;\n \n \targc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);\n \n@@ -638,7 +645,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(git_tag_usage, options);\n \n \tfinalize_colopts(&colopts, -1);\n-\tif (cmdmode == 'l' && lines != -1) {\n+\tif (cmdmode == 'l' && filter.lines != -1) {\n \t\tif (explicitly_enable_column(colopts))\n \t\t\tdie(_(\"--column and -n are incompatible\"));\n \t\tcolopts = 0;\n@@ -651,18 +658,19 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\tcopts.padding = 2;\n \t\t\trun_column_filter(colopts, &copts);\n \t\t}\n-\t\tif (lines != -1 && tag_sort)\n+\t\tif (filter.lines != -1 && tag_sort)\n \t\t\tdie(_(\"--sort and -n are incompatible\"));\n-\t\tret = list_tags(argv, lines == -1 ? 0 : lines, with_commit, tag_sort);\n+\t\tfilter.name_patterns = argv;\n+\t\tret = list_tags(&filter, tag_sort);\n \t\tif (column_active(colopts))\n \t\t\tstop_column_filter();\n \t\treturn ret;\n \t}\n-\tif (lines != -1)\n+\tif (filter.lines != -1)\n \t\tdie(_(\"-n option is only allowed with -l.\"));\n-\tif (with_commit)\n+\tif (filter.with_commit)\n \t\tdie(_(\"--contains option is only allowed with -l.\"));\n-\tif (points_at.nr)\n+\tif (filter.points_at.nr)\n \t\tdie(_(\"--points-at option is only allowed with -l.\"));\n \tif (cmdmode == 'd')\n \t\treturn for_each_tag_name(argv, delete_tag);\n-- \n2.5.1\n"},{"id":"269516","messageId":"1441479135-5285-13-git-send-email-Karthik.188@gmail.com","threadId":"40286","inReplyTo":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v16 12/14] tag.c: use 'ref-filter' APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-05T18:52:13Z","receivedAt":"2015-09-05T18:52:13Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nMake 'tag.c' use 'ref-filter' APIs for iterating through refs, sorting\nand printing of refs. This removes most of the code used in 'tag.c'\nreplacing it with calls to the 'ref-filter' library.\n\nMake 'tag.c' use the 'filter_refs()' function provided by 'ref-filter'\nto filter out tags based on the options set.\n\nFor printing tags we use 'show_ref_array_item()' function provided by\n'ref-filter'.\n\nWe improve the sorting option provided by 'tag.c' by using the sorting\noptions provided by 'ref-filter'. This causes the test 'invalid sort\nparameter on command line' in t7004 to fail, as 'ref-filter' throws an\nerror for all sorting fields which are incorrect. The test is changed\nto reflect the same.\n\nModify documentation for the same.\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n Documentation/git-tag.txt | 16 ++-\n builtin/tag.c | 344 ++++++----------------------------------------\n t/t7004-tag.sh | 8 +-\n 3 files changed, 52 insertions(+), 316 deletions(-)\n\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 84f6496..3ac4a96 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n \t [ | ]\n 'git tag' -d ...\n 'git tag' [-n[]] -l [--contains ] [--points-at ]\n-\t[--column[=] | --no-column] [--create-reflog] [...]\n+\t[--column[=] | --no-column] [--create-reflog] [--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 fe66f7b..27e3566 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 list.\n- * Do not recurse to find out, though, but return -1 if inconclusive.\n- */\n-static enum contains_result contains_test(struct commit *candidate,\n-\t\t\t const struct commit_list *want)\n-{\n-\t/* was it previously marked as containing a want commit? */\n-\tif (candidate->object.flags & TMP_MARK)\n-\t\treturn 1;\n-\t/* or marked as not possibly containing a want commit? */\n-\tif (candidate->object.flags & UNINTERESTING)\n-\t\treturn 0;\n-\t/* or are we it? */\n-\tif (in_commit_list(want, candidate)) {\n-\t\tcandidate->object.flags |= TMP_MARK;\n-\t\treturn 1;\n-\t}\n-\n-\tif (parse_commit(candidate) < 0)\n-\t\treturn 0;\n-\n-\treturn -1;\n-}\n-\n-/*\n- * Mimicking the real stack, this stack lives on the heap, avoiding stack\n- * overflows.\n- *\n- * At each recursion step, the stack items points to the commits whose\n- * ancestors are to be inspected.\n- */\n-struct stack {\n-\tint nr, alloc;\n-\tstruct stack_entry {\n-\t\tstruct commit *commit;\n-\t\tstruct commit_list *parents;\n-\t} *stack;\n-};\n-\n-static void push_to_stack(struct commit *candidate, struct stack *stack)\n-{\n-\tint index = stack->nr++;\n-\tALLOC_GROW(stack->stack, stack->nr, stack->alloc);\n-\tstack->stack[index].commit = candidate;\n-\tstack->stack[index].parents = candidate->parents;\n-}\n-\n-static enum contains_result contains(struct commit *candidate,\n-\t\tconst struct commit_list *want)\n-{\n-\tstruct stack stack = { 0, 0, NULL };\n-\tint result = contains_test(candidate, want);\n-\n-\tif (result != CONTAINS_UNKNOWN)\n-\t\treturn result;\n-\n-\tpush_to_stack(candidate, &stack);\n-\twhile (stack.nr) {\n-\t\tstruct stack_entry *entry = &stack.stack[stack.nr - 1];\n-\t\tstruct commit *commit = entry->commit;\n-\t\tstruct commit_list *parents = entry->parents;\n-\n-\t\tif (!parents) {\n-\t\t\tcommit->object.flags |= UNINTERESTING;\n-\t\t\tstack.nr--;\n-\t\t}\n-\t\t/*\n-\t\t * If we just popped the stack, parents->item has been marked,\n-\t\t * therefore contains_test will return a meaningful 0 or 1.\n-\t\t */\n-\t\telse switch (contains_test(parents->item, want)) {\n-\t\tcase CONTAINS_YES:\n-\t\t\tcommit->object.flags |= TMP_MARK;\n-\t\t\tstack.nr--;\n-\t\t\tbreak;\n-\t\tcase CONTAINS_NO:\n-\t\t\tentry->parents = parents->next;\n-\t\t\tbreak;\n-\t\tcase CONTAINS_UNKNOWN:\n-\t\t\tpush_to_stack(parents->item, &stack);\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-\tfree(stack.stack);\n-\treturn contains_test(candidate, want);\n-}\n-\n-/*\n- * Currently modified and used in ref-filter as append_lines(), will\n- * eventually be removed as we port tag.c to use ref-filter APIs.\n- */\n-static void show_tag_lines(const struct object_id *oid, int lines)\n-{\n-\tint i;\n-\tunsigned long size;\n-\tenum object_type type;\n-\tchar *buf, *sp, *eol;\n-\tsize_t len;\n-\n-\tbuf = read_sha1_file(oid->hash, &type, &size);\n-\tif (!buf)\n-\t\tdie_errno(\"unable to read object %s\", oid_to_hex(oid));\n-\tif (type != OBJ_COMMIT && type != OBJ_TAG)\n-\t\tgoto free_return;\n-\tif (!size)\n-\t\tdie(\"an empty %s object %s?\",\n-\t\t typename(type), oid_to_hex(oid));\n-\n-\t/* skip header */\n-\tsp = strstr(buf, \"\\n\\n\");\n-\tif (!sp)\n-\t\tgoto free_return;\n-\n-\t/* only take up to \"lines\" lines, and strip the signature from a tag */\n-\tif (type == OBJ_TAG)\n-\t\tsize = parse_signature(buf, size);\n-\tfor (i = 0, sp += 2; i < lines && sp < buf + size; i++) {\n-\t\tif (i)\n-\t\t\tprintf(\"\\n \");\n-\t\teol = memchr(sp, '\\n', size - (sp - buf));\n-\t\tlen = eol ? eol - sp : size - (sp - buf);\n-\t\tfwrite(sp, len, 1, stdout);\n-\t\tif (!eol)\n-\t\t\tbreak;\n-\t\tsp = eol + 1;\n-\t}\n-free_return:\n-\tfree(buf);\n-}\n-\n-static void ref_array_append(struct ref_array *array, const char *refname)\n-{\n-\tsize_t len = strlen(refname);\n-\tstruct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) + len + 1);\n-\tmemcpy(ref->refname, refname, len);\n-\tref->refname[len] = '\\0';\n-\tREALLOC_ARRAY(array->items, array->nr + 1);\n-\tarray->items[array->nr++] = ref;\n-}\n-\n-static int show_reference(const char *refname, const struct object_id *oid,\n-\t\t\t int flag, void *cb_data)\n-{\n-\tstruct ref_filter_cbdata *data = cb_data;\n-\tstruct ref_array *array = data->array;\n-\tstruct ref_filter *filter = data->filter;\n-\n-\tif (match_pattern(filter->name_patterns, refname)) {\n-\t\tif (filter->with_commit) {\n-\t\t\tstruct commit *commit;\n-\n-\t\t\tcommit = lookup_commit_reference_gently(oid->hash, 1);\n-\t\t\tif (!commit)\n-\t\t\t\treturn 0;\n-\t\t\tif (!contains(commit, filter->with_commit))\n-\t\t\t\treturn 0;\n-\t\t}\n-\n-\t\tif (filter->points_at.nr && !match_points_at(refname, oid->hash, &filter->points_at))\n-\t\t\treturn 0;\n-\n-\t\tif (!filter->lines) {\n-\t\t\tif (tag_sort)\n-\t\t\t\tref_array_append(array, refname);\n-\t\t\telse\n-\t\t\t\tprintf(\"%s\\n\", refname);\n-\t\t\treturn 0;\n-\t\t}\n-\t\tprintf(\"%-15s \", refname);\n-\t\tshow_tag_lines(oid, filter->lines);\n-\t\tputchar('\\n');\n-\t}\n-\n-\treturn 0;\n-}\n-\n-static int sort_by_version(const void *a_, const void *b_)\n-{\n-\tconst struct ref_array_item *a = *((struct ref_array_item **)a_);\n-\tconst struct ref_array_item *b = *((struct ref_array_item **)b_);\n-\treturn versioncmp(a->refname, b->refname);\n-}\n-\n-static int list_tags(struct ref_filter *filter, int sort)\n+static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting)\n {\n \tstruct ref_array array;\n-\tstruct ref_filter_cbdata data;\n+\tchar *format, *to_free = NULL;\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 = to_free = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) \"\n+\t\t\t\t\t \"%%(contents:lines=%d)\", filter->lines);\n+\telse\n+\t\tformat = \"%(refname:short)\";\n+\n+\tverify_ref_format(format);\n+\tfilter_refs(&array, filter, FILTER_REFS_TAGS);\n+\tref_array_sort(sorting, &array);\n+\n+\tfor (i = 0; i < array.nr; i++)\n+\t\tshow_ref_array_item(array.items[i], format, 0);\n+\tref_array_clear(&array);\n+\tfree(to_free);\n+\n \treturn 0;\n }\n \n@@ -366,35 +122,26 @@ static const char tag_template_nocleanup[] =\n \t\"Lines starting with '%c' will be kept; you may remove them\"\n \t\" yourself if you want to.\\n\");\n \n-/*\n- * Parse a sort string, and return 0 if parsed successfully. Will return\n- * non-zero when the sort string does not parse into a known type. If var is\n- * given, the error message becomes a warning and includes information about\n- * the configuration value.\n- */\n-static int parse_sort_string(const char *var, const char *arg, int *sort)\n+/* Parse arg given and add it the ref_sorting array */\n+static int parse_sorting_string(const char *arg, struct ref_sorting **sorting_tail)\n {\n-\tint type = 0, flags = 0;\n-\n-\tif (skip_prefix(arg, \"-\", &arg))\n-\t\tflags |= REVERSE_SORT;\n+\tstruct ref_sorting *s;\n+\tint len;\n \n-\tif (skip_prefix(arg, \"version:\", &arg) || skip_prefix(arg, \"v:\", &arg))\n-\t\ttype = VERCMP_SORT;\n-\telse\n-\t\ttype = STRCMP_SORT;\n+\ts = xcalloc(1, sizeof(*s));\n+\ts->next = *sorting_tail;\n+\t*sorting_tail = s;\n \n-\tif (strcmp(arg, \"refname\")) {\n-\t\tif (!var)\n-\t\t\treturn error(_(\"unsupported sort specification '%s'\"), arg);\n-\t\telse {\n-\t\t\twarning(_(\"unsupported sort specification '%s' in variable '%s'\"),\n-\t\t\t\tvar, arg);\n-\t\t\treturn -1;\n-\t\t}\n+\tif (*arg == '-') {\n+\t\ts->reverse = 1;\n+\t\targ++;\n \t}\n+\tif (skip_prefix(arg, \"version:\", &arg) ||\n+\t skip_prefix(arg, \"v:\", &arg))\n+\t\ts->version = 1;\n \n-\t*sort = (type | flags);\n+\tlen = strlen(arg);\n+\ts->atom = parse_ref_filter_atom(arg, arg+len);\n \n \treturn 0;\n }\n@@ -402,11 +149,12 @@ static int parse_sort_string(const char *var, const char *arg, int *sort)\n static int git_tag_config(const char *var, const char *value, void *cb)\n {\n \tint status;\n+\tstruct ref_sorting **sorting_tail = (struct ref_sorting **)cb;\n \n \tif (!strcmp(var, \"tag.sort\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n-\t\tparse_sort_string(var, value, &tag_sort);\n+\t\tparse_sorting_string(value, sorting_tail);\n \t\treturn 0;\n \t}\n \n@@ -564,13 +312,6 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)\n \treturn check_refname_format(sb->buf, 0);\n }\n \n-static int parse_opt_sort(const struct option *opt, const char *arg, int unset)\n-{\n-\tint *sort = opt->value;\n-\n-\treturn parse_sort_string(NULL, arg, sort);\n-}\n-\n int cmd_tag(int argc, const char **argv, const char *prefix)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -587,6 +328,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tstruct ref_transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n \tstruct ref_filter filter;\n+\tstatic struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE('l', \"list\", &cmdmode, N_(\"list tag names\"), 'l'),\n \t\t{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_(\"n\"),\n@@ -613,10 +355,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\n \t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n \t\tOPT_WITH(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n-\t\t{\n-\t\t\tOPTION_CALLBACK, 0, \"sort\", &tag_sort, N_(\"type\"), N_(\"sort tags\"),\n-\t\t\tPARSE_OPT_NONEG, parse_opt_sort\n-\t\t},\n+\t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n+\t\t\t N_(\"field name to sort on\"), &parse_opt_ref_sorting),\n \t\t{\n \t\t\tOPTION_CALLBACK, 0, \"points-at\", &filter.points_at, N_(\"object\"),\n \t\t\tN_(\"print only tags of the object\"), 0, parse_opt_object_name\n@@ -624,7 +364,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \n-\tgit_config(git_tag_config, NULL);\n+\tgit_config(git_tag_config, sorting_tail);\n \n \tmemset(&opt, 0, sizeof(opt));\n \tmemset(&filter, 0, sizeof(filter));\n@@ -650,6 +390,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"--column and -n are incompatible\"));\n \t\tcolopts = 0;\n \t}\n+\tif (!sorting)\n+\t\tsorting = ref_default_sorting();\n \tif (cmdmode == 'l') {\n \t\tint ret;\n \t\tif (column_active(colopts)) {\n@@ -658,10 +400,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\tcopts.padding = 2;\n \t\t\trun_column_filter(colopts, &copts);\n \t\t}\n-\t\tif (filter.lines != -1 && tag_sort)\n-\t\t\tdie(_(\"--sort and -n are incompatible\"));\n \t\tfilter.name_patterns = argv;\n-\t\tret = list_tags(&filter, tag_sort);\n+\t\tret = list_tags(&filter, sorting);\n \t\tif (column_active(colopts))\n \t\t\tstop_column_filter();\n \t\treturn ret;\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex d31788c..84153ef 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1462,13 +1462,7 @@ test_expect_success 'invalid sort parameter on command line' '\n \n test_expect_success 'invalid sort parameter in configuratoin' '\n \tgit config tag.sort \"v:notvalid\" &&\n-\tgit tag -l \"foo*\" >actual &&\n-\tcat >expect <<-\\EOF &&\n-\tfoo1.10\n-\tfoo1.3\n-\tfoo1.6\n-\tEOF\n-\ttest_cmp expect actual\n+\ttest_must_fail git tag -l \"foo*\"\n '\n \n test_expect_success 'version sort with prerelease reordering' '\n-- \n2.5.1\n"},{"id":"269515","messageId":"1441479135-5285-14-git-send-email-Karthik.188@gmail.com","threadId":"40286","inReplyTo":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v16 13/14] tag.c: implement '--format' option","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-05T18:52:14Z","receivedAt":"2015-09-05T18:52:14Z","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 | 8 +++++++-\n builtin/tag.c | 22 +++++++++++++---------\n t/t7004-tag.sh | 12 ++++++++++++\n 3 files changed, 32 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 3ac4a96..0c7f4e6 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] [--create-reflog] [--sort=] [...]\n+\t[--column[=] | --no-column] [--create-reflog] [--sort=]\n+\t[--format=] [...]\n 'git tag' -v ...\n \n DESCRIPTION\n@@ -158,6 +159,11 @@ This option is only applicable when listing tags without annotation lines.\n \tThe object that the new tag will refer to, usually a commit.\n \tDefaults to HEAD.\n \n+::\n+\tA string that interpolates `%(fieldname)` from the object\n+\tpointed at by a ref being shown. The format is the same as\n+\tthat of linkgit:git-for-each-ref[1]. When unspecified,\n+\tdefaults to `%(refname:short)`.\n \n CONFIGURATION\n -------------\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 27e3566..f9c56ac 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -23,17 +23,17 @@ static const char * const git_tag_usage[] = {\n \tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ] []\"),\n \tN_(\"git tag -d ...\"),\n \tN_(\"git tag -l [-n[]] [--contains ] [--points-at ]\"\n-\t\t\"\\n\\t\\t[...]\"),\n+\t\t\"\\n\\t\\t[--format=] [...]\"),\n \tN_(\"git tag -v ...\"),\n \tNULL\n };\n \n static unsigned int colopts;\n \n-static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting)\n+static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)\n {\n \tstruct ref_array array;\n-\tchar *format, *to_free = NULL;\n+\tchar *to_free = NULL;\n \tint i;\n \n \tmemset(&array, 0, sizeof(array));\n@@ -41,11 +41,13 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting)\n \tif (filter->lines == -1)\n \t\tfilter->lines = 0;\n \n-\tif (filter->lines)\n-\t\tformat = to_free = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) \"\n-\t\t\t\t\t \"%%(contents:lines=%d)\", filter->lines);\n-\telse\n-\t\tformat = \"%(refname:short)\";\n+\tif (!format) {\n+\t\tif (filter->lines)\n+\t\t\tformat = to_free = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) \"\n+\t\t\t\t\t\t \"%%(contents:lines=%d)\", filter->lines);\n+\t\telse\n+\t\t\tformat = \"%(refname:short)\";\n+\t}\n \n \tverify_ref_format(format);\n \tfilter_refs(&array, filter, FILTER_REFS_TAGS);\n@@ -329,6 +331,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tstruct strbuf err = STRBUF_INIT;\n \tstruct ref_filter filter;\n \tstatic struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;\n+\tconst char *format = NULL;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE('l', \"list\", &cmdmode, N_(\"list tag names\"), 'l'),\n \t\t{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_(\"n\"),\n@@ -361,6 +364,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\tOPTION_CALLBACK, 0, \"points-at\", &filter.points_at, N_(\"object\"),\n \t\t\tN_(\"print only tags of the object\"), 0, parse_opt_object_name\n \t\t},\n+\t\tOPT_STRING( 0 , \"format\", &format, N_(\"format\"), N_(\"format to use for the output\")),\n \t\tOPT_END()\n \t};\n \n@@ -401,7 +405,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\trun_column_filter(colopts, &copts);\n \t\t}\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 84153ef..8987fb1 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1519,4 +1519,16 @@ EOF\"\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--format should list tags as per format given' '\n+\tcat >expect <<-\\EOF &&\n+\trefname : refs/tags/foo1.10\n+\trefname : refs/tags/foo1.3\n+\trefname : refs/tags/foo1.6\n+\trefname : refs/tags/foo1.6-rc1\n+\trefname : refs/tags/foo1.6-rc2\n+\tEOF\n+\tgit tag -l --format=\"refname : %(refname)\" \"foo*\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.5.1\n"},{"id":"269517","messageId":"1441479135-5285-15-git-send-email-Karthik.188@gmail.com","threadId":"40286","inReplyTo":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v16 14/14] tag.c: implement '--merged' and '--no-merged' options","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-05T18:52:15Z","receivedAt":"2015-09-05T18:52:15Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Use 'ref-filter' APIs to implement the '--merged' and '--no-merged'\noptions into 'tag.c'. The '--merged' option lets the user to only list\ntags merged into the named commit. The '--no-merged' option lets the\nuser to only list tags not merged into the named commit. If no object\nis provided it assumes HEAD as the object.\n\nAdd documentation and tests for the same.\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n Documentation/git-tag.txt | 7 ++++++-\n builtin/tag.c | 6 +++++-\n t/t7004-tag.sh | 27 +++++++++++++++++++++++++++\n 3 files changed, 38 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex 0c7f4e6..3803bf7 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] [--create-reflog] [--sort=]\n-\t[--format=] [...]\n+\t[--format=] [--[no-]merged []] [...]\n 'git tag' -v ...\n \n DESCRIPTION\n@@ -165,6 +165,11 @@ This option is only applicable when listing tags without annotation lines.\n \tthat of linkgit:git-for-each-ref[1]. When unspecified,\n \tdefaults to `%(refname:short)`.\n \n+--[no-]merged []::\n+\tOnly list tags whose tips are reachable, or not reachable\n+\tif '--no-merged' is used, from the specified commit ('HEAD'\n+\tif not specified).\n+\n CONFIGURATION\n -------------\n By default, 'git tag' in sign-with-default mode (-s) will use your\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex f9c56ac..f55dfda 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -23,7 +23,7 @@ static const char * const git_tag_usage[] = {\n \tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ] []\"),\n \tN_(\"git tag -d ...\"),\n \tN_(\"git tag -l [-n[]] [--contains ] [--points-at ]\"\n-\t\t\"\\n\\t\\t[--format=] [...]\"),\n+\t\t\"\\n\\t\\t[--format=] [--[no-]merged []] [...]\"),\n \tN_(\"git tag -v ...\"),\n \tNULL\n };\n@@ -358,6 +358,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\n \t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n \t\tOPT_WITH(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n+\t\tOPT_MERGED(&filter, N_(\"print only tags that are merged\")),\n+\t\tOPT_NO_MERGED(&filter, N_(\"print only tags that are not merged\")),\n \t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n \t\t\t N_(\"field name to sort on\"), &parse_opt_ref_sorting),\n \t\t{\n@@ -416,6 +418,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"--contains option is only allowed with -l.\"));\n \tif (filter.points_at.nr)\n \t\tdie(_(\"--points-at option is only allowed with -l.\"));\n+\tif (filter.merge_commit)\n+\t\tdie(_(\"--merged and --no-merged option are only allowed with -l\"));\n \tif (cmdmode == 'd')\n \t\treturn for_each_tag_name(argv, delete_tag);\n \tif (cmdmode == 'v')\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 8987fb1..3dd2f51 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1531,4 +1531,31 @@ test_expect_success '--format should list tags as per format given' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'setup --merged test tags' '\n+\tgit tag mergetest-1 HEAD~2 &&\n+\tgit tag mergetest-2 HEAD~1 &&\n+\tgit tag mergetest-3 HEAD\n+'\n+\n+test_expect_success '--merged cannot be used in non-list mode' '\n+\ttest_must_fail git tag --merged=mergetest-2 foo\n+'\n+\n+test_expect_success '--merged shows merged tags' '\n+\tcat >expect <<-\\EOF &&\n+\tmergetest-1\n+\tmergetest-2\n+\tEOF\n+\tgit tag -l --merged=mergetest-2 mergetest-* >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--no-merged show unmerged tags' '\n+\tcat >expect <<-\\EOF &&\n+\tmergetest-3\n+\tEOF\n+\tgit tag -l --no-merged=mergetest-2 mergetest-* >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.5.1\n"},{"id":"269542","messageId":"vpqa8sz9ydh.fsf@anie.imag.fr","threadId":"40286","inReplyTo":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-09-06T18:49:30Z","receivedAt":"2015-09-06T18:49:30Z","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> @@ -705,9 +719,12 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n> \t\t\tv->s = xstrdup(subpos);\n> \t\telse if (skip_prefix(name, \"contents:lines=\", &valp)) {\n> \t\t\tstruct strbuf s = STRBUF_INIT;\n> +\t\t\tconst char *contents_end = bodylen + bodypos - siglen;\n> +\n> \t\t\tif (strtoul_ui(valp, 10, &v->u.contents.lines))\n> -\t\t\t\tdie(_(\"positive width expected contents:lines=%s\"), valp);\n> -\t\t\tappend_lines(&s, subpos, sublen + bodylen - siglen, v->u.contents.lines);\n> +\t\t\t\tdie(_(\"positive value expected contents:lines=%s\"), valp);\n> +\t\t\t/* Size is the length of the message after removing the signature */\n\nNit: double-space after /*.\n\n> -test_expect_success 'alignment with format quote' '\n> -\tcat >expect <<-EOF &&\n> -\trefname is ${sq} ${sq}\\\\${sq}${sq}master${sq}\\\\${sq}${sq} ${sq}|\n> -\trefname is ${sq} ${sq}\\\\${sq}${sq}side${sq}\\\\${sq}${sq} ${sq}|\n> -\trefname is ${sq} ${sq}\\\\${sq}${sq}odd/spot${sq}\\\\${sq}${sq} ${sq}|\n> -\trefname is ${sq} ${sq}\\\\${sq}${sq}double-tag${sq}\\\\${sq}${sq} ${sq}|\n> -\trefname is ${sq} ${sq}\\\\${sq}${sq}four${sq}\\\\${sq}${sq} ${sq}|\n> -\trefname is ${sq} ${sq}\\\\${sq}${sq}one${sq}\\\\${sq}${sq} ${sq}|\n> -\trefname is ${sq} ${sq}\\\\${sq}${sq}signed-tag${sq}\\\\${sq}${sq} ${sq}|\n> -\trefname is ${sq} ${sq}\\\\${sq}${sq}three${sq}\\\\${sq}${sq} ${sq}|\n> -\trefname is ${sq} ${sq}\\\\${sq}${sq}two${sq}\\\\${sq}${sq} ${sq}|\n> +test_expect_success 'alignment with format quote' \"\n> +\tcat >expect <<-\\EOF &&\n> +\t|' master| A U Thor '|\n> +\t|' side| A U Thor '|\n> +\t|' odd/spot| A U Thor '|\n> +\t|' double-tag| '|\n> +\t|' four| A U Thor '|\n> +\t|' one| A U Thor '|\n> +\t|' signed-tag| '|\n> +\t|' three| A U Thor '|\n> +\t|' two| A U Thor '|\n> \tEOF\n> -\tgit for-each-ref --shell --format=\"refname is %(align:30,middle)${sq}%(refname:short)${sq}%(end)|\" >actual &&\n> +\tgit for-each-ref --shell --format='|%(align:30,middle)%(refname:short)| %(authorname)%(end)|' >actual &&\n> \ttest_cmp expect actual\n> -'\n> +\"\n\nThe new test is indeed easier to read, but you're not testing anymore\nthe consequence of a single-quote within the aligned string.\n\nAn implementation that just adds single-quotes around the string without\nturning ' into '\\'' would be broken but still pass the test.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"269543","messageId":"CAOLa=ZSw8QSrxpHt+Egm29qTgFTsA2fvC-Cj+Favjn-3k=HEYg@mail.gmail.com","threadId":"40286","inReplyTo":"vpqa8sz9ydh.fsf@anie.imag.fr","subject":"Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-06T19:16:15Z","receivedAt":"2015-09-06T19:16:15Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Sep 7, 2015 at 12:19 AM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> @@ -705,9 +719,12 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj\n>> v->s = xstrdup(subpos);\n>> else if (skip_prefix(name, \"contents:lines=\", &valp)) {\n>> struct strbuf s = STRBUF_INIT;\n>> + const char *contents_end = bodylen + bodypos - siglen;\n>> +\n>> if (strtoul_ui(valp, 10, &v->u.contents.lines))\n>> - die(_(\"positive width expected contents:lines=%s\"), valp);\n>> - append_lines(&s, subpos, sublen + bodylen - siglen, v->u.contents.lines);\n>> + die(_(\"positive value expected contents:lines=%s\"), valp);\n>> + /* Size is the length of the message after removing the signature */\n>\n> Nit: double-space after /*.\n>\n>> -test_expect_success 'alignment with format quote' '\n>> - cat >expect <<-EOF &&\n>> - refname is ${sq} ${sq}\\\\${sq}${sq}master${sq}\\\\${sq}${sq} ${sq}|\n>> - refname is ${sq} ${sq}\\\\${sq}${sq}side${sq}\\\\${sq}${sq} ${sq}|\n>> - refname is ${sq} ${sq}\\\\${sq}${sq}odd/spot${sq}\\\\${sq}${sq} ${sq}|\n>> - refname is ${sq} ${sq}\\\\${sq}${sq}double-tag${sq}\\\\${sq}${sq} ${sq}|\n>> - refname is ${sq} ${sq}\\\\${sq}${sq}four${sq}\\\\${sq}${sq} ${sq}|\n>> - refname is ${sq} ${sq}\\\\${sq}${sq}one${sq}\\\\${sq}${sq} ${sq}|\n>> - refname is ${sq} ${sq}\\\\${sq}${sq}signed-tag${sq}\\\\${sq}${sq} ${sq}|\n>> - refname is ${sq} ${sq}\\\\${sq}${sq}three${sq}\\\\${sq}${sq} ${sq}|\n>> - refname is ${sq} ${sq}\\\\${sq}${sq}two${sq}\\\\${sq}${sq} ${sq}|\n>> +test_expect_success 'alignment with format quote' \"\n>> + cat >expect <<-\\EOF &&\n>> + |' master| A U Thor '|\n>> + |' side| A U Thor '|\n>> + |' odd/spot| A U Thor '|\n>> + |' double-tag| '|\n>> + |' four| A U Thor '|\n>> + |' one| A U Thor '|\n>> + |' signed-tag| '|\n>> + |' three| A U Thor '|\n>> + |' two| A U Thor '|\n>> EOF\n>> - git for-each-ref --shell --format=\"refname is %(align:30,middle)${sq}%(refname:short)${sq}%(end)|\" >actual &&\n>> + git for-each-ref --shell --format='|%(align:30,middle)%(refname:short)| %(authorname)%(end)|' >actual &&\n>> test_cmp expect actual\n>> -'\n>> +\"\n>\n> The new test is indeed easier to read, but you're not testing anymore\n> the consequence of a single-quote within the aligned string.\n>\n> An implementation that just adds single-quotes around the string without\n> turning ' into '\\'' would be broken but still pass the test.\n>\n\nYou're right, should squash this in\n\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex d0c0139..778c33b 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -137,17 +137,17 @@ test_expect_success 'right alignment' '\n\n test_expect_success 'alignment with format quote' \"\n cat >expect <<-\\EOF &&\n- |' master| A U Thor '|\n- |' side| A U Thor '|\n- |' odd/spot| A U Thor '|\n- |' double-tag| '|\n- |' four| A U Thor '|\n- |' one| A U Thor '|\n- |' signed-tag| '|\n- |' three| A U Thor '|\n- |' two| A U Thor '|\n+ |' '\\''master| A U Thor'\\'' '|\n+ |' '\\''side| A U Thor'\\'' '|\n+ |' '\\''odd/spot| A U Thor'\\'' '|\n+ |' '\\''double-tag| '\\'' '|\n+ |' '\\''four| A U Thor'\\'' '|\n+ |' '\\''one| A U Thor'\\'' '|\n+ |' '\\''signed-tag| '\\'' '|\n+ |' '\\''three| A U Thor'\\'' '|\n+ |' '\\''two| A U Thor'\\'' '|\n EOF\n- git for-each-ref --shell\n--format='|%(align:30,middle)%(refname:short)| %(authorname)%(end)|'\n>actual &&\n+ git for-each-ref --shell\n--format=\\\"|%(align:30,middle)'%(refname:short)|\n%(authorname)'%(end)|\\\" >actual &&\n test_cmp expect actual\n \"\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"269544","messageId":"CAPig+cSN1s6qAj7p_+qG9W52X2L-7eBR+-j+918vfycADvUyWw@mail.gmail.com","threadId":"40286","inReplyTo":"1441479135-5285-6-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v16 05/14] ref-filter: introduce match_atom_name()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-09-06T19:52:27Z","receivedAt":"2015-09-06T19:52:27Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Sep 5, 2015 at 2:52 PM, Karthik Nayak wrote:\n> Introduce match_atom_name() which helps in checking if a particular\n> atom is the atom we're looking for and if it has a value attached to\n> it or not.\n>\n> Use it instead of starts_with() for checking the value of %(color:...)\n> atom. Write a test for the same.\n>\n> Signed-off-by: Karthik Nayak \n> ---\n> diff --git a/ref-filter.c b/ref-filter.c\n> index a993216..e99c342 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> +static int match_atom_name(const char *name, const char *atom_name, const char **val)\n> +{\n> + const char *body;\n> +\n> + if (!skip_prefix(name, atom_name, &body))\n> + return 0; /* doesn't even begin with \"atom_name\" */\n> + if (!body[0] || !body[1]) {\n> + *val = NULL; /* %(atom_name) and no customization */\n> + return 1;\n\nIf this is invoked as match_atom_name(\"colors\", \"color\", ...), then it\nwill return true (matched, but no value), which is not correct at all;\n\"colors\" is not a match for atom %(color). Maybe you meant?\n\n if (!body[0] || (body[0] == ':' && !body[1])) {\n\nBut, that's getting ugly and complicated, and would be bettered\nhandled by reordering the logic of this function for dealing with the\nvarious valid and invalid cases. However...\n\n> + }\n> + if (body[0] != ':')\n> + return 0; /* \"atom_namefoo\" is not \"atom_name\" or \"atom_name:...\" */\n> + *val = body + 1; /* \"atomname:val\" */\n> + return 1;\n> +}\n\nIt's not clear why this function exists in the first place. It's only\npurpose seems to be to specially recognize instances of atoms which\nshould have a \":\" but lack it, so that the caller can then report an\nerror.\n\nBut why is this better than the more generic solution of merely\nreporting an error for *any* unrecognized atom? How does it help to\nsingle out these special cases?\n\nThere is a further inconsistency in that you are only specially\nrecognizing %(color) and %(align) lacking \":\", but not\n%(content:lines) lacking \"=\" in patch 8/14. Why do %(color:...) and\n%(align:...) get special treatment but %(content:lines=...) does not?\nSuch inconsistency again argues in favor of a generic \"unrecognized\natom\" detection and reporting over special case handling.\n\n> /*\n> * In a format string, find the next occurrence of %(atom).\n> */\n> @@ -721,10 +738,12 @@ static void populate_value(struct ref_array_item *ref)\n> refname = branch_get_push(branch, NULL);\n> if (!refname)\n> continue;\n> - } else if (starts_with(name, \"color:\")) {\n> + } else if (match_atom_name(name, \"color\", &valp)) {\n> char color[COLOR_MAXLEN] = \"\";\n>\n> - if (color_parse(name + 6, color) < 0)\n> + if (!valp)\n> + die(_(\"expected format: %%(color:)\"));\n> + if (color_parse(valp, color) < 0)\n> die(_(\"unable to parse format\"));\n> v->s = xstrdup(color);\n> continue;\n> diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\n> index 505a360..c4f0378 100755\n> --- a/t/t6302-for-each-ref-filter.sh\n> +++ b/t/t6302-for-each-ref-filter.sh\n> @@ -81,4 +81,8 @@ test_expect_success 'filtering with --contains' '\n> +test_expect_success '%(color) must fail' '\n> + test_must_fail git for-each-ref --format=\"%(color)%(refname)\"\n> +'\n> +\n> test_done\n> --\n> 2.5.1\n"},{"id":"269545","messageId":"CAPig+cS28v552BZ63PTKrchYc4M-GYMhhEyfcHXW89vqvYhcOQ@mail.gmail.com","threadId":"40286","inReplyTo":"1441479135-5285-7-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v16 06/14] ref-filter: implement an `align` atom","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-09-06T20:10:34Z","receivedAt":"2015-09-06T20:10:34Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Sep 5, 2015 at 2:52 PM, Karthik Nayak wrote:\n> Implement an `align` atom which left-, middle-, or right-aligns the\n> content between %(align:...) and %(end).\n>\n> It is followed by `:,`, where the `` is\n> either left, right or middle and `` is the size of the area\n> into which the content will be placed. If the content between\n> %(align:...) and %(end) is more than the width then no alignment is\n> performed. e.g. to align a refname atom to the middle with a total\n> width of 40 we can do: --format=\"%(align:middle,40)%(refname)%(end)\".\n>\n> We introduce an `at_end` function for each element of the stack which\n> is to be called when the `end` atom is encountered. Using this we\n> implement end_align_handler() for the `align` atom, this aligns the\n> final strbuf by calling `strbuf_utf8_align()` from utf8.c.\n>\n> Ensure that quote formatting is performed on the whole of\n> %(align:...)...%(end) rather than individual atoms inside. We skip\n> quote formatting for individual atoms when the current stack element\n> is handling an %(align:...) atom and perform quote formatting at the\n> end when we encounter the %(end) atom of the second element of then\n> stack.\n>\n> Add documentation and tests for the same.\n>\n> Signed-off-by: Karthik Nayak \n> ---\n> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> index e49d578..b23412d 100644\n> --- a/Documentation/git-for-each-ref.txt\n> +++ b/Documentation/git-for-each-ref.txt\n> @@ -127,6 +127,16 @@ color::\n> +align::\n> + Left-, middle-, or right-align the content between %(align:...)\n> + and %(end). Followed by `:,`, where the\n> + `` is either left, right or middle and `` is\n> + the total length of the content with alignment. If the\n\nThis should mention that is optional and default to \"left\"\nif omitted.\n\n> + contents length is more than the width then no alignment is\n> + performed. If used with '--quote' everything in between\n> + %(align:...) and %(end) is quoted, but if nested then only the\n> + topmost level performs quoting.\n> diff --git a/ref-filter.c b/ref-filter.c\n> index e99c342..6c9ef08 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -773,6 +837,50 @@ static void populate_value(struct ref_array_item *ref)\n> else\n> v->s = \" \";\n> continue;\n> + } else if (match_atom_name(name, \"align\", &valp)) {\n> + struct align *align = &v->u.align;\n> + struct strbuf **s;\n> +\n> + if (!valp)\n> + die(_(\"expected format: %%(align:, )\"));\n\nI'm pretty sure this parsing code won't deal well with a space after\nthe comma, so the space should be dropped from the diagnostic message\nadvising the user of the correct format: s/, /,/\n\n> + /*\n> + * TODO: Implement a function similar to strbuf_split_str()\n> + * which would strip the terminator at the end.\n\n\"...which would omit the separator from the end of each value.\"\n\n> + */\n> + s = strbuf_split_str(valp, ',', 0);\n> +\n> + /* If the position is given trim the ',' from the first strbuf */\n> + if (s[1])\n> + strbuf_setlen(s[0], s[0]->len - 1);\n> + if (s[2])\n> + die(_(\"align:, followed by garbage: %s\"), s[2]->buf);\n\nIf is omitted, strbuf_split_str(\"42\", ...) will return the\narray [\"42\", NULL] which won't have an element [2], which means this\ncode will access beyond end-of-array. (This is another good argument\nfor looping over s[] as Junio suggested rather than assuming these\nfixed yet optional positions. It can be hard to get it right.)\n\n> + if (strtoul_ui(s[0]->buf, 10, &align->width))\n> + die(_(\"positive width expected align:%s\"), s[0]->buf);\n> +\n> + /*\n> + * TODO: Implement a more general check, so that the values\n> + * do not always have to be in a specific order.\n> + */\n> + if (!s[1])\n> + align->position = ALIGN_LEFT;\n> + else if (!strcmp(s[1]->buf, \"left\"))\n> + align->position = ALIGN_LEFT;\n> + else if (!strcmp(s[1]->buf, \"right\"))\n> + align->position = ALIGN_RIGHT;\n> + else if (!strcmp(s[1]->buf, \"middle\"))\n> + align->position = ALIGN_MIDDLE;\n> + else\n> + die(_(\"improper format entered align:%s\"), s[1]->buf);\n> +\n> + strbuf_list_free(s);\n> +\n> + v->handler = align_atom_handler;\n> + continue;\n> + } else if (!strcmp(name, \"end\")) {\n> + v->handler = end_atom_handler;\n> + continue;\n> } else\n> continue;\n"},{"id":"269551","messageId":"xmqqh9n6agcf.fsf@gitster.mtv.corp.google.com","threadId":"40286","inReplyTo":"1441479135-5285-1-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-07T06:33:36Z","receivedAt":"2015-09-07T06:33:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> index d039f40..c5154bb 100644\n> --- a/Documentation/git-for-each-ref.txt\n> +++ b/Documentation/git-for-each-ref.txt\n> @@ -128,13 +128,14 @@ color::\n> \tare described in `color.branch.*`.\n> \n> align::\n> -\tLeft-, middle-, or right-align the content between %(align:..)\n> +\tLeft-, middle-, or right-align the content between %(align:...)\n> \tand %(end). Followed by `:,`, where the\n> \t`` is either left, right or middle and `` is\n> \tthe total length of the content with alignment. If the\n> \tcontents length is more than the width then no alignment is\n> \tperformed. If used with '--quote' everything in between\n> -\t%(align:..) and %(end) is quoted.\n> +\t%(align:...) and %(end) is quoted, but if nested then only the\n> +\ttopmost level performs quoting.\n\nI am not sure if the description of quote belongs to \"align\". Isn't\nthe general rule at the endgame when there are more opening things\nthat would buffer its effect up to the corresponding %(end):\n\n - Some atoms like %(align) and %(if) always require a matching\n %(end). We call them \"opening atoms\" and sometimes denote them\n as %($open).\n\n - When a scripting language specific quoting is in effect, except\n for opening atoms, replacement from every %(atom) is quoted when\n and only when it appears at the top-level (that is, when it\n appears outside %($open)...%(end)).\n\n - When a scripting language specific quoting is in effect,\n everything between a top-level opening atom and its matching\n %(end) is evaluated according to the semantics of the opening\n atom and its result is quoted.\n\nTo put the third item above in a different way, a matching\n%($open)... %(end) pair behaves as if it is a single normal atom,\nfrom the point of view of the quoting rule. All top-level atoms are\ninvidivually quoted, whether they are normal atoms or open/end pairs.\nAnd that rule is not limited to %(align).\n\nI am flexible with the terminology, but the point is that I think\nthe quoting rules are better be specified _outside_ the description\nof a particular atom, but as a general rule.\n\n> diff --git a/builtin/tag.c b/builtin/tag.c\n> index 9fa1400..f55dfda 100644\n> --- a/builtin/tag.c\n> +++ b/builtin/tag.c\n> @@ -43,8 +43,8 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con\n> \n> \tif (!format) {\n> \t\tif (filter->lines)\n> -\t\t\tformat = to_free = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)\",\n> -\t\t\t\t\t\t filter->lines);\n> +\t\t\tformat = to_free = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) \"\n> +\t\t\t\t\t\t \"%%(contents:lines=%d)\", filter->lines);\n\nThis line still looks overlong. Would it help to stop spelling this\nas a double \"a = b = overlong expression\" assignment?\n\n> \t/*\n> \t * Perform quote formatting when the stack element is that of\n> -\t * a modifier atom and right above the first stack element.\n> +\t * a supporting atom. If nested then perform quote formatting\n> +\t * only on the topmost supporting atom.\n> \t */\n> \tif (!state->stack->prev->prev) {\n> \t\tquote_formatting(&s, current->output.buf, state->quote_style);\n> @@ -261,6 +262,22 @@ static void end_atom_handler(struct atom_value *atomv, struct ref_formatting_sta\n> \tpop_stack_element(&state->stack);\n> }\n> \n> +static int match_atom_name(const char *name, const char *atom_name, const char **val)\n> +{\n> +\tconst char *body;\n> +\n> +\tif (!skip_prefix(name, atom_name, &body))\n> +\t\treturn 0; /* doesn't even begin with \"atom_name\" */\n> +\tif (!body[0] || !body[1]) {\n> +\t\t*val = NULL; /* %(atom_name) and no customization */\n> +\t\treturn 1;\n> +\t}\n\nThis if condition is puzzling. !body[0] would mean the name was\nexactly atom_name, which is what you want to catch.\n\nBut the other part does not make any sense to me. what does\n(body[0] != '\\0' && !body[1]) mean? atom_name asked for was \"align\"\nand name was \"aligna\"? That is not \"%(align) and no customization\".\n\n> +\tif (body[0] != ':')\n> +\t\treturn 0; /* \"atom_namefoo\" is not \"atom_name\" or \"atom_name:...\" */\n\nThis one makes sense to me.\n\n> +\t*val = body + 1; /* \"atomname:val\" */\n\nSpell that \"atom_name:val\" perhaps?\n"},{"id":"269552","messageId":"vpqfv2q91d3.fsf@anie.imag.fr","threadId":"40286","inReplyTo":"xmqqh9n6agcf.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-09-07T06:42:32Z","receivedAt":"2015-09-07T06:42:32Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> I am flexible with the terminology, but the point is that I think\n> the quoting rules are better be specified _outside_ the description\n> of a particular atom, but as a general rule.\n\nI agree, but for now we have only one %($open) ... %(end) pair, so it\nmakes sense to me to leave the series as-is and to move some of the\nexplanations in a general section in the next series (that will\nintroduce %(if) or another opening atom).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"269568","messageId":"CAOLa=ZQFZRKY_hHEVQQ6CuYC9XNWHJFYg1CFZCGUr-68phDMHQ@mail.gmail.com","threadId":"40286","inReplyTo":"xmqqh9n6agcf.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-07T13:56:01Z","receivedAt":"2015-09-07T13:56:01Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Sep 7, 2015 at 12:03 PM, Junio C Hamano wrote:\n> Karthik Nayak writes:\n>\n>> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n>> index d039f40..c5154bb 100644\n>> --- a/Documentation/git-for-each-ref.txt\n>> +++ b/Documentation/git-for-each-ref.txt\n>> @@ -128,13 +128,14 @@ color::\n>> are described in `color.branch.*`.\n>>\n>> align::\n>> - Left-, middle-, or right-align the content between %(align:..)\n>> + Left-, middle-, or right-align the content between %(align:...)\n>> and %(end). Followed by `:,`, where the\n>> `` is either left, right or middle and `` is\n>> the total length of the content with alignment. If the\n>> contents length is more than the width then no alignment is\n>> performed. If used with '--quote' everything in between\n>> - %(align:..) and %(end) is quoted.\n>> + %(align:...) and %(end) is quoted, but if nested then only the\n>> + topmost level performs quoting.\n>\n> I am not sure if the description of quote belongs to \"align\". Isn't\n> the general rule at the endgame when there are more opening things\n> that would buffer its effect up to the corresponding %(end):\n>\n> - Some atoms like %(align) and %(if) always require a matching\n> %(end). We call them \"opening atoms\" and sometimes denote them\n> as %($open).\n>\n> - When a scripting language specific quoting is in effect, except\n> for opening atoms, replacement from every %(atom) is quoted when\n> and only when it appears at the top-level (that is, when it\n> appears outside %($open)...%(end)).\n>\n> - When a scripting language specific quoting is in effect,\n> everything between a top-level opening atom and its matching\n> %(end) is evaluated according to the semantics of the opening\n> atom and its result is quoted.\n>\n> To put the third item above in a different way, a matching\n> %($open)... %(end) pair behaves as if it is a single normal atom,\n> from the point of view of the quoting rule. All top-level atoms are\n> invidivually quoted, whether they are normal atoms or open/end pairs.\n> And that rule is not limited to %(align).\n>\n> I am flexible with the terminology, but the point is that I think\n> the quoting rules are better be specified _outside_ the description\n> of a particular atom, but as a general rule.\n>\n\nI definitely agree, but like Matthieu said, corrently we have only\none such atom and it makes sense to note this behaviour under that.\nWhen we get %(if) to work we could move this over to a more general\nsection?\n\n>> diff --git a/builtin/tag.c b/builtin/tag.c\n>> index 9fa1400..f55dfda 100644\n>> --- a/builtin/tag.c\n>> +++ b/builtin/tag.c\n>> @@ -43,8 +43,8 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con\n>>\n>> if (!format) {\n>> if (filter->lines)\n>> - format = to_free = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)\",\n>> - filter->lines);\n>> + format = to_free = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) \"\n>> + \"%%(contents:lines=%d)\", filter->lines);\n>\n> This line still looks overlong. Would it help to stop spelling this\n> as a double \"a = b = overlong expression\" assignment?\n>\n\nI'm not sure, I get what you mean.\n\n>> /*\n>> * Perform quote formatting when the stack element is that of\n>> - * a modifier atom and right above the first stack element.\n>> + * a supporting atom. If nested then perform quote formatting\n>> + * only on the topmost supporting atom.\n>> */\n>> if (!state->stack->prev->prev) {\n>> quote_formatting(&s, current->output.buf, state->quote_style);\n>> @@ -261,6 +262,22 @@ static void end_atom_handler(struct atom_value *atomv, struct ref_formatting_sta\n>> pop_stack_element(&state->stack);\n>> }\n>>\n>> +static int match_atom_name(const char *name, const char *atom_name, const char **val)\n>> +{\n>> + const char *body;\n>> +\n>> + if (!skip_prefix(name, atom_name, &body))\n>> + return 0; /* doesn't even begin with \"atom_name\" */\n>> + if (!body[0] || !body[1]) {\n>> + *val = NULL; /* %(atom_name) and no customization */\n>> + return 1;\n>> + }\n>\n> This if condition is puzzling. !body[0] would mean the name was\n> exactly atom_name, which is what you want to catch.\n>\n> But the other part does not make any sense to me. what does\n> (body[0] != '\\0' && !body[1]) mean? atom_name asked for was \"align\"\n> and name was \"aligna\"? That is not \"%(align) and no customization\".\n>\n\nThe idea was to ensure that the atom doesn't end at the ':'.\n\n>> + if (body[0] != ':')\n>> + return 0; /* \"atom_namefoo\" is not \"atom_name\" or \"atom_name:...\" */\n>\n> This one makes sense to me.\n>\n>> + *val = body + 1; /* \"atomname:val\" */\n>\n> Spell that \"atom_name:val\" perhaps?\n\nYeah will do.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"269569","messageId":"vpqfv2qqq7y.fsf@scolette.imag.fr","threadId":"40286","inReplyTo":"CAOLa=ZQFZRKY_hHEVQQ6CuYC9XNWHJFYg1CFZCGUr-68phDMHQ@mail.gmail.com","subject":"Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-09-07T14:05:53Z","receivedAt":"2015-09-07T14:05:53Z","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 Mon, Sep 7, 2015 at 12:03 PM, Junio C Hamano wrote:\n>>> diff --git a/builtin/tag.c b/builtin/tag.c\n>>> index 9fa1400..f55dfda 100644\n>>> --- a/builtin/tag.c\n>>> +++ b/builtin/tag.c\n>>> @@ -43,8 +43,8 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con\n>>>\n>>> if (!format) {\n>>> if (filter->lines)\n>>> - format = to_free = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)\",\n>>> - filter->lines);\n>>> + format = to_free = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) \"\n>>> + \"%%(contents:lines=%d)\", filter->lines);\n>>\n>> This line still looks overlong. Would it help to stop spelling this\n>> as a double \"a = b = overlong expression\" assignment?\n>>\n>\n> I'm not sure, I get what you mean.\n\nI guess\n\nformat = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)\",\n filter->lines);\nto_free = format;\n\n(still 83 columns + indentation, but that's a bit shorter than your\nversion).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"269577","messageId":"CAOLa=ZSscCk77f4viVPUhXRhpFXF=fa8c=G9mF0RovRqKqeLmg@mail.gmail.com","threadId":"40286","inReplyTo":"CAPig+cSN1s6qAj7p_+qG9W52X2L-7eBR+-j+918vfycADvUyWw@mail.gmail.com","subject":"Re: [PATCH v16 05/14] ref-filter: introduce match_atom_name()","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-07T18:07:35Z","receivedAt":"2015-09-07T18:07:35Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Sep 7, 2015 at 1:22 AM, Eric Sunshine wrote:\n> On Sat, Sep 5, 2015 at 2:52 PM, Karthik Nayak wrote:\n>> Introduce match_atom_name() which helps in checking if a particular\n>> atom is the atom we're looking for and if it has a value attached to\n>> it or not.\n>>\n>> Use it instead of starts_with() for checking the value of %(color:...)\n>> atom. Write a test for the same.\n>>\n>> Signed-off-by: Karthik Nayak \n>> ---\n>> diff --git a/ref-filter.c b/ref-filter.c\n>> index a993216..e99c342 100644\n>> --- a/ref-filter.c\n>> +++ b/ref-filter.c\n>> +static int match_atom_name(const char *name, const char *atom_name, const char **val)\n>> +{\n>> + const char *body;\n>> +\n>> + if (!skip_prefix(name, atom_name, &body))\n>> + return 0; /* doesn't even begin with \"atom_name\" */\n>> + if (!body[0] || !body[1]) {\n>> + *val = NULL; /* %(atom_name) and no customization */\n>> + return 1;\n>\n> If this is invoked as match_atom_name(\"colors\", \"color\", ...), then it\n> will return true (matched, but no value), which is not correct at all;\n> \"colors\" is not a match for atom %(color). Maybe you meant?\n>\n> if (!body[0] || (body[0] == ':' && !body[1])) {\n>\n> But, that's getting ugly and complicated, and would be bettered\n> handled by reordering the logic of this function for dealing with the\n> various valid and invalid cases. However...\n>\n\nWell as you infered the logic is to check so that there is something\nexisting after the ':'. About why I didn't do something like this\n\"(body[0] == ':' && !body[1])\" is because ref-filter already\nchecks for invalid atom names.\n\nSo before even match_atom_name() is called, the check is done in\nparse_ref_filter_atom().\n\n>> + }\n>> + if (body[0] != ':')\n>> + return 0; /* \"atom_namefoo\" is not \"atom_name\" or \"atom_name:...\" */\n>> + *val = body + 1; /* \"atomname:val\" */\n>> + return 1;\n>> +}\n>\n> It's not clear why this function exists in the first place. It's only\n> purpose seems to be to specially recognize instances of atoms which\n> should have a \":\" but lack it, so that the caller can then report an\n> error.\n>\n> But why is this better than the more generic solution of merely\n> reporting an error for *any* unrecognized atom? How does it help to\n> single out these special cases?\n>\n> There is a further inconsistency in that you are only specially\n> recognizing %(color) and %(align) lacking \":\", but not\n> %(content:lines) lacking \"=\" in patch 8/14. Why do %(color:...) and\n> %(align:...) get special treatment but %(content:lines=...) does not?\n> Such inconsistency again argues in favor of a generic \"unrecognized\n> atom\" detection and reporting over special case handling.\n>\n\nThis is from http://article.gmane.org/gmane.comp.version-control.git/277099\nJunio suggested to have this to check for \":\" rather than rely on\nskip_prefix() and check manually after that.\n\nAgreed, a more generic solution would be better, and If I remember I said,\nI'll work on that after this entire series.\n\nAbout contents:lines, we declare contents:lines itself as an atom, this was to\nkeep it similar to how the other contents atoms are declared, so the testing\nfor this again is already done by parse_ref_filter_atom().\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"269578","messageId":"CAOLa=ZS7zjedyq5bA4yhxats3BcdvcDn-uBDeKYbX0cirxd-Kw@mail.gmail.com","threadId":"40286","inReplyTo":"CAPig+cS28v552BZ63PTKrchYc4M-GYMhhEyfcHXW89vqvYhcOQ@mail.gmail.com","subject":"Re: [PATCH v16 06/14] ref-filter: implement an `align` atom","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-07T18:26:54Z","receivedAt":"2015-09-07T18:26:54Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Sep 7, 2015 at 1:40 AM, Eric Sunshine wrote:\n> On Sat, Sep 5, 2015 at 2:52 PM, Karthik Nayak wrote:\n>> Implement an `align` atom which left-, middle-, or right-aligns the\n>> content between %(align:...) and %(end).\n>>\n>> It is followed by `:,`, where the `` is\n>> either left, right or middle and `` is the size of the area\n>> into which the content will be placed. If the content between\n>> %(align:...) and %(end) is more than the width then no alignment is\n>> performed. e.g. to align a refname atom to the middle with a total\n>> width of 40 we can do: --format=\"%(align:middle,40)%(refname)%(end)\".\n>>\n>> We introduce an `at_end` function for each element of the stack which\n>> is to be called when the `end` atom is encountered. Using this we\n>> implement end_align_handler() for the `align` atom, this aligns the\n>> final strbuf by calling `strbuf_utf8_align()` from utf8.c.\n>>\n>> Ensure that quote formatting is performed on the whole of\n>> %(align:...)...%(end) rather than individual atoms inside. We skip\n>> quote formatting for individual atoms when the current stack element\n>> is handling an %(align:...) atom and perform quote formatting at the\n>> end when we encounter the %(end) atom of the second element of then\n>> stack.\n>>\n>> Add documentation and tests for the same.\n>>\n>> Signed-off-by: Karthik Nayak \n>> ---\n>> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n>> index e49d578..b23412d 100644\n>> --- a/Documentation/git-for-each-ref.txt\n>> +++ b/Documentation/git-for-each-ref.txt\n>> @@ -127,6 +127,16 @@ color::\n>> +align::\n>> + Left-, middle-, or right-align the content between %(align:...)\n>> + and %(end). Followed by `:,`, where the\n>> + `` is either left, right or middle and `` is\n>> + the total length of the content with alignment. If the\n>\n> This should mention that is optional and default to \"left\"\n> if omitted.\n>\n\nWill add that.\n\n>> + contents length is more than the width then no alignment is\n>> + performed. If used with '--quote' everything in between\n>> + %(align:...) and %(end) is quoted, but if nested then only the\n>> + topmost level performs quoting.\n>> diff --git a/ref-filter.c b/ref-filter.c\n>> index e99c342..6c9ef08 100644\n>> --- a/ref-filter.c\n>> +++ b/ref-filter.c\n>> @@ -773,6 +837,50 @@ static void populate_value(struct ref_array_item *ref)\n>> else\n>> v->s = \" \";\n>> continue;\n>> + } else if (match_atom_name(name, \"align\", &valp)) {\n>> + struct align *align = &v->u.align;\n>> + struct strbuf **s;\n>> +\n>> + if (!valp)\n>> + die(_(\"expected format: %%(align:, )\"));\n>\n> I'm pretty sure this parsing code won't deal well with a space after\n> the comma, so the space should be dropped from the diagnostic message\n> advising the user of the correct format: s/, /,/\n>\n\nWill change.\n\n>> + /*\n>> + * TODO: Implement a function similar to strbuf_split_str()\n>> + * which would strip the terminator at the end.\n>\n> \"...which would omit the separator from the end of each value.\"\n>\n\nWill change.\n\n>> + */\n>> + s = strbuf_split_str(valp, ',', 0);\n>> +\n>> + /* If the position is given trim the ',' from the first strbuf */\n>> + if (s[1])\n>> + strbuf_setlen(s[0], s[0]->len - 1);\n>> + if (s[2])\n>> + die(_(\"align:, followed by garbage: %s\"), s[2]->buf);\n>\n> If is omitted, strbuf_split_str(\"42\", ...) will return the\n> array [\"42\", NULL] which won't have an element [2], which means this\n> code will access beyond end-of-array. (This is another good argument\n> for looping over s[] as Junio suggested rather than assuming these\n> fixed yet optional positions. It can be hard to get it right.)\n\nYou're right, probably something on the lines of what Junio suggested here\nhttp://article.gmane.org/gmane.comp.version-control.git/277041\n\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"269592","messageId":"CAOLa=ZTwV3O9LVKqU3zXaqt=HoF1HxAGoMW2PLuj_y9iydRUwQ@mail.gmail.com","threadId":"40286","inReplyTo":"vpqfv2qqq7y.fsf@scolette.imag.fr","subject":"Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-08T05:43:51Z","receivedAt":"2015-09-08T05:43:51Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Sep 7, 2015 at 7:35 PM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> On Mon, Sep 7, 2015 at 12:03 PM, Junio C Hamano wrote:\n>>>> diff --git a/builtin/tag.c b/builtin/tag.c\n>>>> index 9fa1400..f55dfda 100644\n>>>> --- a/builtin/tag.c\n>>>> +++ b/builtin/tag.c\n>>>> @@ -43,8 +43,8 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con\n>>>>\n>>>> if (!format) {\n>>>> if (filter->lines)\n>>>> - format = to_free = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)\",\n>>>> - filter->lines);\n>>>> + format = to_free = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) \"\n>>>> + \"%%(contents:lines=%d)\", filter->lines);\n>>>\n>>> This line still looks overlong. Would it help to stop spelling this\n>>> as a double \"a = b = overlong expression\" assignment?\n>>>\n>>\n>> I'm not sure, I get what you mean.\n>\n> I guess\n>\n> format = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)\",\n> filter->lines);\n> to_free = format;\n>\n> (still 83 columns + indentation, but that's a bit shorter than your\n> version).\n\nAlso we could drop left, its default anyways.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"269618","messageId":"xmqqd1xs93iu.fsf@gitster.mtv.corp.google.com","threadId":"40286","inReplyTo":"vpqfv2qqq7y.fsf@scolette.imag.fr","subject":"Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-08T18:20:25Z","receivedAt":"2015-09-08T18:20:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n>>> This line still looks overlong. Would it help to stop spelling this\n>>> as a double \"a = b = overlong expression\" assignment?\n>>>\n>>\n>> I'm not sure, I get what you mean.\n>\n> I guess\n>\n> format = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)\",\n> filter->lines);\n> to_free = format;\n>\n> (still 83 columns + indentation, but that's a bit shorter than your\n> version).\n\nSure. This may also be possible\n\n\txstrfmt(\"%s %%(contents:lines=%d)\",\n \"%(align:15,left)%(refname:short)%(end)\", filter->lines);\n\nand highlights that the only thing that is variable is the number\nof lines, which might be better.\n"},{"id":"269617","messageId":"xmqq8u8g93i0.fsf@gitster.mtv.corp.google.com","threadId":"40286","inReplyTo":"CAOLa=ZQFZRKY_hHEVQQ6CuYC9XNWHJFYg1CFZCGUr-68phDMHQ@mail.gmail.com","subject":"Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-08T18:20:55Z","receivedAt":"2015-09-08T18:20:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n>> I am flexible with the terminology, but the point is that I think\n>> the quoting rules are better be specified _outside_ the description\n>> of a particular atom, but as a general rule.\n>\n> I definitely agree, but like Matthieu said, corrently we have only\n> one such atom and it makes sense to note this behaviour under that.\n> When we get %(if) to work we could move this over to a more general\n> section?\n\nSure.\n"},{"id":"269663","messageId":"CAOLa=ZS5bM_h7CLJD+Nj1h1qi3FCfjVYX0ZBGCX6PDXt+9XQyw@mail.gmail.com","threadId":"40286","inReplyTo":"xmqqd1xs93iu.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-09-09T17:37:03Z","receivedAt":"2015-09-09T17:37:03Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Tue, Sep 8, 2015 at 11:50 PM, Junio C Hamano wrote:\n> Matthieu Moy writes:\n>\n>>>> This line still looks overlong. Would it help to stop spelling this\n>>>> as a double \"a = b = overlong expression\" assignment?\n>>>>\n>>>\n>>> I'm not sure, I get what you mean.\n>>\n>> I guess\n>>\n>> format = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)\",\n>> filter->lines);\n>> to_free = format;\n>>\n>> (still 83 columns + indentation, but that's a bit shorter than your\n>> version).\n>\n> Sure. This may also be possible\n>\n> xstrfmt(\"%s %%(contents:lines=%d)\",\n> \"%(align:15,left)%(refname:short)%(end)\", filter->lines);\n>\n> and highlights that the only thing that is variable is the number\n> of lines, which might be better.\n\nYeah looks better, thanks.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"269665","messageId":"CAPc5daWiACH6fVDyszdRBAs0aHg1wUi7wjKfH7VKu=TVK6dBtg@mail.gmail.com","threadId":"40286","inReplyTo":"CAOLa=ZS5bM_h7CLJD+Nj1h1qi3FCfjVYX0ZBGCX6PDXt+9XQyw@mail.gmail.com","subject":"Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-09T19:18:50Z","receivedAt":"2015-09-09T19:18:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ehh, I was only kidding, though...\n\nOn Wed, Sep 9, 2015 at 10:37 AM, Karthik Nayak wrote:\n> On Tue, Sep 8, 2015 at 11:50 PM, Junio C Hamano wrote:\n>> Matthieu Moy writes:\n>>\n>>>>> This line still looks overlong. Would it help to stop spelling this\n>>>>> as a double \"a = b = overlong expression\" assignment?\n>>>>>\n>>>>\n>>>> I'm not sure, I get what you mean.\n>>>\n>>> I guess\n>>>\n>>> format = xstrfmt(\"%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)\",\n>>> filter->lines);\n>>> to_free = format;\n>>>\n>>> (still 83 columns + indentation, but that's a bit shorter than your\n>>> version).\n>>\n>> Sure. This may also be possible\n>>\n>> xstrfmt(\"%s %%(contents:lines=%d)\",\n>> \"%(align:15,left)%(refname:short)%(end)\", filter->lines);\n>>\n>> and highlights that the only thing that is variable is the number\n>> of lines, which might be better.\n>\n> Yeah looks better, thanks.\n>\n> --\n> Regards,\n> Karthik Nayak\n"}]}