Re: [PATCHv5 03/12] gitweb: separate heads and remotes lists

From: Jakub Narebski <jnareb@gmail.com>
Date: Sep 26, 2010, 17:39 UTC
Message-ID: <201009261939.12107.jnareb@gmail.com>
In-Reply-To: <1285344167-8518-4-git-send-email-giuseppe.bilotta@gmail.com>

On Fri, 24 Sep 2010, Giuseppe Bilotta wrote:

Show 8 quoted lines

> 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 <giuseppe.bilotta@gmail.com>

Nice (and still uncontroversial) solution.  FWIW

Acked-by: Jakub Narebski <jnareb@gmail.com>

Show 20 quoted lines

> @@ -5118,7 +5120,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') : ();
>  	my @forklist;
>  	my $check_forks = gitweb_check_feature('forks');
>  
> @@ -5196,6 +5199,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=>"remotes")}, "..."));
> +	}

Nice thing. This meanst that "remotes" section is displayed *only* if 'remote_heads' feature is enabled and we actually have remote-tracking branches.

Show 6 quoted lines

> @@ -5503,13 +5513,29 @@ sub git_heads {
>  	git_print_page_nav('','', $head,undef,$head);
>  	git_print_header_div('summary', $project);
>  
> -	my @headslist = git_get_heads_list();
> +	my @headslist = git_get_heads_list(undef, 'heads');

It's a pity that we can't simply write "git_get_heads_list('heads');", but I think it would be serious overengineering for a tiny little gain.

Show 21 quoted lines

>  	if (@headslist) {
>  		git_heads_body(\@headslist, $head);
>  	}
>  	git_footer_html();
>  }
>  
> +sub git_remotes {
> +	gitweb_check_feature('remote_heads')
> +		or die_error(403, "Remote heads view is disabled");
> +
> +	my $head = git_get_head_hash($project);
> +	git_header_html();
> +	git_print_page_nav('','', $head,undef,$head);
> +	git_print_header_div('summary', $project);
> +
> +	my @remotelist = git_get_heads_list(undef, 'remotes');
> +	if (@remotelist) {
> +		git_heads_body(\@remotelist, $head);
> +	}
> +	git_footer_html();
> +}

Hmmmm... what we have there is a bit of code repetition with git_heads and git_remotes, but I think is unevitable... besides you would be extending git_remotes subroutine in subsequent patches. So it doesn't matter, I think.

-- 
Jakub Narebski
Poland