Make sure attributes with inline JavaScript are correctly closed

ccampbell commented

2021-10-29 05:16:59 +00:00

(Migrated from github.com)

Fixes corneliusio/svelte-sublime#16
Fixes corneliusio/svelte-sublime#17

There may be a better way to fix this, but this seems to work for me. It would be nice to have some syntax tests in this repo to be able to verify that things work in more unusual use cases.

Fixes corneliusio/svelte-sublime#16 Fixes corneliusio/svelte-sublime#17 There may be a better way to fix this, but this seems to work for me. It would be nice to have some syntax tests in this repo to be able to verify that things work in more unusual use cases.

corneliusio commented

2021-10-29 13:56:45 +00:00

(Migrated from github.com)

@ccampbell Tested this out locally on my machine and this did not end up fixing the issue for me—will be working on it though. Also, note that Svelte.sublime-syntax.yaml-macros should be the source file where changes are made, which is then compiled to Svelte.sublime-syntax with the YAML Macros package/plugin, just for future reference.

@ccampbell Tested this out locally on my machine and this did not end up fixing the issue for me—will be working on it though. Also, note that `Svelte.sublime-syntax.yaml-macros` should be the source file where changes are made, which is then compiled to `Svelte.sublime-syntax` with the YAML Macros package/plugin, just for future reference.

ccampbell commented

2021-10-29 14:53:41 +00:00

(Migrated from github.com)

Hmmm. For me it definitely works. Did you remove all active svelte syntaxes before installing it? I am on Sublime build 4120 on the dev channel (should be the same as the 4121 stable build), but the highlighting broke for me earlier than that (dev channel gets updated more often).

Before

After

As for the other stuff, sorry about that, I will fix that up so it uses the macros file.

Hmmm. For me it _definitely_ works. Did you remove all active svelte syntaxes before installing it? I am on Sublime build 4120 on the dev channel (should be the same as the 4121 stable build), but the highlighting broke for me earlier than that (dev channel gets updated more often). **Before** <img src="https://user-images.githubusercontent.com/259316/139455944-594376dc-9aad-4fda-b9b6-415026a80d70.png" width="531" height="393" /> **After** <img src="https://user-images.githubusercontent.com/259316/139456161-c3b24371-13f1-4b94-a274-cab6d17079f4.png" width="537" height="388" /> As for the other stuff, sorry about that, I will fix that up so it uses the macros file.

ccampbell commented

2021-10-29 15:05:44 +00:00

(Migrated from github.com)

An easy way to test is to use safe mode.

Mac instructions:

Quit sublime
Hold option key when starting sublime
$ cd ~/Library/Application\ Support/Sublime\ Text\ $Safe\ Mode$/Packages/User
$ git clone git@github.com:ccampbell/svelte-sublime.git
Check syntax

I updated it to use the macros file and YAMLMacros correctly in the this branch: https://github.com/ccampbell/svelte-sublime/tree/macros

Oddly enough, a lot of things seem to change in the .sublime-syntax file when doing it this way including removing the name attribute which seems odd to me.

An easy way to test is to use safe mode. Mac instructions: 1. Quit sublime 2. Hold option key when starting sublime 3. `$ cd ~/Library/Application\ Support/Sublime\ Text\ $Safe\ Mode$/Packages/User` 4. `$ git clone git@github.com:ccampbell/svelte-sublime.git` 5. Check syntax --- I updated it to use the macros file and `YAMLMacros` correctly in the this branch: https://github.com/ccampbell/svelte-sublime/tree/macros Oddly enough, a **lot** of things seem to change in the `.sublime-syntax` file when doing it this way including removing the name attribute which seems odd to me.

corneliusio commented

2021-10-29 15:38:03 +00:00

(Migrated from github.com)

@ccampbell It's very possible the HTML syntax definition that this extends also had updates made to it which would carry over when the source file is built. I should clarify that your solution did partially fix the problem, however there was something odd going on with the syntax definitions in the embed JS scope.

It's very possible I'll only need to build on what you changed, honestly I haven't had to do any extensive work with Sublime's syntax definitions for a while so a little rusty and just getting back into the groove. I'll also double check that I didn't have any other conflicts going on that may have been the root of any issues I'm seeing on my end.

Appreciate your time, work, and patients on this. Will be trying to get it worked out this weekend while I'm out of town as much as possible.

@ccampbell It's very possible the HTML syntax definition that this extends also had updates made to it which would carry over when the source file is built. I should clarify that your solution did partially fix the problem, however there was something odd going on with the syntax definitions in the embed JS scope. It's very possible I'll only need to build on what you changed, honestly I haven't had to do any extensive work with Sublime's syntax definitions for a while so a little rusty and just getting back into the groove. I'll also double check that I didn't have any other conflicts going on that may have been the root of any issues I'm seeing on my end. Appreciate your time, work, and patients on this. Will be trying to get it worked out this weekend while I'm out of town as much as possible.

🎉 1

corneliusio commented

2021-10-29 15:39:30 +00:00

(Migrated from github.com)

I also need to make sure this is backward compatible with Sublime Text 3 and if it's not, create separate releases for them.

deathaxe commented

2021-10-30 15:33:37 +00:00

(Migrated from github.com)

If you built a new file using ST4 the resulting syntax won't be backward compatible as core HTML.sublime-syntax is of version 2 which ships some breaking syntax engine features/fixes.

An ST4 only release could simplify syntax definition by extending HTML.sublime-syntax

To learn how it works you may have a look at https://github.com/vuejs/vue-syntax-highlight/tree/st4. They use nearly the same approach as this package.

If you built a new file using ST4 the resulting syntax won't be backward compatible as core HTML.sublime-syntax is of version 2 which ships some breaking syntax engine features/fixes. An ST4 only release could simplify syntax definition by extending HTML.sublime-syntax To learn how it works you may have a look at https://github.com/vuejs/vue-syntax-highlight/tree/st4. They use nearly the same approach as this package.

corneliusio commented

2021-10-30 16:55:56 +00:00

(Migrated from github.com)

@deathaxe Thanks for the info, I had misspoken in my previous comment—it looks like I included a copy of the HTML syntax definitions in the package to avoid that potential issue. And I just discovered the new extending features in ST4 today while working through the broken Javascript syntax include. I will def be referencing vue-syntax-highlight for that ST4 specific release, they were the original inspiration for much of the solutions in this package. I'll likely need to rewrite most of the definitions from scratch due to this new approach, but for the time being this older, somewhat janky solution should work.

@ccampbell Thanks again for your time helping get this bug resolved. I'm closing out this PR in favor of @deathaxe's fix posted in the issue comments.

@deathaxe Thanks for the info, I had misspoken in my previous comment—it looks like I included a copy of the HTML syntax definitions in the package to avoid that potential issue. And I just discovered the new extending features in ST4 today while working through the broken Javascript syntax include. I will def be referencing vue-syntax-highlight for that ST4 specific release, they were the original inspiration for much of the solutions in this package. I'll likely need to rewrite most of the definitions from scratch due to this new approach, but for the time being this older, somewhat janky solution should work. @ccampbell Thanks again for your time helping get this bug resolved. I'm closing out this PR in favor of @deathaxe's fix posted in the issue comments.

👍 1

Pull request closed

Please reopen this pull request to perform a merge.

Rows
Columns

Make sure attributes with inline JavaScript are correctly closed #18

Pull request closed