Skip to content

Highlight PHP attributes #172

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

Closed

Conversation

javiereguiluz
Copy link
Collaborator

Our current highlighter doesn't support PHP attributes ... but we use more and more attributes in the Symfony Docs.

I tried changing the highlighter (I tested https://github.com/tempestphp/highlight) but, although it solved the attribute issue, they introduced more issues.

So, this PR adds a "hack" to the current highlighter to add support for PHP attributes:

Before // After


I added many tests to ensure that we support all the different ways of defining PHP attributes. There's still an edge case related to multi-line attributes. It's not a big problem and I'll tackle this in a future PR.

@wouterj
Copy link
Contributor

wouterj commented May 15, 2024

I personally feel like we can better update the syntax file: https://github.com/symfony-tools/docs-builder/blob/main/src/Templates/highlight.php/php.json (which we already maintained to support newer syntax) than processing the resulting HTML output.

The latest highlight.js PHP syntax file also supports PHP attributes already, so we can take inspiration from it. Unfortunately, I've tried using the converter from HighlightPHP in the past but this no longer works with the latest highlight.js syntax files so we can't automate it :(

Relevant bit from the highlight.js syntax file: https://github.com/highlightjs/highlight.js/blob/main/src/languages/php.js#L443-L477

@javiereguiluz
Copy link
Collaborator Author

Wouter, thanks for sharing these resources.

However, given that adding support for PHP attributes is quite urgent (we should have made this a long ago) I propose to use this solution for now until we can work on the other solution. From the point of view of symfony.com, this just requires 1 new CSS style, so it will be trivial to revert the changes in the future.

My preference would be to use tempest/highlight which is a modern PHP-based highlighter and supports attributes. However, after creating a proof-of-concept with it, I decided to not continue with it because it introduces other issues that would make some of our current highlighting worse. I'm sure it will improve in time 👍

@javiereguiluz
Copy link
Collaborator Author

Closing in favor of #173, which does the same but it's much simpler and easier to maintain.

javiereguiluz added a commit that referenced this pull request May 20, 2024
…ereguiluz, wouterj)

This PR was merged into the main branch.

Discussion
----------

Update the php syntax file to highlight attributes

Alternative to #172 (as per my suggestion).

`@javiereguiluz` what do you think of this? The output is slightly different from yours, but that's how highlight.js works :)

I tried my suggestion of converting the latest highlight.js syntax file, this unfortunately didn't work (there are too many BC breaks in v10). So I've copy pasted their regex and used it to what is available in our syntax file already.

If we want to go this way, I can also follow up with a PR adding more support for PHP features (e.g. arrow functions). For now, I've just added a couple keywords that plainly were missing (such as trait and enum).

Commits
-------

020a281 Add support for PHP attributes
9764478 Add tests for highlighting PHP attributes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants