On Thu, Jul 4, 2013 at 5:56 PM, Tony Finch <dot@xxxxxxxx> wrote:
> Jakub Narębski <jnareb@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html