Update handlebars for htmlbars compat

[patricklx]

WIP, needs some help/review and discussion :) current htmlbars handlebars goes to commit https://github.com/wycats/handlebars.js/commit/3238645f260a77b02baa9b686cf7d5de3a7d2694

I started by renaming AST names, which maybe wasn't the best idea, didn't expect many changes summary/todo:

• [x] update AST names https://github.com/wycats/handlebars.js/commit/6a7a8c803d48b8a5768d00e8d1c73cf5012106d2
• [x] check if replacing ast names didnt have any sideeffect (was search and replace)

By AST:

• [x] Program
  • [x] inverseStrip, inverse removed. strip added https://github.com/wycats/handlebars.js/commit/eee2c4d4f29e233280907bc89a32556de66fe783
  • [ ] blockParams addded https://github.com/wycats/handlebars.js/commit/b8a9f7264d3b6ac48514272bf35291736cedad00
• [x] MustacheStatement
  • [x] hash parameter removed https://github.com/wycats/handlebars.js/commit/5cfe6a0be16a912a5f00af251753a3d5746a6577
  • [x] add sexpr and escaped parameter https://github.com/wycats/handlebars.js/commit/8a6796e5c09686b47945a35826d77680d589d07c
• [x] BlockStatement
  • [x] close parameter replaced by openStrip, inverseStrip, closeStrip https://github.com/wycats/handlebars.js/commit/eee2c4d4f29e233280907bc89a32556de66fe783 https://github.com/wycats/handlebars.js/commit/928ba56b9577fd6cd874f0a83178f1265a6d0526
  • [x] BlockNode now receives subexpression ? https://github.com/wycats/handlebars.js/commit/12e3d210e8b3b22d19823e59bb789dc8768eb34b
• [x] PartialStatement
  • [x] parameters "partialName, context, hash" replaced by sexpr (https://github.com/wycats/handlebars.js/commit/2a4819d75cba7513946af5cf28ee22881561814f)
  • [x] old school partials
• [x] SubExpression
  • [x] receives path as first argument and not in rawParams https://github.com/wycats/handlebars.js/commit/8a6796e5c09686b47945a35826d77680d589d07c
• [x] PathExpression
  • [x] add depth and original parameter https://github.com/wycats/handlebars.js/commit/8a6796e5c09686b47945a35826d77680d589d07c
  • [x] replaces IdNode and DataNode
  • [x] add data parameter
• [x] Hash
  • [x] parts changed to {key,val}

[machty]

@patricklx this is looking pretty good; are there any changes you'd like to make to it before I merge it in?

[patricklx]

I think its ok. But block parameters are not supported, and htmlbars is already using them. But maybe not so important right now? I am using ember canary, i had to adapt the emblem-ember-cli compiler to use the emblem ast and input it to the htmlbars compiler. I was able to make it work with custom ast plugins for each and with. But i think it works with normal EmberHandlebars, but better check again

[lai]

I'm really excited about what's happening here. @patricklx are you planning on adding the block param syntax soon? I took a look at wycats/handlebars.js@b8a9f72 and if you're not on it yet I can give a try hopefully this week.

[machty]

@patricklx I'm realizing something that may not bode very well for this PR... there are now three major Handlebars AST versions, and version 1 had breaking changes, and it's exhausting (and not very fruitful honestly) to depend on an ever-moving Handlebars AST when maybe it makes more sense for Emblem to output Handlebars text, which hasn't had a breaking changing throughout Handlebars' lifetime. I've run this idea by a few other Ember core team members (and Handlebars maintainers) and it seems like it might be the better way to go.

It would mean that I wouldn't be able to use the code in this PR, so I wanted to take this opportunity to get your opinion on this and make sure I'm not missing some crucial detail before I make a rash decision; I appreciate the time you've put into prepping Emblem for the HTMLbars move, but I want to make sure we're not setting ourselves up for a black hole investment in maintaining an Emblem-Hbs dependency hell.

[patricklx]

Yes, i had the same feeling while working on this.

I think the best would be to get rid of any handlebars ast inside of emblem and directly compile to html+handlebars expression.

I think that would be way more maintainable.

[mmun]

By html+handlebars expression you mean Handlebars source code, (e.g. the string Hello {{name}}), right?

[patricklx]

Exactly So div hello {{name}} would be compiled to <div>hello {{name}}</div>

[lai]

If I understand correctly, Emblem still need to add the block param syntax support regardless what the output will be. It seems to me the amount of work of changing the output to Handlebars template is much more than just adding block params to this PR and things will just work now.

@machty what do you think of adding block params support? do you expect this to take place before the output refactor? and lastly, is there a time frame the output change can happen? (I don't really expect an answer for this last one since people are working on this on their own time and I'm grateful this work can benefit the whole community.)

[piotrpalek]

I am with @lai here, not to pressure anyone (or sound ungrateful) but I just think that this PR is "almost done", and it would be a waste to not add block params (which would benefit many people, including me of course :) ) and change the inner workings after that.

[patricklx]

How is it? @lai, are you working on blockParams? Somehow i thought blockParams are a requirement for using htmlbars, but they are not. We just need an extra step to transform each and with.

I adapted the broccoli-emblem-compiler . I took another approach in the each/with-transform... well, i thought only blockParams work. Maybe this can be done better, using the ember AST plugins?

Anyway, by creating handlebars source we could also go around this issue?

Is someone already working on creating a Emblem->handlebars source version?

[mmun]

The Handlebars and HTMLBars AST are not stable yet. The string strategy seems correct to me at least until then.

[machty]

@patricklx I'm working on the Emblem->Hbs source. I don't think I can use any code from this PR, so closing for now. I really really really appreciate the work and wished we'd figured it out sooner that targeting Handlebars text was the better way to go.

[patricklx]

ok, no problem! While your at it, can you try to make error messaging better?

From what i saw, the problem is that a whole line will be matched at once, if one part fails, all fails. E.g. We can immediately detect if the beginning of the line is a tag or a helper. But if something fails it should throw an error and not try other matches. Maybe this could be easily done in the new implementation?

[machty]

A lot of that is tied to the PEG parser; could you take a look at PEG.js and see if there's been some improvements in this area?

[patricklx]

ok. I did read something some days ago: here it is: https://github.com/pegjs/pegjs/issues/262 PEGjs will backtrack to the last named rule! Which currently is beginStatement. I just did some changes to emblem, removed the named rules BeginStatement and contentStatement and named some others-> way better error message

[machty]

Nice... would be interested in a PR for that if it definitely improves the experience :)