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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>

Nicely done.

For what it is worth:

Acked-by: Jakub Narebski <jnareb@xxxxxxxxx>

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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux