Invalid output caused by combo of if statement, inline node, and comment
Input:
<?php
if (true) {
?>inline<?php
// comment
}
Output:
<?php
if (true) { ?>inline<?php
// comment
// comment
?>}
Note the extra closing tag that has been added, which makes the code invalid, and that the comment is duplicated, which is unexpected.
If you comment out this line, it doesn't print out the closing tag that breaks the code, but it also removes the closing tag from the test file inline3.php.
If you comment out the line above, prettier throws the following error, I think about inline3.php again.
Comment "// test" was not printed. Please report this error!
I've tried to debug, but the printing system is quite large. I'll still keep looking, but might as well put it out to the community too!
Problem aslo with
<!DOCTYPE html>
<html lang="de">
<head>
<?php /* ups */
if (1==1)
{
echo 'FOO';
} ?>
</head>
<body>
</body>
</html>
We're currently changing on the printing of only-comment code blocks by manually adding new AST nodes that represent them in https://github.com/prettier/plugin-php/pull/513. On that branch your example is formatted as
<?php
if (true) { ?>inline<?php }
// comment
which is not pretty, but at least seems to be valid PHP. Ultimately, I think we'll need the fix suggested in https://github.com/glayzzle/php-parser/issues/170 to fix this for good.
/cc @czosel can we solve this problem without https://github.com/prettier/plugin-php/pull/513? IF yes we should do this asap and do release, because it is break code and high priority (sorry for duplicate)
@evilebottnawi I’d recommend to merge #513, because it makes comment handling much simpler on the printing side. Let me know if there’s anything you’d still like to change in #513.
@evilebottnawi The only way to fix this seems to be to add support for "only-comment" AST nodes in the parser. Do you agree?
@czosel need investigate, feel free to experiment. Inline nodes is the most difficult part, comments also, together is incredibly difficult.
Hello, I took a stab at this and I was able to remove the double comment and the extra closing tag. https://github.com/nikolasmatt/plugin-php/commit/6b30d2f0c6ea05979c000285ad301bbd2b3c581f I've got it to the point where the output is
<?php
if (true) { ?>inline<?php
// comment
}
This is ugly but non-breaking.
I've hit a wall on how to make the block break appropriately. Any guidance would be much appreciated.
I think I ran into this today, as well. Here is the example that bit me. (Sorry if this is a different problem; I can open a new issue if so.)
Input
<?php $totalRows_rsUsers = 0; ?>
<?php if ($totalRows_rsUsers == 0) { // Show if recordset empty ?>
No results.
<?php } else { ?>
Found <?php echo $totalRows_rsUsers; ?> users.
<?php } ?>
Output (note the extra <?php tag before the comment)
<?php $totalRows_rsUsers = 0; ?>
<?php if ($totalRows_rsUsers == 0) {<?php
// Show if recordset empty
?>
No results.
<?php } else { ?>
Found <?php echo $totalRows_rsUsers; ?> users.
<?php } ?>
Which of course produces an error:
$ php test3.php
PHP Parse error: syntax error, unexpected '<' in /Users/crcarter/Software/Other/prettier/test3.php on line 2
Parse error: syntax error, unexpected '<' in /Users/crcarter/Software/Other/prettier/test3.php on line 2
This was with Prettier 1.15.2 and plugin-php 0.9.0.
@claytonrcarter yep, it is bug, need fix
@czosel I will spend the whole weekend to come up with an algorithm for the best way to print them, but i have some examples where don't have idea how better print.
Examples:
<?php if (
$veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongVariable ||
$veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongVariable
) { ?>
<div>Test</div>
<?php } ?>
Oh my good:
<?php if (
$veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongVariable ||
$veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongVariable
) {
if ($veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongVariable) { ?>
<div>Test</div>
<?php } ?> test <?php } ?>
Any ideas, other examples?
It is last what we should do and we can release stable version, support php7.3 we can implement in 1.1 version.
@evilebottnawi My feeling is that the whole inline topic is not something that can be solved in a "perfect" way - there is just too much freedom in mixing PHP with HTML. I'd suggest approaching the issue like this:
- Introduce AST node for
<?phpand?> - Try to format simple/typical examples as good as possible
- Wait and collect edge cases
- Decide which edge cases can be fixed
What do you think?
Introduce AST node for
- It is decrease code of lines but not solve a lot of problems
Try to format simple/typical examples as good as possible
We already do this in many cases.
Wait and collect edge cases
Yep
Decide which edge cases can be fixed
Some edge cases already fixed, but i want implement algorithm what cover 90%-95% cases. I have some ideas. Printing inline is difficult only for control structure, in other places it is easy and/or easy to fix.
I'm running into the same issue on the latest version (v0.10.2). Is there a recommended fix for this issue? I tried out the https://github.com/prettier/plugin-php/tree/issue-517 branch and it mostly works, except that running prettier --write followed by prettier -l returns the list of files that were affected by this bug on master.
Should we be watching https://github.com/prettier/plugin-php/issues/1053 instead for a resolution?
Thanks!