{"thread":{"id":"47489","subject":"[RFC] Prepend \"tags/\" when describing tags with embedded name","startedAt":"2017-12-23T14:17:56Z","lastAt":"2017-12-27T18:27:56Z","messageCount":2,"participants":["Daniel Knittl-Frank","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"335226","messageId":"CACx-yZ3VAaPQ4dU40_JFm_=WGH5ZX-28bXKqGqVnj39-6_9duw@mail.gmail.com","threadId":"47489","inReplyTo":null,"subject":"[RFC] Prepend \"tags/\" when describing tags with embedded name","fromName":"Daniel Knittl-Frank","fromEmail":"knittl89@googlemail.com","sentAt":"2017-12-23T14:17:30Z","receivedAt":"2017-12-23T14:17:56Z","isPatch":false,"sender":{"key":"knittl89@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/2149217?v=4"},"body":"On Fri, Dec 15, 2017 at 8:25 PM, Junio C Hamano wrote:\n> I think the code makes sense, but it won't be understandable by\n> those who do not know what you discussed in the original thread.\n>\n> A proper commit log message, with a new test or two in t6120, would\n> be an appropriate way to fix that.\n>\n> Care to follow through, along the lines in\n> Documentation/SubmittingPatches?\n\nThe updated branch including tests can be found at:\nhttp://repo.or.cz/git/dkf.git/shortlog/refs/heads/bugfix/describe-all\n\nOne existing test in t6210 needed updating to match the new behavior,\nthree new tests have been added: annotated tags, lightweight tags, and\nbranches.\n\nDaniel\n\n-- \ntyped with http://neo-layout.org\n"},{"id":"335372","messageId":"xmqq8tdonhzu.fsf@gitster.mtv.corp.google.com","threadId":"47489","inReplyTo":"CACx-yZ3VAaPQ4dU40_JFm_=WGH5ZX-28bXKqGqVnj39-6_9duw@mail.gmail.com","subject":"Re: [RFC] Prepend \"tags/\" when describing tags with embedded name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-27T18:27:49Z","receivedAt":"2017-12-27T18:27:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Knittl-Frank writes:\n\n> On Fri, Dec 15, 2017 at 8:25 PM, Junio C Hamano wrote:\n>> I think the code makes sense, but it won't be understandable by\n>> those who do not know what you discussed in the original thread.\n>>\n>> A proper commit log message, with a new test or two in t6120, would\n>> be an appropriate way to fix that.\n>>\n>> Care to follow through, along the lines in\n>> Documentation/SubmittingPatches?\n>\n> The updated branch including tests can be found at:\n> http://repo.or.cz/git/dkf.git/shortlog/refs/heads/bugfix/describe-all\n>\n> One existing test in t6210 needed updating to match the new behavior,\n> three new tests have been added: annotated tags, lightweight tags, and\n> branches.\n>\n> Daniel\n\nThanks. Looks excellent. \n\nThe second paragraph, while it did not say anything incorrect, felt\na bit too indirect to point out that the commit it refers introduced\nthe regression this is fixing, so I updated the description a bit,\nlike below.\n\n-- >8 --\nFrom: Daniel Knittl-Frank \nDate: Mon, 11 Dec 2017 18:24:54 +0100\nSubject: [PATCH] describe: prepend \"tags/\" when describing tags with embedded name\n\nThe man page of the \"git describe\" command explains the expected\noutput when using the --all option, i.e. the full reference path is\nshown, including heads/ or tags/ prefix.\n\nWhen 212945d4a85dfa172ea55ec73b1d830ef2d8582f (\"Teach git-describe\nto verify annotated tag names before output\") made Git favor the\nembedded name of annotated tags, it accidentally changed the output\nformat when the --all flag is given, only printing the tag's name\nwithout the prefix.\n\nCheck if --all was specified and re-add the \"tags/\" prefix for this\nspecial case to fix the regresssion.\n\nSigned-off-by: Daniel Knittl-Frank \nSigned-off-by: Junio C Hamano \n---\n builtin/describe.c | 7 +++++--\n t/t6120-describe.sh | 6 +++++-\n 2 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 29075dbd0f..2004a1a86e 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -271,10 +271,13 @@ static void display_name(struct commit_name *n)\n \t\tn->name_checked = 1;\n \t}\n \n-\tif (n->tag)\n+\tif (n->tag) {\n+\t\tif (all)\n+\t\t\tprintf(\"tags/\");\n \t\tprintf(\"%s\", n->tag->tag);\n-\telse\n+\t} else {\n \t\tprintf(\"%s\", n->path);\n+\t}\n }\n \n static void show_suffix(int depth, const struct object_id *oid)\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 1c0e8659d9..15612b3bbe 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -122,7 +122,7 @@ test_expect_success 'describe --contains defaults to HEAD without commit-ish' '\n '\n \n : >err.expect\n-check_describe A --all A^0\n+check_describe tags/A --all A^0\n test_expect_success 'no warning was displayed for A' '\n \ttest_cmp err.expect err.actual\n '\n@@ -340,4 +340,8 @@ test_expect_success ULIMIT_STACK_SIZE 'describe works in a deep repo' '\n \ttest_cmp expect actual\n '\n \n+check_describe tags/A --all A\n+check_describe tags/c --all c\n+check_describe heads/branch_A --all --match='branch_*' branch_A\n+\n test_done\n-- \n2.15.1-597-g62d91a8972\n\n"}]}