[pacman-dev] [PATCH v2 4/6] pacsearch: fixed regular expression for local searches

[Andrew Gregory]

On 02/05/14 at 04:19pm, Pierre Neidhardt wrote:
> On 14-02-05 09:22:49, Andrew Gregory wrote:
> > On 02/04/14 at 12:16am, Pierre Neidhardt wrote:
> > > Signed-off-by: Pierre Neidhardt <ambrevar at gmail.com>
> > > ---
> > >  contrib/pacsearch.in | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> >  
> > -1 from me.  I think this is a step in the wrong direction.  The sync
> > and local search code are virtually identical and should be
> > consolidated into a function, not made more different.
> 
> These part were always ~ 50% different. It's less thant 10 lines of code anyway,
> no big deal. Trying to make a common function out of this would result in longer
> and more complex code I believe.

The sync search code is 21 lines.  Only 1 line of that is actually
unique to it and can very easily be moved outside of a consolidated
function.  The only significant difference between the two is that the
sync search prints lines it can't parse and the local search doesn't.
Neither the script nor the git log explains the disparity and I don't
see any reason to continue it.

> > > diff --git a/contrib/pacsearch.in b/contrib/pacsearch.in
> > > index 66b1683..5656b07 100644
> > > --- a/contrib/pacsearch.in
> > > +++ b/contrib/pacsearch.in
> > > @@ -134,8 +134,9 @@ if ($#querypkgs >= 0) {
> > >  }
> > >  
> > >  foreach $_ (@querypkgs) {
> > > -	# we grab the following fields: repo, name, ver, group, installed, and desc
> > > -	my @pkgfields = /^(.*?)\/(.*?) (.*?) ?(\(.*?\))? ?(\[.*\])?\n(.*)$/s;
> > > +	# We grab the same field as before, even the "installed" which is always
> > > +	# empty for local searches. This allows us to reserve a cell in @pkgfields.
> > > +	my @pkgfields = /^(.*?)\/(.*?) (.*?) ?(\(.*?\))?()?\n(.*)$/s;
> > >  	# skip any non-matching line
> > >  	next if not defined $pkgfields[1];
> > >  	# check if the package was listed in the sync out
> > > -- 
> > > 1.8.5.3