Junio C Hamano wrote:
> Jakub Narebski <jnareb@xxxxxxxxx> 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 ...
> }
Well, this should be then corrected perhaps to
my ($revlist, $begin, $end, $refs, $extra) = @_;
$begin = 0 unless defined $from;
$end = scalar(@$revlist) if (!defined $end || @$revlist <= $end);
...
for (my $i = $begin; $i < $end; $i++) {
... draw each item ...
}
I thought that $from..$to ($from <= i <= $to) is more natural
and easier to understand than $begin..$end ($begin <= i < $end)...
guess I guessed wrong.
> 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.
We should check if we want to omit $extra, either in caller or
in callee, the *_body subroutine itself.
--
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