On Tue, Jun 21, 2022 at 03:17:04AM -0400, Jeff King wrote:
> On Mon, Jun 20, 2022 at 06:20:07PM -0400, Derrick Stolee wrote:
>
> > diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> > index 4620f0ca7fa..c255a77e18a 100755
> > --- a/t/t5510-fetch.sh
> > +++ b/t/t5510-fetch.sh
> > @@ -853,7 +853,9 @@ test_configured_prune_type () {
> > then
> > new_cmdline=$cmdline_setup
> > else
> > - new_cmdline=$(printf "%s" "$cmdline" | perl -pe 's[origin(?!/)]["'"$remote_url"'"]g')
> > + new_cmdline=$(printf "%s" "$cmdline" |
> > + sed -e "s~origin ~'$remote_url' ~g" \
> > + -e "s~ origin~ '$remote_url'~g")
> > fi
>
> Doesn't this introduce a new problem if $remote_url contains a tilde?
> Unlikely, but I thought the point of the exercise was defending against
> funny paths.
And it doesn't replace a standalone "origin" word, either, which does
occur a few times in the test script.
> So perhaps something like:
>
> perl -e '
> my ($cmdline, $url) = @ARGV;
> $cmdline =~ s[origin(?!/)][quotemeta($url)]ge;
I don't like this "(?!/)" magic, because I haven't got the slightest
idea of what it might do by merely looking at it, and these characters
are not exactly easy to search for.
The good old "add a space prefix and suffix" trick can help to easily
match the "origin" word even when it stands alone, but, alas, the
result is still not as simple as I'd like with the \s and the string
concatenation:
perl -e '
new_cmdline=$(perl -e '
my ($cmdline, $url) = @ARGV;
$cmdline =~ s[\sorigin\s][" " . quotemeta($url) . " "]ge;
print $cmdline;
' -- " $cmdline " "$remote_url")
(The 'sed' variant looks good to me:
printf " %s " "$cmdline" |sed "s~ origin ~ '$remote_url' ~g"
but then again it just trades one set of troublesome characters for an
other.)
> print $cmdline;
> ' -- "$cmdline" "$remote_url"
>
> I don't mean to golf on this forever, but I wanted to show something
> concrete since you said you don't know perl well. I just think moving to
> sed introduces more opportunities for errors here, not fewer. :)
>
> -Peff
