{"thread":{"id":"18762","subject":"[RFC/PATCH 0/5] making upstream branch information accessible","startedAt":"2009-04-07T07:02:54Z","lastAt":"2009-04-13T17:04:49Z","messageCount":24,"participants":["Jeff King","Bert Wesarg","Michael J Gruber","Paolo Ciarrocchi","Junio C Hamano","Santi Béjar","Wincent Colaiuta"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"110657","messageId":"20090407070254.GA2870@coredump.intra.peff.net","threadId":"18762","inReplyTo":null,"subject":"[RFC/PATCH 0/5] making upstream branch information accessible","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-07T07:02:54Z","receivedAt":"2009-04-07T07:02:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Here are slightly more cleaned-up versions of patches I've thrown out in\nthe last few days. The aim is to make the information on\nupstream/tracking relationships more accessible both to scripts and to\nusers.\n\n  1/5: for-each-ref: refactor get_short_ref function\n  2/5: for-each-ref: refactor refname handling\n\n    Cleanup for 3/5.\n\n  3/5: for-each-ref: add \"upstream\" format field\n\n    Plumbing support.\n\n  4/5: make get_short_ref a public function\n\n    Clean up for 4/5. Builds on 1/5.\n\n  5/5: branch: show upstream branch when double verbose\n\n-Peff\n"},{"id":"110658","messageId":"20090407070501.GA2924@coredump.intra.peff.net","threadId":"18762","inReplyTo":"20090407070254.GA2870@coredump.intra.peff.net","subject":"[PATCH 1/5] for-each-ref: refactor get_short_ref function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-07T07:05:01Z","receivedAt":"2009-04-07T07:05:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This function took a \"refinfo\" object which is unnecessarily\nrestrictive; it only ever looked at the refname field. This\npatch refactors it to take just the ref name as a string.\n\nWhile we're touching the relevant lines, let's give it\nconsistent memory semantics. Previously, some code paths\nwould return an allocated string and some would return the\noriginal string; now it will always return a malloc'd\nstring.\n\nThis doesn't actually fix a bug or a leak, because\nfor-each-ref doesn't clean up its memory, but it makes the\nfunction a lot less surprising for reuse (which will\nhappen in a later patch).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nActually, as the modified version is always prefix-shortened,\nit should be possible to rewrite this in a way that never allocates\nmemory. I picked the least-invasive and most lazy approach.\n\n builtin-for-each-ref.c |   14 +++++++-------\n 1 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c\nindex 5cbb4b0..4aaf75c 100644\n--- a/builtin-for-each-ref.c\n+++ b/builtin-for-each-ref.c\n@@ -569,7 +569,7 @@ static void gen_scanf_fmt(char *scanf_fmt, const char *rule)\n /*\n  * Shorten the refname to an non-ambiguous form\n  */\n-static char *get_short_ref(struct refinfo *ref)\n+static char *get_short_ref(const char *ref)\n {\n \tint i;\n \tstatic char **scanf_fmts;\n@@ -598,17 +598,17 @@ static char *get_short_ref(struct refinfo *ref)\n \n \t/* bail out if there are no rules */\n \tif (!nr_rules)\n-\t\treturn ref->refname;\n+\t\treturn xstrdup(ref);\n \n-\t/* buffer for scanf result, at most ref->refname must fit */\n-\tshort_name = xstrdup(ref->refname);\n+\t/* buffer for scanf result, at most ref must fit */\n+\tshort_name = xstrdup(ref);\n \n \t/* skip first rule, it will always match */\n \tfor (i = nr_rules - 1; i > 0 ; --i) {\n \t\tint j;\n \t\tint short_name_len;\n \n-\t\tif (1 != sscanf(ref->refname, scanf_fmts[i], short_name))\n+\t\tif (1 != sscanf(ref, scanf_fmts[i], short_name))\n \t\t\tcontinue;\n \n \t\tshort_name_len = strlen(short_name);\n@@ -642,7 +642,7 @@ static char *get_short_ref(struct refinfo *ref)\n \t}\n \n \tfree(short_name);\n-\treturn ref->refname;\n+\treturn xstrdup(ref);\n }\n \n \n@@ -684,7 +684,7 @@ static void populate_value(struct refinfo *ref)\n \t\t\tif (formatp) {\n \t\t\t\tformatp++;\n \t\t\t\tif (!strcmp(formatp, \"short\"))\n-\t\t\t\t\trefname = get_short_ref(ref);\n+\t\t\t\t\trefname = get_short_ref(ref->refname);\n \t\t\t\telse\n \t\t\t\t\tdie(\"unknown refname format %s\",\n \t\t\t\t\t    formatp);\n-- \n1.6.2.2.450.gd6aa9.dirty\n"},{"id":"110659","messageId":"20090407070651.GB2924@coredump.intra.peff.net","threadId":"18762","inReplyTo":"20090407070254.GA2870@coredump.intra.peff.net","subject":"[PATCH 2/5] for-each-ref: refactor refname handling","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-07T07:06:51Z","receivedAt":"2009-04-07T07:06:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This code handles some special magic like *-deref and the\n:short formatting specifier. The next patch will add another\nfield which outputs a ref and wants to use the same code.\n\nThis patch splits the \"which ref are we outputting\" from the\nactual formatting. There should be no behavioral change.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThe diff is scary, but it is mostly reindentation.\n\n builtin-for-each-ref.c |   47 ++++++++++++++++++++++++++---------------------\n 1 files changed, 26 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c\nindex 4aaf75c..b50c93b 100644\n--- a/builtin-for-each-ref.c\n+++ b/builtin-for-each-ref.c\n@@ -672,32 +672,37 @@ static void populate_value(struct refinfo *ref)\n \t\tconst char *name = used_atom[i];\n \t\tstruct atom_value *v = &ref->value[i];\n \t\tint deref = 0;\n+\t\tconst char *refname;\n+\t\tconst char *formatp;\n+\n \t\tif (*name == '*') {\n \t\t\tderef = 1;\n \t\t\tname++;\n \t\t}\n-\t\tif (!prefixcmp(name, \"refname\")) {\n-\t\t\tconst char *formatp = strchr(name, ':');\n-\t\t\tconst char *refname = ref->refname;\n-\n-\t\t\t/* look for \"short\" refname format */\n-\t\t\tif (formatp) {\n-\t\t\t\tformatp++;\n-\t\t\t\tif (!strcmp(formatp, \"short\"))\n-\t\t\t\t\trefname = get_short_ref(ref->refname);\n-\t\t\t\telse\n-\t\t\t\t\tdie(\"unknown refname format %s\",\n-\t\t\t\t\t    formatp);\n-\t\t\t}\n \n-\t\t\tif (!deref)\n-\t\t\t\tv->s = refname;\n-\t\t\telse {\n-\t\t\t\tint len = strlen(refname);\n-\t\t\t\tchar *s = xmalloc(len + 4);\n-\t\t\t\tsprintf(s, \"%s^{}\", refname);\n-\t\t\t\tv->s = s;\n-\t\t\t}\n+\t\tif (!prefixcmp(name, \"refname\"))\n+\t\t\trefname = ref->refname;\n+\t\telse\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\tformatp++;\n+\t\t\tif (!strcmp(formatp, \"short\"))\n+\t\t\t\trefname = get_short_ref(refname);\n+\t\t\telse\n+\t\t\t\tdie(\"unknown %.*s format %s\",\n+\t\t\t\t\tformatp - name, name, formatp);\n+\t\t}\n+\n+\t\tif (!deref)\n+\t\t\tv->s = refname;\n+\t\telse {\n+\t\t\tint len = strlen(refname);\n+\t\t\tchar *s = xmalloc(len + 4);\n+\t\t\tsprintf(s, \"%s^{}\", refname);\n+\t\t\tv->s = s;\n \t\t}\n \t}\n \n-- \n1.6.2.2.450.gd6aa9.dirty\n"},{"id":"110661","messageId":"20090407070939.GC2924@coredump.intra.peff.net","threadId":"18762","inReplyTo":"20090407070254.GA2870@coredump.intra.peff.net","subject":"[PATCH 3/5] for-each-ref: add \"upstream\" format field","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-07T07:09:39Z","receivedAt":"2009-04-07T07:09:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The logic for determining the upstream ref of a branch is\nsomewhat complex to perform in a shell script. This patch\nprovides a plumbing mechanism for scripts to access the C\nlogic used internally by git-status, git-branch, etc.\n\nFor example:\n\n  $ git for-each-ref \\\n       --format='%(refname:short) %(upstream:short)' \\\n       refs/heads/\n  master origin/master\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis is a cleaned-up version of what I sent previously. Mainly just code\ncleanups, and it no longer frees the branch struct, which seems to be\nallocated from semi-permanent storage during branch_get.\n\nShould the documentation explain the concept of \"upstream\" more fully? I\nnoticed Santi sent a glossary patch earlier today, so maybe that is\nenough.\n\n Documentation/git-for-each-ref.txt |    5 +++++\n builtin-for-each-ref.c             |   14 ++++++++++++++\n t/t6300-for-each-ref.sh            |   22 ++++++++++++++++++++++\n 3 files changed, 41 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 5061d3e..b362e9e 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -85,6 +85,11 @@ objectsize::\n objectname::\n \tThe object name (aka SHA-1).\n \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+\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 b50c93b..277d1fb 100644\n--- a/builtin-for-each-ref.c\n+++ b/builtin-for-each-ref.c\n@@ -8,6 +8,7 @@\n #include \"blob.h\"\n #include \"quote.h\"\n #include \"parse-options.h\"\n+#include \"remote.h\"\n \n /* Quoting styles */\n #define QUOTE_NONE 0\n@@ -66,6 +67,7 @@ static struct {\n \t{ \"subject\" },\n \t{ \"body\" },\n \t{ \"contents\" },\n+\t{ \"upstream\" },\n };\n \n /*\n@@ -682,6 +684,18 @@ static void populate_value(struct refinfo *ref)\n \n \t\tif (!prefixcmp(name, \"refname\"))\n \t\t\trefname = ref->refname;\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+\t\t\tbranch = branch_get(ref->refname + 11);\n+\n+\t\t\tif (!branch || !branch->merge || !branch->merge[0] ||\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\n \t\t\tcontinue;\n \ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 8bfae44..daf02d5 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -26,6 +26,13 @@ test_expect_success 'Create sample commit with known timestamp' '\n \tgit tag -a -m \"Tagging at $datestamp\" testtag\n '\n \n+test_expect_success 'Create upstream config' '\n+\tgit update-ref refs/remotes/origin/master master &&\n+\tgit remote add origin nowhere &&\n+\tgit config branch.master.remote origin &&\n+\tgit config branch.master.merge refs/heads/master\n+'\n+\n test_atom() {\n \tcase \"$1\" in\n \t\thead) ref=refs/heads/master ;;\n@@ -39,6 +46,7 @@ test_atom() {\n }\n \n test_atom head refname refs/heads/master\n+test_atom head upstream refs/remotes/origin/master\n test_atom head objecttype commit\n test_atom head objectsize 171\n test_atom head objectname 67a36f10722846e891fbada1ba48ed035de75581\n@@ -68,6 +76,7 @@ test_atom head contents 'Initial\n '\n \n test_atom tag refname refs/tags/testtag\n+test_atom tag upstream ''\n test_atom tag objecttype tag\n test_atom tag objectsize 154\n test_atom tag objectname 98b46b1d36e5b07909de1b3886224e3e81e87322\n@@ -203,6 +212,7 @@ test_expect_success 'Check format \"rfc2822\" date fields output' '\n \n cat >expected <<\\EOF\n refs/heads/master\n+refs/remotes/origin/master\n refs/tags/testtag\n EOF\n \n@@ -214,6 +224,7 @@ test_expect_success 'Verify ascending sort' '\n \n cat >expected <<\\EOF\n refs/tags/testtag\n+refs/remotes/origin/master\n refs/heads/master\n EOF\n \n@@ -224,6 +235,7 @@ test_expect_success 'Verify descending sort' '\n \n cat >expected <<\\EOF\n 'refs/heads/master'\n+'refs/remotes/origin/master'\n 'refs/tags/testtag'\n EOF\n \n@@ -244,6 +256,7 @@ test_expect_success 'Quoting style: python' '\n \n cat >expected <<\\EOF\n \"refs/heads/master\"\n+\"refs/remotes/origin/master\"\n \"refs/tags/testtag\"\n EOF\n \n@@ -273,6 +286,15 @@ test_expect_success 'Check short refname format' '\n \ttest_cmp expected actual\n '\n \n+cat >expected <<EOF\n+origin/master\n+EOF\n+\n+test_expect_success 'Check short upstream format' '\n+\tgit for-each-ref --format=\"%(upstream:short)\" refs/heads >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'Check for invalid refname format' '\n \ttest_must_fail git for-each-ref --format=\"%(refname:INVALID)\"\n '\n-- \n1.6.2.2.450.gd6aa9.dirty\n"},{"id":"110662","messageId":"20090407071420.GD2924@coredump.intra.peff.net","threadId":"18762","inReplyTo":"20090407070254.GA2870@coredump.intra.peff.net","subject":"[PATCH 4/5] make get_short_ref a public function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-07T07:14:20Z","receivedAt":"2009-04-07T07:14:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Often we want to shorten a full ref name to something \"prettier\"\nto show a user. For example, \"refs/heads/master\" is often shown\nsimply as \"master\", or \"refs/remotes/origin/master\" is shown as\n\"origin/master\".\n\nMany places in the code use a very simple formula: skip common\nprefixes like refs/heads, refs/remotes, etc. This is codified in\nthe prettify_ref function.\n\nfor-each-ref has a more correct (but more expensive) approach:\nconsider the ref lookup rules, and try shortening as much as\npossible while remaining unambiguous.\n\nThis patch makes the latter strategy globally available as\nshorten_unambiguous_ref.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nActually, I am not quite sure that this function is \"more correct\". It\nlooks at the rev-parsing rules as a hierarchy, so if you have\n\"refs/remotes/foo\" and \"refs/heads/foo\", then it will abbreviate the\nfirst to \"remotes/foo\" (as expected) and the latter to just \"foo\".\n\nThis is technically correct, as \"refs/heads/foo\" will be selected by\n\"foo\", but it will warn about ambiguity. Should we actually try to avoid\nreporting refs which would be ambiguous?\n\nShould this simply replace prettify_ref (and other places which should\nbe using prettify_ref but aren't)? It is definitely more expensive, as\nit has to resolve refs to look for ambiguities, but I don't know if we\ncare in most code paths.\n\n builtin-for-each-ref.c |  105 +-----------------------------------------------\n refs.c                 |   99 +++++++++++++++++++++++++++++++++++++++++++++\n refs.h                 |    1 +\n 3 files changed, 101 insertions(+), 104 deletions(-)\n\ndiff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c\nindex 277d1fb..8c82484 100644\n--- a/builtin-for-each-ref.c\n+++ b/builtin-for-each-ref.c\n@@ -546,109 +546,6 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, v\n }\n \n /*\n- * generate a format suitable for scanf from a ref_rev_parse_rules\n- * rule, that is replace the \"%.*s\" spec with a \"%s\" spec\n- */\n-static void gen_scanf_fmt(char *scanf_fmt, const char *rule)\n-{\n-\tchar *spec;\n-\n-\tspec = strstr(rule, \"%.*s\");\n-\tif (!spec || strstr(spec + 4, \"%.*s\"))\n-\t\tdie(\"invalid rule in ref_rev_parse_rules: %s\", rule);\n-\n-\t/* copy all until spec */\n-\tstrncpy(scanf_fmt, rule, spec - rule);\n-\tscanf_fmt[spec - rule] = '\\0';\n-\t/* copy new spec */\n-\tstrcat(scanf_fmt, \"%s\");\n-\t/* copy remaining rule */\n-\tstrcat(scanf_fmt, spec + 4);\n-\n-\treturn;\n-}\n-\n-/*\n- * Shorten the refname to an non-ambiguous form\n- */\n-static char *get_short_ref(const char *ref)\n-{\n-\tint i;\n-\tstatic char **scanf_fmts;\n-\tstatic int nr_rules;\n-\tchar *short_name;\n-\n-\t/* pre generate scanf formats from ref_rev_parse_rules[] */\n-\tif (!nr_rules) {\n-\t\tsize_t total_len = 0;\n-\n-\t\t/* the rule list is NULL terminated, count them first */\n-\t\tfor (; ref_rev_parse_rules[nr_rules]; nr_rules++)\n-\t\t\t/* no +1 because strlen(\"%s\") < strlen(\"%.*s\") */\n-\t\t\ttotal_len += strlen(ref_rev_parse_rules[nr_rules]);\n-\n-\t\tscanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len);\n-\n-\t\ttotal_len = 0;\n-\t\tfor (i = 0; i < nr_rules; i++) {\n-\t\t\tscanf_fmts[i] = (char *)&scanf_fmts[nr_rules]\n-\t\t\t\t\t+ total_len;\n-\t\t\tgen_scanf_fmt(scanf_fmts[i], ref_rev_parse_rules[i]);\n-\t\t\ttotal_len += strlen(ref_rev_parse_rules[i]);\n-\t\t}\n-\t}\n-\n-\t/* bail out if there are no rules */\n-\tif (!nr_rules)\n-\t\treturn xstrdup(ref);\n-\n-\t/* buffer for scanf result, at most ref must fit */\n-\tshort_name = xstrdup(ref);\n-\n-\t/* skip first rule, it will always match */\n-\tfor (i = nr_rules - 1; i > 0 ; --i) {\n-\t\tint j;\n-\t\tint short_name_len;\n-\n-\t\tif (1 != sscanf(ref, scanf_fmts[i], short_name))\n-\t\t\tcontinue;\n-\n-\t\tshort_name_len = strlen(short_name);\n-\n-\t\t/*\n-\t\t * check if the short name resolves to a valid ref,\n-\t\t * but use only rules prior to the matched one\n-\t\t */\n-\t\tfor (j = 0; j < i; j++) {\n-\t\t\tconst char *rule = ref_rev_parse_rules[j];\n-\t\t\tunsigned char short_objectname[20];\n-\t\t\tchar refname[PATH_MAX];\n-\n-\t\t\t/*\n-\t\t\t * the short name is ambiguous, if it resolves\n-\t\t\t * (with this previous rule) to a valid ref\n-\t\t\t * read_ref() returns 0 on success\n-\t\t\t */\n-\t\t\tmksnpath(refname, sizeof(refname),\n-\t\t\t\t rule, short_name_len, short_name);\n-\t\t\tif (!read_ref(refname, short_objectname))\n-\t\t\t\tbreak;\n-\t\t}\n-\n-\t\t/*\n-\t\t * short name is non-ambiguous if all previous rules\n-\t\t * haven't resolved to a valid ref\n-\t\t */\n-\t\tif (j == i)\n-\t\t\treturn short_name;\n-\t}\n-\n-\tfree(short_name);\n-\treturn xstrdup(ref);\n-}\n-\n-\n-/*\n  * Parse the object referred by ref, and grab needed value.\n  */\n static void populate_value(struct refinfo *ref)\n@@ -704,7 +601,7 @@ static void populate_value(struct refinfo *ref)\n \t\tif (formatp) {\n \t\t\tformatp++;\n \t\t\tif (!strcmp(formatp, \"short\"))\n-\t\t\t\trefname = get_short_ref(refname);\n+\t\t\t\trefname = shorten_unambiguous_ref(refname);\n \t\t\telse\n \t\t\t\tdie(\"unknown %.*s format %s\",\n \t\t\t\t\tformatp - name, name, formatp);\ndiff --git a/refs.c b/refs.c\nindex 59c373f..1e5e7b4 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1652,3 +1652,102 @@ struct ref *find_ref_by_name(const struct ref *list, const char *name)\n \t\t\treturn (struct ref *)list;\n \treturn NULL;\n }\n+\n+/*\n+ * generate a format suitable for scanf from a ref_rev_parse_rules\n+ * rule, that is replace the \"%.*s\" spec with a \"%s\" spec\n+ */\n+static void gen_scanf_fmt(char *scanf_fmt, const char *rule)\n+{\n+\tchar *spec;\n+\n+\tspec = strstr(rule, \"%.*s\");\n+\tif (!spec || strstr(spec + 4, \"%.*s\"))\n+\t\tdie(\"invalid rule in ref_rev_parse_rules: %s\", rule);\n+\n+\t/* copy all until spec */\n+\tstrncpy(scanf_fmt, rule, spec - rule);\n+\tscanf_fmt[spec - rule] = '\\0';\n+\t/* copy new spec */\n+\tstrcat(scanf_fmt, \"%s\");\n+\t/* copy remaining rule */\n+\tstrcat(scanf_fmt, spec + 4);\n+\n+\treturn;\n+}\n+\n+char *shorten_unambiguous_ref(const char *ref)\n+{\n+\tint i;\n+\tstatic char **scanf_fmts;\n+\tstatic int nr_rules;\n+\tchar *short_name;\n+\n+\t/* pre generate scanf formats from ref_rev_parse_rules[] */\n+\tif (!nr_rules) {\n+\t\tsize_t total_len = 0;\n+\n+\t\t/* the rule list is NULL terminated, count them first */\n+\t\tfor (; ref_rev_parse_rules[nr_rules]; nr_rules++)\n+\t\t\t/* no +1 because strlen(\"%s\") < strlen(\"%.*s\") */\n+\t\t\ttotal_len += strlen(ref_rev_parse_rules[nr_rules]);\n+\n+\t\tscanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len);\n+\n+\t\ttotal_len = 0;\n+\t\tfor (i = 0; i < nr_rules; i++) {\n+\t\t\tscanf_fmts[i] = (char *)&scanf_fmts[nr_rules]\n+\t\t\t\t\t+ total_len;\n+\t\t\tgen_scanf_fmt(scanf_fmts[i], ref_rev_parse_rules[i]);\n+\t\t\ttotal_len += strlen(ref_rev_parse_rules[i]);\n+\t\t}\n+\t}\n+\n+\t/* bail out if there are no rules */\n+\tif (!nr_rules)\n+\t\treturn xstrdup(ref);\n+\n+\t/* buffer for scanf result, at most ref must fit */\n+\tshort_name = xstrdup(ref);\n+\n+\t/* skip first rule, it will always match */\n+\tfor (i = nr_rules - 1; i > 0 ; --i) {\n+\t\tint j;\n+\t\tint short_name_len;\n+\n+\t\tif (1 != sscanf(ref, scanf_fmts[i], short_name))\n+\t\t\tcontinue;\n+\n+\t\tshort_name_len = strlen(short_name);\n+\n+\t\t/*\n+\t\t * check if the short name resolves to a valid ref,\n+\t\t * but use only rules prior to the matched one\n+\t\t */\n+\t\tfor (j = 0; j < i; j++) {\n+\t\t\tconst char *rule = ref_rev_parse_rules[j];\n+\t\t\tunsigned char short_objectname[20];\n+\t\t\tchar refname[PATH_MAX];\n+\n+\t\t\t/*\n+\t\t\t * the short name is ambiguous, if it resolves\n+\t\t\t * (with this previous rule) to a valid ref\n+\t\t\t * read_ref() returns 0 on success\n+\t\t\t */\n+\t\t\tmksnpath(refname, sizeof(refname),\n+\t\t\t\t rule, short_name_len, short_name);\n+\t\t\tif (!read_ref(refname, short_objectname))\n+\t\t\t\tbreak;\n+\t\t}\n+\n+\t\t/*\n+\t\t * short name is non-ambiguous if all previous rules\n+\t\t * haven't resolved to a valid ref\n+\t\t */\n+\t\tif (j == i)\n+\t\t\treturn short_name;\n+\t}\n+\n+\tfree(short_name);\n+\treturn xstrdup(ref);\n+}\ndiff --git a/refs.h b/refs.h\nindex 68c2d16..2d0f961 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -80,6 +80,7 @@ extern int for_each_reflog(each_ref_fn, void *);\n extern int check_ref_format(const char *target);\n \n extern const char *prettify_ref(const struct ref *ref);\n+extern char *shorten_unambiguous_ref(const char *ref);\n \n /** rename ref, return 0 on success **/\n extern int rename_ref(const char *oldref, const char *newref, const char *logmsg);\n-- \n1.6.2.2.450.gd6aa9.dirty\n"},{"id":"110663","messageId":"20090407071656.GE2924@coredump.intra.peff.net","threadId":"18762","inReplyTo":"20090407070254.GA2870@coredump.intra.peff.net","subject":"[PATCH 5/5] branch: show upstream branch when double verbose","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-07T07:16:56Z","receivedAt":"2009-04-07T07:16:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This information is easily accessible when we are\ncalculating the relationship. The only reason not to print\nit all the time is that it consumes a fair bit of screen\nspace, and may not be of interest to the user.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis one is very RFC. Should this information be part of the regular\n\"-v\"? Should it be part of \"git branch\" with regular verbosity?\n\nShould the format be different? I wonder if\n\n  master 1234abcd [origin/master: ahead 5, behind 6] whatever\n\nwill be interpreted as \"origin/master is ahead 5, behind 6\" when it is\nreally the reverse. Maybe \"[ahead 5, behind 6 from origin/master]\" would\nbe better?\n\n Documentation/git-branch.txt |    4 +++-\n builtin-branch.c             |   23 +++++++++++++++++------\n 2 files changed, 20 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\nindex 31ba7f2..ba3dea6 100644\n--- a/Documentation/git-branch.txt\n+++ b/Documentation/git-branch.txt\n@@ -100,7 +100,9 @@ OPTIONS\n \n -v::\n --verbose::\n-\tShow sha1 and commit subject line for each head.\n+\tShow sha1 and commit subject line for each head, along with\n+\trelationship to upstream branch (if any). If given twice, print\n+\tthe name of the upstream branch, as well.\n \n --abbrev=<length>::\n \tAlter the sha1's minimum display length in the output listing.\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex ca81d72..3275821 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -301,19 +301,30 @@ static int ref_cmp(const void *r1, const void *r2)\n \treturn strcmp(c1->name, c2->name);\n }\n \n-static void fill_tracking_info(struct strbuf *stat, const char *branch_name)\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 \tstruct branch *branch = branch_get(branch_name);\n \n-\tif (!stat_tracking_info(branch, &ours, &theirs) || (!ours && !theirs))\n+\tif (!stat_tracking_info(branch, &ours, &theirs)) {\n+\t\tif (branch && branch->merge && branch->merge[0]->dst &&\n+\t\t    show_upstream_ref)\n+\t\t\tstrbuf_addf(stat, \"[%s] \",\n+\t\t\t    shorten_unambiguous_ref(branch->merge[0]->dst));\n \t\treturn;\n+\t}\n+\n+\tstrbuf_addch(stat, '[');\n+\tif (show_upstream_ref)\n+\t\tstrbuf_addf(stat, \"%s: \",\n+\t\t\tshorten_unambiguous_ref(branch->merge[0]->dst));\n \tif (!ours)\n-\t\tstrbuf_addf(stat, \"[behind %d] \", theirs);\n+\t\tstrbuf_addf(stat, \"behind %d] \", theirs);\n \telse if (!theirs)\n-\t\tstrbuf_addf(stat, \"[ahead %d] \", ours);\n+\t\tstrbuf_addf(stat, \"ahead %d] \", ours);\n \telse\n-\t\tstrbuf_addf(stat, \"[ahead %d, behind %d] \", ours, theirs);\n+\t\tstrbuf_addf(stat, \"ahead %d, behind %d] \", ours, theirs);\n }\n \n static int matches_merge_filter(struct commit *commit)\n@@ -379,7 +390,7 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n \t\t}\n \n \t\tif (item->kind == REF_LOCAL_BRANCH)\n-\t\t\tfill_tracking_info(&stat, item->name);\n+\t\t\tfill_tracking_info(&stat, item->name, verbose > 1);\n \n \t\tstrbuf_addf(&out, \" %s %s%s\",\n \t\t\tfind_unique_abbrev(item->commit->object.sha1, abbrev),\n-- \n1.6.2.2.450.gd6aa9.dirty\n"},{"id":"110667","messageId":"1239089599-24760-1-git-send-email-bert.wesarg@googlemail.com","threadId":"18762","inReplyTo":"20090407070254.GA2870@coredump.intra.peff.net","subject":"[PATCH] for-each-ref: remove multiple xstrdup() in get_short_ref()","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-04-07T07:33:19Z","receivedAt":"2009-04-07T07:33:19Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Now that get_short_ref() always return an malloced string, consolidate to\none xstrcpy() call.\n\nSigned-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n\n---\nAlso an \n\nAcked-by: Bert Wesarg <bert.wesarg@googlemail.com>\n\nfor Jeffs patch.\n\n builtin-for-each-ref.c |   11 +++++------\n 1 files changed, 5 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c\nindex 4aaf75c..108c128 100644\n--- a/builtin-for-each-ref.c\n+++ b/builtin-for-each-ref.c\n@@ -596,13 +596,13 @@ static char *get_short_ref(const char *ref)\n \t\t}\n \t}\n \n-\t/* bail out if there are no rules */\n-\tif (!nr_rules)\n-\t\treturn xstrdup(ref);\n-\n \t/* buffer for scanf result, at most ref must fit */\n \tshort_name = xstrdup(ref);\n \n+\t/* bail out if there are no rules */\n+\tif (!nr_rules)\n+\t\treturn short_name;\n+\n \t/* skip first rule, it will always match */\n \tfor (i = nr_rules - 1; i > 0 ; --i) {\n \t\tint j;\n@@ -641,8 +641,7 @@ static char *get_short_ref(const char *ref)\n \t\t\treturn short_name;\n \t}\n \n-\tfree(short_name);\n-\treturn xstrdup(ref);\n+\treturn strcpy(short_name, ref);\n }\n \n \n-- \ntg: (e9786d7..) bw/ammend-jk/refactor-get_short_ref (depends on: jk/refactor-get_short_ref)\n"},{"id":"110668","messageId":"36ca99e90904070039m15869c34jc9e12d5ccc48d82@mail.gmail.com","threadId":"18762","inReplyTo":"20090407071420.GD2924@coredump.intra.peff.net","subject":"Re: [PATCH 4/5] make get_short_ref a public function","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-04-07T07:39:58Z","receivedAt":"2009-04-07T07:39:58Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Tue, Apr 7, 2009 at 09:14, Jeff King <peff@peff.net> wrote:\n> Often we want to shorten a full ref name to something \"prettier\"\n> to show a user. For example, \"refs/heads/master\" is often shown\n> simply as \"master\", or \"refs/remotes/origin/master\" is shown as\n> \"origin/master\".\n>\n> Many places in the code use a very simple formula: skip common\n> prefixes like refs/heads, refs/remotes, etc. This is codified in\n> the prettify_ref function.\n>\n> for-each-ref has a more correct (but more expensive) approach:\n> consider the ref lookup rules, and try shortening as much as\n> possible while remaining unambiguous.\n>\n> This patch makes the latter strategy globally available as\n> shorten_unambiguous_ref.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> Actually, I am not quite sure that this function is \"more correct\". It\n> looks at the rev-parsing rules as a hierarchy, so if you have\n> \"refs/remotes/foo\" and \"refs/heads/foo\", then it will abbreviate the\n> first to \"remotes/foo\" (as expected) and the latter to just \"foo\".\n>\n> This is technically correct, as \"refs/heads/foo\" will be selected by\n> \"foo\", but it will warn about ambiguity. Should we actually try to avoid\n> reporting refs which would be ambiguous?\nBack than, there was the idea that the core.warnAmbiguousRefs config\ncould be used for this.\n\nAnyway\n\nAcked-by: Bert Wesarg <bert.wesarg@googlemail.com>\n"},{"id":"110670","messageId":"20090407074435.GB7327@coredump.intra.peff.net","threadId":"18762","inReplyTo":"1239089599-24760-1-git-send-email-bert.wesarg@googlemail.com","subject":"Re: [PATCH] for-each-ref: remove multiple xstrdup() in get_short_ref()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-07T07:44:35Z","receivedAt":"2009-04-07T07:44:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 07, 2009 at 09:33:19AM +0200, Bert Wesarg wrote:\n\n> Now that get_short_ref() always return an malloced string, consolidate to\n> one xstrcpy() call.\n\nMakes sense to squash in on top of what I have. But I think it actually\nis pretty easy to always return a pointer into the existing string\n(patch based on current master):\n\n---\ndiff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c\nindex 5cbb4b0..8b24a4a 100644\n--- a/builtin-for-each-ref.c\n+++ b/builtin-for-each-ref.c\n@@ -569,7 +569,7 @@ static void gen_scanf_fmt(char *scanf_fmt, const char *rule)\n /*\n  * Shorten the refname to an non-ambiguous form\n  */\n-static char *get_short_ref(struct refinfo *ref)\n+static const char *get_short_ref(const char *ref)\n {\n \tint i;\n \tstatic char **scanf_fmts;\n@@ -598,17 +598,17 @@ static char *get_short_ref(struct refinfo *ref)\n \n \t/* bail out if there are no rules */\n \tif (!nr_rules)\n-\t\treturn ref->refname;\n+\t\treturn ref;\n \n \t/* buffer for scanf result, at most ref->refname must fit */\n-\tshort_name = xstrdup(ref->refname);\n+\tshort_name = xstrdup(ref);\n \n \t/* skip first rule, it will always match */\n \tfor (i = nr_rules - 1; i > 0 ; --i) {\n \t\tint j;\n \t\tint short_name_len;\n \n-\t\tif (1 != sscanf(ref->refname, scanf_fmts[i], short_name))\n+\t\tif (1 != sscanf(ref, scanf_fmts[i], short_name))\n \t\t\tcontinue;\n \n \t\tshort_name_len = strlen(short_name);\n@@ -637,12 +637,14 @@ static char *get_short_ref(struct refinfo *ref)\n \t\t * short name is non-ambiguous if all previous rules\n \t\t * haven't resolved to a valid ref\n \t\t */\n-\t\tif (j == i)\n-\t\t\treturn short_name;\n+\t\tif (j == i) {\n+\t\t\tref += strlen(ref) - strlen(short_name);\n+\t\t\tbreak;\n+\t\t}\n \t}\n \n \tfree(short_name);\n-\treturn ref->refname;\n+\treturn ref;\n }\n \n \n@@ -684,7 +686,7 @@ static void populate_value(struct refinfo *ref)\n \t\t\tif (formatp) {\n \t\t\t\tformatp++;\n \t\t\t\tif (!strcmp(formatp, \"short\"))\n-\t\t\t\t\trefname = get_short_ref(ref);\n+\t\t\t\t\trefname = get_short_ref(ref->refname);\n \t\t\t\telse\n \t\t\t\t\tdie(\"unknown refname format %s\",\n \t\t\t\t\t    formatp);\n"},{"id":"110671","messageId":"36ca99e90904070044p48a4fe7bwb7278bde2d64837c@mail.gmail.com","threadId":"18762","inReplyTo":"1239089599-24760-1-git-send-email-bert.wesarg@googlemail.com","subject":"Re: [PATCH] for-each-ref: remove multiple xstrdup() in get_short_ref()","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-04-07T07:44:47Z","receivedAt":"2009-04-07T07:44:47Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Tue, Apr 7, 2009 at 09:33, Bert Wesarg <bert.wesarg@googlemail.com> wrote:\n> Now that get_short_ref() always return an malloced string, consolidate to\n> one xstrcpy() call.\n>\n> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n>\n> ---\n> Also an\n>\n> Acked-by: Bert Wesarg <bert.wesarg@googlemail.com>\nSorry, wrong Message-ID for In-Reply-To, this Ack is for:\n\n[PATCH 1/5] for-each-ref: refactor get_short_ref function\n\nwith Message-ID <20090407070501.GA2924@coredump.intra.peff.net>\n\nBert\n"},{"id":"110673","messageId":"36ca99e90904070054y3bbd21e0g44548162e71f3a13@mail.gmail.com","threadId":"18762","inReplyTo":"20090407074435.GB7327@coredump.intra.peff.net","subject":"Re: [PATCH] for-each-ref: remove multiple xstrdup() in get_short_ref()","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-04-07T07:54:23Z","receivedAt":"2009-04-07T07:54:23Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Tue, Apr 7, 2009 at 09:44, Jeff King <peff@peff.net> wrote:\n> On Tue, Apr 07, 2009 at 09:33:19AM +0200, Bert Wesarg wrote:\n>\n>> Now that get_short_ref() always return an malloced string, consolidate to\n>> one xstrcpy() call.\n>\n> Makes sense to squash in on top of what I have. But I think it actually\n> is pretty easy to always return a pointer into the existing string\n> (patch based on current master):\nYes, thats probably a good idea. The caller can always do a\nxstrdup(get_short_ref(ref)).\n\n> @@ -637,12 +637,14 @@ static char *get_short_ref(struct refinfo *ref)\n>                 * short name is non-ambiguous if all previous rules\n>                 * haven't resolved to a valid ref\n>                 */\n> -               if (j == i)\n> -                       return short_name;\n> +               if (j == i) {\n> +                       ref += strlen(ref) - strlen(short_name);\nwe have strlen(short_name) in short_name_len already.\n\nBert\n"},{"id":"110674","messageId":"49DB077A.1060506@drmicha.warpmail.net","threadId":"18762","inReplyTo":"20090407071420.GD2924@coredump.intra.peff.net","subject":"Re: [PATCH 4/5] make get_short_ref a public function","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2009-04-07T07:57:46Z","receivedAt":"2009-04-07T07:57:46Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 07.04.2009 09:14:\n> Often we want to shorten a full ref name to something \"prettier\"\n> to show a user. For example, \"refs/heads/master\" is often shown\n> simply as \"master\", or \"refs/remotes/origin/master\" is shown as\n> \"origin/master\".\n> \n> Many places in the code use a very simple formula: skip common\n> prefixes like refs/heads, refs/remotes, etc. This is codified in\n> the prettify_ref function.\n> \n> for-each-ref has a more correct (but more expensive) approach:\n> consider the ref lookup rules, and try shortening as much as\n> possible while remaining unambiguous.\n> \n> This patch makes the latter strategy globally available as\n> shorten_unambiguous_ref.\n> \n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> Actually, I am not quite sure that this function is \"more correct\". It\n> looks at the rev-parsing rules as a hierarchy, so if you have\n> \"refs/remotes/foo\" and \"refs/heads/foo\", then it will abbreviate the\n> first to \"remotes/foo\" (as expected) and the latter to just \"foo\".\n> \n> This is technically correct, as \"refs/heads/foo\" will be selected by\n> \"foo\", but it will warn about ambiguity. Should we actually try to avoid\n> reporting refs which would be ambiguous?\n> \n> Should this simply replace prettify_ref (and other places which should\n> be using prettify_ref but aren't)? It is definitely more expensive, as\n> it has to resolve refs to look for ambiguities, but I don't know if we\n> care in most code paths.\n\nI would think that as long as the default is to warn about ambiguous\nrefs we should not generate ambiguous refs...\n\nOther than that it's very nice, it can be used in many places.\n\n> \n>  builtin-for-each-ref.c |  105 +-----------------------------------------------\n>  refs.c                 |   99 +++++++++++++++++++++++++++++++++++++++++++++\n>  refs.h                 |    1 +\n>  3 files changed, 101 insertions(+), 104 deletions(-)\n> \n> diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c\n> index 277d1fb..8c82484 100644\n> --- a/builtin-for-each-ref.c\n> +++ b/builtin-for-each-ref.c\n> @@ -546,109 +546,6 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, v\n>  }\n>  \n>  /*\n> - * generate a format suitable for scanf from a ref_rev_parse_rules\n> - * rule, that is replace the \"%.*s\" spec with a \"%s\" spec\n> - */\n> -static void gen_scanf_fmt(char *scanf_fmt, const char *rule)\n> -{\n> -\tchar *spec;\n> -\n> -\tspec = strstr(rule, \"%.*s\");\n> -\tif (!spec || strstr(spec + 4, \"%.*s\"))\n> -\t\tdie(\"invalid rule in ref_rev_parse_rules: %s\", rule);\n> -\n> -\t/* copy all until spec */\n> -\tstrncpy(scanf_fmt, rule, spec - rule);\n> -\tscanf_fmt[spec - rule] = '\\0';\n> -\t/* copy new spec */\n> -\tstrcat(scanf_fmt, \"%s\");\n> -\t/* copy remaining rule */\n> -\tstrcat(scanf_fmt, spec + 4);\n> -\n> -\treturn;\n> -}\n> -\n> -/*\n> - * Shorten the refname to an non-ambiguous form\n> - */\n> -static char *get_short_ref(const char *ref)\n> -{\n> -\tint i;\n> -\tstatic char **scanf_fmts;\n> -\tstatic int nr_rules;\n> -\tchar *short_name;\n> -\n> -\t/* pre generate scanf formats from ref_rev_parse_rules[] */\n> -\tif (!nr_rules) {\n> -\t\tsize_t total_len = 0;\n> -\n> -\t\t/* the rule list is NULL terminated, count them first */\n> -\t\tfor (; ref_rev_parse_rules[nr_rules]; nr_rules++)\n> -\t\t\t/* no +1 because strlen(\"%s\") < strlen(\"%.*s\") */\n> -\t\t\ttotal_len += strlen(ref_rev_parse_rules[nr_rules]);\n> -\n> -\t\tscanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len);\n> -\n> -\t\ttotal_len = 0;\n> -\t\tfor (i = 0; i < nr_rules; i++) {\n> -\t\t\tscanf_fmts[i] = (char *)&scanf_fmts[nr_rules]\n> -\t\t\t\t\t+ total_len;\n> -\t\t\tgen_scanf_fmt(scanf_fmts[i], ref_rev_parse_rules[i]);\n> -\t\t\ttotal_len += strlen(ref_rev_parse_rules[i]);\n> -\t\t}\n> -\t}\n> -\n> -\t/* bail out if there are no rules */\n> -\tif (!nr_rules)\n> -\t\treturn xstrdup(ref);\n> -\n> -\t/* buffer for scanf result, at most ref must fit */\n> -\tshort_name = xstrdup(ref);\n> -\n> -\t/* skip first rule, it will always match */\n> -\tfor (i = nr_rules - 1; i > 0 ; --i) {\n> -\t\tint j;\n> -\t\tint short_name_len;\n> -\n> -\t\tif (1 != sscanf(ref, scanf_fmts[i], short_name))\n> -\t\t\tcontinue;\n> -\n> -\t\tshort_name_len = strlen(short_name);\n> -\n> -\t\t/*\n> -\t\t * check if the short name resolves to a valid ref,\n> -\t\t * but use only rules prior to the matched one\n> -\t\t */\n> -\t\tfor (j = 0; j < i; j++) {\n> -\t\t\tconst char *rule = ref_rev_parse_rules[j];\n> -\t\t\tunsigned char short_objectname[20];\n> -\t\t\tchar refname[PATH_MAX];\n> -\n> -\t\t\t/*\n> -\t\t\t * the short name is ambiguous, if it resolves\n> -\t\t\t * (with this previous rule) to a valid ref\n> -\t\t\t * read_ref() returns 0 on success\n> -\t\t\t */\n> -\t\t\tmksnpath(refname, sizeof(refname),\n> -\t\t\t\t rule, short_name_len, short_name);\n> -\t\t\tif (!read_ref(refname, short_objectname))\n> -\t\t\t\tbreak;\n> -\t\t}\n> -\n> -\t\t/*\n> -\t\t * short name is non-ambiguous if all previous rules\n> -\t\t * haven't resolved to a valid ref\n> -\t\t */\n> -\t\tif (j == i)\n> -\t\t\treturn short_name;\n> -\t}\n> -\n> -\tfree(short_name);\n> -\treturn xstrdup(ref);\n> -}\n> -\n> -\n> -/*\n>   * Parse the object referred by ref, and grab needed value.\n>   */\n>  static void populate_value(struct refinfo *ref)\n> @@ -704,7 +601,7 @@ static void populate_value(struct refinfo *ref)\n>  \t\tif (formatp) {\n>  \t\t\tformatp++;\n>  \t\t\tif (!strcmp(formatp, \"short\"))\n> -\t\t\t\trefname = get_short_ref(refname);\n> +\t\t\t\trefname = shorten_unambiguous_ref(refname);\n>  \t\t\telse\n>  \t\t\t\tdie(\"unknown %.*s format %s\",\n>  \t\t\t\t\tformatp - name, name, formatp);\n> diff --git a/refs.c b/refs.c\n> index 59c373f..1e5e7b4 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -1652,3 +1652,102 @@ struct ref *find_ref_by_name(const struct ref *list, const char *name)\n>  \t\t\treturn (struct ref *)list;\n>  \treturn NULL;\n>  }\n> +\n> +/*\n> + * generate a format suitable for scanf from a ref_rev_parse_rules\n> + * rule, that is replace the \"%.*s\" spec with a \"%s\" spec\n> + */\n> +static void gen_scanf_fmt(char *scanf_fmt, const char *rule)\n> +{\n> +\tchar *spec;\n> +\n> +\tspec = strstr(rule, \"%.*s\");\n> +\tif (!spec || strstr(spec + 4, \"%.*s\"))\n> +\t\tdie(\"invalid rule in ref_rev_parse_rules: %s\", rule);\n> +\n> +\t/* copy all until spec */\n> +\tstrncpy(scanf_fmt, rule, spec - rule);\n> +\tscanf_fmt[spec - rule] = '\\0';\n> +\t/* copy new spec */\n> +\tstrcat(scanf_fmt, \"%s\");\n> +\t/* copy remaining rule */\n> +\tstrcat(scanf_fmt, spec + 4);\n> +\n> +\treturn;\n> +}\n> +\n> +char *shorten_unambiguous_ref(const char *ref)\n> +{\n> +\tint i;\n> +\tstatic char **scanf_fmts;\n> +\tstatic int nr_rules;\n> +\tchar *short_name;\n> +\n> +\t/* pre generate scanf formats from ref_rev_parse_rules[] */\n> +\tif (!nr_rules) {\n> +\t\tsize_t total_len = 0;\n> +\n> +\t\t/* the rule list is NULL terminated, count them first */\n> +\t\tfor (; ref_rev_parse_rules[nr_rules]; nr_rules++)\n> +\t\t\t/* no +1 because strlen(\"%s\") < strlen(\"%.*s\") */\n> +\t\t\ttotal_len += strlen(ref_rev_parse_rules[nr_rules]);\n> +\n> +\t\tscanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len);\n> +\n> +\t\ttotal_len = 0;\n> +\t\tfor (i = 0; i < nr_rules; i++) {\n> +\t\t\tscanf_fmts[i] = (char *)&scanf_fmts[nr_rules]\n> +\t\t\t\t\t+ total_len;\n> +\t\t\tgen_scanf_fmt(scanf_fmts[i], ref_rev_parse_rules[i]);\n> +\t\t\ttotal_len += strlen(ref_rev_parse_rules[i]);\n> +\t\t}\n> +\t}\n> +\n> +\t/* bail out if there are no rules */\n> +\tif (!nr_rules)\n> +\t\treturn xstrdup(ref);\n> +\n> +\t/* buffer for scanf result, at most ref must fit */\n> +\tshort_name = xstrdup(ref);\n> +\n> +\t/* skip first rule, it will always match */\n> +\tfor (i = nr_rules - 1; i > 0 ; --i) {\n> +\t\tint j;\n> +\t\tint short_name_len;\n> +\n> +\t\tif (1 != sscanf(ref, scanf_fmts[i], short_name))\n> +\t\t\tcontinue;\n> +\n> +\t\tshort_name_len = strlen(short_name);\n> +\n> +\t\t/*\n> +\t\t * check if the short name resolves to a valid ref,\n> +\t\t * but use only rules prior to the matched one\n> +\t\t */\n> +\t\tfor (j = 0; j < i; j++) {\n> +\t\t\tconst char *rule = ref_rev_parse_rules[j];\n> +\t\t\tunsigned char short_objectname[20];\n> +\t\t\tchar refname[PATH_MAX];\n> +\n> +\t\t\t/*\n> +\t\t\t * the short name is ambiguous, if it resolves\n> +\t\t\t * (with this previous rule) to a valid ref\n> +\t\t\t * read_ref() returns 0 on success\n> +\t\t\t */\n> +\t\t\tmksnpath(refname, sizeof(refname),\n> +\t\t\t\t rule, short_name_len, short_name);\n> +\t\t\tif (!read_ref(refname, short_objectname))\n> +\t\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\t/*\n> +\t\t * short name is non-ambiguous if all previous rules\n> +\t\t * haven't resolved to a valid ref\n> +\t\t */\n> +\t\tif (j == i)\n> +\t\t\treturn short_name;\n> +\t}\n> +\n> +\tfree(short_name);\n> +\treturn xstrdup(ref);\n> +}\n> diff --git a/refs.h b/refs.h\n> index 68c2d16..2d0f961 100644\n> --- a/refs.h\n> +++ b/refs.h\n> @@ -80,6 +80,7 @@ extern int for_each_reflog(each_ref_fn, void *);\n>  extern int check_ref_format(const char *target);\n>  \n>  extern const char *prettify_ref(const struct ref *ref);\n> +extern char *shorten_unambiguous_ref(const char *ref);\n>  \n>  /** rename ref, return 0 on success **/\n>  extern int rename_ref(const char *oldref, const char *newref, const char *logmsg);\n"},{"id":"110676","messageId":"49DB089A.7080207@drmicha.warpmail.net","threadId":"18762","inReplyTo":"20090407071656.GE2924@coredump.intra.peff.net","subject":"Re: [PATCH 5/5] branch: show upstream branch when double verbose","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2009-04-07T08:02:34Z","receivedAt":"2009-04-07T08:02:34Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 07.04.2009 09:16:\n> This information is easily accessible when we are\n> calculating the relationship. The only reason not to print\n> it all the time is that it consumes a fair bit of screen\n> space, and may not be of interest to the user.\n> \n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> This one is very RFC. Should this information be part of the regular\n> \"-v\"? Should it be part of \"git branch\" with regular verbosity?\n> \n> Should the format be different? I wonder if\n> \n>   master 1234abcd [origin/master: ahead 5, behind 6] whatever\n> \n> will be interpreted as \"origin/master is ahead 5, behind 6\" when it is\n> really the reverse. Maybe \"[ahead 5, behind 6 from origin/master]\" would\n> be better?\n\nMaybe [origin/master +5 -6]? That should be short enough for sticking it\ninto -v. We could even use [origin/master +0 -0] for an up-to-date\nbranch then.\n\nIn any case, I think often one is interested in one branch only. I would\nexpect \"git branch -v foo\" to give me the -v info just for branch foo.\nCurrently it does not. But that would be an independent patch on top.\n\n>  Documentation/git-branch.txt |    4 +++-\n>  builtin-branch.c             |   23 +++++++++++++++++------\n>  2 files changed, 20 insertions(+), 7 deletions(-)\n> \n> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\n> index 31ba7f2..ba3dea6 100644\n> --- a/Documentation/git-branch.txt\n> +++ b/Documentation/git-branch.txt\n> @@ -100,7 +100,9 @@ OPTIONS\n>  \n>  -v::\n>  --verbose::\n> -\tShow sha1 and commit subject line for each head.\n> +\tShow sha1 and commit subject line for each head, along with\n> +\trelationship to upstream branch (if any). If given twice, print\n> +\tthe name of the upstream branch, as well.\n>  \n>  --abbrev=<length>::\n>  \tAlter the sha1's minimum display length in the output listing.\n> diff --git a/builtin-branch.c b/builtin-branch.c\n> index ca81d72..3275821 100644\n> --- a/builtin-branch.c\n> +++ b/builtin-branch.c\n> @@ -301,19 +301,30 @@ static int ref_cmp(const void *r1, const void *r2)\n>  \treturn strcmp(c1->name, c2->name);\n>  }\n>  \n> -static void fill_tracking_info(struct strbuf *stat, const char *branch_name)\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>  \tstruct branch *branch = branch_get(branch_name);\n>  \n> -\tif (!stat_tracking_info(branch, &ours, &theirs) || (!ours && !theirs))\n> +\tif (!stat_tracking_info(branch, &ours, &theirs)) {\n> +\t\tif (branch && branch->merge && branch->merge[0]->dst &&\n> +\t\t    show_upstream_ref)\n> +\t\t\tstrbuf_addf(stat, \"[%s] \",\n> +\t\t\t    shorten_unambiguous_ref(branch->merge[0]->dst));\n>  \t\treturn;\n> +\t}\n> +\n> +\tstrbuf_addch(stat, '[');\n> +\tif (show_upstream_ref)\n> +\t\tstrbuf_addf(stat, \"%s: \",\n> +\t\t\tshorten_unambiguous_ref(branch->merge[0]->dst));\n>  \tif (!ours)\n> -\t\tstrbuf_addf(stat, \"[behind %d] \", theirs);\n> +\t\tstrbuf_addf(stat, \"behind %d] \", theirs);\n>  \telse if (!theirs)\n> -\t\tstrbuf_addf(stat, \"[ahead %d] \", ours);\n> +\t\tstrbuf_addf(stat, \"ahead %d] \", ours);\n>  \telse\n> -\t\tstrbuf_addf(stat, \"[ahead %d, behind %d] \", ours, theirs);\n> +\t\tstrbuf_addf(stat, \"ahead %d, behind %d] \", ours, theirs);\n>  }\n>  \n>  static int matches_merge_filter(struct commit *commit)\n> @@ -379,7 +390,7 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,\n>  \t\t}\n>  \n>  \t\tif (item->kind == REF_LOCAL_BRANCH)\n> -\t\t\tfill_tracking_info(&stat, item->name);\n> +\t\t\tfill_tracking_info(&stat, item->name, verbose > 1);\n>  \n>  \t\tstrbuf_addf(&out, \" %s %s%s\",\n>  \t\t\tfind_unique_abbrev(item->commit->object.sha1, abbrev),\n"},{"id":"110679","messageId":"4d8e3fd30904070112w72ada661p99525aaa9437f8ff@mail.gmail.com","threadId":"18762","inReplyTo":"20090407071656.GE2924@coredump.intra.peff.net","subject":"Re: [PATCH 5/5] branch: show upstream branch when double verbose","fromName":"Paolo Ciarrocchi","fromEmail":"paolo.ciarrocchi@gmail.com","sentAt":"2009-04-07T08:12:29Z","receivedAt":"2009-04-07T08:12:29Z","isPatch":true,"sender":{"key":"paolo.ciarrocchi@gmail.com","avatar":null},"body":"On Tue, Apr 7, 2009 at 9:16 AM, Jeff King <peff@peff.net> wrote:\n> This information is easily accessible when we are\n> calculating the relationship. The only reason not to print\n> it all the time is that it consumes a fair bit of screen\n> space, and may not be of interest to the user.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> This one is very RFC. Should this information be part of the regular\n> \"-v\"? Should it be part of \"git branch\" with regular verbosity?\n>\n> Should the format be different? I wonder if\n>\n>  master 1234abcd [origin/master: ahead 5, behind 6] whatever\n>\n> will be interpreted as \"origin/master is ahead 5, behind 6\" when it is\n> really the reverse. Maybe \"[ahead 5, behind 6 from origin/master]\" would\n> be better?\n\nYes I think so.\nThanks a lot for your work!\n\n-Paolo\n"},{"id":"110744","messageId":"20090407214144.GA17962@coredump.intra.peff.net","threadId":"18762","inReplyTo":"20090407074435.GB7327@coredump.intra.peff.net","subject":"Re: [PATCH] for-each-ref: remove multiple xstrdup() in get_short_ref()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-07T21:41:44Z","receivedAt":"2009-04-07T21:41:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 07, 2009 at 03:44:35AM -0400, Jeff King wrote:\n\n> +\t\tif (1 != sscanf(ref, scanf_fmts[i], short_name))\n> [...]\n> +\t\tif (j == i) {\n> +\t\t\tref += strlen(ref) - strlen(short_name);\n> +\t\t\tbreak;\n> +\t\t}\n\nActually, I am not sure this is correct, either. It is making the\nassumption that the short_name is always a suffix of the ref. But one of\nthe rev_parse_rules is:\n\n     \"refs/remotes/%.*s/HEAD\"\n\nfor which this is not true. _But_ as it happens, this rule doesn't\nactually work in reverse because of the way scanf works with %s. The\ncode:\n\n    sscanf(\"refs/remotes/origin/HEAD\", \"refs/remotes/%s/HEAD\", buf);\n\nwill put \"origin/HEAD\" in buf, not \"origin\"; %s eats until it sees\nwhitespace (which should not be occuring in a ref, fortunately).\n\nSo it actually _is_ correct to assume with the current code that the\nshort_name is always a suffix, but I am not sure if that is what we\nactually want. We will always see \"$remote/HEAD\" instead of \"$remote\".\n\nPart of me actually thinks the \"incorrect\" behavior we are doing now is\nactually more explicit and readable. But if that is the case, we should\nperhaps simply be excluding that final rule explicitly, and then our\n\"suffix\" assumption will hold.\n\n-Peff\n"},{"id":"110791","messageId":"7vprfnr7es.fsf@gitster.siamese.dyndns.org","threadId":"18762","inReplyTo":"20090407070651.GB2924@coredump.intra.peff.net","subject":"Re: [PATCH 2/5] for-each-ref: refactor refname handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-08T06:22:51Z","receivedAt":"2009-04-08T06:22:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> This code handles some special magic like *-deref and the\n> :short formatting specifier. The next patch will add another\n> field which outputs a ref and wants to use the same code.\n>\n> This patch splits the \"which ref are we outputting\" from the\n> actual formatting. There should be no behavioral change.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> The diff is scary, but it is mostly reindentation.\n\n... and an introduction of a bug ;-)\n\n>  builtin-for-each-ref.c |   47 ++++++++++++++++++++++++++---------------------\n>  1 files changed, 26 insertions(+), 21 deletions(-)\n>\n> diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c\n> index 4aaf75c..b50c93b 100644\n> --- a/builtin-for-each-ref.c\n> +++ b/builtin-for-each-ref.c\n> @@ -672,32 +672,37 @@ static void populate_value(struct refinfo *ref)\n> ...\n> +\t\t/* look for \"short\" refname format */\n> +\t\tif (formatp) {\n> +\t\t\tformatp++;\n> +\t\t\tif (!strcmp(formatp, \"short\"))\n> +\t\t\t\trefname = get_short_ref(refname);\n> +\t\t\telse\n> +\t\t\t\tdie(\"unknown %.*s format %s\",\n> +\t\t\t\t\tformatp - name, name, formatp);\n\n\t\t\t\tdie(\"unknown %.*s format %s\",\n                                    (int)(formatp - name), name, formatp);\n"},{"id":"110792","messageId":"20090408062717.GA26929@coredump.intra.peff.net","threadId":"18762","inReplyTo":"7vprfnr7es.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/5] for-each-ref: refactor refname handling","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-08T06:27:17Z","receivedAt":"2009-04-08T06:27:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 07, 2009 at 11:22:51PM -0700, Junio C Hamano wrote:\n\n> > The diff is scary, but it is mostly reindentation.\n> ... and an introduction of a bug ;-)\n\nOops. \n\n> > +\t\t\t\tdie(\"unknown %.*s format %s\",\n> > +\t\t\t\t\tformatp - name, name, formatp);\n> \t\t\t\tdie(\"unknown %.*s format %s\",\n>                                     (int)(formatp - name), name, formatp);\n\nHey, it's all 32 bits, right? ;)\n\nThanks for spotting it.\n\n-Peff\n"},{"id":"110896","messageId":"20090409081857.GC17221@coredump.intra.peff.net","threadId":"18762","inReplyTo":"36ca99e90904070039m15869c34jc9e12d5ccc48d82@mail.gmail.com","subject":"Re: [PATCH 4/5] make get_short_ref a public function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-09T08:18:58Z","receivedAt":"2009-04-09T08:18:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 07, 2009 at 09:39:58AM +0200, Bert Wesarg wrote:\n\n> > Actually, I am not quite sure that this function is \"more correct\". It\n> > looks at the rev-parsing rules as a hierarchy, so if you have\n> > \"refs/remotes/foo\" and \"refs/heads/foo\", then it will abbreviate the\n> > first to \"remotes/foo\" (as expected) and the latter to just \"foo\".\n> >\n> > This is technically correct, as \"refs/heads/foo\" will be selected by\n> > \"foo\", but it will warn about ambiguity. Should we actually try to avoid\n> > reporting refs which would be ambiguous?\n> Back than, there was the idea that the core.warnAmbiguousRefs config\n> could be used for this.\n\nI'm not quite sure what you mean. Using this function, we may shorten an\nunambiguous name to one that will complain if core.warnAmbiguousRefs is\nset. So what I'm wondering is if it should use a different algorithm\nthat produces a shortened ref which will not cause a warning.\n\nE.g., right now if we have:\n\n  refs/heads/master\n  refs/remotes/master\n\nshowing %(refname:short) gets you:\n\n  master\n  remotes/master\n\nbut \"git show master\" will warn about the ambiguous ref (but still show\nyou the one you want). An alternative would be to show:\n\n  heads/master\n  remotes/master\n\nin this case.\n\n-Peff\n"},{"id":"110898","messageId":"20090409082350.GD17221@coredump.intra.peff.net","threadId":"18762","inReplyTo":"49DB089A.7080207@drmicha.warpmail.net","subject":"Re: [PATCH 5/5] branch: show upstream branch when double verbose","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-09T08:23:50Z","receivedAt":"2009-04-09T08:23:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 07, 2009 at 10:02:34AM +0200, Michael J Gruber wrote:\n\n> > will be interpreted as \"origin/master is ahead 5, behind 6\" when it is\n> > really the reverse. Maybe \"[ahead 5, behind 6 from origin/master]\" would\n> > be better?\n> \n> Maybe [origin/master +5 -6]? That should be short enough for sticking it\n> into -v. We could even use [origin/master +0 -0] for an up-to-date\n> branch then.\n\nI am not opposed to that format, but I don't feel strongly. And not many\npeople are voicing an opinion in this thread (strange, given that it is\nan opportunity for bikeshedding :) ). My patches are in next, so I think\nI am done on the topic for now. But feel free to submit a followup\npatch.\n\n> In any case, I think often one is interested in one branch only. I would\n> expect \"git branch -v foo\" to give me the -v info just for branch foo.\n> Currently it does not. But that would be an independent patch on top.\n\nHmm. I think that is a little counterintuitive because we think of \"-v\"\nas simply \"increase verbosity\" (because that is what it means in just\nabout every program). But here you are fundamentally changing the action\nthat is occurring (from \"create branch\" to \"show branch\").  I think it\nwould make more sense to have a \"show branch\" mode like:\n\n  git branch -s foo\n\nwhich would probably have the more detailed output by default.\n\n-Peff\n"},{"id":"110906","messageId":"36ca99e90904090205g8a6a5a6nea96f8c5f44e076a@mail.gmail.com","threadId":"18762","inReplyTo":"20090409081857.GC17221@coredump.intra.peff.net","subject":"Re: [PATCH 4/5] make get_short_ref a public function","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-04-09T09:05:06Z","receivedAt":"2009-04-09T09:05:06Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Thu, Apr 9, 2009 at 10:18, Jeff King <peff@peff.net> wrote:\n> On Tue, Apr 07, 2009 at 09:39:58AM +0200, Bert Wesarg wrote:\n>\n>> > Actually, I am not quite sure that this function is \"more correct\". It\n>> > looks at the rev-parsing rules as a hierarchy, so if you have\n>> > \"refs/remotes/foo\" and \"refs/heads/foo\", then it will abbreviate the\n>> > first to \"remotes/foo\" (as expected) and the latter to just \"foo\".\n>> >\n>> > This is technically correct, as \"refs/heads/foo\" will be selected by\n>> > \"foo\", but it will warn about ambiguity. Should we actually try to avoid\n>> > reporting refs which would be ambiguous?\n>> Back than, there was the idea that the core.warnAmbiguousRefs config\n>> could be used for this.\n>\n> I'm not quite sure what you mean. Using this function, we may shorten an\n> unambiguous name to one that will complain if core.warnAmbiguousRefs is\n> set. So what I'm wondering is if it should use a different algorithm\n> that produces a shortened ref which will not cause a warning.\n>\n> E.g., right now if we have:\n>\n>  refs/heads/master\n>  refs/remotes/master\n>\n> showing %(refname:short) gets you:\n>\n>  master\n>  remotes/master\n>\n> but \"git show master\" will warn about the ambiguous ref (but still show\n> you the one you want). An alternative would be to show:\n>\n>  heads/master\n>  remotes/master\n>\n> in this case.\nRight, and the idea was to choose the alternatives based on the\ncore.warnAmbiguousRefs setting, i.e. the former for false, the latter\nfor true.\n\nFor what I posted a patch some time ago:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/96464\n\n(which I read though now)\n\nBert\n>\n> -Peff\n>\n"},{"id":"110908","messageId":"adf1fd3d0904090315x10b8c481g311832c40c450c47@mail.gmail.com","threadId":"18762","inReplyTo":"20090409082350.GD17221@coredump.intra.peff.net","subject":"Re: [PATCH 5/5] branch: show upstream branch when double verbose","fromName":"Santi Béjar","fromEmail":"santi@agolina.net","sentAt":"2009-04-09T10:15:08Z","receivedAt":"2009-04-09T10:15:08Z","isPatch":true,"sender":{"key":"santi@agolina.net","avatar":null},"body":"2009/4/9 Jeff King <peff@peff.net>:\n> On Tue, Apr 07, 2009 at 10:02:34AM +0200, Michael J Gruber wrote:\n>\n>> > will be interpreted as \"origin/master is ahead 5, behind 6\" when it is\n>> > really the reverse. Maybe \"[ahead 5, behind 6 from origin/master]\" would\n>> > be better?\n>>\n>> Maybe [origin/master +5 -6]? That should be short enough for sticking it\n>> into -v. We could even use [origin/master +0 -0] for an up-to-date\n>> branch then.\n>\n> I am not opposed to that format, but I don't feel strongly. And not many\n> people are voicing an opinion in this thread (strange, given that it is\n> an opportunity for bikeshedding :) ).\n\nI've been thinking about this and both formats seems OK for me,\nalthough using the +5 -6 format for just -v seems a good point.\n\nJust to bikeshed a bit more :) we could use a format more similar to\nthe \"git fetch\" output, like:\n\n  next         c4628f8 [4...6 origin/next] Merge branch 'jk/no-perl' into next\n  next         c4628f8 [4.. origin/next] Merge branch 'jk/no-perl' into next\n  next         c4628f8 [..6 origin/next] Merge branch 'jk/no-perl' into next\n\n(three dots when they have diverged and two otherwise)\n\nIt can suggest that the left number you know it is about things in\nnext and the right number about things in origin/next. The problem is\nthat is also looks like revisions.\n\nJust my 2cents, but feel free to ignore ;-)\nSanti\n"},{"id":"111167","messageId":"20090413081543.GA9846@coredump.intra.peff.net","threadId":"18762","inReplyTo":"36ca99e90904090205g8a6a5a6nea96f8c5f44e076a@mail.gmail.com","subject":"Re: [PATCH 4/5] make get_short_ref a public function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-13T08:15:43Z","receivedAt":"2009-04-13T08:15:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 09, 2009 at 11:05:06AM +0200, Bert Wesarg wrote:\n\n> > you the one you want). An alternative would be to show:\n> >\n> >  heads/master\n> >  remotes/master\n> >\n> > in this case.\n> Right, and the idea was to choose the alternatives based on the\n> core.warnAmbiguousRefs setting, i.e. the former for false, the latter\n> for true.\n> \n> For what I posted a patch some time ago:\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/96464\n\nAh, OK, now I understand what you meant. I think that is the right\nsolution. Thanks.\n\n-Peff\n"},{"id":"111168","messageId":"20090413083413.GB9846@coredump.intra.peff.net","threadId":"18762","inReplyTo":"adf1fd3d0904090315x10b8c481g311832c40c450c47@mail.gmail.com","subject":"Re: [PATCH 5/5] branch: show upstream branch when double verbose","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-13T08:34:14Z","receivedAt":"2009-04-13T08:34:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 09, 2009 at 12:15:08PM +0200, Santi Béjar wrote:\n\n> I've been thinking about this and both formats seems OK for me,\n> although using the +5 -6 format for just -v seems a good point.\n\nThe trivial patch for this is below:\n\n---\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex 3275821..c056a4d 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -317,14 +317,14 @@ static void fill_tracking_info(struct strbuf *stat, const char *branch_name,\n \n \tstrbuf_addch(stat, '[');\n \tif (show_upstream_ref)\n-\t\tstrbuf_addf(stat, \"%s: \",\n+\t\tstrbuf_addf(stat, \"%s \",\n \t\t\tshorten_unambiguous_ref(branch->merge[0]->dst));\n \tif (!ours)\n-\t\tstrbuf_addf(stat, \"behind %d] \", theirs);\n+\t\tstrbuf_addf(stat, \"-%d] \", theirs);\n \telse if (!theirs)\n-\t\tstrbuf_addf(stat, \"ahead %d] \", ours);\n+\t\tstrbuf_addf(stat, \"+%d] \", ours);\n \telse\n-\t\tstrbuf_addf(stat, \"ahead %d, behind %d] \", ours, theirs);\n+\t\tstrbuf_addf(stat, \"+%d -%d] \", ours, theirs);\n }\n \n static int matches_merge_filter(struct commit *commit)\n\n\nI actually think it looks a bit ugly without the upstream name, as the\nshort bit in brackets blends in with the commit subject:\n\n  next        1412037 [+2] shorter tracking format for branch -v\n\nwhereas I think this is better:\n\n  next        1412037 [origin/next +2] shorter tracking format for branch -v\n\nbut I am still lukewarm on the concept personally (i.e., I am not\nopposed, but I am not taking it further than the \"how about this\" patch\nbelow, so if somebody thinks it is a good idea they should speak up and\nadvocate for it).\n\n> Just to bikeshed a bit more :) we could use a format more similar to\n> the \"git fetch\" output, like:\n> \n>   next         c4628f8 [4...6 origin/next] Merge branch 'jk/no-perl' into next\n>   next         c4628f8 [4.. origin/next] Merge branch 'jk/no-perl' into next\n>   next         c4628f8 [..6 origin/next] Merge branch 'jk/no-perl' into next\n\nPersonally, I find that pretty ugly, and a bit confusing. The \"..\"\nnotation would make more sense if there were actual revisions on the end\nand not numbers. But then of course you have to show the numbers\nseparately. Something like this makes at least has some logic to it:\n\n  origin/next...next: 4,6\n  origin/next..next: 4\n  next..origin/next: 6\n\nbut it looks horribly ugly and is way more confusing than it needs to\nbe.\n\n-Peff\n"},{"id":"111203","messageId":"18CF07A6-68CB-4A17-ACC9-89CD8545F58B@wincent.com","threadId":"18762","inReplyTo":"20090413083413.GB9846@coredump.intra.peff.net","subject":"Re: [PATCH 5/5] branch: show upstream branch when double verbose","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2009-04-13T17:04:49Z","receivedAt":"2009-04-13T17:04:49Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 13/4/2009, a las 10:34, Jeff King escribió:\n\n> On Thu, Apr 09, 2009 at 12:15:08PM +0200, Santi Béjar wrote:\n>\n>> I've been thinking about this and both formats seems OK for me,\n>> although using the +5 -6 format for just -v seems a good point.\n>\n> The trivial patch for this is below:\n>\n> ---\n> diff --git a/builtin-branch.c b/builtin-branch.c\n> index 3275821..c056a4d 100644\n> --- a/builtin-branch.c\n> +++ b/builtin-branch.c\n> @@ -317,14 +317,14 @@ static void fill_tracking_info(struct strbuf  \n> *stat, const char *branch_name,\n>\n> \tstrbuf_addch(stat, '[');\n> \tif (show_upstream_ref)\n> -\t\tstrbuf_addf(stat, \"%s: \",\n> +\t\tstrbuf_addf(stat, \"%s \",\n> \t\t\tshorten_unambiguous_ref(branch->merge[0]->dst));\n> \tif (!ours)\n> -\t\tstrbuf_addf(stat, \"behind %d] \", theirs);\n> +\t\tstrbuf_addf(stat, \"-%d] \", theirs);\n> \telse if (!theirs)\n> -\t\tstrbuf_addf(stat, \"ahead %d] \", ours);\n> +\t\tstrbuf_addf(stat, \"+%d] \", ours);\n> \telse\n> -\t\tstrbuf_addf(stat, \"ahead %d, behind %d] \", ours, theirs);\n> +\t\tstrbuf_addf(stat, \"+%d -%d] \", ours, theirs);\n> }\n>\n> static int matches_merge_filter(struct commit *commit)\n\nI'm skeptical that people will know at a glance that \"-\" and \"+\" in  \nthis context map onto \"behind\" and \"ahead\". I think that trading off  \nthe clarity and obviousness of the words \"behind\" and \"ahead\" for the  \nbrevity of the symbols \"-\" and \"+\" wouldn't be a very good idea.\n\nCheers,\nWincent\n"}]}