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

On Fri, Sep 17, 2010 at 6:06 PM, Jakub Narebski [off-list ref] wrote:

Giuseppe Bilotta wrote:

quoted

On Fri, Sep 17, 2010 at 3:24 AM, Jakub Narebski [off-list ref] wrote:

quoted

... 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, or

(2) we make it like git_do_group($class, $id, sub { <closure that
prints header div> }, sub {<closure that prints the content>}), which
is even more horrible since the header div is just a
git_print_header_div

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).

-- 
Giuseppe "Oblomov" Bilotta