Rewrite to extend ST4's HTML and JS #33

Merged
deathaxe merged 8 commits from st4-extend-html into master 2024-09-28 21:09:35 +00:00
deathaxe commented 2024-09-27 20:14:37 +00:00 (Migrated from github.com)

Fixes #32

requires: ST4143+

This PR rewrites Svelte by replacing YAMLMacros approach by statically extending ST's core HTML and JavaScript syntax to implement Svelte Components.

Motivation

JavaScript supports syntax highlighting in tagged template strings as of ST4181, which causes infinite inclusion loop in push...with_prototype statements of Svelte.

Also with_prototype is at least known to cause huge syntax definitions.

With this commit syntax cache size drops from 1.1MB to 160kB.

Fixes #32 requires: ST4143+ This PR rewrites Svelte by replacing YAMLMacros approach by statically extending ST's core HTML and JavaScript syntax to implement Svelte Components. #### Motivation JavaScript supports syntax highlighting in tagged template strings as of ST4181, which causes infinite inclusion loop in `push...with_prototype` statements of Svelte. Also `with_prototype` is at least known to cause huge syntax definitions. With this commit syntax cache size drops from 1.1MB to 160kB.
deathaxe commented 2024-09-28 13:30:52 +00:00 (Migrated from github.com)

As this PR requires ST4143+ a release should be made with a new tag prefix (e.g.: st4143-) with and package_control_channel being updated accordingly, so only compatible ST builds pull it. Older ST releases should remain on current release.

Maybe even worth a new major release as it is literly a rewrite.

As this PR requires ST4143+ a release should be made with a new tag prefix (e.g.: `st4143-`) with and package_control_channel being updated accordingly, so only compatible ST builds pull it. Older ST releases should remain on current release. Maybe even worth a new major release as it is literly a rewrite.
corneliusio commented 2024-09-28 19:33:04 +00:00 (Migrated from github.com)

@deathaxe Gotta admit, when I first saw this PR open up and pretty much everything was nuked I was, "Alright, bold play but let's let 'em cook." Then I started seeing the new work and was like, "Damn, this is looking great." And only then did I realize who was writing all of this. 😂

Thank you so much for taking the time to do all of this work. 🙏🏼 It's a fantastic rewrite. This is certainly buttoned up enough for me to go ahead and get it merged in—especially with the open issue it resolves. I'll also get a PR over in the package_control_channel today or tomorrow.

I did find one edge case that has regressed, it's a very specific set of conditions that trigger it, but when using a keyed #each block with an index and where the value is a destructured array the following comma is defined as keyword.operator.comma.js instead of punctuation.separator.comma.svelte. This also causes the index variable to get treated as a function call.

Screenshot 2024-09-28 at 2 12 51 PM

This may be a non-trivial fix (I wasn't able to quickly find a solution) and if so the other improvements and the fix more than justify a minor regression like this but I thought I'd mention it in case you're able to easily account for these cases.

@deathaxe Gotta admit, when I first saw this PR open up and pretty much everything was nuked I was, "Alright, bold play but let's let 'em cook." Then I started seeing the new work and was like, "Damn, this is looking great." And only then did I realize who was writing all of this. 😂 Thank you so much for taking the time to do all of this work. 🙏🏼 It's a fantastic rewrite. This is certainly buttoned up enough for me to go ahead and get it merged in—especially with the open issue it resolves. I'll also get a PR over in the package_control_channel today or tomorrow. I did find one edge case that has regressed, it's a very specific set of conditions that trigger it, but when using a keyed `#each` block with an index and where the value is a destructured array the following comma is defined as `keyword.operator.comma.js` instead of `punctuation.separator.comma.svelte`. This also causes the index variable to get treated as a function call. <img width="491" alt="Screenshot 2024-09-28 at 2 12 51 PM" src="https://github.com/user-attachments/assets/34b07629-a439-4f5e-beab-eb62917e6678"> This may be a non-trivial fix (I wasn't able to quickly find a solution) and if so the other improvements and the fix more than justify a minor regression like this but I thought I'd mention it in case you're able to easily account for these cases.
corneliusio commented 2024-09-28 19:55:51 +00:00 (Migrated from github.com)

Actually, and I should have caught this before, but it does seem to have regressed syntax highlighting in <style lang="postcss"></style> tags as the update simply treats it as regular CSS embedded source.

Actually, and I should have caught this before, but it does seem to have regressed syntax highlighting in `<style lang="postcss"></style>` tags as the update simply treats it as regular CSS embedded source.
deathaxe commented 2024-09-28 20:30:50 +00:00 (Migrated from github.com)

What does postcss offer beyond default CSS these days, now that nesting etc. is fully supported?

What does postcss offer beyond default CSS these days, now that nesting etc. is fully supported?
corneliusio commented 2024-09-28 21:01:46 +00:00 (Migrated from github.com)

I'd have to do some digging to see if there are any other common enough cases to consider, but off the top of my head I know that TailwindCSS's @apply gets properly handled (for the most part) when using PostCSS for highlighting.

Screenshot 2024-09-28 at 3 58 10 PM Left is plain CSS, right is PostCSS.

Edit: Ah! Just saw you pushed a new commit for this.

I'd have to do some digging to see if there are any other common enough cases to consider, but off the top of my head I know that TailwindCSS's `@apply` gets properly handled (for the most part) when using PostCSS for highlighting. <img width="1508" alt="Screenshot 2024-09-28 at 3 58 10 PM" src="https://github.com/user-attachments/assets/d4a58b14-ba59-4963-a7ed-91ab9d4d1297"> Left is plain CSS, right is PostCSS. **Edit: Ah! Just saw you pushed a new commit for this.**
corneliusio commented 2024-09-28 21:08:08 +00:00 (Migrated from github.com)

@deathaxe Incredible, this all looks perfect. Truly, thank you so much for taking the time to do all of this.

@deathaxe Incredible, this all looks perfect. Truly, thank you so much for taking the time to do all of this.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
repos/svelte-sublime!33
No description provided.