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

Allow parsing mixed HTML and PHP #234

Merged
merged 4 commits into from
Jun 18, 2021
Merged

Allow parsing mixed HTML and PHP #234

merged 4 commits into from
Jun 18, 2021

Conversation

sirbrillig
Copy link
Owner

When mixing HTML and PHP, we must ignore the HTML tokens and the closing PHP tokens. This PR alters the parsing to consider them "empty" for the sake of all the functions that skip over empty space. Notably, this allows considering a closing HTML tag as the end of the global scope.

Fixes #232

@sirbrillig sirbrillig merged commit 72cb4f9 into 2.10 Jun 18, 2021
@sirbrillig sirbrillig deleted the fix-html-in-php branch June 18, 2021 19:49
Comment on lines +16 to +22
return array_merge(
array_values(Tokens::$emptyTokens),
[
T_INLINE_HTML,
T_CLOSE_TAG,
]
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return array_merge(
array_values(Tokens::$emptyTokens),
[
T_INLINE_HTML,
T_CLOSE_TAG,
]
);
$empty = Tokens::$emptyTokens;
$empty[T_INLINE_HTML] = T_INLINE_HTML;
$empty[T_CLOSE_TAG] = T_CLOSE_TAG;
return $empty;

Array functions like array_merge() and array_values() are "expensive".

Changing it to the above will be more efficient performance + memory-use-wise.

You could even choose to optimize it further like this:

Suggested change
return array_merge(
array_values(Tokens::$emptyTokens),
[
T_INLINE_HTML,
T_CLOSE_TAG,
]
);
static $empty;
if (isset($empty) === false) {
$empty = Tokens::$emptyTokens;
$empty[T_INLINE_HTML] = T_INLINE_HTML;
$empty[T_CLOSE_TAG] = T_CLOSE_TAG;
}
return $empty;

It's not as if this compound array will change at any time during the run.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! Good to know. I reduced how often this function is called in #236 but I made #237 to make sure I remember to clean this up.

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.

File-scope unused variable not detected when ?> is present
2 participants