WIP: Update handlebars syntax for Glimmer and Ember syntaxes #3099

NullVoxPopuli · 2021-04-03T14:48:38Z

It seemed that the handlebars syntax here is pretty old, this PR aims to update it to all the features currently available to Glimmer and Ember projects

Changes

implement new syntax identification:
- modifiers
- angle bracket components (previously, they were just XML elements)
- named blocks
- arguments differentiated from attributes
new helpers:
- let
- fn
- has-block
- everything from Adding Equality Operators to Templates emberjs/rfcs#560, Adding Numeric Comparison Operators to Templates emberjs/rfcs#561, Intent to RFC: Svelte Flags emberjs/rfcs#532
- ember-truth-helpers
new modifiers:
- on
changed classification
- for Helpers, (previously "builtins"), these have been changed from name to function, because helpers are functions.

Checklist

Added markup tests, or they don't apply here because...
Updated the changelog at CHANGES.md

joshgoebel · 2021-04-03T15:20:34Z

src/languages/handlebars.js

+        className: 'template-tag',
+        begin: /<:?/,
+        end: />/,
+        contains: [ OPENING_BLOCK_MUSTACHE_CONTENTS, BLOCK_PARAMS, KEYWORD_CONTENTS ]


You can't put an empty block like KEYWORD_CONTENTS here, it'll do nothing because it has no match or begin tags. If you ant to match keywords here you'd put keywords at the top-level.

Suggested change

className: 'template-tag',

begin: /<:?/,

end: />/,

contains: [ OPENING_BLOCK_MUSTACHE_CONTENTS, BLOCK_PARAMS, KEYWORD_CONTENTS ]

className: 'template-tag',

keywords: // keywords go here

begin: /<:?/,

end: />/,

contains: [ OPENING_BLOCK_MUSTACHE_CONTENTS, BLOCK_PARAMS ]

joshgoebel · 2021-04-03T15:21:08Z

@nknapp Would you be around to help review this when it's ready?

joshgoebel · 2021-04-03T15:25:53Z

If you get overwhelmed here I'd suggest to start with smaller chunks (ie, try to accomplish these things a few at a time) vs a huge PR that does it all. For example you could perhaps clean up the keyword situation first, without trying to add any new tags, etc... so far the PR is "ok" (clean enough for review) but if it gets too complex then we'd ask you to break it into smaller pieces anyways just so we can see what you did and be able to properly review it, etc.

But since this is your first commit and "idk what I'm doing" I'd suggest taking it slow and working in small chunks. (which can be more easily reviewed and merged - which will build momentum and a sense of achievement).

Food for thought.

joshgoebel · 2021-04-03T15:26:44Z

You'll likely need to add $pattern back at some point... may want to review the commit log to see why it was added and what problem it's solving.

joshgoebel · 2021-04-03T15:27:44Z

Also, is this official handlebars are are these Ember and Glimmer specific extensions?

joshgoebel · 2021-04-03T15:28:54Z

src/languages/handlebars.js

@@ -324,6 +333,12 @@ export default function(hljs) {
        end: /\}\}/,
        keywords: 'else if'
      },
