From: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Date: Wed, 23 Oct 2024 12:34:13 GMT
Subject: Re: [PATCH v2 12/12] gitweb: make use of s///r
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.

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