Adding dynamic partials, discussed in #54

[ghost]

The idea is to allow to dynamically load partials.

Brief example:
Data:

{
  content: "mainpage",
  world: "there"
}

Templates:

{{!template.mustache}}
{{*content}}

{{!mainpage.mustache}}
Hello {{world}}!

Result:

Hello there!

[jgonggrijp]

More specific context for who needs it: the discussion that led to this PR started exactly here.

Review @anomal00us:

In principle, I'm in favor of this spec, but there are a couple of things that I believe should be done before merging it:

• The spec should be made optional. That's just a matter of prepending a tilde ~ to the name of the spec, i.e., ~dynamic.yml. Actually, make that ~dynamic-partials.yml, for consistency with the other specs. (You could leave off the tilde if you feel that the spec should be mandatory, but in that case it can't be part of the spec until we bump the major version. That would seem a bit draconic for an extension that isn't very disruptive.)
• As currently proposed, the spec covers only half of the behavior of the {{*}} tag, i.e., the partial-like behavior. I think the other half should be specified as well, i.e., the part where it resolves a name in the context in the same way that an interpolation tag would. That should include things like dotted names, implicit iterators and function calls (note that function calls are part of the standard resolution algorithm even when the lambdas extension isn't implemented).
• I'd like to implement the feature myself (edit: done) and run it against the proposed spec in order to check for any hidden surprises. Only after the previous point is addressed, of course.
• I'd like to see at least one other implementation that passes the proposed spec (at least mostly, with a motivation for any omissions). If I'm right, you already implemented this feature yourself, so this point might be easy to address.

I made a few more minor comments below. Otherwise it looks good to me.

[jgonggrijp]

To emphasize the point, I think you could even do {{content}}<{{#nodes}}{{*template}}{{/nodes}}>. Although the current content of the partial is arguably more plausible, so I don't mind if you keep it this way, either.

[jgonggrijp]

Maybe nuance this sentence a bit by explicitly stating under which circumstances it applies.

[jgonggrijp]

Likewise.

[jgonggrijp]

Update after my previous comment: I just implemented the feature, so that won't be a bottleneck in the remainder of the review process.

[ghost]

• The spec should be made optional. That's just a matter of prepending a tilde ~ to the name of the spec, i.e., ~dynamic.yml. Actually, make that ~dynamic-partials.yml

Yes, that's a good point.

• As currently proposed, the spec covers only half of the behavior of the {{*}} tag, i.e., the partial-like behavior. I think the other half should be specified as well, i.e., the part where it resolves a name in the context in the same way that an interpolation tag would.

Okay so I just need to describe the name resolution in the context.
I'm working on it!

[gasche]

Is there a strong reason for using {{*foo}} instead of the alternative proposal: {{>*foo}}? I find the latter clearer (it's easier to guess what it does, or at least to get some idea), it could be extended to other forms if there was a clear case for it (possibly {{<*foo}} for example?), and it has the advantage of preserving the "syntactic space" of the main symbol for other usages if we wanted to.

[ghost]

Is there a strong reason for using {{*foo}} instead of the alternative proposal: {{>*foo}}?

The main reason was to keep the language simple and not introducing a general deference operator.
Take also look at this comment #54 (comment)
@jgonggrijp I think that active maintainers (like you, bobthecow and the others with more experience with mustache) should discuss or take poll in order to make a decision before we continue working on this PR.

Here I wrote a brief list of both options' pros and cons.
By one side I am intrigued by general dereference, it would make the language complete.
By the other side though I feel like it's unnecessary and too complex.

New general dereference operator

Pros:

• It's syntactically more consistent.
• Takes the language to a new level allowing way more complex (and dynamic) template renderings.

Cons:

• It's harder to implement: the engine tokens will need an extra field, the name resolution(s) have to be rewritten.
• May lead to poor programming choices (such as double or triple dereferences).
• A lot of specs should be updated to illustrate their behaviour with the dereferencing operator (?)
• It's probably an unnecessary over-engineering: I may be wrong but I noticed that everyone who has been asking for deferences aimed to be able to load dynamic partials

Dynamic partials

Pros:

• It's easy to implement.
• Keeps the language syntax simple.
• Solves the dynamic partial loading problem.

Cons:

• It's syntactically less consistent.
• Restricts the dereferences to one level ( no double/triple/+ dereferences) and always perform partial loading.
• Reserves the * operator.

What do you think?

[gasche]

Note: I am not suggesting that this specification should add support for {{#*foo}}, {{<*foo}}, etc. I understand that the proposal is limited to dynamic partials. But even if we restrict it to dynamic partials, the syntax could use {{>*foo}} instead of {{*foo}}, and I am asking why we would want to prefer {{*foo}}.

[jgonggrijp]

@gasche

My reasons for not wanting to have a general purpose dereference operator can be found here and here. Whether you find them strong is up to you. :-)

I think there doesn't need to be a dynamic parent because dynamic partials and parents are mostly alternatives; the former enable polymorphism through a tagged union like construct while the latter enable polymorphism through an inheritance like construct. As for preserving the * sigil for possible other future purposes, that's a point I'll concede, although I think it fits this particular feature well.

(This post crossed the previous two.)

[jgonggrijp]

@anomal00us

Dynamic partials

(...)

Cons:

• It's syntactically less consistent.

I don't think it's less consistent, just consistent in a different (and more limiting) way. It's perfectly consistent with the current practice of having a tag consisting of an opening delimiter, a single-character sigil, a content without much internal structure and finally a closing delimiter.

[gasche]

I think there doesn't need to be a dynamic parent because dynamic partials and parents are mostly alternatives; the former enable polymorphism through a tagged union like construct while the latter enable polymorphism through an inheritance like construct.

On the other hand, if you think of "parent templates" as "partials with parameters" (instead of an inheritance-like construct), then having those selected at runtime based on the data makes just as much sense as for partials.

Again: I'm not proposing this (or any other extension of the feature) for inclusion in the currently-proposed specification. But I see value in making a syntactic choice that leaves our options open in the future, if we found out that there are in fact valid use-cases (at acceptable complexity costs) for other forms of dynamic resolution.

[jgonggrijp]

@gasche I see your point, but I feel that {{>*dynamic.partial}} is a syntax we should avoid unless we also allow {{$*dynamic.block}} and {{#*dynamic.section}}. Allowing only the former would make a really inconsistent situation. It would also require overhauling an implementation to the point where you may just as well implement the general purpose dereference right away (at least, that is true of my own implementation).

As far as I'm concerned, there are only three flavors possible:

1. no dynamic partials in any form;
2. a dedicated sigil (such as *) that is yet another flavor of partial (note that < and $ are special flavors of partials, too);
3. a general purpose dereference operator that is allowed in all tags.

Number 3. would be a severely breaking change requiring a major version upgrade. Also, I would not want to use or implement such a Mustache 2.0 myself.

In my opinion, number 2. is not a very strange thing to do. Partials are a powerful and versatile language feature, so it makes sense to dedicate a relatively large share of the sigils to them. Consider also #63 (comment) and my immediate follow-up comment, which would introduce : as a dedicated sigil for yet another flavor of partial.

Number 1. would be a bit disappointing in my opinion, but not as disappointing as number 3.

[ghost]

But I see value in making a syntactic choice that leaves our options open in the future

I think that we have to decide this now, once for all.
The problem with >* syntax in this proposal is that >* has to be treated as a single token and many mustache implementations will have to rewrite parts of the tokenizer (like this https://github.com/bobthecow/mustache.php/blob/db2bc776c4d01665eb9b7e0e3259b0b9a2d20e84/src/Mustache/Tokenizer.php#L136 @bobthecow ) in order to support the only 2 characters long token prefix.
On the other side, if we go for a single character syntax then if in the future general dereferencing will be implemented we will have a redundancy (being the composition of > and the dereferencing tag)

[jgonggrijp]

@gasche As for parents as "partials with parameters": I subscribe to that way of thinking, but even when you think of it that way, dynamic partials are also an alternative way of passing parameters. You can replace parents and blocks entirely by dynamically resolved non-parametric partials, if you are happy with the inversion of control. Which you must be in order to want to use dynamic partials in the first place.

@anomal00us I think you make a valid point that if we do a dedicated * sigil in an optional extension to Mustache 1.x, this wouldn't stop us from introducing a general purpose * dereference operator in Mustache 2.x. A major version upgrade is going to be a breaking change anyway. Although, again, I wouldn't like to go there myself.

[ghost]

[ ] I'd like to see at least one other implementation that passes the proposed spec (at least mostly, with a motivation for any omissions). If I'm right, you already implemented this feature yourself, so this point might be easy to address.

At the moment my implementation isn't really compliant to mustache standards, regarding whitespaces and names (dotted names and objects) resolution, but for the rest it isn't that bad.
Here if you want to take a look for the dynamic partials: https://pastebin.com/ugk3BsEp

[gasche]

@jgonggrijp

dynamic partials are also an alternative way of passing parameters. You can replace parents and blocks entirely by dynamically resolved non-parametric partials, if you are happy with the inversion of control.

I'm not sure I see what you have in mind, can you show a sketch of the translation?

Allowing only the former would make a really inconsistent situation.

Personally I think that the situation would not be any worse than with the {{*foo}} syntax: the fact of the matter is that, with this specification, only >-style partials can be dynamically resolved. So we have ad-hoc support for dynamic resolution of one form only. Whether the syntax is ad-hoc or more regular does not change this. Personally I am fine with the ad-hoc-ness of it ("we only had clear use-cases for this one"), but I would prefer having a regular syntax, because I think it reads better -- it is easier for users to make a guess at what it does.

[ghost]

A major version upgrade is going to be a breaking change anyway.

Isn't it better if we plan things well now in order to have less breaking changes?
Anyway is there a timeline/planning or something for version 2?

[bobthecow]

Number 3. would be a severely breaking change requiring a major version upgrade.

IMO, neither {{>*foo}} nor {{*foo}} requires a major version upgrade if (1) they're implemented as optional, like lambdas, and (2) they're toggled via pragmas or library flags.

I believe the main concern is whether *foo collides with valid tag names in the rendering context. It collides just as much with a variable (in the {{*foo}} case) as a partial tag (in the {{>*foo}} case) so if one requires a major version, both do.

The problem with >* syntax in this proposal is that >* has to be treated as a single token and many mustache implementations will have to rewrite parts of the tokenizer

I haven't tried it yet, but I imagine it wouldn't be a two-character sigil; I'd implement it as a general purpose dereference operator, restricted to only work inside partial tags. It'd require another couple of transitions in the tokenizer state machine, but no more weird than, e.g., supporting the triple-mustache alt for &.

[bobthecow]

To limit the slipperiness of this slope, I'd be perfectly happy limiting it to a single dereference, which applies to the (resolved) dotted tag name in its entirety. So:

1. {{*foo.bar.baz}} resolves as {{ dereference(foo.bar.baz) }},
2. {{*foo | *bar}}, resolves as {{ dereference(bar)(dereference(foo)) }}, and
3. {{**foo}} and {{*foo.*bar}} are syntax errors.

[ghost]

I haven't tried it yet, but I imagine it wouldn't be a two-character sigil; I'd implement it as a general purpose dereference operator, restricted to only work inside partial tags.

Makes more sense yeah.
So if >* is just a combination of the partial operator > and the dereference operator * , should we update/close this PR and write a spec for the dereference operator?

[bobthecow]

This is my opinion but I think we're far from consensus still 😛

[jgonggrijp]

@anomal00us

At the moment my implementation isn't really compliant to mustache standards, regarding whitespaces and names (dotted names and objects) resolution, but for the rest it isn't that bad. Here if you want to take a look for the dynamic partials: https://pastebin.com/ugk3BsEp

Thanks. To guide my reading, could you mention the line numbers that specifically address dynamic partials?

[jgonggrijp]

@gasche

dynamic partials are also an alternative way of passing parameters. You can replace parents and blocks entirely by dynamically resolved non-parametric partials, if you are happy with the inversion of control.

I'm not sure I see what you have in mind, can you show a sketch of the translation?

Inheritance:

{{!parent.mustache}}
Hello, {{$block}}colleague{{/block}}.

{{!child.mustache}}
{{<parent}}{{$block}}friend{{/block}}{{/parent}}

Dynamic partials, same result:

{
    "relation": "job",
    "greet": "parent",
    "informal": {
        "relation": "social"
    }
}

{{!parent.mustache}}
Hello, {{*relation}}.

{{!child.mustache}}
{{#informal}}{{*greeting}}{{/informal}}

{{!job.mustache}}
colleague

{{!social.mustache}}
friend

[ghost]

Thanks. To guide my reading, could you mention the line numbers that specifically address dynamic partials?

Sure!
Lines 443-471 are the lines which "compile" partials and dynamic partials into code.
Dynamic partials differ by normal partials only by this line (457): $value = $args[$value];, which resolves the reference in the data.
Then, once the script has the partial name in $value it proceeds to load it, compile it and link it to the current template.

So it's pretty simple since it doesn't really handle complex objects.

For being even more clear here's a more detailed example:
Let's say a template is like this: {{*dynamic}}

// These are the arguments we render the partial with.
$args = array("dynamic" => "mainpage");

// This is the abstract syntax tree generated by the parser.
// I don't resolve constants for being more clear
$AST = array(
  0 => array(
    self::AST_TYPE = > self::TOKEN_TYPE_DYNAMIC_PARTIAL,
    self::AST_VALUE => "dynamic"
));

// When the compiler translates the AST into code, 
// we enter a the for loop on line 371 and at some point we will have
// $value = $AST[$i][self::AST_VALUE]; // = "dynamic"

// Now it should be more clear how this line resolves the reference
$value = $args[$value]; // = $args["dynamic"] = "mainpage"

[jgonggrijp]

@bobthecow

Number 3. would be a severely breaking change requiring a major version upgrade.

IMO, neither {{>*foo}} nor {{*foo}} requires a major version upgrade if (1) they're implemented as optional, like lambdas, and (2) they're toggled via pragmas or library flags.

I believe the main concern is whether *foo collides with valid tag names in the rendering context. It collides just as much with a variable (in the {{*foo}} case) as a partial tag (in the {{>*foo}} case) so if one requires a major version, both do.

Good point, agreed.

To limit the slipperiness of this slope, I'd be perfectly happy limiting it to a single dereference, which applies to the (resolved) dotted tag name in its entirety. So:

1. {{*foo.bar.baz}} resolves as {{ dereference(foo.bar.baz) }},
2. {{*foo | *bar}}, resolves as {{ dereference(bar)(dereference(foo)) }}, and
3. {{**foo}} and {{*foo.*bar}} are syntax errors.

It's ironic how you suggest restricting the dereference operator to a single level in order to limit the slipperiness of the slope, and immediately follow up by illustrating exactly what I'm worried about, i.e., more complexity inside the tag content in the form of a pipe operator. It reminds me of filters in Jinja and several other templating languages. I presume your implementation already supports that notation.

Even if we're only adding a single-level dereference operator now, it will set a precedent for people to want to add other notational refinements to the tag content. Your pipe operator might be a likely candidate. I could also think of direct arguments like in Handlebars helpers, or perhaps case modifiers. I'm not opposed to people adding such extensions to their own implementations, but if we start adding them to the spec, I feel that Mustache is giving up its logic-less soul. I feel that the extremely minimal syntax is what distinguishes Mustache from other template languages in the first place.

That doesn't mean that I want to limit the functionality of the language; I just want to uphold the clean, logic-less syntax. Filters, case modifiers, and even dereference operations could all also be achieved by making lambdas just slightly more powerful. All it requires is to pass a second argument to lambdas that accept it, containing the full context stack. This is an extension I've actually been meaning to suggest for a while; I'd call it "power lambdas". We don't need to discuss that now, though; I'm just pointing out that complicating tag contents is not the only way to make the language (even) more powerful.

Long story short: I don't mind if people add support for expression inside tag contents to their own implementations, but please let's keep them out of the spec. If that means that we don't specifiy any way to do dynamic partials, so be it.

[bobthecow]

It's ironic how you suggest restricting the dereference operator to a single level in order to limit the slipperiness of the slope, and immediately follow up by illustrating exactly what I'm worried about, i.e., more complexity inside the tag content in the form of a pipe operator. It reminds me of filters in Jinja and several other templating languages. I presume your implementation already supports that notation.

Right, sorry, I wasn't suggesting that this proposal include that feature, I was illustrating how that same principle would work in a feature (filters / pipe operators) which is an extension already present in some mustache implementations. Happy to remove that example if it makes things clearer 😛

[ghost]

Dereferencing would be a really nice thing to have.

I rewrote part of my script to see how it would look with a general dereference "operator".
On line 382 you can see how references are resolved (with only one dereferencing level) for all tokens: https://pastebin.com/SXiqNwB0
In this way we can have, as discussed earlier:

{{>*dynamic}}
{{*var}}

I thought it would have been worse. What do you think?

[jgonggrijp]

@bobthecow

Right, sorry, I wasn't suggesting that this proposal include that feature, I was illustrating how that same principle would work in a feature (filters / pipe operators) which is an extension already present in some mustache implementations. Happy to remove that example if it makes things clearer 😛

Please don't. :-) I understand that you didn't mean to suggest filter syntax, so you don't need to change your post. In fact, it perfectly illustrates how one operator can bring other in-tag expression syntax to mind. I'm just talking about associations here, and what they might do to future spec developments - if it crosses your mind now, it will cross other people's minds later as well.

[ghost]

Looks better now!
If you want to squash the commits even more just ping me

[gasche]

I'm in holidays in August with poor connectivity, don't wait for me :-)

[jgonggrijp]

@anomal00us I'm always happy to keep details in history, so no more squashing required as far as I'm concerned. If you feel like cleaning up some more, please let me know, otherwise I'll merge this.

Happy holiday, @gasche!

[ghost]

I'm fine with the current commits @jgonggrijp

Happy holidays @gasche !

[jgonggrijp]

Congratulations with your first contribution @anomal00us!

[bobthecow]

i don't know if it's a more recent addition to the spec, but the YAML parser I use for my spec test cases doesn't like implicit null values in JSON-like flow maps:

Malformed inline YAML string { foo: 'fizz', bar: 'buzz', fizz: { buzz: { 'content' } } }

can we change this to the equivalent:

data: { foo: 'fizz', bar: 'buzz', fizz: { buzz: { content: null } } }

(sorry i didn't catch this before 🙁)

[jgonggrijp]

No problem, fortunately I had not tagged a release yet. Should be fixed now on master.

[jgonggrijp]

@anomal00us I nearly forgot this: the manpage should be updated to discuss dynamic names. The core repo is no longer maintained, but you can submit a PR to my fork on the manpage-update-spec branch. If you want. Otherwise I can do it, too.

[ghost]

@anomal00us I nearly forgot this: the manpage should be updated to discuss dynamic names. The core repo is no longer maintained, but you can submit a PR to my fork on the manpage-update-spec branch. If you want. Otherwise I can do it, too.

I could work on that, I'd need some time though

[ghost]

Okay I opened a pull request on your fork: jgonggrijp/mustache#1

[agentgt]

@jgonggrijp it turns out this feature is incredibly useful to JStachio and might become one of its most powerful features except with difference of using sealed class instances and enums.

jstachio/jstachio#140

It basically is dispatching like you do in languages that support pattern matching.

[jgonggrijp]

@agentgt Thanks for notifying me. JStachio is outside my realm of daily experience but that makes it interesting. Happy to hear that you found a good way to integrate dynamic partials with it!