Re: [PATCHv3 2/4] gitweb: git_get_heads_list accepts an optional list of refs.

[Jakub Narebski <jnareb@gmail.com>]

On Sun, 16 Nov 2008, Junio C Hamano wrote:
> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
> 
> > git_get_heads_list(limit, class1, class2, ...) can now be used to retrieve
> > refs/class1, refs/class2 etc. Defaults to ('heads') or ('heads', 'remotes')
> > depending on the remote_heads option.
> >
> > Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> > ---
> >  gitweb/gitweb.perl |   11 +++++++----
> >  1 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index e1f81f6..0512020 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -2681,15 +2681,18 @@ sub parse_from_to_diffinfo {
> >  ## parse to array of hashes functions
> >  
> >  sub git_get_heads_list {
> > -	my $limit = shift;
> > +	my ($limit, @class) = @_;
> > +	unless (defined @class) {
> > +		my $remote_heads = gitweb_check_feature('remote_heads');
> > +		@class = ('heads', $remote_heads ? 'remotes' : undef);
> > +	}
> > +	my @refs = map { "refs/$_" } @class;
> 
> Makes sense, except that I'd suggest passing a hash of "refs/$path" =>
> $class as I illustrated in my comments to [1/4], instead of passing a list
> of ("head", "remote"), because that will later allow you to have
> multi-level $path that does not necessarily limited to a $class that is a
> substring of $path, and doing so does not make the code any more complex.
> There is another reason to do so I'll mention in I comment on [3/4].

By the way, with %head_class hash passed as reference git_get_head_list
could be done in such way, that you can write 

  git_get_heads_list(\%head_class);

instead of longer

  git_get_heads_list(undef, \%head_class);

when there is no limit(er).
-- 
Jakub Narebski
Poland