jawee / language-blade

Blade (Laravel) templating support in Atom
https://atom.io/packages/language-blade
Other
51 stars 21 forks source link

PHP tags in comments should not be ignored #50

Closed Ingramz closed 8 years ago

Ingramz commented 8 years ago

PHP tags take precedence over comments.

{{--<?php echo $this->anchor; ?>--}}

Gets highlighted in some themes, but probably not in an intended way.

<form name="bt_filter" method="get" action="{{--<?php echo $this->anchor; ?>--}}">
Ingramz commented 8 years ago

Laravel's Blade compiler splits source file into tokens using token-get-all, and only performing replacements to tokens that are not PHP code. If there is PHP code between Blade tags, that will invalidate the tags and we should not highlight them. This behavior requires some sort of lookahead, which TM grammars cannot do.

We can however mark the code invalid inside comment tags, to warn the user.

I am slightly starting to doubt why does Laravel need to use Zend's exponential-at-worst* parser to split a string at <?php and ?>**. While most parses do not take more than 10ms and are run only once per file change, this is acceptable, but definitely not the most efficient way.

* Educated guess, not a guaranteed claim \ Closing tag parsing is nontrivial (strings, comments), but still perfectly doable with some insight.

Ingramz commented 8 years ago

An update on this: token_get_all seems to be orders of magnitude faster than simply iterating over all characters of a string in PHP, so fixing it on language level seems impractical.

Ingramz commented 8 years ago

This remains unfixable but there's an intermediary solution where we highlight PHP tags in comments with an .invalid.illegal scope.

Ingramz commented 8 years ago

Final decision, because of no way to backtrack (or lookahead with by covering all possible edge cases), we'll consume comments as usual and just highlight PHP code in it differently to warn the user.