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

[Jakub Narebski <jnareb@gmail.com>]

On Thu, 16 Sep 2010, Giuseppe Bilotta wrote:

> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 6138c6e..92551e4 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3765,6 +3765,21 @@ sub git_print_header_div {
>  	      "\n</div>\n";
>  }
>  
> +sub git_begin_group {
> +	my ($class, $id, @rest) = @_;
> +
> +	$class = ' class="' . join(' ', 'group', $class) . '"';
> +
> +	$id = ' id="' . $id . '"' if $id;
> +
> +	print "<div$class$id>\n";

I agree with Ævar that it would be better to use HTML generation
subroutines from CGI.pm, even start_div and end_div...

> +	git_print_header_div(@rest);
> +}
> +
> +sub git_end_group {
> +	print "</div>\n"
> +}

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

> +
>  sub print_local_time {
>  	print format_local_time(@_);
>  }
> -- 
> 1.7.3.rc1.230.g8b572
> 
> 

-- 
Jakub Narebski
Poland