Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
sunnyAfterSnow opened this issue Jun 15, 2023 · 7 comments
Closed

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

sunnyAfterSnow opened this issue Jun 15, 2023 · 7 comments

Comments

@sunnyAfterSnow
Copy link

sunnyAfterSnow commented Jun 15, 2023

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
Copy link
Contributor

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
Copy link
Owner

daaain commented Sep 19, 2023

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
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

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
Copy link
Owner

daaain commented Sep 25, 2023

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
Copy link
Owner

daaain commented Nov 4, 2023

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!

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

No branches or pull requests

4 participants