Skip to content

Conversation

@deathaxe
Copy link
Collaborator

@deathaxe deathaxe commented Jul 28, 2024

This PR...

  1. teaches HTML to consume

    • any trailing whitespace until end of line after opening script/style tags or optionally following <!-- punctuation.
    • any leading whitespace in front of closing script/style tags or optionally preceded --> punctuation.

    to not apply embedded syntax's scope after standalone opening or in front of standalone closing tags.

    This change makes sure to ...

    • always apply HTML specific indentation rules to enclosing script/style tags or comment punctuation.

    • apply custom background highlighting of embedded syntaxes only on full lines, but not after or in front of standalone tags.

  2. modifies indentation rules for HTML comments to ignore indentation of comments' content.

    This is primarily required to correctly handle indentation of embedded script/style source code, if it is enclosed in HTML comments. In such cases, additional indentation level is probably undesirable.

Highlighting changes

Before:

without comments with enclosing comments
grafik grafik

After:

without comments with enclosing comments
grafik grafik

Background in front of closing </script> no longer has custom source.js.embedded background.

grafik

However everything between tags is scoped source.<name>.embedded if tags and code exist in same line(s).

deathaxe added 2 commits July 28, 2024 17:57
This commit...

1. consumes

   a) any trailing whitespace until end of line after opening script/style tags
      or optionally following `<!--` punctuation.
   b) any leading whitespace in front of closing script/style tags or optionally
      preceded `-->` punctuation.

   to not apply embedded syntax's scope after standalone opening or in front of
   standalone closing tags.

   This change makes sure to ...

   a) always apply HTML specific indentation rules to
      enclosing script/style tags or comment punctuation.

   b) apply custom background highlighting of embedded syntaxes only on full
      lines, but not after or in front of standalone tags.

2. modifies indentation rules for HTML comments to remove indentation
   of comments' content.
keith-hall
keith-hall previously approved these changes Jul 28, 2024
Copy link
Collaborator

@keith-hall keith-hall left a comment

Choose a reason for hiding this comment

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

I always liked having the background coloring of embedded code start immediately after the open tag and end right before the close tag, but agree that it makes sense for indentation to leave those whitespaces unscoped. And that adding meta scopes just to satisfy me likely isn't worth the maintenance burden. Approved

@deathaxe
Copy link
Collaborator Author

deathaxe commented Jul 28, 2024

You prefer it even if those tags are on a separate line? The idea behind this change besides indentation is better visual appearance of embedded contents' background, which is inspired by how fenced codeblocks are handled in Markdown.

With regards to maintanance - it's rather a compatibility issue as adding meta scopes would affect various 3rd-party packages.

Those dependencies between core HTML and 3rd-party syntaxec could significantly be reduced by sublimehq/sublime_text#3787.

@keith-hall
Copy link
Collaborator

You prefer it even if those tags are on a separate line?

I do, but I accept that maybe I'm the odd one out here. I find it draws my eye exactly to the tag boundaries where the embedding really starts and ends.

Those dependencies between core HTML and 3rd-party syntaxec could significantly be reduced by sublimehq/sublime_text#3787.

Upvoted 🙂

@deathaxe deathaxe force-pushed the pr/html/tweak-script-indentation branch from d7e884b to 4dc9b5f Compare August 8, 2024 18:35
@deathaxe
Copy link
Collaborator Author

deathaxe commented Aug 8, 2024

Sorry, added and pushed the PHP fix in wrong branch. Reverted

Copy link
Collaborator

@michaelblyons michaelblyons left a comment

Choose a reason for hiding this comment

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

I actually kind of like this, even if it's not 100% accurate for the start and end of the language scopes. 😅

@deathaxe deathaxe requested a review from keith-hall August 31, 2024 19:38
@deathaxe deathaxe merged commit 57cb4a2 into sublimehq:master Aug 31, 2024
@deathaxe deathaxe deleted the pr/html/tweak-script-indentation branch August 31, 2024 22:38
deathaxe added a commit to SublimeText/Astro that referenced this pull request Oct 4, 2024
deathaxe added a commit to SublimeText/Astro that referenced this pull request Oct 4, 2024
Be a bit more lazy as scope changed from `meta.string.js`
to `meta.string.template.js`
in PR sublimehq/Packages#4020.
deathaxe added a commit to SublimeText/Vue that referenced this pull request Nov 19, 2024
This commit backports sublimehq/Packages#4020
to ensure related indentation rules are correctly applied for ST4181+.

Scope changes are backported primarily due to their effect on syntax tests
and the goal to avoid a need to ship dedicated ST4181+ releases just due
to failing syntax tests, which do otherwise not have any significant effect
on highlighting.
deathaxe added a commit to SublimeText/Vue that referenced this pull request Nov 19, 2024
In order to properly handle `<template>` tag indentation in combination
with script/style comment handling dedicated indentation rules are required.

This commit therefore backports and adjusts indentation rules from
sublimehq/Packages#4020
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.

3 participants