Re: [PATCH (BUGFIX)] gitweb: Fix fixed string (non-regexp) project search

[Jakub Narebski <jnareb@gmail.com>]

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > ....  The fix is about adding new code and should
> > apply cleanly to 'maint' and even to older versions; the only trouble
> > with older version might be whitespace issue related to refactoring
> > code into subroutines.
> 
> OK, so the global $searchtext is what came from form submit from the end
> user, while the global $search_regexp is what the code should be using
> for matching throughout the program, prepared by eval-and-validate-params.
> 
> Here is a hand-ported version of your patch that should apply to 1.7.6.6;
> does it look sane?
> 
>  gitweb/gitweb.perl |   12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 50a835a..d1698b7 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2905,10 +2905,10 @@ sub filter_forks_from_projects_list {
>  sub search_projects_list {
>  	my ($projlist, %opts) = @_;
>  	my $tagfilter  = $opts{'tagfilter'};
> -	my $searchtext = $opts{'searchtext'};
> +	my $search_re = $opts{'search_regexp'};
>  
>  	return @$projlist
> -		unless ($tagfilter || $searchtext);
> +		unless ($tagfilter || $search_re);
>  
>  	my @projects;
>   PROJECT:
> @@ -2920,10 +2920,10 @@ sub search_projects_list {
>  				grep { lc($_) eq lc($tagfilter) } keys %{$pr->{'ctags'}};
>  		}
>  
> -		if ($searchtext) {
> +		if ($search_re) {
>  			next unless
> -				$pr->{'path'} =~ /$searchtext/ ||
> -				$pr->{'descr_long'} =~ /$searchtext/;
> +				$pr->{'path'} =~ /$search_re/ ||
> +				$pr->{'descr_long'} =~ /$search_re/;
>  		}
>  
>  		push @projects, $pr;
> @@ -5097,7 +5097,7 @@ sub git_project_list_body {
>  	@projects = fill_project_list_info(\@projects);
>  	# searching projects require filling to be run before it
>  	@projects = search_projects_list(\@projects,
> -	                                 'searchtext' => $searchtext,
> +	                                 'search_regexp' => $search_regexp,
>  	                                 'tagfilter'  => $tagfilter)
>  		if ($tagfilter || $searchtext);
>  

It looks sane, though 

  	@projects = search_projects_list(\@projects,
 -	                                 'searchtext' => $searchtext,
 +	                                 'search_regexp' => $search_regexp,
  	                                 'tagfilter'  => $tagfilter)
  		if ($tagfilter || $searchtext);

should be better written as

  	@projects = search_projects_list(\@projects,
 -	                                 'searchtext' => $searchtext,
 +	                                 'search_regexp' => $search_regexp,
  	                                 'tagfilter'  => $tagfilter)
 - 		if ($tagfilter || $searchtext);
 + 		if ($tagfilter || $search_regexp);

It is functionally the same, because $search_regexp is derived from
$searchtext, but IMHO it is more clear.

-- 
Jakub Narebski
Poland