{"thread":{"id":"35330","subject":"[PATCH v2 0/3] Minor f-e-r enhacements","startedAt":"2013-11-13T09:36:43Z","lastAt":"2013-11-18T16:45:30Z","messageCount":12,"participants":["Ramkumar Ramachandra","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"230557","messageId":"1384335406-16332-1-git-send-email-artagnon@gmail.com","threadId":"35330","inReplyTo":null,"subject":"[PATCH v2 0/3] Minor f-e-r enhacements","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-11-13T09:36:43Z","receivedAt":"2013-11-13T09:36:43Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nv2 uses the more sane %(color:...) as opposed to %C(...) for changing\nthe color of the output, as suggested by Junio. Everything else is the\nsame.\n\nThanks.\n\nRamkumar Ramachandra (3):\n for-each-ref: introduce %(HEAD) asterisk marker\n for-each-ref: introduce %(upstream:track[short])\n for-each-ref: introduce %(color:...) for color\n\n Documentation/git-for-each-ref.txt | 14 +++++++-\n builtin/for-each-ref.c | 66 ++++++++++++++++++++++++++++++++++----\n 2 files changed, 72 insertions(+), 8 deletions(-)\n\n-- \n1.8.5.rc0.3.g914176d.dirty\n"},{"id":"230558","messageId":"1384335406-16332-2-git-send-email-artagnon@gmail.com","threadId":"35330","inReplyTo":"1384335406-16332-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v2 1/3] for-each-ref: introduce %(HEAD) asterisk marker","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-11-13T09:36:44Z","receivedAt":"2013-11-13T09:36:44Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"'git branch' shows which branch you are currently on with an '*', but\n'git for-each-ref' misses this feature. So, extend its format with\n%(HEAD) for the same effect.\n\nNow you can use the following format in for-each-ref:\n\n %(HEAD) %(refname:short)\n\nto display an asterisk next to the current ref.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n Documentation/git-for-each-ref.txt | 4 ++++\n builtin/for-each-ref.c | 13 +++++++++++--\n 2 files changed, 15 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex f2e08d1..ab3da0e 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -93,6 +93,10 @@ upstream::\n \tfrom the displayed ref. Respects `:short` in the same way as\n \t`refname` above.\n \n+HEAD::\n+\tUsed to indicate the currently checked out branch. Is '*' if\n+\tHEAD points to the current ref, and ' ' otherwise.\n+\n In addition to the above, for commit and tag objects, the header\n field names (`tree`, `parent`, `object`, `type`, and `tag`) can\n be used to specify the value in the header field.\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 1d4083c..5f1842f 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -75,6 +75,7 @@ static struct {\n \t{ \"upstream\" },\n \t{ \"symref\" },\n \t{ \"flag\" },\n+\t{ \"HEAD\" },\n };\n \n /*\n@@ -675,8 +676,16 @@ static void populate_value(struct refinfo *ref)\n \t\t\t\tv->s = xstrdup(buf + 1);\n \t\t\t}\n \t\t\tcontinue;\n-\t\t}\n-\t\telse\n+\t\t} else if (!strcmp(name, \"HEAD\")) {\n+\t\t\tconst char *head;\n+\t\t\tunsigned char sha1[20];\n+\t\t\thead = resolve_ref_unsafe(\"HEAD\", sha1, 1, NULL);\n+\t\t\tif (!strcmp(ref->refname, head))\n+\t\t\t\tv->s = \"*\";\n+\t\t\telse\n+\t\t\t\tv->s = \" \";\n+\t\t\tcontinue;\n+\t\t} else\n \t\t\tcontinue;\n \n \t\tformatp = strchr(name, ':');\n-- \n1.8.5.rc0.3.g914176d.dirty\n"},{"id":"230559","messageId":"1384335406-16332-3-git-send-email-artagnon@gmail.com","threadId":"35330","inReplyTo":"1384335406-16332-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v2 2/3] for-each-ref: introduce %(upstream:track[short])","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-11-13T09:36:45Z","receivedAt":"2013-11-13T09:36:45Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Introduce %(upstream:track) to display \"[ahead M, behind N]\" and\n%(upstream:trackshort) to display \"=\", \">\", \"<\", or \"<>\"\nappropriately (inspired by contrib/completion/git-prompt.sh).\n\nNow you can use the following format in for-each-ref:\n\n %(refname:short)%(upstream:trackshort)\n\nto display refs with terse tracking information.\n\nNote that :track and :trackshort only work with \"upstream\", and error\nout when used with anything else.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n Documentation/git-for-each-ref.txt | 6 +++++-\n builtin/for-each-ref.c | 40 +++++++++++++++++++++++++++++++++++---\n 2 files changed, 42 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex ab3da0e..c9b192e 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -91,7 +91,11 @@ objectname::\n upstream::\n \tThe name of a local ref which can be considered ``upstream''\n \tfrom the displayed ref. Respects `:short` in the same way as\n-\t`refname` above.\n+\t`refname` above. Additionally respects `:track` to show\n+\t\"[ahead N, behind M]\" and `:trackshort` to show the terse\n+\tversion (like the prompt) \">\", \"<\", \"<>\", or \"=\". Has no\n+\teffect if the ref does not have tracking information\n+\tassociated with it.\n \n HEAD::\n \tUsed to indicate the currently checked out branch. Is '*' if\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 5f1842f..ed81407 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -641,6 +641,7 @@ static void populate_value(struct refinfo *ref)\n \t\tint deref = 0;\n \t\tconst char *refname;\n \t\tconst char *formatp;\n+\t\tstruct branch *branch;\n \n \t\tif (*name == '*') {\n \t\t\tderef = 1;\n@@ -652,7 +653,6 @@ static void populate_value(struct refinfo *ref)\n \t\telse if (!prefixcmp(name, \"symref\"))\n \t\t\trefname = ref->symref ? ref->symref : \"\";\n \t\telse if (!prefixcmp(name, \"upstream\")) {\n-\t\t\tstruct branch *branch;\n \t\t\t/* only local branches may have an upstream */\n \t\t\tif (prefixcmp(ref->refname, \"refs/heads/\"))\n \t\t\t\tcontinue;\n@@ -679,6 +679,7 @@ static void populate_value(struct refinfo *ref)\n \t\t} else if (!strcmp(name, \"HEAD\")) {\n \t\t\tconst char *head;\n \t\t\tunsigned char sha1[20];\n+\n \t\t\thead = resolve_ref_unsafe(\"HEAD\", sha1, 1, NULL);\n \t\t\tif (!strcmp(ref->refname, head))\n \t\t\t\tv->s = \"*\";\n@@ -689,13 +690,46 @@ static void populate_value(struct refinfo *ref)\n \t\t\tcontinue;\n \n \t\tformatp = strchr(name, ':');\n-\t\t/* look for \"short\" refname format */\n \t\tif (formatp) {\n+\t\t\tint num_ours, num_theirs;\n+\n \t\t\tformatp++;\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\n+\t\t\telse if (!strcmp(formatp, \"track\") &&\n+\t\t\t\t!prefixcmp(name, \"upstream\")) {\n+\t\t\t\tchar buf[40];\n+\n+\t\t\t\tstat_tracking_info(branch, &num_ours, &num_theirs);\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\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\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\t\tnum_ours, num_theirs);\n+\t\t\t\t\tv->s = xstrdup(buf);\n+\t\t\t\t}\n+\t\t\t\tcontinue;\n+\t\t\t} else if (!strcmp(formatp, \"trackshort\") &&\n+\t\t\t\t!prefixcmp(name, \"upstream\")) {\n+\n+\t\t\t\tstat_tracking_info(branch, &num_ours, &num_theirs);\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\tv->s = \"<\";\n+\t\t\t\telse if (!num_theirs)\n+\t\t\t\t\tv->s = \">\";\n+\t\t\t\telse\n+\t\t\t\t\tv->s = \"<>\";\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);\n \t\t}\n-- \n1.8.5.rc0.3.g914176d.dirty\n"},{"id":"230560","messageId":"1384335406-16332-4-git-send-email-artagnon@gmail.com","threadId":"35330","inReplyTo":"1384335406-16332-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v2 3/3] for-each-ref: introduce %(color:...) for color","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-11-13T09:36:46Z","receivedAt":"2013-11-13T09:36:46Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Enhance 'git for-each-ref' with color formatting options. You can now\nuse the following format in for-each-ref:\n\n %(color:green)%(refname:short)%(color:reset)\n\nSigned-off-by: Ramkumar Ramachandra \n---\n Documentation/git-for-each-ref.txt | 4 ++++\n builtin/for-each-ref.c | 13 +++++++++++--\n 2 files changed, 15 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex c9b192e..2f3ac22 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -101,6 +101,10 @@ HEAD::\n \tUsed to indicate the currently checked out branch. Is '*' if\n \tHEAD points to the current ref, and ' ' otherwise.\n \n+color::\n+\tUsed to change color of output. Followed by `:`,\n+\twhere names are described in `color.branch.*`.\n+\n In addition to the above, for commit and tag objects, the header\n field names (`tree`, `parent`, `object`, `type`, and `tag`) can\n be used to specify the value in the header field.\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex ed81407..c59fffe 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -9,6 +9,7 @@\n #include \"quote.h\"\n #include \"parse-options.h\"\n #include \"remote.h\"\n+#include \"color.h\"\n \n /* Quoting styles */\n #define QUOTE_NONE 0\n@@ -76,6 +77,7 @@ static struct {\n \t{ \"symref\" },\n \t{ \"flag\" },\n \t{ \"HEAD\" },\n+\t{ \"color\" },\n };\n \n /*\n@@ -662,8 +664,9 @@ static void populate_value(struct refinfo *ref)\n \t\t\t !branch->merge[0]->dst)\n \t\t\t\tcontinue;\n \t\t\trefname = branch->merge[0]->dst;\n-\t\t}\n-\t\telse if (!strcmp(name, \"flag\")) {\n+\t\t} else if (!prefixcmp(name, \"color\")) {\n+\t\t\t;\n+\t\t} else if (!strcmp(name, \"flag\")) {\n \t\t\tchar buf[256], *cp = buf;\n \t\t\tif (ref->flag & REF_ISSYMREF)\n \t\t\t\tcp = copy_advance(cp, \",symref\");\n@@ -729,6 +732,12 @@ static void populate_value(struct refinfo *ref)\n \t\t\t\telse\n \t\t\t\t\tv->s = \"<>\";\n \t\t\t\tcontinue;\n+\t\t\t} else if (!prefixcmp(name, \"color\")) {\n+\t\t\t\tchar color[COLOR_MAXLEN] = \"\";\n+\n+\t\t\t\tcolor_parse(formatp, \"--format\", color);\n+\t\t\t\tv->s = xstrdup(color);\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);\n-- \n1.8.5.rc0.3.g914176d.dirty\n"},{"id":"230572","messageId":"xmqqk3gcdr6z.fsf@gitster.dls.corp.google.com","threadId":"35330","inReplyTo":"1384335406-16332-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH v2 2/3] for-each-ref: introduce %(upstream:track[short])","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-13T19:42:12Z","receivedAt":"2013-11-13T19:42:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"You need to squash 78465bb, which has been queued on the previous\nround, in to this step. There also is a similar issue introduced by\nthe updated 3/3:\n\nbuiltin/for-each-ref.c: In function 'populate_value':\nbuiltin/for-each-ref.c:701:13: error: 'refname' may be used uninitialized in this function [-Werror=uninitialized]\n\n\n 700 if (!strcmp(formatp, \"short\"))\n 701 refname = shorten_unambiguous_ref(refname,\n 702 warn_ambiguous_refs);\n\nThanks.\n\n-- >8 --\nDate: Thu, 31 Oct 2013 14:17:36 -0700\nSubject: [PATCH] fixup! for-each-ref: introduce %(upstream:track[short])\n\nThe condition !prefixcmp(name, \"upstream\") must be true for the\nvariable \"branch\" to be reused, so the variable should be always set\nwhen it gets used, but GCC does not seem to realize this fact.\n---\n builtin/for-each-ref.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex ed81407..67168a1 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -641,7 +641,7 @@ static void populate_value(struct refinfo *ref)\n \t\tint deref = 0;\n \t\tconst char *refname;\n \t\tconst char *formatp;\n-\t\tstruct branch *branch;\n+\t\tstruct branch *branch = NULL;\n \n \t\tif (*name == '*') {\n \t\t\tderef = 1;\n@@ -719,6 +719,7 @@ static void populate_value(struct refinfo *ref)\n \t\t\t} else if (!strcmp(formatp, \"trackshort\") &&\n \t\t\t\t!prefixcmp(name, \"upstream\")) {\n \n+\t\t\t\tassert(branch != NULL);\n \t\t\t\tstat_tracking_info(branch, &num_ours, &num_theirs);\n \t\t\t\tif (!num_ours && !num_theirs)\n \t\t\t\t\tv->s = \"=\";\n-- \n1.8.5-rc1-322-g9c05f5a\n"},{"id":"230573","messageId":"xmqqfvr0dr33.fsf@gitster.dls.corp.google.com","threadId":"35330","inReplyTo":"xmqqk3gcdr6z.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 2/3] for-each-ref: introduce %(upstream:track[short])","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-13T19:44:32Z","receivedAt":"2013-11-13T19:44:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> You need to squash 78465bb,...\n\noops; make it 2249926. 784 is a cherry-pick of it on top of this round.\n"},{"id":"230574","messageId":"xmqqbo1odqb4.fsf@gitster.dls.corp.google.com","threadId":"35330","inReplyTo":"1384335406-16332-4-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH v2 3/3] for-each-ref: introduce %(color:...) for color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-13T20:01:19Z","receivedAt":"2013-11-13T20:01:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> +\t\t} else if (!prefixcmp(name, \"color\")) {\n> +\t\t\t;\n\nHow is \"%(color:short)\" parsed with this code?\n\nThis part says, \"Yeah, I see something starting with color\", and\nthen later strchr(name, ':') will point formatp to \"short\".\n\nLuckily, \"%(colorgarbage:short)\" does not even come this far because\nparse_atom() would not have allowed the codeflow to, but comparing\nwith \"color:\" here may be a lot more defensive and safe, I think.\n\nAnd find the color-value here, stuffing v->s inside this \"else if\",\ncontinue, without letting the formatp part work on refname this\npiece of code does not even set. Just like how we handle \"flag\"\nwithout falling thru to the formatp code.\n\n\n> +\t\t} else if (!strcmp(name, \"flag\")) {\n> \t\t\tchar buf[256], *cp = buf;\n> \t\t\tif (ref->flag & REF_ISSYMREF)\n> \t\t\t\tcp = copy_advance(cp, \",symref\");\n> @@ -729,6 +732,12 @@ static void populate_value(struct refinfo *ref)\n> \t\t\t\telse\n> \t\t\t\t\tv->s = \"<>\";\n> \t\t\t\tcontinue;\n> +\t\t\t} else if (!prefixcmp(name, \"color\")) {\n> +\t\t\t\tchar color[COLOR_MAXLEN] = \"\";\n> +\n> +\t\t\t\tcolor_parse(formatp, \"--format\", color);\n> +\t\t\t\tv->s = xstrdup(color);\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);\n"},{"id":"230576","messageId":"xmqq38n0dpgy.fsf@gitster.dls.corp.google.com","threadId":"35330","inReplyTo":"xmqqbo1odqb4.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 3/3] for-each-ref: introduce %(color:...) for color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-13T20:19:25Z","receivedAt":"2013-11-13T20:19:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ramkumar Ramachandra writes:\n>\n>> +\t\t} else if (!prefixcmp(name, \"color\")) {\n>> +\t\t\t;\n>\n> How is \"%(color:short)\" parsed with this code?\n>\n> This part says, \"Yeah, I see something starting with color\", and\n> then later strchr(name, ':') will point formatp to \"short\".\n>\n> Luckily, \"%(colorgarbage:short)\" does not even come this far because\n> parse_atom() would not have allowed the codeflow to, but comparing\n> with \"color:\" here may be a lot more defensive and safe, I think.\n>\n> And find the color-value here, stuffing v->s inside this \"else if\",\n> continue, without letting the formatp part work on refname this\n> piece of code does not even set. Just like how we handle \"flag\"\n> without falling thru to the formatp code.\n\nPerhaps like this (obviously not tested as these three patches did\nnot add any tests ;-)\n\nI also think that there should be a mechanism to do \"color:reset\"\nafter each record is issued automatically, and also have the color\noutput honor --color=auto from the command line, i.e.\n\n\tgit for-each-ref --color=auto --format='%(color:blue)%(subject)' | cat\n\nshould turn the coloring off.\n\nSo I think this patch may be a first step in the right direction,\nbut there are quite a lot more work that is needed before it gets\nready for production use.\n\nThanks.\n\n builtin/for-each-ref.c | 14 ++++++--------\n 1 file changed, 6 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 9e07571..07a9385 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -664,8 +664,12 @@ static void populate_value(struct refinfo *ref)\n \t\t\t !branch->merge[0]->dst)\n \t\t\t\tcontinue;\n \t\t\trefname = branch->merge[0]->dst;\n-\t\t} else if (!prefixcmp(name, \"color\")) {\n-\t\t\t;\n+\t\t} else if (!prefixcmp(name, \"color:\")) {\n+\t\t\tchar color[COLOR_MAXLEN] = \"\";\n+\n+\t\t\tcolor_parse(name + 6, \"--format\", color);\n+\t\t\tv->s = xstrdup(color);\n+\t\t\tcontinue;\n \t\t} else if (!strcmp(name, \"flag\")) {\n \t\t\tchar buf[256], *cp = buf;\n \t\t\tif (ref->flag & REF_ISSYMREF)\n@@ -733,12 +737,6 @@ static void populate_value(struct refinfo *ref)\n \t\t\t\telse\n \t\t\t\t\tv->s = \"<>\";\n \t\t\t\tcontinue;\n-\t\t\t} else if (!prefixcmp(name, \"color\")) {\n-\t\t\t\tchar color[COLOR_MAXLEN] = \"\";\n-\n-\t\t\t\tcolor_parse(formatp, \"--format\", color);\n-\t\t\t\tv->s = xstrdup(color);\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);\n"},{"id":"230585","messageId":"CALkWK0kSuSLjmShPx6cibysjvhrU0xqBvm=ivGLCn3L1kTDBFw@mail.gmail.com","threadId":"35330","inReplyTo":"xmqqk3gcdr6z.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 2/3] for-each-ref: introduce %(upstream:track[short])","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-11-14T06:53:29Z","receivedAt":"2013-11-14T06:53:29Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> builtin/for-each-ref.c: In function 'populate_value':\n> builtin/for-each-ref.c:701:13: error: 'refname' may be used uninitialized in this function [-Werror=uninitialized]\n\nIn my defense, the gcc on my end (4.8.2) doesn't seem to complain. Are\nyou using extra cflags?\n\nHowever, I do get the following compile warnings:\n\nwt-status.c: In function ‘wt_status_print_unmerged_header’:\nwt-status.c:187:2: warning: zero-length gnu_printf format string\n[-Wformat-zero-length]\n status_printf_ln(s, c, \"\");\n ^\nwt-status.c: In function ‘wt_status_print_cached_header’:\nwt-status.c:203:2: warning: zero-length gnu_printf format string\n[-Wformat-zero-length]\n status_printf_ln(s, c, \"\");\n ^\nwt-status.c: In function ‘wt_status_print_dirty_header’:\nwt-status.c:222:2: warning: zero-length gnu_printf format string\n[-Wformat-zero-length]\n status_printf_ln(s, c, \"\");\n ^\nwt-status.c: In function ‘wt_status_print_other_header’:\nwt-status.c:234:2: warning: zero-length gnu_printf format string\n[-Wformat-zero-length]\n status_printf_ln(s, c, \"\");\n ^\nwt-status.c: In function ‘wt_status_print_trailer’:\nwt-status.c:239:2: warning: zero-length gnu_printf format string\n[-Wformat-zero-length]\n status_printf_ln(s, color(WT_STATUS_HEADER, s), \"\");\n ^\nwt-status.c: In function ‘wt_status_print_other’:\nwt-status.c:767:2: warning: zero-length gnu_printf format string\n[-Wformat-zero-length]\n status_printf_ln(s, GIT_COLOR_NORMAL, \"\");\n ^\nwt-status.c: In function ‘wt_status_print_tracking’:\nwt-status.c:827:3: warning: zero-length gnu_printf format string\n[-Wformat-zero-length]\n fprintf_ln(s->fp, \"\");\n ^\nwt-status.c: In function ‘wt_status_print’:\nwt-status.c:1243:3: warning: zero-length gnu_printf format string\n[-Wformat-zero-length]\n status_printf(s, color(WT_STATUS_HEADER, s), \"\");\n ^\nwt-status.c:1256:3: warning: zero-length gnu_printf format string\n[-Wformat-zero-length]\n status_printf_ln(s, color(WT_STATUS_HEADER, s), \"\");\n ^\nwt-status.c:1258:3: warning: zero-length gnu_printf format string\n[-Wformat-zero-length]\n status_printf_ln(s, color(WT_STATUS_HEADER, s), \"\");\n ^\nwt-status.c:1275:4: warning: zero-length gnu_printf format string\n[-Wformat-zero-length]\n status_printf_ln(s, GIT_COLOR_NORMAL, \"\");\n ^\nbuiltin/commit.c: In function ‘prepare_to_commit’:\nbuiltin/commit.c:808:4: warning: zero-length gnu_printf format string\n[-Wformat-zero-length]\n status_printf_ln(s, GIT_COLOR_NORMAL, \"\");\n ^\n"},{"id":"230586","messageId":"CALkWK0k4MudojZt4PkLjnq0uZe322n30WP-=N0ckLw2QcnbzMA@mail.gmail.com","threadId":"35330","inReplyTo":"xmqq38n0dpgy.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 3/3] for-each-ref: introduce %(color:...) for color","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-11-14T07:03:05Z","receivedAt":"2013-11-14T07:03:05Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> Perhaps like this (obviously not tested as these three patches did\n> not add any tests ;-)\n\nSorry about that. I didn't notice t6300-for-each-ref.sh. Will fix in\nthe next round.\n\n> I also think that there should be a mechanism to do \"color:reset\"\n> after each record is issued automatically, and also have the color\n> output honor --color=auto from the command line, i.e.\n>\n> git for-each-ref --color=auto --format='%(color:blue)%(subject)' | cat\n>\n> should turn the coloring off.\n\nWe can add --color=auto later, but I'm wondering about auto-reset\ncolor after each token. What happens if I do:\n\n $ git for-each-ref --format='%(subject)%(color:blue)'\n\nNo color, right? So, it should be auto-reset color after each token\n_and_ at end of format-string.\n"},{"id":"230756","messageId":"xmqq4n79d6f1.fsf@gitster.dls.corp.google.com","threadId":"35330","inReplyTo":"CALkWK0k4MudojZt4PkLjnq0uZe322n30WP-=N0ckLw2QcnbzMA@mail.gmail.com","subject":"Re: [PATCH v2 3/3] for-each-ref: introduce %(color:...) for color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-18T16:24:34Z","receivedAt":"2013-11-18T16:24:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Junio C Hamano wrote:\n>> Perhaps like this (obviously not tested as these three patches did\n>> not add any tests ;-)\n>\n> Sorry about that. I didn't notice t6300-for-each-ref.sh. Will fix in\n> the next round.\n>\n>> I also think that there should be a mechanism to do \"color:reset\"\n>> after each record is issued automatically, and also have the color\n>> output honor --color=auto from the command line, i.e.\n>>\n>> git for-each-ref --color=auto --format='%(color:blue)%(subject)' | cat\n>>\n>> should turn the coloring off.\n>\n> We can add --color=auto later, but I'm wondering about auto-reset\n> color after each token. What happens if I do:\n>\n> $ git for-each-ref --format='%(subject)%(color:blue)'\n>\n> No color, right? So, it should be auto-reset color after each token\n> _and_ at end of format-string.\n\nIf you are saying, by after each token, that\n\n\t--format='%(color:blue)%(A)literal string%(B)'\n\nshould result in\n\n\t \"literal string\" \n\nthen I would disagree. I was suggesting it to instead produce\n\n\t \"literal string\" \n\nwhere the always comes when some color is used and we\nhit the end of the format string. A bonus point if we can make it so\nthat we emit the final reset only when the last \"%(color:some)\" is\nnot \"%(color:reset)\", but unconditional \"reset if we ever used\ncolor\" is fine.\n\nThanks.\n"},{"id":"230757","messageId":"CALkWK0ngKTkFbaq7NzCW7T3KyfmWxpDEv8TLj05LoThob1fpFg@mail.gmail.com","threadId":"35330","inReplyTo":"xmqq4n79d6f1.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 3/3] for-each-ref: introduce %(color:...) for color","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-11-18T16:45:30Z","receivedAt":"2013-11-18T16:45:30Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> If you are saying, by after each token, that\n>\n> --format='%(color:blue)%(A)literal string%(B)'\n>\n> should result in\n>\n> \"literal string\" \n>\n> then I would disagree.\n\nHm, I didn't think it was a bad idea to reset after each token. The\nwhole point of having color is to make sure that two consecutive\ntokens don't have the same color, no? Then again, my scheme would\nresult in extra unnecessary resets like\n\n %(color:blue)%(A)%(color:green)%(B)\n\nbeing turned into:\n\n %(color:blue)%(A)%(color:reset)%(color:green)%(B)%(color:reset)\n\nHere, the first %(color:reset) is completely unnecessary.\n\n> I was suggesting it to instead produce\n>\n> \"literal string\" \n>\n> where the always comes when some color is used and we\n> hit the end of the format string. A bonus point if we can make it so\n> that we emit the final reset only when the last \"%(color:some)\" is\n> not \"%(color:reset)\", but unconditional \"reset if we ever used\n> color\" is fine.\n\nOkay, a simple don't-leak-color. I'll submit another iteration soon.\n\nThanks.\n"}]}