{"thread":{"id":"40517","subject":"[PATCH v2 01/10] ref-filter: implement %(if), %(then), and %(else) atoms","startedAt":"2015-10-08T09:17:55Z","lastAt":"2015-10-20T20:53:33Z","messageCount":66,"participants":["Karthik Nayak","Matthieu Moy","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":10},"messages":[{"id":"271304","messageId":"1444295885-1657-1-git-send-email-Karthik.188@gmail.com","threadId":"40517","inReplyTo":null,"subject":"[PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-08T09:17:55Z","receivedAt":"2015-10-08T09:17:55Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"This is part of unification of the commands 'git tag -l, git branch -l\nand git for-each-ref'. This ports over branch.c to use ref-filter's\nprinting options. The previous version of the patch can be found at:\nhttp://article.gmane.org/gmane.comp.version-control.git/278926\n\nKarthik Nayak (10):\n ref-filter: implement %(if), %(then), and %(else) atoms\n ref-filter: implement %(if:equals=) and\n %(if:notequals=)\n ref-filter: add support for %(refname:dir) and %(refname:base)\n ref-filter: modify \"%(objectname:short)\" to take length\n ref-filter: adopt get_head_description() from branch.c\n ref-filter: introduce format_ref_array_item()\n ref-filter: make %(upstream:track) prints \"[gone]\" for invalid\n upstreams\n ref-filter: add support for %(upstream:track,nobracket)\n branch: use ref-filter printing APIs\n branch: implement '--format' option\n\n Documentation/git-branch.txt | 7 +-\n Documentation/git-for-each-ref.txt | 63 +++++++-\n builtin/branch.c | 265 +++++++++-------------------------\n ref-filter.c | 285 +++++++++++++++++++++++++++++++++----\n ref-filter.h | 5 +\n t/t3203-branch-output.sh | 11 ++\n t/t6040-tracking-info.sh | 2 +-\n t/t6300-for-each-ref.sh | 33 ++++-\n t/t6302-for-each-ref-filter.sh | 132 +++++++++++++++++\n 9 files changed, 573 insertions(+), 230 deletions(-)\n\nChanges in this version:\n\n1. Use %(refname:dir) and %(refname:base) instead of %(path) and\n%(path:short).\n2. Make it %(objectname:short=) rather than %(objectname:short,).\n3. Add %(upstream:track,nobracket) which removes the brackets from tracking\ninformation.\n4. calc_maxwidth() now also considers head ref descriptions, this wasn't done\nin the last patch and would cause detached head description to be not aligned with the other ref-names.\n5. remove the if_atom field in if_then_else struct.\n6. introduce better error checking for the %(if), %(then), %(else) atoms\nas suggested by Matthieu (when used along with %(align) atom and so on).\n7. refactor code and improve documentation.\n\nInterdiff:\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 5097915..a38cbf6 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -92,7 +92,9 @@ refname::\n \tThe name of the ref (the part after $GIT_DIR/).\n \tFor a non-ambiguous short name of the ref append `:short`.\n \tThe option core.warnAmbiguousRefs is used to select the strict\n-\tabbreviation mode.\n+\tabbreviation mode. For the base directory of the ref (i.e. foo\n+\tin refs/foo/bar/boz) append `:base`. For the entire directory\n+\tpath append `:dir`.\n \n objecttype::\n \tThe type of the object (`blob`, `tree`, `commit`, `tag`).\n@@ -102,9 +104,9 @@ objectsize::\n \n objectname::\n \tThe object name (aka SHA-1).\n-\tFor a non-ambiguous abbreviation of the object name append `:short`.\n-\tThe length can be explicitly specified by appending either\n-\t`:short,` or `:,short`. Minimum length is 4.\n+\tFor a non-ambiguous abbreviation of the object name append\n+\t`:short`. The length can be explicitly specified by appending\n+\t`:short=`. Minimum length being 4.\n \n upstream::\n \tThe name of a local ref which can be considered ``upstream''\n@@ -112,8 +114,10 @@ upstream::\n \t`refname` above. Additionally respects `:track` to show\n \t\"[ahead N, behind M]\" and `:trackshort` to show the terse\n \tversion: \">\" (ahead), \"<\" (behind), \"<>\" (ahead and behind),\n-\tor \"=\" (in sync). Has no effect if the ref does not have\n-\ttracking information associated with it.\n+\tor \"=\" (in sync). Append `:track,nobracket` to show tracking\n+\tinformation without brackets (i.e \"ahead N, behind M\"). Has\n+\tno effect if the ref does not have tracking information\n+\tassociated with it.\n \n push::\n \tThe name of a local ref which represents the `@{push}` location\n@@ -143,14 +147,12 @@ if::\n \tIf there is an atom with value or string literal after the\n \t%(if) then everything after the %(then) is printed, else if\n \tthe %(else) atom is used, then everything after %(else) is\n-\tprinted. Append \":equals=\" or \":notequals=\" to\n-\tcompare the value between the %(if:...) and %(then) atoms with the\n-\tgiven string.\n-\n-path::\n-\tThe path of the given ref. For a shortened version listing\n-\tonly the name of the directory the ref is under append\n-\t`:short`.\n+\tprinted. We ignore space when evaluating the string before\n+\t%(then), this is useful when we use the %(HEAD) atom which\n+\tprints either \"*\" or \" \" and we want to apply the 'if'\n+\tcondition only on the 'HEAD' ref. Append \":equals=\" or\n+\t\":notequals=\" to compare the value between the\n+\t%(if:...) and %(then) atoms with the given string.\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@@ -191,8 +193,9 @@ opening atoms, replacement from every %(atom) is quoted when and only\n when it appears at the top-level (that is, when it appears outside\n %($open)...%(end)).\n \n-When a scripting language specific quoting is in effect, everything\n-between a top-level opening atom and its matching %(end) is evaluated\n+When a scripting language specific quoting is in effect (i.e. one of\n+`--shell`, `--perl`, `--python`, `--tcl` is used), everything between\n+a top-level opening atom and its matching %(end) is evaluated\n according to the semantics of the opening atom and its result is\n quoted.\n \n@@ -282,6 +285,26 @@ eval=`git for-each-ref --shell --format=\"$fmt\" \\\n eval \"$eval\"\n ------------\n \n+\n+An example to show the usage of %(if)...%(then)...%(else)...%(end).\n+This prefixes the current branch with a star.\n+\n+------------\n+#!/bin/sh\n+\n+git for-each-ref --format=\"%(if)%(HEAD)%(then)* %(else) %(end)%(refname:short)\" refs/heads/\n+------------\n+\n+\n+An example to show the usage of %(if)...%(then)...%(end).\n+This adds a red color to authorname, if present\n+\n+------------\n+#!/bin/sh\n+\n+git for-each-ref --format=\"%(refname)%(if)%(authorname)%(then)%(color:red)%(end) %(authorname)\"\n+------------\n+\n SEE ALSO\n --------\n linkgit:git-show-ref[1]\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex ae3ecfb..2356e72 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -282,7 +282,10 @@ static int calc_maxwidth(struct ref_array *refs, int remote_bonus)\n \n \t\tskip_prefix(it->refname, \"refs/heads/\", &desc);\n \t\tskip_prefix(it->refname, \"refs/remotes/\", &desc);\n-\t\tw = utf8_strwidth(desc);\n+\t\tif (it->kind == FILTER_REFS_DETACHED_HEAD)\n+\t\t\tw = strlen(get_head_description());\n+\t\telse\n+\t\t\tw = utf8_strwidth(desc);\n \n \t\tif (it->kind == FILTER_REFS_REMOTES)\n \t\t\tw += remote_bonus;\n@@ -292,44 +295,41 @@ static int calc_maxwidth(struct ref_array *refs, int remote_bonus)\n \treturn max;\n }\n \n-static char *get_format(struct ref_filter *filter, int maxwidth, const char *remote_prefix)\n+static char *build_format(struct ref_filter *filter, int maxwidth, const char *remote_prefix)\n {\n-\tchar *remote = NULL;\n-\tchar *local = NULL;\n-\tchar *final = NULL;\n+\tstruct strbuf fmt = STRBUF_INIT;\n+\tstruct strbuf local = STRBUF_INIT;\n+\tstruct strbuf remote = STRBUF_INIT;\n+\n+\tstrbuf_addf(&fmt, \"%%(if)%%(HEAD)%%(then)* %s%%(else) %%(end)\", branch_get_color(BRANCH_COLOR_CURRENT));\n \n \tif (filter->verbose) {\n-\t\tif (filter->verbose > 1)\n-\t\t\tlocal = xstrfmt(\"%%(if)%%(HEAD)%%(then)* %s%%(else) %%(end)%%(align:%d,left)%%(refname:short)%%(end)%s\"\n-\t\t\t\t\t\" %%(objectname:short,7) %%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s] %%(end)\"\n-\t\t\t\t\t\"%%(if)%%(upstream:track)%%(then)%%(upstream:track) %%(end)%%(contents:subject)\",\n-\t\t\t\t\tbranch_get_color(BRANCH_COLOR_CURRENT), maxwidth, branch_get_color(BRANCH_COLOR_RESET),\n-\t\t\t\t\tbranch_get_color(BRANCH_COLOR_UPSTREAM), branch_get_color(BRANCH_COLOR_RESET));\n+\t\tstrbuf_addf(&local, \"%%(align:%d,left)%%(refname:short)%%(end)\", maxwidth);\n+\t\tstrbuf_addf(&local, \"%s\", branch_get_color(BRANCH_COLOR_RESET));\n+\t\tstrbuf_addf(&local, \" %%(objectname:short=7) \");\n \n+\t\tif (filter->verbose > 1)\n+\t\t\tstrbuf_addf(&local, \"%%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s%%(if)%%(upstream:track)\"\n+\t\t\t\t \"%%(then): %%(upstream:track,nobracket)%%(end)] %%(end)%%(contents:subject)\",\n+\t\t\t\t branch_get_color(BRANCH_COLOR_UPSTREAM), branch_get_color(BRANCH_COLOR_RESET));\n \t\telse\n-\t\t\tlocal = xstrfmt(\"%%(if)%%(HEAD)%%(then)* %s%%(else) %%(end)%%(align:%d,left)%\"\n-\t\t\t\t\t\"%(refname:short)%%(end)%s %%(objectname:short,7) %%(if)%%(upstream:track)%\"\n-\t\t\t\t\t\"%(then)%%(upstream:track) %%(end)%%(contents:subject)\",\n-\t\t\t\t\tbranch_get_color(BRANCH_COLOR_CURRENT), maxwidth, branch_get_color(BRANCH_COLOR_RESET));\n-\n-\t\tremote = xstrfmt(\" %s%%(align:%d,left)%s%%(refname:short)%%(end)%s%%(if)%%(symref)%%(then) -> %%(symref:short)\"\n-\t\t\t\t \"%%(else) %%(objectname:short,7) %%(contents:subject)%%(end)\",\n-\t\t\t\t branch_get_color(BRANCH_COLOR_REMOTE), maxwidth,\n-\t\t\t\t remote_prefix, branch_get_color(BRANCH_COLOR_RESET));\n-\t\tfinal = xstrfmt(\"%%(if:notequals=remotes)%%(path:short)%%(then)%s%%(else)%s%%(end)\", local, remote);\n+\t\t\tstrbuf_addf(&local, \"%%(if)%%(upstream:track)%%(then)%%(upstream:track) %%(end)%%(contents:subject)\");\n \n+\t\tstrbuf_addf(&remote, \"%s%%(align:%d,left)%s%%(refname:short)%%(end)%s%%(if)%%(symref)%%(then) -> %%(symref:short)\"\n+\t\t\t \"%%(else) %%(objectname:short=7) %%(contents:subject)%%(end)\",\n+\t\t\t branch_get_color(BRANCH_COLOR_REMOTE), maxwidth,\n+\t\t\t remote_prefix, branch_get_color(BRANCH_COLOR_RESET));\n \t} else {\n-\t\tlocal = xstrfmt(\"%%(if)%%(HEAD)%%(then)* %s%%(else) %%(end)%%(refname:short)%s\",\n-\t\t\t\tbranch_get_color(BRANCH_COLOR_CURRENT), branch_get_color(BRANCH_COLOR_RESET));\n-\t\tremote = xstrfmt(\" %s%s%%(refname:short)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)\",\n-\t\t\t\t branch_get_color(BRANCH_COLOR_REMOTE), remote_prefix, branch_get_color(BRANCH_COLOR_RESET));\n-\t\tfinal = xstrfmt(\"%%(if:notequals=remotes)%%(path:short)%%(then)%s%%(else)%s%%(end)\", local, remote);\n+\t\tstrbuf_addf(&local, \"%%(refname:short)%s\", branch_get_color(BRANCH_COLOR_RESET));\n+\t\tstrbuf_addf(&remote, \"%s%s%%(refname:short)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)\",\n+\t\t\t branch_get_color(BRANCH_COLOR_REMOTE), remote_prefix, branch_get_color(BRANCH_COLOR_RESET));\n \t}\n \n-\tfree(local);\n-\tfree(remote);\n+\tstrbuf_addf(&fmt, \"%%(if:notequals=remotes)%%(refname:base)%%(then)%s%%(else)%s%%(end)\", local.buf, remote.buf);\n \n-\treturn final;\n+\tstrbuf_release(&local);\n+\tstrbuf_release(&remote);\n+\treturn strbuf_detach(&fmt, NULL);\n }\n \n static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)\n@@ -357,7 +357,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n \t\tmaxwidth = calc_maxwidth(&array, strlen(remote_prefix));\n \n \tif (!format)\n-\t\tformat = to_free = get_format(filter, maxwidth, remote_prefix);\n+\t\tformat = to_free = build_format(filter, maxwidth, remote_prefix);\n \tverify_ref_format(format);\n \n \t/*\n@@ -375,6 +375,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n \t\tformat_ref_array_item(array.items[i], format, 0, &out);\n \t\tif (column_active(colopts)) {\n \t\t\tassert(!filter->verbose && \"--column and --verbose are incompatible\");\n+\t\t\t /* format to a string_list to let print_columns() do its job */\n \t\t\tstring_list_append(&output, out.buf);\n \t\t} else {\n \t\t\tfwrite(out.buf, 1, out.len, stdout);\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 48b06e3..6044eb0 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -59,7 +59,6 @@ static struct {\n \t{ \"if\" },\n \t{ \"then\" },\n \t{ \"else\" },\n-\t{ \"path\" },\n };\n \n #define REF_FORMATTING_STATE_INIT { 0, NULL }\n@@ -77,8 +76,7 @@ struct contents {\n struct if_then_else {\n \tconst char *if_equals,\n \t\t*not_equals;\n-\tunsigned int if_atom : 1,\n-\t\tthen_atom : 1,\n+\tunsigned int then_atom : 1,\n \t\telse_atom : 1,\n \t\tcondition_satisfied : 1;\n };\n@@ -257,22 +255,27 @@ static void if_then_else_handler(struct ref_formatting_stack **stack)\n \tstruct ref_formatting_stack *prev = cur->prev;\n \tstruct if_then_else *if_then_else = (struct if_then_else *)cur->at_end_data;\n \n-\t/*\n-\t * If the 'if' condition is satisfied, then if there exists an\n-\t * 'else' atom drop the 'else' atom's state. Else we swap the\n-\t * buffer of the 'else' atom with the previous state and drop\n-\t * that state. If the condition is satisfied and no 'else' atom\n-\t * exists, then just clear the buffer.\n-\t */\n-\tif (if_then_else->condition_satisfied && if_then_else->else_atom) {\n-\t\tstrbuf_reset(&cur->output);\n-\t\tpop_stack_element(&cur);\n-\t} else if (if_then_else->else_atom) {\n-\t\tstrbuf_swap(&cur->output, &prev->output);\n-\t\tstrbuf_reset(&cur->output);\n-\t\tpop_stack_element(&cur);\n+\tif (if_then_else->else_atom) {\n+\t\t/*\n+\t\t * There is an %(else) atom: we need to drop one state from the\n+\t\t * stack, either the %(else) branch if the condition is satisfied, or\n+\t\t * the %(then) branch if it isn't.\n+\t\t */\n+\t\tif (if_then_else->condition_satisfied) {\n+\t\t\tstrbuf_reset(&cur->output);\n+\t\t\tpop_stack_element(&cur);\n+\t\t} else {\n+\t\t\tstrbuf_swap(&cur->output, &prev->output);\n+\t\t\tstrbuf_reset(&cur->output);\n+\t\t\tpop_stack_element(&cur);\n+\t\t}\n \t} else if (!if_then_else->condition_satisfied)\n+\t\t/*\n+\t\t * No %(else) atom: just drop the %(then) branch if the\n+\t\t * condition is not satisfied.\n+\t\t */\n \t\tstrbuf_reset(&cur->output);\n+\n \t*stack = cur;\n \tfree(if_then_else);\n }\n@@ -283,7 +286,6 @@ static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_stat\n \tstruct if_then_else *if_then_else = xcalloc(sizeof(struct if_then_else), 1);\n \tconst char *valp;\n \n-\tif_then_else->if_atom = 1;\n \tif (skip_prefix(atomv->s, \"equals=\", &valp))\n \t\tif_then_else->if_equals = valp;\n \telse if (skip_prefix(atomv->s, \"notequals=\", &valp))\n@@ -297,7 +299,7 @@ static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_stat\n \tnew->at_end_data = if_then_else;\n }\n \n-static int is_empty(const char * s){\n+static int is_empty(const char *s){\n \twhile (*s != '\\0') {\n \t\tif (!isspace(*s))\n \t\t\treturn 0;\n@@ -309,10 +311,14 @@ static int is_empty(const char * s){\n static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)\n {\n \tstruct ref_formatting_stack *cur = state->stack;\n-\tstruct if_then_else *if_then_else = (struct if_then_else *)cur->at_end_data;\n+\tstruct if_then_else *if_then_else = NULL;\n \n+\tif (cur->at_end == if_then_else_handler)\n+\t\tif_then_else = (struct if_then_else *)cur->at_end_data;\n \tif (!if_then_else)\n \t\tdie(_(\"format: %%(then) atom used without an %%(if) atom\"));\n+\tif (if_then_else->then_atom)\n+\t\tdie(_(\"format: %%(then) atom used more than once\"));\n \tif_then_else->then_atom = 1;\n \n \t/*\n@@ -334,12 +340,16 @@ static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_st\n static void else_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)\n {\n \tstruct ref_formatting_stack *prev = state->stack;\n-\tstruct if_then_else *if_then_else = (struct if_then_else *)state->stack->at_end_data;\n+\tstruct if_then_else *if_then_else = NULL;\n \n+\tif (prev->at_end == if_then_else_handler)\n+\t\tif_then_else = (struct if_then_else *)prev->at_end_data;\n \tif (!if_then_else)\n \t\tdie(_(\"format: %%(else) atom used without an %%(if) atom\"));\n \tif (!if_then_else->then_atom)\n \t\tdie(_(\"format: %%(else) atom used without a %%(then) atom\"));\n+\tif (if_then_else->else_atom)\n+\t\tdie(_(\"format: %%(else) atom used more than once\"));\n \tif_then_else->else_atom = 1;\n \tpush_stack_element(&state->stack);\n \tstate->stack->at_end_data = prev->at_end_data;\n@@ -355,7 +365,7 @@ static void end_atom_handler(struct atom_value *atomv, struct ref_formatting_sta\n \t\tdie(_(\"format: %%(end) atom used without corresponding atom\"));\n \tcurrent->at_end(&state->stack);\n \n-\t/* Stack may have been popped, hence reset the current pointer */\n+\t/* Stack may have been popped within at_end(), hence reset the current pointer */\n \tcurrent = state->stack;\n \n \t/*\n@@ -458,33 +468,24 @@ static int grab_objectname(const char *name, const unsigned char *sha1,\n \tconst char *p;\n \n \tif (match_atom_name(name, \"objectname\", &p)) {\n-\t\tstruct strbuf **s, **to_free;\n-\t\tint length = DEFAULT_ABBREV;\n-\n \t\t/* No arguments given, copy the entire objectname */\n \t\tif (!p) {\n \t\t\tchar *s = xmalloc(41);\n \t\t\tstrcpy(s, sha1_to_hex(sha1));\n \t\t\tv->s = s;\n \t\t} else {\n-\t\t\ts = to_free = strbuf_split_str(p, ',', 0);\n-\t\t\twhile (*s) {\n-\t\t\t\t/* Strip trailing comma */\n-\t\t\t\tif (s[1])\n-\t\t\t\t\tstrbuf_setlen(s[0], s[0]->len - 1);\n-\t\t\t\tif (!strtoul_ui(s[0]->buf, 10, (unsigned int *)&length))\n-\t\t\t\t\t;\n-\t\t\t\t/* The `short` argument uses the default length */\n-\t\t\t\telse if (!strcmp(\"short\", s[0]->buf))\n-\t\t\t\t\t;\n-\t\t\t\telse\n-\t\t\t\t\tdie(_(\"format: unknown option entered with objectname:%s\"), s[0]->buf);\n-\t\t\t\ts++;\n-\t\t\t}\n+\t\t\tunsigned int length = DEFAULT_ABBREV;\n+\n+\t\t\tif (skip_prefix(p, \"short\", &p)) {\n+\t\t\t\tif (p[0] == '=')\n+\t\t\t\t\tif (strtoul_ui(p + 1, 10, &length))\n+\t\t\t\t\t\tdie(_(\"positive length expected with objectname:%s\"), p + 1);\n+\t\t\t} else\n+\t\t\t\tdie(_(\"format: unknown option entered with objectname:%s\"), p);\n+\n \t\t\tif (length < MINIMUM_ABBREV)\n \t\t\t\tlength = MINIMUM_ABBREV;\n \t\t\tv->s = xstrdup(find_unique_abbrev(sha1, length));\n-\t\t\tfree(to_free);\n \t\t}\n \t\treturn 1;\n \t}\n@@ -916,7 +917,7 @@ static inline char *copy_advance(char *dst, const char *src)\n \treturn dst;\n }\n \n-static char *get_head_description(void)\n+char *get_head_description(void)\n {\n \tstruct strbuf desc = STRBUF_INIT;\n \tstruct wt_status_state state;\n@@ -1100,22 +1101,6 @@ static void populate_value(struct ref_array_item *ref)\n \t\t} else if (!strcmp(name, \"else\")) {\n \t\t\tv->handler = else_atom_handler;\n \t\t\tcontinue;\n-\t\t} else if (match_atom_name(name, \"path\", &valp)) {\n-\t\t\tconst char *sp, *ep;\n-\n-\t\t\tif (ref->kind & FILTER_REFS_DETACHED_HEAD)\n-\t\t\t\tcontinue;\n-\n-\t\t\tsp = strchr(ref->refname, '/');\n-\t\t\tep = strchr(++sp, '/');\n-\n-\t\t\tif (!valp)\n-\t\t\t\tsp = ref->refname;\n-\t\t\telse if (strcmp(valp, \"short\"))\n-\t\t\t\tdie(_(\"format: invalid value given path:%s\"), valp);\n-\n-\t\t\tv->s = xstrndup(sp, ep - sp);\n-\t\t\tcontinue;\n \t\t} else\n \t\t\tcontinue;\n \n@@ -1127,27 +1112,45 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\tif (!strcmp(formatp, \"short\"))\n \t\t\t\trefname = shorten_unambiguous_ref(refname,\n \t\t\t\t\t\t warn_ambiguous_refs);\n-\t\t\telse if (!strcmp(formatp, \"track\") &&\n+\t\t\telse if (skip_prefix(formatp, \"track\", &valp) &&\n+\t\t\t\t strcmp(formatp, \"trackshort\") &&\n \t\t\t\t (starts_with(name, \"upstream\") ||\n \t\t\t\t starts_with(name, \"push\"))) {\n \t\t\t\tchar buf[40];\n+\t\t\t\tunsigned int nobracket = 0;\n+\n+\t\t\t\tif (!strcmp(valp, \",nobracket\"))\n+\t\t\t\t\tnobracket = 1;\n \n \t\t\t\tif (stat_tracking_info(branch, &num_ours,\n \t\t\t\t\t\t &num_theirs, NULL)) {\n-\t\t\t\t\tv->s = xstrdup(\"[gone]\");\n+\t\t\t\t\tif (nobracket)\n+\t\t\t\t\t\tv->s = \"gone\";\n+\t\t\t\t\telse\n+\t\t\t\t\t\tv->s = \"[gone]\";\n \t\t\t\t\tcontinue;\n \t\t\t\t}\n \n \t\t\t\tif (!num_ours && !num_theirs)\n \t\t\t\t\tv->s = \"\";\n \t\t\t\telse if (!num_ours) {\n-\t\t\t\t\tsprintf(buf, \"[behind %d]\", num_theirs);\n+\t\t\t\t\tif (nobracket)\n+\t\t\t\t\t\tsprintf(buf, \"behind %d\", num_theirs);\n+\t\t\t\t\telse\n+\t\t\t\t\t\tsprintf(buf, \"[behind %d]\", num_theirs);\n \t\t\t\t\tv->s = xstrdup(buf);\n \t\t\t\t} else if (!num_theirs) {\n-\t\t\t\t\tsprintf(buf, \"[ahead %d]\", num_ours);\n+\t\t\t\t\tif (nobracket)\n+\t\t\t\t\t\tsprintf(buf, \"ahead %d\", num_ours);\n+\t\t\t\t\telse\n+\t\t\t\t\t\tsprintf(buf, \"[ahead %d]\", num_ours);\n \t\t\t\t\tv->s = xstrdup(buf);\n \t\t\t\t} else {\n-\t\t\t\t\tsprintf(buf, \"[ahead %d, behind %d]\",\n+\t\t\t\t\tif (nobracket)\n+\t\t\t\t\t\tsprintf(buf, \"ahead %d, behind %d\",\n+\t\t\t\t\t\t\tnum_ours, num_theirs);\n+\t\t\t\t\telse\n+\t\t\t\t\t\tsprintf(buf, \"[ahead %d, behind %d]\",\n \t\t\t\t\t\tnum_ours, num_theirs);\n \t\t\t\t\tv->s = xstrdup(buf);\n \t\t\t\t}\n@@ -1170,6 +1173,29 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\t\telse\n \t\t\t\t\tv->s = \"<>\";\n \t\t\t\tcontinue;\n+\t\t\t} else if (!strcmp(formatp, \"dir\") &&\n+\t\t\t\t (starts_with(name, \"refname\"))) {\n+\t\t\t\tconst char *sp, *ep, *tmp;\n+\n+\t\t\t\tsp = tmp = ref->refname;\n+\t\t\t\t/* Obtain refs/foo/bar/ from refname refs/foo/bar/abc */\n+\t\t\t\tdo {\n+\t\t\t\t\tep = tmp;\n+\t\t\t\t\ttmp = strchr(ep + 1, '/');\n+\t\t\t\t} while (tmp);\n+\t\t\t\tv->s = xstrndup(sp, ep - sp);\n+\t\t\t\tcontinue;\n+\t\t\t} else if (!strcmp(formatp, \"base\") &&\n+\t\t\t\t (starts_with(name, \"refname\"))) {\n+\t\t\t\tconst char *sp, *ep;\n+\n+\t\t\t\tif (skip_prefix(ref->refname, \"refs/\", &sp)) {\n+\t\t\t\t\tep = strchr(sp, '/');\n+\t\t\t\t\tif (!ep)\n+\t\t\t\t\t\tcontinue;\n+\t\t\t\t\tv->s = xstrndup(sp, ep - sp);\n+\t\t\t\t}\n+\t\t\t\tcontinue;\n \t\t\t} else\n \t\t\t\tdie(\"unknown %.*s format %s\",\n \t\t\t\t (int)(formatp - name), name, formatp);\ndiff --git a/ref-filter.h b/ref-filter.h\nindex e0b26e8..0014b92 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -109,5 +109,7 @@ int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset);\n struct ref_sorting *ref_default_sorting(void);\n /* Function to parse --merged and --no-merged options */\n int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset);\n+/* Get the current HEAD's description */\n+char *get_head_description(void);\n \n #endif /* REF_FILTER_H */\ndiff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh\nindex d110f26..97a0765 100755\n--- a/t/t6040-tracking-info.sh\n+++ b/t/t6040-tracking-info.sh\n@@ -58,11 +58,11 @@ test_expect_success 'branch -v' '\n '\n \n cat >expect <<\\EOF\n-b1 [origin/master] [ahead 1, behind 1] d\n-b2 [origin/master] [ahead 1, behind 1] d\n-b3 [origin/master] [behind 1] b\n-b4 [origin/master] [ahead 2] f\n-b5 [brokenbase] [gone] g\n+b1 [origin/master: ahead 1, behind 1] d\n+b2 [origin/master: ahead 1, behind 1] d\n+b3 [origin/master: behind 1] b\n+b4 [origin/master: ahead 2] f\n+b5 [brokenbase: gone] g\n b6 [origin/master] c\n EOF\n \ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 2a01645..7ab8bf8 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -344,6 +344,15 @@ test_expect_success 'Check upstream:track format' '\n '\n \n cat >expected <actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected <\n EOF\n \n@@ -390,7 +399,7 @@ $(git rev-parse --short=1 HEAD)\n EOF\n \n test_expect_success 'Check objectname:short format when size is less than MINIMUM_ABBREV' '\n-\tgit for-each-ref --format=\"%(objectname:short,1)\" refs/heads >actual &&\n+\tgit for-each-ref --format=\"%(objectname:short=1)\" refs/heads >actual &&\n \ttest_cmp expected actual\n '\n \n@@ -399,12 +408,12 @@ $(git rev-parse --short=10 HEAD)\n EOF\n \n test_expect_success 'Check objectname:short format' '\n-\tgit for-each-ref --format=\"%(objectname:short,10)\" refs/heads >actual &&\n+\tgit for-each-ref --format=\"%(objectname:short=10)\" refs/heads >actual &&\n \ttest_cmp expected actual\n '\n \n test_expect_success 'Check objectname:short format for invalid input' '\n-\ttest_must_fail git for-each-ref --format=\"%(objectname:short,-1)\" refs/heads\n+\ttest_must_fail git for-each-ref --format=\"%(objectname:short=-1)\" refs/heads\n '\n \n test_expect_success 'Check for invalid refname format' '\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex 5557657..19a5075 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -303,6 +303,25 @@ test_expect_success 'check %(if)...%(then)...%(else)...%(end) atoms' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'ignore spaces in %(if) atom usage' '\n+\tgit for-each-ref --format=\"%(refname:short): %(if)%(HEAD)%(then)Head ref%(else)Not Head ref%(end)\" >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tmaster: Head ref\n+\tside: Not Head ref\n+\todd/spot: Not Head ref\n+\tdouble-tag: Not Head ref\n+\tfoo1.10: Not Head ref\n+\tfoo1.3: Not Head ref\n+\tfoo1.6: Not Head ref\n+\tfour: Not Head ref\n+\tone: Not Head ref\n+\tsigned-tag: Not Head ref\n+\tthree: Not Head ref\n+\ttwo: Not Head ref\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'check %(if:equals=)' '\n \tgit for-each-ref --format=\"%(if:equals=master)%(refname:short)%(then)Found master%(else)Not master%(end)\" refs/heads/ >actual &&\n \tcat >expect <<-\\EOF &&\n@@ -312,7 +331,6 @@ test_expect_success 'check %(if:equals=)' '\n \ttest_cmp expect actual\n '\n \n-\n test_expect_success 'check %(if:notequals=)' '\n \tgit for-each-ref --format=\"%(if:notequals=master)%(refname:short)%(then)Not master%(else)Found master%(end)\" refs/heads/ >actual &&\n \tcat >expect <<-\\EOF &&\n@@ -322,9 +340,16 @@ test_expect_success 'check %(if:notequals=)' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'check %(path)' '\n-\tgit for-each-ref --format=\"%(path)\" >actual &&\n+test_expect_success 'update refs for %(refname:dir) and %(refname:base)' '\n+\tgit update-ref refs/foo HEAD &&\n+\tgit update-ref refs/foodir/bar/boz HEAD\n+'\n+\n+test_expect_success 'check %(refname:dir)' '\n+\tgit for-each-ref --format=\"%(refname:dir)\" >actual &&\n \tcat >expect <<-\\EOF &&\n+\trefs\n+\trefs/foodir/bar\n \trefs/heads\n \trefs/heads\n \trefs/odd\n@@ -341,9 +366,11 @@ test_expect_success 'check %(path)' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'check %(path:short)' '\n-\tgit for-each-ref --format=\"%(path:short)\" >actual &&\n+test_expect_success 'check %(refname:base)' '\n+\tgit for-each-ref --format=\"%(refname:base)\" >actual &&\n \tcat >expect <<-\\EOF &&\n+\t\n+\tfoodir\n \theads\n \theads\n \todd\n\n--\n2.6.0\n"},{"id":"271302","messageId":"1444295885-1657-2-git-send-email-Karthik.188@gmail.com","threadId":"40517","inReplyTo":"1444295885-1657-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v2 01/10] ref-filter: implement %(if), %(then), and %(else) atoms","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-08T09:17:56Z","receivedAt":"2015-10-08T09:17:56Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Implement %(if), %(then) and %(else) atoms. Used as\n%(if)...%(then)...%(end) or %(if)...%(then)...%(else)...%(end). If the\nformat string between %(if) and %(then) expands to an empty string, or\nto only whitespaces, then the whole %(if)...%(end) expands to the\nstring following %(then). Otherwise, it expands to the string\nfollowing %(else), if any. Nesting of this construct is possible.\n\nThis is in preperation for porting over `git branch -l` to use\nref-filter APIs for printing.\n\nAdd Documentation and tests regarding the same.\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n Documentation/git-for-each-ref.txt | 47 +++++++++++++-\n ref-filter.c | 127 +++++++++++++++++++++++++++++++++++--\n t/t6302-for-each-ref-filter.sh | 67 +++++++++++++++++++\n 3 files changed, 231 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 16b4ac5..7345acb 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -134,9 +134,17 @@ align::\n \t`` is either left, right or middle, default being\n \tleft and `` is the total length of the content with\n \talignment. If the contents length is more than the width then\n-\tno alignment is performed. If used with '--quote' everything\n-\tin between %(align:...) and %(end) is quoted, but if nested\n-\tthen only the topmost level performs quoting.\n+\tno alignment is performed.\n+\n+if::\n+\tUsed as %(if)..%(then)..(%end) or %(if)..%(then)..%(else)..%(end).\n+\tIf there is an atom with value or string literal after the\n+\t%(if) then everything after the %(then) is printed, else if\n+\tthe %(else) atom is used, then everything after %(else) is\n+\tprinted. We ignore space when evaluating the string before\n+\t%(then), this is useful when we use the %(HEAD) atom which\n+\tprints either \"*\" or \" \" and we want to apply the 'if'\n+\tcondition only on the 'HEAD' ref.\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@@ -169,6 +177,19 @@ the date by adding one of `:default`, `:relative`, `:short`, `:local`,\n `:iso8601`, `:rfc2822` or `:raw` to the end of the fieldname; e.g.\n `%(taggerdate:relative)`.\n \n+Some atoms like %(align) and %(if) always require a matching %(end).\n+We call them \"opening atoms\" and sometimes denote them as %($open).\n+\n+When a scripting language specific quoting is in effect, except for\n+opening atoms, replacement from every %(atom) is quoted when and only\n+when it appears at the top-level (that is, when it appears outside\n+%($open)...%(end)).\n+\n+When a scripting language specific quoting is in effect (i.e. one of\n+`--shell`, `--perl`, `--python`, `--tcl` is used), everything between\n+a top-level opening atom and its matching %(end) is evaluated\n+according to the semantics of the opening atom and its result is\n+quoted.\n \n EXAMPLES\n --------\n@@ -256,6 +277,26 @@ eval=`git for-each-ref --shell --format=\"$fmt\" \\\n eval \"$eval\"\n ------------\n \n+\n+An example to show the usage of %(if)...%(then)...%(else)...%(end).\n+This prefixes the current branch with a star.\n+\n+------------\n+#!/bin/sh\n+\n+git for-each-ref --format=\"%(if)%(HEAD)%(then)* %(else) %(end)%(refname:short)\" refs/heads/\n+------------\n+\n+\n+An example to show the usage of %(if)...%(then)...%(end).\n+This adds a red color to authorname, if present\n+\n+------------\n+#!/bin/sh\n+\n+git for-each-ref --format=\"%(refname)%(if)%(authorname)%(then)%(color:red)%(end) %(authorname)\"\n+------------\n+\n SEE ALSO\n --------\n linkgit:git-show-ref[1]\ndiff --git a/ref-filter.c b/ref-filter.c\nindex dbd8fce..95f007c 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -55,6 +55,9 @@ static struct {\n \t{ \"color\" },\n \t{ \"align\" },\n \t{ \"end\" },\n+\t{ \"if\" },\n+\t{ \"then\" },\n+\t{ \"else\" },\n };\n \n #define REF_FORMATTING_STATE_INIT { 0, NULL }\n@@ -69,10 +72,16 @@ struct contents {\n \tstruct object_id oid;\n };\n \n+struct if_then_else {\n+\tunsigned int then_atom : 1,\n+\t\telse_atom : 1,\n+\t\tcondition_satisfied : 1;\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)(struct ref_formatting_stack **stack);\n \tvoid *at_end_data;\n };\n \n@@ -216,13 +225,14 @@ 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+static void end_align_handler(struct ref_formatting_stack **stack)\n {\n-\tstruct align *align = (struct align *)stack->at_end_data;\n+\tstruct ref_formatting_stack *cur = *stack;\n+\tstruct align *align = (struct align *)cur->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_utf8_align(&s, align->position, align->width, cur->output.buf);\n+\tstrbuf_swap(&cur->output, &s);\n \tstrbuf_release(&s);\n }\n \n@@ -236,6 +246,97 @@ static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_s\n \tnew->at_end_data = &atomv->u.align;\n }\n \n+static void if_then_else_handler(struct ref_formatting_stack **stack)\n+{\n+\tstruct ref_formatting_stack *cur = *stack;\n+\tstruct ref_formatting_stack *prev = cur->prev;\n+\tstruct if_then_else *if_then_else = (struct if_then_else *)cur->at_end_data;\n+\n+\tif (if_then_else->else_atom) {\n+\t\t/*\n+\t\t * There is an %(else) atom: we need to drop one state from the\n+\t\t * stack, either the %(else) branch if the condition is satisfied, or\n+\t\t * the %(then) branch if it isn't.\n+\t\t */\n+\t\tif (if_then_else->condition_satisfied) {\n+\t\t\tstrbuf_reset(&cur->output);\n+\t\t\tpop_stack_element(&cur);\n+\t\t} else {\n+\t\t\tstrbuf_swap(&cur->output, &prev->output);\n+\t\t\tstrbuf_reset(&cur->output);\n+\t\t\tpop_stack_element(&cur);\n+\t\t}\n+\t} else if (!if_then_else->condition_satisfied)\n+\t\t/*\n+\t\t * No %(else) atom: just drop the %(then) branch if the\n+\t\t * condition is not satisfied.\n+\t\t */\n+\t\tstrbuf_reset(&cur->output);\n+\n+\t*stack = cur;\n+\tfree(if_then_else);\n+}\n+\n+static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)\n+{\n+\tstruct ref_formatting_stack *new;\n+\tstruct if_then_else *if_then_else = xcalloc(sizeof(struct if_then_else), 1);\n+\n+\tpush_stack_element(&state->stack);\n+\tnew = state->stack;\n+\tnew->at_end = if_then_else_handler;\n+\tnew->at_end_data = if_then_else;\n+}\n+\n+static int is_empty(const char *s){\n+\twhile (*s != '\\0') {\n+\t\tif (!isspace(*s))\n+\t\t\treturn 0;\n+\t\ts++;\n+\t}\n+\treturn 1;\n+}\n+\n+static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)\n+{\n+\tstruct ref_formatting_stack *cur = state->stack;\n+\tstruct if_then_else *if_then_else = NULL;\n+\n+\tif (cur->at_end == if_then_else_handler)\n+\t\tif_then_else = (struct if_then_else *)cur->at_end_data;\n+\tif (!if_then_else)\n+\t\tdie(_(\"format: %%(then) atom used without an %%(if) atom\"));\n+\tif (if_then_else->then_atom)\n+\t\tdie(_(\"format: %%(then) atom used more than once\"));\n+\tif_then_else->then_atom = 1;\n+\t/*\n+\t * If there exists non-empty string between the 'if' and\n+\t * 'then' atom then the 'if' condition is satisfied.\n+\t */\n+\tif (cur->output.len && !is_empty(cur->output.buf))\n+\t\tif_then_else->condition_satisfied = 1;\n+\tstrbuf_reset(&cur->output);\n+}\n+\n+static void else_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)\n+{\n+\tstruct ref_formatting_stack *prev = state->stack;\n+\tstruct if_then_else *if_then_else = NULL;\n+\n+\tif (prev->at_end == if_then_else_handler)\n+\t\tif_then_else = (struct if_then_else *)prev->at_end_data;\n+\tif (!if_then_else)\n+\t\tdie(_(\"format: %%(else) atom used without an %%(if) atom\"));\n+\tif (!if_then_else->then_atom)\n+\t\tdie(_(\"format: %%(else) atom used without a %%(then) atom\"));\n+\tif (if_then_else->else_atom)\n+\t\tdie(_(\"format: %%(else) atom used more than once\"));\n+\tif_then_else->else_atom = 1;\n+\tpush_stack_element(&state->stack);\n+\tstate->stack->at_end_data = prev->at_end_data;\n+\tstate->stack->at_end = prev->at_end;\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@@ -243,14 +344,17 @@ static void end_atom_handler(struct atom_value *atomv, struct ref_formatting_sta\n \n \tif (!current->at_end)\n \t\tdie(_(\"format: %%(end) atom used without corresponding atom\"));\n-\tcurrent->at_end(current);\n+\tcurrent->at_end(&state->stack);\n+\n+\t/* Stack may have been popped within at_end(), hence reset the current pointer */\n+\tcurrent = state->stack;\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+\tif (!current->prev->prev) {\n \t\tquote_formatting(&s, current->output.buf, state->quote_style);\n \t\tstrbuf_swap(¤t->output, &s);\n \t}\n@@ -920,6 +1024,15 @@ static void populate_value(struct ref_array_item *ref)\n \t\t} else if (!strcmp(name, \"end\")) {\n \t\t\tv->handler = end_atom_handler;\n \t\t\tcontinue;\n+\t\t} else if (!strcmp(name, \"if\")) {\n+\t\t\tv->handler = if_atom_handler;\n+\t\t\tcontinue;\n+\t\t} else if (!strcmp(name, \"then\")) {\n+\t\t\tv->handler = then_atom_handler;\n+\t\t\tcontinue;\n+\t\t} else if (!strcmp(name, \"else\")) {\n+\t\t\tv->handler = else_atom_handler;\n+\t\t\tcontinue;\n \t\t} else\n \t\t\tcontinue;\n \ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex fe4796c..617fa1f 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -255,4 +255,71 @@ test_expect_success 'reverse version sort' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'improper usage of %(if), %(then), %(else) and %(end) atoms' '\n+\ttest_must_fail git for-each-ref --format=\"%(if)\" &&\n+\ttest_must_fail git for-each-ref --format=\"%(then)\" &&\n+\ttest_must_fail git for-each-ref --format=\"%(else)\" &&\n+\ttest_must_fail git for-each-ref --format=\"%(if) %(else)\" &&\n+\ttest_must_fail git for-each-ref --format=\"%(then) %(else)\" &&\n+\ttest_must_fail git for-each-ref --format=\"%(if) %(else)\" &&\n+\ttest_must_fail git for-each-ref --format=\"%(if) %(then) %(else)\"\n+'\n+\n+test_expect_success 'check %(if)...%(then)...%(end) atoms' '\n+\tgit for-each-ref --format=\"%(if)%(authorname)%(then)%(authorname): %(refname)%(end)\" >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tA U Thor: refs/heads/master\n+\tA U Thor: refs/heads/side\n+\tA U Thor: refs/odd/spot\n+\t\n+\tA U Thor: refs/tags/foo1.10\n+\tA U Thor: refs/tags/foo1.3\n+\tA U Thor: refs/tags/foo1.6\n+\tA U Thor: refs/tags/four\n+\tA U Thor: refs/tags/one\n+\t\n+\tA U Thor: refs/tags/three\n+\tA U Thor: refs/tags/two\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'check %(if)...%(then)...%(else)...%(end) atoms' '\n+\tgit for-each-ref --format=\"%(if)%(authorname)%(then)%(authorname)%(else)No author%(end): %(refname)\" >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tA U Thor: refs/heads/master\n+\tA U Thor: refs/heads/side\n+\tA U Thor: refs/odd/spot\n+\tNo author: refs/tags/double-tag\n+\tA U Thor: refs/tags/foo1.10\n+\tA U Thor: refs/tags/foo1.3\n+\tA U Thor: refs/tags/foo1.6\n+\tA U Thor: refs/tags/four\n+\tA U Thor: refs/tags/one\n+\tNo author: refs/tags/signed-tag\n+\tA U Thor: refs/tags/three\n+\tA U Thor: refs/tags/two\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'ignore spaces in %(if) atom usage' '\n+\tgit for-each-ref --format=\"%(refname:short): %(if)%(HEAD)%(then)Head ref%(else)Not Head ref%(end)\" >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tmaster: Head ref\n+\tside: Not Head ref\n+\todd/spot: Not Head ref\n+\tdouble-tag: Not Head ref\n+\tfoo1.10: Not Head ref\n+\tfoo1.3: Not Head ref\n+\tfoo1.6: Not Head ref\n+\tfour: Not Head ref\n+\tone: Not Head ref\n+\tsigned-tag: Not Head ref\n+\tthree: Not Head ref\n+\ttwo: Not Head ref\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.6.0\n"},{"id":"271303","messageId":"1444295885-1657-3-git-send-email-Karthik.188@gmail.com","threadId":"40517","inReplyTo":"1444295885-1657-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v2 02/10] ref-filter: implement %(if:equals=) and %(if:notequals=)","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-08T09:17:57Z","receivedAt":"2015-10-08T09:17:57Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Implement %(if:equals=) wherein the if condition is only\nsatisfied if the value obtained between the %(if:...) and %(then) atom\nis the same as the given ''.\n\nSimilarly, implement (if:notequals=) wherein the if condition\nis only satisfied if the value obtained between the %(if:...) and\n%(then) atom is differnt from the given ''.\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-for-each-ref.txt | 4 +++-\n ref-filter.c | 28 ++++++++++++++++++++++++----\n t/t6302-for-each-ref-filter.sh | 18 ++++++++++++++++++\n 3 files changed, 45 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 7345acb..206d646 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -144,7 +144,9 @@ if::\n \tprinted. We ignore space when evaluating the string before\n \t%(then), this is useful when we use the %(HEAD) atom which\n \tprints either \"*\" or \" \" and we want to apply the 'if'\n-\tcondition only on the 'HEAD' ref.\n+\tcondition only on the 'HEAD' ref. Append \":equals=\" or\n+\t\":notequals=\" to compare the value between the\n+\t%(if:...) and %(then) atoms with the given string.\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/ref-filter.c b/ref-filter.c\nindex 95f007c..359c76d 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -73,6 +73,8 @@ struct contents {\n };\n \n struct if_then_else {\n+\tconst char *if_equals,\n+\t\t*not_equals;\n \tunsigned int then_atom : 1,\n \t\telse_atom : 1,\n \t\tcondition_satisfied : 1;\n@@ -281,6 +283,14 @@ static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_stat\n {\n \tstruct ref_formatting_stack *new;\n \tstruct if_then_else *if_then_else = xcalloc(sizeof(struct if_then_else), 1);\n+\tconst char *valp;\n+\n+\tif (skip_prefix(atomv->s, \"equals=\", &valp))\n+\t\tif_then_else->if_equals = valp;\n+\telse if (skip_prefix(atomv->s, \"notequals=\", &valp))\n+\t\tif_then_else->not_equals = valp;\n+\telse if (atomv->s[0])\n+\t\tdie(_(\"format: unknown format if:%s\"), atomv->s);\n \n \tpush_stack_element(&state->stack);\n \tnew = state->stack;\n@@ -309,11 +319,19 @@ static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_st\n \tif (if_then_else->then_atom)\n \t\tdie(_(\"format: %%(then) atom used more than once\"));\n \tif_then_else->then_atom = 1;\n+\n \t/*\n-\t * If there exists non-empty string between the 'if' and\n-\t * 'then' atom then the 'if' condition is satisfied.\n+\t * If the 'equals' or 'notequals' attribute is used then\n+\t * perform the required comparison. If not, only non-empty\n+\t * strings satisfy the 'if' condition.\n \t */\n-\tif (cur->output.len && !is_empty(cur->output.buf))\n+\tif (if_then_else->if_equals) {\n+\t\tif (!strcmp(if_then_else->if_equals, cur->output.buf))\n+\t\t\tif_then_else->condition_satisfied = 1;\n+\t} else \tif (if_then_else->not_equals) {\n+\t\tif (strcmp(if_then_else->not_equals, cur->output.buf))\n+\t\t\tif_then_else->condition_satisfied = 1;\n+\t} else if (cur->output.len && !is_empty(cur->output.buf))\n \t\tif_then_else->condition_satisfied = 1;\n \tstrbuf_reset(&cur->output);\n }\n@@ -1024,8 +1042,10 @@ static void populate_value(struct ref_array_item *ref)\n \t\t} else if (!strcmp(name, \"end\")) {\n \t\t\tv->handler = end_atom_handler;\n \t\t\tcontinue;\n-\t\t} else if (!strcmp(name, \"if\")) {\n+\t\t} else if (match_atom_name(name, \"if\", &valp)) {\n \t\t\tv->handler = if_atom_handler;\n+\t\t\tif (valp)\n+\t\t\t\tv->s = xstrdup(valp);\n \t\t\tcontinue;\n \t\t} else if (!strcmp(name, \"then\")) {\n \t\t\tv->handler = then_atom_handler;\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex 617fa1f..f45ac1f 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -322,4 +322,22 @@ test_expect_success 'ignore spaces in %(if) atom usage' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'check %(if:equals=)' '\n+\tgit for-each-ref --format=\"%(if:equals=master)%(refname:short)%(then)Found master%(else)Not master%(end)\" refs/heads/ >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tFound master\n+\tNot master\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'check %(if:notequals=)' '\n+\tgit for-each-ref --format=\"%(if:notequals=master)%(refname:short)%(then)Not master%(else)Found master%(end)\" refs/heads/ >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tFound master\n+\tNot master\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.6.0\n"},{"id":"271312","messageId":"1444295885-1657-4-git-send-email-Karthik.188@gmail.com","threadId":"40517","inReplyTo":"1444295885-1657-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v2 03/10] ref-filter: add support for %(refname:dir) and %(refname:base)","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-08T09:17:58Z","receivedAt":"2015-10-08T09:17:58Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Add the options `:dir` and `:base` to the %(refname) atom. The `:dir`\noption gives the directory (the part after $GIT_DIR/) of the ref\nwithout the refname. The `:base` option gives the base directory of\nthe given ref (i.e. the directory following $GIT_DIR/refs/).\n\nAdd tests and documentation for the same.\n---\n Documentation/git-for-each-ref.txt | 4 +++-\n ref-filter.c | 23 +++++++++++++++++++\n t/t6302-for-each-ref-filter.sh | 47 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 73 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 206d646..b8d33a1 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -92,7 +92,9 @@ refname::\n \tThe name of the ref (the part after $GIT_DIR/).\n \tFor a non-ambiguous short name of the ref append `:short`.\n \tThe option core.warnAmbiguousRefs is used to select the strict\n-\tabbreviation mode.\n+\tabbreviation mode. For the base directory of the ref (i.e. foo\n+\tin refs/foo/bar/boz) append `:base`. For the entire directory\n+\tpath append `:dir`.\n \n objecttype::\n \tThe type of the object (`blob`, `tree`, `commit`, `tag`).\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 359c76d..94d8754 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1105,6 +1105,29 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\t\telse\n \t\t\t\t\tv->s = \"<>\";\n \t\t\t\tcontinue;\n+\t\t\t} else if (!strcmp(formatp, \"dir\") &&\n+\t\t\t\t (starts_with(name, \"refname\"))) {\n+\t\t\t\tconst char *sp, *ep, *tmp;\n+\n+\t\t\t\tsp = tmp = ref->refname;\n+\t\t\t\t/* Obtain refs/foo/bar/ from refname refs/foo/bar/abc */\n+\t\t\t\tdo {\n+\t\t\t\t\tep = tmp;\n+\t\t\t\t\ttmp = strchr(ep + 1, '/');\n+\t\t\t\t} while (tmp);\n+\t\t\t\tv->s = xstrndup(sp, ep - sp);\n+\t\t\t\tcontinue;\n+\t\t\t} else if (!strcmp(formatp, \"base\") &&\n+\t\t\t\t (starts_with(name, \"refname\"))) {\n+\t\t\t\tconst char *sp, *ep;\n+\n+\t\t\t\tif (skip_prefix(ref->refname, \"refs/\", &sp)) {\n+\t\t\t\t\tep = strchr(sp, '/');\n+\t\t\t\t\tif (!ep)\n+\t\t\t\t\t\tcontinue;\n+\t\t\t\t\tv->s = xstrndup(sp, ep - sp);\n+\t\t\t\t}\n+\t\t\t\tcontinue;\n \t\t\t} else\n \t\t\t\tdie(\"unknown %.*s format %s\",\n \t\t\t\t (int)(formatp - name), name, formatp);\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex f45ac1f..19a5075 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -340,4 +340,51 @@ test_expect_success 'check %(if:notequals=)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'update refs for %(refname:dir) and %(refname:base)' '\n+\tgit update-ref refs/foo HEAD &&\n+\tgit update-ref refs/foodir/bar/boz HEAD\n+'\n+\n+test_expect_success 'check %(refname:dir)' '\n+\tgit for-each-ref --format=\"%(refname:dir)\" >actual &&\n+\tcat >expect <<-\\EOF &&\n+\trefs\n+\trefs/foodir/bar\n+\trefs/heads\n+\trefs/heads\n+\trefs/odd\n+\trefs/tags\n+\trefs/tags\n+\trefs/tags\n+\trefs/tags\n+\trefs/tags\n+\trefs/tags\n+\trefs/tags\n+\trefs/tags\n+\trefs/tags\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'check %(refname:base)' '\n+\tgit for-each-ref --format=\"%(refname:base)\" >actual &&\n+\tcat >expect <<-\\EOF &&\n+\t\n+\tfoodir\n+\theads\n+\theads\n+\todd\n+\ttags\n+\ttags\n+\ttags\n+\ttags\n+\ttags\n+\ttags\n+\ttags\n+\ttags\n+\ttags\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.6.0\n"},{"id":"271305","messageId":"1444295885-1657-5-git-send-email-Karthik.188@gmail.com","threadId":"40517","inReplyTo":"1444295885-1657-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v2 04/10] ref-filter: modify \"%(objectname:short)\" to take length","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-08T09:17:59Z","receivedAt":"2015-10-08T09:17:59Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Add support for %(objectname:short=) which would print the\nabbreviated unique objectname of given length. When no length is\nspecified 7 is used. The minimum length is 4.\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-for-each-ref.txt | 4 +++-\n ref-filter.c | 30 ++++++++++++++++++++++--------\n t/t6300-for-each-ref.sh | 22 ++++++++++++++++++++++\n 3 files changed, 47 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex b8d33a1..c713ec0 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -104,7 +104,9 @@ objectsize::\n \n objectname::\n \tThe object name (aka SHA-1).\n-\tFor a non-ambiguous abbreviation of the object name append `:short`.\n+\tFor a non-ambiguous abbreviation of the object name append\n+\t`:short`. The length can be explicitly specified by appending\n+\t`:short=`. Minimum length being 4.\n \n upstream::\n \tThe name of a local ref which can be considered ``upstream''\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 94d8754..de4d451 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -464,14 +464,28 @@ static void *get_obj(const unsigned char *sha1, struct object **obj, unsigned lo\n static int grab_objectname(const char *name, const unsigned char *sha1,\n \t\t\t struct atom_value *v)\n {\n-\tif (!strcmp(name, \"objectname\")) {\n-\t\tchar *s = xmalloc(41);\n-\t\tstrcpy(s, sha1_to_hex(sha1));\n-\t\tv->s = s;\n-\t\treturn 1;\n-\t}\n-\tif (!strcmp(name, \"objectname:short\")) {\n-\t\tv->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));\n+\tconst char *p;\n+\n+\tif (match_atom_name(name, \"objectname\", &p)) {\n+\t\t/* No arguments given, copy the entire objectname */\n+\t\tif (!p) {\n+\t\t\tchar *s = xmalloc(41);\n+\t\t\tstrcpy(s, sha1_to_hex(sha1));\n+\t\t\tv->s = s;\n+\t\t} else {\n+\t\t\tunsigned int length = DEFAULT_ABBREV;\n+\n+\t\t\tif (skip_prefix(p, \"short\", &p)) {\n+\t\t\t\tif (p[0] == '=')\n+\t\t\t\t\tif (strtoul_ui(p + 1, 10, &length))\n+\t\t\t\t\t\tdie(_(\"positive length expected with objectname:%s\"), p + 1);\n+\t\t\t} else\n+\t\t\t\tdie(_(\"format: unknown option entered with objectname:%s\"), p);\n+\n+\t\t\tif (length < MINIMUM_ABBREV)\n+\t\t\t\tlength = MINIMUM_ABBREV;\n+\t\t\tv->s = xstrdup(find_unique_abbrev(sha1, length));\n+\t\t}\n \t\treturn 1;\n \t}\n \treturn 0;\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 7c9bec7..6fc569e 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -385,6 +385,28 @@ test_expect_success 'Check short objectname format' '\n \ttest_cmp expected actual\n '\n \n+cat >expected <actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected <actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'Check objectname:short format for invalid input' '\n+\ttest_must_fail git for-each-ref --format=\"%(objectname:short=-1)\" refs/heads\n+'\n+\n test_expect_success 'Check for invalid refname format' '\n \ttest_must_fail git for-each-ref --format=\"%(refname:INVALID)\"\n '\n-- \n2.6.0\n"},{"id":"271306","messageId":"1444295885-1657-6-git-send-email-Karthik.188@gmail.com","threadId":"40517","inReplyTo":"1444295885-1657-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v2 05/10] ref-filter: adopt get_head_description() from branch.c","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-08T09:18:00Z","receivedAt":"2015-10-08T09:18:00Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Copy the implementation of get_head_description() from branch.c. This\ngives a description of the HEAD ref if called. This is used as the\nrefname for the HEAD ref whenever the FILTER_REFS_DETACHED_HEAD option\nis used. Make it public because we need it to calculate the length of\nthe HEAD refs description in branch.c:calc_maxwidth() when we port\nbranch.c to use ref-filter APIs.\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n builtin/branch.c | 4 ++++\n ref-filter.c | 38 ++++++++++++++++++++++++++++++++++++--\n ref-filter.h | 2 ++\n 3 files changed, 42 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex b7a60f4..67ef9f1 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -355,6 +355,10 @@ static void add_verbose_info(struct strbuf *out, struct ref_array_item *item,\n \tstrbuf_release(&subject);\n }\n \n+/*\n+ * This is duplicated in ref-filter.c, will be removed when we adopt\n+ * ref-filter's printing APIs.\n+ */\n static char *get_head_description(void)\n {\n \tstruct strbuf desc = STRBUF_INIT;\ndiff --git a/ref-filter.c b/ref-filter.c\nindex de4d451..a3cc3af 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -13,6 +13,7 @@\n #include \"utf8.h\"\n #include \"git-compat-util.h\"\n #include \"version.h\"\n+#include \"wt-status.h\"\n \n typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n \n@@ -916,6 +917,37 @@ static inline char *copy_advance(char *dst, const char *src)\n \treturn dst;\n }\n \n+char *get_head_description(void)\n+{\n+\tstruct strbuf desc = STRBUF_INIT;\n+\tstruct wt_status_state state;\n+\tmemset(&state, 0, sizeof(state));\n+\twt_status_get_state(&state, 1);\n+\tif (state.rebase_in_progress ||\n+\t state.rebase_interactive_in_progress)\n+\t\tstrbuf_addf(&desc, _(\"(no branch, rebasing %s)\"),\n+\t\t\t state.branch);\n+\telse if (state.bisect_in_progress)\n+\t\tstrbuf_addf(&desc, _(\"(no branch, bisect started on %s)\"),\n+\t\t\t state.branch);\n+\telse if (state.detached_from) {\n+\t\t/* TRANSLATORS: make sure these match _(\"HEAD detached at \")\n+\t\t and _(\"HEAD detached from \") in wt-status.c */\n+\t\tif (state.detached_at)\n+\t\t\tstrbuf_addf(&desc, _(\"(HEAD detached at %s)\"),\n+\t\t\t\tstate.detached_from);\n+\t\telse\n+\t\t\tstrbuf_addf(&desc, _(\"(HEAD detached from %s)\"),\n+\t\t\t\tstate.detached_from);\n+\t}\n+\telse\n+\t\tstrbuf_addstr(&desc, _(\"(no branch)\"));\n+\tfree(state.branch);\n+\tfree(state.onto);\n+\tfree(state.detached_from);\n+\treturn strbuf_detach(&desc, NULL);\n+}\n+\n /*\n * Parse the object referred by ref, and grab needed value.\n */\n@@ -954,9 +986,11 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\tname++;\n \t\t}\n \n-\t\tif (starts_with(name, \"refname\"))\n+\t\tif (starts_with(name, \"refname\")) {\n \t\t\trefname = ref->refname;\n-\t\telse if (starts_with(name, \"symref\"))\n+\t\t\tif (ref->kind & FILTER_REFS_DETACHED_HEAD)\n+\t\t\t\trefname = get_head_description();\n+\t\t} else if (starts_with(name, \"symref\"))\n \t\t\trefname = ref->symref ? ref->symref : \"\";\n \t\telse if (starts_with(name, \"upstream\")) {\n \t\t\tconst char *branch_name;\ndiff --git a/ref-filter.h b/ref-filter.h\nindex 14d435e..4aea594 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -106,5 +106,7 @@ int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset);\n struct ref_sorting *ref_default_sorting(void);\n /* Function to parse --merged and --no-merged options */\n int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset);\n+/* Get the current HEAD's description */\n+char *get_head_description(void);\n \n #endif /* REF_FILTER_H */\n-- \n2.6.0\n"},{"id":"271307","messageId":"1444295885-1657-7-git-send-email-Karthik.188@gmail.com","threadId":"40517","inReplyTo":"1444295885-1657-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v2 06/10] ref-filter: introduce format_ref_array_item()","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-08T09:18:01Z","receivedAt":"2015-10-08T09:18:01Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"To allow column display, we will need to first render the output in a\nstring list to allow print_columns() to compute the proper size of\neach column before starting the actual output. Introduce the function\nformat_ref_array_item() that does the formatting of a ref_array_item\nto an strbuf.\n\nshow_ref_array_item() is kept as a convenience wrapper around it which\nobtains the strbuf and prints it the standard output.\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n ref-filter.c | 16 ++++++++++++----\n ref-filter.h | 3 +++\n 2 files changed, 15 insertions(+), 4 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex a3cc3af..38e4424 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1756,10 +1756,10 @@ static void append_literal(const char *cp, const char *ep, struct ref_formatting\n \t}\n }\n \n-void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)\n+void format_ref_array_item(struct ref_array_item *info, const char *format,\n+\t\t\t int quote_style, struct strbuf *final_buf)\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@@ -1789,9 +1789,17 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\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+\tstrbuf_addbuf(final_buf, &state.stack->output);\n \tpop_stack_element(&state.stack);\n+}\n+\n+void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)\n+{\n+\tstruct strbuf final_buf = STRBUF_INIT;\n+\n+\tformat_ref_array_item(info, format, quote_style, &final_buf);\n+\tfwrite(final_buf.buf, 1, final_buf.len, stdout);\n+\tstrbuf_release(&final_buf);\n \tputchar('\\n');\n }\n \ndiff --git a/ref-filter.h b/ref-filter.h\nindex 4aea594..0014b92 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -98,6 +98,9 @@ int parse_ref_filter_atom(const char *atom, const char *ep);\n int verify_ref_format(const char *format);\n /* Sort the given ref_array as per the ref_sorting provided */\n void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);\n+/* Based on the given format and quote_style, fill the strbuf */\n+void format_ref_array_item(struct ref_array_item *info, const char *format,\n+\t\t\t int quote_style, struct strbuf *final_buf);\n /* Print the ref using the given format and quote_style */\n void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style);\n /* Callback function for parsing the sort option */\n-- \n2.6.0\n"},{"id":"271311","messageId":"1444295885-1657-8-git-send-email-Karthik.188@gmail.com","threadId":"40517","inReplyTo":"1444295885-1657-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v2 07/10] ref-filter: make %(upstream:track) prints \"[gone]\" for invalid upstreams","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-08T09:18:02Z","receivedAt":"2015-10-08T09:18:02Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Borrowing from branch.c's implementation print \"[gone]\" whenever an\nunknown upstream ref is encountered instead of just ignoring it.\n\nThis makes sure that when branch.c is ported over to using ref-filter\nAPIs for printing, this feature is not lost.\n\nMake changes to t/t6300-for-each-ref.sh to reflect this change.\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n ref-filter.c | 4 +++-\n t/t6300-for-each-ref.sh | 2 +-\n 2 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 38e4424..6a38089 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1118,8 +1118,10 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\t\tchar buf[40];\n \n \t\t\t\tif (stat_tracking_info(branch, &num_ours,\n-\t\t\t\t\t\t &num_theirs, NULL))\n+\t\t\t\t\t\t &num_theirs, NULL)) {\n+\t\t\t\t\tv->s = \"[gone]\";\n \t\t\t\t\tcontinue;\n+\t\t\t\t}\n \n \t\t\t\tif (!num_ours && !num_theirs)\n \t\t\t\t\tv->s = \"\";\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 6fc569e..4f620bf 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -359,7 +359,7 @@ test_expect_success 'Check that :track[short] cannot be used with other atoms' '\n \n test_expect_success 'Check that :track[short] works when upstream is invalid' '\n \tcat >expected <<-\\EOF &&\n-\n+\t[gone]\n \n \tEOF\n \ttest_when_finished \"git config branch.master.merge refs/heads/master\" &&\n-- \n2.6.0\n"},{"id":"271310","messageId":"1444295885-1657-9-git-send-email-Karthik.188@gmail.com","threadId":"40517","inReplyTo":"1444295885-1657-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v2 08/10] ref-filter: add support for %(upstream:track,nobracket)","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-08T09:18:03Z","receivedAt":"2015-10-08T09:18:03Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Add support for %(upstream:track,nobracket) which will print the\ntracking information without the brackets (i.e. \"ahead N, behind M\").\n\nAdd test and documentation for the same.\n---\n Documentation/git-for-each-ref.txt | 6 ++++--\n ref-filter.c | 28 +++++++++++++++++++++++-----\n t/t6300-for-each-ref.sh | 9 +++++++++\n 3 files changed, 36 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex c713ec0..a38cbf6 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -114,8 +114,10 @@ upstream::\n \t`refname` above. Additionally respects `:track` to show\n \t\"[ahead N, behind M]\" and `:trackshort` to show the terse\n \tversion: \">\" (ahead), \"<\" (behind), \"<>\" (ahead and behind),\n-\tor \"=\" (in sync). Has no effect if the ref does not have\n-\ttracking information associated with it.\n+\tor \"=\" (in sync). Append `:track,nobracket` to show tracking\n+\tinformation without brackets (i.e \"ahead N, behind M\"). Has\n+\tno effect if the ref does not have tracking information\n+\tassociated with it.\n \n push::\n \tThe name of a local ref which represents the `@{push}` location\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 6a38089..6044eb0 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1112,27 +1112,45 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\tif (!strcmp(formatp, \"short\"))\n \t\t\t\trefname = shorten_unambiguous_ref(refname,\n \t\t\t\t\t\t warn_ambiguous_refs);\n-\t\t\telse if (!strcmp(formatp, \"track\") &&\n+\t\t\telse if (skip_prefix(formatp, \"track\", &valp) &&\n+\t\t\t\t strcmp(formatp, \"trackshort\") &&\n \t\t\t\t (starts_with(name, \"upstream\") ||\n \t\t\t\t starts_with(name, \"push\"))) {\n \t\t\t\tchar buf[40];\n+\t\t\t\tunsigned int nobracket = 0;\n+\n+\t\t\t\tif (!strcmp(valp, \",nobracket\"))\n+\t\t\t\t\tnobracket = 1;\n \n \t\t\t\tif (stat_tracking_info(branch, &num_ours,\n \t\t\t\t\t\t &num_theirs, NULL)) {\n-\t\t\t\t\tv->s = \"[gone]\";\n+\t\t\t\t\tif (nobracket)\n+\t\t\t\t\t\tv->s = \"gone\";\n+\t\t\t\t\telse\n+\t\t\t\t\t\tv->s = \"[gone]\";\n \t\t\t\t\tcontinue;\n \t\t\t\t}\n \n \t\t\t\tif (!num_ours && !num_theirs)\n \t\t\t\t\tv->s = \"\";\n \t\t\t\telse if (!num_ours) {\n-\t\t\t\t\tsprintf(buf, \"[behind %d]\", num_theirs);\n+\t\t\t\t\tif (nobracket)\n+\t\t\t\t\t\tsprintf(buf, \"behind %d\", num_theirs);\n+\t\t\t\t\telse\n+\t\t\t\t\t\tsprintf(buf, \"[behind %d]\", num_theirs);\n \t\t\t\t\tv->s = xstrdup(buf);\n \t\t\t\t} else if (!num_theirs) {\n-\t\t\t\t\tsprintf(buf, \"[ahead %d]\", num_ours);\n+\t\t\t\t\tif (nobracket)\n+\t\t\t\t\t\tsprintf(buf, \"ahead %d\", num_ours);\n+\t\t\t\t\telse\n+\t\t\t\t\t\tsprintf(buf, \"[ahead %d]\", num_ours);\n \t\t\t\t\tv->s = xstrdup(buf);\n \t\t\t\t} else {\n-\t\t\t\t\tsprintf(buf, \"[ahead %d, behind %d]\",\n+\t\t\t\t\tif (nobracket)\n+\t\t\t\t\t\tsprintf(buf, \"ahead %d, behind %d\",\n+\t\t\t\t\t\t\tnum_ours, num_theirs);\n+\t\t\t\t\telse\n+\t\t\t\t\t\tsprintf(buf, \"[ahead %d, behind %d]\",\n \t\t\t\t\t\tnum_ours, num_theirs);\n \t\t\t\t\tv->s = xstrdup(buf);\n \t\t\t\t}\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 4f620bf..7ab8bf8 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -344,6 +344,15 @@ test_expect_success 'Check upstream:track format' '\n '\n \n cat >expected <actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected <\n EOF\n \n-- \n2.6.0\n"},{"id":"271309","messageId":"1444295885-1657-10-git-send-email-Karthik.188@gmail.com","threadId":"40517","inReplyTo":"1444295885-1657-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v2 09/10] branch: use ref-filter printing APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-08T09:18:04Z","receivedAt":"2015-10-08T09:18:04Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Port branch.c to use ref-filter APIs for printing. This clears out\nmost of the code used in branch.c for printing and replaces them with\ncalls made to the ref-filter library.\n\nIntroduce get_format() which gets the format required for printing of\nrefs. Make amendments to print_ref_list() to reflect these changes.\n\nChange calc_maxwidth() to also account for the length of HEAD ref, by\ncalling ref-filter:get_head_discription().\n\nMentored-by: Christian Couder \nMentored-by: Matthieu Moy \nSigned-off-by: Karthik Nayak \n---\n builtin/branch.c | 261 ++++++++++++-----------------------------------\n t/t6040-tracking-info.sh | 2 +-\n 2 files changed, 66 insertions(+), 197 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 67ef9f1..4d8b570 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -35,12 +35,12 @@ static unsigned char head_sha1[20];\n \n static int branch_use_color = -1;\n static char branch_colors[][COLOR_MAXLEN] = {\n-\tGIT_COLOR_RESET,\n-\tGIT_COLOR_NORMAL,\t/* PLAIN */\n-\tGIT_COLOR_RED,\t\t/* REMOTE */\n-\tGIT_COLOR_NORMAL,\t/* LOCAL */\n-\tGIT_COLOR_GREEN,\t/* CURRENT */\n-\tGIT_COLOR_BLUE,\t\t/* UPSTREAM */\n+\t\"%(color:reset)\",\n+\t\"%(color:reset)\",\t/* PLAIN */\n+\t\"%(color:red)\",\t\t/* REMOTE */\n+\t\"%(color:reset)\",\t/* LOCAL */\n+\t\"%(color:green)\",\t/* CURRENT */\n+\t\"%(color:blue)\",\t/* UPSTREAM */\n };\n enum color_branch {\n \tBRANCH_COLOR_RESET = 0,\n@@ -271,192 +271,6 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \treturn(ret);\n }\n \n-static void fill_tracking_info(struct strbuf *stat, const char *branch_name,\n-\t\tint show_upstream_ref)\n-{\n-\tint ours, theirs;\n-\tchar *ref = NULL;\n-\tstruct branch *branch = branch_get(branch_name);\n-\tconst char *upstream;\n-\tstruct strbuf fancy = STRBUF_INIT;\n-\tint upstream_is_gone = 0;\n-\tint added_decoration = 1;\n-\n-\tif (stat_tracking_info(branch, &ours, &theirs, &upstream) < 0) {\n-\t\tif (!upstream)\n-\t\t\treturn;\n-\t\tupstream_is_gone = 1;\n-\t}\n-\n-\tif (show_upstream_ref) {\n-\t\tref = shorten_unambiguous_ref(upstream, 0);\n-\t\tif (want_color(branch_use_color))\n-\t\t\tstrbuf_addf(&fancy, \"%s%s%s\",\n-\t\t\t\t\tbranch_get_color(BRANCH_COLOR_UPSTREAM),\n-\t\t\t\t\tref, branch_get_color(BRANCH_COLOR_RESET));\n-\t\telse\n-\t\t\tstrbuf_addstr(&fancy, ref);\n-\t}\n-\n-\tif (upstream_is_gone) {\n-\t\tif (show_upstream_ref)\n-\t\t\tstrbuf_addf(stat, _(\"[%s: gone]\"), fancy.buf);\n-\t\telse\n-\t\t\tadded_decoration = 0;\n-\t} else if (!ours && !theirs) {\n-\t\tif (show_upstream_ref)\n-\t\t\tstrbuf_addf(stat, _(\"[%s]\"), fancy.buf);\n-\t\telse\n-\t\t\tadded_decoration = 0;\n-\t} else if (!ours) {\n-\t\tif (show_upstream_ref)\n-\t\t\tstrbuf_addf(stat, _(\"[%s: behind %d]\"), fancy.buf, theirs);\n-\t\telse\n-\t\t\tstrbuf_addf(stat, _(\"[behind %d]\"), theirs);\n-\n-\t} else if (!theirs) {\n-\t\tif (show_upstream_ref)\n-\t\t\tstrbuf_addf(stat, _(\"[%s: ahead %d]\"), fancy.buf, ours);\n-\t\telse\n-\t\t\tstrbuf_addf(stat, _(\"[ahead %d]\"), ours);\n-\t} else {\n-\t\tif (show_upstream_ref)\n-\t\t\tstrbuf_addf(stat, _(\"[%s: ahead %d, behind %d]\"),\n-\t\t\t\t fancy.buf, ours, theirs);\n-\t\telse\n-\t\t\tstrbuf_addf(stat, _(\"[ahead %d, behind %d]\"),\n-\t\t\t\t ours, theirs);\n-\t}\n-\tstrbuf_release(&fancy);\n-\tif (added_decoration)\n-\t\tstrbuf_addch(stat, ' ');\n-\tfree(ref);\n-}\n-\n-static void add_verbose_info(struct strbuf *out, struct ref_array_item *item,\n-\t\t\t struct ref_filter *filter, const char *refname)\n-{\n-\tstruct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;\n-\tconst char *sub = _(\" **** invalid ref ****\");\n-\tstruct commit *commit = item->commit;\n-\n-\tif (!parse_commit(commit)) {\n-\t\tpp_commit_easy(CMIT_FMT_ONELINE, commit, &subject);\n-\t\tsub = subject.buf;\n-\t}\n-\n-\tif (item->kind == FILTER_REFS_BRANCHES)\n-\t\tfill_tracking_info(&stat, refname, filter->verbose > 1);\n-\n-\tstrbuf_addf(out, \" %s %s%s\",\n-\t\tfind_unique_abbrev(item->commit->object.sha1, filter->abbrev),\n-\t\tstat.buf, sub);\n-\tstrbuf_release(&stat);\n-\tstrbuf_release(&subject);\n-}\n-\n-/*\n- * This is duplicated in ref-filter.c, will be removed when we adopt\n- * ref-filter's printing APIs.\n- */\n-static char *get_head_description(void)\n-{\n-\tstruct strbuf desc = STRBUF_INIT;\n-\tstruct wt_status_state state;\n-\tmemset(&state, 0, sizeof(state));\n-\twt_status_get_state(&state, 1);\n-\tif (state.rebase_in_progress ||\n-\t state.rebase_interactive_in_progress)\n-\t\tstrbuf_addf(&desc, _(\"(no branch, rebasing %s)\"),\n-\t\t\t state.branch);\n-\telse if (state.bisect_in_progress)\n-\t\tstrbuf_addf(&desc, _(\"(no branch, bisect started on %s)\"),\n-\t\t\t state.branch);\n-\telse if (state.detached_from) {\n-\t\t/* TRANSLATORS: make sure these match _(\"HEAD detached at \")\n-\t\t and _(\"HEAD detached from \") in wt-status.c */\n-\t\tif (state.detached_at)\n-\t\t\tstrbuf_addf(&desc, _(\"(HEAD detached at %s)\"),\n-\t\t\t\tstate.detached_from);\n-\t\telse\n-\t\t\tstrbuf_addf(&desc, _(\"(HEAD detached from %s)\"),\n-\t\t\t\tstate.detached_from);\n-\t}\n-\telse\n-\t\tstrbuf_addstr(&desc, _(\"(no branch)\"));\n-\tfree(state.branch);\n-\tfree(state.onto);\n-\tfree(state.detached_from);\n-\treturn strbuf_detach(&desc, NULL);\n-}\n-\n-static void format_and_print_ref_item(struct ref_array_item *item, int maxwidth,\n-\t\t\t\t struct ref_filter *filter, const char *remote_prefix)\n-{\n-\tchar c;\n-\tint current = 0;\n-\tint color;\n-\tstruct strbuf out = STRBUF_INIT, name = STRBUF_INIT;\n-\tconst char *prefix = \"\";\n-\tconst char *desc = item->refname;\n-\tchar *to_free = NULL;\n-\n-\tswitch (item->kind) {\n-\tcase FILTER_REFS_BRANCHES:\n-\t\tskip_prefix(desc, \"refs/heads/\", &desc);\n-\t\tif (!filter->detached && !strcmp(desc, head))\n-\t\t\tcurrent = 1;\n-\t\telse\n-\t\t\tcolor = BRANCH_COLOR_LOCAL;\n-\t\tbreak;\n-\tcase FILTER_REFS_REMOTES:\n-\t\tskip_prefix(desc, \"refs/remotes/\", &desc);\n-\t\tcolor = BRANCH_COLOR_REMOTE;\n-\t\tprefix = remote_prefix;\n-\t\tbreak;\n-\tcase FILTER_REFS_DETACHED_HEAD:\n-\t\tdesc = to_free = get_head_description();\n-\t\tcurrent = 1;\n-\t\tbreak;\n-\tdefault:\n-\t\tcolor = BRANCH_COLOR_PLAIN;\n-\t\tbreak;\n-\t}\n-\n-\tc = ' ';\n-\tif (current) {\n-\t\tc = '*';\n-\t\tcolor = BRANCH_COLOR_CURRENT;\n-\t}\n-\n-\tstrbuf_addf(&name, \"%s%s\", prefix, desc);\n-\tif (filter->verbose) {\n-\t\tint utf8_compensation = strlen(name.buf) - utf8_strwidth(name.buf);\n-\t\tstrbuf_addf(&out, \"%c %s%-*s%s\", c, branch_get_color(color),\n-\t\t\t maxwidth + utf8_compensation, name.buf,\n-\t\t\t branch_get_color(BRANCH_COLOR_RESET));\n-\t} else\n-\t\tstrbuf_addf(&out, \"%c %s%s%s\", c, branch_get_color(color),\n-\t\t\t name.buf, branch_get_color(BRANCH_COLOR_RESET));\n-\n-\tif (item->symref) {\n-\t\tskip_prefix(item->symref, \"refs/remotes/\", &desc);\n-\t\tstrbuf_addf(&out, \" -> %s\", desc);\n-\t}\n-\telse if (filter->verbose)\n-\t\t/* \" f7c0c00 [ahead 58, behind 197] vcs-svn: drop obj_pool.h\" */\n-\t\tadd_verbose_info(&out, item, filter, desc);\n-\tif (column_active(colopts)) {\n-\t\tassert(!filter->verbose && \"--column and --verbose are incompatible\");\n-\t\tstring_list_append(&output, out.buf);\n-\t} else {\n-\t\tprintf(\"%s\\n\", out.buf);\n-\t}\n-\tstrbuf_release(&name);\n-\tstrbuf_release(&out);\n-\tfree(to_free);\n-}\n-\n static int calc_maxwidth(struct ref_array *refs, int remote_bonus)\n {\n \tint i, max = 0;\n@@ -467,7 +281,10 @@ static int calc_maxwidth(struct ref_array *refs, int remote_bonus)\n \n \t\tskip_prefix(it->refname, \"refs/heads/\", &desc);\n \t\tskip_prefix(it->refname, \"refs/remotes/\", &desc);\n-\t\tw = utf8_strwidth(desc);\n+\t\tif (it->kind == FILTER_REFS_DETACHED_HEAD)\n+\t\t\tw = strlen(get_head_description());\n+\t\telse\n+\t\t\tw = utf8_strwidth(desc);\n \n \t\tif (it->kind == FILTER_REFS_REMOTES)\n \t\t\tw += remote_bonus;\n@@ -477,12 +294,51 @@ static int calc_maxwidth(struct ref_array *refs, int remote_bonus)\n \treturn max;\n }\n \n+static char *build_format(struct ref_filter *filter, int maxwidth, const char *remote_prefix)\n+{\n+\tstruct strbuf fmt = STRBUF_INIT;\n+\tstruct strbuf local = STRBUF_INIT;\n+\tstruct strbuf remote = STRBUF_INIT;\n+\n+\tstrbuf_addf(&fmt, \"%%(if)%%(HEAD)%%(then)* %s%%(else) %%(end)\", branch_get_color(BRANCH_COLOR_CURRENT));\n+\n+\tif (filter->verbose) {\n+\t\tstrbuf_addf(&local, \"%%(align:%d,left)%%(refname:short)%%(end)\", maxwidth);\n+\t\tstrbuf_addf(&local, \"%s\", branch_get_color(BRANCH_COLOR_RESET));\n+\t\tstrbuf_addf(&local, \" %%(objectname:short=7) \");\n+\n+\t\tif (filter->verbose > 1)\n+\t\t\tstrbuf_addf(&local, \"%%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s%%(if)%%(upstream:track)\"\n+\t\t\t\t \"%%(then): %%(upstream:track,nobracket)%%(end)] %%(end)%%(contents:subject)\",\n+\t\t\t\t branch_get_color(BRANCH_COLOR_UPSTREAM), branch_get_color(BRANCH_COLOR_RESET));\n+\t\telse\n+\t\t\tstrbuf_addf(&local, \"%%(if)%%(upstream:track)%%(then)%%(upstream:track) %%(end)%%(contents:subject)\");\n+\n+\t\tstrbuf_addf(&remote, \"%s%%(align:%d,left)%s%%(refname:short)%%(end)%s%%(if)%%(symref)%%(then) -> %%(symref:short)\"\n+\t\t\t \"%%(else) %%(objectname:short=7) %%(contents:subject)%%(end)\",\n+\t\t\t branch_get_color(BRANCH_COLOR_REMOTE), maxwidth,\n+\t\t\t remote_prefix, branch_get_color(BRANCH_COLOR_RESET));\n+\t} else {\n+\t\tstrbuf_addf(&local, \"%%(refname:short)%s\", branch_get_color(BRANCH_COLOR_RESET));\n+\t\tstrbuf_addf(&remote, \"%s%s%%(refname:short)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)\",\n+\t\t\t branch_get_color(BRANCH_COLOR_REMOTE), remote_prefix, branch_get_color(BRANCH_COLOR_RESET));\n+\t}\n+\n+\tstrbuf_addf(&fmt, \"%%(if:notequals=remotes)%%(refname:base)%%(then)%s%%(else)%s%%(end)\", local.buf, remote.buf);\n+\n+\tstrbuf_release(&local);\n+\tstrbuf_release(&remote);\n+\treturn strbuf_detach(&fmt, NULL);\n+}\n+\n static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sorting)\n {\n \tint i;\n \tstruct ref_array array;\n \tint maxwidth = 0;\n \tconst char *remote_prefix = \"\";\n+\tstruct strbuf out = STRBUF_INIT;\n+\tchar *format;\n \n \t/*\n \t * If we are listing more than just remote branches,\n@@ -494,12 +350,14 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n \n \tmemset(&array, 0, sizeof(array));\n \n-\tverify_ref_format(\"%(refname)%(symref)\");\n \tfilter_refs(&array, filter, filter->kind | FILTER_REFS_INCLUDE_BROKEN);\n \n \tif (filter->verbose)\n \t\tmaxwidth = calc_maxwidth(&array, strlen(remote_prefix));\n \n+\tformat = build_format(filter, maxwidth, remote_prefix);\n+\tverify_ref_format(format);\n+\n \t/*\n \t * If no sorting parameter is given then we default to sorting\n \t * by 'refname'. This would give us an alphabetically sorted\n@@ -511,10 +369,21 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n \t\tsorting = ref_default_sorting();\n \tref_array_sort(sorting, &array);\n \n-\tfor (i = 0; i < array.nr; i++)\n-\t\tformat_and_print_ref_item(array.items[i], maxwidth, filter, remote_prefix);\n+\tfor (i = 0; i < array.nr; i++) {\n+\t\tformat_ref_array_item(array.items[i], format, 0, &out);\n+\t\tif (column_active(colopts)) {\n+\t\t\tassert(!filter->verbose && \"--column and --verbose are incompatible\");\n+\t\t\t /* format to a string_list to let print_columns() do its job */\n+\t\t\tstring_list_append(&output, out.buf);\n+\t\t} else {\n+\t\t\tfwrite(out.buf, 1, out.len, stdout);\n+\t\t\tputchar('\\n');\n+\t\t}\n+\t\tstrbuf_release(&out);\n+\t}\n \n \tref_array_clear(&array);\n+\tfree(format);\n }\n \n static void rename_branch(const char *oldname, const char *newname, int force)\ndiff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh\nindex 3d5c238..97a0765 100755\n--- a/t/t6040-tracking-info.sh\n+++ b/t/t6040-tracking-info.sh\n@@ -44,7 +44,7 @@ b1 [ahead 1, behind 1] d\n b2 [ahead 1, behind 1] d\n b3 [behind 1] b\n b4 [ahead 2] f\n-b5 g\n+b5 [gone] g\n b6 c\n EOF\n \n-- \n2.6.0\n"},{"id":"271308","messageId":"1444295885-1657-11-git-send-email-Karthik.188@gmail.com","threadId":"40517","inReplyTo":"1444295885-1657-1-git-send-email-Karthik.188@gmail.com","subject":"[PATCH v2 10/10] branch: implement '--format' option","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-08T09:18:05Z","receivedAt":"2015-10-08T09:18:05Z","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-branch.txt | 7 ++++++-\n builtin/branch.c | 14 +++++++++-----\n t/t3203-branch-output.sh | 11 +++++++++++\n 3 files changed, 26 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\nindex 03c7af1..5227206 100644\n--- a/Documentation/git-branch.txt\n+++ b/Documentation/git-branch.txt\n@@ -12,7 +12,7 @@ SYNOPSIS\n \t[--list] [-v [--abbrev= | --no-abbrev]]\n \t[--column[=] | --no-column]\n \t[(--merged | --no-merged | --contains) []] [--sort=]\n-\t[--points-at ] [...]\n+\t[--points-at ] [--format=] [...]\n 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] []\n 'git branch' (--set-upstream-to= | -u ) []\n 'git branch' --unset-upstream []\n@@ -244,6 +244,11 @@ start-point is either a local or remote-tracking branch.\n --points-at ::\n \tOnly list branches of the given object.\n \n+--format ::\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].\n+\n Examples\n --------\n \ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 4d8b570..2356e72 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -27,6 +27,7 @@ static const char * const builtin_branch_usage[] = {\n \tN_(\"git branch [] [-r] (-d | -D) ...\"),\n \tN_(\"git branch [] (-m | -M) [] \"),\n \tN_(\"git branch [] [-r | -a] [--points-at]\"),\n+\tN_(\"git branch [] [-r | -a] [--format]\"),\n \tNULL\n };\n \n@@ -331,14 +332,14 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r\n \treturn strbuf_detach(&fmt, NULL);\n }\n \n-static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sorting)\n+static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)\n {\n \tint i;\n \tstruct ref_array array;\n \tint maxwidth = 0;\n \tconst char *remote_prefix = \"\";\n \tstruct strbuf out = STRBUF_INIT;\n-\tchar *format;\n+\tchar *to_free = NULL;\n \n \t/*\n \t * If we are listing more than just remote branches,\n@@ -355,7 +356,8 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n \tif (filter->verbose)\n \t\tmaxwidth = calc_maxwidth(&array, strlen(remote_prefix));\n \n-\tformat = build_format(filter, maxwidth, remote_prefix);\n+\tif (!format)\n+\t\tformat = to_free = build_format(filter, maxwidth, remote_prefix);\n \tverify_ref_format(format);\n \n \t/*\n@@ -383,7 +385,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n \t}\n \n \tref_array_clear(&array);\n-\tfree(format);\n+\tfree(to_free);\n }\n \n static void rename_branch(const char *oldname, const char *newname, int force)\n@@ -484,6 +486,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \tenum branch_track track;\n \tstruct ref_filter filter;\n \tstatic struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;\n+\tconst char *format = NULL;\n \n \tstruct option options[] = {\n \t\tOPT_GROUP(N_(\"Generic options\")),\n@@ -524,6 +527,7 @@ int cmd_branch(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 branches 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@@ -582,7 +586,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tif ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)\n \t\t\tfilter.kind |= FILTER_REFS_DETACHED_HEAD;\n \t\tfilter.name_patterns = argv;\n-\t\tprint_ref_list(&filter, sorting);\n+\t\tprint_ref_list(&filter, sorting, format);\n \t\tprint_columns(&output, colopts, NULL);\n \t\tstring_list_clear(&output, 0);\n \t\treturn 0;\ndiff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh\nindex f1ae5ff..a475ff1 100755\n--- a/t/t3203-branch-output.sh\n+++ b/t/t3203-branch-output.sh\n@@ -163,4 +163,15 @@ test_expect_success 'git branch --points-at option' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git branch --format option' '\n+\tcat >expect <<-\\EOF &&\n+\tRefname is (HEAD detached from fromtag)\n+\tRefname is refs/heads/branch-one\n+\tRefname is refs/heads/branch-two\n+\tRefname is refs/heads/master\n+\tEOF\n+\tgit branch --format=\"Refname is %(refname)\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.6.0\n"},{"id":"271316","messageId":"vpqr3l5zgst.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"1444295885-1657-1-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-08T12:27:30Z","receivedAt":"2015-10-08T12:27: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> +An example to show the usage of %(if)...%(then)...%(else)...%(end).\n> +This prefixes the current branch with a star.\n> +\n> +------------\n> +#!/bin/sh\n> +\n> +git for-each-ref --format=\"%(if)%(HEAD)%(then)* %(else) %(end)%(refname:short)\" refs/heads/\n\nI don't think the #!/bin/sh adds any value here. Just the 'git\nfor-each-ref' line is sufficient IMHO.\n\n> +An example to show the usage of %(if)...%(then)...%(end).\n> +This adds a red color to authorname, if present\n\nI don't think this is such a good example.\n%(color:red)%(authorname)%(color:reset) just works even if %(authorname)\nis empty.\n\nA better example would be\n\ngit for-each-ref --format='%(if)%(authorname)%(then)Authored by %(authorname)%(end)'\n\nwhich avoids writting \"Authored by \" with no author.\n\n> -static int is_empty(const char * s){\n> +static int is_empty(const char *s){\n\nYou still have the { on the declaration line, it should be on the next\nline.\n\n> @@ -309,10 +311,14 @@ static int is_empty(const char * s){\n> static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)\n> {\n> \tstruct ref_formatting_stack *cur = state->stack;\n> -\tstruct if_then_else *if_then_else = (struct if_then_else *)cur->at_end_data;\n> +\tstruct if_then_else *if_then_else = NULL;\n> \n> +\tif (cur->at_end == if_then_else_handler)\n> +\t\tif_then_else = (struct if_then_else *)cur->at_end_data;\n\nOK, now the cast is safe since at_end_data has to be of type struct\nif_then_else * if at_end is if_then_else_handler.\n\n> +\t\t\t\tunsigned int nobracket = 0;\n> +\n> +\t\t\t\tif (!strcmp(valp, \",nobracket\"))\n> +\t\t\t\t\tnobracket = 1;\n\nThe code to parse comma-separated values is different here and\nelsewhere. I'd rather have the same code (possibly factored into a\nhelper function), both to get consistent behavior (you're not allowing\n%(upstream:nobracket,track) for example, right?) and consistent code.\n\n> \t\t\t\tif (!num_ours && !num_theirs)\n> \t\t\t\t\tv->s = \"\";\n> \t\t\t\telse if (!num_ours) {\n> -\t\t\t\t\tsprintf(buf, \"[behind %d]\", num_theirs);\n> +\t\t\t\t\tif (nobracket)\n> +\t\t\t\t\t\tsprintf(buf, \"behind %d\", num_theirs);\n> +\t\t\t\t\telse\n> +\t\t\t\t\t\tsprintf(buf, \"[behind %d]\", num_theirs);\n\nPerhaps use sprintf(buf, \"%sbehind %d%s\", obracket, num_their, cbracket)\nunconditionnally, and set obracket = \"\" or obracket = \"[\" once and for\nall when you test for \"nobracket\" above. This avoids these \"if\n(nobracket)\" spread accross the code, but at the price of extra %s in\nthe format strings.\n\n> @@ -1170,6 +1173,29 @@ static void populate_value(struct ref_array_item *ref)\n> \t\t\t\telse\n> \t\t\t\t\tv->s = \"<>\";\n> \t\t\t\tcontinue;\n> +\t\t\t} else if (!strcmp(formatp, \"dir\") &&\n> +\t\t\t\t (starts_with(name, \"refname\"))) {\n> +\t\t\t\tconst char *sp, *ep, *tmp;\n> +\n> +\t\t\t\tsp = tmp = ref->refname;\n> +\t\t\t\t/* Obtain refs/foo/bar/ from refname refs/foo/bar/abc */\n> +\t\t\t\tdo {\n> +\t\t\t\t\tep = tmp;\n> +\t\t\t\t\ttmp = strchr(ep + 1, '/');\n> +\t\t\t\t} while (tmp);\n\nTo look for the last occurence of '/' you can also use strrchr().\nDoesn't it do what you want here?\n\n> --- a/t/t6040-tracking-info.sh\n> +++ b/t/t6040-tracking-info.sh\n> @@ -58,11 +58,11 @@ test_expect_success 'branch -v' '\n> '\n> \n> cat >expect <<\\EOF\n> -b1 [origin/master] [ahead 1, behind 1] d\n> -b2 [origin/master] [ahead 1, behind 1] d\n> -b3 [origin/master] [behind 1] b\n> -b4 [origin/master] [ahead 2] f\n> -b5 [brokenbase] [gone] g\n> +b1 [origin/master: ahead 1, behind 1] d\n> +b2 [origin/master: ahead 1, behind 1] d\n> +b3 [origin/master: behind 1] b\n> +b4 [origin/master: ahead 2] f\n> +b5 [brokenbase: gone] g\n> b6 [origin/master] c\n> EOF\n\nCool!\n\nI didn't go through the patches themselves, but modulo my remarks above\nthe interdiff looks good. Thanks.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271318","messageId":"CAOLa=ZSuPZAPETH=-fUjBokgHScAHOFRhA=A3ei6tVL0V-=FwA@mail.gmail.com","threadId":"40517","inReplyTo":"vpqr3l5zgst.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-08T13:17:04Z","receivedAt":"2015-10-08T13:17:04Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Thu, Oct 8, 2015 at 5:57 PM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> +An example to show the usage of %(if)...%(then)...%(else)...%(end).\n>> +This prefixes the current branch with a star.\n>> +\n>> +------------\n>> +#!/bin/sh\n>> +\n>> +git for-each-ref --format=\"%(if)%(HEAD)%(then)* %(else) %(end)%(refname:short)\" refs/heads/\n>\n> I don't think the #!/bin/sh adds any value here. Just the 'git\n> for-each-ref' line is sufficient IMHO.\n>\n\nSure, we can remove that.\n\n>> +An example to show the usage of %(if)...%(then)...%(end).\n>> +This adds a red color to authorname, if present\n>\n> I don't think this is such a good example.\n> %(color:red)%(authorname)%(color:reset) just works even if %(authorname)\n> is empty.\n>\n> A better example would be\n>\n> git for-each-ref --format='%(if)%(authorname)%(then)Authored by %(authorname)%(end)'\n>\n> which avoids writting \"Authored by \" with no author.\n>\n\nYeah, will include this.\n\n>> -static int is_empty(const char * s){\n>> +static int is_empty(const char *s){\n>\n> You still have the { on the declaration line, it should be on the next\n> line.\n>\n\nOops, will change that.\n\n>> @@ -309,10 +311,14 @@ static int is_empty(const char * s){\n>> static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)\n>> {\n>> struct ref_formatting_stack *cur = state->stack;\n>> - struct if_then_else *if_then_else = (struct if_then_else *)cur->at_end_data;\n>> + struct if_then_else *if_then_else = NULL;\n>>\n>> + if (cur->at_end == if_then_else_handler)\n>> + if_then_else = (struct if_then_else *)cur->at_end_data;\n>\n> OK, now the cast is safe since at_end_data has to be of type struct\n> if_then_else * if at_end is if_then_else_handler.\n>\n>> + unsigned int nobracket = 0;\n>> +\n>> + if (!strcmp(valp, \",nobracket\"))\n>> + nobracket = 1;\n>\n> The code to parse comma-separated values is different here and\n> elsewhere. I'd rather have the same code (possibly factored into a\n> helper function), both to get consistent behavior (you're not allowing\n> %(upstream:nobracket,track) for example, right?) and consistent code.\n>\n\nEh, okay, will do that!\n\n>> if (!num_ours && !num_theirs)\n>> v->s = \"\";\n>> else if (!num_ours) {\n>> - sprintf(buf, \"[behind %d]\", num_theirs);\n>> + if (nobracket)\n>> + sprintf(buf, \"behind %d\", num_theirs);\n>> + else\n>> + sprintf(buf, \"[behind %d]\", num_theirs);\n>\n> Perhaps use sprintf(buf, \"%sbehind %d%s\", obracket, num_their, cbracket)\n> unconditionnally, and set obracket = \"\" or obracket = \"[\" once and for\n> all when you test for \"nobracket\" above. This avoids these \"if\n> (nobracket)\" spread accross the code, but at the price of extra %s in\n> the format strings.\n>\n\nOkay! will do that.\n\n>> @@ -1170,6 +1173,29 @@ static void populate_value(struct ref_array_item *ref)\n>> else\n>> v->s = \"<>\";\n>> continue;\n>> + } else if (!strcmp(formatp, \"dir\") &&\n>> + (starts_with(name, \"refname\"))) {\n>> + const char *sp, *ep, *tmp;\n>> +\n>> + sp = tmp = ref->refname;\n>> + /* Obtain refs/foo/bar/ from refname refs/foo/bar/abc */\n>> + do {\n>> + ep = tmp;\n>> + tmp = strchr(ep + 1, '/');\n>> + } while (tmp);\n>\n> To look for the last occurence of '/' you can also use strrchr().\n> Doesn't it do what you want here?\n>\n\nDidn't know that, thanks will do that.\n\n>> --- a/t/t6040-tracking-info.sh\n>> +++ b/t/t6040-tracking-info.sh\n>> @@ -58,11 +58,11 @@ test_expect_success 'branch -v' '\n>> '\n>>\n>> cat >expect <<\\EOF\n>> -b1 [origin/master] [ahead 1, behind 1] d\n>> -b2 [origin/master] [ahead 1, behind 1] d\n>> -b3 [origin/master] [behind 1] b\n>> -b4 [origin/master] [ahead 2] f\n>> -b5 [brokenbase] [gone] g\n>> +b1 [origin/master: ahead 1, behind 1] d\n>> +b2 [origin/master: ahead 1, behind 1] d\n>> +b3 [origin/master: behind 1] b\n>> +b4 [origin/master: ahead 2] f\n>> +b5 [brokenbase: gone] g\n>> b6 [origin/master] c\n>> EOF\n>\n> Cool!\n>\n> I didn't go through the patches themselves, but modulo my remarks above\n> the interdiff looks good. Thanks.\n>\n\nThanks for the review.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271320","messageId":"CAOLa=ZQvB_S2-nw8hOABt7aQJOWJXvfK1U2zurpnZmaAgJNnGA@mail.gmail.com","threadId":"40517","inReplyTo":"vpqr3l5zgst.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-08T16:09:19Z","receivedAt":"2015-10-08T16:09:19Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Thu, Oct 8, 2015 at 5:57 PM, Matthieu Moy\n wrote:\n>> + unsigned int nobracket = 0;\n>> +\n>> + if (!strcmp(valp, \",nobracket\"))\n>> + nobracket = 1;\n>\n> The code to parse comma-separated values is different here and\n> elsewhere. I'd rather have the same code (possibly factored into a\n> helper function), both to get consistent behavior (you're not allowing\n> %(upstream:nobracket,track) for example, right?) and consistent code.\n>\n\nSpeaking of comma-separated values, the only other place we use that is\nin the align atom. Also I find this very specific to get a function out of.\nSomehow I think this is the simplest way to go about this.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271326","messageId":"vpq8u7dp9qr.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"CAOLa=ZQvB_S2-nw8hOABt7aQJOWJXvfK1U2zurpnZmaAgJNnGA@mail.gmail.com","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-08T17:10:04Z","receivedAt":"2015-10-08T17:10:04Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Karthik Nayak writes:\n\n> On Thu, Oct 8, 2015 at 5:57 PM, Matthieu Moy\n> wrote:\n>>> + unsigned int nobracket = 0;\n>>> +\n>>> + if (!strcmp(valp, \",nobracket\"))\n>>> + nobracket = 1;\n>>\n>> The code to parse comma-separated values is different here and\n>> elsewhere. I'd rather have the same code (possibly factored into a\n>> helper function), both to get consistent behavior (you're not allowing\n>> %(upstream:nobracket,track) for example, right?) and consistent code.\n>>\n>\n> Speaking of comma-separated values, the only other place we use that is\n> in the align atom. Also I find this very specific to get a function out of.\n> Somehow I think this is the simplest way to go about this.\n\nWell, most pieces of code start with one instance, then two, then\nmore ;-). When the second instance starts being different from the\nfirst, it doesn't give a good example for the future third instance.\n\nThis particular piece of code is so important and I won't fight for a\nbetter factored one, but in general \"there are only two instances\" is a\ndubious argument to avoid refactoring.\n\nStill, I find it weird to force the nobracket to be always at the same\nposition.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271328","messageId":"CAOLa=ZQOO9BjoTj1B-b=kUviL=617F7y46BeX1sOXpeHcatFVQ@mail.gmail.com","threadId":"40517","inReplyTo":"vpq8u7dp9qr.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-08T17:28:31Z","receivedAt":"2015-10-08T17:28:31Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Thu, Oct 8, 2015 at 10:40 PM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> On Thu, Oct 8, 2015 at 5:57 PM, Matthieu Moy\n>> wrote:\n>>>> + unsigned int nobracket = 0;\n>>>> +\n>>>> + if (!strcmp(valp, \",nobracket\"))\n>>>> + nobracket = 1;\n>>>\n>>> The code to parse comma-separated values is different here and\n>>> elsewhere. I'd rather have the same code (possibly factored into a\n>>> helper function), both to get consistent behavior (you're not allowing\n>>> %(upstream:nobracket,track) for example, right?) and consistent code.\n>>>\n>>\n>> Speaking of comma-separated values, the only other place we use that is\n>> in the align atom. Also I find this very specific to get a function out of.\n>> Somehow I think this is the simplest way to go about this.\n>\n> Well, most pieces of code start with one instance, then two, then\n> more ;-). When the second instance starts being different from the\n> first, it doesn't give a good example for the future third instance.\n>\n\nTotally agree with you here.\n\n> This particular piece of code is so important and I won't fight for a\n> better factored one, but in general \"there are only two instances\" is a\n> dubious argument to avoid refactoring.\n>\n> Still, I find it weird to force the nobracket to be always at the same\n> position.\n>\n\nNo i mean I could follow up with the way we use it in align, but I don't see\nhow I can make a function out of this.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271330","messageId":"vpqoag9nrna.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"CAOLa=ZQOO9BjoTj1B-b=kUviL=617F7y46BeX1sOXpeHcatFVQ@mail.gmail.com","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-08T18:26:17Z","receivedAt":"2015-10-08T18:26:17Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Karthik Nayak writes:\n\n> On Thu, Oct 8, 2015 at 10:40 PM, Matthieu Moy\n> wrote:\n>\n>> This particular piece of code is so important and I won't fight for a\n>> better factored one, but in general \"there are only two instances\" is a\n>> dubious argument to avoid refactoring.\n>>\n>> Still, I find it weird to force the nobracket to be always at the same\n>> position.\n>>\n>\n> No i mean I could follow up with the way we use it in align, but I don't see\n> how I can make a function out of this.\n\nAh, OK. Well, there are already two instances of\n\n\t/* Strip trailing comma */\n\tif (s[1])\n\t\tstrbuf_setlen(s[0], s[0]->len - 1);\n\n\n(for objectname and align), one of which says\n\n\t/*\n\t * TODO: Implement a function similar to strbuf_split_str()\n\t * which would omit the separator from the end of each value.\n\t */\n\nI guess it's time to replace this TODO with actual code. A\nstraightforward implementation would be to call strbuf_split_str and\nthen use the same \"if (s[1])\" as you already have. There's probably a\nmore efficient way.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271331","messageId":"xmqq4mi1rywu.fsf@gitster.mtv.corp.google.com","threadId":"40517","inReplyTo":"CAOLa=ZQOO9BjoTj1B-b=kUviL=617F7y46BeX1sOXpeHcatFVQ@mail.gmail.com","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-08T18:35:45Z","receivedAt":"2015-10-08T18:35:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> No i mean I could follow up with the way we use it in align, but I don't see\n> how I can make a function out of this.\n\nAt least you should be able to pre-parse the %(:)\nconstruct, instead of doing strcmp() every time populate_value() is\ncalled, no? Then your parser would not be doing\n\n>>>> + if (!strcmp(valp, \",nobracket\"))\n>>>> + nobracket = 1;\n\nwith 'valp' that scans over 'name' (which is an element of\nused_atom[], i.e. a name from valid_atom[] followed by \":\"\nfor each customization). Instead your used_atom[] would be an array\nof a data structure that is richer than just a string.\n\nThe would be split at comma boundary and made into an\narray of two field structure, as the most general form of it is\n\n ::= | ',' \n ::= '=' | | \n\nif we recall the discussion we had while designing %(align), i.e.\n\n * The most general form is \"%(align:position=left,width=32)\"\n\n * The value domain of attributes may be distinct, in which case the\n value itself could imply what attr it is talking about,\n e.g. \"%(align:32,left)\" is sufficiently clear as '32' cannot be\n position and 'left' cannot be width.\n\nAnd clearly there can be an attr whose presense alone can imply its\nvalue (iow, a boolean), even though your %(align) does not yet have\nsuch a modifier. It is easy to imagine that you would later want to\nadd %(align:position=left,width=32,truncate) that truncates the value\nwhen its display width exceeds '32'. The 'nobracket' above would be\nan another example of a boolean (whose name is negative, which is\nnot a very good design in general, though).\n\nThen used_atom[] could become something like\n\n struct {\n \tconst char *str; /* e.g. \"align:position=left,32\" */\n\tstruct {\n \tconst char *part0; /* everything before '=' */\n const char *part1; /* optional */\n\t} *modifier;\n int modifier_nr;\n } *used_atom;\n\nand \"align:position=left,32\" would be parsed into\n\n\t.str = \"align:position=left,32\";\n .modifier = { { \"position\", \"left\" }, { \"32\", NULL } };\n .modifier_nr = 2;\n\nwhen the format string is read, which is done only once.\n\nThe looping over all the refs done in populate_value() could just\nuse the pre-parsed representation.\n\nHmm?\n"},{"id":"271332","messageId":"vpqa8rtnqzp.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"1444295885-1657-8-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v2 07/10] ref-filter: make %(upstream:track) prints \"[gone]\" for invalid upstreams","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-08T18:40:26Z","receivedAt":"2015-10-08T18:40:26Z","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> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1118,8 +1118,10 @@ static void populate_value(struct ref_array_item *ref)\n> \t\t\t\tchar buf[40];\n> \n> \t\t\t\tif (stat_tracking_info(branch, &num_ours,\n> -\t\t\t\t\t\t &num_theirs, NULL))\n> +\t\t\t\t\t\t &num_theirs, NULL)) {\n> +\t\t\t\t\tv->s = \"[gone]\";\n\nMy remark about translation still holds. The string was previously\ntranslated in \"branch\" and you are removing this translation (well, not\nhere, but when 09/10 starts using this code).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271335","messageId":"vpqvbahmc1i.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"1444295885-1657-2-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v2 01/10] ref-filter: implement %(if), %(then), and %(else) atoms","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-08T18:48:41Z","receivedAt":"2015-10-08T18:48:41Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Karthik Nayak writes:\n\n> +static void if_then_else_handler(struct ref_formatting_stack **stack)\n> +{\n> +\tstruct ref_formatting_stack *cur = *stack;\n> +\tstruct ref_formatting_stack *prev = cur->prev;\n> +\tstruct if_then_else *if_then_else = (struct if_then_else *)cur->at_end_data;\n> +\n\nYou should add\n\n\tif (!if_then_else->then_atom)\n\t\tdie(_(\"format: %%(if) atom used without a %%(then) atom\"));\n\nhere ...\n\n> +static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)\n> +{\n> +\tstruct ref_formatting_stack *cur = state->stack;\n> +\tstruct if_then_else *if_then_else = NULL;\n> +\n> +\tif (cur->at_end == if_then_else_handler)\n> +\t\tif_then_else = (struct if_then_else *)cur->at_end_data;\n> +\tif (!if_then_else)\n> +\t\tdie(_(\"format: %%(then) atom used without an %%(if) atom\"));\n> +\tif (if_then_else->then_atom)\n> +\t\tdie(_(\"format: %%(then) atom used more than once\"));\n> +\tif_then_else->then_atom = 1;\n\n... and\n\n\tif (if_then_else->else_atom)\n\t\tdie(_(\"format: %%(then) atom used after %%(else)\"));\n\nhere, just in case (adding the two corresponding test_must_fail wouldn't\nharm of course).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271336","messageId":"vpqpp0pmbxf.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"1444295885-1657-3-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v2 02/10] ref-filter: implement %(if:equals=) and %(if:notequals=)","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-08T18:51:08Z","receivedAt":"2015-10-08T18:51:08Z","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> @@ -309,11 +319,19 @@ static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_st\n> \tif (if_then_else->then_atom)\n> \t\tdie(_(\"format: %%(then) atom used more than once\"));\n> \tif_then_else->then_atom = 1;\n> +\n> \t/*\n\nUseless new blank line.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271337","messageId":"vpqh9m1mbln.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"1444295885-1657-5-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v2 04/10] ref-filter: modify \"%(objectname:short)\" to take length","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-08T18:58:12Z","receivedAt":"2015-10-08T18:58:12Z","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> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -464,14 +464,28 @@ static void *get_obj(const unsigned char *sha1, struct object **obj, unsigned lo\n> static int grab_objectname(const char *name, const unsigned char *sha1,\n> \t\t\t struct atom_value *v)\n> {\n> -\tif (!strcmp(name, \"objectname\")) {\n> -\t\tchar *s = xmalloc(41);\n> -\t\tstrcpy(s, sha1_to_hex(sha1));\n> -\t\tv->s = s;\n> -\t\treturn 1;\n> -\t}\n> -\tif (!strcmp(name, \"objectname:short\")) {\n> -\t\tv->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));\n> +\tconst char *p;\n> +\n> +\tif (match_atom_name(name, \"objectname\", &p)) {\n> +\t\t/* No arguments given, copy the entire objectname */\n> +\t\tif (!p) {\n> +\t\t\tchar *s = xmalloc(41);\n\nWhile you're there, you may want to get rid of this hardcoded 41 and\nwrite GIT_SHA1_HEXSZ + 1 instead.\n\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index 7c9bec7..6fc569e 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -385,6 +385,28 @@ test_expect_success 'Check short objectname format' '\n> \ttest_cmp expected actual\n> '\n> \n> +cat >expected < +$(git rev-parse --short=1 HEAD)\n> +EOF\n\nPlease write all code within test_expect_success including this\n(t/README:\n\n - Put all code inside test_expect_success and other assertions.\n\n Even code that isn't a test per se, but merely some setup code\n should be inside a test assertion.\n).\n\n> +test_expect_success 'Check objectname:short format when size is less than MINIMUM_ABBREV' '\n> +\tgit for-each-ref --format=\"%(objectname:short=1)\" refs/heads >actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> +cat >expected < +$(git rev-parse --short=10 HEAD)\n> +EOF\n\nLikewise.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271338","messageId":"vpq8u7dmbgw.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"1444295885-1657-6-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v2 05/10] ref-filter: adopt get_head_description() from branch.c","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-08T19:01:03Z","receivedAt":"2015-10-08T19:01:03Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Karthik Nayak writes:\n\n> Copy the implementation of get_head_description() from branch.c. This\n> gives a description of the HEAD ref if called. This is used as the\n> refname for the HEAD ref whenever the FILTER_REFS_DETACHED_HEAD option\n> is used. Make it public because we need it to calculate the length of\n> the HEAD refs description in branch.c:calc_maxwidth() when we port\n> branch.c to use ref-filter APIs.\n\nIf it's made public, then it could be simpler to just _move_ the\nfunction instead of copying it. You'd need to add a #include\n to branch.c, but you're going to add one anyway.\n\nCode movement is more \"git blame\" friendly than code copy, and as a\nreviewer I'd rather see the code movement here and not hear about it\nlater in the series.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271342","messageId":"vpqziztkw4o.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"1444295885-1657-9-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v2 08/10] ref-filter: add support for %(upstream:track,nobracket)","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-08T19:17:43Z","receivedAt":"2015-10-08T19:17:43Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Karthik Nayak writes:\n\n> Add support for %(upstream:track,nobracket) which will print the\n> tracking information without the brackets (i.e. \"ahead N, behind M\").\n>\n> Add test and documentation for the same.\n> ---\n> Documentation/git-for-each-ref.txt | 6 ++++--\n> ref-filter.c | 28 +++++++++++++++++++++++-----\n> t/t6300-for-each-ref.sh | 9 +++++++++\n> 3 files changed, 36 insertions(+), 7 deletions(-)\n>\n> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> index c713ec0..a38cbf6 100644\n> --- a/Documentation/git-for-each-ref.txt\n> +++ b/Documentation/git-for-each-ref.txt\n> @@ -114,8 +114,10 @@ upstream::\n> \t`refname` above. Additionally respects `:track` to show\n> \t\"[ahead N, behind M]\" and `:trackshort` to show the terse\n> \tversion: \">\" (ahead), \"<\" (behind), \"<>\" (ahead and behind),\n> -\tor \"=\" (in sync). Has no effect if the ref does not have\n> -\ttracking information associated with it.\n> +\tor \"=\" (in sync). Append `:track,nobracket` to show tracking\n> +\tinformation without brackets (i.e \"ahead N, behind M\"). Has\n> +\tno effect if the ref does not have tracking information\n> +\tassociated with it.\n> \n> push::\n> \tThe name of a local ref which represents the `@{push}` location\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 6a38089..6044eb0 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1112,27 +1112,45 @@ static void populate_value(struct ref_array_item *ref)\n> \t\t\tif (!strcmp(formatp, \"short\"))\n> \t\t\t\trefname = shorten_unambiguous_ref(refname,\n> \t\t\t\t\t\t warn_ambiguous_refs);\n> -\t\t\telse if (!strcmp(formatp, \"track\") &&\n> +\t\t\telse if (skip_prefix(formatp, \"track\", &valp) &&\n> +\t\t\t\t strcmp(formatp, \"trackshort\") &&\n> \t\t\t\t (starts_with(name, \"upstream\") ||\n> \t\t\t\t starts_with(name, \"push\"))) {\n> \t\t\t\tchar buf[40];\n> +\t\t\t\tunsigned int nobracket = 0;\n> +\n> +\t\t\t\tif (!strcmp(valp, \",nobracket\"))\n> +\t\t\t\t\tnobracket = 1;\n> \n> \t\t\t\tif (stat_tracking_info(branch, &num_ours,\n> \t\t\t\t\t\t &num_theirs, NULL)) {\n> -\t\t\t\t\tv->s = \"[gone]\";\n> +\t\t\t\t\tif (nobracket)\n> +\t\t\t\t\t\tv->s = \"gone\";\n> +\t\t\t\t\telse\n> +\t\t\t\t\t\tv->s = \"[gone]\";\n> \t\t\t\t\tcontinue;\n> \t\t\t\t}\n> \n> \t\t\t\tif (!num_ours && !num_theirs)\n> \t\t\t\t\tv->s = \"\";\n> \t\t\t\telse if (!num_ours) {\n> -\t\t\t\t\tsprintf(buf, \"[behind %d]\", num_theirs);\n> +\t\t\t\t\tif (nobracket)\n> +\t\t\t\t\t\tsprintf(buf, \"behind %d\", num_theirs);\n> +\t\t\t\t\telse\n> +\t\t\t\t\t\tsprintf(buf, \"[behind %d]\", num_theirs);\n> \t\t\t\t\tv->s = xstrdup(buf);\n> \t\t\t\t} else if (!num_theirs) {\n> -\t\t\t\t\tsprintf(buf, \"[ahead %d]\", num_ours);\n> +\t\t\t\t\tif (nobracket)\n> +\t\t\t\t\t\tsprintf(buf, \"ahead %d\", num_ours);\n> +\t\t\t\t\telse\n> +\t\t\t\t\t\tsprintf(buf, \"[ahead %d]\", num_ours);\n> \t\t\t\t\tv->s = xstrdup(buf);\n> \t\t\t\t} else {\n> -\t\t\t\t\tsprintf(buf, \"[ahead %d, behind %d]\",\n> +\t\t\t\t\tif (nobracket)\n> +\t\t\t\t\t\tsprintf(buf, \"ahead %d, behind %d\",\n> +\t\t\t\t\t\t\tnum_ours, num_theirs);\n> +\t\t\t\t\telse\n> +\t\t\t\t\t\tsprintf(buf, \"[ahead %d, behind %d]\",\n> \t\t\t\t\t\tnum_ours, num_theirs);\n> \t\t\t\t\tv->s = xstrdup(buf);\n> \t\t\t\t}\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index 4f620bf..7ab8bf8 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -344,6 +344,15 @@ test_expect_success 'Check upstream:track format' '\n> '\n> \n> cat >expected < +ahead 1\n> +EOF\n> +\n> +test_expect_success 'Check upstream:track,nobracket format' '\n> +\tgit for-each-ref --format=\"%(upstream:track,nobracket)\" refs/heads >actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> +cat >expected < >\n> EOF\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271343","messageId":"vpqtwq1kw26.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"vpqziztkw4o.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 08/10] ref-filter: add support for %(upstream:track,nobracket)","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-08T19:19:13Z","receivedAt":"2015-10-08T19:19:13Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Oops, sorry, I sent the wrong message, this one is empty. Please ignore.\n\nMatthieu Moy writes:\n\n> Karthik Nayak writes:\n>\n>> Add support for %(upstream:track,nobracket) which will print the\n>> tracking information without the brackets (i.e. \"ahead N, behind M\").\n>>\n>> Add test and documentation for the same.\n>> ---\n>> Documentation/git-for-each-ref.txt | 6 ++++--\n>> ref-filter.c | 28 +++++++++++++++++++++++-----\n>> t/t6300-for-each-ref.sh | 9 +++++++++\n>> 3 files changed, 36 insertions(+), 7 deletions(-)\n>>\n>> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n>> index c713ec0..a38cbf6 100644\n>> --- a/Documentation/git-for-each-ref.txt\n>> +++ b/Documentation/git-for-each-ref.txt\n>> @@ -114,8 +114,10 @@ upstream::\n>> \t`refname` above. Additionally respects `:track` to show\n>> \t\"[ahead N, behind M]\" and `:trackshort` to show the terse\n>> \tversion: \">\" (ahead), \"<\" (behind), \"<>\" (ahead and behind),\n>> -\tor \"=\" (in sync). Has no effect if the ref does not have\n>> -\ttracking information associated with it.\n>> +\tor \"=\" (in sync). Append `:track,nobracket` to show tracking\n>> +\tinformation without brackets (i.e \"ahead N, behind M\"). Has\n>> +\tno effect if the ref does not have tracking information\n>> +\tassociated with it.\n>> \n>> push::\n>> \tThe name of a local ref which represents the `@{push}` location\n>> diff --git a/ref-filter.c b/ref-filter.c\n>> index 6a38089..6044eb0 100644\n>> --- a/ref-filter.c\n>> +++ b/ref-filter.c\n>> @@ -1112,27 +1112,45 @@ static void populate_value(struct ref_array_item *ref)\n>> \t\t\tif (!strcmp(formatp, \"short\"))\n>> \t\t\t\trefname = shorten_unambiguous_ref(refname,\n>> \t\t\t\t\t\t warn_ambiguous_refs);\n>> -\t\t\telse if (!strcmp(formatp, \"track\") &&\n>> +\t\t\telse if (skip_prefix(formatp, \"track\", &valp) &&\n>> +\t\t\t\t strcmp(formatp, \"trackshort\") &&\n>> \t\t\t\t (starts_with(name, \"upstream\") ||\n>> \t\t\t\t starts_with(name, \"push\"))) {\n>> \t\t\t\tchar buf[40];\n>> +\t\t\t\tunsigned int nobracket = 0;\n>> +\n>> +\t\t\t\tif (!strcmp(valp, \",nobracket\"))\n>> +\t\t\t\t\tnobracket = 1;\n>> \n>> \t\t\t\tif (stat_tracking_info(branch, &num_ours,\n>> \t\t\t\t\t\t &num_theirs, NULL)) {\n>> -\t\t\t\t\tv->s = \"[gone]\";\n>> +\t\t\t\t\tif (nobracket)\n>> +\t\t\t\t\t\tv->s = \"gone\";\n>> +\t\t\t\t\telse\n>> +\t\t\t\t\t\tv->s = \"[gone]\";\n>> \t\t\t\t\tcontinue;\n>> \t\t\t\t}\n>> \n>> \t\t\t\tif (!num_ours && !num_theirs)\n>> \t\t\t\t\tv->s = \"\";\n>> \t\t\t\telse if (!num_ours) {\n>> -\t\t\t\t\tsprintf(buf, \"[behind %d]\", num_theirs);\n>> +\t\t\t\t\tif (nobracket)\n>> +\t\t\t\t\t\tsprintf(buf, \"behind %d\", num_theirs);\n>> +\t\t\t\t\telse\n>> +\t\t\t\t\t\tsprintf(buf, \"[behind %d]\", num_theirs);\n>> \t\t\t\t\tv->s = xstrdup(buf);\n>> \t\t\t\t} else if (!num_theirs) {\n>> -\t\t\t\t\tsprintf(buf, \"[ahead %d]\", num_ours);\n>> +\t\t\t\t\tif (nobracket)\n>> +\t\t\t\t\t\tsprintf(buf, \"ahead %d\", num_ours);\n>> +\t\t\t\t\telse\n>> +\t\t\t\t\t\tsprintf(buf, \"[ahead %d]\", num_ours);\n>> \t\t\t\t\tv->s = xstrdup(buf);\n>> \t\t\t\t} else {\n>> -\t\t\t\t\tsprintf(buf, \"[ahead %d, behind %d]\",\n>> +\t\t\t\t\tif (nobracket)\n>> +\t\t\t\t\t\tsprintf(buf, \"ahead %d, behind %d\",\n>> +\t\t\t\t\t\t\tnum_ours, num_theirs);\n>> +\t\t\t\t\telse\n>> +\t\t\t\t\t\tsprintf(buf, \"[ahead %d, behind %d]\",\n>> \t\t\t\t\t\tnum_ours, num_theirs);\n>> \t\t\t\t\tv->s = xstrdup(buf);\n>> \t\t\t\t}\n>> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n>> index 4f620bf..7ab8bf8 100755\n>> --- a/t/t6300-for-each-ref.sh\n>> +++ b/t/t6300-for-each-ref.sh\n>> @@ -344,6 +344,15 @@ test_expect_success 'Check upstream:track format' '\n>> '\n>> \n>> cat >expected <> +ahead 1\n>> +EOF\n>> +\n>> +test_expect_success 'Check upstream:track,nobracket format' '\n>> +\tgit for-each-ref --format=\"%(upstream:track,nobracket)\" refs/heads >actual &&\n>> +\ttest_cmp expected actual\n>> +'\n>> +\n>> +cat >expected <> >\n>> EOF\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271344","messageId":"vpqsi5lkw1h.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"1444295885-1657-2-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v2 01/10] ref-filter: implement %(if), %(then), and %(else) atoms","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-08T19:19:38Z","receivedAt":"2015-10-08T19:19:38Z","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> --- a/Documentation/git-for-each-ref.txt\n> +++ b/Documentation/git-for-each-ref.txt\n> @@ -134,9 +134,17 @@ align::\n> \t`` is either left, right or middle, default being\n> \tleft and `` is the total length of the content with\n> \talignment. If the contents length is more than the width then\n> -\tno alignment is performed. If used with '--quote' everything\n> -\tin between %(align:...) and %(end) is quoted, but if nested\n> -\tthen only the topmost level performs quoting.\n> +\tno alignment is performed.\n> +\n> +if::\n> +\tUsed as %(if)..%(then)..(%end) or %(if)..%(then)..%(else)..%(end).\n\nI guess you forgot to replace .. with ... (I think you agreed with me\nthat it was better).\n\n> @@ -69,10 +72,16 @@ struct contents {\n> \tstruct object_id oid;\n> };\n> \n> +struct if_then_else {\n> +\tunsigned int then_atom : 1,\n> +\t\telse_atom : 1,\n\nMaybe \"then_atom_seen\" and \"else_atom_seen\" would be better names. Or\nmaybe they'd be too long, I leave it up to you.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271379","messageId":"vpqpp0ojvs6.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"xmqq4mi1rywu.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-09T08:22:49Z","receivedAt":"2015-10-09T08:22:49Z","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> Then used_atom[] could become something like\n>\n> struct {\n> \tconst char *str; /* e.g. \"align:position=left,32\" */\n> \tstruct {\n> \tconst char *part0; /* everything before '=' */\n> const char *part1; /* optional */\n> \t} *modifier;\n> int modifier_nr;\n> } *used_atom;\n\nIf the goal is to prepare as much as possible when parsing the format\nstring, I'd even push it one step further and have stg like\n\n struct {\n \tconst char *str; /* e.g. \"align:position=left,32\" */\n \tunion {\n \tstruct {\n\t\t\tint position;\n\t\t\tenum { left, right, center } kind;\n\t\t} align;\n struct {\n \t....;\n } objectname;\n int modifier_nr;\n } *used_atom;\n\nJust a thought, I'm not sure how useful this would be, and this may be\ntoo much change for this series (so it may deserve a separate topic).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271386","messageId":"xmqqfv1jq4jy.fsf@gitster.mtv.corp.google.com","threadId":"40517","inReplyTo":"vpqpp0ojvs6.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-09T18:29:05Z","receivedAt":"2015-10-09T18:29:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Junio C Hamano writes:\n>\n>> Then used_atom[] could become something like\n>>\n>> struct {\n>> \tconst char *str; /* e.g. \"align:position=left,32\" */\n>> \tstruct {\n>> \tconst char *part0; /* everything before '=' */\n>> const char *part1; /* optional */\n>> \t} *modifier;\n>> int modifier_nr;\n>> } *used_atom;\n>\n> If the goal is to prepare as much as possible when parsing the format\n> string, I'd even push it one step further and have stg like\n>\n> struct {\n> \tconst char *str; /* e.g. \"align:position=left,32\" */\n> \tunion {\n> \tstruct {\n> \t\t\tint position;\n> \t\t\tenum { left, right, center } kind;\n> \t\t} align;\n> struct {\n> \t....;\n> } objectname;\n> int modifier_nr;\n> } *used_atom;\n>\n> Just a thought, I'm not sure how useful this would be, and this may be\n> too much change for this series (so it may deserve a separate topic).\n\nYes, if we are willing to enrich the element of valid_atom[] array\nwith a type-specific parsing functions, we could even do that. Then\npopulate_value() would not have to do any parsing and just do the\nfilling.\n\nI was shooting for a middle ground, but certainly with an eye\ntowards such an endgame state in the future.\n"},{"id":"271398","messageId":"CAOLa=ZSVJnZu2vuhR8breWkiYTQWb2ZxygGOv3QB4kqJ5nBDiA@mail.gmail.com","threadId":"40517","inReplyTo":"vpqvbahmc1i.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 01/10] ref-filter: implement %(if), %(then), and %(else) atoms","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-10T16:22:11Z","receivedAt":"2015-10-10T16:22:11Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Fri, Oct 9, 2015 at 12:18 AM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> +static void if_then_else_handler(struct ref_formatting_stack **stack)\n>> +{\n>> + struct ref_formatting_stack *cur = *stack;\n>> + struct ref_formatting_stack *prev = cur->prev;\n>> + struct if_then_else *if_then_else = (struct if_then_else *)cur->at_end_data;\n>> +\n>\n> You should add\n>\n> if (!if_then_else->then_atom)\n> die(_(\"format: %%(if) atom used without a %%(then) atom\"));\n>\n> here ...\n>\n\nWill do.\n\n>> +static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)\n>> +{\n>> + struct ref_formatting_stack *cur = state->stack;\n>> + struct if_then_else *if_then_else = NULL;\n>> +\n>> + if (cur->at_end == if_then_else_handler)\n>> + if_then_else = (struct if_then_else *)cur->at_end_data;\n>> + if (!if_then_else)\n>> + die(_(\"format: %%(then) atom used without an %%(if) atom\"));\n>> + if (if_then_else->then_atom)\n>> + die(_(\"format: %%(then) atom used more than once\"));\n>> + if_then_else->then_atom = 1;\n>\n> ... and\n>\n> if (if_then_else->else_atom)\n> die(_(\"format: %%(then) atom used after %%(else)\"));\n>\n> here, just in case (adding the two corresponding test_must_fail wouldn't\n> harm of course).\n>\n\nWill do, thanks!\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271400","messageId":"CAOLa=ZTkknFK8ZPEBkuSK2pP3pPoP8MhQ1AumeUqrg6CYp9VWg@mail.gmail.com","threadId":"40517","inReplyTo":"vpqsi5lkw1h.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 01/10] ref-filter: implement %(if), %(then), and %(else) atoms","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-10T17:12:54Z","receivedAt":"2015-10-10T17:12:54Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Fri, Oct 9, 2015 at 12:49 AM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> --- a/Documentation/git-for-each-ref.txt\n>> +++ b/Documentation/git-for-each-ref.txt\n>> @@ -134,9 +134,17 @@ align::\n>> `` is either left, right or middle, default being\n>> left and `` is the total length of the content with\n>> alignment. If the contents length is more than the width then\n>> - no alignment is performed. If used with '--quote' everything\n>> - in between %(align:...) and %(end) is quoted, but if nested\n>> - then only the topmost level performs quoting.\n>> + no alignment is performed.\n>> +\n>> +if::\n>> + Used as %(if)..%(then)..(%end) or %(if)..%(then)..%(else)..%(end).\n>\n> I guess you forgot to replace .. with ... (I think you agreed with me\n> that it was better).\n>\n\nOops! I missed this.\n\n>> @@ -69,10 +72,16 @@ struct contents {\n>> struct object_id oid;\n>> };\n>>\n>> +struct if_then_else {\n>> + unsigned int then_atom : 1,\n>> + else_atom : 1,\n>\n> Maybe \"then_atom_seen\" and \"else_atom_seen\" would be better names. Or\n> maybe they'd be too long, I leave it up to you.\n>\n\nThey do describe the usage in a better way, so why not.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271401","messageId":"CAOLa=ZQaMbqXc8GT_DC5_csjNO-47ksbOoOGnPrBB+JPYvrY_g@mail.gmail.com","threadId":"40517","inReplyTo":"vpqpp0pmbxf.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 02/10] ref-filter: implement %(if:equals=) and %(if:notequals=)","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-10T17:22:30Z","receivedAt":"2015-10-10T17:22:30Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Fri, Oct 9, 2015 at 12:21 AM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> @@ -309,11 +319,19 @@ static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_st\n>> if (if_then_else->then_atom)\n>> die(_(\"format: %%(then) atom used more than once\"));\n>> if_then_else->then_atom = 1;\n>> +\n>> /*\n>\n> Useless new blank line.\n>\n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n\nWill remove it, thanks!\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271402","messageId":"CAOLa=ZT8YydU-4HH2migNewtiAvB=wOCWx2TGV+Y+8YVHCdaNQ@mail.gmail.com","threadId":"40517","inReplyTo":"vpqh9m1mbln.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 04/10] ref-filter: modify \"%(objectname:short)\" to take length","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-10T18:15:46Z","receivedAt":"2015-10-10T18:15:46Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Fri, Oct 9, 2015 at 12:28 AM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> --- a/ref-filter.c\n>> +++ b/ref-filter.c\n>> @@ -464,14 +464,28 @@ static void *get_obj(const unsigned char *sha1, struct object **obj, unsigned lo\n>> static int grab_objectname(const char *name, const unsigned char *sha1,\n>> struct atom_value *v)\n>> {\n>> - if (!strcmp(name, \"objectname\")) {\n>> - char *s = xmalloc(41);\n>> - strcpy(s, sha1_to_hex(sha1));\n>> - v->s = s;\n>> - return 1;\n>> - }\n>> - if (!strcmp(name, \"objectname:short\")) {\n>> - v->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));\n>> + const char *p;\n>> +\n>> + if (match_atom_name(name, \"objectname\", &p)) {\n>> + /* No arguments given, copy the entire objectname */\n>> + if (!p) {\n>> + char *s = xmalloc(41);\n>\n> While you're there, you may want to get rid of this hardcoded 41 and\n> write GIT_SHA1_HEXSZ + 1 instead.\n>\n\nSure, will change that.\n\n>> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n>> index 7c9bec7..6fc569e 100755\n>> --- a/t/t6300-for-each-ref.sh\n>> +++ b/t/t6300-for-each-ref.sh\n>> @@ -385,6 +385,28 @@ test_expect_success 'Check short objectname format' '\n>> test_cmp expected actual\n>> '\n>>\n>> +cat >expected <> +$(git rev-parse --short=1 HEAD)\n>> +EOF\n>\n> Please write all code within test_expect_success including this\n> (t/README:\n>\n> - Put all code inside test_expect_success and other assertions.\n>\n> Even code that isn't a test per se, but merely some setup code\n> should be inside a test assertion.\n> ).\n>\n\nWas just following the previous syntax, should have read that. fixed it\n\n>> +test_expect_success 'Check objectname:short format when size is less than MINIMUM_ABBREV' '\n>> + git for-each-ref --format=\"%(objectname:short=1)\" refs/heads >actual &&\n>> + test_cmp expected actual\n>> +'\n>> +\n>> +cat >expected <> +$(git rev-parse --short=10 HEAD)\n>> +EOF\n>\n> Likewise.\n>\n\nfixed it here too. Thanks\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271403","messageId":"CAOLa=ZQ2vqBiYssW-95ojy7UeKUrzzpP9PpJy9MTEtRL2wovsA@mail.gmail.com","threadId":"40517","inReplyTo":"vpq8u7dmbgw.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 05/10] ref-filter: adopt get_head_description() from branch.c","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-10T18:17:14Z","receivedAt":"2015-10-10T18:17:14Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Fri, Oct 9, 2015 at 12:31 AM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> Copy the implementation of get_head_description() from branch.c. This\n>> gives a description of the HEAD ref if called. This is used as the\n>> refname for the HEAD ref whenever the FILTER_REFS_DETACHED_HEAD option\n>> is used. Make it public because we need it to calculate the length of\n>> the HEAD refs description in branch.c:calc_maxwidth() when we port\n>> branch.c to use ref-filter APIs.\n>\n> If it's made public, then it could be simpler to just _move_ the\n> function instead of copying it. You'd need to add a #include\n> to branch.c, but you're going to add one anyway.\n>\n> Code movement is more \"git blame\" friendly than code copy, and as a\n> reviewer I'd rather see the code movement here and not hear about it\n> later in the series.\n>\n\nAlso this needs to be done, cause now compiling branch.c would give a\nmultiple declaration error. Will do as you suggested. Thanks\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271404","messageId":"CAOLa=ZTvD5cXduPH3G0bGDba_hevLQsuwYsoZFtmfTd2SS4Prw@mail.gmail.com","threadId":"40517","inReplyTo":"vpqa8rtnqzp.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 07/10] ref-filter: make %(upstream:track) prints \"[gone]\" for invalid upstreams","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-10T18:19:10Z","receivedAt":"2015-10-10T18:19:10Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Fri, Oct 9, 2015 at 12:10 AM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> --- a/ref-filter.c\n>> +++ b/ref-filter.c\n>> @@ -1118,8 +1118,10 @@ static void populate_value(struct ref_array_item *ref)\n>> char buf[40];\n>>\n>> if (stat_tracking_info(branch, &num_ours,\n>> - &num_theirs, NULL))\n>> + &num_theirs, NULL)) {\n>> + v->s = \"[gone]\";\n>\n> My remark about translation still holds. The string was previously\n> translated in \"branch\" and you are removing this translation (well, not\n> here, but when 09/10 starts using this code).\n>\n\nI should have mentioned in my cover letter, I didn't really understand\nwhat has to be done about this, couldn't find much reference to go\nabout this. What do you suggest?\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271410","messageId":"CAOLa=ZQkjMFXVeJ==myQLjyRs6EcejnYnszYKJLyskFufjeqiA@mail.gmail.com","threadId":"40517","inReplyTo":"xmqqfv1jq4jy.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-11T12:48:43Z","receivedAt":"2015-10-11T12:48:43Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Fri, Oct 9, 2015 at 11:59 PM, Junio C Hamano wrote:\n> Matthieu Moy writes:\n>\n>> Junio C Hamano writes:\n>>\n>>> Then used_atom[] could become something like\n>>>\n>>> struct {\n>>> const char *str; /* e.g. \"align:position=left,32\" */\n>>> struct {\n>>> const char *part0; /* everything before '=' */\n>>> const char *part1; /* optional */\n>>> } *modifier;\n>>> int modifier_nr;\n>>> } *used_atom;\n>>\n>> If the goal is to prepare as much as possible when parsing the format\n>> string, I'd even push it one step further and have stg like\n>>\n>> struct {\n>> const char *str; /* e.g. \"align:position=left,32\" */\n>> union {\n>> struct {\n>> int position;\n>> enum { left, right, center } kind;\n>> } align;\n>> struct {\n>> ....;\n>> } objectname;\n>> int modifier_nr;\n>> } *used_atom;\n>>\n>> Just a thought, I'm not sure how useful this would be, and this may be\n>> too much change for this series (so it may deserve a separate topic).\n>\n> Yes, if we are willing to enrich the element of valid_atom[] array\n> with a type-specific parsing functions, we could even do that. Then\n> populate_value() would not have to do any parsing and just do the\n> filling.\n>\n> I was shooting for a middle ground, but certainly with an eye\n> towards such an endgame state in the future.\n\nI like the idea's here, I was trying out what you suggested before\nMatthieu suggestion.\n\nWe could have:\n\nstatic void parse_atom_modifiers(struct used_atom *atom)\n{\n const char *sp = NULL;\n\n atom->modifier = NULL;\n atom->modifier_nr = 0;\n\n if ((sp = strchr(atom->str, ':'))) {\n while (sp[1]) {\n const char *equal, *comma, *ep;\n int no = atom->modifier_nr;\n\n atom->modifier_nr++;\n sp++;\n REALLOC_ARRAY(atom->modifier, atom->modifier_nr);\n\n equal = strchr(sp, '=');\n comma = strchr(sp, ',');\n\n if (comma)\n ep = comma;\n else\n ep = sp + strlen(sp);\n\n if (!equal) {\n atom->modifier[no].part0 = xstrndup(sp, ep - sp);\n atom->modifier[no].part1 = NULL;\n } else {\n atom->modifier[no].part0 = xstrndup(sp, equal - sp);\n atom->modifier[no].part1 = xstrndup(equal + 1, ep - equal - 1);\n }\n sp = ep;\n }\n }\n}\n\nor something on these lines for what you suggested. We could improve by\nhaving a special parsing function for selected atoms and leave this to\nbe default.\n\nAlso does it make sense to integrate these changes here? Or would you like to\nhave another series on this?\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271412","messageId":"vpqlhb92xx9.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"CAOLa=ZT8YydU-4HH2migNewtiAvB=wOCWx2TGV+Y+8YVHCdaNQ@mail.gmail.com","subject":"Re: [PATCH v2 04/10] ref-filter: modify \"%(objectname:short)\" to take length","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-11T16:05:22Z","receivedAt":"2015-10-11T16:05:22Z","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>>> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n>>> index 7c9bec7..6fc569e 100755\n>>> --- a/t/t6300-for-each-ref.sh\n>>> +++ b/t/t6300-for-each-ref.sh\n>>> @@ -385,6 +385,28 @@ test_expect_success 'Check short objectname format' '\n>>> test_cmp expected actual\n>>> '\n>>>\n>>> +cat >expected <>> +$(git rev-parse --short=1 HEAD)\n>>> +EOF\n>>\n>> Please write all code within test_expect_success including this\n>> (t/README:\n>>\n>> - Put all code inside test_expect_success and other assertions.\n>>\n>> Even code that isn't a test per se, but merely some setup code\n>> should be inside a test assertion.\n>> ).\n>>\n>\n> Was just following the previous syntax, should have read that. fixed it\n\nThe common practice (not necessarily a rule, though) when you write code\nnext to other code that does not follow the style is:\n\n* If it's not too disturbing, adopt the new style and keep the old code\n as-is. I think we are in this case.\n\n* If the new and the old style do not mix well, prepend a \"modernize\n style\" patch to the series, and adopt the new style in the patch\n itself.\n\n* If you're too lazy to do a \"modernize style\", adopt the old style for\n consistency.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271413","messageId":"vpqa8rp2xku.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"CAOLa=ZTvD5cXduPH3G0bGDba_hevLQsuwYsoZFtmfTd2SS4Prw@mail.gmail.com","subject":"Re: [PATCH v2 07/10] ref-filter: make %(upstream:track) prints \"[gone]\" for invalid upstreams","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-11T16:12:49Z","receivedAt":"2015-10-11T16:12:49Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Karthik Nayak writes:\n\n> On Fri, Oct 9, 2015 at 12:10 AM, Matthieu Moy\n> wrote:\n>> Karthik Nayak writes:\n>>\n>>> --- a/ref-filter.c\n>>> +++ b/ref-filter.c\n>>> @@ -1118,8 +1118,10 @@ static void populate_value(struct ref_array_item *ref)\n>>> char buf[40];\n>>>\n>>> if (stat_tracking_info(branch, &num_ours,\n>>> - &num_theirs, NULL))\n>>> + &num_theirs, NULL)) {\n>>> + v->s = \"[gone]\";\n>>\n>> My remark about translation still holds. The string was previously\n>> translated in \"branch\" and you are removing this translation (well, not\n>> here, but when 09/10 starts using this code).\n>>\n>\n> I should have mentioned in my cover letter, I didn't really understand\n> what has to be done about this, couldn't find much reference to go\n> about this. What do you suggest?\n\n>From the user point of view :\n\ngit for-each-ref --format '%(upstream:track)' => Should always be the\nsame, because this may be parsed by scripts (plumbing). Should not\ndepend on $LANG, and shouldn't change from a version of Git to another.\n\ngit branch --format '%(upstream:track)' => Should show what is most\npleasant to the user (porcelain): translated according to $LANG and\nfriends, and may be improved in the future.\n\nI already pointed out a fix where a string was translated in a plumbing\ncommand. Another example is setup_unpack_trees_porcelain() in\nunpack-trees.c which solves exactly the same problem.\n\nI'll followup with a small series on top of yours to show the way. I did\nnot try to polish it since I guess you have local changes on the same\npart of the code. Feel free to squash patches together or to squash them\nwith yours. The commit messages are not meant to be final either.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271414","messageId":"1444580188-18773-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"40517","inReplyTo":"vpqa8rp2xku.fsf@grenoble-inp.fr","subject":"[PATCH 1/3] fixup: use xstrfmt instead of fixed-size buf + sprintf + xstrdup","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-10-11T16:16:26Z","receivedAt":"2015-10-11T16:16:26Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The char buf[40] is safe (at least while the strings are not\ntranslated), but I'd rather avoid magic numbers like this 40 in the\ncode, and use a construct that does not have this size limitation.\nEspecially if it makes the code shorter.\n\nSigned-off-by: Matthieu Moy \n---\n ref-filter.c | 20 ++++++++------------\n 1 file changed, 8 insertions(+), 12 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 6044eb0..7932c21 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1116,7 +1116,6 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\t\t strcmp(formatp, \"trackshort\") &&\n \t\t\t\t (starts_with(name, \"upstream\") ||\n \t\t\t\t starts_with(name, \"push\"))) {\n-\t\t\t\tchar buf[40];\n \t\t\t\tunsigned int nobracket = 0;\n \n \t\t\t\tif (!strcmp(valp, \",nobracket\"))\n@@ -1135,24 +1134,21 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\t\t\tv->s = \"\";\n \t\t\t\telse if (!num_ours) {\n \t\t\t\t\tif (nobracket)\n-\t\t\t\t\t\tsprintf(buf, \"behind %d\", num_theirs);\n+\t\t\t\t\t\tv->s = xstrfmt(\"behind %d\", num_theirs);\n \t\t\t\t\telse\n-\t\t\t\t\t\tsprintf(buf, \"[behind %d]\", num_theirs);\n-\t\t\t\t\tv->s = xstrdup(buf);\n+\t\t\t\t\t\tv->s = xstrfmt(\"[behind %d]\", num_theirs);\n \t\t\t\t} else if (!num_theirs) {\n \t\t\t\t\tif (nobracket)\n-\t\t\t\t\t\tsprintf(buf, \"ahead %d\", num_ours);\n+\t\t\t\t\t\tv->s = xstrfmt(\"ahead %d\", num_ours);\n \t\t\t\t\telse\n-\t\t\t\t\t\tsprintf(buf, \"[ahead %d]\", num_ours);\n-\t\t\t\t\tv->s = xstrdup(buf);\n+\t\t\t\t\t\tv->s = xstrfmt(\"[ahead %d]\", num_ours);\n \t\t\t\t} else {\n \t\t\t\t\tif (nobracket)\n-\t\t\t\t\t\tsprintf(buf, \"ahead %d, behind %d\",\n-\t\t\t\t\t\t\tnum_ours, num_theirs);\n+\t\t\t\t\t\tv->s = xstrfmt(\"ahead %d, behind %d\",\n+\t\t\t\t\t\t\t num_ours, num_theirs);\n \t\t\t\t\telse\n-\t\t\t\t\t\tsprintf(buf, \"[ahead %d, behind %d]\",\n-\t\t\t\t\t\tnum_ours, num_theirs);\n-\t\t\t\t\tv->s = xstrdup(buf);\n+\t\t\t\t\t\tv->s = xstrfmt(\"[ahead %d, behind %d]\",\n+\t\t\t\t\t\t\t num_ours, num_theirs);\n \t\t\t\t}\n \t\t\t\tcontinue;\n \t\t\t} else if (!strcmp(formatp, \"trackshort\") &&\n-- \n2.6.0.rc2.24.gb06d8e9.dirty\n"},{"id":"271416","messageId":"1444580188-18773-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"40517","inReplyTo":"1444580188-18773-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 2/3] ref-filter: allow porcelain to translate messages in the output","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-10-11T16:16:27Z","receivedAt":"2015-10-11T16:16:27Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"This patch shows the way, but is obviously incomplete as it works only\nfor \"nobracket\" version. Actually, I think the code should first build\nthe unbracketed output string and then do something like\n\nif (!nobracket) {\n\tconst char *to_free = v->s;\n\tv->s = xstrfmt(\"[%s]\", v->s);\n\tfree(to_free);\n}\n\nso we don't have to worry about brackets anywhere else in the code.\n\nSigned-off-by: Matthieu Moy \n---\n ref-filter.c | 28 ++++++++++++++++++++++++----\n ref-filter.h | 3 +++\n 2 files changed, 27 insertions(+), 4 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 7932c21..c2ee8c9 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -15,6 +15,26 @@\n #include \"version.h\"\n #include \"wt-status.h\"\n \n+static struct ref_msg {\n+\tconst char *gone;\n+\tconst char *ahead;\n+\tconst char *behind;\n+\tconst char *ahead_behind;\n+} msgs = {\n+\t\"gone\",\n+\t\"ahead %d\",\n+\t\"behind %d\",\n+\t\"ahead %d, behind %d\"\n+};\n+\n+void setup_ref_filter_porcelain_msg(void)\n+{\n+\tmsgs.gone = _(\"gone\");\n+\tmsgs.ahead = _(\"ahead %d\");\n+\tmsgs.behind = _(\"behind %d\");\n+\tmsgs.ahead_behind = _(\"ahead %d, behind %d\");\n+}\n+\n typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n \n static struct {\n@@ -1124,7 +1144,7 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\t\tif (stat_tracking_info(branch, &num_ours,\n \t\t\t\t\t\t &num_theirs, NULL)) {\n \t\t\t\t\tif (nobracket)\n-\t\t\t\t\t\tv->s = \"gone\";\n+\t\t\t\t\t\tv->s = msgs.gone;\n \t\t\t\t\telse\n \t\t\t\t\t\tv->s = \"[gone]\";\n \t\t\t\t\tcontinue;\n@@ -1134,17 +1154,17 @@ static void populate_value(struct ref_array_item *ref)\n \t\t\t\t\tv->s = \"\";\n \t\t\t\telse if (!num_ours) {\n \t\t\t\t\tif (nobracket)\n-\t\t\t\t\t\tv->s = xstrfmt(\"behind %d\", num_theirs);\n+\t\t\t\t\t\tv->s = xstrfmt(msgs.behind, num_theirs);\n \t\t\t\t\telse\n \t\t\t\t\t\tv->s = xstrfmt(\"[behind %d]\", num_theirs);\n \t\t\t\t} else if (!num_theirs) {\n \t\t\t\t\tif (nobracket)\n-\t\t\t\t\t\tv->s = xstrfmt(\"ahead %d\", num_ours);\n+\t\t\t\t\t\tv->s = xstrfmt(msgs.ahead, num_ours);\n \t\t\t\t\telse\n \t\t\t\t\t\tv->s = xstrfmt(\"[ahead %d]\", num_ours);\n \t\t\t\t} else {\n \t\t\t\t\tif (nobracket)\n-\t\t\t\t\t\tv->s = xstrfmt(\"ahead %d, behind %d\",\n+\t\t\t\t\t\tv->s = xstrfmt(msgs.ahead_behind,\n \t\t\t\t\t\t\t num_ours, num_theirs);\n \t\t\t\t\telse\n \t\t\t\t\t\tv->s = xstrfmt(\"[ahead %d, behind %d]\",\ndiff --git a/ref-filter.h b/ref-filter.h\nindex 0014b92..2cce02c 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -112,4 +112,7 @@ int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset)\n /* Get the current HEAD's description */\n char *get_head_description(void);\n \n+/* Set up translated strings in the output. */\n+void setup_ref_filter_porcelain_msg(void);\n+\n #endif /* REF_FILTER_H */\n-- \n2.6.0.rc2.24.gb06d8e9.dirty\n"},{"id":"271415","messageId":"1444580188-18773-3-git-send-email-Matthieu.Moy@imag.fr","threadId":"40517","inReplyTo":"1444580188-18773-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 3/3] branch, tag: use porcelain output","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-10-11T16:16:28Z","receivedAt":"2015-10-11T16:16:28Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Signed-off-by: Matthieu Moy \n---\n builtin/branch.c | 2 ++\n builtin/tag.c | 2 ++\n 2 files changed, 4 insertions(+)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 9d6c062..041c649 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -531,6 +531,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_END(),\n \t};\n \n+\tsetup_ref_filter_porcelain_msg();\n+\n \tmemset(&filter, 0, sizeof(filter));\n \tfilter.kind = FILTER_REFS_BRANCHES;\n \tfilter.abbrev = -1;\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 9e17dca..b6d262f 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -372,6 +372,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \n+\tsetup_ref_filter_porcelain_msg();\n+\n \tgit_config(git_tag_config, sorting_tail);\n \n \tmemset(&opt, 0, sizeof(opt));\n-- \n2.6.0.rc2.24.gb06d8e9.dirty\n"},{"id":"271417","messageId":"vpq4mhx2x5z.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"CAOLa=ZQkjMFXVeJ==myQLjyRs6EcejnYnszYKJLyskFufjeqiA@mail.gmail.com","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-11T16:21:44Z","receivedAt":"2015-10-11T16:21:44Z","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> Also does it make sense to integrate these changes here? Or would you like to\n> have another series on this?\n\nTo me, the important in this series is to avoid introducing duplicated\nand inconsistent code, because it would make further refactoring harder.\n\nBut this series starts getting close to finished, so you should not try\nto add to much to it (at least for me: I spend some time reviewing v2\nand I have an idea of what the interdiff should look like, I'd rather\navoid having new distraction in the v2->v3 interdiff).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271419","messageId":"CAOLa=ZSQMtW8Kd-B7u=9nJ0d3AtN7f1mtz0k0dS9WKmWSv5CNQ@mail.gmail.com","threadId":"40517","inReplyTo":"vpqlhb92xx9.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 04/10] ref-filter: modify \"%(objectname:short)\" to take length","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-11T17:44:56Z","receivedAt":"2015-10-11T17:44:56Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sun, Oct 11, 2015 at 9:35 PM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>>>> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n>>>> index 7c9bec7..6fc569e 100755\n>>>> --- a/t/t6300-for-each-ref.sh\n>>>> +++ b/t/t6300-for-each-ref.sh\n>>>> @@ -385,6 +385,28 @@ test_expect_success 'Check short objectname format' '\n>>>> test_cmp expected actual\n>>>> '\n>>>>\n>>>> +cat >expected <>>> +$(git rev-parse --short=1 HEAD)\n>>>> +EOF\n>>>\n>>> Please write all code within test_expect_success including this\n>>> (t/README:\n>>>\n>>> - Put all code inside test_expect_success and other assertions.\n>>>\n>>> Even code that isn't a test per se, but merely some setup code\n>>> should be inside a test assertion.\n>>> ).\n>>>\n>>\n>> Was just following the previous syntax, should have read that. fixed it\n>\n> The common practice (not necessarily a rule, though) when you write code\n> next to other code that does not follow the style is:\n>\n> * If it's not too disturbing, adopt the new style and keep the old code\n> as-is. I think we are in this case.\n>\n> * If the new and the old style do not mix well, prepend a \"modernize\n> style\" patch to the series, and adopt the new style in the patch\n> itself.\n>\n> * If you're too lazy to do a \"modernize style\", adopt the old style for\n> consistency.\n>\n\nAh thanks, Makes sense :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271420","messageId":"CAOLa=ZRvTUpz2Ey9ZLwe_iqVGPaFZGJ2_HiASpn=txAgV57otg@mail.gmail.com","threadId":"40517","inReplyTo":"vpqa8rp2xku.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 07/10] ref-filter: make %(upstream:track) prints \"[gone]\" for invalid upstreams","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-11T17:46:48Z","receivedAt":"2015-10-11T17:46:48Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sun, Oct 11, 2015 at 9:42 PM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> On Fri, Oct 9, 2015 at 12:10 AM, Matthieu Moy\n>> wrote:\n>>> Karthik Nayak writes:\n>>>\n>>>> --- a/ref-filter.c\n>>>> +++ b/ref-filter.c\n>>>> @@ -1118,8 +1118,10 @@ static void populate_value(struct ref_array_item *ref)\n>>>> char buf[40];\n>>>>\n>>>> if (stat_tracking_info(branch, &num_ours,\n>>>> - &num_theirs, NULL))\n>>>> + &num_theirs, NULL)) {\n>>>> + v->s = \"[gone]\";\n>>>\n>>> My remark about translation still holds. The string was previously\n>>> translated in \"branch\" and you are removing this translation (well, not\n>>> here, but when 09/10 starts using this code).\n>>>\n>>\n>> I should have mentioned in my cover letter, I didn't really understand\n>> what has to be done about this, couldn't find much reference to go\n>> about this. What do you suggest?\n>\n> From the user point of view :\n>\n> git for-each-ref --format '%(upstream:track)' => Should always be the\n> same, because this may be parsed by scripts (plumbing). Should not\n> depend on $LANG, and shouldn't change from a version of Git to another.\n>\n\nA little blurry on how this works, as in how translation takes place,\nprobably need to look at some code.\n\n> git branch --format '%(upstream:track)' => Should show what is most\n> pleasant to the user (porcelain): translated according to $LANG and\n> friends, and may be improved in the future.\n>\n> I already pointed out a fix where a string was translated in a plumbing\n> command. Another example is setup_unpack_trees_porcelain() in\n> unpack-trees.c which solves exactly the same problem.\n>\n\nYou did. I was just too clueless.\n\n> I'll followup with a small series on top of yours to show the way. I did\n> not try to polish it since I guess you have local changes on the same\n> part of the code. Feel free to squash patches together or to squash them\n> with yours. The commit messages are not meant to be final either.\n>\n\nThanks a lot for this. This should help me out. I'll probably squash them along.\nThanks :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271425","messageId":"vpqvbadz37a.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"CAOLa=ZRvTUpz2Ey9ZLwe_iqVGPaFZGJ2_HiASpn=txAgV57otg@mail.gmail.com","subject":"Re: [PATCH v2 07/10] ref-filter: make %(upstream:track) prints \"[gone]\" for invalid upstreams","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-11T18:10:17Z","receivedAt":"2015-10-11T18:10:17Z","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> A little blurry on how this works, as in how translation takes place,\n> probably need to look at some code.\n\nWhat you really need to understand is: _(\"foo\") is translated, \"foo\" is\nnot and will always be \"foo\". Technically, _ is a macro, it could be\ncalled get_the_translated_string_for(...) but that would be too long.\n\nIn git, _(\"foo\") should be used when talking to a user (porcelain), and\n\"foo\" when talking to a program (plumbing). This way a user running\n\ngit | grep \"some-plumbing-message\"\n\nwill always get the same result regardless of the current locale.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271426","messageId":"CAOLa=ZTjuuO_TmdynQvJGVtQHZokwDpqZLUOWgCx-_UO=JKvvw@mail.gmail.com","threadId":"40517","inReplyTo":"vpq4mhx2x5z.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-11T18:38:10Z","receivedAt":"2015-10-11T18:38:10Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sun, Oct 11, 2015 at 9:51 PM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> Also does it make sense to integrate these changes here? Or would you like to\n>> have another series on this?\n>\n> To me, the important in this series is to avoid introducing duplicated\n> and inconsistent code, because it would make further refactoring harder.\n>\n\nWould you suggest duplicating whats done with %(align) here?\n\n> But this series starts getting close to finished, so you should not try\n> to add to much to it (at least for me: I spend some time reviewing v2\n> and I have an idea of what the interdiff should look like, I'd rather\n> avoid having new distraction in the v2->v3 interdiff).\n>\n\nYes! makes sense, thanks :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271428","messageId":"CAOLa=ZTdXLWqf675M-7mcAQsPvLRN5r9E6yxa-wkwVNKWe5y8Q@mail.gmail.com","threadId":"40517","inReplyTo":"1444580188-18773-2-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 2/3] ref-filter: allow porcelain to translate messages in the output","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-11T19:25:16Z","receivedAt":"2015-10-11T19:25:16Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sun, Oct 11, 2015 at 9:46 PM, Matthieu Moy wrote:\n> This patch shows the way, but is obviously incomplete as it works only\n> for \"nobracket\" version. Actually, I think the code should first build\n> the unbracketed output string and then do something like\n>\n> if (!nobracket) {\n> const char *to_free = v->s;\n> v->s = xstrfmt(\"[%s]\", v->s);\n> free(to_free);\n> }\n>\n\nMakes sense.\n\n> so we don't have to worry about brackets anywhere else in the code.\n>\n> Signed-off-by: Matthieu Moy \n> ---\n> ref-filter.c | 28 ++++++++++++++++++++++++----\n> ref-filter.h | 3 +++\n> 2 files changed, 27 insertions(+), 4 deletions(-)\n>\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 7932c21..c2ee8c9 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -15,6 +15,26 @@\n> #include \"version.h\"\n> #include \"wt-status.h\"\n>\n> +static struct ref_msg {\n> + const char *gone;\n> + const char *ahead;\n> + const char *behind;\n> + const char *ahead_behind;\n> +} msgs = {\n> + \"gone\",\n> + \"ahead %d\",\n> + \"behind %d\",\n> + \"ahead %d, behind %d\"\n> +};\n> +\n> +void setup_ref_filter_porcelain_msg(void)\n> +{\n> + msgs.gone = _(\"gone\");\n> + msgs.ahead = _(\"ahead %d\");\n> + msgs.behind = _(\"behind %d\");\n> + msgs.ahead_behind = _(\"ahead %d, behind %d\");\n> +}\n> +\n> typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n>\n> static struct {\n> @@ -1124,7 +1144,7 @@ static void populate_value(struct ref_array_item *ref)\n> if (stat_tracking_info(branch, &num_ours,\n> &num_theirs, NULL)) {\n> if (nobracket)\n> - v->s = \"gone\";\n> + v->s = msgs.gone;\n> else\n> v->s = \"[gone]\";\n> continue;\n> @@ -1134,17 +1154,17 @@ static void populate_value(struct ref_array_item *ref)\n> v->s = \"\";\n> else if (!num_ours) {\n> if (nobracket)\n> - v->s = xstrfmt(\"behind %d\", num_theirs);\n> + v->s = xstrfmt(msgs.behind, num_theirs);\n> else\n> v->s = xstrfmt(\"[behind %d]\", num_theirs);\n> } else if (!num_theirs) {\n> if (nobracket)\n> - v->s = xstrfmt(\"ahead %d\", num_ours);\n> + v->s = xstrfmt(msgs.ahead, num_ours);\n> else\n> v->s = xstrfmt(\"[ahead %d]\", num_ours);\n> } else {\n> if (nobracket)\n> - v->s = xstrfmt(\"ahead %d, behind %d\",\n> + v->s = xstrfmt(msgs.ahead_behind,\n> num_ours, num_theirs);\n> else\n> v->s = xstrfmt(\"[ahead %d, behind %d]\",\n> diff --git a/ref-filter.h b/ref-filter.h\n> index 0014b92..2cce02c 100644\n> --- a/ref-filter.h\n> +++ b/ref-filter.h\n> @@ -112,4 +112,7 @@ int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset)\n> /* Get the current HEAD's description */\n> char *get_head_description(void);\n>\n> +/* Set up translated strings in the output. */\n> +void setup_ref_filter_porcelain_msg(void);\n> +\n> #endif /* REF_FILTER_H */\n> --\n> 2.6.0.rc2.24.gb06d8e9.dirty\n>\n\nI'm squashing this and the other two patches into my series. Thanks :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271429","messageId":"vpqlhb9yzeg.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"CAOLa=ZTjuuO_TmdynQvJGVtQHZokwDpqZLUOWgCx-_UO=JKvvw@mail.gmail.com","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-11T19:32:23Z","receivedAt":"2015-10-11T19:32:23Z","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 Sun, Oct 11, 2015 at 9:51 PM, Matthieu Moy\n> wrote:\n>> Karthik Nayak writes:\n>>\n>>> Also does it make sense to integrate these changes here? Or would you like to\n>>> have another series on this?\n>>\n>> To me, the important in this series is to avoid introducing duplicated\n>> and inconsistent code, because it would make further refactoring harder.\n>>\n>\n> Would you suggest duplicating whats done with %(align) here?\n\nI think introducing a function to split according to commas and remove\ncommas would make sense, but I won't insist on that. Mimicking what's\ndone with %(align) is acceptable to me: we'll have several instances of\nthe same pattern, not ideal but easy enough to refactor later.\nEspecially if you actually plan to work on that :-).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271430","messageId":"CAOLa=ZSjGwTrS=bMSMtRTg=f5D_E-_xHCu4JYuZtqtDEMvUKVg@mail.gmail.com","threadId":"40517","inReplyTo":"vpqlhb9yzeg.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-11T19:38:17Z","receivedAt":"2015-10-11T19:38:17Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Oct 12, 2015 at 1:02 AM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> On Sun, Oct 11, 2015 at 9:51 PM, Matthieu Moy\n>> wrote:\n>>> Karthik Nayak writes:\n>>>\n>>>> Also does it make sense to integrate these changes here? Or would you like to\n>>>> have another series on this?\n>>>\n>>> To me, the important in this series is to avoid introducing duplicated\n>>> and inconsistent code, because it would make further refactoring harder.\n>>>\n>>\n>> Would you suggest duplicating whats done with %(align) here?\n>\n> I think introducing a function to split according to commas and remove\n> commas would make sense, but I won't insist on that. Mimicking what's\n> done with %(align) is acceptable to me: we'll have several instances of\n> the same pattern, not ideal but easy enough to refactor later.\n> Especially if you actually plan to work on that :-).\n>\n\nI was planning on working on what Junio and you suggested after this ;-).\nSo I didn't see the need of \" introducing a function to split\naccording to commas\"\nif we plan to rewrite the whole parsing part of ref-filter.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271431","messageId":"CAOLa=ZT5ADOY0x_rFRWVx5DtWtOBH3MvqBgh8v4COC1a1ND3NA@mail.gmail.com","threadId":"40517","inReplyTo":"vpqvbadz37a.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 07/10] ref-filter: make %(upstream:track) prints \"[gone]\" for invalid upstreams","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-11T19:38:59Z","receivedAt":"2015-10-11T19:38:59Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sun, Oct 11, 2015 at 11:40 PM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> A little blurry on how this works, as in how translation takes place,\n>> probably need to look at some code.\n>\n> What you really need to understand is: _(\"foo\") is translated, \"foo\" is\n> not and will always be \"foo\". Technically, _ is a macro, it could be\n> called get_the_translated_string_for(...) but that would be too long.\n>\n> In git, _(\"foo\") should be used when talking to a user (porcelain), and\n> \"foo\" when talking to a program (plumbing). This way a user running\n>\n> git | grep \"some-plumbing-message\"\n>\n> will always get the same result regardless of the current locale.\n\nThat helped a lot, thanks a bunch.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271433","messageId":"xmqqbnc4ord4.fsf@gitster.mtv.corp.google.com","threadId":"40517","inReplyTo":"CAOLa=ZQkjMFXVeJ==myQLjyRs6EcejnYnszYKJLyskFufjeqiA@mail.gmail.com","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-12T00:36:07Z","receivedAt":"2015-10-12T00:36:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> On Fri, Oct 9, 2015 at 11:59 PM, Junio C Hamano wrote:\n> ...\n> Also does it make sense to integrate these changes here? Or would you like to\n> have another series on this?\n\nI do not think you would want to ask that question, as my answer\nwould most likely be \"The most preferable would be a series to clean\nup the existing codepath that deals with %(align) first, on top of\nwhich everything in flight that is not yet in 'next' is rebased.\"\n"},{"id":"271461","messageId":"xmqqk2qsm14x.fsf@gitster.mtv.corp.google.com","threadId":"40517","inReplyTo":"1444580188-18773-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 1/3] fixup: use xstrfmt instead of fixed-size buf + sprintf + xstrdup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-12T17:45:34Z","receivedAt":"2015-10-12T17:45:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> The char buf[40] is safe (at least while the strings are not\n> translated), but I'd rather avoid magic numbers like this 40 in the\n> code, and use a construct that does not have this size limitation.\n> Especially if it makes the code shorter.\n>\n> Signed-off-by: Matthieu Moy \n> ---\n\nThe construct being fixed with this change looks very similar to\nPeff's a5e03bf5 (ref-filter: drop sprintf and strcpy calls,\n2015-09-24) on jk/war-on-sprintf topic, but the new code since that\ncommit cleaned up.\n\nI'd expect that this will be rolled into Karthik's series in the\nnext reroll? \n\nLooking good. Thanks.\n\n> ref-filter.c | 20 ++++++++------------\n> 1 file changed, 8 insertions(+), 12 deletions(-)\n>\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 6044eb0..7932c21 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1116,7 +1116,6 @@ static void populate_value(struct ref_array_item *ref)\n> \t\t\t\t strcmp(formatp, \"trackshort\") &&\n> \t\t\t\t (starts_with(name, \"upstream\") ||\n> \t\t\t\t starts_with(name, \"push\"))) {\n> -\t\t\t\tchar buf[40];\n> \t\t\t\tunsigned int nobracket = 0;\n> \n> \t\t\t\tif (!strcmp(valp, \",nobracket\"))\n> @@ -1135,24 +1134,21 @@ static void populate_value(struct ref_array_item *ref)\n> \t\t\t\t\tv->s = \"\";\n> \t\t\t\telse if (!num_ours) {\n> \t\t\t\t\tif (nobracket)\n> -\t\t\t\t\t\tsprintf(buf, \"behind %d\", num_theirs);\n> +\t\t\t\t\t\tv->s = xstrfmt(\"behind %d\", num_theirs);\n> \t\t\t\t\telse\n> -\t\t\t\t\t\tsprintf(buf, \"[behind %d]\", num_theirs);\n> -\t\t\t\t\tv->s = xstrdup(buf);\n> +\t\t\t\t\t\tv->s = xstrfmt(\"[behind %d]\", num_theirs);\n> \t\t\t\t} else if (!num_theirs) {\n> \t\t\t\t\tif (nobracket)\n> -\t\t\t\t\t\tsprintf(buf, \"ahead %d\", num_ours);\n> +\t\t\t\t\t\tv->s = xstrfmt(\"ahead %d\", num_ours);\n> \t\t\t\t\telse\n> -\t\t\t\t\t\tsprintf(buf, \"[ahead %d]\", num_ours);\n> -\t\t\t\t\tv->s = xstrdup(buf);\n> +\t\t\t\t\t\tv->s = xstrfmt(\"[ahead %d]\", num_ours);\n> \t\t\t\t} else {\n> \t\t\t\t\tif (nobracket)\n> -\t\t\t\t\t\tsprintf(buf, \"ahead %d, behind %d\",\n> -\t\t\t\t\t\t\tnum_ours, num_theirs);\n> +\t\t\t\t\t\tv->s = xstrfmt(\"ahead %d, behind %d\",\n> +\t\t\t\t\t\t\t num_ours, num_theirs);\n> \t\t\t\t\telse\n> -\t\t\t\t\t\tsprintf(buf, \"[ahead %d, behind %d]\",\n> -\t\t\t\t\t\tnum_ours, num_theirs);\n> -\t\t\t\t\tv->s = xstrdup(buf);\n> +\t\t\t\t\t\tv->s = xstrfmt(\"[ahead %d, behind %d]\",\n> +\t\t\t\t\t\t\t num_ours, num_theirs);\n> \t\t\t\t}\n> \t\t\t\tcontinue;\n> \t\t\t} else if (!strcmp(formatp, \"trackshort\") &&\n"},{"id":"271462","messageId":"CAOLa=ZT5AUAOgHNhX3AwpY20AZkm39=-JVQjUCgb0_x6LTHXaA@mail.gmail.com","threadId":"40517","inReplyTo":"xmqqbnc4ord4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-12T17:48:05Z","receivedAt":"2015-10-12T17:48:05Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Oct 12, 2015 at 6:06 AM, Junio C Hamano wrote:\n> Karthik Nayak writes:\n>\n>> On Fri, Oct 9, 2015 at 11:59 PM, Junio C Hamano wrote:\n>> ...\n>> Also does it make sense to integrate these changes here? Or would you like to\n>> have another series on this?\n>\n> I do not think you would want to ask that question, as my answer\n> would most likely be \"The most preferable would be a series to clean\n> up the existing codepath that deals with %(align) first, on top of\n> which everything in flight that is not yet in 'next' is rebased.\"\n>\n\nAh, but I might take a while to get there, So I'd rather push code which\nis almost ready and work on that slowly, if that's ok?\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271465","messageId":"CAOLa=ZTFrZK5B8ddVzkWQS-JbtrgrYrHRXjvaf0j8d+6p+LivQ@mail.gmail.com","threadId":"40517","inReplyTo":"xmqqk2qsm14x.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/3] fixup: use xstrfmt instead of fixed-size buf + sprintf + xstrdup","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-12T18:01:24Z","receivedAt":"2015-10-12T18:01:24Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Oct 12, 2015 at 11:15 PM, Junio C Hamano wrote:\n> Matthieu Moy writes:\n>\n>> The char buf[40] is safe (at least while the strings are not\n>> translated), but I'd rather avoid magic numbers like this 40 in the\n>> code, and use a construct that does not have this size limitation.\n>> Especially if it makes the code shorter.\n>>\n>> Signed-off-by: Matthieu Moy \n>> ---\n>\n> The construct being fixed with this change looks very similar to\n> Peff's a5e03bf5 (ref-filter: drop sprintf and strcpy calls,\n> 2015-09-24) on jk/war-on-sprintf topic, but the new code since that\n> commit cleaned up.\n>\n\nYes, pretty much the same.\n\n> I'd expect that this will be rolled into Karthik's series in the\n> next reroll?\n>\n> Looking good. Thanks.\n\nYes, I've adding this into my series.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271466","messageId":"vpq7fms9cjs.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"CAOLa=ZT5AUAOgHNhX3AwpY20AZkm39=-JVQjUCgb0_x6LTHXaA@mail.gmail.com","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-12T18:17:27Z","receivedAt":"2015-10-12T18:17:27Z","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, Oct 12, 2015 at 6:06 AM, Junio C Hamano wrote:\n>> Karthik Nayak writes:\n>>\n>>> On Fri, Oct 9, 2015 at 11:59 PM, Junio C Hamano wrote:\n>>> ...\n>>> Also does it make sense to integrate these changes here? Or would you like to\n>>> have another series on this?\n>>\n>> I do not think you would want to ask that question, as my answer\n>> would most likely be \"The most preferable would be a series to clean\n>> up the existing codepath that deals with %(align) first, on top of\n>> which everything in flight that is not yet in 'next' is rebased.\"\n>\n> Ah, but I might take a while to get there, So I'd rather push code which\n> is almost ready and work on that slowly, if that's ok?\n\nThat's OK to me. The \"most preferable way\" above would lead to a cleaner\nhistory, but also more work for you and for me as a reviewer.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271477","messageId":"xmqq37xfncak.fsf@gitster.mtv.corp.google.com","threadId":"40517","inReplyTo":"vpq7fms9cjs.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-12T18:59:15Z","receivedAt":"2015-10-12T18:59:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Karthik Nayak writes:\n>\n>> On Mon, Oct 12, 2015 at 6:06 AM, Junio C Hamano wrote:\n>>> Karthik Nayak writes:\n>>>\n>>>> On Fri, Oct 9, 2015 at 11:59 PM, Junio C Hamano wrote:\n>>>> ...\n>>>> Also does it make sense to integrate these changes here? Or would you like to\n>>>> have another series on this?\n>>>\n>>> I do not think you would want to ask that question, as my answer\n>>> would most likely be \"The most preferable would be a series to clean\n>>> up the existing codepath that deals with %(align) first, on top of\n>>> which everything in flight that is not yet in 'next' is rebased.\"\n>>\n>> Ah, but I might take a while to get there, So I'd rather push code which\n>> is almost ready and work on that slowly, if that's ok?\n>\n> That's OK to me. The \"most preferable way\" above would lead to a cleaner\n> history, but also more work for you and for me as a reviewer.\n\nI do not think the cleanliness of the resulting history is of prime\nconcern. At least, that was not where my preference came from.\n\nIf you design a new infrastructure to help refactoring early\n(i.e. before adding many copies of code that need to be cleaned up\nlater), it would make the work of reviewing of the design of the\nhelper and refactoring using that helper smaller, not larger. And\nthe new codepaths that use the helper would become easier to follow\n(otherwise we wouldn't be doing such a refactoring in the first\nplace), making the reviews easier. That is where my preference\ncomes from.\n\nThere _is_ a sunk-cost that has already been invested in reviewing\nthe older round that added code that needs to be cleaned up; there\nare some parts other than the \"need to be cleaned-up\" parts that we\nwould feel confortable having in the reroll without having to\nre-review them. I do not know if that is a very high cost, though,\nespecially for those who have already seen the previous rounds.\n\nAnyway, I wouldn't insist and we can go either way.\n"},{"id":"271478","messageId":"vpq612bzz0o.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"xmqq37xfncak.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-12T19:07:35Z","receivedAt":"2015-10-12T19:07:35Z","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> If you design a new infrastructure to help refactoring early\n> (i.e. before adding many copies of code that need to be cleaned up\n> later), it would make the work of reviewing of the design of the\n> helper and refactoring using that helper smaller, not larger.\n\nBut most of the code concerned is already reviewed. The first instances\nof the pattern to refactor is already in next. With a real time machine,\nwe could go back in past, refactor and then have cleaner series, but\nwith Git as our only tool we can't ;-).\n\nThe current series will just add one more instance of sub-optimal code,\nit isn't hard to review. Inserting new code before them would make the\ninterdiff far bigger.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271579","messageId":"xmqqr3kyafxa.fsf@gitster.mtv.corp.google.com","threadId":"40517","inReplyTo":"1444295885-1657-10-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v2 09/10] branch: use ref-filter printing APIs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-13T16:31:29Z","receivedAt":"2015-10-13T16:31:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> Port branch.c to use ref-filter APIs for printing. This clears out\n> most of the code used in branch.c for printing and replaces them with\n> calls made to the ref-filter library.\n>\n> Introduce get_format() which gets the format required for printing of\n> refs. Make amendments to print_ref_list() to reflect these changes.\n\nIs it get_format() still?\n\n>\n> Change calc_maxwidth() to also account for the length of HEAD ref, by\n> calling ref-filter:get_head_discription().\n>\n> Mentored-by: Christian Couder \n> Mentored-by: Matthieu Moy \n> Signed-off-by: Karthik Nayak \n> ---\n> builtin/branch.c | 261 ++++++++++++-----------------------------------\n> t/t6040-tracking-info.sh | 2 +-\n> 2 files changed, 66 insertions(+), 197 deletions(-)\n>\n> diff --git a/builtin/branch.c b/builtin/branch.c\n> index 67ef9f1..4d8b570 100644\n> --- a/builtin/branch.c\n> +++ b/builtin/branch.c\n> @@ -35,12 +35,12 @@ static unsigned char head_sha1[20];\n> \n> static int branch_use_color = -1;\n> static char branch_colors[][COLOR_MAXLEN] = {\n> -\tGIT_COLOR_RESET,\n> -\tGIT_COLOR_NORMAL,\t/* PLAIN */\n> -\tGIT_COLOR_RED,\t\t/* REMOTE */\n> -\tGIT_COLOR_NORMAL,\t/* LOCAL */\n> -\tGIT_COLOR_GREEN,\t/* CURRENT */\n> -\tGIT_COLOR_BLUE,\t\t/* UPSTREAM */\n> +\t\"%(color:reset)\",\n> +\t\"%(color:reset)\",\t/* PLAIN */\n> +\t\"%(color:red)\",\t\t/* REMOTE */\n> +\t\"%(color:reset)\",\t/* LOCAL */\n> +\t\"%(color:green)\",\t/* CURRENT */\n> +\t\"%(color:blue)\",\t/* UPSTREAM */\n> };\n> enum color_branch {\n> \tBRANCH_COLOR_RESET = 0,\n> @@ -271,192 +271,6 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n> \treturn(ret);\n> }\n> \n> -static void fill_tracking_info(struct strbuf *stat, const char *branch_name,\n> -\t\tint show_upstream_ref)\n> -{\n> -\tint ours, theirs;\n> -\tchar *ref = NULL;\n> -\tstruct branch *branch = branch_get(branch_name);\n> -\tconst char *upstream;\n> -\tstruct strbuf fancy = STRBUF_INIT;\n> -\tint upstream_is_gone = 0;\n> -\tint added_decoration = 1;\n> -\n> -\tif (stat_tracking_info(branch, &ours, &theirs, &upstream) < 0) {\n> -\t\tif (!upstream)\n> -\t\t\treturn;\n> -\t\tupstream_is_gone = 1;\n> -\t}\n> -\n> -\tif (show_upstream_ref) {\n> -\t\tref = shorten_unambiguous_ref(upstream, 0);\n> -\t\tif (want_color(branch_use_color))\n> -\t\t\tstrbuf_addf(&fancy, \"%s%s%s\",\n> -\t\t\t\t\tbranch_get_color(BRANCH_COLOR_UPSTREAM),\n> -\t\t\t\t\tref, branch_get_color(BRANCH_COLOR_RESET));\n> -\t\telse\n> -\t\t\tstrbuf_addstr(&fancy, ref);\n> -\t}\n> -\n> -\tif (upstream_is_gone) {\n> -\t\tif (show_upstream_ref)\n> -\t\t\tstrbuf_addf(stat, _(\"[%s: gone]\"), fancy.buf);\n> -\t\telse\n> -\t\t\tadded_decoration = 0;\n> -\t} else if (!ours && !theirs) {\n> -\t\tif (show_upstream_ref)\n> -\t\t\tstrbuf_addf(stat, _(\"[%s]\"), fancy.buf);\n> -\t\telse\n> -\t\t\tadded_decoration = 0;\n> -\t} else if (!ours) {\n> -\t\tif (show_upstream_ref)\n> -\t\t\tstrbuf_addf(stat, _(\"[%s: behind %d]\"), fancy.buf, theirs);\n> -\t\telse\n> -\t\t\tstrbuf_addf(stat, _(\"[behind %d]\"), theirs);\n> -\n> -\t} else if (!theirs) {\n> -\t\tif (show_upstream_ref)\n> -\t\t\tstrbuf_addf(stat, _(\"[%s: ahead %d]\"), fancy.buf, ours);\n> -\t\telse\n> -\t\t\tstrbuf_addf(stat, _(\"[ahead %d]\"), ours);\n> -\t} else {\n> -\t\tif (show_upstream_ref)\n> -\t\t\tstrbuf_addf(stat, _(\"[%s: ahead %d, behind %d]\"),\n> -\t\t\t\t fancy.buf, ours, theirs);\n> -\t\telse\n> -\t\t\tstrbuf_addf(stat, _(\"[ahead %d, behind %d]\"),\n> -\t\t\t\t ours, theirs);\n> -\t}\n> -\tstrbuf_release(&fancy);\n> -\tif (added_decoration)\n> -\t\tstrbuf_addch(stat, ' ');\n> -\tfree(ref);\n> -}\n> -\n> -static void add_verbose_info(struct strbuf *out, struct ref_array_item *item,\n> -\t\t\t struct ref_filter *filter, const char *refname)\n> -{\n> -\tstruct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;\n> -\tconst char *sub = _(\" **** invalid ref ****\");\n> -\tstruct commit *commit = item->commit;\n> -\n> -\tif (!parse_commit(commit)) {\n> -\t\tpp_commit_easy(CMIT_FMT_ONELINE, commit, &subject);\n> -\t\tsub = subject.buf;\n> -\t}\n> -\n> -\tif (item->kind == FILTER_REFS_BRANCHES)\n> -\t\tfill_tracking_info(&stat, refname, filter->verbose > 1);\n> -\n> -\tstrbuf_addf(out, \" %s %s%s\",\n> -\t\tfind_unique_abbrev(item->commit->object.sha1, filter->abbrev),\n> -\t\tstat.buf, sub);\n> -\tstrbuf_release(&stat);\n> -\tstrbuf_release(&subject);\n> -}\n> -\n> -/*\n> - * This is duplicated in ref-filter.c, will be removed when we adopt\n> - * ref-filter's printing APIs.\n> - */\n> -static char *get_head_description(void)\n> -{\n> -\tstruct strbuf desc = STRBUF_INIT;\n> -\tstruct wt_status_state state;\n> -\tmemset(&state, 0, sizeof(state));\n> -\twt_status_get_state(&state, 1);\n> -\tif (state.rebase_in_progress ||\n> -\t state.rebase_interactive_in_progress)\n> -\t\tstrbuf_addf(&desc, _(\"(no branch, rebasing %s)\"),\n> -\t\t\t state.branch);\n> -\telse if (state.bisect_in_progress)\n> -\t\tstrbuf_addf(&desc, _(\"(no branch, bisect started on %s)\"),\n> -\t\t\t state.branch);\n> -\telse if (state.detached_from) {\n> -\t\t/* TRANSLATORS: make sure these match _(\"HEAD detached at \")\n> -\t\t and _(\"HEAD detached from \") in wt-status.c */\n> -\t\tif (state.detached_at)\n> -\t\t\tstrbuf_addf(&desc, _(\"(HEAD detached at %s)\"),\n> -\t\t\t\tstate.detached_from);\n> -\t\telse\n> -\t\t\tstrbuf_addf(&desc, _(\"(HEAD detached from %s)\"),\n> -\t\t\t\tstate.detached_from);\n> -\t}\n> -\telse\n> -\t\tstrbuf_addstr(&desc, _(\"(no branch)\"));\n> -\tfree(state.branch);\n> -\tfree(state.onto);\n> -\tfree(state.detached_from);\n> -\treturn strbuf_detach(&desc, NULL);\n> -}\n> -\n> -static void format_and_print_ref_item(struct ref_array_item *item, int maxwidth,\n> -\t\t\t\t struct ref_filter *filter, const char *remote_prefix)\n> -{\n> -\tchar c;\n> -\tint current = 0;\n> -\tint color;\n> -\tstruct strbuf out = STRBUF_INIT, name = STRBUF_INIT;\n> -\tconst char *prefix = \"\";\n> -\tconst char *desc = item->refname;\n> -\tchar *to_free = NULL;\n> -\n> -\tswitch (item->kind) {\n> -\tcase FILTER_REFS_BRANCHES:\n> -\t\tskip_prefix(desc, \"refs/heads/\", &desc);\n> -\t\tif (!filter->detached && !strcmp(desc, head))\n> -\t\t\tcurrent = 1;\n> -\t\telse\n> -\t\t\tcolor = BRANCH_COLOR_LOCAL;\n> -\t\tbreak;\n> -\tcase FILTER_REFS_REMOTES:\n> -\t\tskip_prefix(desc, \"refs/remotes/\", &desc);\n> -\t\tcolor = BRANCH_COLOR_REMOTE;\n> -\t\tprefix = remote_prefix;\n> -\t\tbreak;\n> -\tcase FILTER_REFS_DETACHED_HEAD:\n> -\t\tdesc = to_free = get_head_description();\n> -\t\tcurrent = 1;\n> -\t\tbreak;\n> -\tdefault:\n> -\t\tcolor = BRANCH_COLOR_PLAIN;\n> -\t\tbreak;\n> -\t}\n> -\n> -\tc = ' ';\n> -\tif (current) {\n> -\t\tc = '*';\n> -\t\tcolor = BRANCH_COLOR_CURRENT;\n> -\t}\n> -\n> -\tstrbuf_addf(&name, \"%s%s\", prefix, desc);\n> -\tif (filter->verbose) {\n> -\t\tint utf8_compensation = strlen(name.buf) - utf8_strwidth(name.buf);\n> -\t\tstrbuf_addf(&out, \"%c %s%-*s%s\", c, branch_get_color(color),\n> -\t\t\t maxwidth + utf8_compensation, name.buf,\n> -\t\t\t branch_get_color(BRANCH_COLOR_RESET));\n> -\t} else\n> -\t\tstrbuf_addf(&out, \"%c %s%s%s\", c, branch_get_color(color),\n> -\t\t\t name.buf, branch_get_color(BRANCH_COLOR_RESET));\n> -\n> -\tif (item->symref) {\n> -\t\tskip_prefix(item->symref, \"refs/remotes/\", &desc);\n> -\t\tstrbuf_addf(&out, \" -> %s\", desc);\n> -\t}\n> -\telse if (filter->verbose)\n> -\t\t/* \" f7c0c00 [ahead 58, behind 197] vcs-svn: drop obj_pool.h\" */\n> -\t\tadd_verbose_info(&out, item, filter, desc);\n> -\tif (column_active(colopts)) {\n> -\t\tassert(!filter->verbose && \"--column and --verbose are incompatible\");\n> -\t\tstring_list_append(&output, out.buf);\n> -\t} else {\n> -\t\tprintf(\"%s\\n\", out.buf);\n> -\t}\n> -\tstrbuf_release(&name);\n> -\tstrbuf_release(&out);\n> -\tfree(to_free);\n> -}\n> -\n> static int calc_maxwidth(struct ref_array *refs, int remote_bonus)\n> {\n> \tint i, max = 0;\n> @@ -467,7 +281,10 @@ static int calc_maxwidth(struct ref_array *refs, int remote_bonus)\n> \n> \t\tskip_prefix(it->refname, \"refs/heads/\", &desc);\n> \t\tskip_prefix(it->refname, \"refs/remotes/\", &desc);\n> -\t\tw = utf8_strwidth(desc);\n> +\t\tif (it->kind == FILTER_REFS_DETACHED_HEAD)\n> +\t\t\tw = strlen(get_head_description());\n> +\t\telse\n> +\t\t\tw = utf8_strwidth(desc);\n> \n> \t\tif (it->kind == FILTER_REFS_REMOTES)\n> \t\t\tw += remote_bonus;\n> @@ -477,12 +294,51 @@ static int calc_maxwidth(struct ref_array *refs, int remote_bonus)\n> \treturn max;\n> }\n> \n> +static char *build_format(struct ref_filter *filter, int maxwidth, const char *remote_prefix)\n> +{\n> +\tstruct strbuf fmt = STRBUF_INIT;\n> +\tstruct strbuf local = STRBUF_INIT;\n> +\tstruct strbuf remote = STRBUF_INIT;\n> +\n> +\tstrbuf_addf(&fmt, \"%%(if)%%(HEAD)%%(then)* %s%%(else) %%(end)\", branch_get_color(BRANCH_COLOR_CURRENT));\n> +\n> +\tif (filter->verbose) {\n> +\t\tstrbuf_addf(&local, \"%%(align:%d,left)%%(refname:short)%%(end)\", maxwidth);\n> +\t\tstrbuf_addf(&local, \"%s\", branch_get_color(BRANCH_COLOR_RESET));\n> +\t\tstrbuf_addf(&local, \" %%(objectname:short=7) \");\n> +\n> +\t\tif (filter->verbose > 1)\n> +\t\t\tstrbuf_addf(&local, \"%%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s%%(if)%%(upstream:track)\"\n> +\t\t\t\t \"%%(then): %%(upstream:track,nobracket)%%(end)] %%(end)%%(contents:subject)\",\n> +\t\t\t\t branch_get_color(BRANCH_COLOR_UPSTREAM), branch_get_color(BRANCH_COLOR_RESET));\n> +\t\telse\n> +\t\t\tstrbuf_addf(&local, \"%%(if)%%(upstream:track)%%(then)%%(upstream:track) %%(end)%%(contents:subject)\");\n> +\n> +\t\tstrbuf_addf(&remote, \"%s%%(align:%d,left)%s%%(refname:short)%%(end)%s%%(if)%%(symref)%%(then) -> %%(symref:short)\"\n> +\t\t\t \"%%(else) %%(objectname:short=7) %%(contents:subject)%%(end)\",\n> +\t\t\t branch_get_color(BRANCH_COLOR_REMOTE), maxwidth,\n> +\t\t\t remote_prefix, branch_get_color(BRANCH_COLOR_RESET));\n> +\t} else {\n> +\t\tstrbuf_addf(&local, \"%%(refname:short)%s\", branch_get_color(BRANCH_COLOR_RESET));\n> +\t\tstrbuf_addf(&remote, \"%s%s%%(refname:short)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)\",\n> +\t\t\t branch_get_color(BRANCH_COLOR_REMOTE), remote_prefix, branch_get_color(BRANCH_COLOR_RESET));\n> +\t}\n> +\n> +\tstrbuf_addf(&fmt, \"%%(if:notequals=remotes)%%(refname:base)%%(then)%s%%(else)%s%%(end)\", local.buf, remote.buf);\n> +\n> +\tstrbuf_release(&local);\n> +\tstrbuf_release(&remote);\n> +\treturn strbuf_detach(&fmt, NULL);\n> +}\n> +\n> static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sorting)\n> {\n> \tint i;\n> \tstruct ref_array array;\n> \tint maxwidth = 0;\n> \tconst char *remote_prefix = \"\";\n> +\tstruct strbuf out = STRBUF_INIT;\n> +\tchar *format;\n> \n> \t/*\n> \t * If we are listing more than just remote branches,\n> @@ -494,12 +350,14 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n> \n> \tmemset(&array, 0, sizeof(array));\n> \n> -\tverify_ref_format(\"%(refname)%(symref)\");\n> \tfilter_refs(&array, filter, filter->kind | FILTER_REFS_INCLUDE_BROKEN);\n> \n> \tif (filter->verbose)\n> \t\tmaxwidth = calc_maxwidth(&array, strlen(remote_prefix));\n> \n> +\tformat = build_format(filter, maxwidth, remote_prefix);\n> +\tverify_ref_format(format);\n> +\n> \t/*\n> \t * If no sorting parameter is given then we default to sorting\n> \t * by 'refname'. This would give us an alphabetically sorted\n> @@ -511,10 +369,21 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n> \t\tsorting = ref_default_sorting();\n> \tref_array_sort(sorting, &array);\n> \n> -\tfor (i = 0; i < array.nr; i++)\n> -\t\tformat_and_print_ref_item(array.items[i], maxwidth, filter, remote_prefix);\n> +\tfor (i = 0; i < array.nr; i++) {\n> +\t\tformat_ref_array_item(array.items[i], format, 0, &out);\n> +\t\tif (column_active(colopts)) {\n> +\t\t\tassert(!filter->verbose && \"--column and --verbose are incompatible\");\n> +\t\t\t /* format to a string_list to let print_columns() do its job */\n> +\t\t\tstring_list_append(&output, out.buf);\n> +\t\t} else {\n> +\t\t\tfwrite(out.buf, 1, out.len, stdout);\n> +\t\t\tputchar('\\n');\n> +\t\t}\n> +\t\tstrbuf_release(&out);\n> +\t}\n> \n> \tref_array_clear(&array);\n> +\tfree(format);\n> }\n> \n> static void rename_branch(const char *oldname, const char *newname, int force)\n> diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh\n> index 3d5c238..97a0765 100755\n> --- a/t/t6040-tracking-info.sh\n> +++ b/t/t6040-tracking-info.sh\n> @@ -44,7 +44,7 @@ b1 [ahead 1, behind 1] d\n> b2 [ahead 1, behind 1] d\n> b3 [behind 1] b\n> b4 [ahead 2] f\n> -b5 g\n> +b5 [gone] g\n> b6 c\n> EOF\n"},{"id":"271581","messageId":"CAOLa=ZQ5nC7bqz2gLYvO2LxBrMhkf+Ga42h3SkQOzvd78OVYxA@mail.gmail.com","threadId":"40517","inReplyTo":"xmqqr3kyafxa.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 09/10] branch: use ref-filter printing APIs","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-13T17:43:27Z","receivedAt":"2015-10-13T17:43:27Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Tue, Oct 13, 2015 at 10:01 PM, Junio C Hamano wrote:\n> Karthik Nayak writes:\n>\n>> Port branch.c to use ref-filter APIs for printing. This clears out\n>> most of the code used in branch.c for printing and replaces them with\n>> calls made to the ref-filter library.\n>>\n>> Introduce get_format() which gets the format required for printing of\n>> refs. Make amendments to print_ref_list() to reflect these changes.\n>\n> Is it get_format() still?\n\nWill change that, thanks :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271583","messageId":"CAOLa=ZRZszOqHqJfOHhFqa-XDZbrcPCuSdvM_zMEYHURv8kW_A@mail.gmail.com","threadId":"40517","inReplyTo":"1444295885-1657-9-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v2 08/10] ref-filter: add support for %(upstream:track,nobracket)","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-13T18:13:45Z","receivedAt":"2015-10-13T18:13:45Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Thu, Oct 8, 2015 at 2:48 PM, Karthik Nayak wrote:\n> Add support for %(upstream:track,nobracket) which will print the\n> tracking information without the brackets (i.e. \"ahead N, behind M\").\n>\n> Add test and documentation for the same.\n> ---\n> Documentation/git-for-each-ref.txt | 6 ++++--\n> ref-filter.c | 28 +++++++++++++++++++++++-----\n> t/t6300-for-each-ref.sh | 9 +++++++++\n> 3 files changed, 36 insertions(+), 7 deletions(-)\n>\n> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> index c713ec0..a38cbf6 100644\n> --- a/Documentation/git-for-each-ref.txt\n> +++ b/Documentation/git-for-each-ref.txt\n> @@ -114,8 +114,10 @@ upstream::\n> `refname` above. Additionally respects `:track` to show\n> \"[ahead N, behind M]\" and `:trackshort` to show the terse\n> version: \">\" (ahead), \"<\" (behind), \"<>\" (ahead and behind),\n> - or \"=\" (in sync). Has no effect if the ref does not have\n> - tracking information associated with it.\n> + or \"=\" (in sync). Append `:track,nobracket` to show tracking\n> + information without brackets (i.e \"ahead N, behind M\"). Has\n> + no effect if the ref does not have tracking information\n> + associated with it.\n>\n> push::\n> The name of a local ref which represents the `@{push}` location\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 6a38089..6044eb0 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1112,27 +1112,45 @@ static void populate_value(struct ref_array_item *ref)\n> if (!strcmp(formatp, \"short\"))\n> refname = shorten_unambiguous_ref(refname,\n> warn_ambiguous_refs);\n> - else if (!strcmp(formatp, \"track\") &&\n> + else if (skip_prefix(formatp, \"track\", &valp) &&\n> + strcmp(formatp, \"trackshort\") &&\n> (starts_with(name, \"upstream\") ||\n> starts_with(name, \"push\"))) {\n\nIf you see here, we detect \"track\" first for\n%(upstream:track,nobracket) so although\nthe idea was to use something similar to %(align:...) I don't see a\ngood way of going\nabout this. If we want %(upstream:nobracket,track) to be supported then, I think\nwe'll have to change this whole layout and have the detection done up where we\nlocat \"upstream\" / \"push\", what would be a nice way to go around this?\n\nWhat I could think of:\n1. Do the cleanup that Junio and Matthieu suggested, where we\nbasically parse the\natoms and store them into a used_atom struct. I could either work on\nthose patches\nbefore this and then rebase this on top.\n2. Let this be and come back on it when implementing the above series.\nAfter reading Matthieu's and Junio's discussion, I lean towards the latter.\n\n> char buf[40];\n> + unsigned int nobracket = 0;\n> +\n> + if (!strcmp(valp, \",nobracket\"))\n> + nobracket = 1;\n>\n> if (stat_tracking_info(branch, &num_ours,\n> &num_theirs, NULL)) {\n> - v->s = \"[gone]\";\n> + if (nobracket)\n> + v->s = \"gone\";\n> + else\n> + v->s = \"[gone]\";\n> continue;\n> }\n>\n> if (!num_ours && !num_theirs)\n> v->s = \"\";\n> else if (!num_ours) {\n> - sprintf(buf, \"[behind %d]\", num_theirs);\n> + if (nobracket)\n> + sprintf(buf, \"behind %d\", num_theirs);\n> + else\n> + sprintf(buf, \"[behind %d]\", num_theirs);\n> v->s = xstrdup(buf);\n> } else if (!num_theirs) {\n> - sprintf(buf, \"[ahead %d]\", num_ours);\n> + if (nobracket)\n> + sprintf(buf, \"ahead %d\", num_ours);\n> + else\n> + sprintf(buf, \"[ahead %d]\", num_ours);\n> v->s = xstrdup(buf);\n> } else {\n> - sprintf(buf, \"[ahead %d, behind %d]\",\n> + if (nobracket)\n> + sprintf(buf, \"ahead %d, behind %d\",\n> + num_ours, num_theirs);\n> + else\n> + sprintf(buf, \"[ahead %d, behind %d]\",\n> num_ours, num_theirs);\n> v->s = xstrdup(buf);\n> }\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index 4f620bf..7ab8bf8 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -344,6 +344,15 @@ test_expect_success 'Check upstream:track format' '\n> '\n>\n> cat >expected < +ahead 1\n> +EOF\n> +\n> +test_expect_success 'Check upstream:track,nobracket format' '\n> + git for-each-ref --format=\"%(upstream:track,nobracket)\" refs/heads >actual &&\n> + test_cmp expected actual\n> +'\n> +\n> +cat >expected < >\n> EOF\n>\n> --\n> 2.6.0\n>\n\n\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271587","messageId":"vpqeggy61ma.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"CAOLa=ZRZszOqHqJfOHhFqa-XDZbrcPCuSdvM_zMEYHURv8kW_A@mail.gmail.com","subject":"Re: [PATCH v2 08/10] ref-filter: add support for %(upstream:track,nobracket)","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-13T18:54:05Z","receivedAt":"2015-10-13T18:54:05Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Karthik Nayak writes:\n\n> On Thu, Oct 8, 2015 at 2:48 PM, Karthik Nayak wrote:\n>> Add support for %(upstream:track,nobracket) which will print the\n>> tracking information without the brackets (i.e. \"ahead N, behind M\").\n>>\n>> Add test and documentation for the same.\n>> ---\n>> Documentation/git-for-each-ref.txt | 6 ++++--\n>> ref-filter.c | 28 +++++++++++++++++++++++-----\n>> t/t6300-for-each-ref.sh | 9 +++++++++\n>> 3 files changed, 36 insertions(+), 7 deletions(-)\n>>\n>> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n>> index c713ec0..a38cbf6 100644\n>> --- a/Documentation/git-for-each-ref.txt\n>> +++ b/Documentation/git-for-each-ref.txt\n>> @@ -114,8 +114,10 @@ upstream::\n>> `refname` above. Additionally respects `:track` to show\n>> \"[ahead N, behind M]\" and `:trackshort` to show the terse\n>> version: \">\" (ahead), \"<\" (behind), \"<>\" (ahead and behind),\n>> - or \"=\" (in sync). Has no effect if the ref does not have\n>> - tracking information associated with it.\n>> + or \"=\" (in sync). Append `:track,nobracket` to show tracking\n>> + information without brackets (i.e \"ahead N, behind M\"). Has\n>> + no effect if the ref does not have tracking information\n>> + associated with it.\n>>\n>> push::\n>> The name of a local ref which represents the `@{push}` location\n>> diff --git a/ref-filter.c b/ref-filter.c\n>> index 6a38089..6044eb0 100644\n>> --- a/ref-filter.c\n>> +++ b/ref-filter.c\n>> @@ -1112,27 +1112,45 @@ static void populate_value(struct ref_array_item *ref)\n>> if (!strcmp(formatp, \"short\"))\n>> refname = shorten_unambiguous_ref(refname,\n>> warn_ambiguous_refs);\n>> - else if (!strcmp(formatp, \"track\") &&\n>> + else if (skip_prefix(formatp, \"track\", &valp) &&\n>> + strcmp(formatp, \"trackshort\") &&\n>> (starts_with(name, \"upstream\") ||\n>> starts_with(name, \"push\"))) {\n>\n> If you see here, we detect \"track\" first for\n> %(upstream:track,nobracket)\n\nYes, but I still think that this was a bad idea. If you want nobracket\nto apply to \"track\", then the syntax should be\n%(upstream:track=nobracket). I think the \"nobracket\" should apply to\n\"upstream\" (i.e. be global to the atom), hence\n%(upstream:nobracket,track) and %(upstream:track,nobracket) should both\nbe accepted. Possibly %(upstream:,nobracket) could complain,\nbut just ignoring \"nobracket\" in this case would make sense to me.\n\nSpecial-casing the implementation of \"nobracket\" also means you're\nspecial-casing its user-visible behavior. And as a user, I hate it when\nthe behavior is subtely different for things that look like each other\n(in this case, %(align:...) and %(upstream:...) ).\n\n> %(upstream:nobracket,track) to be supported then, I think we'll have\n> to change this whole layout and have the detection done up where we\n> locat \"upstream\" / \"push\", what would be a nice way to go around this?\n\nYou mean, below\n\n\t\telse if (starts_with(name, \"upstream\")) {\n\nwithin populate_value()?\n\nI think it would, yes.\n\n> What I could think of:\n> 1. Do the cleanup that Junio and Matthieu suggested, where we\n> basically parse the\n> atoms and store them into a used_atom struct. I could either work on\n> those patches\n> before this and then rebase this on top.\n> 2. Let this be and come back on it when implementing the above series.\n> After reading Matthieu's and Junio's discussion, I lean towards the latter.\n\nLeaving it as-is does not fit in my arguments to do the refactoring\nlater. It's not introducing \"another instance of an existing pattern\",\nbut actually a new pattern.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271589","messageId":"xmqqmvvma8lt.fsf@gitster.mtv.corp.google.com","threadId":"40517","inReplyTo":"vpqeggy61ma.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 08/10] ref-filter: add support for %(upstream:track,nobracket)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-13T19:09:34Z","receivedAt":"2015-10-13T19:09:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n>> If you see here, we detect \"track\" first for\n>> %(upstream:track,nobracket)\n>\n> Yes, but I still think that this was a bad idea. If you want\n> nobracket to apply to \"track\", then the syntax should be\n> %(upstream:track=nobracket). I think the \"nobracket\" should apply\n> to \"upstream\" (i.e. be global to the atom), hence\n> %(upstream:nobracket,track) and %(upstream:track,nobracket) should\n> both be accepted.\n\nThat makes sense to me, as I think what is being discussed would be\n%(upstream:track=yes,bracket=no) when it is fully spelled out.\n"},{"id":"271635","messageId":"CAOLa=ZRuTLfRbc3hQ_3AKGD-Vj6v2VcXXVThG+oXptDkYCHo_w@mail.gmail.com","threadId":"40517","inReplyTo":"vpqeggy61ma.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 08/10] ref-filter: add support for %(upstream:track,nobracket)","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-14T06:12:55Z","receivedAt":"2015-10-14T06:12:55Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":" Wed, Oct 14, 2015 at 12:24 AM, Matthieu Moy\n wrote:\n> Yes, but I still think that this was a bad idea. If you want nobracket\n> to apply to \"track\", then the syntax should be\n> %(upstream:track=nobracket). I think the \"nobracket\" should apply to\n> \"upstream\" (i.e. be global to the atom), hence\n> %(upstream:nobracket,track) and %(upstream:track,nobracket) should both\n> be accepted. Possibly %(upstream:,nobracket) could complain,\n> but just ignoring \"nobracket\" in this case would make sense to me.\n>\n\nOh okay, was thinking only WRT the \"track\" option.\n\n> Special-casing the implementation of \"nobracket\" also means you're\n> special-casing its user-visible behavior. And as a user, I hate it when\n> the behavior is subtely different for things that look like each other\n> (in this case, %(align:...) and %(upstream:...) ).\n>\n\nMakes sense, was just looking for opinions.\n\n>> %(upstream:nobracket,track) to be supported then, I think we'll have\n>> to change this whole layout and have the detection done up where we\n>> locat \"upstream\" / \"push\", what would be a nice way to go around this?\n>\n> You mean, below\n>\n> else if (starts_with(name, \"upstream\")) {\n>\n> within populate_value()?\n>\n> I think it would, yes.\n>\n\nYes, that's what I meant.\n\n>> What I could think of:\n>> 1. Do the cleanup that Junio and Matthieu suggested, where we\n>> basically parse the\n>> atoms and store them into a used_atom struct. I could either work on\n>> those patches\n>> before this and then rebase this on top.\n>> 2. Let this be and come back on it when implementing the above series.\n>> After reading Matthieu's and Junio's discussion, I lean towards the latter.\n>\n> Leaving it as-is does not fit in my arguments to do the refactoring\n> later. It's not introducing \"another instance of an existing pattern\",\n> but actually a new pattern.\n>\n\nI meant after changing whatever we discussed above.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271884","messageId":"CAOLa=ZQ5dCx4XheMvaFo1u-fR=uaR-LU-n7KHR3xNJ7TuF-zww@mail.gmail.com","threadId":"40517","inReplyTo":"vpq612bzz0o.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-18T19:07:05Z","receivedAt":"2015-10-18T19:07:05Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Tue, Oct 13, 2015 at 12:37 AM, Matthieu Moy\n wrote:\n> Junio C Hamano writes:\n>\n>> If you design a new infrastructure to help refactoring early\n>> (i.e. before adding many copies of code that need to be cleaned up\n>> later), it would make the work of reviewing of the design of the\n>> helper and refactoring using that helper smaller, not larger.\n>\n> But most of the code concerned is already reviewed. The first instances\n> of the pattern to refactor is already in next. With a real time machine,\n> we could go back in past, refactor and then have cleaner series, but\n> with Git as our only tool we can't ;-).\n>\n> The current series will just add one more instance of sub-optimal code,\n> it isn't hard to review. Inserting new code before them would make the\n> interdiff far bigger.\n>\n\nSorry for the delay, was a bit busy with college work. For the most\npart I've been\ntrying to integrate the %(upstream) and its options 'track',\n'trachshort' and 'short'\nso we could implement %(upstream:track,nobracket) or\n%(upstream:nobracket,track).\nWhile doing so I realized I was moving a lot of code around and this\nhad me thinking it's\nperhaps easier to do the cleaning up first as Junio suggested.\n\nMaybe then it'd be simpler to do implement this rather than move code around now\nand then move code around when we implement the parsing methods we\nspoke about earlier?\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271889","messageId":"xmqqzizfa2n7.fsf@gitster.mtv.corp.google.com","threadId":"40517","inReplyTo":"CAOLa=ZQ5dCx4XheMvaFo1u-fR=uaR-LU-n7KHR3xNJ7TuF-zww@mail.gmail.com","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-19T04:44:12Z","receivedAt":"2015-10-19T04:44:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> ... While doing so I realized I was moving a lot of code around\n> and this had me thinking it's perhaps easier to do the cleaning up\n> first as Junio suggested.\n> \n> Maybe then it'd be simpler to do implement this rather than move\n> code around now and then move code around when we implement the\n> parsing methods we spoke about earlier?\n\nWhen I suggested the approach, I hadn't have done any of what you\nactually did, so I was primarily talking out of \"hunch\" (perhaps\nthat is what some people call \"taste\") but I expected something like\nthat may happen ;-).\n\nRight now, you practically own the topic, in the sense that nobody\nseems to be in a very urgent need to compete with you in parallel to\nimplement what we have discussed before you do, so I personally\nwould suggest whichever order you feel more comfortable and less\nerror-prone.\n\nThanks.\n"},{"id":"271930","messageId":"vpqy4ezjmyp.fsf@grenoble-inp.fr","threadId":"40517","inReplyTo":"xmqqzizfa2n7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-19T08:12:46Z","receivedAt":"2015-10-19T08:12:46Z","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 personally would suggest whichever order you feel more comfortable\n> and less error-prone.\n\nThis is a good summary, and I fully agree with it.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271994","messageId":"CAOLa=ZT1AEyXbETrRdMQMfBFv5qJhNL4msRxFGpKe0K5aA7SsQ@mail.gmail.com","threadId":"40517","inReplyTo":"vpqy4ezjmyp.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-20T20:53:33Z","receivedAt":"2015-10-20T20:53:33Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Oct 19, 2015 at 1:42 PM, Matthieu Moy\n wrote:\n> Junio C Hamano writes:\n>\n>> I personally would suggest whichever order you feel more comfortable\n>> and less error-prone.\n>\n> This is a good summary, and I fully agree with it.\n\nWell then, I'm working on the parsing part of it before this series :)\n\n-- \nRegards,\nKarthik Nayak\n"}]}