ractivejs / ractive

Next-generation DOM manipulation
http://ractive.js.org
MIT License
5.94k stars 396 forks source link

Allow extra space in mustache templates for block elements #983

Closed kurdin closed 10 years ago

kurdin commented 10 years ago

In templates, I have a habit to add extra space before {{ and }} like this {{ title }} this usually helps to mix mustache front end templates with backend dusts js templates. Dust will ignore curly brackets with space between content. Ractive template works fine inside dustjs server template in most of the cases with adding spaces but this is throw error:

    <h2>Articles</h2>
    {{ #each pages.articles }}
      <article>
        <h3>{{ title }}</h3>
        <p>Posted by {{author}}</p>
        <p>{{ body }}</p>
      </article>
    {{ /each }}  <= error because of extra space before and after /each 
    {{ >accessError }}

all endings of block elements in ractive templates must have no space, everything else work fine with extra spaces, this would work fine:

{{ #if loggedIn }}
      <p>Hey <b>{{ user.username }}</b>! You are already logged in.</p>
      <a href="#/logout" on-click="logout">Log out</a>
    {{ else }}
      {{ >login_form }}
      {{ #if errorMessage }}
        <div class="alert alert-error" intro="fade">{{ errorMessage }}</div>
      {{/if}}
 {{/if}}

can we fix this, so I can use {{ /each }} or {{ /if }} in ractive templates.

MartinKolarik commented 10 years ago

This applies only to handlebars syntax and I guess it's because handlebars.js doesn't support it. That said, I don't see a reason not to support it in Ractive.

kurdin commented 10 years ago

I was under impression that Ractive has its own mustache template parser and does not use handlebars.js for anything. So I guess it does not matter if handlebars does not support it, Ractive can add this.

MartinKolarik commented 10 years ago

That's right, Ractive has it's own parser and as I said above, I don't see a reason not to support it as it won't break backwards compatibility.

jonvuri commented 10 years ago

The problem with this is that it could be hard to differentiate between a closing mustache and a mustache containing an expression starting with a regex.

MartinKolarik commented 10 years ago

@jrajav you can't have RegExp in an expression.

jonvuri commented 10 years ago

Oh. Didn't know that. ... Is that just because it would conflict with a closing mustache?

kurdin commented 10 years ago

I think doing RegExp in templates is defiantly overkill. You should not do that for any reasons.

MartinKolarik commented 10 years ago

@jrajav I guess it might be one of the reasons.

"Only a subset of the infinite range of JavaScript expressions is supported (albeit an infinite subset...) - those which do not use assignment operators (such as = or ++), regular expressions, function literals, or new/delete/void operators. This is to ensure security and prevent side-effects." https://github.com/ractivejs/template-spec

Rich-Harris commented 10 years ago

Security and side-effects is the stated reason - if you had a long list of items, and were testing a string against a regex in each section, you'd end up recreating the same regex n times which probably isn't ideal (though to be fair browser regex engines are blazing fast).

If I'm completely honest, the main reason is that I couldn't face writing a regex parser! (Does anyone know of a regex that tests for a valid regex...?) But the ambiguity with closing sections is an excellent reason by itself - we're already jumping through enough hoops to distinguish between

{{! comment, ignore me}}

and

{{!ready ? 'set' : 'go'}}

so I lean towards keeping regexes banned.

Thinking about this issue, do we want to be more forgiving generally with whitespace? E.g.

{{ # if foo }}...{{ / if }}

I'm about to submit a PR that allows exactly that - good news is it doesn't seem to break anything else. So it's just a question of whether we think it's a good idea.

kurdin commented 10 years ago

Thinking about this issue, do we want to be more forgiving generally with whitespace? E.g.

Personally, I don't think that its more readable with extra spaces but someone might find this useful.

MartinKolarik commented 10 years ago

Thinking about this issue, do we want to be more forgiving generally with whitespace?

Before submitting #985, I thought it was supported. Found out it isn't while hacking on this. Basically agree with @kurdin.

Rich-Harris commented 10 years ago

Forgot to actually submit that PR last night, it's #1029

Rich-Harris commented 10 years ago

Had a chance to reflect on this, I think allowing crazy whitespace combinations is a case of YAGNI that we might regret later. Will close the second PR and merge @MartinKolarik's (I thought it was already merged - my bad)