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

Quotes in curlies confuse highlighter #84

Closed vitch closed 7 years ago

vitch commented 8 years ago

The following is valid handlebars (in emberjs):

{{concat prop1 ' ' prop2}

But it seems to confuse the highlighter in Sublime:

screen shot 2016-06-24 at 14 57 13

The content of the page after this is also confused (it seems to think it is still inside a single quote).

daaain commented 8 years ago

Hey @vitch!

Hmm, first time I've seen this :)

Do you have a link to some (official) documentation about this feature?

Without checking the code, my guess is that the regular expression expects some content other than whitespace inside the quotes.

vitch commented 8 years ago

Hey - thanks for the reply :)

There is a similar example in the ember documentation: https://guides.emberjs.com/v2.6.0/templates/built-in-helpers/#toc_nesting-built-in-helpers

And I think that your guess is spot on:

screen shot 2016-06-26 at 13 09 26

The docs on the concat helper more specifically give this example:

{{some-component name=(concat firstName " " lastName)}}

Funnily enough, it seems that one confuses GitHub's syntax highlighting as well 😸

It seems like there are issues as well as whitespace - you can see an html entity also breaks it (I tried a non-breaking space to work around it):

screen shot 2016-06-26 at 13 12 02

If you can point me at the relevant regular expression I can try to tweak it. Or I can at least add a failing test if that would help?

daaain commented 8 years ago

All right, so it sounds like there should be a pattern definition for the different types of whitespace and HTML entity characters which are valid for this use case. The biggest help would be if you could list these.

The single and double quote definitions are here: https://github.com/daaain/Handlebars/blob/master/grammars/Handlebars.json#L648 https://github.com/daaain/Handlebars/blob/master/grammars/Handlebars.json#L488

To make this work, something which defines whitespace should be included in patterns for both.

There's a short guide about testing and contribution in the readme: https://github.com/daaain/Handlebars#testing--contribution

I'll give you a guess which syntax definition library Github uses for Handlebars ;)

vitch commented 8 years ago

there should be a pattern definition for the different types of whitespace and HTML entity characters which are valid for this use case. The biggest help would be if you could list these.

If I could list the valid whitespace and HTML entity characters that are valid within a string. I think this is "anything except a linebreak or an endquote" (e.g. a blacklist would be more efficient that a whitelist). Does that make sense or am I missing the point?

Thanks for showing me where the changes need to go and for the instructions on how to hack it - I'll have a go at it if you can let me know if the comment above makes sense or not...

daaain commented 8 years ago

In some way it's already like that, the quote definitions I linked only specify the starting and finishing quotes. The issue is that the inside still needs to be parsed (you can have escaped quotes, comments, etc), so that's why a simple blacklist wouldn't work. That said, it might still be possible to put a blacklist like, catch-all pattern on the bottom.

daaain commented 7 years ago

I had a bit of play tonight, and as it turns out (poring over scopes) the problem wasn't with the quotes but the whitespace at the end of attribute values which means that it captured the opening quote. Anyway, this problem is fixed now, hopefully I didn't break something else :)

vitch commented 7 years ago

Awesome - thanks so much. Sorry I didn't get a chance to try to do it myself!