+      {
+        className: 'bulit_in',


joshgoebel · 2021-04-03T15:31:00Z

src/languages/handlebars.js

-Website: https://handlebarsjs.com
-Category: template
+Website: https://handlebarsjs.com, https://emberjs.com
+Category: common, template


Please revert, this is not your decision to make - which languages are in our common set. If you just want to add handlebars to the build your an do so on the console:

./tools/build -t node :common handlebars

Etc. Builds common + handlebars.

Also what is embers relationship to Handlebars?

Also what is embers relationship to Handlebars?

ember has similar syntax, but has built a bunch of stuff on top, as a result, the handlebars syntax in highlight.js ~7 years out of date :(
(but that's why I'm making a PR!) :D

as a result, the handlebars syntax in highlight.js ~7 years out of date

I'm not sure that's a correct statement [in the general sense]. I know @nknapp put a lot of work into it just last year to improve it.

https://handlebarsjs.com

AFAIK Handlebar exists as it's own independent project and there are surely those using it outside of Ember - like our very own project, actually. So the question then becomes whether this is appropriate to be added to Handlebars at all or whether a separate Ember grammar may be needed. I'm curious if nknapp has any thoughts on that.

In any case I can certainly see how this would be helpful for Ember users, but if we decide it's not appropriate as part of Handlebars proper then it would need to be distributed as a 3rd party Ember grammar - rather than mixed with our existing support.

Yeah that's totally fair! Most editors have an ember-variant to use for hbs highlighting

joshgoebel · 2021-04-03T15:32:21Z

src/languages/handlebars.js

@@ -205,7 +205,7 @@ export default function(hljs) {
  };

  const SUB_EXPRESSION_CONTENTS = hljs.inherit(HELPER_NAME_OR_PATH_EXPRESSION, {
-    className: 'name',
+    className: 'function',


Please make sure that your PR explanation justifies (visual) changes like this and explains why they are necessary...

can do!

In this particular case, helpers are functions (and take args), and not actually just a name, like you'd have with the name of a Component invocation.

NullVoxPopuli · 2021-04-03T15:35:49Z

Thanks for the early comments! these are helpful!!

You'll likely need to add $pattern back at some point... may want to review the commit log to see why it was added and what problem it's solving.

I'll check it out 👍

Also, is this official handlebars are are these Ember and Glimmer specific extensions?

handlebars doesn't have components, so about everything I'm doing is going to be on top of handlebars, but for ember and glimmer (they use the same hbs file extension though).

I saw there was an HTMLBars syntax, but it looked like it was going to be removed.

I can move all of this into a new ember/glimmer specific syntax, if that's desired and try extending from handlebars.

NullVoxPopuli · 2021-04-03T15:43:07Z

I have a test template that I'm working through here in the developer.html:

(taken from: joukevandermaas/vim-ember-hbs#13)

My plan was to get the whole thing looking correct before taking the PR out of draft.
Most of it should "just" be adding component invocation support

joshgoebel · 2021-04-03T15:46:40Z

I saw there was an HTMLBars syntax, but it looked like it was going to be removed.

Correct, also it's only an alias of Handlebars at this point.

I can move all of this into a new ember/glimmer specific syntax, if that's desired and try extending from handlebars.

That is indeed the question. Though at that point it becomes a 3rd party module so I wouldn't worry about "extending" anything (you could, but we don't really encourage or support run-time dependencies)... at that point you'd just be "forking" the grammar essentially and then carrying it forward as Ember/Glimmer specific. But perhaps that's all you meant by "extending" in the first place. :-)

NullVoxPopuli · 2021-04-03T16:24:41Z

I'm gonna move all this to a new file, called glimmer.js, and that way we don't need to worry about handlebars maintenance / maintainers also having to deal with ember/glimmer stuff.
It also gives me a chance to start over, and make sure only what's needed is implemented -- handlebars has some features that ember/glimmer no longer has

joshgoebel · 2021-04-03T16:34:58Z

handlebars has some features that ember/glimmer no longer has

This definitely seems to seal the deal that this is an entirely separate grammar, not Handlebars (though I was already leaning that way in my thinking).

At this point you'd be creating a 3rd party grammar module (in your own repository) so I'd check out our documentation on doing that:

https://github.com/highlightjs/highlight.js/blob/main/extra/3RD_PARTY_QUICK_START.md

NullVoxPopuli · 2021-04-03T16:51:17Z

Cool! thanks for having all this documentation written up!

joshgoebel reviewed Apr 3, 2021

View reviewed changes

Prepare for a lot of changes

c166a8f

Component Invocations work work appropriately

1a16272

NullVoxPopuli force-pushed the update-handlebars branch from 33086b5 to 1a16272 Compare April 3, 2021 15:27

joshgoebel reviewed Apr 3, 2021

View reviewed changes

Revert category change

e4c2470

NullVoxPopuli closed this Apr 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Update handlebars syntax for Glimmer and Ember syntaxes #3099

WIP: Update handlebars syntax for Glimmer and Ember syntaxes #3099

NullVoxPopuli commented Apr 3, 2021 •

edited

Loading

joshgoebel Apr 3, 2021

joshgoebel commented Apr 3, 2021

joshgoebel commented Apr 3, 2021

joshgoebel commented Apr 3, 2021

joshgoebel commented Apr 3, 2021

joshgoebel Apr 3, 2021

joshgoebel Apr 3, 2021 •

edited

Loading

NullVoxPopuli Apr 3, 2021

joshgoebel Apr 3, 2021

NullVoxPopuli Apr 3, 2021

joshgoebel Apr 3, 2021

NullVoxPopuli Apr 3, 2021

NullVoxPopuli commented Apr 3, 2021

NullVoxPopuli commented Apr 3, 2021

joshgoebel commented Apr 3, 2021

NullVoxPopuli commented Apr 3, 2021

joshgoebel commented Apr 3, 2021 •

edited

Loading

NullVoxPopuli commented Apr 3, 2021

WIP: Update handlebars syntax for Glimmer and Ember syntaxes #3099

WIP: Update handlebars syntax for Glimmer and Ember syntaxes #3099

Conversation

NullVoxPopuli commented Apr 3, 2021 • edited Loading

Changes

Checklist

joshgoebel Apr 3, 2021

Choose a reason for hiding this comment

joshgoebel commented Apr 3, 2021

joshgoebel commented Apr 3, 2021

joshgoebel commented Apr 3, 2021

joshgoebel commented Apr 3, 2021

joshgoebel Apr 3, 2021

Choose a reason for hiding this comment

joshgoebel Apr 3, 2021 • edited Loading

Choose a reason for hiding this comment

NullVoxPopuli Apr 3, 2021

Choose a reason for hiding this comment

joshgoebel Apr 3, 2021

Choose a reason for hiding this comment

NullVoxPopuli Apr 3, 2021

Choose a reason for hiding this comment

joshgoebel Apr 3, 2021

Choose a reason for hiding this comment

NullVoxPopuli Apr 3, 2021

Choose a reason for hiding this comment

NullVoxPopuli commented Apr 3, 2021

NullVoxPopuli commented Apr 3, 2021

joshgoebel commented Apr 3, 2021

NullVoxPopuli commented Apr 3, 2021

joshgoebel commented Apr 3, 2021 • edited Loading

NullVoxPopuli commented Apr 3, 2021

NullVoxPopuli commented Apr 3, 2021 •

edited

Loading

joshgoebel Apr 3, 2021 •

edited

Loading

joshgoebel commented Apr 3, 2021 •

edited

Loading