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

PHP tags in comments should not be ignored #50

Closed
Ingramz opened this issue Jul 19, 2016 · 4 comments
Closed

PHP tags in comments should not be ignored #50

Ingramz opened this issue Jul 19, 2016 · 4 comments
Assignees
Labels
bug The issue is a defect of this package.

Comments

@Ingramz
Copy link
Collaborator

Ingramz commented Jul 19, 2016

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 Ingramz added the bug The issue is a defect of this package. label Jul 19, 2016
@Ingramz Ingramz self-assigned this Jul 19, 2016
@Ingramz Ingramz changed the title PHP tags in comments should be ignored in HTML attributes PHP tags in comments should not be ignored Jul 19, 2016
@Ingramz
Copy link
Collaborator Author

Ingramz commented Jul 20, 2016

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
Copy link
Collaborator Author

Ingramz commented Aug 19, 2016

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 Ingramz added the help wanted The maintainers of package are welcoming pull requests that solve the issue. label Sep 15, 2016
@Ingramz
Copy link
Collaborator Author

Ingramz commented Sep 15, 2016

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

@Ingramz
Copy link
Collaborator Author

Ingramz commented Sep 23, 2016

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.

@Ingramz Ingramz closed this as completed Sep 23, 2016
@Ingramz Ingramz removed the help wanted The maintainers of package are welcoming pull requests that solve the issue. label Jan 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a defect of this package.
Projects
None yet
Development

No branches or pull requests

1 participant