Re: [PATCHv5 08/12] gitweb: auxiliary function to group data
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Sep 27, 2010, 08:12 UTC
- Message-ID
- <201009271012.23175.jnareb@gmail.com>
- In-Reply-To
- <AANLkTimOJ7RXDWXy=tF+rZf1gnfB7_GHCZuU5bZ5Wc91@mail.gmail.com>
Dnia poniedziałek 27. września 2010 09:26, Giuseppe Bilotta napisał:
Show 10 quoted lines
> 2010/9/26 Jakub Narebski <jnareb@gmail.com>: >> On Fri, 24 Sep 2010, Giuseppe Bilotta wrote: >> >>> Subject: gitweb: auxiliary function to group data >>> >> >> Errr... what!? git_group() is not "auxiliary function to group data", >> but a template for output of group of data. > > I will rephrase
Though I don't know how such rephrasing should look like.
Show 5 quoted lines
>> It would be probably good to describe how this output looks like >> (using e.g. ASCII-art mockup) in a commit message. > > Well, that would depend on the CSS that is used ... should I squash > the styling in this patch then?
No, I don't think it is needed, but it can be done.
Show 8 quoted lines
>>> +sub git_group {
>>
>> Name?
>
> git_collection? git_collect_data? I'm a little short on ideas.
> git_section? git_subsection? the function (with different styling) can
> probably be used even for the main sections in each view (think
> summary view in particular).git_group_html / git_subgroup_html / git_subsection_html print_group / print_subgroup / print_section / print_subsection
It needs either *_html suffix (like git_header_html), or print_* prefix to denote that it prints HTML fragment, I think.
Show 12 quoted lines
>>> + $content_func->() if defined $content_func;
>>
>> More defensive programming would be to use
>>
>> + $content_func->() if ref($content_func) eq 'CODE';
>>
>> Or even:
>>
>> + if (ref($content) eq 'CODE') {
>> + $content->();
>> + } elsif (ref($content) eq 'ARRAY') {
>> + print @$content;The 'ARRAY' part is probably unnecessary overengineering.
>> + } elsif (!ref($content) && defined($content)) {
>> + print $content;
>> + }Or even (in the vein of further overengineering)
+ } elsif (ref($content) eq 'SCALAR') {
+ print esc_html($$content);
+ } elsif (!ref($content) && defined($content)) {
+ print $content;
+ }or vice versa ;-)
>> >> Well, $content could be also open filehandle...
Though I don't know how to check that. ref on filehandles return 'GLOB'... well, we can use 'openhandle' from Scalar::Util (core). But that is probably unnecessary overengineering.
Show 5 quoted lines
> Ah, very interesting and very flexible, I'll steal your idea. > > -- > Giuseppe "Oblomov" Bilotta >
-- Jakub Narebski Poland