Sublime-Instincts / BetterTwig

A Sublime Text package that provides enhanced syntax highlighting, completions, snippets & more for PHP Twig templates.
https://packagecontrol.io/packages/Twig
MIT License
11 stars 5 forks source link

Variable Interpolation in inline CSS style flagged as syntax error #25

Closed mikevaux closed 2 years ago

mikevaux commented 2 years ago

Description of the bug

When using a twig variable in an inline CSS style, the opening curly brace, and both closing curly braces are flagged as syntax errors.

Steps to reproduce

  1. Open new file, and set syntax to twig
  2. Write <div style="color: {{ theme }};"></div>

Expected behavior

The code to display without any syntax errors

Actual behavior

Screenshot 2022-01-18 at 14 28 11

BetterTwig version

2.0.0

mikevaux commented 2 years ago

@Ultra-Instinct-05 I just wanted to check in to see if you had any thoughts on this syntax bug? Is it likely to be a simple or complicated one to fix do you think?

UltraInstinct05 commented 2 years ago

From what I can see, it looks like this issue is coming from the embedded CSS syntax (that is being embedded in the style attribute). It's matching the expression token { as an invalid one. I'll have to see if it's possible to hook into some context to not make it mark it as illegal. But I haven't delved too deep into this.

UltraInstinct05 commented 2 years ago

After doing a bit of digging, looks like I'll have to refactor the syntax into a core Twig syntax (just bare bones Twig with no HTML), extend the default CSS syntax to include Twig (which means creating a new syntax) and then use that new syntax for embedding into HTML + Twig syntax, just to avoid the invalid issue. I kind of have all this working on my local branch, but would like to know

  1. Do you perhaps know of any open source repos where Twig is heavily used in CSS ? (For testing purposes)
  2. Are Twig tags allowed in CSS ?
UltraInstinct05 commented 2 years ago

Looking at what Rails does, this indeed does seem to be the correct approach https://github.com/sublimehq/Packages/tree/master/Rails

mikevaux commented 2 years ago

Apologies for the delay. This isn't an area I'm familiar with at all I'm afraid, but I'll feed in where I can! :)

Do you perhaps know of any open source repos where Twig is heavily used in CSS ? (For testing purposes) Are Twig tags allowed in CSS ?

Sorry, I don't totally understand the questions - twig would compile down server-side to a flat html file, so by the time the browser looks at the CSS (be it either as an internal style sheet, or inline styles), there'd be no twig there. You certainly wouldn't use twig in an external CSS style sheet.

I know that Vue JS uses a similar curly brace syntax, which you can use in attributes, though it may be (it's been a while) that this is only in the dynamic attributes, i.e. :style="{{ styles }}"

Looking at what Rails does, this indeed does seem to be the correct approach

Ah, that's good to know!

Also, fwiw, it looks like it's also a bit of an issue in Sublime Merge - syntax highlighting has something a bit funky going on as well, though it's not a problem in GitLab.

Screenshot 2022-05-10 at 16 24 22

UltraInstinct05 commented 2 years ago

Sorry, I don't totally understand the questions - twig would compile down server-side to a flat html file, so by the time the browser looks at the CSS (be it either as an internal style sheet, or inline styles), there'd be no twig there. You certainly wouldn't use twig in an external CSS style sheet.

I was actually asking for examples for syntax testing purposes actually :) I do know Twig, like any other templating engine compiles to HTML in the end. But it's alright, actually :)

Also, fwiw, it looks like it's also a bit of an issue in Sublime Merge - syntax highlighting has something a bit funky going on as well, though it's not a problem in GitLab.

Sublime Merge uses the same syntax highlighting engine as Sublime Text. So yes, it would look kinda of funky since there is a problem in this package itself. Once this issue is fixed, I believe this would also resolve itself.

mikevaux commented 2 years ago

I was actually asking for examples for syntax testing purposes actually :) I do know Twig, like any other templating engine compiles to HTML in the end. But it's alright, actually :)

Apologies if I misunderstood you! :-)

Sublime Merge uses the same syntax highlighting engine as Sublime Text. So yes, it would look kinda of funky since there is a problem in this package itself. Once this issue is fixed, I believe this would also resolve itself.

Ah right, that's helpful to know.

Thanks a lot - do let me know if there's anything else I can help with 👍

UltraInstinct05 commented 2 years ago

This issue should be addressed as of v2.1.0. It will be available in a couple hours. Would appreciate if you could confirm if you are no longer running into this issue.

mikevaux commented 2 years ago

Amazing! 🙌

Yes, looks great!

Screenshot 2022-05-11 at 12 37 37

There is a slight regression I've found when creating this test scenario (sorry!!), which is that if you try and comment out the line when the cursor is within the " of the style attribute, instead of using the {# #}, it instead tries to use a CSS comment! 😅

Screenshot 2022-05-11 at 12 38 38

Do you want me to create a separate issue for this?

Thanks very much, really appreciate it.

UltraInstinct05 commented 2 years ago

There is a slight regression I've found when creating this test scenario (sorry!!), which is that if you try and comment out the line when the cursor is within the " of the style attribute, instead of using the {# #}, it instead tries to use a CSS comment!

Was that working earlier before v2.1.0 ?

UltraInstinct05 commented 2 years ago

It appears to use Twig comments on my side instead of CSS comments on Build 4131. What version of ST are you running ?

image

I can actually reproduce this on Build 4126, but it happens even for normal HTML files (nothing related to Twig). I know there were some changes related to comments that have been done in builds after 4126, but I'd have to dig a bit deeper to see it. But since it doesn't happen on the latest dev build and not related to Twig, I'd just recommend upgrading to dev or waiting for the next stable release.

image

mikevaux commented 2 years ago

Huh, interesting. I'm on 4126. Ok, sure - happy to wait for ST to upgrade!

Reworked comment toggling to better handle embedded languages

API: The toggle_comment command can now take a variant argument for languages with multiple comment variants

Yes, these look pretty relevant! 😄