Re: [PATCH v2 12/12] gitweb: make use of s///r

From: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Date: Oct 23, 2024, 12:34 UTC
Message-ID: <ZxjtRXf8IUrvn1tK@ugly>
In-Reply-To: <20241023004600.1645313-13-sandals@crustytoothpaste.net>

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.

Show 5 quoted lines

>+++ 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.