Re: [PATCH v2 07/11] gitweb: add 'remotes' action

["Giuseppe Bilotta" <giuseppe.bilotta@gmail.com>]

On Sat, Nov 15, 2008 at 1:16 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Thu, 13 Nov 2008, Giuseppe "Oblomov" Bilotta wrote:
>
>> This action is similar to the 'heads' action, but it displays
>> remote heads, grouped by remote repository.
>
> I think I would prefer would go together with the change that split
> the 'heads' ('branches') part of summary view into 'heads' and
> 'remotes', so that both section title header, and '...' continuation
> if present, lead to proper view.
>
> So either
>
>  [heads]  # or [branches]
>  master
>  to-submit
>  origin/master
>  origin/next
>  ...
>
> where both '[heads]' and (possibly) '...' link to 'heads' view showing
> _both_ local branches (refs/heads/*) and remote-tracking branches
> (refs/remotes/*), like in first patch of series (perhaps with some
> subdivision).
>
> Or
>
>  [heads]
>  master
>  to-submit
>  ...
>  [remotes]
>  origin/master
>  origin/next
>  ...
>
> where '[heads]' link to 'heads' view which shows only local branches
> (refs/heads/*), and '[remotes]' link to 'remotes' view which shows only
> remote-tracking branches.

That's funny, I just squashed this patch with the summary list split
view patch 8-) I'm going for the second option, to have [heads] link
to heads which only lists local heads, and [remotes] linking to
remotes that lists the remotes. We may or may not want to rather have
[branches] instead of [heads], and keep the heads action to mean *all*
heads, local and remote, but I'm not sure about it.

>> -     my @headslist = git_get_heads_list();
>> +     my @headslist = git_get_heads_list(undef, 'heads');
>
> Hmmm... I wonder if it would be possible to use some DWIM-mery on
> the side of git_get_heads_list (for example checking if first argument
> is a number, and assuming that nobody would be insane enough to use
> refs/15 for namespace), and just use git_get_heads_list('heads') here.
>
> But I guess that this form is good enough...

I've been wondering about this myself. Another possibility would be to
use named options instead of positional parameters, but then again it
all looks like overkill, at least for the time being.

>>       if (@headslist) {
>>               git_heads_body(\@headslist, $head);
>>       }
>>       git_footer_html();
>>  }
>>
>> +sub git_remotes {
>> +     my $head = git_get_head_hash($project);
>> +     git_header_html();
>> +     git_print_page_nav('','', $head,undef,$head);
>> +     git_print_header_div('summary', $project . ' remotes');
>> +
>> +     my @headslist = git_get_heads_list(undef, 'remotes');
>> +     if (@headslist) {
>> +             git_split_heads_body(\@headslist, $head);
>> +     }
>> +     git_footer_html();
>> +}
>
> Nice. I see the difference from git_heads is using $project . ' remotes'
> in place of $project in git_print_header_div() (why?),

FWIW, I decided to scratch that additional ' remotes' string when
squashing this patch.

-- 
Giuseppe "Oblomov" Bilotta