Skip to content
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

PrettyPrinter:Unnecessary escape in single quoted. #266

Closed
wants to merge 4 commits into from

Conversation

algo13
Copy link
Contributor

@algo13 algo13 commented Apr 13, 2016

>php-parse --pretty-print "<?php echo 'path\to';
====> Code <?php echo 'path\to';
==> Pretty print:
<?php

echo 'path\\to';

@nikic
Copy link
Owner

nikic commented Apr 13, 2016

Escaping \ is not unnecessary if you consider something like '\\\''. This parses to string contents \', and printing it as '\\'' would not be correct. This is the cause of the build failure @ https://travis-ci.org/nikic/PHP-Parser/jobs/122780318.

This reverts commit 99bad7e.
@algo13
Copy link
Contributor Author

algo13 commented Apr 13, 2016

sorry.
It was my mistake.

print \0 ?

>php-parse --pretty-print "<?php echo trim('str', \"\n\");
#!/usr/bin/env php
====> Code <?php echo trim('str', "\n");
==> Pretty print:
<?php

echo trim('str', "\n");
>php-parse --pretty-print "<?php echo trim('str', \"\0\");
====> Code <?php echo trim('str', "\0");
==> Pretty print:
<?php

echo trim('str', " "); //< real \0

@nikic
Copy link
Owner

nikic commented Apr 19, 2016

Escaping \0 sounds like a good idea. However we should probably escape the entire \0-\37 control character range (excluding the existing \n\r\t\f\v) and not just \0.

@nikic
Copy link
Owner

nikic commented Apr 19, 2016

Merged as 371c783, extended to the whole control character range.

@nikic nikic closed this Apr 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants