search

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

Apostrophes within a script break the syntax highlighting thereafter #68

Closed jamesl1001 closed 8 years ago

jamesl1001 commented 9 years ago

handlebarshighlightingbug

jamesl1001 commented 8 years ago

For anyone wanting a temporary workaround for this issue, escape the apostrophe with '.

daaain commented 8 years ago

Hey @jamesl1001,

Really sorry for not replying anything yet, I did manage to reproduce the error, but won't get around to have a go at fixing it until next week the earliest – I have a two weeks old newborn next to me and will speak at re:develop conference this Friday...

daaain commented 8 years ago

Well, even though I remember seeing this issue happening, now embarrassingly I can't reproduce again. Would you mind pasting a full reproduction here?

Also, just realised you're using type="x-tmpl-handlebars", any particular reason for that? Since the script tag can contain different stuff, any type which should be treated as Handlebars needs to be whitelisted, otherwise it'll be rendered as however the text editor's default HTML package would treat it. The current whitelist is: (?:text/x-handlebars-template|text/x-handlebars|text/template).

jamesl1001 commented 8 years ago

Ahh yes, the script type was the problem!

I've tested all three types from your whitelist and they all handle the apostrophe perfectly.

The reason I was using type="x-tmpl-handlebars" was because half way through development, I switched from using Mustache. In their documentation they use type="x-tmpl-mustache", so I simply changed 'mustache' thinking nothing of it!

Maybe it would be an idea to add this script type to the whitelist to catch this edge case for anyone who migrates from Mustache to Handlebars in the future?

I have opened PR #70 above if you would like to make this change :)

daaain commented 8 years ago

Of course it's fine adding this, I just merged your PR and created a new version, so the update should be live both for Atom and Sublime.

Thanks for taking your time to create a PR!

jamesl1001 commented 8 years ago

Brilliant! Just checked it in Sublime Text 3 and it's working just fine!