Perl formatter improvement #173
Conversation
| value = value.replace(/\r/g, '\\r'); | ||
| value = value.replace(/\n/g, '\\n'); | ||
| value = value.replace(/@/g, '\\@'); | ||
| value = value.replace(/\$/g, '\\$'); |
There was a problem hiding this comment.
Why is the escaping of @ and $ removed?
|
Generally I think this patch looks fine. Have you signed the CLA? I'd like @samitbadle to have a look. |
|
Thank you for quick reply.
Single quotation doesn't expand variables so you don't need to escape them any more.
It's a matter of taste. And I must say that q{} and q() usage is more debatable. Some people may don't like it.
Yes I did. |
Sorry your code was better and it7s not a matter of taste. |
|
I will give it a run today, I have a new webdriver based Perl formatter as On Wed, Feb 26, 2014 at 5:20 AM, jamadam [email protected] wrote:
|
|
I found a bug in one of the patches and had to put it aside until I got more time. I hope to check it out further tomorrow. Were there any tests for the patches?
|
|
@samitbadle, did you have time to look at the bug you found? |
|
I believe I fixed it, but need to merge the code. I will not have any time before this Friday.
|
|
Awaiting write access |
|
@samitbadle, consider yourself invited! |
| } else { | ||
| return '""'; | ||
| return "'"; |
There was a problem hiding this comment.
It should be return "''"; I will fix it
|
Thanks Andreas, now waiting for the clone, I need faster internet! |
The patch improves Perl code on non-ascii output. It reduces charactor corruption and well known warnings 'Wide character in print'.
It also change the literal delimiter from "" to '' or q{} or q() to reduce variable escaping so that the test cases get more readable.
Since I haven't figure out how to build xpi so far, the patch is not properly tested.