daaain / Handlebars

Fullest Handlebars.js templating support for Sublime Text 2 / 3. Also drives syntax colouring on Github and in Visual Studio Code. Install from: https://packagecontrol.io/packages/Handlebars.
MIT License
301 stars 48 forks source link

Syntax Highlighting Error ... or No Error? #110

Closed sunnyAfterSnow closed 8 months ago

sunnyAfterSnow commented 1 year ago

When I installed the Handlebars package, there was a problem with the syntax highlighting. As shown below. How can I solve it?

My environment: SublimeText win-x64 4143 build (latest fourth edition) Handlebars package version: 2019.04.13.17.11.43 (the latest version)

HBS_ERROR

Then, strangely enough, when I installed the same package in sublimetext 3, everything worked fine. what is going on?

My environment: SublimeText win-x64 3211 build (latest third version) Handlebars package version: 2019.04.13.17.11.43 (the latest version)

HBS_OK

Conclusion: ST3 and ST4 are not fully compatible, and the same version of the plug-in package will have problems

MatthiasPortzel commented 10 months ago

This bug has previously been reported in #109 but there's a lot of unnecessary discussion there so I'm going to pick up here.

TLDR: Use https://tildegit.org/matthias/Sublime-Handlebars

I've spent a bit of time looking into this. Basically, this syntax includes the JavaScript syntax (with include: scope:source.js). I asked in the Discord and @Thom1729 gave this explanation for why this is a problem:

For context, - include: scope:source.js means “whatever the current set of packages associates with the source.js scope, include that”. So that line makes implicit assumptions about other packages outside the control of the Handlebars package. It may be that those assumptions are true about the ST3 JavaScript package but not true about the ST4 JavaScript package.

So my minimum reproduction case is currently as follows:

name: Handlebars_Min_Repo
file_extensions:
  - handlebars
  - hbs
scope: text.html.handlebars
contexts:
  main:
    - match: '<works>'
      push:
        - meta_scope: constant.numeric.debug

        - match: '</works>'
          pop: true

    - match: '<doesnt>'
      push:
        - meta_scope: constant.numeric.debug
        - include: scope:source.js

        - match: '</doesnt>'
          pop: true
image

(The image shows that the syntax highlighting fails to pop only after importing the JavaScript scope. I've converted this syntax into .sublime-syntax for this example because it's easier to work with and more clearly demonstrates that this is a behavior issue. I'm not entirely convinced that the above hypothesis is correct; I still think it's possible there's a ST4 bug at the root of this.)

The easy solution at this point is to stop using include and to use the modern embed directive instead, which was designed to solve exactly this problem.

I've taken this step and created a fork that ports the syntax to the modern .sublime-syntax file format and replaces the offending include with an embed. (On Tildegit instead of GitHub because GitHub has been showing me misleading CoPilot ads when viewing a file.) This does resolve the issue.

https://tildegit.org/matthias/Sublime-Handlebars

This satisfies my immediate need to edit Handlebars files.

The obvious problem with this solution is that it uses a feature of sublime-syntax files which I don't believe is available in .tmLanguage files, and VSCode doesn't support .sublime-syntax files. I haven't checked. So right now, it looks to me like we have to choose between doing nothing and leaving Sublime in a broken state, or moving to modern Sublime features, and potentially forcing VSCode to stay on the current version indefinitely. As a Sublime user, I would prefer the latter option, but I do want to hear from @daaain.

daaain commented 10 months ago

Thanks a lot for the thorough investigation and writeup @MatthiasPortzel, really appreciated!

Do you think it would be possible to have both .sublime-syntax and the old .tmLanguage in the same package?

We definitely can't get rid of .tmLanguage because this repo is what Github uses for Handlebars syntax highlighting and I think Linguist needs that old syntax.

MatthiasPortzel commented 10 months ago

I don't think it would be difficult to maintain them side-by-side. (Not ideal obviously, but better than the status quo.)

With both files present in the package, Sublime will show duplicate entries for "Handlebars". (It seems to prefer the sublime-syntax one.) It looks like it's possible to configure package control to ignore certain files when creating a package, so it should be possible to tell it to ignore .tmLanguage files and add both files to grammars/.

I mostly want to get to a state where PC: Install Package > Handlebars gives Sublime users a pleasant experience.

keith-hall commented 10 months ago

With both files present in the package, Sublime will show duplicate entries for "Handlebars". (It seems to prefer the sublime-syntax one.)

Are you sure Sublime shows the same syntax twice? My previous experience was if you have the sublime-syntax in the same folder as the tmLanguage, Sublime pretends the tmLanguage is the sublime-syntax. I.e. even directly setting the syntax for a view to the tmLanguage highlights with the sublime-syntax. As they are deemed to be the same resource, it shouldn't show twice in any default syntax picker menu.

It looks like it's possible to configure package control to ignore certain files when creating a package, so it should be possible to tell it to ignore .tmLanguage files and add both files to grammars/.

Iirc that is done by editing the .gitattributes and affects the zip file download from the releases. If VSCode also uses this mechanism then it would break VSCode. Having separate branches could help, or even split the Sublime package into a separate repository and update Package Control to point to it.

MatthiasPortzel commented 10 months ago

Thanks for correcting me on the first point. It does seem that the sublime-syntax overrides the tmLanguage file. (I had two versions of the package installed.)

I was referring to the files_to_ignore package control setting. I now realize that's not particularly convenient, as it's per-user, not per-package. If we can ship both files the point is insignificant.

daaain commented 10 months ago

I checked and the VSCode package is a mirror of this one: https://github.com/microsoft/vscode/blob/main/extensions/handlebars/syntaxes/Handlebars.tmLanguage.json#L2-L9

So even if having sublime-syntax would break VSCode or something, its package can be maintained as a fork (which it already is).

But anyway, it seems that we have a promising way forward, would you mind creating a PR @MatthiasPortzel (or anyone else) adding the new syntax, but keeping the old one? I guess the best scenario would be to have some common source and that would be compiled to each language output file, but since there isn't a huge amount of activity on the syntax these days, they can even be just hand updated separately. The important thing is to put some notes about this in the README.

daaain commented 8 months ago

This got auto-closed apparently 🤷 Github had an outage yesterday so it took a while for the changes to propagate to Package Control, but just checked and the new version is out so please update and let me know if the new version solves the issues you are seeing or if the issue should be reopened!