Skip to content

fix(language-tools): improve multiline style embedded language detection#16165

Open
Misrilal-Sah wants to merge 2 commits intowithastro:mainfrom
Misrilal-Sah:fix/langtools-multiline-style-sass-14657
Open

fix(language-tools): improve multiline style embedded language detection#16165
Misrilal-Sah wants to merge 2 commits intowithastro:mainfrom
Misrilal-Sah:fix/langtools-multiline-style-sass-14657

Conversation

@Misrilal-Sah
Copy link
Copy Markdown
Contributor

Issue ID: #14657

Description: Fixes syntax-highlighting language injection when lang=sass is not on the same line as style start, preventing CSS fallback mis-highlighting.

Copilot AI review requested due to automatic review settings March 31, 2026 17:27
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 31, 2026

⚠️ No Changeset found

Latest commit: b623052

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the Astro TextMate grammar to improve embedded language injection for <style> blocks (e.g., lang="sass") and reduce cases where styles fall back to incorrect CSS highlighting.

Changes:

  • Adds a special-case pattern for “open + content + </style> on the same line” style blocks for multiple style languages.
  • Replaces end: (?=</) with a while: guard intended to keep the embedded grammar active until </style>.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
packages/language-tools/vscode/syntaxes/astro.tmLanguage.src.yaml Updates injected style-language patterns (stylus/sass/css/scss/less/postcss/default) to avoid early termination on </.
packages/language-tools/vscode/syntaxes/astro.tmLanguage.json Applies the same injection-grammar updates in the generated JSON form.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 105 to 110
{
begin: '(?<=>)(?!</)',
end: '(?=</)',
while: '^(?!\s*</style[\s>])',
name: meta.embedded.block.astro,
contentName: source.stylus,
patterns: [{ include: source.stylus }],
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

Using begin + while anchored to ^ will only terminate the embedded block when </style> starts a line (optionally preceded by whitespace). This regresses cases where the closing tag appears after content on the same line (e.g., body { ... }</style>), because the while check won’t fail on that line, so the embedded scope can incorrectly consume </style> and potentially run to EOF. A more robust approach is to use end: '(?=</style[\\s>])' (instead of while) so termination works regardless of where the closing tag appears on the line; this also makes the new single-line special-case pattern unnecessary.

Copilot uses AI. Check for mistakes.
Comment on lines 112 to 117
{
"begin": "(?<=>)(?!</)",
"end": "(?=</)",
"while": "^(?!\\s*</style[\\s>])",
"name": "meta.embedded.block.astro",
"contentName": "source.stylus",
"patterns": [
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

Same issue as the YAML source: while: '^(?!\\s*</style[\\s>])' only detects a closing </style> at the start of a line, so lines like ... }</style> won’t terminate the embedded region correctly. Consider switching back to a proper end condition that targets only the style close tag (e.g., (?=</style[\\s>])) so the embedded scope ends correctly even when </style> is not at BOL, and then you can drop the extra single-line pattern.

Copilot uses AI. Check for mistakes.
@matthewp
Copy link
Copy Markdown
Contributor

matthewp commented Apr 1, 2026

@Misrilal-Sah can you add a changeset? pnpm run changeset

Copy link
Copy Markdown
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

  • changeset missing
  • restore PR template
  • use magic comments to auto close the issue

@Princesseuh
Copy link
Copy Markdown
Member

There's been a lot of previous tries at this and none of them worked. I'd love to see a test added to make sure it actually does in fact work.

@gllmt
Copy link
Copy Markdown

gllmt commented Apr 2, 2026

Thanks for working on this.

I took some time to test the current approach locally on top of upstream/main, and I think there may still be one regression to address before this is ready.

With the current patch, pnpm test:grammar still fails on packages/language-tools/vscode/test/grammar/fixtures/style/expression.astro. From what I can tell, this seems related to the new while: '^(?!\\s*</style[\\s>])' logic: when </style> appears on the same line as style content, the embedded block does not terminate cleanly.

I also tried a different approach locally by updating tags-lang in astro.tmLanguage.src.yaml so lang / type can still be detected when they appear on later lines of the opening tag. That version seems to avoid the same-line closing-tag regression, and it passes pnpm test:grammar locally.

Given Princesseuh’s note about previous attempts, I think adding dedicated grammar fixtures would probably make this much easier to validate with confidence:

  • multiline <style lang="sass">
  • multiline <style lang="scss">
  • multiline <style lang="less">
  • ideally one multiline <script ...> case too, since the same tags-lang path handles both

What do you think about moving the fix closer to tags-lang and validating it through fixtures/snapshots instead of adjusting only the style injection patterns?

@RedCMD
Copy link
Copy Markdown

RedCMD commented Apr 2, 2026

I don't have any good ideas on how to solve this
but while isn't the best
as while is line based
so something like this would fail

<style>css
css
css<style> html <!-- comment
still comement -->

and you definitely cannot just anchor to the start of a line
"while": "(?!<style>)"

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.

7 participants