search

atom / language-php

PHP package for Atom
Other
119 stars 118 forks source link

Losing some context with HTML block in function #402

Open roblourens opened 3 years ago

roblourens commented 3 years ago

This sort of thing has been covered before like in https://github.com/atom/language-php/issues/246, and I don't know whether this can be improved, I think there is a tradeoff between preserving PHP scope info between blocks and having correct colorization in between them. So close this if it's not practical.

Originally https://github.com/microsoft/vscode/issues/74382

54168483-781f5280-444d-11e9-8868-390dcc20b940

Copy and paste the code below. The brackets are with the wrong color

<?php if ($test) { ?>
    <p>Lorem ipsum dolor, sit amet consectetur adipisicing elit. Molestiae ipsam eius quos voluptate hic recusandae excepturi cum. Dolores vitae sunt optio assumenda soluta aliquam numquam voluptatum modi minima, labore illo.</p>
<?php } else { ?>
    <p>Lorem ipsum dolor, sit amet consectetur adipisicing elit. Molestiae ipsam eius quos voluptate hic recusandae excepturi cum. Dolores vitae sunt optio assumenda soluta aliquam numquam voluptatum modi minima, labore illo.</p>
<?php } ?>
54559784-1f0d6c80-49c1-11e9-8a34-b807e081b18e 54559671-d81f7700-49c0-11e9-8152-5d4f059ab28f

As you can see it provides us just a generic source.php meta.embedded.line.phpthat we can't target. This is sort of bug because the second curly bracket (the opening one) have a different scope.

KapitanOczywisty commented 3 years ago

@roblourens php is injected into html, so each php block has no context of one before.

We could add closing bracket in global context (in html.cson) and this will fix this exact problem.

Other issue #246 is about how injection are (not)working, and probably not worth trying to fix. If you're doing more than if/else in html files you probably need template engine anyway (or smth client-side like react).

roblourens commented 3 years ago

I think #246 was fixed, and it was the opposite problem that the html in between was not colored correctly. Just mentioning it as generally related.

KapitanOczywisty commented 3 years ago

246 is only partially fixed in sense that html won't be eaten by php, but some things after <?php will be broken, since context is forgotten in 2nd block. Proper fix would allow html injections inside php injections. However with TM, then there would be the same problems as with sql. This is a bit sad tbh.