Re: [PATCH] gitweb: Show '...' links in "summary" view only if there are more items

[Jakub Narebski <jnareb@gmail.com>]

Robert Fitzsimons wrote:
> Show "..." links in "summary" view to shortlog, heads (if there are
> any), and tags (if there are any) only if there are more items to show
> than shown already.
> 
> This means that "..." link is shown below shortened shortlog if there
> are more than 16 commits, "..." link below shortened heads list if
> there are more than 16 heads refs (16 branches), "..." link below
> shortened tags list if there are more than 16 tags.
> 
> Modified patch from Jakub to to apply cleanly to master, also preform
> the same "..." link logic to the forks list.

Junio usually puts such comments in brackets (I don't know if it is
always used, i.e. if it is some 'convention'), e.g.:

  Also perform the same "..." link logic to the forks list.

  [rf: Modified patch from Jakub to to apply cleanly to master]

or something like that. Just a nitpick.


By the way, it looks like git_get_projects_list($project) used to get
list of forks does not have any count limit option.

[...]
> ---
>
> > By the way, I have _NOT_ applied Robert Fitzsimons patch, but they
> > (this patch and Robert patch) should be not in conflict if we
> > remove last chunk of Robert's patch (this changing --count=17 to
> > --count=15 in git_summary).
>
> Just removing the last chunk isn't correct, there are two slightly
> different changes in that chuck.  The reduction in the max-count
> value and a removal of a call to git_get_head_hash.

The last chunk I meant to be removed was:

> @@ -2876,8 +2879,8 @@ sub git_summary {
>                 }
>         }
>  
> -       open my $fd, "-|", git_cmd(), "rev-list", "--max-count=17",
> -               git_get_head_hash($project), "--"
> +       open my $fd, "-|", git_cmd(), "rev-list", "--max-count=16",
> +               $head, "--"
>                 or die_error(undef, "Open git-rev-list failed");
>         my @revlist = map { chomp; $_ } <$fd>;
>         close $fd;

and if we remove that chunk, then your earlier patch would not
touch git_summary at all, so mine would cleanly apply (I think).

[...]
> -	my @taglist  = git_get_tags_list(15);
> -	my @headlist = git_get_heads_list(15);
> +	# we need to request one more than 16 (0..15) to check if those 16 are all
> +	my @taglist  = git_get_tags_list(16);
> +	my @headlist = git_get_heads_list(16);

It needs to be 17, not 16, otherwise we never would get "...". By default
we show _16_ items, from 0 to 15 inclusive, so we must get _17_ items
to check if there are more than 16.

>  	my @forklist;
>  	my ($check_forks) = gitweb_check_feature('forks');
>  
> @@ -2955,30 +2956,35 @@ sub git_summary {
>  		}
>  	}
>  
> -	open my $fd, "-|", git_cmd(), "rev-list", "--max-count=16",
> +	# we need to request one more than 16 (0..15) to check if those 16 are all
> +	open my $fd, "-|", git_cmd(), "rev-list", "--max-count=17",

Here you have 17.


>  	if (@forklist) {
>  		git_print_header_div('forks');
>  		git_project_list_body(\@forklist, undef, 0, 15,
> +		                      $#forklist <= 15 ? undef :
>  		                      $cgi->a({-href => href(action=>"forks")}, "..."),
>  				      'noheader');
>  	}

Nice catch. I forgot about this one.

-- 
Jakub Narebski