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

["Jakub Narębski" <jnareb@gmail.com>]

On Thu, Jul 4, 2013 at 5:56 PM, Tony Finch <dot@dotat.at> wrote:
> Jakub Narębski <jnareb@gmail.com> wrote:
>>
>> First, do I understand correctly that @extra_breadcrumbs are rendered *after*
>> $home_link*, and in exactly the same manner?
>
> Before the home link, and yes, in the same manner. The extra breadcrumbs
> are for links to parent pages above gitweb in some hierarchy.

Hmmm... I would have thought that they were after home link. I wonder
if leaving it up to user to configure @extra_breadcrumbs to include $home_link
in appropriate place (the unshift / push solution to adding to
@extra_breadcrumbs,
starting with $home_link) would be good idea, or over-engineering.

In what situation do you need those extra breadcrumbs useful? What
necessity / itch to scratch is behind idea of this patch?

>> 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,
>
> We could save a line that way:
>
> -       print $cgi->a({-href => esc_url($home_link)}, $home_link_str) . " / ";
> +       for my $crumb (@extra_breadcrumbs, [ $home_link_str => $home_link ]) {
> +               print $cgi->a({-href => esc_url($crumb->[1])}, $crumb->[0]) . " / ";
> +       }

And avoid a bit of code duplication; now we are sure that both
@extra_breadcrumbs and $home_link are rendered in the same way.

>> 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.
>
> I thought the fat arrow would be mnemonic enough, and less verbose.

Yes, hashref solution is a bit verbose. I don't like abusing fat arrow notation,
but here it gives nice mnemonic (hopefully explained in documentation).

-- 
Jakub Narebski