Re: [PATCHv5 07/12] gitweb: remotes view for a single remote
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Sep 26, 2010, 20:55 UTC
- Message-ID
- <201009262255.45959.jnareb@gmail.com>
- In-Reply-To
- <1285344167-8518-8-git-send-email-giuseppe.bilotta@gmail.com>
On Fri, 24 Sep 2010, Giuseppe Bilotta wrote:
> If the hash parameter is passed to gitweb, remotes will interpret it as > the name of a remote and limit the view the the heads of that remote.
Errr... I think this commit message needs rewriting to be more clear. Perhaps:
When 'remotes' view is passed 'hash' parameter, it would interprete it as the name of a remote ...
Show 17 quoted lines
>
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
> gitweb/gitweb.perl | 25 ++++++++++++++++++++-----
> 1 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 76cf806..7c62701 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -5547,13 +5547,28 @@ sub git_remotes {
> 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,format_ref_views('remotes'));
> + my $remote = $input_params{'hash'};I am not sure about using 'hash' parameter for that.
On one hand it is a hack that allow us to not worry about adding extra code to evaluate_path_info() subroutine, so that natural path_info URL of http://git.example.com/repo.git/remotes/<remote> would use <remote> as name of remote to limit to.
On the other hand it is abusing semantic of 'hash' parameter. Remote name is not revision name or object id.
What makes this issue stronger is the fact that URL is part of API, and if we make mistake here, we would have to maintain backward compatibility (at least if it appears in a released version).
> + > + git_header_html(undef, undef, 'header_extra' => $remote);
I don't quite like the name of this parameter, and I am not sure if I like the API either.
> + git_print_page_nav('', '', $head, undef, $head,
> + format_ref_views($remote ? '' : 'remotes'));Why this change?
Show 12 quoted lines
> git_print_header_div('summary', $project);
>
> - my @remotelist = git_get_heads_list(undef, 'remotes');
> - if (@remotelist) {
> - git_heads_body(\@remotelist, $head);
> + if (defined $remote) {
> + # only display the heads in a given remote
> + my @headslist = map {
> + my $ref = $_ ;
> + $ref->{'name'} =~ s!^$remote/!!;
> + $ref
> + } git_get_heads_list(undef, "remotes/$remote");Hmmm... do we need this temporary variable? Does it make anything more clear?
> + if (@headslist) {
> + git_heads_body(\@headslist, $head);
> + }This part is the same (modulo name of variable) in both branches of this conditional.
Show 12 quoted lines
> + } else {
> + my @remotelist = git_get_heads_list(undef, 'remotes');
> + if (@remotelist) {
> + git_heads_body(\@remotelist, $head);
> + }
> }
> git_footer_html();
> }
> --
> 1.7.3.68.g6ec8
>
> -- Jakub Narebski Poland