New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add template inheritance spec #75

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@jazzdan

jazzdan commented Apr 14, 2014

This adds a specification for hogan.js style template inheritance. As of time of writing, three mustache implementations are 100% compatible with this implementation:

Here's how it works:

{{! header.mustache }}
<head>
  <title>{{$title}}Default title{{/title}}</title>
</head>
{{! base.mustache }}
<html>
  {{$header}}{{/header}}
  {{$content}}{{/content}}
</html>
{{! mypage.mustache }}
{{<base}}
  {{$header}}
    {{<header}}
      {{$title}}My page title{{/title}}
    {{/header}}
  {{/header}}

  {{$content}}
    <h1>Hello world</h1>
  {{/content}}
{{/base}}

Rendering mypage.mustache would output:

<html><head><title>My page title</title></head><h1>Hello world</h1></html>

A full specification can be seen below.

At @etsy, much like @twitter (see #38), we often want to have one template where we maintain the "bones" of the page and each real page on the site merely replaces sections of that page with appropriate content. A simple example of this can be seen above, where a page can grab base, getting all of our analytics scripts and other markup that every page needs, while retaining the ability to swap in a completely different header. Alternatives, such as copying the bones to every top level template, or simulating this behavior with a multi-step mustache render on the server, are not ideal.

Etsy is in the process of rolling out the new version of mustache.php that supports template inheritance to our production cluster after testing this implementation extensively. Hogan.js and mustache.java are both being used in production at Twitter.

Many thanks to @dtb, @mdg, @bobthecow for their assistance. Special thanks to @spullara for quickly making mustache.java compatible with this spec.

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Apr 14, 2014

Member

Hey, @groue — I remember you saying GRMustache was moving to this style of inheritance as well... Do you mind running this spec against it and seeing how it works?

Member

bobthecow commented Apr 14, 2014

Hey, @groue — I remember you saying GRMustache was moving to this style of inheritance as well... Do you mind running this spec against it and seeing how it works?

@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 15, 2014

Hi @bobthecow. Yes, GRMustache passes all those tests (modulo white-space conformance, that GRMustache has never focused on).

Generally speaking, GRMustache passes all inheritance tests from this PR, from hogan.js, and spullara/mustache.java.

I'm curious about your own implementations' conformance to GRMustache tests as well - they cover a few cases not included in this PR:

groue commented Apr 15, 2014

Hi @bobthecow. Yes, GRMustache passes all those tests (modulo white-space conformance, that GRMustache has never focused on).

Generally speaking, GRMustache passes all inheritance tests from this PR, from hogan.js, and spullara/mustache.java.

I'm curious about your own implementations' conformance to GRMustache tests as well - they cover a few cases not included in this PR:

@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 15, 2014

@jazzdan wrote:

As of time of writing, three mustache implementations are 100% compatible with this implementation:

There are four or them, actually :-) You can add groue/GRMustache.

groue commented Apr 15, 2014

@jazzdan wrote:

As of time of writing, three mustache implementations are 100% compatible with this implementation:

There are four or them, actually :-) You can add groue/GRMustache.

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Apr 15, 2014

Member

Your third bullet (separate namespace for inheritance vs. regular variable keys) was addressed in the PHP implementation.

Member

bobthecow commented Apr 15, 2014

Your third bullet (separate namespace for inheritance vs. regular variable keys) was addressed in the PHP implementation.

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow
Member

bobthecow commented Apr 15, 2014

@groue This one is interesting, and I'm not sure I'm convinced it's the right way to treat it:

https://github.com/groue/GRMustache/blob/GRMustache7/src/tests/Public/v7.0/Suites/groue:GRMustache/GRMustacheSuites/inheritable_partials.json#L59-L64

Sell me on it :)

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Apr 15, 2014

Member

I just tested the PHP implementation against your second bullet point (several conflicting inheritable sections), and that works as well.

Member

bobthecow commented Apr 15, 2014

I just tested the PHP implementation against your second bullet point (several conflicting inheritable sections), and that works as well.

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Apr 15, 2014

Member

Okay. Your inheritable partials tests mostly pass in the PHP implementation (2 failures, 5 errors). The two failures are due to treatment of blocks inside sections (my "not convinced" comment above).

The five errors are exceptions thrown because Mustache.php, like Hogan.js, throws errors for things like this:

