| Message ID | 20241023004600.1645313-13-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Update versions of libcurl and Perl | expand |
On Wed, Oct 23, 2024 at 12:46:00AM +0000, brian m. carlson wrote: >In Perl 5.14, released in May 2011, the r modifier was added to the s/// >operator to allow it to return the modified string instead of modifying >the string in place. >This allows to write nicer, more succinct code in >several cases, so let's do that here. > "several" is a bit of an overstatement. >+++ b/gitweb/gitweb.perl >@@ -1188,7 +1188,7 @@ sub evaluate_and_validate_params { >- (my $error = $@) =~ s/ at \S+ line \d+.*\n?//; >+ my $error = $@ =~ s/ at \S+ line \d+.*\n?//r; > i'm a fan of "excess" parentheses where the syntax relies heavily on the binding and priority of operators: my $error = ($@ =~ s/ at \S+ line \d+.*\n?//r); which is arguably semantically clearer than the old idiom, but not shorter. >@@ -2700,7 +2700,7 @@ sub git_cmd { >- map { my $a = $_; $a =~ s/(['!])/'\\$1'/g; "'$a'" } @_ ); >+ map { my $a = $_ =~ s/(['!])/'\\$1'/gr; "'$a'" } @_ ); > i think map { "'".(s/(['!])/'\\$1'/gr)."'" } @_ ); should work, and is an actually significant improvement.
On 2024-10-23 at 12:34:13, Oswald Buddenhagen wrote: > On Wed, Oct 23, 2024 at 12:46:00AM +0000, brian m. carlson wrote: > > In Perl 5.14, released in May 2011, the r modifier was added to the s/// > > operator to allow it to return the modified string instead of modifying > > the string in place. > > > This allows to write nicer, more succinct code in > > several cases, so let's do that here. > > > "several" is a bit of an overstatement. I can rephrase if a v3 is necessary. > > +++ b/gitweb/gitweb.perl > > @@ -1188,7 +1188,7 @@ sub evaluate_and_validate_params { > > - (my $error = $@) =~ s/ at \S+ line \d+.*\n?//; > > + my $error = $@ =~ s/ at \S+ line \d+.*\n?//r; > > > i'm a fan of "excess" parentheses where the syntax relies heavily on > the binding and priority of operators: > > my $error = ($@ =~ s/ at \S+ line \d+.*\n?//r); > > which is arguably semantically clearer than the old idiom, but not shorter. I don't think those are necessary. It's obvious to people who use the s///r idiom what's meant here, and in my experience most Perl code using that idiom doesn't use them. > > @@ -2700,7 +2700,7 @@ sub git_cmd { > > - map { my $a = $_; $a =~ s/(['!])/'\\$1'/g; "'$a'" } @_ ); > > + map { my $a = $_ =~ s/(['!])/'\\$1'/gr; "'$a'" } @_ ); > > > i think > > map { "'".(s/(['!])/'\\$1'/gr)."'" } @_ ); > > should work, and is an actually significant improvement. I'm sorry, I don't necessarily like that much better than what we have now. It's not that I think it's awful, just that I don't think it's a significant improvement. If I do a v3, I can omit the `$_ =~`, though.
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index da1486cab2..c4e0008d59 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -1188,7 +1188,7 @@ sub evaluate_and_validate_params { if ($search_use_regexp) { $search_regexp = $searchtext; if (!eval { qr/$search_regexp/; 1; }) { - (my $error = $@) =~ s/ at \S+ line \d+.*\n?//; + my $error = $@ =~ s/ at \S+ line \d+.*\n?//r; die_error(400, "Invalid search regexp '$search_regexp'", esc_html($error)); } @@ -2700,7 +2700,7 @@ sub git_cmd { # Try to avoid using this function wherever possible. sub quote_command { return join(' ', - map { my $a = $_; $a =~ s/(['!])/'\\$1'/g; "'$a'" } @_ ); + map { my $a = $_ =~ s/(['!])/'\\$1'/gr; "'$a'" } @_ ); } # get HEAD ref of given project as hash
In Perl 5.14, released in May 2011, the r modifier was added to the s/// operator to allow it to return the modified string instead of modifying the string in place. This allows to write nicer, more succinct code in several cases, so let's do that here. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- gitweb/gitweb.perl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)