{"thread":{"id":"47425","subject":"[PATCH] builtin/tag.c: return appropriate value when --points-at finds an empty list","startedAt":"2017-12-11T13:44:24Z","lastAt":"2017-12-11T15:25:36Z","messageCount":3,"participants":["George Papanikolaou","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"334595","messageId":"20171211134409.13339-1-g3orge.app@gmail.com","threadId":"47425","inReplyTo":null,"subject":"[PATCH] builtin/tag.c: return appropriate value when --points-at finds an empty list","fromName":"George Papanikolaou","fromEmail":"g3orge.app@gmail.com","sentAt":"2017-12-11T13:44:09Z","receivedAt":"2017-12-11T13:44:24Z","isPatch":true,"sender":{"key":"g3orge.app@gmail.com","avatar":"https://gravatar.com/avatar/57d9756bb8e51dd138b79f2276e0bb0be2bfdc9aa60f08704de80e96d3a1c0f8?d=mp&s=160"},"body":"`git tag --points-at` can simply return if the given rev does not have\nany tags pointing to it. It's not a failure but it shouldn't return\nwith 0 value.\n---\n builtin/tag.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex b38329b59..68b84db2a 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -58,6 +58,10 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,\n \t\tdie(_(\"unable to parse format string\"));\n \tfilter->with_commit_tag_algo = 1;\n \tfilter_refs(&array, filter, FILTER_REFS_TAGS);\n+\n+\tif (array.nr == 0)\n+\t\treturn -1;\n+\n \tref_array_sort(sorting, &array);\n \n \tfor (i = 0; i < array.nr; i++)\n-- \n2.11.0\n\n"},{"id":"334596","messageId":"ca421d38-2d9a-8681-7947-3799c59984a7@gmail.com","threadId":"47425","inReplyTo":"20171211134409.13339-1-g3orge.app@gmail.com","subject":"Re: [PATCH] builtin/tag.c: return appropriate value when --points-at finds an empty list","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2017-12-11T14:05:26Z","receivedAt":"2017-12-11T14:05:35Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 12/11/2017 8:44 AM, George Papanikolaou wrote:\n> `git tag --points-at` can simply return if the given rev does not have\n> any tags pointing to it. It's not a failure but it shouldn't return\n> with 0 value.\n\nI disagree. I think the 0 return means \"I completed successfully\" and \nthe empty output means \"I didn't find any tags pointing to this object.\"\n\nChanging the return value here could break a lot of scripts out in the \nwild, and I consider this to be an \"API\" compatibility that needs to \nstay as-is.\n\nWhat are you using \"--points-at\" where you need a nonzero exit code \ninstead of a different indicator?\n\nThanks,\n-Stolee\n\n"},{"id":"334607","messageId":"CAByyCQDBtCdxB9fLUeH6A8o-g444TO_yD+O7ap8RO2Bg0b6aAA@mail.gmail.com","threadId":"47425","inReplyTo":"ca421d38-2d9a-8681-7947-3799c59984a7@gmail.com","subject":"Re: [PATCH] builtin/tag.c: return appropriate value when --points-at finds an empty list","fromName":"George Papanikolaou","fromEmail":"g3orge.app@gmail.com","sentAt":"2017-12-11T15:25:30Z","receivedAt":"2017-12-11T15:25:36Z","isPatch":true,"sender":{"key":"g3orge.app@gmail.com","avatar":"https://gravatar.com/avatar/57d9756bb8e51dd138b79f2276e0bb0be2bfdc9aa60f08704de80e96d3a1c0f8?d=mp&s=160"},"body":"I agree with what you're saying, just I thought this might be ultra-minor for\nAPI-breakage. To me, 0 doesn't necessarily mean \"I didn't segfault\".\nI lot of tools use ret-values to give information back. And that way it's much\neasier to just `||` the command to something else instead of `[[ -z ]]` in the\nscript.\n\nBut I see what you're saying...\n--\n/ΓΠ\n\n\nOn Mon, Dec 11, 2017 at 4:05 PM, Derrick Stolee <stolee@gmail.com> wrote:\n> On 12/11/2017 8:44 AM, George Papanikolaou wrote:\n>>\n>> `git tag --points-at` can simply return if the given rev does not have\n>> any tags pointing to it. It's not a failure but it shouldn't return\n>> with 0 value.\n>\n>\n> I disagree. I think the 0 return means \"I completed successfully\" and the\n> empty output means \"I didn't find any tags pointing to this object.\"\n>\n> Changing the return value here could break a lot of scripts out in the wild,\n> and I consider this to be an \"API\" compatibility that needs to stay as-is.\n>\n> What are you using \"--points-at\" where you need a nonzero exit code instead\n> of a different indicator?\n>\n> Thanks,\n> -Stolee\n>\n"}]}