leshill / handlebars_assets

Use handlebars.js templates with the Rails asset pipeline.
MIT License
648 stars 159 forks source link

Handlebars conditionals don't work as expected #62

Open wulftone opened 11 years ago

wulftone commented 11 years ago

So after some fiddling, I figured out that handlebars conditionals will work if you do this:

| {{#if something}}
h1 This works
| {{/if}}

But why not this?:

{{#if something}}
h1 This does not work
{{/if}}

The above gives me this error:

Unknown line indicator
  (__TEMPLATE__), Line 1, Column 0
    {{#if isNew}}
    ^

I don't know if this is the intended behavior or not... but it seems odd.

AlexRiedler commented 11 years ago

The first works because the handlebars in slim needs to be treated like text; how slimbars is compiled is basically:

Pass through slim compiler; than through the handlebars precompiler.

slim compiler is trying to determine what "{{" is and it does not make sense to be an element to it.

If you wrote a "slim+handlebars" combined compiler you would be able to do what you are indicating; it is not simplistic due to cases like these:

| {{# if foo}}
h1
| {{else}}
h2
| {{/fi}
    p
    | holy moly, how do I know I am suppose to be indented or not hmmm

as the slim compiler will think p is accidentally indented cause of the text of the "{{/fi}}"

If you are up to writing a combined compiler, or know someone that has we would be willing to integrate it :)

AlexRiedler commented 11 years ago

In an ideal world; I would want to make slim/haml aware of handlebars blocks: e.g.

{{#if foo}}
    h1
{{else}}
    h2
{{/fi}}
    p
    | yes this would work

however that is still not clear whether the p element is contained in the h1/h2 or not.

wulftone commented 11 years ago

Haha yeah, I realize it isn't a trivial problem. I think it's worth highlighting though. I didn't catch it anywhere in the docs, which led to the confusion. Right now it doesn't bother me enough to warrant that additional workload, but maybe someone has the time/desire?

leshill commented 11 years ago

Hi @wulftone,

@AlexRiedler has this right. The slim integration is shallow, we pass through slim and then handlebars. This seems like a syntax incompatibility.

AlexRiedler commented 11 years ago

https://github.com/jamesotron/hamlbars is sorta what you want but for slim, correct?

wulftone commented 11 years ago

Yeah I think so, but on an aesthetic point, the reason I use slim is because it's so ridiculously easy to read and I wouldn't want to ruin that with awkward syntax... handlebars_assets fits the bill really nicely--or about 98% nicely. : )

I considered using hamlbars, but if you look at their documentation, they make some syntactic concessions "due to the nature of Haml".

AlexRiedler commented 11 years ago

If you can think of a nice way to integrate (even just syntactically) slim with JS templating engines I would be really interested; I haven't invested much time into thinking about it. (I personally use HAML)

wulftone commented 11 years ago

Here's one idea:

h1 MyPage - {{subtitle}}

{{#if currentUser}}
h2 Welcome {{currentUser.name}}!
div
{{else}}
div
  | Displayed if no user
  form action="{{formAction}}"
    / form inputs, etc.
{{/if}}
  | This gets put inside the parent div no matter what.
  div
    | This is also inside
    {{#if something}}
    span Do wah ditty ditty {{thisWorksToo}} dum ditty do
    {{/if}}

footer This is back at the body-level, like the first `h1`.

No indents really needed inside the if statements, they stand out enough as is, and I think it would probably just confuse slim to no end. With this scheme, I think slim would need to convert lines that start with {{ as a text command (| or ') that hasn't been indented properly. (when normally it would've)...?

The alternate version you demonstrated that keeps the indentation within the conditionals seems okay at first glance, but you're right in that it would be confusing, since it would have to consider all contents at an indentation level of "one less indent" than they appear... and it would just be inhibiting when you, as a human, are trying to parse in your head what is happening. So I don't think that's a good idea.

In a case where someone makes a mistake, like

{{#if blah}}
p
  ' Stuff
{{/if}}
  span I don't know what I'm doing!

It should throw an error, because why didn't they put that span inside the conditional? More likely they intended this:

p
  ' {{if blah}}Stuff{{/if}}
  span I do know what I am doing

And that works right now anyway.

After some experimentation, I discovered some things. This below works as expected:

| {{#if title}}
h1
  | Hey
  | {{else}}
h2
  | Ho
  | {{/if}}
  p holy moly, this actually works!

But that doesn't really suit slim very well, even it it works. The next is more interesting:

| {{#if title}}
h1 Hey
| {{else}}
h2
  | Ho
  | {{/if}}
  p holy moly, how do I know I am suppose to be indented or not hmmm

...results in either

<h1>Hey</h1>
<p> holy moly...</p>

or

<h1>Ho
  <p>holy moly...</p>
</h1>

and then there's this, which is syntactically close to what I'd like to do (without the pipes), but it gets confused:

| {{#if title}}
h1 Hey
| {{else}}
h2 Ho
| {{/if}}
  p holy moly, how do I know I am suppose to be indented or not hmmm

results in the following HTML:

<h1>Hey</h1>
p holy moly...

...when it "should've" worked.

At least this shows that it's possible as it stands, even if it is a bit cumbersome. : D

I tried to realize the example at the top of the page and got this:

h1 MyPage - {{title}}

| {{#if title}}
h2 Welcome {{title}}!
div
  | {{else}}
div
  ' Displayed if no user
  form class="{{noUser}}"
    / form inputs, etc.
  | {{/if}}
  | This gets put inside the parent div no matter what.
  div
    ' This is also inside
    | {{#if title}}
    span class="{{title}}" Do wah ditty ditty {{title}} dum ditty do
    | {{/if}}

p This is back at the body-level, like the first `h1`.

This, amazingly, works, and obviously it's ugly as hell. I guess this is why people like keeping conditionals out of HTML... hahah. But it is nice to know it is possible.

EDIT: Fixed some syntax errors

rpocklin commented 10 years ago

+1 [slim/haml]bars + handlebars indentation support would be awesome. Agree it's probably not simple :(