[pacman-dev] [PATCHv3 7/7] pacsearch: factored -Ss and -Qs parts into one single function

[Pierre Neidhardt]

On 14-02-10 11:15:25, Andrew Gregory wrote:
> On 02/09/14 at 07:41pm, Pierre Neidhardt wrote:
> > Signed-off-by: Pierre Neidhardt <ambrevar at gmail.com>
> > ---
> >  contrib/pacsearch.in | 65 +++++++++++++++++++++++-----------------------------
> >  1 file changed, 29 insertions(+), 36 deletions(-)
> > 
> > diff --git a/contrib/pacsearch.in b/contrib/pacsearch.in
> > index 0dad366..bcfedf0 100644
> > --- a/contrib/pacsearch.in
> > +++ b/contrib/pacsearch.in
> > @@ -98,44 +98,37 @@ sub print_pkg {
> >  my %allpkgs = ();
> >  my @pkglist = ();
> >  
> > -open (my $syncout, '-|', 'pacman', '-Ss', @ARGV) or exit;
> > -while ( readline($syncout) ) {
> > -	# We grab the following fields: repo, name, ver, group, installed, and
> > -	# desc. We grab leading space for 'group' and 'installed' so that we do not
> > -	# need to test if non-empty when printing.
> > -	my @pkgfields = /^(.*?)\/(.*?) (.*?)( \(.*?\))?( \[.*\])?$/s;
> > -	my $desc = readline($syncout);
> > -	# skip any non-matching line
> > -	next if not defined $pkgfields[1];
> > -	# since 'group' and 'installed' are optional, we should fill it in if necessary
> > -	$pkgfields[3] = "" if not defined $pkgfields[3];
> > -	$pkgfields[4] = "" if not defined $pkgfields[4];
> > -	$pkgfields[5] = $desc;
> > -	# Add each sync pkg by name/ver to a hash table.
> > -	# Any value is good since we only check for existence.
> > -	$allpkgs{$pkgfields[1] . $pkgfields[2]} = 1;
> > -	push (@pkglist, \@pkgfields);
> > -}
> > -close ($syncout);
> > -
> > -open (my $queryout, '-|', 'pacman', '-Qs', @ARGV) or exit;
> > -while ( readline($queryout) ) {
> > -	# 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 = /^(.*?)\/(.*?) (.*?)( \(.*?\))?( \[.*\])?$/s;
> > -	my $desc = readline($queryout);
> > -	# skip any non-matching line
> > -	next if not defined $pkgfields[1];
> > -	# check if the package was listed in the sync out
> > -	if (not exists $allpkgs{$pkgfields[1] . $pkgfields[2]}) {
> > -		# since 'group' is optional, we should fill it in if necessary
> > -		$pkgfields[3] = "" if not defined $pkgfields[3];
> > -		$pkgfields[4] = " [$LC_INSTALLED]";
> > -		$pkgfields[5] = $desc;
> > -		push (@pkglist, \@pkgfields);
> > +sub list_pkg {
> > +	my $db = shift;
> > +	my $installstr = shift;
> > +	open (my $out, '-|', 'pacman', $db , @ARGV) or exit;
> > +	while ( readline($out) ) {
> > +		# We grab the following fields: repo, name, ver, group, installed, and
> > +		# desc. We grab leading space for 'group' and 'installed' so that we do
> > +		# not need to test if non-empty when printing.
> > +		my @pkgfields = /^(.*?)\/(.*?) (.*?)( \(.*?\))?( \[.*\])?$/s;
> > +		my $desc = readline($out);
> > +		# skip any non-matching line
> > +		next if not defined $pkgfields[1];
> > +		my $pkgid = $pkgfields[1] . $pkgfields[2];
> > +		# if -Qs, check if the package was listed during list_pkg -Ss
> > +		if ( $db eq "-Ss" || not exists $allpkgs{$pkgid}) {
> 
> I see no reason to bother with these db-specific modifications inside
> this function.  Return the list of found packages and do the
> modifications afterward.
> 
>  my @sync = list_pkg('-Ss', @ARGV);
>  my %allpkgs = map {$_->[1] . $_->[2] => 1} @sync;
>  my @local = grep { not $allpkgs{$_->[1] . $_->[2]} } list_pkg('-Qs', @ARGV);
>  $_->[4] = " [$LC_INSTALLED]" foreach @local;
>  print_pkg($_) foreach (@sync, @local);

Yes, looks much better to me.

> > +			# since 'group' and 'installed' are optional, we should fill it in
> > +			# if necessary
> > +			$pkgfields[3] = "" if not defined $pkgfields[3];
> > +			$pkgfields[4] = $installstr if not defined $pkgfields[4];
> > +			$pkgfields[5] = $desc;
> > +			# Add each sync pkg by name/ver to a hash table.
> > +			# Any value is good since we only check for existence.
> > +			$allpkgs{$pkgid} = 1;
> > +			push (@pkglist, \@pkgfields);
> > +		}
> >  	}
> > +	close ($out);
> >  }
> > -close ($queryout);
> > +
> > +list_pkg ('-Ss', "");
> > +list_pkg ('-Qs', " [$LC_INSTALLED]");
> >  
> >  foreach (@pkglist) {
> >  	print_pkg (@{$_});
> > -- 
> > 1.8.5.4

-- 
Pierre Neidhardt

Rule of Defactualization:
	Information deteriorates upward through bureaucracies.