Subject: Re: [PATCH v2 03/11] gitweb: separate heads and remotes list in summary view

From: Jakub Narebski <[email-masked]>
Date: Fri, 14 Nov 2008 21:04:34 +0100

Dnia czwartek 13. listopada 2008 23:49, Giuseppe Bilotta napisał:

Very nice patch. Now that I have read it, I don't think it should be
squashed with previous patch (well, again that is only a suggestion).
Barring one issue (see below) its conciseness shows that gitweb has
quite good internal API.

> Signed-off-by: Giuseppe Bilotta <[email-masked]>
> ---
>  gitweb/gitweb.perl |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index d7c97a3..ab29aec 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -4449,6 +4449,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);
>  
> @@ -4456,7 +4457,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') : ();

Nice.

>  	my @forklist;
>  	my ($check_forks) = gitweb_check_feature('forks');
>  
> @@ -4535,6 +4537,13 @@ sub git_summary {
>  		               $cgi->a({-href => href(action=>"heads")}, "..."));
>  	}
>  
> +	if (@remotelist) {
> +		git_print_header_div('remotes');
> +		git_heads_body(\@remotelist, $head, 0, 15,
> +		               $#remotelist <= 15 ? undef :
> +		               $cgi->a({-href => href(action=>"heads")}, "..."));
> +	}
> +

The only problem is that link leads to list of _all_ heads (best case),
or list to local branches (worst case, but I don't think gitweb does
it), instead of only list of remotes refs (remote-tracking branches),
as one would think.  Perhaps we could use 'h' (hash), or 'opt (extra
options) parameter for this action, or just add 'remotes' action? 

>  	if (@forklist) {
>  		git_print_header_div('forks');
>  		git_project_list_body(\@forklist, 'age', 0, 15,
> -- 
> 1.5.6.5

P.S. Not uptodate (git version 1.6.0.4)? Just kidding...

-- 
Jakub Narebski
Poland