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

Incorrect syntax highlighting of {{ }} statements #94

Open
FeBe95 opened this issue Jan 7, 2025 · 2 comments
Open

Incorrect syntax highlighting of {{ }} statements #94

FeBe95 opened this issue Jan 7, 2025 · 2 comments

Comments

@FeBe95
Copy link

FeBe95 commented Jan 7, 2025

There is an issue when using the {{ }} statement in blade files. The first closing curly bracket is not highlighted correctly when the statement is used inside of HTML attribute values.

Example:

<img src="{{ $foo }}">        <!-- incorrect -->

<span>foo {{ $bar }}</span>   <!-- correct -->

Note

I came across this issue while browsing some blade files in our GitHub repository. GitHub is using your package for highlighting blade files, see: https://github.com/github-linguist/linguist/blob/main/vendor/README.md.

@Ingramz
Copy link
Collaborator

Ingramz commented Jan 7, 2025

Hi! Thank you for the issue report.

I can explain what the differently highlighted curly bracket was used for. In Atom when someone typed {{}} and placed their cursor in the middle, the parser did not understand that inside it may contain PHP code, so none of the autocomplete options appeared. When the contents between curly brackets had length greater than 0, everything worked fine.

To remedy this, I used a trick that I saw in some other TextMate grammar, where the first character of the closing token was scoped the same as the contents would have been (source.php). To fix any potential visual inconsistencies, we added some custom styles to reset the styles for that token.

The case you have provided was likely never tested against GitHub Linguist and it's fair to assume that they should not be supporting the Atom-specific CSS overrides to accommodate the same hack, which leads to possible options to solve this particular use case:

  1. Erase the hack from the codebase - Atom has been EOL for a while now and it's reasonable to assume that anyone who is interested in syntax highlighting for Blade is either using a fork of this repository or a different editor/package.
  2. Create a separate grammar for Linguist which does not include editor-specific workarounds. This should be only done if there is someone that still depends on this repository for some Atom fork.
  3. Have Linguist switch to a different repository for the TextMate grammar. My day-to-day work unfortunately does not currently involve working with PHP and therefore I am not too invested in anything related the language. The most straightforward way to go about this would be to look at whatever is currently the de-facto Visual Studio Code grammar for Blade and politely pass the maintenance on them. This way Linguist would also be benefiting from new Blade constructs "for free" if there are any.

I'll try researching these options sometime soon, but in meantime I welcome any thoughts or feedback regarding the direction we should be taking.

@FeBe95
Copy link
Author

FeBe95 commented Jan 7, 2025

Hi, thanks for the quick and detailed response. I'm always curious to know why things behave in a specific way, and your explanation makes a lot of sense to me!

Regarding your three options I have the following thoughts:

  1. I remember reading somewhere that Atom was being retired some months/years ago, but I've never really used Atom myself. I also don't know how the Atom ecosystem works as of today, and if changing this repository might break Blade support for existing Atom installations. If this is the case, then I would argue against removing all Atom-related hacks from this repo, because there might be still some people using Atom as their main editor.
  2. I agree. This would be a good alternative, but only if people depend on the hacks to be present in this repo.
  3. I've done some research for alternative solutions, see below.

Alternative solutions

AFAIK GitHub Linguist supports various types of syntax definitions. The main types being Text Mate (.tmLanguage) and tree-sitter definitions. There are other file formats, such as the newer .sublime-syntax, but I am unsure whether Linguist is able to use those. Sublime Text was primarily using Text Mate syntax definitions, but they switched to this newer format in the recent years.

I've found some discussions about Sublime's new syntax files being (un-)supported by Linguist, but I cannot exactly tell if the syntax is supported as of today.

Note: There might also be tools available to convert one file format to the other.

Regardless, here are some relevant repositories I have found:

There are also some other Laravel Blade VS Code extensions available with a couple of 100k downloads, but those seem to be less maintained or even completely unmaintained. Some of them are even converted from the definitions in your repository (laravel-blade, Laravel Blade).

Conclusion

The most straightforward solution would be to remove everything related to Atom from this repository and wait for GitHub Linguist to catch up.
If we don't want to go with this option, we could request to change the Linguist Blade grammar to any of the packages above. I'd probably go with Medalink/laravel-blade then, if .sublime-syntax files are supported by Linguist, else with EmranMR/tree-sitter-blade.

Note: The people contributing to Linguist are usually pretty quick when it comes to responding and deciding on such topics. I just saw such a replacement being done for the .vue syntax, happening all within a few weeks (see: Replace Vue Grammar by yuichkun · Pull Request #7086 · github-linguist/linguist).

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

No branches or pull requests

2 participants