Bug 12151 - remove remaining uses of the Perl smartmatch operator
Summary: remove remaining uses of the Perl smartmatch operator
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on: 12301 12338
Blocks: 12593 15250
  Show dependency treegraph
 
Reported: 2014-04-28 15:21 UTC by Galen Charlton
Modified: 2016-12-05 21:23 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 12151: Remove uses of smartmatch operator in Search.pm and opac-search.pl (2.43 KB, patch)
2014-05-31 03:35 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 12151: Remove uses of smartmatch operator in report scripts (8.89 KB, patch)
2014-05-31 03:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 12151: Remove uses of smartmatch operator from Koha/Solr/QueryBuilder.pm (1.47 KB, patch)
2014-06-02 14:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 12151: Remove use of smartmatch operator in tools/batchMod.pl (1.17 KB, patch)
2014-06-02 14:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 12151: Remove uses of smartmatch operator in Search.pm and opac-search.pl (2.17 KB, patch)
2014-06-19 13:07 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 12151: Remove uses of smartmatch operator in report scripts (8.89 KB, patch)
2014-06-19 13:07 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 12151: Remove uses of smartmatch operator from Koha/Solr/QueryBuilder.pm (1.52 KB, patch)
2014-06-19 13:07 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 12151: Remove use of smartmatch operator in tools/batchMod.pl (1.22 KB, patch)
2014-06-19 13:07 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12151: Remove uses of smartmatch operator in Search.pm and opac-search.pl (2.24 KB, patch)
2014-07-03 14:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12151: Remove uses of smartmatch operator in report scripts (8.96 KB, patch)
2014-07-03 14:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12151: Remove uses of smartmatch operator from Koha/Solr/QueryBuilder.pm (1.59 KB, patch)
2014-07-03 14:51 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12151: Remove use of smartmatch operator in tools/batchMod.pl (1.29 KB, patch)
2014-07-03 14:51 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 12151: Remove uses of smartmatch operator in Search.pm and opac-search.pl (2.36 KB, patch)
2014-07-06 17:39 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12151: Remove uses of smartmatch operator in report scripts (9.11 KB, patch)
2014-07-06 17:39 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12151: Remove uses of smartmatch operator from Koha/Solr/QueryBuilder.pm (1.69 KB, patch)
2014-07-06 17:39 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12151: Remove use of smartmatch operator in tools/batchMod.pl (1.42 KB, patch)
2014-07-06 17:39 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12151: Remove uses of smartmatch operator from Koha/Solr/QueryBuilder.pm (1.70 KB, patch)
2014-07-06 17:41 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Galen Charlton 2014-04-28 15:21:42 UTC
The Perl smartmatch operator has been marked as experimental as of Perl 5.18.  As the syntax and interpretation of it is therefore subject to change, current uses of it should be removed.  This is especially the case when running under Perl 5.18, as by default use of experimental constructs generates warning messages.

The following source files contain the ~~ operator:

