Re: [PATCH (BUGFIX)] gitweb: Fix fixed string (non-regexp) project search
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Mar 5, 2012, 09:03 UTC
- Message-ID
- <201203051003.10605.jnareb@gmail.com>
- In-Reply-To
- <7vaa3w55i3.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
Show 57 quoted lines
> 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