[perl #126955] [BUG] more .perl string quoting problems
On Mon, 21 Dec 2015 11:09:40 -0800, zef...@fysh.org wrote: > Elizabeth Mattijsen via RT wrote: > >All of that goodness now in 591783d116a56d4b2c54f . > > You left a stray line in IO::Path.perl, which calls PERLIFY-STR in > sink context. > > >Indeed! Now, if this had been in a Pull Request, I could have just > >clicked on Merge, which would have been less work for me :-) > > But it being a Pull Request would involve me signing over my firstborn > to github. > > >Ever considered getting a commit bit ? > > I think that would be a bad idea, for a number of reasons. I'm suffering > project overload as it is, and haven't used my Perl 5 commit bit in ages. > You also know my overall opinion of Perl 6, and that doesn't make Rakudo > an attractive bugfixing target for me. It seems best that my involvement > remain peripheral. > > -zefram > The stray line was removed with commit 34c4fe977e067b4e4eb02b5d. Test needed.
Re: [perl #126955] [BUG] more .perl string quoting problems
Elizabeth Mattijsen via RT wrote: >All of that goodness now in 591783d116a56d4b2c54f . You left a stray line in IO::Path.perl, which calls PERLIFY-STR in sink context. >Indeed! Now, if this had been in a Pull Request, I could have just >clicked on Merge, which would have been less work for me :-) But it being a Pull Request would involve me signing over my firstborn to github. >Ever considered getting a commit bit ? I think that would be a bad idea, for a number of reasons. I'm suffering project overload as it is, and haven't used my Perl 5 commit bit in ages. You also know my overall opinion of Perl 6, and that doesn't make Rakudo an attractive bugfixing target for me. It seems best that my involvement remain peripheral. -zefram
Re: [perl #126955] [BUG] more .perl string quoting problems
> On 21 Dec 2015, at 18:06, Zefram wrote:
> Elizabeth Mattijsen via RT wrote:
>> There is a subtle difference between Str.perl and
>> Rakudo::Internals.PERLIFY-STR: The former puts double quotes around, the
>> latter doesn't.
>
> Yes, hence the doubling when you call the former and also wrap the result
> in quotes.
>
>> I'm afraid we'll have to settle for poor factoring.
>
> What? You need to call .perl and *not* put an extra layer of quotes
> around the result. What's the difficulty? You managed to do it for
> $!SPEC in IO::Path.perl.
>
>multi method perl(CompUnit::Repository::Locally:D:) {
> "{$?CLASS.^name}.new({$!prefix.abspath.perl})"
>}
>
>multi method perl(IO::Local:D:) { "{$!abspath.perl}.IO" }
>
>multi method perl(IO::Path:D:) {
> $!is-absolute
> ?? "{$.abspath.perl}.IO(:SPEC({$!SPEC.perl}))"
> !! "{$.path.perl}.IO(:SPEC({$!SPEC.perl}),:CWD({$!CWD.perl}))"
>}
>
> Further factoring win is available by taking advanage of Pair.perl.
> Aside from just being less code, it also gets you potential future
> wins if Pair.perl later gets clever enough to use :foo syntax.
> I showed how to factor this in a previous message on [perl #126935]:
>
>multi method perl(IO::Path:D:) {
> $!is-absolute
> ?? "{$.abspath.perl}.IO({:$!SPEC.perl})"
> !! "{$.path.perl}.IO({:$!SPEC.perl},{:$!CWD.perl})"
>}
>
> The same should really be done with $?CLASS, in case class deparsing
> gets cleverer about name scoping in the future:
>
>multi method perl(CompUnit::Repository::Locally:D:) {
> "{$?CLASS.perl}.new({$!prefix.abspath.perl})"
>}
All of that goodness now in 591783d116a56d4b2c54f .
> hen writing a pervasive value deparsing system, such as .perl is intended
> to be, a great convenience is that one of the tools available to you is
> a pervasive value deparsing system (for all types other than whichever
> one you're working on right now). So you don't have to do the whole
> deparsing job yourself, and in fact you get better results by doing less
> of the work yourself.
Indeed! Now, if this had been in a Pull Request, I could have just clicked on
Merge, which would have been less work for me :-)
Ever considered getting a commit bit ?
Liz
Re: [perl #126955] [BUG] more .perl string quoting problems
Elizabeth Mattijsen via RT wrote:
>There is a subtle difference between Str.perl and
>Rakudo::Internals.PERLIFY-STR: The former puts double quotes around, the
>latter doesn't.
Yes, hence the doubling when you call the former and also wrap the result
in quotes.
>I'm afraid we'll have to settle for poor factoring.
What? You need to call .perl and *not* put an extra layer of quotes
around the result. What's the difficulty? You managed to do it for
$!SPEC in IO::Path.perl.
multi method perl(CompUnit::Repository::Locally:D:) {
"{$?CLASS.^name}.new({$!prefix.abspath.perl})"
}
multi method perl(IO::Local:D:) { "{$!abspath.perl}.IO" }
multi method perl(IO::Path:D:) {
$!is-absolute
?? "{$.abspath.perl}.IO(:SPEC({$!SPEC.perl}))"
!! "{$.path.perl}.IO(:SPEC({$!SPEC.perl}),:CWD({$!CWD.perl}))"
}
Further factoring win is available by taking advanage of Pair.perl.
Aside from just being less code, it also gets you potential future
wins if Pair.perl later gets clever enough to use :foo syntax.
I showed how to factor this in a previous message on [perl #126935]:
multi method perl(IO::Path:D:) {
$!is-absolute
?? "{$.abspath.perl}.IO({:$!SPEC.perl})"
!! "{$.path.perl}.IO({:$!SPEC.perl},{:$!CWD.perl})"
}
The same should really be done with $?CLASS, in case class deparsing
gets cleverer about name scoping in the future:
multi method perl(CompUnit::Repository::Locally:D:) {
"{$?CLASS.perl}.new({$!prefix.abspath.perl})"
}
When writing a pervasive value deparsing system, such as .perl is intended
to be, a great convenience is that one of the tools available to you is
a pervasive value deparsing system (for all types other than whichever
one you're working on right now). So you don't have to do the whole
deparsing job yourself, and in fact you get better results by doing less
of the work yourself.
-zefram
Re: [perl #126955] [BUG] more .perl string quoting problems
> On 21 Dec 2015, at 16:58, Zefram wrote: > > Elizabeth Mattijsen via RT wrote: >> Good point. Followed your suggestion in 8ddfff5533d4d77dbd7da65 . > > You're now duplicating the delimiters for those two. For IO::Path you've > removed the superfluous pipe escaping but retain the poor factoring. There is a subtle difference between Str.perl and Rakudo::Internals.PERLIFY-STR: The former puts double quotes around, the latter doesn’t. I’m afraid we’ll have to settle for poor factoring. Or if you have a better solution, a PR is very much welcome! Liz
Re: [perl #126955] [BUG] more .perl string quoting problems
Elizabeth Mattijsen via RT wrote: >Good point. Followed your suggestion in 8ddfff5533d4d77dbd7da65 . You're now duplicating the delimiters for those two. For IO::Path you've removed the superfluous pipe escaping but retain the poor factoring. -zefram
Re: [perl #126955] [BUG] more .perl string quoting problems
> On 20 Dec 2015, at 23:18, Zefram wrote: > > Elizabeth Mattijsen via RT wrote: >> Fixed with ae8d9809c432f071643384 > > It's good that you're now factoring out the escaping code. This means > that these methods (including for IO::Path) are now correct. But the > factoring is still flawed, in that you've inlined what remains of > the body of Str.perl into each of these .perl methods. This inlining > doesn't achieve anything; it would be much more sensible for them to > call .perl on their Str values. Good point. Followed your suggestion in 8ddfff5533d4d77dbd7da65 . > In the case of IO::Path you also have > some gratuitous escaping of pipe characters, a relic of former versions > of the code that used that as the delimiter. Another good point, fixed in 8245d9072177f26251f128 >> Zefram: I assume you checked all .perl methods in core for this, or >> should I look at them some more? > I looked systematically through rakudo/src, and the ones I mentioned > are all that I found. Thank you! Liz
Re: [perl #126955] [BUG] more .perl string quoting problems
Elizabeth Mattijsen via RT wrote: >Fixed with ae8d9809c432f071643384 It's good that you're now factoring out the escaping code. This means that these methods (including for IO::Path) are now correct. But the factoring is still flawed, in that you've inlined what remains of the body of Str.perl into each of these .perl methods. This inlining doesn't achieve anything; it would be much more sensible for them to call .perl on their Str values. In the case of IO::Path you also have some gratuitous escaping of pipe characters, a relic of former versions of the code that used that as the delimiter. >Zefram: I assume you checked all .perl methods in core for this, or >should I look at them some more? I looked systematically through rakudo/src, and the ones I mentioned are all that I found. -zefram
Re: [perl #126955] [BUG] more .perl string quoting problems
> On 18 Dec 2015, at 08:21, Zefram (via RT) > wrote: > > # New Ticket Created by Zefram > # Please include the string: [perl #126955] > # in the subject line of all future correspondence about this issue. > # https://rt.perl.org/Ticket/Display.html?id=126955 > > > > The roles CompUnit::Repository::Locally and IO::Local each have a bug > similar to [perl #126935], where a .perl method rolls its own string > quoting to inadequate effect. (Version also rolls its own string quoting, > but the string content is sufficiently restricted that it works.) In each > case, delegating to .perl on the Str value would produce a correct result. Fixed with ae8d9809c432f071643384 , tests still needed. Zefram: I assume you checked all .perl methods in core for this, or should I look at them some more? Liz
[perl #126955] [BUG] more .perl string quoting problems
# New Ticket Created by Zefram # Please include the string: [perl #126955] # in the subject line of all future correspondence about this issue. # https://rt.perl.org/Ticket/Display.html?id=126955 > The roles CompUnit::Repository::Locally and IO::Local each have a bug similar to [perl #126935], where a .perl method rolls its own string quoting to inadequate effect. (Version also rolls its own string quoting, but the string content is sufficiently restricted that it works.) In each case, delegating to .perl on the Str value would produce a correct result. -zefram
