corneliusio / svelte-sublime

💡Sublime Text syntax highlighting for Svelte components
MIT License
58 stars 6 forks source link

Rewrite to extend ST4's HTML and JS #33

Closed deathaxe closed 3 weeks ago

deathaxe commented 3 weeks ago

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 3 weeks ago

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 3 weeks ago

@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.

corneliusio commented 3 weeks ago

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 3 weeks ago

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

corneliusio commented 3 weeks ago

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.

corneliusio commented 3 weeks ago

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