Re: [PATCH 5/7] gitweb: auxiliary functions to group data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 17 Sep 2010, Giuseppe Bilotta wrote:
> On Fri, Sep 17, 2010 at 6:06 PM, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
>> Giuseppe Bilotta wrote:
>>> On Fri, Sep 17, 2010 at 3:24 AM, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
>>>>
>>>> ... but I think that having separate subroutines for opening and
>>>> closing tags is a bad design / bad API (except in some rare cases).
>>>> It is begging for unbalanced HTML.
>>>>
>>>> It would be better if it was a single subroutine wrapping 'div' around
>>>> contents given either as a string, or via callback (subroutine reference),
>>>> in my opinion.
>>>
>>> I'm not sure that in this case the string or callback approach would
>>> be any cleaner. I'll see if perl supports closures or something like
>>> that.
>>
>> Perl supports closures (thanks to anonymous subroutines 'sub { ... }'
>> and lexical variables 'my $var'), see perlsub and "Function Templates"
>> in perlref.
>>
>> I also recommend free ebook "Higher-Order Perl" http://hop.perl.plover.com/
> 
> Thanks for the suggestion. I'm still not convinced that such an
> implementation would be better though. Aside from the general
> aesthetical suckiness of passing closures around (and the experience
> is not any more pleasurable in Perl), there's also the matter of the
> calling convention to use. I can think of two options:
> 
> (1) we make the function callable as git_do_group($class, $id, sub {
> <closure that prints the content> }, <paramters that go to
> git_print_header_div>), which is somewhat illogical since we're
> specifying the content before the header, 

Why not

  git_do_group($class, $id, <print_header_div parameters>, sub { ... })

or even use subroutine prototypes?  We can use 'pop @_' to get last
argument of a subroutine.

[...]
> Overall, I still get the impression that the current API is
> considerably cleaner, even with the small counterweight of the risk of
> leaving groups open (which is not something so dramatic anyway, IMO).

Might be.

But as currently git_*group() is used in only one place, isn't it
premature generalization ;-) ?
-- 
Jakub Narebski
Poland
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]