On Thu, 16 Sep 2010, Giuseppe Bilotta wrote:
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 92551e4..66b5400 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2758,6 +2758,16 @@ sub git_get_last_activity {
> return (undef, undef);
> }
>
> +sub git_get_remotes {
> + my ($limit) = @_;
Probably more Perl-ish way would be to use
+ my $limit = shift;
but this version is also all right.
> + open my $fd, '-|' , git_cmd(), 'remote';
> + return () unless $fd;
Gitweb usualy uses
+ open my $fd, '-|' , git_cmd(), 'remote';
+ or return;
> + my @remotes = map { chomp ; $_ } <$fd>;
About Ævar comment: while
+ chomp(my @remotes = <$fd>);
might be more Perl-ish, the above syntax is used thorough gitweb code.
So I'd leave it as it is now as the matter of code consistency.
> + close $fd or return ();
+ close $fd or return;
> + my @remoteheads = git_get_heads_list($limit, 'remotes');
> + return (\@remotes, \@remoteheads);
Why do you want for git_get_remotes() to also return remote-tracking
branches (refs/remotes/)? Those are separate issues, and I think it
would be better API for git_get_remotes() to provide only list of
remotes, i.e.
+ return @remotes;
Especially that we might want in the summary view to only list remotes,
without listing remote-tracking branches.
That would require more changes to the code.
> +}
> +
> sub git_get_references {
> my $type = shift || "";
> my %refs;
> @@ -4979,7 +4989,7 @@ sub git_heads_body {
> "<td class=\"link\">" .
> $cgi->a({-href => href(action=>"shortlog", hash=>$ref{'fullname'})}, "shortlog") . " | " .
> $cgi->a({-href => href(action=>"log", hash=>$ref{'fullname'})}, "log") . " | " .
> - $cgi->a({-href => href(action=>"tree", hash=>$ref{'fullname'}, hash_base=>$ref{'name'})}, "tree") .
> + $cgi->a({-href => href(action=>"tree", hash=>$ref{'fullname'}, hash_base=>$ref{'fullname'})}, "tree") .
This is independent change, and should be in a separate commit, isn't it?
> "</td>\n" .
> "</tr>";
> }
> @@ -4991,6 +5001,19 @@ sub git_heads_body {
> print "</table>\n";
> }
>
> +sub git_remotes_body {
> + my ($remotedata, $head) = @_;
> + my @remotenames = @{$remotedata->[0]};
> + my @allheads = @{$remotedata->[1]};
Why not
+ my ($remotenames, $allheads, $head) = @_;
Beside, isn't it $remote_heads and not $allheads?
> + foreach my $remote (@remotenames) {
It would be then
+ foreach my $remote (@$remotenames) {
> + my @remoteheads = grep { $_->{'name'} =~ s!^\Q$remote\E/!! } @allheads;
Should we display remote even if it doesn't have any remote heads
associated with it?
> + git_begin_group("remotes", $remote, "remotes/$remote",$remote);
> + git_heads_body(\@remoteheads, $head);
> + git_end_group();
This would have to be modified with change to git_begin_group() /
/ git_end_group().
BTW isn't it premature generalization? It is only place AFAIKS that
uses git_*_group() subroutines.
> + }
> +
> +}
> +
> sub git_search_grep_body {
> my ($commitlist, $from, $to, $extra) = @_;
> $from = 0 unless defined $from;
> @@ -5137,7 +5160,7 @@ sub git_summary {
> # there are more ...
> my @taglist = git_get_tags_list(16);
> my @headlist = git_get_heads_list(16, 'heads');
> - my @remotelist = $remote_heads ? git_get_heads_list(16, 'remotes') : ();
> + my @remotelist = $remote_heads ? git_get_remotes(16) : ();
No change of git_get_remotes() does only one thing: returning list
of remotes.
> my @forklist;
> my $check_forks = gitweb_check_feature('forks');
>
> @@ -5217,9 +5240,7 @@ sub git_summary {
>
> if (@remotelist) {
> git_print_header_div('remotes');
> - git_heads_body(\@remotelist, $head, 0, 15,
> - $#remotelist <= 15 ? undef :
> - $cgi->a({-href => href(action=>"remotes")}, "..."));
> + git_remotes_body(\@remotelist, $head);
Calling convention would change with my proposed change.
We might want to print only list of remotes (perhaps with number of
tracked branches) in the 'summary' view. Just an idea...
> }
>
> if (@forklist) {
> @@ -5551,9 +5572,9 @@ sub git_remotes {
> git_print_page_nav('','', $head,undef,$head, $heads_nav);
> git_print_header_div('summary', $project);
>
> - my @remotelist = git_get_heads_list(undef, 'remotes');
> + my @remotelist = git_get_remotes();
> if (@remotelist) {
> - git_heads_body(\@remotelist, $head);
> + git_remotes_body(\@remotelist, $head);
Same here.
> }
> git_footer_html();
> }
> --
> 1.7.3.rc1.230.g8b572
>
>
--
Jakub Narebski
Poland
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html