Re: [PATCHv5 07/12] gitweb: remotes view for a single remote

From: Jakub Narebski <jnareb@gmail.com>
Date: Sep 27, 2010, 07:53 UTC
Message-ID: <201009270953.52562.jnareb@gmail.com>
In-Reply-To: <AANLkTimemfDPdMycasFmqUvk0eF-eD9z7P1RFnitLD9G@mail.gmail.com>

On Mon, 27 Sep 2010, Giuseppe Bilotta wrote:

> 2010/9/26 Jakub Narebski <jnareb@gmail.com>:
>> On Fri, 24 Sep 2010, Giuseppe Bilotta wrote:

Show 5 quoted lines

>>> +
>>> +     git_header_html(undef, undef, 'header_extra' => $remote);
>>
>> I don't quite like the name of this parameter, and I am not sure
>> if I like the API either.

Explanation: what I don't like (a tiny bit) about API is the need for
those 'undef, undef'... but this might be unavoidable, at least without
heavy hackery for little gain, because of the way Perl passes arguments
to subroutines.  (And no, rewrite of gitweb in Perl 6 is not a viable
solution ;-))

Show 9 quoted lines

> As I mentioned in my replies to the other respective patches, I think
> it makes sense to make "all remotes" view easily accessible from the
> "single remote" view, and there are two ways I can think of: one is
> the "extra header text" way, by making the action name before it point
> to "all remotes". The other is to enable 'remotes' in the page nav
> submenu when we are in single remotes view (which is why I had the
> $current in format_ref_views instead of $action, and which is what is
> done by this change).  IMO it makes sense to have both ways available,
> but I'm open to suggestions about different approaches.

Now I understand it, and I completly agree that it is a very good solution. But it needs better description in the commit message.

Sidenote: http://git.oblomov.eu/rbot/remotes/anonj2 looks like doesn't
have this patch.  In the navigation bar it has

  _projects_ / _rbot_ / remotes

and not

  _projects_ / _rbot_ / _remotes_ / anonj2

>>> -     git_print_page_nav('','', $head,undef,$head,format_ref_views('remotes'));

[...]

>>> +     git_print_page_nav('', '',  $head, undef, $head,

Why not

    +     git_print_page_nav('','', $head,undef,$head,

>>> +             format_ref_views($remote ? '' : 'remotes'));
>>
>> Why this change?

I might be not clear, but I meant here the change in formatting of call to git_print_page_nav... and what I have not noticed is the fact that format_ref_views is passed diferent argument.

And I see now here why passing action-like parameter ($current) to
format_ref_views allow for this.  I withdraw then proposal for using
$action global variable in place of $current argument; this API is
better (though perhaps this could be described in commit message)..

-- 
Jakub Narebski
Poland