Subject: Re: [PATCHv3 3/4] gitweb: separate heads and remotes lists

From: Junio C Hamano <[email-masked]>
Date: Sun, 16 Nov 2008 09:34:53 -0800

Giuseppe Bilotta <[email-masked]> writes:

> We specialize the 'heads' action to only display local branches, and
> introduce a 'remotes' action to display the remote branches (only
> available when the remotes_head feature is enabled).
>
> Mirroring this, we also split the heads list in summary view into
> local and remote lists, each linking to the appropriate action.
>
> Signed-off-by: Giuseppe Bilotta <[email-masked]>
> ---
>  gitweb/gitweb.perl |   30 ++++++++++++++++++++++++++++--
>  1 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 0512020..6b09918 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -527,6 +527,7 @@ our %actions = (
>  	"heads" => \&git_heads,
>  	"history" => \&git_history,
>  	"log" => \&git_log,
> +	"remotes" => \&git_remotes,
>  	"rss" => \&git_rss,
>  	"atom" => \&git_atom,
>  	"search" => \&git_search,
> @@ -4467,6 +4468,7 @@ sub git_summary {
>  	my %co = parse_commit("HEAD");
>  	my %cd = %co ? parse_date($co{'committer_epoch'}, $co{'committer_tz'}) : ();
>  	my $head = $co{'id'};
> +	my $remote_heads = gitweb_check_feature('remote_heads');
>  
>  	my $owner = git_get_project_owner($project);
>  
> @@ -4474,7 +4476,8 @@ sub git_summary {
>  	# These get_*_list functions return one more to allow us to see if
>  	# there are more ...
>  	my @taglist  = git_get_tags_list(16);
> -	my @headlist = git_get_heads_list(16);
> +	my @headlist = git_get_heads_list(16, 'heads');
> +	my @remotelist = $remote_heads ? git_get_heads_list(16, 'remotes') : ();

Wasteful to run one for-each-ref for each list.

You earlier introduced $ref_item{'class'} so that you can differenciate
what you got from git_get_heads_list(); make use of it, perhaps like:

	my @heads_list = git_get_heads_list(16, \%head_class);
        my @headlist = grep { $_->{'class'} eq 'head' } @heads_list;
        my @remotelist = grep { $_->{'class'} eq 'remote' } @heads_list;

By the way, your [2/4] used "heads" and "remotes" as class, while your
[1/4] stored 'head' and 'remote' in $ref_item{'class'}.  Notice the above
suggestion corrects this discrepancy as well.