{{< parent }}
  {{# section }}this will throw an exception!{{/ section }}
{{/ parent }}
Member

bobthecow commented Apr 15, 2014

Okay. Your inheritable partials tests mostly pass in the PHP implementation (2 failures, 5 errors). The two failures are due to treatment of blocks inside sections (my "not convinced" comment above).

The five errors are exceptions thrown because Mustache.php, like Hogan.js, throws errors for things like this:

{{< parent }}
  {{# section }}this will throw an exception!{{/ section }}
{{/ parent }}
@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 15, 2014

Thanks for your time @bobthecow :-)

https://github.com/groue/GRMustache/blob/GRMustache7/src/tests/Public/v7.0/Suites/groue:GRMustache/GRMustacheSuites/inheritable_partials.json#L59-L64

Sell me on it :)

Generally speaking, I tend to think that this behavior is required by those two arguments:

  • The content of inheritable sections is rendered in the current context: jazzdan@ab22750#diff-d3a1efe06cc23358b500e2bdab06504eR22
  • the rendering of inheritable sections is triggered by the layout. It may be overriden or not. The what is decided by inherited templates, but the when is decided by the layout. This "moment" may be inside a regular mustache section, with some particular context.

The two arguments join on the "context" word: if the layout template changes the context, inherited sections need to render in this context. Hence the test you question, and the example below:

layout.mustache

{{!--
Layout for a document.

Exposed inheritable sections:
- $title: What to be rendered for the document title.
          When rendered, the `document` object is at the top of the context stack.
          Default value: `document.title`.
}}
{{#document}}
    {{$title}}
        {{title}}
    {{/title}}
    ...
{{/document}}

essay.mustache

{{<layout}}
    {{$title}}
        {{! Prepend "Essay" to the document title }}
        Essay: {{title}}
    {{/title}}
{{/layout}}

Rendering:

render('essay', { document: ...});

groue commented Apr 15, 2014

Thanks for your time @bobthecow :-)

https://github.com/groue/GRMustache/blob/GRMustache7/src/tests/Public/v7.0/Suites/groue:GRMustache/GRMustacheSuites/inheritable_partials.json#L59-L64

Sell me on it :)

Generally speaking, I tend to think that this behavior is required by those two arguments:

  • The content of inheritable sections is rendered in the current context: jazzdan@ab22750#diff-d3a1efe06cc23358b500e2bdab06504eR22
  • the rendering of inheritable sections is triggered by the layout. It may be overriden or not. The what is decided by inherited templates, but the when is decided by the layout. This "moment" may be inside a regular mustache section, with some particular context.

The two arguments join on the "context" word: if the layout template changes the context, inherited sections need to render in this context. Hence the test you question, and the example below:

layout.mustache

{{!--
Layout for a document.

Exposed inheritable sections:
- $title: What to be rendered for the document title.
          When rendered, the `document` object is at the top of the context stack.
          Default value: `document.title`.
}}
{{#document}}
    {{$title}}
        {{title}}
    {{/title}}
    ...
{{/document}}

essay.mustache

{{<layout}}
    {{$title}}
        {{! Prepend "Essay" to the document title }}
        Essay: {{title}}
    {{/title}}
{{/layout}}

Rendering:

render('essay', { document: ...});
@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 15, 2014

The five errors are exceptions thrown because Mustache.php, like Hogan.js, throws errors for things like this:

{{< parent }}
  {{# section }}this will throw an exception!{{/ section }}
{{/ parent }}

I see. What is the intent? To prevent users to embed ignored content? If so, do you throw exception for plain text as well? If not, what is the intent of you exception?

{{< parent }}
  Not only sections, but this text also is ignored.
{{/ parent }}

groue commented Apr 15, 2014

The five errors are exceptions thrown because Mustache.php, like Hogan.js, throws errors for things like this:

{{< parent }}
  {{# section }}this will throw an exception!{{/ section }}
{{/ parent }}

I see. What is the intent? To prevent users to embed ignored content? If so, do you throw exception for plain text as well? If not, what is the intent of you exception?

{{< parent }}
  Not only sections, but this text also is ignored.
{{/ parent }}
@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Apr 15, 2014

Member

@groue I'm actually leaning toward that (see comment). The reason it came up was things like this:

{{< parent }}
  {{# items }}
    {{$ block }}wheee!{{/ block }}
  {{/ items }}
{{/ parent }}

… which are weird. I'd be in favor of explicitly whitelisting only a few things that can live in the top level of a parent tag:

  • Whitespace
  • {{! comments }}
  • {{$ blocks }}
  • {{=[[ ]]=}}
Member

bobthecow commented Apr 15, 2014

@groue I'm actually leaning toward that (see comment). The reason it came up was things like this:

{{< parent }}
  {{# items }}
    {{$ block }}wheee!{{/ block }}
  {{/ items }}
{{/ parent }}

… which are weird. I'd be in favor of explicitly whitelisting only a few things that can live in the top level of a parent tag:

  • Whitespace
  • {{! comments }}
  • {{$ blocks }}
  • {{=[[ ]]=}}
@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 15, 2014

@bobthecow All right, I understand, and I'm not against returning errors for all ignored tags, but the ones you are listing: white space, comments, inheritable sections, and change-delimiter tags.

BTW, GRMustache also has tests for such parsing errors: https://github.com/groue/GRMustache/blob/GRMustache7/src/tests/Public/v7.0/Suites/groue:GRMustache/GRMustacheSuites/tag_parsing_errors.json & https://github.com/groue/GRMustache/blob/GRMustache7/src/tests/Public/v7.0/Suites/groue:GRMustache/GRMustacheSuites/expression_parsing_errors.json

Back on your exceptions:

There are only six GRMustache inheritance tests which involve regular sections:

Only 1 of them embeds a regular section inside a inheritable section, and has your implementation throw an exception. Let's forget about this one:

The others should not raise any exception. I mean, your implementation does not have to handle them, but you may be interested in looking at them more precisely:

groue commented Apr 15, 2014

@bobthecow All right, I understand, and I'm not against returning errors for all ignored tags, but the ones you are listing: white space, comments, inheritable sections, and change-delimiter tags.

BTW, GRMustache also has tests for such parsing errors: https://github.com/groue/GRMustache/blob/GRMustache7/src/tests/Public/v7.0/Suites/groue:GRMustache/GRMustacheSuites/tag_parsing_errors.json & https://github.com/groue/GRMustache/blob/GRMustache7/src/tests/Public/v7.0/Suites/groue:GRMustache/GRMustacheSuites/expression_parsing_errors.json

Back on your exceptions:

There are only six GRMustache inheritance tests which involve regular sections:

Only 1 of them embeds a regular section inside a inheritable section, and has your implementation throw an exception. Let's forget about this one:

The others should not raise any exception. I mean, your implementation does not have to handle them, but you may be interested in looking at them more precisely:

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Apr 15, 2014

Member

Those aren't the ones that raised exceptions, these are:

[
  // ...
  {
    "name": "The content of the last inheritable sections in partials is rendered in the inherited section",
    "data": { },
    "template": "{{<partial1}}{{$inheritable}}1{{/inheritable}}{{>partial2}}{{$inheritable}}4{{/inheritable}}{{/partial1}}",
    "partials": {
      "partial1": "{{$inheritable}}ignored{{/inheritable}}",
      "partial2": "{{$inheritable}}2{{/inheritable}}{{$inheritable}}3{{/inheritable}}" },
    "expected": "4"
  },
  {
    "name": "The content of the last inheritable sections in partials is rendered in the inherited section",
    "data": { },
    "template": "{{<partial1}}{{$inheritable}}1{{/inheritable}}{{>partial2}}{{/partial1}}",
    "partials": {
      "partial1": "{{$inheritable}}ignored{{/inheritable}}",
      "partial2": "{{$inheritable}}2{{/inheritable}}{{$inheritable}}3{{/inheritable}}" },
    "expected": "3"
  },
  // ...
  {
    "name": "Partials in inheritable partials can override inheritable sections",
    "data": { },
    "template": "{{<partial2}}{{>partial1}}{{/partial2}}",
    "partials": {
        "partial1": "{{$inheritable}}partial1{{/inheritable}}",
        "partial2": "{{$inheritable}}ignored{{/inheritable}}" },
    "expected": "partial1"
  },
  // ...
  {
    "name": "Templates sections can not override inheritable sections in inheritable partial",
    "data": { "section": true },
    "template": "{{<partial}}{{#section}}inherited{{/section}}{{/partial}}",
    "partials": { "partial": "{{$section}}success{{/section}}" },
    "expected": "success"
  },
  // ...
]

Note that they all include tags besides inheritable sections inside parent tags, which is the current criteria for throwing an exception.

I did just realize that two of these revolve around allowing partials to define inheritable sections. I'm inclined to push back against this one and say only inheritable sections actually defined inside the parent tag count.

Member

bobthecow commented Apr 15, 2014

Those aren't the ones that raised exceptions, these are:

[
  // ...
  {
    "name": "The content of the last inheritable sections in partials is rendered in the inherited section",
    "data": { },
    "template": "{{<partial1}}{{$inheritable}}1{{/inheritable}}{{>partial2}}{{$inheritable}}4{{/inheritable}}{{/partial1}}",
    "partials": {
      "partial1": "{{$inheritable}}ignored{{/inheritable}}",
      "partial2": "{{$inheritable}}2{{/inheritable}}{{$inheritable}}3{{/inheritable}}" },
    "expected": "4"
  },
  {
    "name": "The content of the last inheritable sections in partials is rendered in the inherited section",
    "data": { },
    "template": "{{<partial1}}{{$inheritable}}1{{/inheritable}}{{>partial2}}{{/partial1}}",
    "partials": {
      "partial1": "{{$inheritable}}ignored{{/inheritable}}",
      "partial2": "{{$inheritable}}2{{/inheritable}}{{$inheritable}}3{{/inheritable}}" },
    "expected": "3"
  },
  // ...
  {
    "name": "Partials in inheritable partials can override inheritable sections",
    "data": { },
    "template": "{{<partial2}}{{>partial1}}{{/partial2}}",
    "partials": {
        "partial1": "{{$inheritable}}partial1{{/inheritable}}",
        "partial2": "{{$inheritable}}ignored{{/inheritable}}" },
    "expected": "partial1"
  },
  // ...
  {
    "name": "Templates sections can not override inheritable sections in inheritable partial",
    "data": { "section": true },
    "template": "{{<partial}}{{#section}}inherited{{/section}}{{/partial}}",
    "partials": { "partial": "{{$section}}success{{/section}}" },
    "expected": "success"
  },
  // ...
]

Note that they all include tags besides inheritable sections inside parent tags, which is the current criteria for throwing an exception.

I did just realize that two of these revolve around allowing partials to define inheritable sections. I'm inclined to push back against this one and say only inheritable sections actually defined inside the parent tag count.

@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 15, 2014

All right, you can do as you wish. My take on that point is as stated above:

if a given string is a valid template, then any substring that leaves open/close tags intact can be extracted to a partial, or an inheritable partial. Why? Because one should be able to extract a partial without any worry, as long as the resulting templates are syntactically correct.

groue commented Apr 15, 2014

All right, you can do as you wish. My take on that point is as stated above:

if a given string is a valid template, then any substring that leaves open/close tags intact can be extracted to a partial, or an inheritable partial. Why? Because one should be able to extract a partial without any worry, as long as the resulting templates are syntactically correct.

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Apr 15, 2014

Member

All right, you can do as you wish.

That's not constructive. I'm trying to have a conversation and come to a consensus about common functionality so that we can actually standardize this behavior.

Member

bobthecow commented Apr 15, 2014

All right, you can do as you wish.

That's not constructive. I'm trying to have a conversation and come to a consensus about common functionality so that we can actually standardize this behavior.

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Apr 15, 2014

Member

All right, you can do as you wish. My take on that point is as stated above:

if a given string is a valid template, then any substring that leaves open/close tags intact can be extracted to a partial, or an inheritable partial. Why? Because one should be able to extract a partial without any worry, as long as the resulting templates are syntactically correct.

But that's currently not the case. Consider this template:

{{=<% %>=}}
<% win %>
{{ fail }}

You can't extract any one line of this template into a partial, because the whole thing will break.

I'm just saying that I feel like inheritable sections are a special case too.

Member

bobthecow commented Apr 15, 2014

All right, you can do as you wish. My take on that point is as stated above:

if a given string is a valid template, then any substring that leaves open/close tags intact can be extracted to a partial, or an inheritable partial. Why? Because one should be able to extract a partial without any worry, as long as the resulting templates are syntactically correct.

But that's currently not the case. Consider this template:

{{=<% %>=}}
<% win %>
{{ fail }}

You can't extract any one line of this template into a partial, because the whole thing will break.

I'm just saying that I feel like inheritable sections are a special case too.

@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 15, 2014

That's not constructive. I'm trying to have a conversation and come to a consensus about common functionality so that we can actually standardize this behavior.

I'm sorry. I did not want to pollute @jazzdan's PR. I did state my reason after. Your answer:

But that's currently not the case.

Yes, you are right. Set delimiter tags ruin everything. I wish they were not there. I never use them. They are a hack. That's why I think my take is still valid. And this is why GRMustache tests for snippet-extraction-into-partials at several levels. So that I'm sure my users can extract whatever they want into a partial, as long as it looks sensible to them. Who am I to say it's forbidden? Not implemented, why not. But forbidden? When they have a legitimate need? When it can be implemented in a rigourous way? When it has been implemented?

You may not include those tests in this spec. This is what I meant by "you can do as you wish". But I won't remove this feature from GRMustache.

groue commented Apr 15, 2014

That's not constructive. I'm trying to have a conversation and come to a consensus about common functionality so that we can actually standardize this behavior.

I'm sorry. I did not want to pollute @jazzdan's PR. I did state my reason after. Your answer:

But that's currently not the case.

Yes, you are right. Set delimiter tags ruin everything. I wish they were not there. I never use them. They are a hack. That's why I think my take is still valid. And this is why GRMustache tests for snippet-extraction-into-partials at several levels. So that I'm sure my users can extract whatever they want into a partial, as long as it looks sensible to them. Who am I to say it's forbidden? Not implemented, why not. But forbidden? When they have a legitimate need? When it can be implemented in a rigourous way? When it has been implemented?

You may not include those tests in this spec. This is what I meant by "you can do as you wish". But I won't remove this feature from GRMustache.

@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 15, 2014

About free partials extraction, let me quote @defunkt on http://mustache.github.io/mustache.5.html

In this way you may want to think of partials as includes, or template expansion, even though it's not literally true.

For example, this template and partial:

base.mustache:
<h2>Names</h2>
{{#names}}
  {{> user}}
{{/names}}

user.mustache:
<strong>{{name}}</strong>

Can be thought of as a single, expanded template:

<h2>Names</h2>
{{#names}}
  <strong>{{name}}</strong>
{{/names}}

This is the spirit.

groue commented Apr 15, 2014

About free partials extraction, let me quote @defunkt on http://mustache.github.io/mustache.5.html

In this way you may want to think of partials as includes, or template expansion, even though it's not literally true.

For example, this template and partial:

base.mustache:
<h2>Names</h2>
{{#names}}
  {{> user}}
{{/names}}

user.mustache:
<strong>{{name}}</strong>

Can be thought of as a single, expanded template:

<h2>Names</h2>
{{#names}}
  <strong>{{name}}</strong>
{{/names}}

This is the spirit.

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Apr 15, 2014

Member

Allowing partials inside parent tags changes what would be a parse error ("unexpected section tag inside a parent tag!") into a runtime exception, because you can't be sure that a partial won't include something that isn't allowed directly inside a parent tag. It complicates things :)

Member

bobthecow commented Apr 15, 2014

Allowing partials inside parent tags changes what would be a parse error ("unexpected section tag inside a parent tag!") into a runtime exception, because you can't be sure that a partial won't include something that isn't allowed directly inside a parent tag. It complicates things :)

@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 15, 2014

Allowing partials inside parent tags changes what would be a parse error ("unexpected section tag inside a parent tag!") into a runtime exception, because you can't be sure that a partial won't include something that isn't allowed directly inside a parent tag. It complicates things :)

What runtime exception are you talking about? I'm not aware of runtime errors I should raise when rendering plain Mustache templates.

In GRMustache, the only runtime errors are for missing filters (a feature added by GRMustache, not in the spec: unknown filter f in {{ f(x) }}), and inconsistent mixing of HTML and text, a pretty rare programming error that can happen in GRMustache only (text templates are not in the spec: https://github.com/groue/GRMustache/blob/master/Guides/html_vs_text.md).

groue commented Apr 15, 2014

Allowing partials inside parent tags changes what would be a parse error ("unexpected section tag inside a parent tag!") into a runtime exception, because you can't be sure that a partial won't include something that isn't allowed directly inside a parent tag. It complicates things :)

What runtime exception are you talking about? I'm not aware of runtime errors I should raise when rendering plain Mustache templates.

In GRMustache, the only runtime errors are for missing filters (a feature added by GRMustache, not in the spec: unknown filter f in {{ f(x) }}), and inconsistent mixing of HTML and text, a pretty rare programming error that can happen in GRMustache only (text templates are not in the spec: https://github.com/groue/GRMustache/blob/master/Guides/html_vs_text.md).

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Apr 15, 2014

Member

If things like section tags are not allowed directly inside parent tags, this would be a parse error:

{{< parent }}
  {{# foo }}...{{/ foo }}
{{/ parent }}

But this would be a runtime error:

{{< parent }}
  {{> partial }}
{{/ parent }}

partial.mustache

{{# foo }}...{{/ foo }}
Member

bobthecow commented Apr 15, 2014

If things like section tags are not allowed directly inside parent tags, this would be a parse error:

{{< parent }}
  {{# foo }}...{{/ foo }}
{{/ parent }}

But this would be a runtime error:

{{< parent }}
  {{> partial }}
{{/ parent }}

partial.mustache

{{# foo }}...{{/ foo }}
@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 15, 2014

Well, in your last example, we would have a parse error because the {{> partial }} tag would be disallowed inside the {{< parent }}...{{/}} tag. So we still have no runtime error here.

groue commented Apr 15, 2014

Well, in your last example, we would have a parse error because the {{> partial }} tag would be disallowed inside the {{< parent }}...{{/}} tag. So we still have no runtime error here.

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Apr 15, 2014

Member

This thing you just said:

Well, in your last example, we would have a parse error because the {{> partial }} tag would be disallowed inside the {{< parent }}...{{/}} tag. So we still have no runtime error here.

Directly contradicts that other thing you keep saying:

if a given string is a valid template, then any substring that leaves open/close tags intact can be extracted to a partial, or an inheritable partial.

… and, in fact, agrees completely with what I'm trying to say. So maybe we misunderstand each other :)

Member

bobthecow commented Apr 15, 2014

This thing you just said:

Well, in your last example, we would have a parse error because the {{> partial }} tag would be disallowed inside the {{< parent }}...{{/}} tag. So we still have no runtime error here.

Directly contradicts that other thing you keep saying:

if a given string is a valid template, then any substring that leaves open/close tags intact can be extracted to a partial, or an inheritable partial.

… and, in fact, agrees completely with what I'm trying to say. So maybe we misunderstand each other :)

@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 15, 2014

@bobthecow, I think you mix two questions:

  • how are processed tags inside a inherited template (the famous white-list of white-space, change delimiters, comment, inheritable section)
  • the question raised by the GRMustache tests about partial extraction.

The two topics are different.

Your last example talks about the white-list. Since partial tags should raise a parsing exception, according to this white-list rule, this example would not raise any runtime exception.

About the second topic: please consider that all the partial-extraction tests are rendered by GRMustache. They work. They have a meaning. They test the fact that one can extract a partial from any template snippet, as long as the template snippet can be rendered, and that the two new templates are valid.

groue commented Apr 15, 2014

@bobthecow, I think you mix two questions:

  • how are processed tags inside a inherited template (the famous white-list of white-space, change delimiters, comment, inheritable section)
  • the question raised by the GRMustache tests about partial extraction.

The two topics are different.

Your last example talks about the white-list. Since partial tags should raise a parsing exception, according to this white-list rule, this example would not raise any runtime exception.

About the second topic: please consider that all the partial-extraction tests are rendered by GRMustache. They work. They have a meaning. They test the fact that one can extract a partial from any template snippet, as long as the template snippet can be rendered, and that the two new templates are valid.

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Apr 15, 2014

Member

I was, and have been, talking about the four tests with exceptions I cited above. They throw exceptions because they include tags that are not on the whitelist. I thought you were talking about them as well, and saying that they should be valid.

Member

bobthecow commented Apr 15, 2014

I was, and have been, talking about the four tests with exceptions I cited above. They throw exceptions because they include tags that are not on the whitelist. I thought you were talking about them as well, and saying that they should be valid.

@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 15, 2014

The three first tests are about choosing which overriding section wins, when several ones could be rendered. They make sure the last one wins. They do not involve the white-list.

The last test could raise an exception, with the white-list. Today, it just makes sure an regular section is ignored, and does not mess with inheritance.

groue commented Apr 15, 2014

The three first tests are about choosing which overriding section wins, when several ones could be rendered. They make sure the last one wins. They do not involve the white-list.

The last test could raise an exception, with the white-list. Today, it just makes sure an regular section is ignored, and does not mess with inheritance.

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Apr 15, 2014

Member

… partial tags should raise a parsing exception, according to this white-list rule…

All three of these raise exceptions, because they all have partials tags directly inside parent tags:

{{<partial1}}
  {{$inheritable}}1{{/inheritable}}
  {{>partial2}}
  {{$inheritable}}4{{/inheritable}}
{{/partial1}}
{{<partial1}}
  {{$inheritable}}1{{/inheritable}}
  {{>partial2}}
{{/partial1}}
{{<partial2}}
  {{>partial1}}
{{/partial2}}
Member

bobthecow commented Apr 15, 2014

… partial tags should raise a parsing exception, according to this white-list rule…

All three of these raise exceptions, because they all have partials tags directly inside parent tags:

{{<partial1}}
  {{$inheritable}}1{{/inheritable}}
  {{>partial2}}
  {{$inheritable}}4{{/inheritable}}
{{/partial1}}
{{<partial1}}
  {{$inheritable}}1{{/inheritable}}
  {{>partial2}}
{{/partial1}}
{{<partial2}}
  {{>partial1}}
{{/partial2}}
@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 15, 2014

OMG let me check my tests

groue commented Apr 15, 2014

OMG let me check my tests

@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 15, 2014

Ha yeah, I got it :-)

Those tests are a consequence of general partial extraction. Given the following template:

template.mustache
{{< parent }}
    {{$ inheritable}}...{{/ inheritable }}
{{/ parent }}

The user may want to extract the content into a partial:

template.mustache
{{< parent }}
    {{> partial }}
{{/ parent }}

partial.mustache
{{$ inheritable}}...{{/ inheritable }}

So if I come back to the white-list, I would state the partial tags should be white-listed as well.

groue commented Apr 15, 2014

Ha yeah, I got it :-)

Those tests are a consequence of general partial extraction. Given the following template:

template.mustache
{{< parent }}
    {{$ inheritable}}...{{/ inheritable }}
{{/ parent }}

The user may want to extract the content into a partial:

template.mustache
{{< parent }}
    {{> partial }}
{{/ parent }}

partial.mustache
{{$ inheritable}}...{{/ inheritable }}

So if I come back to the white-list, I would state the partial tags should be white-listed as well.

@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 15, 2014

The last example may look contrived. I have to imagine somebody who wants to reuse common inheritable sections for various layouts, and then extracts them into a partial.

groue commented Apr 15, 2014

The last example may look contrived. I have to imagine somebody who wants to reuse common inheritable sections for various layouts, and then extracts them into a partial.

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Apr 15, 2014

Member

Allowing partial tags in parent tags changes what would be a parse error ("unexpected section tag inside parent tag!") into a runtime error ("unexpected section tag inside partial tag included inside parent tag!").

If things like section tags are not allowed directly inside parent tags, this would be a parse error:

{{< parent }}
  {{# foo }}...{{/ foo }}
{{/ parent }}

But this would be a runtime error:

{{< parent }}
  {{> partial }}
{{/ parent }}

partial.mustache

{{# foo }}...{{/ foo }}
Member

bobthecow commented Apr 15, 2014

Allowing partial tags in parent tags changes what would be a parse error ("unexpected section tag inside parent tag!") into a runtime error ("unexpected section tag inside partial tag included inside parent tag!").

If things like section tags are not allowed directly inside parent tags, this would be a parse error:

{{< parent }}
  {{# foo }}...{{/ foo }}
{{/ parent }}

But this would be a runtime error:

{{< parent }}
  {{> partial }}
{{/ parent }}

partial.mustache

{{# foo }}...{{/ foo }}
@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 15, 2014

Your implementation may have limits, that GRMustache has not. Again, do as you want. I'm not the non-constructive guy, this time.

groue commented Apr 15, 2014

Your implementation may have limits, that GRMustache has not. Again, do as you want. I'm not the non-constructive guy, this time.

@groue

This comment has been minimized.

Show comment
Hide comment
@groue

groue Apr 16, 2014

As a temporary conclusion: we have four implementations which share a lot of common behaviors. It looks that GRMustache differs from mustache.php and hogan.js in a few aspects:

  • how to process ignored content inside a {{< parent }}...{{/parent}} tag. Some implementations ignore this ignored content, some others raise a parse error. The format of the mustache spec YAML tests does not allow for error testing. This question is thus, until the spec can test for errors, on hold.
  • how far goes the support for @defunkt's "you may want to think of partials as includes, or template expansion" definition of partials. GRMustache goes far, here. It supports partial extraction of the content of a {{< parent }}...{{/parent}} tag. The tests for this behavior can be found in the GRMustache repo, available for study for other implementations who would want support this feature.
  • how well is tested the separation of namespace between inheritable sections {{$ name }} and value tags {{ name }}, {{# name }}, {{^ name }}. GRMustache has tests for that.
  • how well is tested the resolution of the conflict between several multiple inheritable sections: {{< parent }}{{$ a }}content1{{/ a }}{{$ a }}content2{{/ a }}{{/ parent }}. GRMustache has tests for that, which enforce that the last section should win. GRMustache has also tests for that in the context of partial extraction: given {{< parent }}{{$ a }}content1{{/ a }}{{ > partial }}{{/ parent }} and partial.mustache: {{$ a }}content2{{/ a }}, the result should be content2.

Those feature proposals and tests are available for anybody in the community who would be interested in them.

groue commented Apr 16, 2014

As a temporary conclusion: we have four implementations which share a lot of common behaviors. It looks that GRMustache differs from mustache.php and hogan.js in a few aspects:

  • how to process ignored content inside a {{< parent }}...{{/parent}} tag. Some implementations ignore this ignored content, some others raise a parse error. The format of the mustache spec YAML tests does not allow for error testing. This question is thus, until the spec can test for errors, on hold.
  • how far goes the support for @defunkt's "you may want to think of partials as includes, or template expansion" definition of partials. GRMustache goes far, here. It supports partial extraction of the content of a {{< parent }}...{{/parent}} tag. The tests for this behavior can be found in the GRMustache repo, available for study for other implementations who would want support this feature.
  • how well is tested the separation of namespace between inheritable sections {{$ name }} and value tags {{ name }}, {{# name }}, {{^ name }}. GRMustache has tests for that.
  • how well is tested the resolution of the conflict between several multiple inheritable sections: {{< parent }}{{$ a }}content1{{/ a }}{{$ a }}content2{{/ a }}{{/ parent }}. GRMustache has tests for that, which enforce that the last section should win. GRMustache has also tests for that in the context of partial extraction: given {{< parent }}{{$ a }}content1{{/ a }}{{ > partial }}{{/ parent }} and partial.mustache: {{$ a }}content2{{/ a }}, the result should be content2.

Those feature proposals and tests are available for anybody in the community who would be interested in them.

@wheresrhys

This comment has been minimized.

Show comment
Hide comment
@wheresrhys

wheresrhys May 13, 2014

What's the status on this. Are there still ongoing discussions offline? Has it completely stalled?

wheresrhys commented May 13, 2014

What's the status on this. Are there still ongoing discussions offline? Has it completely stalled?

@jazzdan jazzdan referenced this pull request Jun 17, 2014

Open

Logic-less logic #77

@jazzdan

This comment has been minimized.

Show comment
Hide comment
@jazzdan

jazzdan Jul 19, 2014

Thanks for all the feedback everyone!

My goal with proposing this spec change wasn't to add new features to mustache. Instead, I was aiming to canonicalize a feature that already exists in four major implementations.

Adding a spec, no matter how minimal, for a feature that four out of the top ten popular mustache implementations support would be a good first step to a more healthy mustache ecosystem.

What's the best path forward to getting this merged?

jazzdan commented Jul 19, 2014

Thanks for all the feedback everyone!

My goal with proposing this spec change wasn't to add new features to mustache. Instead, I was aiming to canonicalize a feature that already exists in four major implementations.

Adding a spec, no matter how minimal, for a feature that four out of the top ten popular mustache implementations support would be a good first step to a more healthy mustache ecosystem.

What's the best path forward to getting this merged?

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Jul 19, 2014

Member

I don't know that it would get merged into the actual spec until v2, whenever that happens, but I think adding a proposed folder where we can document (and at least partially canonicalize) proposed specs for things like template inheritance, filters, and anchored dot notation makes a lot of sense sense. What do you think @pvande @locks @groue @janl?

Member

bobthecow commented Jul 19, 2014

I don't know that it would get merged into the actual spec until v2, whenever that happens, but I think adding a proposed folder where we can document (and at least partially canonicalize) proposed specs for things like template inheritance, filters, and anchored dot notation makes a lot of sense sense. What do you think @pvande @locks @groue @janl?

@thedeeno

This comment has been minimized.

Show comment
Hide comment
@thedeeno

thedeeno Sep 4, 2014

I don't know that it would get merged into the actual spec until v2, whenever that happens

Isn't the above proposal an enhacement? What part breaks the existing public api? Seems like this could be merged sooner.

.

Lack of template inheritence is primary drag on mustache adoption for me. I'd love to see this in the spec; and soon.

I feel like there's too much "what if" anlaysis going on in this (and other) threads. Clearly this is a useful feature that's being used in production and is actually supported by a few major implementations in a compatible way. Failure to merge the common parts of these feature into the spec makes me nervous that the mustache ecosystem is fragmenting - making the case for mustache weaker.

People may not actually get bitten by the edge cases above. Adding what's already here to the spec will help us see wider adoption (and likely improvement) of this feature over time.

Thanks @jazzdan for kick starting this and everyone else who's been deliberating here for trying to make mustache more awesome. That's just my 2cents.

thedeeno commented Sep 4, 2014

I don't know that it would get merged into the actual spec until v2, whenever that happens

Isn't the above proposal an enhacement? What part breaks the existing public api? Seems like this could be merged sooner.

.

Lack of template inheritence is primary drag on mustache adoption for me. I'd love to see this in the spec; and soon.

I feel like there's too much "what if" anlaysis going on in this (and other) threads. Clearly this is a useful feature that's being used in production and is actually supported by a few major implementations in a compatible way. Failure to merge the common parts of these feature into the spec makes me nervous that the mustache ecosystem is fragmenting - making the case for mustache weaker.

People may not actually get bitten by the edge cases above. Adding what's already here to the spec will help us see wider adoption (and likely improvement) of this feature over time.

Thanks @jazzdan for kick starting this and everyone else who's been deliberating here for trying to make mustache more awesome. That's just my 2cents.

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Sep 4, 2014

Member

It's not actually compatible, it restricts previously valid tag names and actually conflicts with some implementations (e.g. R) which rely on the currently acceptable tag names. It also redefines an existing Mustache tag ({{< foo }}) to mean "parent" instead of being a synonym for {{> foo }}.

Member

bobthecow commented Sep 4, 2014

It's not actually compatible, it restricts previously valid tag names and actually conflicts with some implementations (e.g. R) which rely on the currently acceptable tag names. It also redefines an existing Mustache tag ({{< foo }}) to mean "parent" instead of being a synonym for {{> foo }}.

@thedeeno

This comment has been minimized.

Show comment
Hide comment
@thedeeno

thedeeno Sep 4, 2014

@bobthecow Aha, I see. That wasn't obvious to me after reading a few thread on this. The < breaking change DOES seem unnecessary - especially since it's easy to just pick some other delimiter. Easy to fix for the spec though right?

I'm not sure I understand the 'restricting currently acceptable tag names' case. Wouldn't you only run into these restrictions if you're leveraging inheritence (meaning old templates won't break)?

thedeeno commented Sep 4, 2014

@bobthecow Aha, I see. That wasn't obvious to me after reading a few thread on this. The < breaking change DOES seem unnecessary - especially since it's easy to just pick some other delimiter. Easy to fix for the spec though right?

I'm not sure I understand the 'restricting currently acceptable tag names' case. Wouldn't you only run into these restrictions if you're leveraging inheritence (meaning old templates won't break)?

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Sep 4, 2014

Member

If you have a template with an interpolation tag {{$foo}} (which is allowed by the spec) it'll be treated as a block by a Mustache implementation which supports template inheritance. The same thing will happen no matter what the sigil you choose for any new tag. Because everything that's not explicitly used for something else is allowed by the spec, adding any new tag types to Mustache will break backwards compatibility.

Member

bobthecow commented Sep 4, 2014

If you have a template with an interpolation tag {{$foo}} (which is allowed by the spec) it'll be treated as a block by a Mustache implementation which supports template inheritance. The same thing will happen no matter what the sigil you choose for any new tag. Because everything that's not explicitly used for something else is allowed by the spec, adding any new tag types to Mustache will break backwards compatibility.

@nadako

This comment has been minimized.

Show comment
Hide comment
@nadako

nadako Apr 4, 2016

I just added support for this in hxmustache. It passes all spec tests.

nadako commented Apr 4, 2016

I just added support for this in hxmustache. It passes all spec tests.

@Danappelxx

This comment has been minimized.

Show comment
Hide comment
@Danappelxx

Danappelxx Sep 8, 2016

My Swift implementation, MuttonChop, also just implemented template inheritance (conforming to this spec). Maybe its time to push for Mustache v2 and make this official?

Danappelxx commented Sep 8, 2016

My Swift implementation, MuttonChop, also just implemented template inheritance (conforming to this spec). Maybe its time to push for Mustache v2 and make this official?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment