Jakub Narebski <jnareb@xxxxxxxxx> writes:
> Make git_get_refs_list do also work of git_get_references, to avoid
> calling git-peek-remote twice. Change meaning of git_get_refs_list
> meaning: it is now type, and not a full path, e.g. we now use
> git_get_refs_list("heads") instead of former
> git_get_refs_list("refs/heads").
>
> Modify git_summary to use only one call to git_get_refs_list instead
> of one call to git_get_references and two to git_get_refs_list.
Will apply as-is; the following comments might be useful for
further improvements.
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 034a88c..01fae94 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1127,14 +1128,21 @@ sub git_get_refs_list {
> + if ($line =~ m/^([0-9a-fA-F]{40})\trefs\/($type\/?([^\^]+))(\^\{\})?$/) {
> + if (defined $refs{$1}) {
> + push @{$refs{$1}}, $2;
> + } else {
> + $refs{$1} = [ $2 ];
> + }
Logically this should be "exists $refs{$1}" but defined would
work fine in this particular case, because the elements of the
hash are always an arrayref (this is just an advise to have the
right habit).
> + if (! $4) { # unpeeled, direct reference
> + push @refs, { hash => $1, name => $3 }; # without type
> + } elsif ($3 eq $refs[-1]{'name'}) {
> + # most likely a tag is followed by its peeled
> + # (deref) one, and when that happens we know the
> + # previous one was of type 'tag'.
> + $refs[-1]{'type'} = "tag";
> + }
>...
> - return \@reflist;
> + return (\@reflist, \%refs);
You are maintaining reflist (an array of hashrefs each element
of which describes name, type, hash and other parse_ref()
information for the ref) and refs (a hash that maps from name to
hash) separatly. I wonder if this really has the performance
advantage over just compute and return reflist from here and
have the callers who need the mapping to derive it from the list
(i.e.
my ($reflist) = git_get_refs_list();
my %refs = map { $_->{name} => $_->{hash} } @$reflist;
).
> @@ -2114,14 +2122,14 @@ sub git_tags_body {
>
> sub git_heads_body {
> # uses global variable $project
> - my ($taglist, $head, $from, $to, $extra) = @_;
> + my ($headlist, $head, $from, $to, $extra) = @_;
> $from = 0 unless defined $from;
> - $to = $#{$taglist} if (!defined $to || $#{$taglist} < $to);
> + $to = $#{$headlist} if (!defined $to || $#{$headlist} < $to);
>
> print "<table class=\"heads\" cellspacing=\"0\">\n";
> my $alternate = 0;
> for (my $i = $from; $i <= $to; $i++) {
> - my $entry = $taglist->[$i];
> + my $entry = $headlist->[$i];
> my %tag = %$entry;
> my $curr = $tag{'id'} eq $head;
> if ($alternate) {
I guess either would do but I think it is more conventional to
say:
my $limit = @list;
for (my $i = $from; $i < $limit; $i++) { ... }
than
my $limit = $#list;
for (my $i = $from; $i <= $limit; $i++) { ... }
At least the former would be easier to read for C types among us.
The remaining hunks are very nice improvements.
> @@ -2291,7 +2299,19 @@ sub git_summary {
> @@ -2321,17 +2341,15 @@ sub git_summary {
> @@ -2542,7 +2560,7 @@ sub git_tags {
> @@ -2555,9 +2573,9 @@ sub git_heads {
-
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