[pacman-dev] [PATCHv3 1/7] pacsearch: removed useless comment
Signed-off-by: Pierre Neidhardt <ambrevar@gmail.com> --- contrib/pacsearch.in | 1 - 1 file changed, 1 deletion(-) diff --git a/contrib/pacsearch.in b/contrib/pacsearch.in index 21043d4..71e0107 100644 --- a/contrib/pacsearch.in +++ b/contrib/pacsearch.in @@ -138,7 +138,6 @@ if ($#querypkgs >= 0) { foreach $_ (@querypkgs) { # we grab the following fields: repo, name, ver, group, installed, and desc my @pkgfields = /^(.*?)\/(.*?) (.*?) ?(\(.*?\))? ?(\[.*\])?\n(.*)$/s; - # 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.4
In the old pacsearch, packages were identified uniquely by pkgfields[1], which contained pkgname+pkgver. Since commit 4d13558 pkgver is stored in pkgfields[2], and packages have been identified with pkgfields[1] only. Because of that packages with a different version would appear once only. This fixes the regression by identifying packages with both pkgfields[1] and pkgfields[2]. Signed-off-by: Pierre Neidhardt <ambrevar@gmail.com> --- contrib/pacsearch.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/pacsearch.in b/contrib/pacsearch.in index 71e0107..d3d461f 100644 --- a/contrib/pacsearch.in +++ b/contrib/pacsearch.in @@ -124,7 +124,7 @@ foreach $_ (@syncpkgs) { # add a last field that indicates original order push (@pkgfields, $cnt++); # add each sync pkg by name/ver to a hash table for quick lookup - $allpkgs{$pkgfields[1]} = [ @pkgfields ]; + $allpkgs{$pkgfields[1] . $pkgfields[2]} = [ @pkgfields ]; } my $queryout = `pacman -Qs '@ARGV'`; @@ -141,14 +141,14 @@ foreach $_ (@querypkgs) { # 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]}) { + 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]"; # add a last field that indicates original order (after sync) push (@pkgfields, $cnt++); # add our local-only package to the hash - $allpkgs{$pkgfields[1]} = [ @pkgfields ]; + $allpkgs{$pkgfields[1] . $pkgfields[2]} = [ @pkgfields ]; } } -- 1.8.5.4
Package are processed in the same order as pacman output, so there is no real need to sort. This makes the code simpler and faster. The only difference is that local packages will always be printed at the end. Previously, they were printed before multilib for instance. Signed-off-by: Pierre Neidhardt <ambrevar@gmail.com> --- contrib/pacsearch.in | 47 +++++++++++++++++++---------------------------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/contrib/pacsearch.in b/contrib/pacsearch.in index d3d461f..91cf364 100644 --- a/contrib/pacsearch.in +++ b/contrib/pacsearch.in @@ -81,24 +81,25 @@ if ($ARGV[0] eq "--nocolor" || $ARGV[0] eq "-n") { # localization my $LC_INSTALLED = `gettext pacman installed`; -# Color a "repo/pkgname pkgver (groups) [installed]" line. -# We try to stick to pacman colors. -sub to_color { +# Print a "repo/pkgname pkgver (groups) [installed]" line. +# We stick to pacman colors. +sub print_pkg { my @v = @_; - my $line = "$RESET$BOLD"; + print "$RESET$BOLD"; if ( "$v[0]" eq "local" ) { - $line .= "$RED"; + print "$RED"; } else { - $line .= "$MAGENTA"; + print "$MAGENTA"; } - $line .= "$v[0]/$RESET$BOLD$v[1] $GREEN$v[2]"; - $line .= " $BLUE$v[3]" if $v[3] ne ""; - $line .= " $CYAN$v[4]" if $v[4] ne ""; - $line .= " $RESET"; - return $line; + print "$v[0]/$RESET$BOLD$v[1] $GREEN$v[2]"; + print " $BLUE$v[3]" if $v[3] ne ""; + print " $CYAN$v[4]" if $v[4] ne ""; + print " $RESET\n"; + print " $v[5]\n"; } my %allpkgs = (); +my @pkglist = (); my $syncout = `pacman -Ss '@ARGV'`; # split each sync search entry into its own array entry @@ -108,8 +109,6 @@ if ($#syncpkgs >= 0) { chomp($syncpkgs[$#syncpkgs]); } -# counter var for packages, used here and in the query loop too -my $cnt = 0; foreach $_ (@syncpkgs) { # we grab the following fields: repo, name, ver, group, installed, and desc my @pkgfields = /^(.*?)\/(.*?) (.*?) ?(\(.*?\))? ?(\[.*\])?\n(.*)$/s; @@ -121,10 +120,10 @@ foreach $_ (@syncpkgs) { # 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]; - # add a last field that indicates original order - push (@pkgfields, $cnt++); - # add each sync pkg by name/ver to a hash table for quick lookup - $allpkgs{$pkgfields[1] . $pkgfields[2]} = [ @pkgfields ]; + # 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); } my $queryout = `pacman -Qs '@ARGV'`; @@ -145,20 +144,12 @@ foreach $_ (@querypkgs) { # since 'group' is optional, we should fill it in if necessary $pkgfields[3] = "" if not defined $pkgfields[3]; $pkgfields[4] = "[$LC_INSTALLED]"; - # add a last field that indicates original order (after sync) - push (@pkgfields, $cnt++); - # add our local-only package to the hash - $allpkgs{$pkgfields[1] . $pkgfields[2]} = [ @pkgfields ]; + push (@pkglist, \@pkgfields); } } -# sort by original order (the last field) and print -foreach $_ ( sort{ @{$allpkgs{$a}}[6] <=> @{$allpkgs{$b}}[6] } keys %allpkgs) { - my @v = @{$allpkgs{$_}}; - my $line = to_color(@v); - # print colorized "repo/pkgname pkgver ..." string with possible installed text - print "$line\n"; - print "$v[5]\n"; +foreach (@pkglist) { + print_pkg (@{$_}); } #vim: set noet: -- 1.8.5.4
We include the leading space in the match for 'group' and 'installed'. This allows us to remove the conditions when printing. Signed-off-by: Pierre Neidhardt <ambrevar@gmail.com> --- contrib/pacsearch.in | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/contrib/pacsearch.in b/contrib/pacsearch.in index 91cf364..0ab840e 100644 --- a/contrib/pacsearch.in +++ b/contrib/pacsearch.in @@ -91,11 +91,8 @@ sub print_pkg { } else { print "$MAGENTA"; } - print "$v[0]/$RESET$BOLD$v[1] $GREEN$v[2]"; - print " $BLUE$v[3]" if $v[3] ne ""; - print " $CYAN$v[4]" if $v[4] ne ""; - print " $RESET\n"; - print " $v[5]\n"; + print "$v[0]/$RESET$BOLD$v[1] $GREEN$v[2]$BLUE$v[3]$CYAN$v[4]$RESET\n"; + print "$v[5]\n"; } my %allpkgs = (); @@ -110,8 +107,10 @@ if ($#syncpkgs >= 0) { } foreach $_ (@syncpkgs) { - # we grab the following fields: repo, name, ver, group, installed, and desc - my @pkgfields = /^(.*?)\/(.*?) (.*?) ?(\(.*?\))? ?(\[.*\])?\n(.*)$/s; + # 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 = /^(.*?)\/(.*?) (.*?)( \(.*?\))?( \[.*\])?\n(.*)$/s; if(not @pkgfields) { # skip any non-matching line and just print it for the user print $_, "\n"; @@ -135,15 +134,16 @@ 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 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[4] = " [$LC_INSTALLED]"; push (@pkglist, \@pkgfields); } } -- 1.8.5.4
If input cannot be parsed, it is not desirable to output it along other valid entries. This would make pacsearch output unpredictable. Signed-off-by: Pierre Neidhardt <ambrevar@gmail.com> --- contrib/pacsearch.in | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/contrib/pacsearch.in b/contrib/pacsearch.in index 0ab840e..df0d62d 100644 --- a/contrib/pacsearch.in +++ b/contrib/pacsearch.in @@ -111,11 +111,8 @@ foreach $_ (@syncpkgs) { # desc. We grab leading space for 'group' and 'installed' so that we do not # need to test if non-empty when printing. my @pkgfields = /^(.*?)\/(.*?) (.*?)( \(.*?\))?( \[.*\])?\n(.*)$/s; - if(not @pkgfields) { - # skip any non-matching line and just print it for the user - print $_, "\n"; - next; - } + # 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]; -- 1.8.5.4
On 02/09/14 at 07:41pm, Pierre Neidhardt wrote:
If input cannot be parsed, it is not desirable to output it along other valid entries. This would make pacsearch output unpredictable.
Signed-off-by: Pierre Neidhardt <ambrevar@gmail.com> --- contrib/pacsearch.in | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
-1. pacsearch is a thin pacman wrapper and should not hide output. Since it doesn't do any real argument parsing, a user can pass an option that alters pacman's output (-q for instance) and this would hide all output from the user without giving any indication of an error.
diff --git a/contrib/pacsearch.in b/contrib/pacsearch.in index 0ab840e..df0d62d 100644 --- a/contrib/pacsearch.in +++ b/contrib/pacsearch.in @@ -111,11 +111,8 @@ foreach $_ (@syncpkgs) { # desc. We grab leading space for 'group' and 'installed' so that we do not # need to test if non-empty when printing. my @pkgfields = /^(.*?)\/(.*?) (.*?)( \(.*?\))?( \[.*\])?\n(.*)$/s; - if(not @pkgfields) { - # skip any non-matching line and just print it for the user - print $_, "\n"; - next; - } + # 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]; -- 1.8.5.4
On 14-02-10 11:06:51, Andrew Gregory wrote:
On 02/09/14 at 07:41pm, Pierre Neidhardt wrote:
If input cannot be parsed, it is not desirable to output it along other valid entries. This would make pacsearch output unpredictable.
Signed-off-by: Pierre Neidhardt <ambrevar@gmail.com> --- contrib/pacsearch.in | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
-1. pacsearch is a thin pacman wrapper and should not hide output. Since it doesn't do any real argument parsing, a user can pass an option that alters pacman's output (-q for instance) and this would hide all output from the user without giving any indication of an error.
Then I suggest to print a message to stderr instead, since outputing unparseable lines among the regular output can introdcue subtle bugs in programs using pacsearch output. -- Pierre Neidhardt "Right now I feel that I've got my feet on the ground as far as my head is concerned." -- Baseball pitcher Bo Belinsky
On 11/02/14 02:23, Pierre Neidhardt wrote:
On 14-02-10 11:06:51, Andrew Gregory wrote:
On 02/09/14 at 07:41pm, Pierre Neidhardt wrote:
If input cannot be parsed, it is not desirable to output it along other valid entries. This would make pacsearch output unpredictable.
Signed-off-by: Pierre Neidhardt <ambrevar@gmail.com> --- contrib/pacsearch.in | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
-1. pacsearch is a thin pacman wrapper and should not hide output. Since it doesn't do any real argument parsing, a user can pass an option that alters pacman's output (-q for instance) and this would hide all output from the user without giving any indication of an error.
Then I suggest to print a message to stderr instead, since outputing unparseable lines among the regular output can introdcue subtle bugs in programs using pacsearch output.
Do you have an example with unparsable output? A
On 14-02-11 10:12:04, Allan McRae wrote:
On 11/02/14 02:23, Pierre Neidhardt wrote:
On 14-02-10 11:06:51, Andrew Gregory wrote:
On 02/09/14 at 07:41pm, Pierre Neidhardt wrote:
If input cannot be parsed, it is not desirable to output it along other valid entries. This would make pacsearch output unpredictable.
Signed-off-by: Pierre Neidhardt <ambrevar@gmail.com> --- contrib/pacsearch.in | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
-1. pacsearch is a thin pacman wrapper and should not hide output. Since it doesn't do any real argument parsing, a user can pass an option that alters pacman's output (-q for instance) and this would hide all output from the user without giving any indication of an error.
Then I suggest to print a message to stderr instead, since outputing unparseable lines among the regular output can introdcue subtle bugs in programs using pacsearch output.
Do you have an example with unparsable output?
Yes, as aforementioned by Andrew: pacsearch -q pacman Here -q is seen as an option, not a package. Same thing with -v, and so on. Otherwise, we could just add '--' before 'ARGV', this would prevent this issue and then 'unparseable output' would never happen. -- Pierre Neidhardt What good is it if you talk in flowers, and they think in pastry? -- Ashleigh Brilliant
On 11/02/14 10:23, Pierre Neidhardt wrote:
On 14-02-11 10:12:04, Allan McRae wrote:
On 11/02/14 02:23, Pierre Neidhardt wrote:
On 14-02-10 11:06:51, Andrew Gregory wrote:
On 02/09/14 at 07:41pm, Pierre Neidhardt wrote:
If input cannot be parsed, it is not desirable to output it along other valid entries. This would make pacsearch output unpredictable.
Signed-off-by: Pierre Neidhardt <ambrevar@gmail.com> --- contrib/pacsearch.in | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
-1. pacsearch is a thin pacman wrapper and should not hide output. Since it doesn't do any real argument parsing, a user can pass an option that alters pacman's output (-q for instance) and this would hide all output from the user without giving any indication of an error.
Then I suggest to print a message to stderr instead, since outputing unparseable lines among the regular output can introdcue subtle bugs in programs using pacsearch output.
Do you have an example with unparsable output?
Yes, as aforementioned by Andrew:
pacsearch -q pacman
Here -q is seen as an option, not a package. Same thing with -v, and so on.
Otherwise, we could just add '--' before 'ARGV', this would prevent this issue and then 'unparseable output' would never happen.
That does not seem a bad option. A
Previously only one pattern was allowed. $ pacsearch foo bar Search for packages containing 'foo bar'. $ pacman -Ss foo bar Search for packages containing both 'foo' and 'bar'. Note that removing the quotes from the call was not enough since $ pacsearch 'foo|bar' would then fail. Signed-off-by: Pierre Neidhardt <ambrevar@gmail.com> --- contrib/pacsearch.in | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/contrib/pacsearch.in b/contrib/pacsearch.in index df0d62d..0dad366 100644 --- a/contrib/pacsearch.in +++ b/contrib/pacsearch.in @@ -92,48 +92,38 @@ sub print_pkg { print "$MAGENTA"; } print "$v[0]/$RESET$BOLD$v[1] $GREEN$v[2]$BLUE$v[3]$CYAN$v[4]$RESET\n"; - print "$v[5]\n"; + print "$v[5]"; } my %allpkgs = (); my @pkglist = (); -my $syncout = `pacman -Ss '@ARGV'`; -# split each sync search entry into its own array entry -my @syncpkgs = split(/\n^(?=\w)/m, $syncout); -# remove the extra \n from the last desc entry -if ($#syncpkgs >= 0) { - chomp($syncpkgs[$#syncpkgs]); -} - -foreach $_ (@syncpkgs) { +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 = /^(.*?)\/(.*?) (.*?)( \(.*?\))?( \[.*\])?\n(.*)$/s; + 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); -my $queryout = `pacman -Qs '@ARGV'`; -# split each querysearch entry into its own array entry -my @querypkgs = split(/\n^(?=\w)/m, $queryout); -# remove the extra \n from the last desc entry -if ($#querypkgs >= 0) { - chomp ($querypkgs[$#querypkgs]); -} - -foreach $_ (@querypkgs) { +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 = /^(.*?)\/(.*?) (.*?)( \(.*?\))?( \[.*\])?\n(.*)$/s; + 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 @@ -141,9 +131,11 @@ foreach $_ (@querypkgs) { # 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); } } +close ($queryout); foreach (@pkglist) { print_pkg (@{$_}); -- 1.8.5.4
On 02/09/14 at 07:41pm, Pierre Neidhardt wrote:
Previously only one pattern was allowed.
$ pacsearch foo bar Search for packages containing 'foo bar'.
$ pacman -Ss foo bar Search for packages containing both 'foo' and 'bar'.
Note that removing the quotes from the call was not enough since $ pacsearch 'foo|bar' would then fail.
Signed-off-by: Pierre Neidhardt <ambrevar@gmail.com> --- contrib/pacsearch.in | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/contrib/pacsearch.in b/contrib/pacsearch.in index df0d62d..0dad366 100644 --- a/contrib/pacsearch.in +++ b/contrib/pacsearch.in @@ -92,48 +92,38 @@ sub print_pkg { print "$MAGENTA"; } print "$v[0]/$RESET$BOLD$v[1] $GREEN$v[2]$BLUE$v[3]$CYAN$v[4]$RESET\n"; - print "$v[5]\n"; + print "$v[5]"; }
my %allpkgs = (); my @pkglist = ();
-my $syncout = `pacman -Ss '@ARGV'`; -# split each sync search entry into its own array entry -my @syncpkgs = split(/\n^(?=\w)/m, $syncout); -# remove the extra \n from the last desc entry -if ($#syncpkgs >= 0) { - chomp($syncpkgs[$#syncpkgs]); -} - -foreach $_ (@syncpkgs) { +open (my $syncout, '-|', 'pacman', '-Ss', @ARGV) or exit;
If we can't run pacman we need to print an error message and exit non-zero.
+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 = /^(.*?)\/(.*?) (.*?)( \(.*?\))?( \[.*\])?\n(.*)$/s; + my @pkgfields = /^(.*?)\/(.*?) (.*?)( \(.*?\))?( \[.*\])?$/s; + my $desc = readline($syncout);
This still needs to be moved after the match check. If we can't parse the previous line we have no way of knowing that the next one is actually a description.
# 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);
-my $queryout = `pacman -Qs '@ARGV'`; -# split each querysearch entry into its own array entry -my @querypkgs = split(/\n^(?=\w)/m, $queryout); -# remove the extra \n from the last desc entry -if ($#querypkgs >= 0) { - chomp ($querypkgs[$#querypkgs]); -} - -foreach $_ (@querypkgs) { +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 = /^(.*?)\/(.*?) (.*?)( \(.*?\))?( \[.*\])?\n(.*)$/s; + 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 @@ -141,9 +131,11 @@ foreach $_ (@querypkgs) { # 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); } } +close ($queryout);
foreach (@pkglist) { print_pkg (@{$_}); -- 1.8.5.4
-- apg
On 14-02-10 11:09:25, Andrew Gregory wrote:
On 02/09/14 at 07:41pm, Pierre Neidhardt wrote:
Previously only one pattern was allowed.
$ pacsearch foo bar Search for packages containing 'foo bar'.
$ pacman -Ss foo bar Search for packages containing both 'foo' and 'bar'.
Note that removing the quotes from the call was not enough since $ pacsearch 'foo|bar' would then fail.
Signed-off-by: Pierre Neidhardt <ambrevar@gmail.com> --- contrib/pacsearch.in | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/contrib/pacsearch.in b/contrib/pacsearch.in index df0d62d..0dad366 100644 --- a/contrib/pacsearch.in +++ b/contrib/pacsearch.in @@ -92,48 +92,38 @@ sub print_pkg { print "$MAGENTA"; } print "$v[0]/$RESET$BOLD$v[1] $GREEN$v[2]$BLUE$v[3]$CYAN$v[4]$RESET\n"; - print "$v[5]\n"; + print "$v[5]"; }
my %allpkgs = (); my @pkglist = ();
-my $syncout = `pacman -Ss '@ARGV'`; -# split each sync search entry into its own array entry -my @syncpkgs = split(/\n^(?=\w)/m, $syncout); -# remove the extra \n from the last desc entry -if ($#syncpkgs >= 0) { - chomp($syncpkgs[$#syncpkgs]); -} - -foreach $_ (@syncpkgs) { +open (my $syncout, '-|', 'pacman', '-Ss', @ARGV) or exit;
If we can't run pacman we need to print an error message and exit non-zero.
'open' already prints an error message. Writing +open (my $syncout, '-|', 'pacman', '-Ss', @ARGV) or die "pacman not found: $!"; prints 2 times the same error.
+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 = /^(.*?)\/(.*?) (.*?)( \(.*?\))?( \[.*\])?\n(.*)$/s; + my @pkgfields = /^(.*?)\/(.*?) (.*?)( \(.*?\))?( \[.*\])?$/s; + my $desc = readline($syncout);
This still needs to be moved after the match check. If we can't parse the previous line we have no way of knowing that the next one is actually a description.
Right, I'll fix this. -- Pierre Neidhardt I got this powdered water -- now I don't know what to add. -- Steven Wright
On 02/10/14 at 05:26pm, Pierre Neidhardt wrote:
On 14-02-10 11:09:25, Andrew Gregory wrote:
On 02/09/14 at 07:41pm, Pierre Neidhardt wrote:
Previously only one pattern was allowed.
$ pacsearch foo bar Search for packages containing 'foo bar'.
$ pacman -Ss foo bar Search for packages containing both 'foo' and 'bar'.
Note that removing the quotes from the call was not enough since $ pacsearch 'foo|bar' would then fail.
Signed-off-by: Pierre Neidhardt <ambrevar@gmail.com> --- contrib/pacsearch.in | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/contrib/pacsearch.in b/contrib/pacsearch.in index df0d62d..0dad366 100644 --- a/contrib/pacsearch.in +++ b/contrib/pacsearch.in @@ -92,48 +92,38 @@ sub print_pkg { print "$MAGENTA"; } print "$v[0]/$RESET$BOLD$v[1] $GREEN$v[2]$BLUE$v[3]$CYAN$v[4]$RESET\n"; - print "$v[5]\n"; + print "$v[5]"; }
my %allpkgs = (); my @pkglist = ();
-my $syncout = `pacman -Ss '@ARGV'`; -# split each sync search entry into its own array entry -my @syncpkgs = split(/\n^(?=\w)/m, $syncout); -# remove the extra \n from the last desc entry -if ($#syncpkgs >= 0) { - chomp($syncpkgs[$#syncpkgs]); -} - -foreach $_ (@syncpkgs) { +open (my $syncout, '-|', 'pacman', '-Ss', @ARGV) or exit;
If we can't run pacman we need to print an error message and exit non-zero.
'open' already prints an error message. Writing
So perl prints its own warning on open() failures only for pipes, breaking a common idiom... brilliant. I'm tempted to disable that warning and print our own to get rid of the script name and line number, but given how unlikely it is to happen, it may not be worth it. Any thoughts? We still need to exit non-zero either way.
+open (my $syncout, '-|', 'pacman', '-Ss', @ARGV) or die "pacman not found: $!";
prints 2 times the same error.
+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 = /^(.*?)\/(.*?) (.*?)( \(.*?\))?( \[.*\])?\n(.*)$/s; + my @pkgfields = /^(.*?)\/(.*?) (.*?)( \(.*?\))?( \[.*\])?$/s; + my $desc = readline($syncout);
This still needs to be moved after the match check. If we can't parse the previous line we have no way of knowing that the next one is actually a description.
Right, I'll fix this.
On 14-02-10 12:15:03, Andrew Gregory wrote:
On 02/10/14 at 05:26pm, Pierre Neidhardt wrote:
On 14-02-10 11:09:25, Andrew Gregory wrote:
On 02/09/14 at 07:41pm, Pierre Neidhardt wrote:
+open (my $syncout, '-|', 'pacman', '-Ss', @ARGV) or exit;
If we can't run pacman we need to print an error message and exit non-zero.
'open' already prints an error message. Writing
So perl prints its own warning on open() failures only for pipes, breaking a common idiom... brilliant. I'm tempted to disable that warning and print our own to get rid of the script name and line number, but given how unlikely it is to happen, it may not be worth it. Any thoughts? We still need to exit non-zero either way.
+open (my $syncout, '-|', 'pacman', '-Ss', @ARGV) or die "pacman not found: $!";
I was quite surprised as well, considering that the above line comes directly from the official documentation... How do you disable error output from open? -- Pierre Neidhardt Max told his friend that he'd just as soon not go hiking in the hills. Said he, "I'm an anti-climb Max." [So is that punchline.]
On 02/10/14 at 06:27pm, Pierre Neidhardt wrote:
On 14-02-10 12:15:03, Andrew Gregory wrote:
On 02/10/14 at 05:26pm, Pierre Neidhardt wrote:
On 14-02-10 11:09:25, Andrew Gregory wrote:
On 02/09/14 at 07:41pm, Pierre Neidhardt wrote:
+open (my $syncout, '-|', 'pacman', '-Ss', @ARGV) or exit;
If we can't run pacman we need to print an error message and exit non-zero.
'open' already prints an error message. Writing
So perl prints its own warning on open() failures only for pipes, breaking a common idiom... brilliant. I'm tempted to disable that warning and print our own to get rid of the script name and line number, but given how unlikely it is to happen, it may not be worth it. Any thoughts? We still need to exit non-zero either way.
+open (my $syncout, '-|', 'pacman', '-Ss', @ARGV) or die "pacman not found: $!";
I was quite surprised as well, considering that the above line comes directly from the official documentation...
How do you disable error output from open?
"no warnings 'io';" should do it. See perldoc perllexwarn
On 14-02-10 12:35:39, Andrew Gregory wrote:
On 02/10/14 at 06:27pm, Pierre Neidhardt wrote:
On 14-02-10 12:15:03, Andrew Gregory wrote:
On 02/10/14 at 05:26pm, Pierre Neidhardt wrote:
On 14-02-10 11:09:25, Andrew Gregory wrote:
On 02/09/14 at 07:41pm, Pierre Neidhardt wrote:
+open (my $syncout, '-|', 'pacman', '-Ss', @ARGV) or exit;
If we can't run pacman we need to print an error message and exit non-zero.
'open' already prints an error message. Writing
So perl prints its own warning on open() failures only for pipes, breaking a common idiom... brilliant. I'm tempted to disable that warning and print our own to get rid of the script name and line number, but given how unlikely it is to happen, it may not be worth it. Any thoughts? We still need to exit non-zero either way.
+open (my $syncout, '-|', 'pacman', '-Ss', @ARGV) or die "pacman not found: $!";
I was quite surprised as well, considering that the above line comes directly from the official documentation...
How do you disable error output from open?
"no warnings 'io';" should do it. See perldoc perllexwarn
Alright. We have the choice between * using 'exit' and rely on perl's behaviour printing its own warnings; * disabling warning for the 'open' line only. More reliable, but more code and looks hackish. Not beautiful either way, but this is perl. I suggest we stick to the lightest solution with a simple exit call. -- Pierre Neidhardt He's dead, Jim.
Signed-off-by: Pierre Neidhardt <ambrevar@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}) { + # 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
On 02/09/14 at 07:41pm, Pierre Neidhardt wrote:
Signed-off-by: Pierre Neidhardt <ambrevar@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);
+ # 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
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@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.
participants (3)
-
Allan McRae
-
Andrew Gregory
-
Pierre Neidhardt