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

From: Jakub Narebski <jnareb@gmail.com>
Date: Dec 19, 2006, 12:28 UTC
Message-ID: <200612191328.08928.jnareb@gmail.com>
In-Reply-To: <20061219120854.GA16429@localhost>

Robert Fitzsimons wrote:

Show 11 quoted lines

> 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.

[...]

Show 10 quoted lines

> ---
>
> > 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:

Show 11 quoted lines

> @@ -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).

[...]

Show 5 quoted lines

> -	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.

Show 10 quoted lines

>  	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.

Show 7 quoted lines

>  	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