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

From: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Date: Nov 15, 2008, 12:32 UTC
Message-ID: <cb7bb73a0811150432s2f3ae4bfkc0fc29d92accc635@mail.gmail.com>
In-Reply-To: <200811151316.32024.jnareb@gmail.com>

On Sat, Nov 15, 2008 at 1:16 PM, Jakub Narebski <jnareb@gmail.com> wrote:

Show 38 quoted lines

> 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.

Show 9 quoted lines

>> -     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.

Show 21 quoted lines

>>       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