Subject: Re: [PATCH v4 2/2] repo: add --all to git-repo-info

From: Lucas Seiki Oshiro <[email-masked]>
Date: Tue, 18 Nov 2025 17:16:09 -0300

>> + for (unsigned long i = 0; i < ARRAY_SIZE(repo_info_fields); i++) {
>> 
> I am not sure if "unsigned long i" is the type you want here.  I do
> not mind, and actually I prefer, a simple platform natural "int i"
> for something simple like this [*], but I know other people prefer to
> use "size_t" to work with ARRAY_SIZE() these days.

Yeah, I also thought it an unsigned long feels out of place, but I
was only following ARRAY_SIZE. Actually, I was trying to avoid a
warning. In this case we have very few `repo_info_field`s and any
int type would work here...

I'll replace it by size_t, then.

>    Side note: The reason they insist using size_t here is that
>        "-Wsign-compare" makes the compiler complain.  But I would
>        say that it only shows what a misguided feature
>        -Wsign-compare warning is, especially given that the
>        compiler perfectly well knows how big repo_info_fields[]
>        array is and the iteration cannot do any harm if done with a
>        signed integer smaller than size_t

Perhaps if ARRAY_SIZE(repo_info_fields) is bigger than the maximum
limit of the integer type, which would overflow and this for would
loop forever. But, obviously this wouldn't happen here.

> This would work, but the symmetry between a list of keys vs the
> "--all" option is lost.
> 
> I'd rather see something like the following after a #leftoverbits
> clean-up commit:
> 
> if (all_keys && argc)
> die(_("--all and <key> cannot be used together"));
> 
> if (all_keys)
> return print_all_fields(repo, format);
> else
> return print_fields(argc, argv, repo, format);

I'll change it in v5.

> Again, this would work for now, but maybe "git repo info --keys"
> that emits these would be easier to manage.  This can be left to
> #leftoverbits of course.

I can't see a use for it other than these tests. What about writing
a helper inside t/helpers for that?

> But then we have seem to have seen too many #leftoverbits material,
> you might want to handle some or all of them in this series in a
> reroll?  I am starting to become undecided.

I agree with all of them, but I think they were too much for this
series... I also think that after git-repo-structure being added to
repo.c I think that it deserves a patchset only for refactoring.

But I'll send a v5 containing the changes directly related to this
series.

Thanks again. I'll send a v5 soon.