C4/Search.pm:                                    unless ( $data ~~ @used_datas ) {
misc/translator/translate:    if ( $cmd !~ /create/ && $lang && not $lang ~~ $installer->{langs} ) {
opac/opac-search.pl:    if ( $sort ~~ @allowed_sortby ) {
tools/batchMod.pl:                unless ( $field_to_update->subfield($subfield->[0]) ~~ $subfield->[1] ) {

The following source files contain the given construct, which also uses smartmatch:

C4/Serials.pm:    given ($num_type) {
reports/acquisitions_stats.pl:        given ($_) {
reports/acquisitions_stats.pl:                given ($podsp) {
reports/acquisitions_stats.pl:                given ($rodsp) {
reports/acquisitions_stats.pl:    given ($process) {
reports/borrowers_stats.pl:    given ($line) {
reports/borrowers_stats.pl:    given ($column) {
reports/borrowers_stats.pl:            given ($i)
Comment 1 Chris Cormack 2014-05-06 04:54:54 UTC
unless ( $data ~~ @used_datas ) {

Should be the same as 

unless ( grep ( $_ eq $data) @used_datas ) {

(I think, just noting it down for when I get the chance to try it)
Comment 2 Chris Cormack 2014-05-06 20:43:21 UTC
(In reply to Chris Cormack from comment #1)
> unless ( $data ~~ @used_datas ) {
> 
> Should be the same as 
> 
> unless ( grep ( $_ eq $data) @used_datas ) {
> 
> (I think, just noting it down for when I get the chance to try it)

It should be 
unless ( grep { $_ eq $data } @used_datas ) {
Comment 3 Tomás Cohen Arazi 2014-05-31 03:35:57 UTC Comment hidden (obsolete)
Comment 4 Tomás Cohen Arazi 2014-05-31 03:37:02 UTC Comment hidden (obsolete)
Comment 5 Tomás Cohen Arazi 2014-06-02 14:37:12 UTC Comment hidden (obsolete)
Comment 6 Tomás Cohen Arazi 2014-06-02 14:37:17 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2014-06-11 11:06:54 UTC
Comment on attachment 28580 [details] [review]
Bug 12151: Remove uses of smartmatch operator in Search.pm and opac-search.pl

Review of attachment 28580 [details] [review]:
-----------------------------------------------------------------

::: C4/Search.pm
@@ +533,4 @@
>                                  foreach my $field (@fields) {
>                                      my $data = $field->as_string( $subfield_letters, $facet->{sep} );
>  
> +                                    unless ( any { $data eq $_ } @used_datas ) {

Why don't you use grep?
  unless( grep /^$date$/, @used_datas )

Should do the same without the List::MoreUtils deps.

::: opac/opac-search.pl
@@ +358,4 @@
>  @sort_by = $cgi->param('sort_by');
>  $sort_by[0] = $default_sort_by if !$sort_by[0] && defined($default_sort_by);
>  foreach my $sort (@sort_by) {
> +    if ( any { $sort eq $_ } @allowed_sortby ) {

Same here.
Comment 8 Tomás Cohen Arazi 2014-06-11 12:22:10 UTC
(In reply to Jonathan Druart from comment #7)
> Comment on attachment 28580 [details] [review] [review]
> Bug 12151: Remove uses of smartmatch operator in Search.pm and opac-search.pl
> 
> Review of attachment 28580 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: C4/Search.pm
> @@ +533,4 @@
> >                                  foreach my $field (@fields) {
> >                                      my $data = $field->as_string( $subfield_letters, $facet->{sep} );
> >  
> > +                                    unless ( any { $data eq $_ } @used_datas ) {
> 
> Why don't you use grep?
>   unless( grep /^$date$/, @used_datas )
> 
> Should do the same without the List::MoreUtils deps.

I was more used to the Lists::Moreutils which don't have much footprint, and was already used all over the project.

Feel free to change it.
Comment 9 Tomás Cohen Arazi 2014-06-19 13:07:35 UTC Comment hidden (obsolete)
Comment 10 Tomás Cohen Arazi 2014-06-19 13:07:40 UTC Comment hidden (obsolete)
Comment 11 Tomás Cohen Arazi 2014-06-19 13:07:44 UTC Comment hidden (obsolete)
Comment 12 Tomás Cohen Arazi 2014-06-19 13:07:50 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall 2014-07-03 14:50:11 UTC Comment hidden (obsolete)
Comment 14 Kyle M Hall 2014-07-03 14:50:36 UTC Comment hidden (obsolete)
Comment 15 Kyle M Hall 2014-07-03 14:51:51 UTC Comment hidden (obsolete)
Comment 16 Kyle M Hall 2014-07-03 14:51:54 UTC Comment hidden (obsolete)
Comment 17 Katrin Fischer 2014-07-06 17:39:14 UTC
Created attachment 29519 [details] [review]
[PASSED QA] Bug 12151: Remove uses of smartmatch operator in Search.pm and opac-search.pl

This patch removes the use of smartmatch operators in the search code.

Regards
To+

Edit: this revision uses 'grep' instead of Lists::MoreUtils::any

Sponsored-by: Universidad Nacional de Cordoba

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.
Tested search, no problems found.
Comment 18 Katrin Fischer 2014-07-06 17:39:18 UTC
Created attachment 29520 [details] [review]
[PASSED QA] Bug 12151: Remove uses of smartmatch operator in report scripts

This patch removes the use of smartmatch operators in report scripts.

Regards
To+

Sponsored-by: Universidad Nacional de Cordoba

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes QA script and tests.
Acquisition and Patron statistics wizard tested, no regressions found.
Comment 19 Katrin Fischer 2014-07-06 17:39:21 UTC Comment hidden (obsolete)
Comment 20 Katrin Fischer 2014-07-06 17:39:24 UTC
Created attachment 29522 [details] [review]
[PASSED QA] Bug 12151: Remove use of smartmatch operator in tools/batchMod.pl

The '~~' smartmatch operator is used to compare MARC::Field->subfield(code)
(i.e. a string) and the text element of each MARC::Field->subfields() which
is also plain text.

Substituting '~~' for 'eq' should be harmless then.

Regards
To+

Sponsored-by: Universidad Nacional de Cordoba

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes QA script and tests.
Tested batch modification of items, no problems found.
Comment 21 Katrin Fischer 2014-07-06 17:41:41 UTC
Created attachment 29523 [details] [review]
[PASSED QA] Bug 12151: Remove uses of smartmatch operator from Koha/Solr/QueryBuilder.pm

Just that.

Regards
To+

Sponsored-by: Universidad Nacional de Cordoba

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes QA script and tests.
Could only verify by reading the code.
Comment 22 Tomás Cohen Arazi 2014-07-07 13:19:27 UTC
Patches pushed to master.
Comment 23 Dobrica Pavlinusic 2014-07-17 10:36:37 UTC
Please see Bug 12593 for fix of problem with facets and data with square brackets
Comment 24 Galen Charlton 2014-09-23 20:52:47 UTC
Pushed to 3.16.x for inclusion in 3.16.4.
Comment 25 Fridolin Somers 2015-01-23 12:47:22 UTC
I choose to not push to 3.14.x
I set as resolved