Re: [PATCH v3 2/3] gitweb: Split git_project_list_body in two functions

[Jakub Narebski <jnareb@gmail.com>]

On Tue, 4 Dec 2008 at 01:44, Sébastien Cevey wrote:

> Extract the printing of project rows on the project page into a
> separate print_project_rows function. This makes it easier to reuse
> the code to print different subsets of the whole project list.

Err... it was not obvious for me that 'project rows' are rows of
projects list table, i.e. currently the body of projects list table.

But I think it is a good description.

> 
> The row printing code is merely moved into a separate function, but
> note that $projects is passed as a reference now.
> 
> Signed-off-by: Sebastien Cevey <seb@cine7.net>

Nicely done.

For what it is worth:

Acked-by: Jakub Narebski <jnareb@gmail.com>

> ---
> 
> Boo, the diff is still quite scarier than it really should be...

Well, nothing short of blame -C would make it work; the issue is that
we split subroutine into two, extracting from the middle, but with
some common/similar header

  2        1
  1        1
  2        1
  2
  2   ->   2
  1        2
  1        2
  2        2
  2        2
           2

And diff is forward preferring, i.e. it finds match then second part
of split as add, rather than first first part of split as add then
match.
> 
>  gitweb/gitweb.perl |  103 +++++++++++++++++++++++++++++-----------------------
>  1 files changed, 57 insertions(+), 46 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index b31274c..a6bb702 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3972,59 +3972,19 @@ sub print_sort_th {
>  	}
>  }
>  
> -sub git_project_list_body {
> +# print a row for each project in the given list, using the given
> +# range and extra display options
> +sub print_project_rows {
>  	# actually uses global variable $project
> -	my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
> -
> -	my ($check_forks) = gitweb_check_feature('forks');
> -	my @projects = fill_project_list_info($projlist, $check_forks);
> +	my ($projects, $from, $to, $check_forks, $show_ctags) = @_;

I see that print_project_rows gets @$projects list already sorted, and
it passes explicitly $check_forks and $show_ctags from caller.

>  
> -	$order ||= $default_projects_order;
>  	$from = 0 unless defined $from;
> -	$to = $#projects if (!defined $to || $#projects < $to);
> -
> -	my %order_info = (
> -		project => { key => 'path', type => 'str' },
> -		descr => { key => 'descr_long', type => 'str' },
> -		owner => { key => 'owner', type => 'str' },
> -		age => { key => 'age', type => 'num' }
> -	);
> -	my $oi = $order_info{$order};
> -	if ($oi->{'type'} eq 'str') {
> -		@projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @projects;
> -	} else {
> -		@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
> -	}
> +	$to = $#$projects if (!defined $to || $#$projects < $to);
>  
> -	my $show_ctags = gitweb_check_feature('ctags');
> -	if ($show_ctags) {
> -		my %ctags;
> -		foreach my $p (@projects) {
> -			foreach my $ct (keys %{$p->{'ctags'}}) {
> -				$ctags{$ct} += $p->{'ctags'}->{$ct};
> -			}
> -		}
> -		my $cloud = git_populate_project_tagcloud(\%ctags);
> -		print git_show_project_tagcloud($cloud, 64);
> -	}
> -
> -	print "<table class=\"project_list\">\n";
> -	unless ($no_header) {
> -		print "<tr>\n";
> -		if ($check_forks) {
> -			print "<th></th>\n";
> -		}
> -		print_sort_th('project', $order, 'Project');
> -		print_sort_th('descr', $order, 'Description');
> -		print_sort_th('owner', $order, 'Owner');
> -		print_sort_th('age', $order, 'Last Change');
> -		print "<th></th>\n" . # for links
> -		      "</tr>\n";
> -	}
>  	my $alternate = 1;
>  	my $tagfilter = $cgi->param('by_tag');
>  	for (my $i = $from; $i <= $to; $i++) {
> -		my $pr = $projects[$i];
> +		my $pr = $projects->[$i];
>  
>  		next if $tagfilter and $show_ctags and not grep { lc $_ eq lc $tagfilter } keys %{$pr->{'ctags'}};
>  		next if $searchtext and not $pr->{'path'} =~ /$searchtext/
> @@ -4068,6 +4028,57 @@ sub git_project_list_body {
>  		      "</td>\n" .
>  		      "</tr>\n";
>  	}
> +}
> +
> +sub git_project_list_body {
> +	my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
> +
> +	my ($check_forks) = gitweb_check_feature('forks');
> +	my @projects = fill_project_list_info($projlist, $check_forks);
> +
> +	$order ||= $default_projects_order;
> +
> +	my %order_info = (
> +		project => { key => 'path', type => 'str' },
> +		descr => { key => 'descr_long', type => 'str' },
> +		owner => { key => 'owner', type => 'str' },
> +		age => { key => 'age', type => 'num' }
> +	);
> +	my $oi = $order_info{$order};
> +	if ($oi->{'type'} eq 'str') {
> +		@projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @projects;
> +	} else {
> +		@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
> +	}
> +
> +	my $show_ctags = gitweb_check_feature('ctags');
> +	if ($show_ctags) {
> +		my %ctags;
> +		foreach my $p (@projects) {
> +			foreach my $ct (keys %{$p->{'ctags'}}) {
> +				$ctags{$ct} += $p->{'ctags'}->{$ct};
> +			}
> +		}
> +		my $cloud = git_populate_project_tagcloud(\%ctags);
> +		print git_show_project_tagcloud($cloud, 64);
> +	}
> +
> +	print "<table class=\"project_list\">\n";
> +	unless ($no_header) {
> +		print "<tr>\n";
> +		if ($check_forks) {
> +			print "<th></th>\n";
> +		}
> +		print_sort_th('project', $order, 'Project');
> +		print_sort_th('descr', $order, 'Description');
> +		print_sort_th('owner', $order, 'Owner');
> +		print_sort_th('age', $order, 'Last Change');
> +		print "<th></th>\n" . # for links
> +		      "</tr>\n";
> +	}
> +
> +	print_project_rows(\@projects, $from, $to, $check_forks, $show_ctags);
> +
>  	if (defined $extra) {
>  		print "<tr>\n";
>  		if ($check_forks) {
> -- 
> 1.5.6.5
> 
> 

-- 
Jakub Narebski
Poland