# [PATCH] builtin/tag.c: return appropriate value when --points-at finds an empty list 3 messages from 2017-12-11 to 2017-12-11. Participants: George Papanikolaou, Derrick Stolee. Thread: https://gitlist.dev/t/47425 ## George Papanikolaou, 2017-12-11 13:44 Subject: [PATCH] builtin/tag.c: return appropriate value when --points-at finds an empty list Message-ID: <20171211134409.13339-1-g3orge.app@gmail.com> URL: https://gitlist.dev/e/20171211134409.13339-1-g3orge.app%40gmail.com ``` `git tag --points-at` can simply return if the given rev does not have any tags pointing to it. It's not a failure but it shouldn't return with 0 value. --- builtin/tag.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/builtin/tag.c b/builtin/tag.c index b38329b59..68b84db2a 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -58,6 +58,10 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, die(_("unable to parse format string")); filter->with_commit_tag_algo = 1; filter_refs(&array, filter, FILTER_REFS_TAGS); + + if (array.nr == 0) + return -1; + ref_array_sort(sorting, &array); for (i = 0; i < array.nr; i++) -- 2.11.0 ``` ## Derrick Stolee, 2017-12-11 14:05 Subject: Re: [PATCH] builtin/tag.c: return appropriate value when --points-at finds an empty list Message-ID: URL: https://gitlist.dev/e/ca421d38-2d9a-8681-7947-3799c59984a7%40gmail.com In-Reply-To: <20171211134409.13339-1-g3orge.app@gmail.com> ``` On 12/11/2017 8:44 AM, George Papanikolaou wrote: > `git tag --points-at` can simply return if the given rev does not have > any tags pointing to it. It's not a failure but it shouldn't return > with 0 value. I disagree. I think the 0 return means "I completed successfully" and the empty output means "I didn't find any tags pointing to this object." Changing the return value here could break a lot of scripts out in the wild, and I consider this to be an "API" compatibility that needs to stay as-is. What are you using "--points-at" where you need a nonzero exit code instead of a different indicator? Thanks, -Stolee ``` ## George Papanikolaou, 2017-12-11 15:25 Subject: Re: [PATCH] builtin/tag.c: return appropriate value when --points-at finds an empty list Message-ID: URL: https://gitlist.dev/e/CAByyCQDBtCdxB9fLUeH6A8o-g444TO_yD%2BO7ap8RO2Bg0b6aAA%40mail.gmail.com In-Reply-To: ``` I agree with what you're saying, just I thought this might be ultra-minor for API-breakage. To me, 0 doesn't necessarily mean "I didn't segfault". I lot of tools use ret-values to give information back. And that way it's much easier to just `||` the command to something else instead of `[[ -z ]]` in the script. But I see what you're saying... -- /ΓΠ On Mon, Dec 11, 2017 at 4:05 PM, Derrick Stolee wrote: > On 12/11/2017 8:44 AM, George Papanikolaou wrote: >> >> `git tag --points-at` can simply return if the given rev does not have >> any tags pointing to it. It's not a failure but it shouldn't return >> with 0 value. > > > I disagree. I think the 0 return means "I completed successfully" and the > empty output means "I didn't find any tags pointing to this object." > > Changing the return value here could break a lot of scripts out in the wild, > and I consider this to be an "API" compatibility that needs to stay as-is. > > What are you using "--points-at" where you need a nonzero exit code instead > of a different indicator? > > Thanks, > -Stolee > ```