Re: [PATCH] gitweb: allow extra breadcrumbs to prefix the trail

From: Jakub Narębski <jnareb@gmail.com>
Date: Jul 4, 2013, 15:34 UTC
Message-ID: <CANQwDwfCYPBjGfmKOLju-Zey4WrVrXfaymJtu9g1OYvOUfBTcw@mail.gmail.com>
In-Reply-To: <alpine.LSU.2.00.1307041559140.26246@hermes-2.csi.cam.ac.uk>

On Thu, Jul 4, 2013 at 5:11 PM, Tony Finch <dot@dotat.at> wrote:

Show 6 quoted lines

> Jakub Narębski <jnareb@gmail.com> wrote:
>>
>> It would be better to improve documentation, than follow current bad
>> practice... ;-P
>
> The v2 patch does just that :-)

Thanks.

Show 10 quoted lines

>> Perhaps even make ( [ $home_link_str, $home_link ] ) to be default
>> value for @extra_breadcrumbs, making new feature generalization
>> of $home_link*, similarly to how it was done for $stylesheet -> @stylesheets
>> transition.
>
> I don't think that's a win. There's a lot of existing gitweb.conf out
> there which sets $home_link_str, so the code would have to either print
> the last element of @extra_breadcrumbs or the $home_link variables
> depending on whether the variables were modified. And the documentation
> would have to explain this complicated arrangement.

First, do I understand corrctly that @extra_breadcrumbs are rendered *after* $home_link*, and in exactly the same manner?

Second, I misremembered how $stylesheet / @stylesheets is handled. I was thinking more about having in gitweb.perl the following default initialization for @extra_breadcrumbs:

  our @extra_breadcrumbs = ( [ $home_link_str, $home_link ] );

Then one can add breadcrumbs with

  push @extra_breadcrumbs, [ $foo_html, $foo_url ], [
esc_html($bar_txt), $bar_url ];

But now I think that we can do better, simply put $home_link_str and $home_link in @extra_breadcrumbs / @top_level_breadcrumbs / @nav_breadcrumbs before using it, either via

  unshift @nav_breadcrumbs, [ $home_link_str, $home_link ];

or

  for $breadcrumb ([ $home_link_str, $home_link ], @nav_breadcrumbs) {

... unless we treat home link in some special way (do we?).

P.S. It is a bit late, but wouldn't { name => $link_name, href => $link_url } (like %features hash) be a better solution than [ $link_name, $link_url ], i.e. hashref (named parameters) instead of arrayref (positional parameters). You wouldn't have to remember which is first: text or URL.

-- 
Jakub Narebski