Re: [PATCH] builtin/tag.c: return appropriate value when --points-at finds an empty list

[George Papanikolaou]

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
>

Re: [PATCH] builtin/tag.c: return appropriate value when --points-at finds an empty list

[Derrick Stolee]

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

[PATCH] builtin/tag.c: return appropriate value when --points-at finds an empty list

[George Papanikolaou]

`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(, filter, FILTER_REFS_TAGS);
+
+   if (array.nr == 0)
+   return -1;
+
ref_array_sort(sorting, );
 
for (i = 0; i < array.nr; i++)
-- 
2.11.0