Re: [PATCHv6 10/10] gitweb: group remote heads by remote

> On Wed, 3 Nov 2010, Giuseppe Bilotta wrote:
>>
>> I think that in most cases there won't be any need for limiting.
>> Public cases of lots of remotes with lots of branches are, I suspect,
>> rare.
>
> It's not about _public_ cases; I think that it is in very rare cases
> that public repository would want to display remotes and remote-tracking
> branches.

> I think remote_heads feature is more important for _local_ use, for
> example browsing one own repository using git-instaweb.  In such cases
> number of remotes and of remote-tracking branches might be large (I have
> 11 remotes, not all active, and 58 remote-tracking branches).
>
> BTW. would next version of this series include patch to git-instaweb
> enabling 'remote_heads' feature for it (gitweb_conf function)?

>> with all the remotes/<remotename> pathspecs as a single array
>> argument. This _does_ mean that when the total number of remote heads
>> is greater than the limit some remotes will not display complete
>> information in summary view. The real issue here is, I think, that
>> there is no trivial way to tell which remotes have incomplete
>> information and which don't, meaning that in the subsequent
>> git_remote_block calls we'll have no way to provide visual feedback
>> (the ellipsis) when some heads are missing.
>
> Errr... shouldn't we leave limiting number of heads to fill_remote_heads,
> which can do limiting per remote (with each remote having up to $limit
> remote-tracking branches / remote heads), instead of having
> git_get_heads_list do it?
>
> Something like this:
>
> +sub fill_remote_heads {
> +       my ($remotes, $limit) = @_;
> +
> +       my @heads = map { "remotes/$_" } keys %$remotes;
> +       my @remoteheads = git_get_heads_list(undef, @heads);
> +       foreach my $remote (keys %$remotes) {
> +               $remotes->{$remote}{'heads'} =
> +                       [ grep { $_->{'name'} =~ s!^$remote/!! } @remoteheads ];
> +               $remotes->{$remote}{'heads'} =
> +                       [ @{$remotes->{$remote}{'heads'}}[0..$limit-1] ]
> +                       if (@{$remotes->{$remote}{'heads'}} > $limit);
> +       }
> +}
>
> Though perhaps it will be more clear with if as statement, not as modifier:
>
> +sub fill_remote_heads {
> +       my ($remotes, $limit) = @_;
> +
> +       my @heads = map { "remotes/$_" } keys %$remotes;
> +       my @remoteheads = git_get_heads_list(undef, @heads);
> +       foreach my $remote (keys %$remotes) {
> +               $remotes->{$remote}{'heads'} =
> +                       [ grep { $_->{'name'} =~ s!^$remote/!! } @remoteheads ];
> +               if (@{$remotes->{$remote}{'heads'}} > $limit) {
> +                       $remotes->{$remote}{'heads'} =
> +                               [ @{$remotes->{$remote}{'heads'}}[0..$limit-1] ]
> +               }
> +       }
> +}