Re: [PATCH] Small optimizations to gitweb

From: Junio C Hamano <junkio@cox.net>
Date: Dec 18, 2006, 23:45 UTC
Message-ID: <7vbqm0vkd6.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <em77cg$obn$1@sea.gmane.org>

Jakub Narebski <jnareb@gmail.com> writes:

> Actually, that is needed to implement checking if we have more than
> the number of commits to show to add '...' at the end only if there
> are some commits which we don't show.

The counting code in git_*_body is seriously unusual to tempt anybody who reviews the code to reduce that 17 to 16.

The caller says:

	git_shortlog_body(\@revlist, 0, 15, $refs,
	                  $cgi->a({-href => href(action=>"shortlog")}, "..."));

If it counts up, especially if it counts from zero, the loop would usually say:

	for (i = bottom; i < end; i++)

and anybody who reads that caller would expect it to show 15 lines of output.

But the actual code does this instead:

    sub git_shortlog_body {
            # uses global variable $project
            my ($revlist, $from, $to, $refs, $extra) = @_;

            $from = 0 unless defined $from;
            $to = $#{$revlist} if (!defined $to || $#{$revlist} < $to);
            ...
            for (my $i = $from; $i <= $to; $i++) {
                    ... draw each item ...
            }
            if (defined $extra) {
                    print "<tr>\n" .
                          "<td colspan=\"4\">$extra</td>\n" .
                          "</tr>\n";
            }
    }

By the way, I wonder how that $extra is omitted when $revlist is longer than $to; it should be a trivial fix but it seems to me that it is always spitted out with the current code.