New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
more .perl string quoting problems #4887
Comments
From zefram@fysh.orgThe roles CompUnit::Repository::Locally and IO::Local each have a bug -zefram |
From @lizmat
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 |
The RT System itself - Status changed from 'new' to 'open' |
From zefram@fysh.orgElizabeth Mattijsen via RT wrote:
It's good that you're now factoring out the escaping code. This means
I looked systematically through rakudo/src, and the ones I mentioned -zefram |
From @lizmat
Good point. Followed your suggestion in 8ddfff5533d4d77dbd7da65 .
Another good point, fixed in 8245d9072177f26251f128
Thank you! Liz |
From zefram@fysh.orgElizabeth Mattijsen via RT wrote:
You're now duplicating the delimiters for those two. For IO::Path you've -zefram |
From @lizmat
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 |
From zefram@fysh.orgElizabeth Mattijsen via RT wrote:
Yes, hence the doubling when you call the former and also wrap the result
What? You need to call .perl and *not* put an extra layer of quotes multi method perl(CompUnit::Repository::Locally:D:) { multi method perl(IO::Local:D:) { "{$!abspath.perl}.IO" } multi method perl(IO::Path:D:) { Further factoring win is available by taking advanage of Pair.perl. multi method perl(IO::Path:D:) { The same should really be done with $?CLASS, in case class deparsing multi method perl(CompUnit::Repository::Locally:D:) { When writing a pervasive value deparsing system, such as .perl is intended -zefram |
From @lizmat
All of that goodness now in 591783d116a56d4b2c54f .
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 |
From zefram@fysh.orgElizabeth Mattijsen via RT wrote:
You left a stray line in IO::Path.perl, which calls PERLIFY-STR in
But it being a Pull Request would involve me signing over my firstborn
I think that would be a bad idea, for a number of reasons. I'm suffering -zefram |
From @dogbert17On Mon, 21 Dec 2015 11:09:40 -0800, zefram@fysh.org wrote:
The stray line was removed with commit 34c4fe977e067b4e4eb02b5d. Test needed. |
Migrated from rt.perl.org#126955 (status was 'open')
Searchable as RT126955$
The text was updated successfully, but these errors were encountered: